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

Parameter enum const functions fail #3999

Closed
sarutob1 opened this issue Mar 2, 2023 · 5 comments
Closed

Parameter enum const functions fail #3999

sarutob1 opened this issue Mar 2, 2023 · 5 comments
Labels
area: elaboration Issue involves elaboration phase resolution: fixed Closed; fixed

Comments

@sarutob1
Copy link
Contributor

sarutob1 commented Mar 2, 2023

I'm trying to write a const function to export the list of enum values as a string written to a parameter but I encountered a segmentaion fault when trying to compile. I think I've isolated that the issue has something to do with using the enum next function with data flow into parameter variables.

module t ();

   typedef enum logic {E0, E1} enm_t;

   function automatic enm_t get_2();
      enm_t enm;
      enm = E0;
      return enm.next;
   endfunction

   function automatic enm_t get_second();
      enm_t enm;
      enm = E0;
      return enm.next;
   endfunction

   localparam enm_t enum_second = get_2();
   // localparam enm_t enum_second = E1;

   $info("%d %d %d", get_2(), get_second(), enum_second);

endmodule

From gdb, I've found that the issue at this line nodep->getChildDTypep()->numeric().isSigned() at V3LinkDot.cpp:1256 getChildDType() is null and nodep->m_name = "__Venumtab_enum_next0". I'm stuck understanding what the LinkDot visitor is trying to do and how it affects the enum next function.

@sarutob1 sarutob1 added the new New issue not seen by maintainers label Mar 2, 2023
@wsnyder
Copy link
Member

wsnyder commented Mar 2, 2023

This seems to fail in the second call to V3LinkDot, after initial elaboration. At that point the if signed test is not need, so try adding an additional test "if (nodep->getChildDTypep() && ...)". If that works (or if you find another fix) please submit a pull request with the fix and a test case, thanks for debugging.

@wsnyder wsnyder added status: ready Issue is ready for someone to fix; then goes to 'status: assigned' and removed new New issue not seen by maintainers labels Mar 3, 2023
@sarutob1
Copy link
Contributor Author

sarutob1 commented Mar 3, 2023

I think I've identified the problem better (i've modified the example program). It seems that when the function result is assigned to a parameter or localparam, it will duplicate the enum_next variable in the AST as such:

src/obj_dir/Vt_enum_const_methods_017_const.tree
    1: PACKAGE 0x555557970c60 <e1117> {a0aa}  __024unit  L0 [LIB] [NONE]
    1:2: VAR 0x5555579b0300 <e975> {d9am} @dt=0x5555579d9e10@(w1)u[1:0]  __Venumtab_enum_next0 [CONST] MODULETEMP
    1:2:3: INITARRAY 0x5555579dc800 <e976> {d9am} @dt=0x5555579d9e10@(w1)u[1:0] [0]=0x5555579da8f0 [1]=0x5555579da9a0
    1:2:3:1: CONST 0x5555579dc900 <e1489> {d9am} @dt=0x5555579d8d00@(G/w1)  1'h0
    1:2:3:2: INITITEM 0x5555579da8f0 <e986> {d9am}
    1:2:3:2:1: CONST 0x5555579dca00 <e1490> {d9bc} @dt=0x5555579d8d00@(G/w1)  1'h1
    1:2:3:2: INITITEM 0x5555579da9a0 <e990> {d9am}
    1:2:3:2:1: CONST 0x5555579dcb00 <e1491> {d9ay} @dt=0x5555579d8d00@(G/w1)  1'h0
    1:2: VAR 0x5555579b0480 <e1249> {d9am} @dt=0x5555579a15f0@(w1)u[1:0]  __Venumtab_enum_next0 [CONST] MODULETEMP
    1:2:3: INITARRAY 0x5555579dd000 <e1248> {d9am} @dt=0x5555579a15f0@(w1)u[1:0] [0]=0x5555579db130 [1]=0x5555579db1e0
    1:2:3:1: CONST 0x5555579ddd00 <e1492> {d9am} @dt=0x5555579d8d00@(G/w1)  1'h0
    1:2:3:2: INITITEM 0x5555579db130 <e1257> {d9am}
    1:2:3:2:1: CONST 0x5555579a2000 <e1258> {d9bc} @dt=0x55555797ce10@(w1)enum  1'h1
    1:2:3:2: INITITEM 0x5555579db1e0 <e1261> {d9am}
    1:2:3:2:1: CONST 0x5555579ddf00 <e1260> {d9ay} @dt=0x55555797ce10@(w1)enum  1'h0
    3: TYPETABLE 0x555557978000 <e2> {a0aa}

