[PATCH] D118317: [InstCombine] Fold and-reduce idiom

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

I think it would make most sense to do at backend level, but i suppose it would be fine to do it here too.



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:5885
 
+static Instruction *foldReductionIdiom(ICmpInst &I,
+                                       InstCombiner::BuilderTy &Builder,
----------------
Add a comment explaining what this does?


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:5894
+    return nullptr;
+  auto *BitCastOp = dyn_cast<ICmpInst>(BitCast->getOperand(0));
+  if (!BitCastOp || !BitCastOp->hasOneUse())
----------------
This should probably be `InnerICmp`


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:5897-5898
+    return nullptr;
+  auto *OpTy = dyn_cast<FixedVectorType>(BitCastOp->getType());
+  if (!OpTy)
+    return nullptr;
----------------
I think the LHSTy should just be hoisted here, and used instead.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:5907
+        OpTy->getNumElements() * LHSTy->getElementType()->getIntegerBitWidth();
+    if (DL.isLegalInteger(NumBits)) {
+      auto *ScalarTy = Builder.getIntNTy(NumBits);
----------------
I suspect this should be relaxed to something like "is less than a legal integer"


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

https://reviews.llvm.org/D118317