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

Delay is wrongly treated as IntConst #3447

Open
hs-apotell opened this issue Jan 25, 2023 · 7 comments
Open

Delay is wrongly treated as IntConst #3447

hs-apotell opened this issue Jan 25, 2023 · 7 comments
Assignees

Comments

@hs-apotell
Copy link
Collaborator

Example:

module main;
  parameter p_dly = 0;
  wire a, b;
  assign #p_dly a = b;
endmodule

Uhdm output:

  |vpiContAssign:
  \_cont_assign: , line:4:17, endln:4:22
    |vpiParent:
    \_module_inst: work@main (work@main), file:abc.v, line:1:1, endln:5:10
    |vpiDelay:
    \_constant: , line:4:10, endln:4:16
      |vpiDecompile:p_dly
      |vpiSize:64
      |UINT:0
      |vpiConstType:9

Note that the non-numeric delay is being parsed as a number.

The issue is from the following snippet where delay_value is assumed to be an IntConst which isn't true as per the grammar.

void SV3_1aTreeShapeListener::exitPound_delay_value(
SV3_1aParser::Pound_delay_valueContext *ctx) {
if (ctx->Pound_delay()) {
addVObject(ctx, ctx->Pound_delay()->getText(), VObjectType::slIntConst);
} else if (ctx->Pound_Pound_delay()) {
addVObject(ctx, ctx->Pound_Pound_delay()->getText(),
VObjectType::slPound_Pound_delay);
} else if (ctx->delay_value()) {
addVObject(ctx, ctx->delay_value()->getText(), VObjectType::slIntConst);
}
}

delay_value
: Integral_number
| Real_number
| ps_identifier
| time_literal
| ONESTEP
| ps_or_hierarchical_identifier
//REMOVED | Decimal_number
;

Following tests are impacted by this issue -

  • LibraryIntercon
  • Icarus (sched1.v)
  • Unisim (ISERDES.v)
@alaindargelas alaindargelas self-assigned this Jan 28, 2023
@alaindargelas
Copy link
Collaborator

Fixed by #3453

@hs-apotell
Copy link
Collaborator Author

Still noticing problems with delay values. Here's an example and result with your change.

For #p_dly on line 6, nodes n(75) t(StringConst) has node n()74) t(Delay_value) as child.
For #delay on line 10, nodes n(123) t(StringConst) has node n(122) t(Delay_value) as child.

Neither of these two cases are dictated by the grammar.

