Menu
▾
▴
iverilog-devel
|
From: Larry D. <ldo...@re...> - 2016-03-13 23:37:58
Attachments:
use_func.tar.gz
|
Friends -
See attached test case for VHDL functions, which fails because the
width of bit_counter comes out as 1, not 4. I'd like to thank
Maciej for huge strides towards making this work! This has turned
into a SystemVerilog question. With all the recent changes in
iverilog git master, I thought I should check if the problem is
still present. It is.
It's a pretty small tar file, 3122 bytes spread across 7 files.
If you bypass the use of next_highest_power_of_two() in use_func.vhd,
you get
$ make
iverilog -Wall -Wno-timescale -g2005-sv -civhdl.cfg -o use_func_tb use_func_tb.v use_func.vhd
vvp -n use_func_tb | awk -f testcode.awk
3 PASS
But with the buggy next_highest_power_of_two() call in place,
out-of-the-tarball, you get
$ make
iverilog -Wall -Wno-timescale -g2005-sv -civhdl.cfg -o use_func_tb use_func_tb.v use_func.vhd
vvp -n use_func_tb | awk -f testcode.awk
20 FAIL
make: *** [use_func_check] Error 1
Back on 28 Jan 2016, when I discussed this with Maciej,
he discovered that if you change the following line:
int \temp = \value ;
to
int \temp ; \temp = \value ;
in the generated code, function next_highest_power_of_two(), then it
works correctly. The question is if Icarus' SystemVerilog processing
should be fixed, or ivlpp should stop emitting broken SystemVerilog.
- Larry
|
|
From: Larry D. <ldo...@re...> - 2016-03-13 23:16:48
|
Friends -
On Sun, Mar 13, 2016 at 04:03:33PM -0700, Larry Doolittle wrote:
> The question is if Icarus' SystemVerilog processing
> should be fixed, or ivlpp should stop emitting broken SystemVerilog.
^^^^^
s/ivlpp/vhdlpp/ of course.
- Larry
|
|
From: Kevin C. <iv...@gr...> - 2016-03-14 07:15:20
|
Does anybody on this reflector belong to an IEEE-SA member company? Kev. |
|
From: Cary R. <cy...@ya...> - 2016-03-15 08:47:41
|
Yes, I work for an IEEE-SA member company. Cary On Monday, March 14, 2016 12:15 AM, Kevin Cameron <iv...@gr...> wrote: Does anybody on this reflector belong to an IEEE-SA member company? Kev. ------------------------------------------------------------------------------ Transform Data into Opportunity. Accelerate data analysis in your applications with Intel Data Analytics Acceleration Library. Click to learn more. http://pubads.g.doubleclick.net/gampad/clk?id=278785231&iu=/4140 _______________________________________________ Iverilog-devel mailing list Ive...@li... https://lists.sourceforge.net/lists/listinfo/iverilog-devel |
|
From: Martin W. <mai...@ma...> - 2016-03-14 20:30:09
|
Hello Larry,
Extracting the rounded_down_power_of_two function from the output of vhdlpp and converting to a
simple SystemVerilog test case gives:
module test();
function int rounded_down_power_of_two(input int value);
int n = 32'd0;
int temp = value;
while (temp > 32'd1) begin
temp = temp / 32'd2;
n = n + 32'd1;
end
return n;
endfunction
localparam value = rounded_down_power_of_two(34);
initial begin
$display("%d", value);
end
endmodule
This is illegal, because the standard says:
"Variables declared in a static task, function, or procedural block default to a static lifetime and
a local scope. However, an explicit static keyword shall be required when an initialization value is
specified as part of a static variable’s declaration to indicate the user’s intent of executing that
initialization only once at the beginning of simulation."
(at least the 2012 standard does - I haven't checked earlier versions).
It appears Icarus silently accepts static initialisation, and doesn't currently support
static/automatic overrides on variable declarations.
Declaring the function as automatic should work round this problem, but it appears this is broken too.
Martin
P.S. I think there is another issue with the converted function - vhdlpp has output unsigned literal
values, whereas I think they should be signed values.
Larry Doolittle wrote:
> Friends -
>
> See attached test case for VHDL functions, which fails because the
> width of bit_counter comes out as 1, not 4. I'd like to thank
> Maciej for huge strides towards making this work! This has turned
> into a SystemVerilog question. With all the recent changes in
> iverilog git master, I thought I should check if the problem is
> still present. It is.
>
> It's a pretty small tar file, 3122 bytes spread across 7 files.
> If you bypass the use of next_highest_power_of_two() in use_func.vhd,
> you get
> $ make
> iverilog -Wall -Wno-timescale -g2005-sv -civhdl.cfg -o use_func_tb use_func_tb.v use_func.vhd
> vvp -n use_func_tb | awk -f testcode.awk
> 3 PASS
> But with the buggy next_highest_power_of_two() call in place,
> out-of-the-tarball, you get
> $ make
> iverilog -Wall -Wno-timescale -g2005-sv -civhdl.cfg -o use_func_tb use_func_tb.v use_func.vhd
> vvp -n use_func_tb | awk -f testcode.awk
> 20 FAIL
> make: *** [use_func_check] Error 1
>
> Back on 28 Jan 2016, when I discussed this with Maciej,
> he discovered that if you change the following line:
> int \temp = \value ;
> to
> int \temp ; \temp = \value ;
> in the generated code, function next_highest_power_of_two(), then it
> works correctly. The question is if Icarus' SystemVerilog processing
> should be fixed, or ivlpp should stop emitting broken SystemVerilog.
>
> - Larry
>
>
>
> ------------------------------------------------------------------------------
> Transform Data into Opportunity.
> Accelerate data analysis in your applications with
> Intel Data Analytics Acceleration Library.
> Click to learn more.
> http://pubads.g.doubleclick.net/gampad/clk?id=278785111&iu=/4140
>
>
>
> _______________________________________________
> Iverilog-devel mailing list
> Ive...@li...
> https://lists.sourceforge.net/lists/listinfo/iverilog-devel
>
|
|
From: Larry D. <ldo...@re...> - 2016-03-14 20:47:53
|
Martin -
Thanks for looking into this. I'm not much of a SystemVerilog
standards guru.
On Mon, Mar 14, 2016 at 08:16:41PM +0000, Martin Whitaker wrote:
> Extracting the rounded_down_power_of_two function from the output of vhdlpp and converting to a
> simple SystemVerilog test case gives:
>
> module test();
>
> function int rounded_down_power_of_two(input int value);
> int n = 32'd0;
> int temp = value;
> while (temp > 32'd1) begin
> temp = temp / 32'd2;
> n = n + 32'd1;
> end
> return n;
> endfunction
>
> localparam value = rounded_down_power_of_two(34);
>
> initial begin
> $display("%d", value);
> end
>
> endmodule
>
> This is illegal, because [...]
Does the following version look better? It does print "5".
And Maciej, does it seem like it correctly captures the semantics
of the original VHDL?
module test();
function int rounded_down_power_of_two(input int value);
int n;
int temp;
n = 32'd0;
temp = value;
while (temp > 32'd1) begin
temp = temp / 32'd2;
n = n + 32'd1;
end
return n;
endfunction
localparam value = rounded_down_power_of_two(34);
initial begin
$display("%d", value);
end
endmodule
It would be easier for vhdlpp to emit
int n; n = 32'd0;
int temp; temp = value;
but that is not accepted by iverilog -g2005-sv.
- Larry
|
|
From: Martin W. <mai...@ma...> - 2016-03-14 21:27:40
|
Larry Doolittle wrote: > Martin - > > Thanks for looking into this. I'm not much of a SystemVerilog > standards guru. Nor am I really - I mostly use traditional Verilog. ... > Does the following version look better? It does print "5". > And Maciej, does it seem like it correctly captures the semantics > of the original VHDL? ... > function int rounded_down_power_of_two(input int value); > int n; > int temp; > n = 32'd0; > temp = value; > while (temp > 32'd1) begin > temp = temp / 32'd2; > n = n + 32'd1; > end > return n; > endfunction ... I think that's fine for this example. I'm not familiar with VHDL, but I'd guess it doesn't make variables static by default, so if a function contains any form of delay, vhdlpp should declare the emitted function to be automatic. > It would be easier for vhdlpp to emit > int n; n = 32'd0; > int temp; temp = value; > but that is not accepted by iverilog -g2005-sv. Yes, SystemVerilog is like C - you can't mix declarations and statements. Martin |
|
From: Larry D. <ldo...@re...> - 2016-03-14 20:56:20
|
Friends -
Just for fun, I added an additional test to this sample code.
module test();
function int rounded_down_power_of_two(input int value);
int n = 32'd0;
int temp;
//n = 32'd0;
temp = value;
while (temp > 32'd1) begin
temp = temp / 32'd2;
n = n + 32'd1;
end
return n;
endfunction
localparam value5 = rounded_down_power_of_two(34);
localparam value3 = rounded_down_power_of_two(8);
initial $display("%d %d", value5, value3);
endmodule
If n were considered static, I might expect this to print
5 8
but in fact n seems to be initialized on every invocation
(elaboration?) of rounded_down_power_of_two(), so it prints
5 3
Martin, does this fit your understanding of how SystemVerilog
is supposed to work?
- Larry
|
|
From: Martin W. <mai...@ma...> - 2016-03-14 21:48:19
|
Larry Doolittle wrote:
> Friends -
>
> Just for fun, I added an additional test to this sample code.
>
> module test();
> function int rounded_down_power_of_two(input int value);
> int n = 32'd0;
> int temp;
> //n = 32'd0;
> temp = value;
> while (temp > 32'd1) begin
> temp = temp / 32'd2;
> n = n + 32'd1;
> end
> return n;
> endfunction
> localparam value5 = rounded_down_power_of_two(34);
> localparam value3 = rounded_down_power_of_two(8);
> initial $display("%d %d", value5, value3);
> endmodule
>
> If n were considered static, I might expect this to print
> 5 8
> but in fact n seems to be initialized on every invocation
> (elaboration?) of rounded_down_power_of_two(), so it prints
> 5 3
> Martin, does this fit your understanding of how SystemVerilog
> is supposed to work?
Yes, because these are constant function calls, and there's a special rule for that:
"Constant function calls are evaluated at elaboration time. Their execution has no effect on the
initial values of the variables used either at simulation time or among multiple invocations of a
function at elaboration time. In each of these cases, the variables are initialized as they would be
for normal simulation."
If you try
$display("%d", rounded_down_power_of_two(34));
$display("%d", rounded_down_power_of_two(8));
you will get
5
8
Having said that, I've just tried changing the initial value of n to 1, and it has no effect. The
compiler doesn't seem to be generating the initialisation code. This is a bit of a surprise, as I
was looking at the vlog95 target code generator at the weekend, and it has code to handle this case.
Martin
|
|
From: Cary R. <cy...@ya...> - 2016-03-15 08:43:50
|
That vlog95 support could be for a few reasons. It used to work that way, I expected it to be supported since it is in the standard so added the appropriate support to match named blocks or something I don't remember. I'm not exactly sure which it was, but given the code/comments I would expect the first option since the comments are fairly explicit about how this should work for SV tasks/functions.
Cary
On Monday, March 14, 2016 2:48 PM, Martin Whitaker <mai...@ma...> wrote:
Larry Doolittle wrote:
> Friends -
>
> Just for fun, I added an additional test to this sample code.
>
> module test();
> function int rounded_down_power_of_two(input int value);
> int n = 32'd0;
> int temp;
> //n = 32'd0;
> temp = value;
> while (temp > 32'd1) begin
> temp = temp / 32'd2;
> n = n + 32'd1;
> end
> return n;
> endfunction
> localparam value5 = rounded_down_power_of_two(34);
> localparam value3 = rounded_down_power_of_two(8);
> initial $display("%d %d", value5, value3);
> endmodule
>
> If n were considered static, I might expect this to print
> 5 8
> but in fact n seems to be initialized on every invocation
> (elaboration?) of rounded_down_power_of_two(), so it prints
> 5 3
> Martin, does this fit your understanding of how SystemVerilog
> is supposed to work?
Yes, because these are constant function calls, and there's a special rule for that:
"Constant function calls are evaluated at elaboration time. Their execution has no effect on the
initial values of the variables used either at simulation time or among multiple invocations of a
function at elaboration time. In each of these cases, the variables are initialized as they would be
for normal simulation."
If you try
$display("%d", rounded_down_power_of_two(34));
$display("%d", rounded_down_power_of_two(8));
you will get
5
8
Having said that, I've just tried changing the initial value of n to 1, and it has no effect. The
compiler doesn't seem to be generating the initialisation code. This is a bit of a surprise, as I
was looking at the vlog95 target code generator at the weekend, and it has code to handle this case.
Martin
------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785231&iu=/4140
_______________________________________________
Iverilog-devel mailing list
Ive...@li...
https://lists.sourceforge.net/lists/listinfo/iverilog-devel
|
|
From: Martin W. <mai...@ma...> - 2016-03-15 19:56:55
|
Cary R. wrote: > That vlog95 support could be for a few reasons. It used to work that way, I expected it to be > supported since it is in the standard so added the appropriate support to match named blocks or > something I don't remember. I'm not exactly sure which it was, but given the code/comments I > would expect the first option since the comments are fairly explicit about how this should work > for SV tasks/functions. It appears that the compiler generates the initialisation code for tasks, but not for functions. There's a test in the test suite that checks the behaviour for tasks, which is no doubt why you supported this in the vlog95 target. Martin |
|
From: Cary R. <cy...@ya...> - 2016-03-18 00:25:17
|
That sounds like a good explanation to me. I'm hoping very soon that work gets back to a more normal schedule so I an start to think about Icarus again. Cary On Tuesday, March 15, 2016 12:56 PM, Martin Whitaker <mai...@ma...> wrote: Cary R. wrote: > That vlog95 support could be for a few reasons. It used to work that way, I expected it to be > supported since it is in the standard so added the appropriate support to match named blocks or > something I don't remember. I'm not exactly sure which it was, but given the code/comments I > would expect the first option since the comments are fairly explicit about how this should work > for SV tasks/functions. It appears that the compiler generates the initialisation code for tasks, but not for functions. There's a test in the test suite that checks the behaviour for tasks, which is no doubt why you supported this in the vlog95 target. Martin |
|
From: Maciej S. <mac...@ce...> - 2016-03-15 15:14:18
Attachments:
signature.asc
|
Hi, Martin, thank you for the explanations. You are right, VHDL subprograms are automatic by default. Larry, there is a branch [1] that produces code in the way you have suggested. I believe the semantics is correct here. I had tried simply adding 'automatic' keyword to the function declaration, but it did not help. I am going to include the branch in the next pull request. Regards, Orson 1. https://github.com/orsonmmz/iverilog/tree/automatic On 03/14/2016 09:47 PM, Larry Doolittle wrote: > Martin - > > Thanks for looking into this. I'm not much of a SystemVerilog > standards guru. > > On Mon, Mar 14, 2016 at 08:16:41PM +0000, Martin Whitaker wrote: >> Extracting the rounded_down_power_of_two function from the output of vhdlpp and converting to a >> simple SystemVerilog test case gives: >> >> module test(); >> >> function int rounded_down_power_of_two(input int value); >> int n = 32'd0; >> int temp = value; >> while (temp > 32'd1) begin >> temp = temp / 32'd2; >> n = n + 32'd1; >> end >> return n; >> endfunction >> >> localparam value = rounded_down_power_of_two(34); >> >> initial begin >> $display("%d", value); >> end >> >> endmodule >> >> This is illegal, because [...] > > Does the following version look better? It does print "5". > And Maciej, does it seem like it correctly captures the semantics > of the original VHDL? > > module test(); > > function int rounded_down_power_of_two(input int value); > int n; > int temp; > n = 32'd0; > temp = value; > while (temp > 32'd1) begin > temp = temp / 32'd2; > n = n + 32'd1; > end > return n; > endfunction > > localparam value = rounded_down_power_of_two(34); > > initial begin > $display("%d", value); > end > > endmodule > > It would be easier for vhdlpp to emit > int n; n = 32'd0; > int temp; temp = value; > but that is not accepted by iverilog -g2005-sv. > > - Larry > > ------------------------------------------------------------------------------ > Transform Data into Opportunity. > Accelerate data analysis in your applications with > Intel Data Analytics Acceleration Library. > Click to learn more. > http://pubads.g.doubleclick.net/gampad/clk?id=278785231&iu=/4140 > _______________________________________________ > Iverilog-devel mailing list > Ive...@li... > https://lists.sourceforge.net/lists/listinfo/iverilog-devel > |
|
From: Martin W. <mai...@ma...> - 2016-03-17 08:53:53
|
Maciej Sumiński wrote: > Hi, > > Martin, thank you for the explanations. You are right, VHDL subprograms > are automatic by default. > > Larry, there is a branch [1] that produces code in the way you have > suggested. I believe the semantics is correct here. > > I had tried simply adding 'automatic' keyword to the function > declaration, but it did not help. I am going to include the branch in > the next pull request. I have a fix for variable initialisation in automatic functions (and tasks), which I will push once I've fixed some of the other issues. Martin |
|
From: Martin W. <mai...@ma...> - 2016-03-19 21:33:34
|
Martin Whitaker wrote:
> I have a fix for variable initialisation in automatic functions (and tasks), which I will push once
> I've fixed some of the other issues.
I have pushed the fixes for these issues. Also included are:
- if you select a SystemVerilog generation, variable initialisation
specified as part of the declaration is now performed before any
initial/always process is started (as required by the SystemVerilog
standard)
- default subroutine lifetimes are supported (in module/program/
interface/package/class declarations)
- tasks and functions can be explicitly declared as static
- variables can be explicitly declared as static or automatic,
although the compiler currently throws an error if the declaration
is different to the default value
- a warning is generated if static variables in tasks/functions/blocks
are initialised in their declaration but not explicitly declared as
static (IEEE1800 actually requires this to be an error, but I've
made it a warning for now).
Do we want to backport these changes to v10? Currently v10 silently fails to run the variable
initialisation in many cases.
@Orson - two of the VHDL tests now fail. To fix them, vhdlpp needs to explicitly declare variables
inside blocks as static when the declaration includes an initialisation expression.
|
|
From: Maciej S. <mac...@ce...> - 2016-03-22 15:06:44
Attachments:
signature.asc
|
On 03/19/2016 10:33 PM, Martin Whitaker wrote: > Martin Whitaker wrote: [snip] > @Orson - two of the VHDL tests now fail. To fix them, vhdlpp needs to explicitly declare variables > inside blocks as static when the declaration includes an initialisation expression. Hi Martin, I presume you talk about vhdl_string & vhdl_image_attr tests. The variables causing warnings are initialized inside a module scope. If I understand the standard correctly (6.21 @ 1800-2012), it is not required to explicitly declare such variables as static, because it is illegal for them to be automatic (see an example on p.91 @ 1800-2012). Anyway, if you would rather vhdlpp emit lifetime for each variable, I pushed a fix to automatic_rebased branch [1] on github. It also contains patches to make Larry's use_func test case work and passes all the tests, so feel free to merge it. Regards, Orson 1. https://github.com/orsonmmz/iverilog/tree/automatic_rebased |
|
From: Martin W. <mai...@ma...> - 2016-03-22 19:30:11
|
Hi Orson,
Maciej Sumiński wrote:
> I presume you talk about vhdl_string & vhdl_image_attr tests. The
> variables causing warnings are initialized inside a module scope. If I
> understand the standard correctly (6.21 @ 1800-2012), it is not required
> to explicitly declare such variables as static, because it is illegal
> for them to be automatic (see an example on p.91 @ 1800-2012).
Yes, those are the tests I was talking about. Taking vhdl_image_attr as an example, the relevant
part of the vhdlpp output is:
always begin : __scope_1
bit[32'd7:32'd0] var_char = "o";
int var_int = 32'd10;
real var_real = 12.34;
time var_time = 10ns;
...
These variables are declared inside a begin/end block, so SystemVerilog permits them to be declared
as static or automatic, hence the rule about having an explicit static keyword applies. The
equivalent example on page 91 of 1800-2012 is
initial begin
int svar2 = 2; // static/automatic needed to show intent
...
To be sure I'm reading the standard correctly, I tried this example
module test();
integer legal = 1;
initial begin: named_block
integer illegal = 1;
end
endmodule
on another simulator. It accepted the declaration of 'legal' and warned that the declaration of
'illegal' required an explicit 'static' keyword.
> Anyway, if you would rather vhdlpp emit lifetime for each variable, I
> pushed a fix to automatic_rebased branch [1] on github. It also contains
> patches to make Larry's use_func test case work and passes all the
> tests, so feel free to merge it.
I'll leave this for Steve.
Regards,
Martin
|
|
From: Maciej S. <mac...@ce...> - 2016-03-22 19:58:40
Attachments:
signature.asc
|
Hi Martin, I see your point, I had overlooked the always block. I agree it has to be changed, and patches in the automatic_rebased branch should fix the problem. Regards, Orson On 03/22/2016 08:30 PM, Martin Whitaker wrote: > Hi Orson, > > Maciej Sumiński wrote: >> I presume you talk about vhdl_string & vhdl_image_attr tests. The >> variables causing warnings are initialized inside a module scope. If I >> understand the standard correctly (6.21 @ 1800-2012), it is not required >> to explicitly declare such variables as static, because it is illegal >> for them to be automatic (see an example on p.91 @ 1800-2012). > > Yes, those are the tests I was talking about. Taking vhdl_image_attr as an example, the relevant > part of the vhdlpp output is: > > always begin : __scope_1 > bit[32'd7:32'd0] var_char = "o"; > int var_int = 32'd10; > real var_real = 12.34; > time var_time = 10ns; > ... > > These variables are declared inside a begin/end block, so SystemVerilog permits them to be declared > as static or automatic, hence the rule about having an explicit static keyword applies. The > equivalent example on page 91 of 1800-2012 is > > initial begin > int svar2 = 2; // static/automatic needed to show intent > ... > > To be sure I'm reading the standard correctly, I tried this example > > module test(); > > integer legal = 1; > > initial begin: named_block > integer illegal = 1; > end > > endmodule > > on another simulator. It accepted the declaration of 'legal' and warned that the declaration of > 'illegal' required an explicit 'static' keyword. > >> Anyway, if you would rather vhdlpp emit lifetime for each variable, I >> pushed a fix to automatic_rebased branch [1] on github. It also contains >> patches to make Larry's use_func test case work and passes all the >> tests, so feel free to merge it. > > I'll leave this for Steve. > > Regards, > > Martin > > ------------------------------------------------------------------------------ > Transform Data into Opportunity. > Accelerate data analysis in your applications with > Intel Data Analytics Acceleration Library. > Click to learn more. > http://pubads.g.doubleclick.net/gampad/clk?id=278785351&iu=/4140 > _______________________________________________ > Iverilog-devel mailing list > Ive...@li... > https://lists.sourceforge.net/lists/listinfo/iverilog-devel > |
|
From: Stephen W. <st...@ic...> - 2016-03-22 20:22:09
|
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Sounds good, send me a pull request when you feel ready. On 03/22/2016 12:58 PM, Maciej Sumi?ski wrote: > Hi Martin, > > I see your point, I had overlooked the always block. I agree it has > to be changed, and patches in the automatic_rebased branch should > fix the problem. > > Regards, Orson > > On 03/22/2016 08:30 PM, Martin Whitaker wrote: >> Hi Orson, >> >> Maciej Sumi?ski wrote: >>> I presume you talk about vhdl_string & vhdl_image_attr tests. >>> The variables causing warnings are initialized inside a module >>> scope. If I understand the standard correctly (6.21 @ >>> 1800-2012), it is not required to explicitly declare such >>> variables as static, because it is illegal for them to be >>> automatic (see an example on p.91 @ 1800-2012). >> >> Yes, those are the tests I was talking about. Taking >> vhdl_image_attr as an example, the relevant part of the vhdlpp >> output is: >> >> always begin : __scope_1 bit[32'd7:32'd0] var_char = "o"; int >> var_int = 32'd10; real var_real = 12.34; time var_time = >> 10ns; ... >> >> These variables are declared inside a begin/end block, so >> SystemVerilog permits them to be declared as static or automatic, >> hence the rule about having an explicit static keyword applies. >> The equivalent example on page 91 of 1800-2012 is >> >> initial begin int svar2 = 2; // static/automatic needed to show >> intent ... >> >> To be sure I'm reading the standard correctly, I tried this >> example >> >> module test(); >> >> integer legal = 1; >> >> initial begin: named_block integer illegal = 1; end >> >> endmodule >> >> on another simulator. It accepted the declaration of 'legal' and >> warned that the declaration of 'illegal' required an explicit >> 'static' keyword. >> >>> Anyway, if you would rather vhdlpp emit lifetime for each >>> variable, I pushed a fix to automatic_rebased branch [1] on >>> github. It also contains patches to make Larry's use_func test >>> case work and passes all the tests, so feel free to merge it. >> >> I'll leave this for Steve. >> >> Regards, >> >> Martin >> >> ------------------------------------------------------------------------------ >> >> Transform Data into Opportunity. >> Accelerate data analysis in your applications with Intel Data >> Analytics Acceleration Library. Click to learn more. >> http://pubads.g.doubleclick.net/gampad/clk?id=278785351&iu=/4140 >> _______________________________________________ Iverilog-devel >> mailing list Ive...@li... >> https://lists.sourceforge.net/lists/listinfo/iverilog-devel >> > > > > ------------------------------------------------------------------------------ > > Transform Data into Opportunity. > Accelerate data analysis in your applications with Intel Data > Analytics Acceleration Library. Click to learn more. > http://pubads.g.doubleclick.net/gampad/clk?id=278785351&iu=/4140 > > > > _______________________________________________ Iverilog-devel > mailing list Ive...@li... > https://lists.sourceforge.net/lists/listinfo/iverilog-devel > - -- Steve Williams "The woods are lovely, dark and deep. steve at icarus.com But I have promises to keep, http://www.icarus.com and lines to code before I sleep, http://www.picturel.com And lines to code before I sleep." -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iEYEARECAAYFAlbxqWkACgkQrPt1Sc2b3ing5ACfTHwD/TJB49cXB8lK047Qtg7W go4AoNcAAoZogA7GuqcUEdaM2H2I/PCx =MyTm -----END PGP SIGNATURE----- |
|
From: Maciej S. <mac...@ce...> - 2016-03-29 17:02:58
|
Hi, I have just created a pull request for the mentioned patches [1]. The branch is rebased on the current master. I have also found a minor bug, that must have been hard to trigger and observe. During net resolution (compile_cleanup()@vvp/compile.cc), nets are assigned the last observed scope (aka current_scope), which is not always the real parent scope. The bug can be triggered by running the attached example: ---------------- vpi error: cannot place value change callback on automatically allocated variable 'clk' ---------------- even though clk should belong to 'auto_scope' module, which is a static scope. Upon closer inspection it has 'strange_label' scope assigned, as it was the last .scope directive in the a.out file. It is enough to comment out the scope label to make it run properly. I have fixed it by simply correcting the scope after a net is created. I am not sure if that is the most elegant way of solving the problem, but I hope it helps to demonstrate the issue. The patch is not included in the pull request. Regards, Orson 1. https://github.com/steveicarus/iverilog/pull/95 On 03/22/2016 09:22 PM, Stephen Williams wrote: > > Sounds good, send me a pull request when you feel ready. > > On 03/22/2016 12:58 PM, Maciej Sumi?ski wrote: >> Hi Martin, > >> I see your point, I had overlooked the always block. I agree it has >> to be changed, and patches in the automatic_rebased branch should >> fix the problem. > >> Regards, Orson > >> On 03/22/2016 08:30 PM, Martin Whitaker wrote: >>> Hi Orson, >>> >>> Maciej Sumi?ski wrote: >>>> I presume you talk about vhdl_string & vhdl_image_attr tests. >>>> The variables causing warnings are initialized inside a module >>>> scope. If I understand the standard correctly (6.21 @ >>>> 1800-2012), it is not required to explicitly declare such >>>> variables as static, because it is illegal for them to be >>>> automatic (see an example on p.91 @ 1800-2012). >>> >>> Yes, those are the tests I was talking about. Taking >>> vhdl_image_attr as an example, the relevant part of the vhdlpp >>> output is: >>> >>> always begin : __scope_1 bit[32'd7:32'd0] var_char = "o"; int >>> var_int = 32'd10; real var_real = 12.34; time var_time = >>> 10ns; ... >>> >>> These variables are declared inside a begin/end block, so >>> SystemVerilog permits them to be declared as static or automatic, >>> hence the rule about having an explicit static keyword applies. >>> The equivalent example on page 91 of 1800-2012 is >>> >>> initial begin int svar2 = 2; // static/automatic needed to show >>> intent ... >>> >>> To be sure I'm reading the standard correctly, I tried this >>> example >>> >>> module test(); >>> >>> integer legal = 1; >>> >>> initial begin: named_block integer illegal = 1; end >>> >>> endmodule >>> >>> on another simulator. It accepted the declaration of 'legal' and >>> warned that the declaration of 'illegal' required an explicit >>> 'static' keyword. >>> >>>> Anyway, if you would rather vhdlpp emit lifetime for each >>>> variable, I pushed a fix to automatic_rebased branch [1] on >>>> github. It also contains patches to make Larry's use_func test >>>> case work and passes all the tests, so feel free to merge it. >>> >>> I'll leave this for Steve. >>> >>> Regards, >>> >>> Martin >>> >>> ------------------------------------------------------------------------------ >>> >>> > Transform Data into Opportunity. >>> Accelerate data analysis in your applications with Intel Data >>> Analytics Acceleration Library. Click to learn more. >>> http://pubads.g.doubleclick.net/gampad/clk?id=278785351&iu=/4140 >>> _______________________________________________ Iverilog-devel >>> mailing list Ive...@li... >>> https://lists.sourceforge.net/lists/listinfo/iverilog-devel >>> > > > >> ------------------------------------------------------------------------------ > > > Transform Data into Opportunity. >> Accelerate data analysis in your applications with Intel Data >> Analytics Acceleration Library. Click to learn more. >> http://pubads.g.doubleclick.net/gampad/clk?id=278785351&iu=/4140 > > > >> _______________________________________________ Iverilog-devel >> mailing list Ive...@li... >> https://lists.sourceforge.net/lists/listinfo/iverilog-devel > > > > > ------------------------------------------------------------------------------ > Transform Data into Opportunity. > Accelerate data analysis in your applications with > Intel Data Analytics Acceleration Library. > Click to learn more. > http://pubads.g.doubleclick.net/gampad/clk?id=278785351&iu=/4140 > _______________________________________________ > Iverilog-devel mailing list > Ive...@li... > https://lists.sourceforge.net/lists/listinfo/iverilog-devel > |
|
From: Maciej S. <mac...@ce...> - 2016-05-02 12:30:42
Attachments:
signature.asc
|
Hi, Just a gentle reminder, so the issue does not get completely lost. Any comment is welcome. I may rebase the pull request if necessary. Regards, Orson On 03/29/2016 06:46 PM, Maciej Sumiński wrote: > Hi, > > I have just created a pull request for the mentioned patches [1]. The > branch is rebased on the current master. > > I have also found a minor bug, that must have been hard to trigger and > observe. During net resolution (compile_cleanup()@vvp/compile.cc), nets > are assigned the last observed scope (aka current_scope), which is not > always the real parent scope. > > The bug can be triggered by running the attached example: > ---------------- > vpi error: cannot place value change callback on automatically allocated > variable 'clk' > ---------------- > even though clk should belong to 'auto_scope' module, which is a static > scope. Upon closer inspection it has 'strange_label' scope assigned, as > it was the last .scope directive in the a.out file. It is enough to > comment out the scope label to make it run properly. > > I have fixed it by simply correcting the scope after a net is created. I > am not sure if that is the most elegant way of solving the problem, but > I hope it helps to demonstrate the issue. The patch is not included in > the pull request. > > Regards, > Orson > > 1. https://github.com/steveicarus/iverilog/pull/95 > > On 03/22/2016 09:22 PM, Stephen Williams wrote: >> >> Sounds good, send me a pull request when you feel ready. >> >> On 03/22/2016 12:58 PM, Maciej Sumi?ski wrote: >>> Hi Martin, >> >>> I see your point, I had overlooked the always block. I agree it has >>> to be changed, and patches in the automatic_rebased branch should >>> fix the problem. >> >>> Regards, Orson >> >>> On 03/22/2016 08:30 PM, Martin Whitaker wrote: >>>> Hi Orson, >>>> >>>> Maciej Sumi?ski wrote: >>>>> I presume you talk about vhdl_string & vhdl_image_attr tests. >>>>> The variables causing warnings are initialized inside a module >>>>> scope. If I understand the standard correctly (6.21 @ >>>>> 1800-2012), it is not required to explicitly declare such >>>>> variables as static, because it is illegal for them to be >>>>> automatic (see an example on p.91 @ 1800-2012). >>>> >>>> Yes, those are the tests I was talking about. Taking >>>> vhdl_image_attr as an example, the relevant part of the vhdlpp >>>> output is: >>>> >>>> always begin : __scope_1 bit[32'd7:32'd0] var_char = "o"; int >>>> var_int = 32'd10; real var_real = 12.34; time var_time = >>>> 10ns; ... >>>> >>>> These variables are declared inside a begin/end block, so >>>> SystemVerilog permits them to be declared as static or automatic, >>>> hence the rule about having an explicit static keyword applies. >>>> The equivalent example on page 91 of 1800-2012 is >>>> >>>> initial begin int svar2 = 2; // static/automatic needed to show >>>> intent ... >>>> >>>> To be sure I'm reading the standard correctly, I tried this >>>> example >>>> >>>> module test(); >>>> >>>> integer legal = 1; >>>> >>>> initial begin: named_block integer illegal = 1; end >>>> >>>> endmodule >>>> >>>> on another simulator. It accepted the declaration of 'legal' and >>>> warned that the declaration of 'illegal' required an explicit >>>> 'static' keyword. >>>> >>>>> Anyway, if you would rather vhdlpp emit lifetime for each >>>>> variable, I pushed a fix to automatic_rebased branch [1] on >>>>> github. It also contains patches to make Larry's use_func test >>>>> case work and passes all the tests, so feel free to merge it. >>>> >>>> I'll leave this for Steve. >>>> >>>> Regards, >>>> >>>> Martin |
Thanks for helping keep SourceForge clean.
X