Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conditional event controls ("iff") #1482

Closed
veripoolbot opened this issue Jul 26, 2019 · 15 comments
Closed

Conditional event controls ("iff") #1482

veripoolbot opened this issue Jul 26, 2019 · 15 comments
Labels
area: assertions Issue involves assertions effort: days Expect this issue to require roughly days of invested effort to resolve type: feature-IEEE Request to add new feature, described in IEEE 1800

Comments

@veripoolbot
Copy link
Contributor


Author Name: Paul Donahue
Original Redmine Issue: 1482 from https://www.veripool.org


Section 9.4.2.3 of IEEE 1800-2017 allows "iff" qualifiers on @ event controls. The example code in 9.4.2.3 is fairly straightforward:

module latch (output logic [31:0] y, input [31:0] a, input enable);
  always @(a iff enable == 1)
     y <= a; //latch is in transparent mode
endmodule

I'm currently using Verilator only for lint and I get this error on the above code:
syntax error, unexpected iff, expecting ')' or ',' or or

The above code seems equivalent to the following which Verilator does support (at least for lint):

module latch (output logic [31:0] y, input [31:0] a, input enable);
  always @(a) if (enable == 1)
     y <= a; //latch is in transparent mode
endmodule

I also get lint errors when doing something similar in assertions:

assert property (@(posedge clk iff enable)
                  disable iff (reset)
                  (expr));

Can you introduce iff support? Thanks.

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-07-26T23:31:59Z


The event parsing needs some rewriting to handle this, but is relatively straight forward.

Added a disabled t_iff.v test as a placeholder.

@veripoolbot veripoolbot added area: assertions Issue involves assertions effort: days Expect this issue to require roughly days of invested effort to resolve type: feature-IEEE Request to add new feature, described in IEEE 1800 labels Dec 22, 2019
@pdonahue-ventana
Copy link

/cc @pdonahue-ventana

@sahilshahpatel
Copy link

This feature still seems to be missing. Are there any plans to support iff in events?

@wsnyder
Copy link
Member

wsnyder commented Sep 16, 2022

Perhaps you'd be willing to submit a pull to support it? (On the develop-v5 branch.)

@sahilshahpatel
Copy link

I'd be interested in trying to make an attempt. If you have any starter tips (like where is event parsing done?) that would be awesome! If not, I'll maybe try looking on my own

@wsnyder
Copy link
Member

wsnyder commented Sep 16, 2022

@kbieganski checking to make sure you aren't already working on "iff"? Do you know what files might need updating to support it?

  • Use develop-v5 branch as origin for the pull.
  • Make test_regress style tests checking the expected behavior, confirm pass on another simulator.
  • Add AstIff node type in AstNodeOther.h
  • Improve verilog.y parser to "new AstIff" to handle the yIFF. Note the proper syntax should already be there, but commented out or with UNSUPPORTED error.
  • Add code to V3Width to properly size it.
  • Grep for places AstSenTree is used to see which need to be improved.

@gezalore
Copy link
Member

gezalore commented Sep 17, 2022

For event control expressions posedge a iff enable == 1 at least, I am not sure there is a need for a new AstIff node, just add the condition expression under the existing AstSenItem. V3SenExprBuilder on develop-v5 can be taught how to handle them, which might be all is needed (other than type checking in V3Width and thorough testing), but likely there will be more detail elsewhere.

@kbieganski
Copy link
Member

I'm not working on this and not planning to, so you're good to go @sahilshahpatel (or anyone).

@varunr71
Copy link

varunr71 commented Feb 6, 2023

Do we have the support for "iff"?? Also am wondering if there is documentation for what SVA support is present in Verilator at the moment?

@sahilshahpatel
Copy link

I haven't been working on this myself, just haven't had the time. If you want to, go ahead. If not, I still want to get to it eventually, just not sure when.

kbieganski added a commit that referenced this issue Nov 29, 2023
Adds a new field to `AstSenItem` that stores the `iff` condition which is then handled by `SenExprBuilder`.

Signed-off-by: Krzysztof Bieganski <kbieganski@antmicro.com>
@kbieganski
Copy link
Member

This is now supported, see: #4626

@udif
Copy link
Contributor

udif commented Dec 12, 2023

I'm getting failures on a similar construct:

some_label: assert property (disable iff(rst !== 1'b0) @(posedge clk) !(expr));

@wsnyder
Copy link
Member

wsnyder commented Dec 12, 2023

@udif can you make a small test case and open a new issue please?

@udif
Copy link
Contributor

udif commented Dec 12, 2023

By testcase do you mean a pull request only adding the test that fails?

@wsnyder
Copy link
Member

wsnyder commented Dec 12, 2023

A pull request with the test, or if it is small you can just post the .v file that is needed in the issue itself.

toddstrader pushed a commit that referenced this issue Dec 13, 2023
Adds a new field to `AstSenItem` that stores the `iff` condition which is then handled by `SenExprBuilder`.

Signed-off-by: Krzysztof Bieganski <kbieganski@antmicro.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: assertions Issue involves assertions effort: days Expect this issue to require roughly days of invested effort to resolve type: feature-IEEE Request to add new feature, described in IEEE 1800
Projects
None yet
Development

No branches or pull requests

8 participants