[PATCH] D98860: [RISCV] Optimize all-constant mask BUILD_VECTORs

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:1178
+        bool BitValue = !V.isUndef() && cast<ConstantSDNode>(V)->getZExtValue();
+        Bits |= (BitValue << BitPos);
+      }
----------------
frasercrmck wrote:
> craig.topper wrote:
> > I think BitValue needs to be casted to uint64_t otherwise NumViaIntegerBits==64 doesn't work. I think otherwise BitValue is promoted to 'int' for the shift. Which is UB if BitPos > 32. Then I think the shift result is sign extended to be 64 bits for the OR.
> > 
> > If NumViaIntegerBits is 32 or less you probably want to sign extend Bits from 32 to 64 to get optimal constant materialization.
> Yes you're right. Not the first time I've made that mistake, sadly. Thanks.
> 
> Can we not sign extend from NumViaIntegerBits to 64? I was thinking you might better catch some i8/i16s when the sign-extended value is easier to materialize.
That would be fine. I’m not sure it will have any effect on the number of instructions needed to materialize.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98860/new/

https://reviews.llvm.org/D98860