steveicarus / iverilog

Icarus Verilog
https://steveicarus.github.io/iverilog/
GNU General Public License v2.0
2.82k stars 523 forks source link

master hear build fails indirect macros #374

Closed Johnlon closed 3 years ago

Johnlon commented 4 years ago

Expected console output from the below is

 opt1
 opt2

But I get an error

indirect.v:10: warning: macro OPT1 undefined (and assumed null) at this point.
indirect.v:11: warning: macro OPT2 undefined (and assumed null) at this point.
indirect.v:11: syntax error
I give up.
ERROR exit code iverilog

Test case..

`define OPT1_DISPLAY $display("opt1");
`define OPT2_DISPLAY $display("opt2");

`define INDIRECT_OPT(OPTN) ```OPTN``_DISPLAY

module t;

  initial begin
    `INDIRECT_OPT(OPT1)
    `INDIRECT_OPT(OPT2)
  end
endmodule
Johnlon commented 4 years ago

This is a recent break.

I believe it works ok at bd0a1c75edc2644ec093c8e6940a4d0f6373f4b2 I picked that commit half randomly because the subsequent commit has a commit message that looked related to this function.

martinwhitaker commented 4 years ago

On investigation, other simulators agree with your expected behaviour. I could revert the change (`` delimiting a macro usage), but that gives a problem with this example:

`define PREFIX  my_prefix
`define SUFFIX  my_suffix

`define name1  `PREFIX``_```SUFFIX
`define name2(p,s)  p``_``s

`define stringify(text)  `"text`"

module test();

initial begin
  $display(`stringify(`name1));
  $display(`stringify(`name2(`PREFIX, `SUFFIX)));
end

endmodule

The IEEE standard states:

The macro text and argument defaults may contain usages of other text macros. Such usages shall be substituted after the outer macro is substituted, not when it is defined. If an actual argument or an argument default contains a macro usage, the macro usage shall be expanded only after substitution into the outer macro text.

from which I conclude both $display calls should print the same text. Other simulators vary in what they print for the first $display, but almost all print my_prefix_my_suffix for the second $display, which requires the `` to terminate the macro usage.

Johnlon commented 4 years ago

Hi Martin

I don't know what the answer is but this does break my sim so I down-graded. I've seen the same pattern that I've used in various other places (from where I nicked it) so I expect other breaks when folk upgrade.

Is there a tweak to get it to work again with different but reasonable syntax.

One consideration is the de facto standard vs de jura - is a flag an option?? I guess it might have to be on a per file basis though - no idea , complex.

JL

On Tue, 6 Oct 2020 at 17:10, martinwhitaker notifications@github.com wrote:

On investigation, other simulators agree with your expected behaviour. I could revert the change (`` delimiting a macro usage), but that gives a problem with this example:

define PREFIX my_prefix define SUFFIX my_suffix

define name1PREFIX_```SUFFIX `define name2(p,s) p_``s

define stringify(text)"text`"

module test();

initial begin $display(stringify(name1)); $display(stringify(name2(PREFIX,SUFFIX))); end

endmodule

The IEEE standard states:

The macro text and argument defaults may contain usages of other text macros. Such usages shall be substituted after the outer macro is substituted, not when it is defined. If an actual argument or an argument default contains a macro usage, the macro usage shall be expanded only after substitution into the outer macro text.

from which I conclude both $display calls should print the same text. Other simulators vary in what they print for the first $display, but almost all print my_prefix_my_suffix for the second $display, which requires the `` to terminate the macro usage.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/steveicarus/iverilog/issues/374#issuecomment-704386974, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGMFGEPIPOP3ZIAPPTOU6LSJM6PPANCNFSM4SDWA6FQ .

martinwhitaker commented 4 years ago

The best solution I can come up with is to mimic the behaviour of Verilator, best illustrated by an example:

module test();

`define T1     `TA``_b
`define T2(a)  `a``_b
`define T3(a)  a``_b

`define stringify(text) `"text`"

initial begin
  `define TA_b tb
  $display(`stringify(`T1));
  $display(`stringify(`T2(TA)));
  $display(`stringify(`T3(`TA)));
  `define TA ta
  $display(`stringify(`T1));
  $display(`stringify(`T2(TA)));
  $display(`stringify(`T3(`TA)));
  $finish;
end

endmodule

for which Verilator generates the following output:

tb
tb
tb
ta_b
ta_b
ta_b

i.e., when TA is not defined, you get the old Icarus behaviour (which I think matches a strict reading of the IEEE standard), and when TA is defined you get the current Icarus behaviour. Other simulators seem to have an inconsistent mix of the two behaviours.

martinwhitaker commented 3 years ago

Fixed in both the master and v11 branches.