8000 Node has no type Error on cast within concatenation · Issue #5989 · verilator/verilator · GitHub
[go: up one dir, main page]
Skip to content

Node has no type Error on cast within concatenation #5989

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.

Sign up for GitHub

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

Jump to bottom
Closed
obliguedon opened this issue May 6, 2025 · 9 comments
Closed

Node has no type Error on cast within concatenation #5989

obliguedon opened this issue May 6, 2025 · 9 comments

Comments

@obliguedon
Copy link

Thanks for taking the time to report this.

Can you attach an example that shows the issue? (Must be openly licensed, completely self-contained and self-checking so can directly run what you provide. Ideally use test_regress format, see https://veripool.org/guide/latest/contributing.html?highlight=test_regress#reporting-bugs
-test.sv:

package my_pkg;
  typedef enum logic [1:0] {
    SIG_0, SIG_1, SIG_2
  } sig_t;
endpackage : my_pkg

module test
  import my_pkg::*;
(
  input logic [6:0] data,
  output logic [2:0] opt
);
  assign opt = {data[5], sig_t'(data[1:0])};
endmodule : test

What 'verilator' command line do we use to run your example?

verilator --cc --exe --main --Wno-fatal ./test.sv

What 'verilator --version' are you using? Did you try it with the git master version?

Verilator 5.037 devel rev v5.036-32-g66e105b44

What OS and distribution are you using?

OS: CentOS Linux 7
Kernel: Linux 3.10.0-957.5.1.el7.x86_64

May we assist you in trying to fix this in Verilator yourself?

  • I'm far from good in C++

reported message:

%Error: Internal Error: test.sv:16:31: ../V3Width.cpp:7311: Node has no type
                                     : ... note: In instance 'test'
   16 |    assign opt = {data[5], sig_t'(data[1:0])};
      |                                ^
@obliguedon obliguedon added the new New issue not seen by maintainers label May 6, 2025
@obliguedon
Copy link
Author

it work like this:

module test
  import my_pkg::*;
(
input logic [6:0] data,
  output logic [2:0] opt
);
  sig_t sig;
  assign sig = sig_t'(data[1:0]);
  assign opt = {data[5], sig};

endmodule : test

but the cast within concatenation work with other simulator

@wsnyder
Copy link
Member
wsnyder commented May 11, 2025

I have a patch to fix it... but it breaks other things, so needs more attention.

@wsnyder wsnyder removed the new New issue not seen by maintainers label May 11, 2025
@wsnyder wsnyder mentioned this issue May 16, 2025
Closed
@toddstrader
Copy link
Member

I have a patch to fix it... but it breaks other things, so needs more attention.

@wsnyder not sure how in-the-weeds the additional attention is, but if it would help I can attempt to push more on this if you can push it to a branch.

@wsnyder
Copy link
Member
wsnyder commented May 16, 2025

The fix for this was just:

diff --git a/src/V3Width.cpp b/src/V3Width.cpp
index 9e307391c..562697ba6 100644
--- a/src/V3Width.cpp
+++ b/src/V3Width.cpp
@@ -2100,6 +2100,7 @@ class WidthVisitor final : public VNVisitor {
                                               VFlagChildDType{}, refp};
             nodep->replaceWith(newp);
             VL_DO_DANGLING(pushDeletep(nodep), nodep);
+            userIterate(newp, m_vup);  //<----- added this
         } else {
             nodep->v3warn(E_UNSUPPORTED,
                           "Unsupported: Cast to " << nodep->dtp()->prettyTypeName());

Unfortunately the streaming operator tests didn't like that, so that's what needs investigation.

@wsnyder
Copy link
Member
wsnyder commented May 16, 2025

There's an ugly interaction between streaming and cast, because IEEE linked the two operations in a weird way. Maybe it would be a lot cleaner to earlier move the cast into part of the streaming Ast node.

@toddstrader
Copy link
Member

Oof. I just read 6.24.3 and 11.4.14. I'm sure I don't fully understand this yet, but this doesn't look fun. I see that t_cast_stream and t_uvm_todo are failing with your fix and that the former sets up the cast + stream like so:

    1:2:2:2:1: CAST 0x555557009a40 <e1414#> {f26bk} @dt=0x5555570a4f00@(G/sw32)
    1:2:2:2:1:1: SIGNED 0x555556fcaa50 <e1409#> {f26bk} @dt=0x5555570a5e00@(G/sw0)
    1:2:2:2:1:1:1: STREAML 0x555557005a20 <e1404#> {f26bo} @dt=0x555556fca9a0@(w0)stream
    1:2:2:2:1:1:1:1: VARREF 0x5555570a6c30 <e1375#> {f26bx} @dt=0x55555700dc80@(w0)$[]  array [RV] <- VAR 0x555556f79e60 <e1148#> {f19al} @dt=0x55555700dc80@(w0)$[]  array [VSTATIC]  VAR
    1:2:2:2:1:1:1:2: CONST 0x555556fccd20 <e1396#> {f26bm} @dt=0x5555570a4f00@(G/sw32)  32'h1

Are you suggesting to do an earlier pass where we push the datatype from the CAST down into the STREAML and then discard the CAST (and I guess SIGNED as well)?

Also, if I'm just getting in the way please ignore me.

@wsnyder
Copy link
Member
wsnyder commented May 16, 2025

Yes, I was contemplating that ast edit, but might not help, unsure. I'm not working on this now so all help welcome.

@toddstrader
Copy link
Member

I updated #6012 with a stab at this. I'll double-check next week that the PR also handles the code posted in this issue, but I'm pretty sure it will.

@toddstrader toddstrader mentioned this issue May 19, 2025
Merged
@toddstrader
Copy link
Member

I believe this is resolved now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@wsnyder @toddstrader @obliguedon
0