01. module main;
02.   parameter p_dly = 0;
03.   wire a, b;
04.   logic [0:1] outvar;
05.   int iterations = 256;
06.   assign #p_dly a = b;
07.   initial
08.   begin
09.     for (int i = 0; i < iterations; i++)
10.       #delay outvar++;
11.   end
12. endmodule
n<> u<0> t<_INVALID_> f<0> l<0:0>
n<> u<1> t<Null_rule> p<158> s<157> l<1:1> el<1:0>
n<module> u<2> t<Module_keyword> p<4> s<3> l<1:1> el<1:7>
n<main> u<3> t<StringConst> p<4> l<1:8> el<1:12>
n<> u<4> t<Module_ansi_header> p<155> c<2> s<20> l<1:1> el<1:13>
n<> u<5> t<Data_type_or_implicit> p<15> s<14> l<2:13> el<2:13>
n<p_dly> u<6> t<StringConst> p<13> s<12> l<2:13> el<2:18>
n<0> u<7> t<IntConst> p<8> l<2:21> el<2:22>
n<> u<8> t<Primary_literal> p<9> c<7> l<2:21> el<2:22>
n<> u<9> t<Constant_primary> p<10> c<8> l<2:21> el<2:22>
n<> u<10> t<Constant_expression> p<11> c<9> l<2:21> el<2:22>
n<> u<11> t<Constant_mintypmax_expression> p<12> c<10> l<2:21> el<2:22>
n<> u<12> t<Constant_param_expression> p<13> c<11> l<2:21> el<2:22>
n<> u<13> t<Param_assignment> p<14> c<6> l<2:13> el<2:22>
n<> u<14> t<List_of_param_assignments> p<15> c<13> l<2:13> el<2:22>
n<> u<15> t<Parameter_declaration> p<16> c<5> l<2:3> el<2:22>
n<> u<16> t<Package_or_generate_item_declaration> p<17> c<15> l<2:3> el<2:23>
n<> u<17> t<Module_or_generate_item_declaration> p<18> c<16> l<2:3> el<2:23>
n<> u<18> t<Module_common_item> p<19> c<17> l<2:3> el<2:23>
n<> u<19> t<Module_or_generate_item> p<20> c<18> l<2:3> el<2:23>
n<> u<20> t<Non_port_module_item> p<155> c<19> s<33> l<2:3> el<2:23>
n<> u<21> t<NetType_Wire> p<28> s<22> l<3:3> el<3:7>
n<> u<22> t<Data_type_or_implicit> p<28> s<27> l<3:8> el<3:8>
n<a> u<23> t<StringConst> p<24> l<3:8> el<3:9>
n<> u<24> t<Net_decl_assignment> p<27> c<23> s<26> l<3:8> el<3:9>
n<b> u<25> t<StringConst> p<26> l<3:11> el<3:12>
n<> u<26> t<Net_decl_assignment> p<27> c<25> l<3:11> el<3:12>
n<> u<27> t<List_of_net_decl_assignments> p<28> c<24> l<3:8> el<3:12>
n<> u<28> t<Net_declaration> p<29> c<21> l<3:3> el<3:13>
n<> u<29> t<Package_or_generate_item_declaration> p<30> c<28> l<3:3> el<3:13>
n<> u<30> t<Module_or_generate_item_declaration> p<31> c<29> l<3:3> el<3:13>
n<> u<31> t<Module_common_item> p<32> c<30> l<3:3> el<3:13>
n<> u<32> t<Module_or_generate_item> p<33> c<31> l<3:3> el<3:13>
n<> u<33> t<Non_port_module_item> p<155> c<32> s<55> l<3:3> el<3:13>
n<> u<34> t<IntVec_TypeLogic> p<45> s<44> l<4:3> el<4:8>
n<0> u<35> t<IntConst> p<36> l<4:10> el<4:11>
n<> u<36> t<Primary_literal> p<37> c<35> l<4:10> el<4:11>
n<> u<37> t<Constant_primary> p<38> c<36> l<4:10> el<4:11>
n<> u<38> t<Constant_expression> p<43> c<37> s<42> l<4:10> el<4:11>
n<1> u<39> t<IntConst> p<40> l<4:12> el<4:13>
n<> u<40> t<Primary_literal> p<41> c<39> l<4:12> el<4:13>
n<> u<41> t<Constant_primary> p<42> c<40> l<4:12> el<4:13>
n<> u<42> t<Constant_expression> p<43> c<41> l<4:12> el<4:13>
n<> u<43> t<Constant_range> p<44> c<38> l<4:10> el<4:13>
n<> u<44> t<Packed_dimension> p<45> c<43> l<4:9> el<4:14>
n<> u<45> t<Data_type> p<49> c<34> s<48> l<4:3> el<4:14>
n<outvar> u<46> t<StringConst> p<47> l<4:15> el<4:21>
n<> u<47> t<Variable_decl_assignment> p<48> c<46> l<4:15> el<4:21>
n<> u<48> t<List_of_variable_decl_assignments> p<49> c<47> l<4:15> el<4:21>
n<> u<49> t<Variable_declaration> p<50> c<45> l<4:3> el<4:22>
n<> u<50> t<Data_declaration> p<51> c<49> l<4:3> el<4:22>
n<> u<51> t<Package_or_generate_item_declaration> p<52> c<50> l<4:3> el<4:22>
n<> u<52> t<Module_or_generate_item_declaration> p<53> c<51> l<4:3> el<4:22>
n<> u<53> t<Module_common_item> p<54> c<52> l<4:3> el<4:22>
n<> u<54> t<Module_or_generate_item> p<55> c<53> l<4:3> el<4:22>
n<> u<55> t<Non_port_module_item> p<155> c<54> s<71> l<4:3> el<4:22>
n<> u<56> t<IntegerAtomType_Int> p<57> l<5:3> el<5:6>
n<> u<57> t<Data_type> p<65> c<56> s<64> l<5:3> el<5:6>
n<iterations> u<58> t<StringConst> p<63> s<62> l<5:7> el<5:17>
n<256> u<59> t<IntConst> p<60> l<5:20> el<5:23>
n<> u<60> t<Primary_literal> p<61> c<59> l<5:20> el<5:23>
n<> u<61> t<Primary> p<62> c<60> l<5:20> el<5:23>
n<> u<62> t<Expression> p<63> c<61> l<5:20> el<5:23>
n<> u<63> t<Variable_decl_assignment> p<64> c<58> l<5:7> el<5:23>
n<> u<64> t<List_of_variable_decl_assignments> p<65> c<63> l<5:7> el<5:23>
n<> u<65> t<Variable_declaration> p<66> c<57> l<5:3> el<5:24>
n<> u<66> t<Data_declaration> p<67> c<65> l<5:3> el<5:24>
n<> u<67> t<Package_or_generate_item_declaration> p<68> c<66> l<5:3> el<5:24>
n<> u<68> t<Module_or_generate_item_declaration> p<69> c<67> l<5:3> el<5:24>
n<> u<69> t<Module_common_item> p<70> c<68> l<5:3> el<5:24>
n<> u<70> t<Module_or_generate_item> p<71> c<69> l<5:3> el<5:24>
n<> u<71> t<Non_port_module_item> p<155> c<70> s<91> l<5:3> el<5:24>
n<p_dly> u<72> t<StringConst> p<73> l<6:11> el<6:16>
n<> u<73> t<Ps_identifier> p<74> c<72> l<6:11> el<6:16>
n<> u<74> t<Delay_value> p<75> c<73> l<6:11> el<6:16>
n<p_dly> u<75> t<StringConst> p<76> c<74> l<6:10> el<6:16>
n<> u<76> t<Delay3> p<88> c<75> s<87> l<6:10> el<6:16>
n<a> u<77> t<StringConst> p<78> l<6:17> el<6:18>
n<> u<78> t<Ps_or_hierarchical_identifier> p<81> c<77> s<80> l<6:17> el<6:18>
n<> u<79> t<Constant_bit_select> p<80> l<6:19> el<6:19>
n<> u<80> t<Constant_select> p<81> c<79> l<6:19> el<6:19>
n<> u<81> t<Net_lvalue> p<86> c<78> s<85> l<6:17> el<6:18>
n<b> u<82> t<StringConst> p<83> l<6:21> el<6:22>
n<> u<83> t<Primary_literal> p<84> c<82> l<6:21> el<6:22>
n<> u<84> t<Primary> p<85> c<83> l<6:21> el<6:22>
n<> u<85> t<Expression> p<86> c<84> l<6:21> el<6:22>
n<> u<86> t<Net_assignment> p<87> c<81> l<6:17> el<6:22>
n<> u<87> t<List_of_net_assignments> p<88> c<86> l<6:17> el<6:22>
n<> u<88> t<Continuous_assign> p<89> c<76> l<6:3> el<6:23>
n<> u<89> t<Module_common_item> p<90> c<88> l<6:3> el<6:23>
n<> u<90> t<Module_or_generate_item> p<91> c<89> l<6:3> el<6:23>
n<> u<91> t<Non_port_module_item> p<155> c<90> s<153> l<6:3> el<6:23>
n<> u<92> t<IntegerAtomType_Int> p<93> l<9:10> el<9:13>
n<> u<93> t<Data_type> p<99> c<92> s<94> l<9:10> el<9:13>
n<i> u<94> t<StringConst> p<99> s<98> l<9:14> el<9:15>
n<0> u<95> t<IntConst> p<96> l<9:18> el<9:19>
n<> u<96> t<Primary_literal> p<97> c<95> l<9:18> el<9:19>
n<> u<97> t<Primary> p<98> c<96> l<9:18> el<9:19>
n<> u<98> t<Expression> p<99> c<97> l<9:18> el<9:19>
n<> u<99> t<For_variable_declaration> p<100> c<93> l<9:10> el<9:19>
n<> u<100> t<For_initialization> p<141> c<99> s<110> l<9:10> el<9:19>
n<i> u<101> t<StringConst> p<102> l<9:21> el<9:22>
n<> u<102> t<Primary_literal> p<103> c<101> l<9:21> el<9:22>
n<> u<103> t<Primary> p<104> c<102> l<9:21> el<9:22>
n<> u<104> t<Expression> p<110> c<103> s<109> l<9:21> el<9:22>
n<iterations> u<105> t<StringConst> p<106> l<9:25> el<9:35>
n<> u<106> t<Primary_literal> p<107> c<105> l<9:25> el<9:35>
n<> u<107> t<Primary> p<108> c<106> l<9:25> el<9:35>
n<> u<108> t<Expression> p<110> c<107> l<9:25> el<9:35>
n<> u<109> t<BinOp_Less> p<110> s<108> l<9:23> el<9:24>
n<> u<110> t<Expression> p<141> c<104> s<119> l<9:21> el<9:35>
n<i> u<111> t<StringConst> p<112> l<9:37> el<9:38>
n<> u<112> t<Ps_or_hierarchical_identifier> p<115> c<111> s<114> l<9:37> el<9:38>
n<> u<113> t<Bit_select> p<114> l<9:38> el<9:38>
n<> u<114> t<Select> p<115> c<113> l<9:38> el<9:38>
n<> u<115> t<Variable_lvalue> p<117> c<112> s<116> l<9:37> el<9:38>
n<> u<116> t<IncDec_PlusPlus> p<117> l<9:38> el<9:40>
n<> u<117> t<Inc_or_dec_expression> p<118> c<115> l<9:37> el<9:40>
n<> u<118> t<For_step_assignment> p<119> c<117> l<9:37> el<9:40>
n<> u<119> t<For_step> p<141> c<118> s<139> l<9:37> el<9:40>
n<delay> u<120> t<StringConst> p<121> l<10:8> el<10:13>
n<> u<121> t<Ps_identifier> p<122> c<120> l<10:8> el<10:13>
n<> u<122> t<Delay_value> p<123> c<121> l<10:8> el<10:13>
n<delay> u<123> t<StringConst> p<124> c<122> l<10:7> el<10:13>
n<> u<124> t<Delay_control> p<125> c<123> l<10:7> el<10:13>
n<> u<125> t<Procedural_timing_control> p<136> c<124> s<135> l<10:7> el<10:13>
n<outvar> u<126> t<StringConst> p<127> l<10:14> el<10:20>
n<> u<127> t<Ps_or_hierarchical_identifier> p<130> c<126> s<129> l<10:14> el<10:20>
n<> u<128> t<Bit_select> p<129> l<10:20> el<10:20>
n<> u<129> t<Select> p<130> c<128> l<10:20> el<10:20>
n<> u<130> t<Variable_lvalue> p<132> c<127> s<131> l<10:14> el<10:20>
n<> u<131> t<IncDec_PlusPlus> p<132> l<10:20> el<10:22>
n<> u<132> t<Inc_or_dec_expression> p<133> c<130> l<10:14> el<10:22>
n<> u<133> t<Statement_item> p<134> c<132> l<10:14> el<10:23>
n<> u<134> t<Statement> p<135> c<133> l<10:14> el<10:23>
n<> u<135> t<Statement_or_null> p<136> c<134> l<10:14> el<10:23>
n<> u<136> t<Procedural_timing_control_statement> p<137> c<125> l<10:7> el<10:23>
n<> u<137> t<Statement_item> p<138> c<136> l<10:7> el<10:23>
n<> u<138> t<Statement> p<139> c<137> l<10:7> el<10:23>
n<> u<139> t<Statement_or_null> p<141> c<138> l<10:7> el<10:23>
n<> u<140> t<For> p<141> s<100> l<9:5> el<9:8>
n<> u<141> t<Loop_statement> p<142> c<140> l<9:5> el<10:23>
n<> u<142> t<Statement_item> p<143> c<141> l<9:5> el<10:23>
n<> u<143> t<Statement> p<144> c<142> l<9:5> el<10:23>
n<> u<144> t<Statement_or_null> p<146> c<143> s<145> l<9:5> el<10:23>
n<> u<145> t<End> p<146> l<11:3> el<11:6>
n<> u<146> t<Seq_block> p<147> c<144> l<8:3> el<11:6>
n<> u<147> t<Statement_item> p<148> c<146> l<8:3> el<11:6>
n<> u<148> t<Statement> p<149> c<147> l<8:3> el<11:6>
n<> u<149> t<Statement_or_null> p<150> c<148> l<8:3> el<11:6>
n<> u<150> t<Initial_construct> p<151> c<149> l<7:3> el<11:6>
n<> u<151> t<Module_common_item> p<152> c<150> l<7:3> el<11:6>
n<> u<152> t<Module_or_generate_item> p<153> c<151> l<7:3> el<11:6>
n<> u<153> t<Non_port_module_item> p<155> c<152> s<154> l<7:3> el<11:6>
n<> u<154> t<Endmodule> p<155> l<12:1> el<12:10>
n<> u<155> t<Module_declaration> p<156> c<4> l<1:1> el<12:10>
n<> u<156> t<Description> p<157> c<155> l<1:1> el<12:10>
n<> u<157> t<Source_text> p<158> c<156> l<1:1> el<12:10>
n<> u<158> t<Top_level_rule> c<1> l<1:1> el<12:10>

@hs-apotell hs-apotell reopened this Jan 30, 2023
@hs-apotell
Copy link
Collaborator Author

@alaindargelas Added a few comments to the change review as well. Please take a moment to address those as well. Thanks!

@alaindargelas
Copy link
Collaborator

The Verilog grammar is ambiguous and cannot be fixed.
The UHDM tree can be fixed as you can see with some hacking of the AST and associated interpretation when creating the UHDM model.
Don't look at the AST, look at the UHDM model, do you see any issues in the UHDM model for your examples?

@hs-apotell
Copy link
Collaborator Author

I am working on linting, so I need a correct AST.

@alaindargelas
Copy link
Collaborator

Then please take on this task. Good luck with fixing the Verilog grammar :-)
This is not my priority. As long as the resulting UHDM is correct, it achieves my goal.

@alaindargelas alaindargelas removed their assignment Jan 31, 2023
@hs-apotell
Copy link
Collaborator Author

I will investigate.

@hs-apotell hs-apotell self-assigned this Jan 31, 2023
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

No branches or pull requests

2 participants