MikePopoloski / slang

SystemVerilog compiler and language services
MIT License
558 stars 122 forks source link

Handling of packed structures for -Wbitwise-op-mismatch #1008

Closed jrudess closed 1 month ago

jrudess commented 1 month ago

I'm hoping that there is a way to suppress this kind of case for the new bitwise-op-mismatch warning. I think what I'm looking for is that packed structs be treated as op-matches to their equivalent sized logic.

slangtest164.sv:11:18: warning: bitwise operation between operands of different types ('logic[3:0]' and 's_t') [-Wbitwise-op-mismatch]
      d = a |  b | c;
          ~~~~~~ ^ ~
module top;

  typedef struct packed {
      logic [1:0] a;
      logic [1:0] b;
  } s_t;

  s_t a, b, c, d;

  always_comb begin
      d = a |  b | c;
  end

endmodule
MikePopoloski commented 1 month ago

Oh I see, the type of a | b is being computed as logic[3:0] even though it could probably just be s_t. Is the thing you're actually asking for here that I should fix the type of the subexpression or do you actually want what you stated directly? Both are relatively trivial to do from a mechanical perspective but the former seems non-controversial whereas I could see two different users legitimately wanting either behavior for the latter.

jrudess commented 1 month ago

I agree that there may be cases where another user would want to see a warning for a struct vs logic operation. Since the more conservative route of changing a | b to return s_t type solves the main RTL issue I've seen, that's the better choice for now. I'm still collecting data as I trial out the new warnings on various repos.

MikePopoloski commented 1 month ago

Fixed in 0e6a1a27fe4bbbfc56c5c71ad8e2385faa9ae2f1