erichkeane added inline comments.
================
Comment at: clang/utils/TableGen/SveEmitter.cpp:302
unsigned Shift = llvm::countr_zero(Mask);
+ assert(Shift >= 64 && "Shift is out of encodable range");
return (V << Shift) & Mask;
----------------
Shouldn't this be: `assert(Shift < 64 &&"...")`?
`expr.shift` (https://eel.is/c++draft/expr.shift) says:
```
The operands shall be of integral or unscoped enumeration type and integral
promotions are performed.
The type of the result is that of the promoted left operand.
The behavior is undefined if the right operand is negative, or greater than or
equal to the width of the promoted left operand.```
uint64 stays as an `unsigned long`, so it is still 64 bits, so the only invalid
value for `Shift` is 64 (though >64 is 'nonsense', but only impossible because
of `llvm::countr_zero`).
One thing to consider: I wonder if we should instead be changing the 'shift' to
be:
`(V << (Shift % 64)) && Mask` ? It looks like `arm_sve.td` has the `NoFlags`
value as zero, which I think will end up going through here possibly (or at
least, inserted into `FlagTypes`.
So I suspect an assert might not be sufficient, since a 64 bit shift is
possible in that case (since a zero 'Mask' is the only case where `countr_zero`
will end up being 64).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D150140/new/
https://reviews.llvm.org/D150140
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
