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

Listing the hierarchy of a module when a warning in that module fires and the code depends on parameters? #1125

Closed
jrudess opened this issue Sep 16, 2024 · 3 comments

Comments

@jrudess
Copy link
Contributor

jrudess commented Sep 16, 2024

While attempting to enable every slang warning in many repos, one issue I run into is when warnings only fire within a module when parameters of that module are specific values. i.e. there is no warning in all compiles, but only under certain configurations. In large models, it can be difficult to track down exactly which hierarchy the warning is related to, as the warning fires generically against the child module without any context.

In the following example, if module-instance m2 is commented out, there is no warning, but when the warning fires it is not always obvious that it is only specific to m2 and not both m1 and m2.

What do you think about adding new info printouts after the warning-message which provide the module hierarchy for the module that contains the warning? I don't have an exhaustive list of all the warnings where this extra information would be useful yet, and always dumping an Info message might be annoying.

Note: In the below example, the static-width-cast was added to the RTL to suppress LHS vs RHS width mismatches from a different vendor LINT tool used by a project, which then leads to the Slang warning. I don't have a simple way to make both tools happy at the same time yet. :-)

slangtest173_b.sv:9:39: warning: useless cast from 'integer' to the same type [-Wuseless-cast]
        always_comb a[i] = $bits(a[i])'(i);
                           ~~~~~~~~~~~^ ~
module m #(
    parameter int unsigned num,
    parameter int unsigned width
) (
    output logic [num-1:0][width-1:0] a
);

    for (genvar i = 0; i < num; i++) begin
        always_comb a[i] = $bits(a[i])'(i);
    end

endmodule

module top;

    logic [1:0][15:0] a1;
    logic [1:0][31:0] a2;

    m #(
        .num   (2),
        .width (16)
    ) m1(
        .a     (a1)
    );

    m #(
        .num   (2),
        .width (32)
    ) m2(
        .a     (a2)
    );

endmodule
@jrudess
Copy link
Contributor Author

jrudess commented Sep 16, 2024

Some more thoughts:

  • The hierarchy information only applies to warnings that are elaboration phase instead of analysis, so that limits the scope of when the info-message would print.
  • If there are multiple offending instances, would there be just one warning printed with the info message dumping all offending hierarchies? Or should a separate warning print for each offending hierarchy? This is probably tangentially related to Add caching of identical instance bodies #949.

@MikePopoloski
Copy link
Owner

slang already sort of has what you're asking for, controlled by the --diag-hierarchy option. By default it's set to auto, where slang will only print the hierarchy if it thinks there is some ambiguity in location (like there are multiple instances of some module M but only one of them has the diagnostic). You can set it to always to always get them printed. If you could give more details about a case where you wanted the hierarchy to appear but it didn't with auto mode I might be able to tune the heuristics a bit.

Separately, I think Wuseless-cast is a little overzealous in that it just checks integral types with the same width; probably it should be a little more exact, and probably shouldn't warn about genvars, etc.

@jrudess
Copy link
Contributor Author

jrudess commented Sep 16, 2024

--diag-hierarchy is exactly what I was looking for.

I had copied VIM errorformat setup for Clang syntax to use for Slang, and the 'in instance' output doesn't get parsed into VIMs quickfix window since it doesn't follow the standard file:line:column: error/warning/info/note convention. I'll have to play around with VIM error formats to see if that is possible. I guess this message also prints before the warning/error instead of after which may make the VIM parsing more difficult, but that is consistent with how the "in file included from" messages print, so it make sense to print before.

I don't have any examples off-hand where --diag-hierarchy auto isn't automatically printing the instance name other than the minimized case in this issue. But I also don't ever recall seeing in instance: printouts before, so I'm not sure what is a good example case where auto would cause it to print.

The cases where knowing the hierarchy would have been helpful are almost always width-mismatch or conversion warnings where parameters are either configured wrong, or the RTL isn't well written.

Most of the time I can tell how to fix the RTL from casually reading the code. But sometimes, the RTL code is of... sufficiently low-quality as to make groking its intent a bit time-consuming. So being able to get the param values from the instance helps speed up the learning curve.

I don't have the width-mismatch and conversion warnings turned on yet for a couple of larger repos, so I'll keep my eye out for some good examples that I can easily minimize.

@jrudess jrudess closed this as completed Sep 16, 2024
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