[llvm] [SelectOpt] Refactor to prepare for support more select-like operations (PR #115745)

David Green via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 13 05:18:18 PST 2024


================
@@ -530,34 +467,53 @@ void SelectOptimizeImpl::optimizeSelectsInnerLoops(Function &F,
   }
 }
 
-/// If \p isTrue is true, return the true value of \p SI, otherwise return
-/// false value of \p SI. If the true/false value of \p SI is defined by any
-/// select instructions in \p Selects, look through the defining select
-/// instruction until the true/false value is not defined in \p Selects.
-static Value *
-getTrueOrFalseValue(SelectOptimizeImpl::SelectLike SI, bool isTrue,
-                    const SmallPtrSet<const Instruction *, 2> &Selects,
-                    IRBuilder<> &IB) {
-  Value *V = nullptr;
-  for (SelectInst *DefSI = dyn_cast<SelectInst>(SI.getI());
-       DefSI != nullptr && Selects.count(DefSI);
-       DefSI = dyn_cast<SelectInst>(V)) {
-    if (DefSI->getCondition() == SI.getCondition())
-      V = (isTrue ? DefSI->getTrueValue() : DefSI->getFalseValue());
-    else // Handle inverted SI
-      V = (!isTrue ? DefSI->getTrueValue() : DefSI->getFalseValue());
+/// Returns optimised value on \p IsTrue branch. For SelectInst that would be
+/// either True or False value. For (BinaryOperator) instructions, where the
+/// condition may be skipped, the operation will use a non-conditional operand.
+/// For example, for `or(V,zext(cond))` this function would return V.
+/// However, if the conditional operand on \p IsTrue branch matters, we create a
+/// clone of instruction at the end of that branch \p B and replace the
+/// condition operand with a constant.
+///
+/// Also /p OptSelects contains previously optimised select-like instructions.
+/// If the current value uses one of the optimised values, we can optimise it
+/// further by replacing it with the corresponding value on the given branch
+static Value *getTrueOrFalseValue(
+    SelectOptimizeImpl::SelectLike &SI, bool isTrue,
+    SmallDenseMap<Instruction *, std::pair<Value *, Value *>, 2> &OptSelects,
+    BasicBlock *B) {
+  Value *V = isTrue ? SI.getTrueValue() : SI.getFalseValue();
+  if (V) {
+    auto *IV = dyn_cast<Instruction>(V);
+    if (IV && OptSelects.count(IV))
+      return isTrue ? OptSelects[IV].first : OptSelects[IV].second;
+    return V;
   }
 
-  if (isa<BinaryOperator>(SI.getI())) {
-    assert(SI.getI()->getOpcode() == Instruction::Or &&
-           "Only currently handling Or instructions.");
-    V = SI.getFalseValue();
-    if (isTrue)
-      V = IB.CreateOr(V, ConstantInt::get(V->getType(), 1));
+  auto *BO = cast<BinaryOperator>(SI.getI());
+  assert(BO->getOpcode() == Instruction::Or &&
+         "Only currently handling Or instructions.");
+
+  auto *CBO = BO->clone();
+  auto CondIdx = SI.getConditionOpIndex();
+  auto *AuxI = cast<Instruction>(CBO->getOperand(CondIdx));
+  if (isa<ZExtInst>(AuxI) || isa<LShrOperator>(AuxI)) {
----------------
davemgreen wrote:

My understanding is that this can't be reached with this instruction at the moment? If so, yeah it sounds OK to remove.

https://github.com/llvm/llvm-project/pull/115745


More information about the llvm-commits mailing list