Fix buggy memory allocation introduced in #5152:
1) clean up ast_stack to reflect AST node rearrangement when necessary,
to avoid dangling pointer;
2) call free_attr() on unused attribute list when no new syntax node is
created, to avoid leaking it.
* verilog: fix string literal regular expression.
A backslash was improperly quoted, causing string literal matching
to fail when the final token before a closing quote was an escaped
backslash.
* verilog: add regression test for string literal regex bug.
Test for bug triggered by escaped backslash immediately before
closing quote (introduced in ca7d94af and fixed by 40aa7eaf).
Use a greedy regular expression to match input inside a string
literal, so that flex can accumulate a longer match instead of
invoking a rule for each individual character.
There are two elements involved:
1) Apply the relevant full_case and/or parallel_case attribute(s) to
the generated AST_CASE node(s), so that the existing AST frontend and
subsequent passes will generate RTLIL with appropriate behaviour.
(This is handled in the parser "if_attr" non-terminal.)
2) Rearrange the AST_CASE structure when necessary. For "priority if"
(i.e., full_case), this requires only ensuring that directly nested
"else if" branches also inherit the full_case attribute. For
"unique if" and "unique0 if" (i.e., parallel_case+full_case and
parallel_case alone), there are two steps:
a) Flatten the AST_CASE structure such that any direct "else if"
branches are mapped to additional AST_CONDs in the parent;
b) Reverse the "direction" of the test: the constant 1 (true)
is provided in the AST_CASE node, and the expression(s) in the
if statement(s) are given in each AST_COND. This is necessary
because the constant 1, being the common factor, must occupy the
shared AST_CASE position.
(This is handled in the parser "TOK_IF" expansion of behavioral_stmt.)
Observe that:
* The generated AST has not been changed for bare "if"s (those
without unique/priority). This should minimise the risk of
unexpected regressions.
* It is possible that the flattening described in 2) a) above might
affect the behaviour of expressions with side effects in "unique if"
statements (consider "unique if( a ) ...; else if( b++ ) ...": if
a is true, is b incremented?). While it might be possible to provide
precise semantics here, IEEE 1800-2012 12.4.2 seems to be deliberately
vague ("In unique-if and unique0-if, the conditions may be evaluated
and compared in any order[...] The presence of side effects in
conditions may cause nondeterministic results.") and so it seems
doubtful that there is benefit in Yosys providing stronger promises
on the interpretation of questionable code.
Add support to the "read_verilog -sv" parser to validate the
"unique", "unique0", and "priority" keywords in contexts where
they're legal according to 1800-2012 12.4.2.
This affects only the grammar accepted; the behaviour of conditionals
is not changed. (But accepting this syntax will provide scope for
possible optimisations as future work.)
Three test cases ("unique_if", "unique_if_else", and
"unique_if_else_begin") verify that the keywords are accepted where
legal and rejected where illegal, as described in the final paragraph
of 12.4.2.
`read_verilog_file_list` should not try to read arguments as selection args. Without this, trying to pass a file without a `-f|-F` flag is misleading, in the best case giving a warning about the selection not matching any module, or in worst case just doing nothing (if the filename is a valid selection).
Consider this SystemVerilog file:
module top(...);
input clk;
input [7:0] data;
input ack;
always @(posedge clk)
if (ack) begin
assert(data != 8'h0a);
end
endmodule
Before this commit, the span for the assert was:
if (ack) begin>
assert(data != 8'h0a)<;
After this commit, the span for the assert is:
if (ack) begin
>assert(data != 8'h0a)<;
This helps editor integrations that only look at the beginning
of the span.
- Add support for assignments within expressions, e.g., `x[y++] = z;` or
`x = (y *= 2) - 1;`. The logic is handled entirely within the parser
by injecting statements into the current procedural block.
- Add support for pre-increment/decrement statements, which are
behaviorally equivalent to post-increment/decrement statements.
- Fix non-standard attribute position used for post-increment/decrement
statements.
(* nowrshmsk *) on a struct / union variable now affects dynamic
bit slice assignments to members of the struct / union.
(* nowrshmsk *) can in some cases yield significant resource savings; the
combination of pipeline shifting and indexed writes is an example of this.
Constructs similar to the one below can benefit from (* nowrshmsk *), and
in addition it is no longer necessary to split out the shift assignments
on separate lines in order to avoid the error message "ERROR: incompatible
mix of lookahead and non-lookahead IDs in LHS expression."
always_ff @(posedge clk) begin
if (rotate) begin
{ v5, v4, v3, v2, v1, v0 } <= { v4, v3, v2, v1, v0, v5 };
if (res) begin
v0.bytes <= '0;
end else if (w) begin
v0.bytes[addr] <= data;
end
end
end
The difference between void functions and tasks is that always_comb's
implicit sensitivity list behaves as if functions were inlined, but
ignores signals read only in tasks. This only matters for event based
simulation, and for synthesis we can treat a void function like a task.
The preprocessor currently destroys double slash containing escaped
identifiers (for example \a//b ). This is due to next_token trying to
convert single line comments (//) into /* */ comments. This then leads
to an unintuitive error message like this:
ERROR: syntax error, unexpected '*'
This patch fixes the error by recognizing escaped identifiers and
returning them as single token. It also adds a testcase.
Yosys works with bison 3.0 (or newer), but not bison 2.7 (the previous
release). Ideally, we would require "3" rather than "3.0" to give a
better error message, but bison 2.3, which still ships with macOS, does
not support major-only version requirements. With this change, building
with an outdated bison yields: `frontends/rtlil/rtlil_parser.y:25.10-14:
require bison 3.0, but have 2.3`.
This enables the usage of declarations of wand or wor with a base type
of logic, integer, or a typename. Note that declarations of nets with
2-state base types is still permitted, in violation of the spec.
- Root AST_PREFIX nodes are now subject to genblk expansion to allow
them to refer to a locally-visible generate block
- Part selects on AST_PREFIX member leafs can now refer to generate
block items (previously would not resolve and raise an error)
- Add source location information to AST_PREFIX nodes
This is accomplished by generating a unique name for the genvar,
renaming references to the genvar only in the loop's initialization,
guard, and incrementation, and finally adding a localparam inside the
loop body with the original name so that the genvar can be shadowed as
expected.
- User-defined types must be data types. Using a net type (e.g. wire) is
a syntax error.
- User-defined types without a net type are always variables (i.e.
logic).
- Nets and variables can now be explicitly declared using user-defined
types:
typedef logic [1:0] W;
wire W w;
typedef logic [1:0] V;
var V v;
Fixes#2846
This doesn't do anything useful yet: the patch just adds support for
the syntax to the lexer and parser and adds some tests to check the
syntax parses properly. This generates AST nodes, but doesn't yet
generate RTLIL.
Since our existing hierarchical_identifier parser doesn't allow bit
selects (so you can't do something like foo[1].bar[2].baz), I've also
not added support for a trailing bit select (the "constant_bit_select"
non-terminal in "bind_target_instance" in the spec). If we turn out to
need this in future, we'll want to augment hierarchical_identifier and
its other users too.
Note that you can't easily use the BNF from the spec:
bind_directive ::=
"bind" bind_target_scope [ : bind_target_instance_list]
bind_instantiation ;
| "bind" bind_target_instance bind_instantiation ;
even if you fix the lookahead problem, because code like this matches
both branches in the BNF:
bind a b b_i (.*);
The problem is that 'a' could either be a module name or a degenerate
hierarchical reference. This seems to be a genuine syntactic
ambiguity, which the spec resolves (p739) by saying that we have to
wait until resolution time (the hierarchy pass) and take whatever is
defined, treating 'a' as an instance name if it names both an instance
and a module.
To keep the parser simple, it currently accepts this invalid syntax:
bind a.b : c d e (.*);
This is invalid because we're in the first branch of the BNF above, so
the "a.b" term should match bind_target_scope: a module or interface
identifier, not an arbitrary hierarchical identifier.
This will fail in the hierarchy pass (when it's implemented in a
future patch).
- disallow [gen]blocks with an end label but not begin label
- check validity of module end label
- fix memory leak of package name and end label
- fix memory leak of module end label
This was actually a ticking UB bomb: after running the parser, the type
maps contain pointers to children of the current AST, which is
recursively deleted after the pass has executed. This leaves the
pointers in user_type_stack dangling, which just happened to never be a
problem due to another bug that causes typedefs from higher-level type
maps to never be considered.
Rebuilding the type stack from the design's globals ensures the AstNode
pointers are valid.
This breaks the ability to use a global typename as a standard
identifier in a subsequent input file. This is otherwise backwards
compatible, including for sources which previously included conflicting
typedefs in each input file.
- Modules with a parameter without a default value will be automatically
deferred until the hierarchy pass
- Allows for parameters without defaults as module items, rather than
just int the `parameter_port_list`, despite being forbidden in the LRM
- Check for parameters without defaults that haven't been overriden
- Add location info to parameter/localparam declarations
Declaring the ports as standard module items already worked as expected.
This adds a missing usage of `checkRange()` so that headers such as
`module m(output integer x);` now work correctly.
- Standard data declarations can now use any integer type
- Parameters and localparams can now use any integer type
- Function returns types can now use any integer type
- Fix `parameter logic`, `localparam reg`, etc. to be 1 bit (previously 32 bits)
- Added longint type (64 bits)
- Unified parser source for integer type widths
This would previously complain about an undefined internal macro if the
unapplied macro had not already been used. If it had, it would
incorrectly use the arguments from the previous invocation.
This is a somewhat obscure edge case I encountered while working on test
cases for earlier changes. Declarations in generate blocks should not be
checked against the list of ports. This change also adds a check
forbidding declarations within generate blocks being tagged as inputs or
outputs.
This change set contains a number of bug fixes and improvements related to
scoping and resolution in generate and procedural blocks. While many of the
frontend changes are interdependent, it may be possible bring the techmap
changes in under a separate PR.
Declarations within unnamed generate blocks previously encountered issues
because the data declarations were left un-prefixed, breaking proper scoping.
The LRM outlines behavior for generating names for unnamed generate blocks. The
original goal was to add this implicit labelling, but doing so exposed a number
of issues downstream. Additional testing highlighted other closely related scope
resolution issues, which have been fixed. This change also adds support for
block item declarations within unnamed blocks in SystemVerilog mode.
1. Unlabled generate blocks are now implicitly named according to the LRM in
`label_genblks`, which is invoked at the beginning of module elaboration
2. The Verilog parser no longer wraps explicitly named generate blocks in a
synthetic unnamed generate block to avoid creating extra hierarchy levels
where they should not exist
3. The techmap phase now allows special control identifiers to be used outside
of the topmost scope, which is necessary because such wires and cells often
appear in unlabeled generate blocks, which now prefix the declarations within
4. Some techlibs required modifications because they relied on the previous
invalid scope resolution behavior
5. `expand_genblock` has been simplified, now only expanding the outermost
scope, completely deferring the inspection and elaboration of nested scopes;
names are now resolved by looking in the innermost scope and stepping outward
6. Loop variables now always become localparams during unrolling, allowing them
to be resolved and shadowed like any other identifier
7. Identifiers in synthetic function call scopes are now prefixed and resolved
in largely the same manner as other blocks
before: `$func$\func_01$tests/simple/scopes.blk.v:60$5$\blk\x`
after: `\func_01$func$tests/simple/scopes.v:60$5.blk.x`
8. Support identifiers referencing a local generate scope nested more
than 1 level deep, i.e. `B.C.x` while within generate scope `A`, or using a
prefix of a current or parent scope, i.e. `B.C.D.x` while in `A.B`, `A.B.C`,
or `A.B.C.D`
9. Variables can now be declared within unnamed blocks in SystemVerilog mode
Addresses the following issues: 656, 2423, 2493
Rahul Bhagwat
Gary Wong
N. Engelhardt
garytwong
KrystalDelusion
George Rennie
Miodrag Milanovic
Jason Xu
Emil J. Tywoniak
rherveille
Dag Lem
Catherine
Zachary Snow
Charlotte
Jannis Harder
Thomas Sailer
Claire Xen
Marcelina Kościelnicka
Brett Witherspoon
Michael Singer
Rupert Swarbrick
Xiretza
TimRudy
Kamil Rakoczy
Lukasz Dalek
Tom Verbeure
Claire Xenia Wolf