MikePopoloski / slang

SystemVerilog compiler and language services
MIT License
620 stars 138 forks source link

Severity Tasks in Deferred Assertion Causing Errors #925

Closed hankhsu1996 closed 7 months ago

hankhsu1996 commented 7 months ago

Describe the bug After the commit c35b879f5bfb334b84af192bbe2585e08d9b5195, severity tasks are now being classified as SubroutineKind::Function. This classification is causing my existing SystemVerilog code to fail when using with deferred assertions.

To Reproduce Here is a simplified snippet:

module Test;
  function logic my_func();
    static logic my_var = 1'b1;
    assert final (my_var == 1'b1)
      else $error();
  endfunction
endmodule

Result:

error: cannot invoke a system function from a deferred assertion action (system tasks are allowed)

Additional context In the LRM's sections on deferred assertions, it states:

The pass and fail statements in a deferred assertion’s action_block, if present, shall each consist of a single subroutine call. The subroutine can be a task, task method, void function, void function method, or system task.

My thoughts:

  1. Since severity tasks are still classified as system tasks in the LRM, I believe this syntax should be considered valid.
  2. Even if we were to classify them as system functions, the deferred assertion seems to accept functions provided the return value is void.

Regarding the comment in the following code snippet: https://github.com/MikePopoloski/slang/blob/9705113788c5a11749ae463f3ee3e49751948666/source/ast/builtins/SystemTasks.cpp#L121-L123

LRM says:

All system task calls within a constant function shall be ignored except for the elaboration severity system tasks.

Therefore, it refers to "elaboration" severity system tasks (LRM 20.10.1), which are different from the run-time severity system tasks (LRM 20.10) used in the deferred assertion action block.

MikePopoloski commented 7 months ago

Yeah, there is basically no difference between a system task and a void-returning system function, since system tasks don't ever consume time anyway. There's a discussion on the Accellera issue tracker where they considered making the severity tasks into void-returning functions but I think they just ran out of time before the 2023 LRM shipped. I think it makes sense here to just remove the restriction on the deferred assertion action.

likeamahoney commented 7 months ago

Yeah, there is basically no difference between a system task and a void-returning system function, since system tasks don't ever consume time anyway. There's a discussion on the Accellera issue tracker where they considered making the severity tasks into void-returning functions but I think they just ran out of time before the 2023 LRM shipped. I think it makes sense here to just remove the restriction on the deferred assertion action.

Hi. Also this bug appears while elaborating such opensource code with slang.

MikePopoloski commented 7 months ago

Fixed in 0b436f94a4745a573a236a17ca9425dc9a585f4b