The nodep that gives a nullptr getChildDTypep is the second one which doesn't exist when the two param assignments from my above example are commented and uncommented respectively. This duplicated variable appears to be added between the tree dump for the files src/obj_dir/Vt_enum_const_methods_009_linkdotparam.tree and src/obj_dir/Vt_enum_const_methods_011_width.tree.

src/obj_dir/Vt_enum_const_methods_009_linkdotparam.tree
    1: PACKAGE 0x555557970c60 <e1117#> {a0aa}  __024unit  L0 [LIB] [NONE]
    1:2: VAR 0x5555579b0300 <e975#> {d9am} @dt=0x5555579d9e10@(w1)u[1:0]  __Venumtab_enum_next0 [CONST] MODULETEMP
    1:2:3: INITARRAY 0x5555579dc800 <e976#> {d9am} @dt=0x5555579d9e10@(w1)u[1:0] [0]=0x5555579da8f0 [1]=0x5555579da9a0
    1:2:3:1: CONST 0x5555579dc900 <e985#> {d9am} @dt=0x555557980f70@(G/w1)  1'h0
    1:2:3:2: INITITEM 0x5555579da8f0 <e986#> {d9am}
    1:2:3:2:1: CONST 0x5555579dca00 <e987#> {d9bc} @dt=0x555557980f70@(G/w1)  1'h1
    1:2:3:2: INITITEM 0x5555579da9a0 <e990#> {d9am}
    1:2:3:2:1: CONST 0x5555579dcb00 <e989#> {d9ay} @dt=0x555557980f70@(G/w1)  1'h0
    3: TYPETABLE 0x555557978000 <e2> {a0aa}

The next thing I think I should do is stop the ast from duplicating this variable and then make sure that the function that tried to use it pulls enum_next from the existing var.

@sarutob1
Copy link
Contributor Author

sarutob1 commented Mar 6, 2023

I tried analyzing the AST tree that comes out from when I add "if (nodep->getChildDTypep() && ...)" and what happens is that the duplicated VAR 0x5555579b0480 is removed from the tree but is still referenced by:
VARREF 0x555557970ea0 <e1287> {d9am} @dt=0x555557a695f0@(w1)u[1:0] __Venumtab_enum_next0 pkg=0x555557970c60 [RV] <- VAR 0x5555579a6480 <e1545#> {d9am} @dt=0x555557a695f0@(w1)u[1:0] __Venumtab_enum_next0 [CONST] MODULETEMP
Is this variable supposed to be removed or is the VARREF supposed to be updated to point to the original enum value (0x5555579b0300 )?

Update:
I used gdb to substitute the original varp in the varref which then lets the compilation complete normally

@wsnyder
Copy link
Member

wsnyder commented Mar 6, 2023

It shouldn't have gotten duplicated, especially not with an identical name, so that's one problem. I think the enum being referred to needs to be in the dtype table once it is referred to, so if that type goes away the function can still refer to it.

Alternatively we'd somehow track which of these variables gets deleted if a enum data type gets deleted, but that seems a lot of new work to do.

@wsnyder
Copy link
Member

wsnyder commented Oct 7, 2023

Full fix including this merged manually.

@wsnyder wsnyder closed this as completed Oct 7, 2023
@wsnyder wsnyder added resolution: fixed Closed; fixed area: elaboration Issue involves elaboration phase and removed status: ready Issue is ready for someone to fix; then goes to 'status: assigned' labels Oct 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: elaboration Issue involves elaboration phase resolution: fixed Closed; fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants