[PATCH] D91343: [InstCombine] Optimize away the unnecessary multi-use sign-extend

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added a reviewer: RKSimon.
RKSimon added a comment.

vector tests?



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp:912
+      if (match(I->getOperand(1), m_APInt(SRA)) &&
+          match(Op0->getOperand(1), m_APInt(SLA))) {
+        unsigned BitWidth = DemandedMask.getBitWidth();
----------------
Is it worth merging the Shl matching and LeftShiftAmt == RightShiftAmt here as well?


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp:915
+        uint32_t RightShiftAmt = SRA->getLimitedValue(BitWidth - 1);
+        uint32_t LeftShiftAmt = SLA->getLimitedValue(BitWidth - 1);
+
----------------
Safer just to do a SRA->getValue()->ult(BitWidth) bounds check?


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp:921
+          APInt ValMask(BitWidth, 0);
+          ValMask.setBits(0, BitWidth - RightShiftAmt - 1);
+          // If so, return the operand of Shl rendering the Shl/AShr dead for
----------------
Use APInt::setLowBits ?


================
Comment at: llvm/test/Transforms/InstCombine/simplify-multiuse-demanded-bits-ashr.ll:8
+
+define dso_local i64 @multi_use_sign_extend_not_affecting_demanded_bits(i64 %a, i32 %b, i16 signext %c, i32 %d) local_unnamed_addr addrspace(1) #0 {
+
----------------
strip the metadata/attaributes?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91343