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

fix event control asterisk to accept whitespace between @ and * #102

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yousifBilal
Copy link

This is a fix for EventControlAsterisk to have 2 symbols @ and *. Most compilers seems to accept a whitespace between @ and *. This module was not being parsed with sv-parser. While others like https://sv-lang.com/explore/ and verilator considered it valid.

module test;
    reg a, b;
    reg out; 

    always @ * begin
        out = a & b;
    end

    always @* begin
        out = a | b;
    end
    initial begin
        // Initialize inputs
        a = 0; b = 0;
        #10; a = 1; b = 0;
        #10; a = 0; b = 1;
        #10; a = 1; b = 1;
        #10; $finish;
    end

    initial begin
        $monitor("At time %0t: a = %0b, b = %0b, out = %0b", $time, a, b, out);
    end
endmodule

@dalance
Copy link
Owner

dalance commented Oct 3, 2024

Thank you for your contribution.
This seems to be difficult problem.

According to IEEE1800-2017, the definition of event_control is below.
It shows space between @ and * is not allowed.

event_control ::=
@ hierarchical_event_identifier
| @ ( event_expression )
| @*
| @ (*)
| @ ps_or_hierarchical_sequence_identifier

Actually, the following code causes syntax error by Synopsys VCS.
We can't request to fix it because it seems to be forbidden by language specification.

always @ /* comment */ * begin
    out = a & b;
end

On the other hand, IEEE1800-2023 is below.
It shows space is allowed.

event_control ::=
clocking_event
| @ *
| @ ( * )

So I think the current behaviour should be kept at least to check IEEE1800-2017 compliant.
And "IEEE1800-2023 mode" which accepts the space is probably required too.

@yousifBilal
Copy link
Author

How would realize a "IEEE1800-2023 mode"? would you do it as part of this project or a separate one dedicated for 2023?

@dalance
Copy link
Owner

dalance commented Oct 7, 2024

I don't have enough motivation to create individual project for 2023.
So I'll consider about adding mode switch in this project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants