[llvm] 6a2a8eb - Revert "[SLP][NFC]Extract values state/operands analysis into separate class"

Alex Bradbury via llvm-commits llvm-commits at lists.llvm.org
Sat May 10 08:04:32 PDT 2025


Author: Alex Bradbury
Date: 2025-05-10T16:02:47+01:00
New Revision: 6a2a8ebe27c1941f5b952313239fc6d155f58e9d

URL: https://github.com/llvm/llvm-project/commit/6a2a8ebe27c1941f5b952313239fc6d155f58e9d
DIFF: https://github.com/llvm/llvm-project/commit/6a2a8ebe27c1941f5b952313239fc6d155f58e9d.diff

LOG: Revert "[SLP][NFC]Extract values state/operands analysis into separate class"

This reverts commit 512a5d0b8aa82749995204f4852e93757192288a.

It broke RISC-V vector code generation on some inputs (oggenc.c from
llvm-test-suite), as found by our CI. Reduced test case and more
information posted in #138274.

Added: 
    

Modified: 
    llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 8536bdaa9a063..7fbbb2681b9ed 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -2855,13 +2855,9 @@ class BoUpSLP {
     }
 
     /// Go through the instructions in VL and append their operands.
-    void appendOperands(ArrayRef<Value *> VL, ArrayRef<ValueList> Operands,
-                        const InstructionsState &S) {
-      assert(!Operands.empty() && !VL.empty() && "Bad list of operands");
-      assert((empty() || all_of(Operands,
-                                [this](const ValueList &VL) {
-                                  return VL.size() == getNumLanes();
-                                })) &&
+    void appendOperandsOfVL(ArrayRef<Value *> VL, const InstructionsState &S) {
+      assert(!VL.empty() && "Bad VL");
+      assert((empty() || VL.size() == getNumLanes()) &&
              "Expected same number of lanes");
       assert(S.valid() && "InstructionsState is invalid.");
       // IntrinsicInst::isCommutative returns true if swapping the first "two"
@@ -2870,7 +2866,7 @@ class BoUpSLP {
       Instruction *MainOp = S.getMainOp();
       unsigned NumOperands = MainOp->getNumOperands();
       ArgSize = isa<IntrinsicInst>(MainOp) ? IntrinsicNumOperands : NumOperands;
-      OpsVec.resize(ArgSize);
+      OpsVec.resize(NumOperands);
       unsigned NumLanes = VL.size();
       for (OperandDataVec &Ops : OpsVec)
         Ops.resize(NumLanes);
@@ -2878,6 +2874,18 @@ class BoUpSLP {
         Value *V = VL[Lane];
         assert((isa<Instruction>(V) || isa<PoisonValue>(V)) &&
                "Expected instruction or poison value");
+        if (isa<PoisonValue>(V)) {
+          for (unsigned OpIdx : seq<unsigned>(NumOperands))
+            OpsVec[OpIdx][Lane] = {
+                PoisonValue::get(MainOp->getOperand(OpIdx)->getType()), true,
+                false};
+          if (auto *EI = dyn_cast<ExtractElementInst>(MainOp)) {
+            OpsVec[0][Lane] = {EI->getVectorOperand(), true, false};
+          } else if (auto *EV = dyn_cast<ExtractValueInst>(MainOp)) {
+            OpsVec[0][Lane] = {EV->getAggregateOperand(), true, false};
+          }
+          continue;
+        }
         // Our tree has just 3 nodes: the root and two operands.
         // It is therefore trivial to get the APO. We only need to check the
         // opcode of V and whether the operand at OpIdx is the LHS or RHS
@@ -2888,16 +2896,11 @@ class BoUpSLP {
         // Since operand reordering is performed on groups of commutative
         // operations or alternating sequences (e.g., +, -), we can safely tell
         // the inverse operations by checking commutativity.
-        if (isa<PoisonValue>(V)) {
-          for (unsigned OpIdx : seq<unsigned>(NumOperands))
-            OpsVec[OpIdx][Lane] = {Operands[OpIdx][Lane], true, false};
-          continue;
-        }
-        auto [SelectedOp, Ops] = convertTo(cast<Instruction>(V), S);
+        auto [SelectedOp, Ops] = convertTo(cast<Instruction>(VL[Lane]), S);
         bool IsInverseOperation = !isCommutative(SelectedOp);
-        for (unsigned OpIdx : seq<unsigned>(ArgSize)) {
+        for (unsigned OpIdx = 0; OpIdx != NumOperands; ++OpIdx) {
           bool APO = (OpIdx == 0) ? false : IsInverseOperation;
-          OpsVec[OpIdx][Lane] = {Operands[OpIdx][Lane], APO, false};
+          OpsVec[OpIdx][Lane] = {Ops[OpIdx], APO, false};
         }
       }
     }
@@ -3003,12 +3006,12 @@ class BoUpSLP {
 
   public:
     /// Initialize with all the operands of the instruction vector \p RootVL.
-    VLOperands(ArrayRef<Value *> RootVL, ArrayRef<ValueList> Operands,
-               const InstructionsState &S, const BoUpSLP &R)
+    VLOperands(ArrayRef<Value *> RootVL, const InstructionsState &S,
+               const BoUpSLP &R)
         : TLI(*R.TLI), DL(*R.DL), SE(*R.SE), R(R),
           L(R.LI->getLoopFor(S.getMainOp()->getParent())) {
       // Append all the operands of RootVL.
-      appendOperands(RootVL, Operands, S);
+      appendOperandsOfVL(RootVL, S);
     }
 
     /// \Returns a value vector with the operands across all lanes for the
@@ -3818,6 +3821,12 @@ class BoUpSLP {
     /// Interleaving factor for interleaved loads Vectorize nodes.
     unsigned InterleaveFactor = 0;
 
+  public:
+    /// Returns interleave factor for interleave nodes.
+    unsigned getInterleaveFactor() const { return InterleaveFactor; }
+    /// Sets interleaving factor for the interleaving nodes.
+    void setInterleave(unsigned Factor) { InterleaveFactor = Factor; }
+
     /// Set this bundle's \p OpIdx'th operand to \p OpVL.
     void setOperand(unsigned OpIdx, ArrayRef<Value *> OpVL) {
       if (Operands.size() < OpIdx + 1)
@@ -3829,16 +3838,13 @@ class BoUpSLP {
       copy(OpVL, Operands[OpIdx].begin());
     }
 
-  public:
-    /// Returns interleave factor for interleave nodes.
-    unsigned getInterleaveFactor() const { return InterleaveFactor; }
-    /// Sets interleaving factor for the interleaving nodes.
-    void setInterleave(unsigned Factor) { InterleaveFactor = Factor; }
-
-    /// Set this bundle's operands from \p Operands.
-    void setOperands(ArrayRef<ValueList> Operands) {
-      for (unsigned I : seq<unsigned>(Operands.size()))
-        setOperand(I, Operands[I]);
+    /// Set this bundle's operand from Scalars.
+    void setOperand(const BoUpSLP &R, bool RequireReorder = false) {
+      VLOperands Ops(Scalars, S, R);
+      if (RequireReorder)
+        Ops.reorder();
+      for (unsigned I : seq<unsigned>(S.getMainOp()->getNumOperands()))
+        setOperand(I, Ops.getVL(I));
     }
 
     /// Reorders operands of the node to the given mask \p Mask.
@@ -4864,11 +4870,12 @@ class BoUpSLP {
           // where their second (immediate) operand is not added. Since
           // immediates do not affect scheduler behavior this is considered
           // okay.
-          assert(In &&
-                 (isa<ExtractValueInst, ExtractElementInst, CallBase>(In) ||
-                  In->getNumOperands() ==
-                      Bundle->getTreeEntry()->getNumOperands()) &&
-                 "Missed TreeEntry operands?");
+          assert(
+              In &&
+              (isa<ExtractValueInst, ExtractElementInst, IntrinsicInst>(In) ||
+               In->getNumOperands() ==
+                   Bundle->getTreeEntry()->getNumOperands()) &&
+              "Missed TreeEntry operands?");
 
           for (unsigned OpIdx :
                seq<unsigned>(Bundle->getTreeEntry()->getNumOperands()))
@@ -9757,184 +9764,6 @@ bool BoUpSLP::canBuildSplitNode(ArrayRef<Value *> VL,
   return true;
 }
 
-namespace {
-/// Class accepts incoming list of values and generates the list of values
-/// for scheduling and list of operands for the new nodes.
-class InstructionsCompatibilityAnalysis {
-  DominatorTree &DT;
-  const DataLayout &DL;
-  const TargetTransformInfo &TTI;
-  const TargetLibraryInfo &TLI;
-
-  /// Builds operands for the original instructions.
-  void
-  buildOriginalOperands(const InstructionsState &S, ArrayRef<Value *> VL,
-                        SmallVectorImpl<BoUpSLP::ValueList> &Operands) const {
-
-    unsigned ShuffleOrOp =
-        S.isAltShuffle() ? (unsigned)Instruction::ShuffleVector : S.getOpcode();
-    Instruction *VL0 = S.getMainOp();
-
-    switch (ShuffleOrOp) {
-    case Instruction::PHI: {
-      auto *PH = cast<PHINode>(VL0);
-
-      // Keeps the reordered operands to avoid code duplication.
-      PHIHandler Handler(DT, PH, VL);
-      Handler.buildOperands();
-      Operands.assign(PH->getNumOperands(), {});
-      for (unsigned I : seq<unsigned>(PH->getNumOperands()))
-        Operands[I].assign(Handler.getOperands(I).begin(),
-                           Handler.getOperands(I).end());
-      return;
-    }
-    case Instruction::ExtractValue:
-    case Instruction::ExtractElement:
-      // This is a special case, as it does not gather, but at the same time
-      // we are not extending buildTree_rec() towards the operands.
-      Operands.assign(1, {VL.size(), VL0->getOperand(0)});
-      return;
-    case Instruction::InsertElement:
-      Operands.assign(2, {VL.size(), nullptr});
-      for (auto [Idx, V] : enumerate(VL)) {
-        auto *IE = cast<InsertElementInst>(V);
-        for (auto [OpIdx, Ops] : enumerate(Operands))
-          Ops[Idx] = IE->getOperand(OpIdx);
-      }
-      return;
-    case Instruction::Load:
-      Operands.assign(
-          1, {VL.size(),
-              PoisonValue::get(cast<LoadInst>(VL0)->getPointerOperandType())});
-      for (auto [V, Op] : zip(VL, Operands.back())) {
-        auto *LI = dyn_cast<LoadInst>(V);
-        if (!LI)
-          continue;
-        Op = LI->getPointerOperand();
-      }
-      return;
-    case Instruction::ZExt:
-    case Instruction::SExt:
-    case Instruction::FPToUI:
-    case Instruction::FPToSI:
-    case Instruction::FPExt:
-    case Instruction::PtrToInt:
-    case Instruction::IntToPtr:
-    case Instruction::SIToFP:
-    case Instruction::UIToFP:
-    case Instruction::Trunc:
-    case Instruction::FPTrunc:
-    case Instruction::BitCast:
-    case Instruction::ICmp:
-    case Instruction::FCmp:
-    case Instruction::Select:
-    case Instruction::FNeg:
-    case Instruction::Add:
-    case Instruction::FAdd:
-    case Instruction::Sub:
-    case Instruction::FSub:
-    case Instruction::Mul:
-    case Instruction::FMul:
-    case Instruction::UDiv:
-    case Instruction::SDiv:
-    case Instruction::FDiv:
-    case Instruction::URem:
-    case Instruction::SRem:
-    case Instruction::FRem:
-    case Instruction::Shl:
-    case Instruction::LShr:
-    case Instruction::AShr:
-    case Instruction::And:
-    case Instruction::Or:
-    case Instruction::Xor:
-    case Instruction::Freeze:
-    case Instruction::Store:
-    case Instruction::ShuffleVector:
-      Operands.assign(VL0->getNumOperands(), {VL.size(), nullptr});
-      for (auto [Idx, V] : enumerate(VL)) {
-        auto *I = dyn_cast<Instruction>(V);
-        if (!I) {
-          for (auto [OpIdx, Ops] : enumerate(Operands))
-            Ops[Idx] = PoisonValue::get(VL0->getOperand(OpIdx)->getType());
-          continue;
-        }
-        auto [Op, ConvertedOps] = convertTo(I, S);
-        for (auto [OpIdx, Ops] : enumerate(Operands))
-          Ops[Idx] = ConvertedOps[OpIdx];
-      }
-      return;
-    case Instruction::GetElementPtr: {
-      Operands.assign(2, {VL.size(), nullptr});
-      // Need to cast all indices to the same type before vectorization to
-      // avoid crash.
-      // Required to be able to find correct matches between 
diff erent gather
-      // nodes and reuse the vectorized values rather than trying to gather them
-      // again.
-      const unsigned IndexIdx = 1;
-      Type *VL0Ty = VL0->getOperand(IndexIdx)->getType();
-      Type *Ty =
-          all_of(VL,
-                 [&](Value *V) {
-                   auto *GEP = dyn_cast<GetElementPtrInst>(V);
-                   return !GEP || VL0Ty == GEP->getOperand(IndexIdx)->getType();
-                 })
-              ? VL0Ty
-              : DL.getIndexType(cast<GetElementPtrInst>(VL0)
-                                    ->getPointerOperandType()
-                                    ->getScalarType());
-      for (auto [Idx, V] : enumerate(VL)) {
-        auto *GEP = dyn_cast<GetElementPtrInst>(V);
-        if (!GEP) {
-          Operands[0][Idx] = V;
-          Operands[1][Idx] = ConstantInt::getNullValue(Ty);
-          continue;
-        }
-        Operands[0][Idx] = GEP->getPointerOperand();
-        auto *Op = GEP->getOperand(IndexIdx);
-        auto *CI = dyn_cast<ConstantInt>(Op);
-        Operands[1][Idx] = CI ? ConstantFoldIntegerCast(
-                                    CI, Ty, CI->getValue().isSignBitSet(), DL)
-                              : Op;
-      }
-      return;
-    }
-    case Instruction::Call: {
-      auto *CI = cast<CallInst>(VL0);
-      Intrinsic::ID ID = getVectorIntrinsicIDForCall(CI, &TLI);
-      for (unsigned Idx : seq<unsigned>(CI->arg_size())) {
-        if (isVectorIntrinsicWithScalarOpAtArg(ID, Idx, &TTI))
-          continue;
-        auto &Ops = Operands.emplace_back();
-        for (Value *V : VL) {
-          auto *I = dyn_cast<Instruction>(V);
-          Ops.push_back(I ? I->getOperand(Idx)
-                          : PoisonValue::get(VL0->getOperand(Idx)->getType()));
-        }
-      }
-      return;
-    }
-    default:
-      break;
-    }
-    llvm_unreachable("Unexpected vectorization of the instructions.");
-  }
-
-public:
-  InstructionsCompatibilityAnalysis(DominatorTree &DT, const DataLayout &DL,
-                                    const TargetTransformInfo &TTI,
-                                    const TargetLibraryInfo &TLI)
-      : DT(DT), DL(DL), TTI(TTI), TLI(TLI) {}
-
-  SmallVector<BoUpSLP::ValueList> buildOperands(const InstructionsState &S,
-                                                ArrayRef<Value *> VL) {
-    assert(S && "Invalid state!");
-    SmallVector<BoUpSLP::ValueList> Operands;
-    buildOriginalOperands(S, VL, Operands);
-    return Operands;
-  }
-};
-} // namespace
-
 BoUpSLP::ScalarsVectorizationLegality
 BoUpSLP::getScalarsVectorizationLegality(ArrayRef<Value *> VL, unsigned Depth,
                                          const EdgeInfo &UserTreeIdx) const {
@@ -10307,8 +10136,6 @@ void BoUpSLP::buildTreeRec(ArrayRef<Value *> VLRef, unsigned Depth,
       registerNonVectorizableLoads(ArrayRef(VL));
     return;
   }
-  InstructionsCompatibilityAnalysis Analysis(*DT, *DL, *TTI, *TLI);
-  SmallVector<ValueList> Operands = Analysis.buildOperands(S, VL);
   ScheduleBundle Empty;
   ScheduleBundle &Bundle = BundlePtr.value() ? *BundlePtr.value() : Empty;
   LLVM_DEBUG(dbgs() << "SLP: We are able to schedule this bundle.\n");
@@ -10333,12 +10160,21 @@ void BoUpSLP::buildTreeRec(ArrayRef<Value *> VLRef, unsigned Depth,
   };
   switch (ShuffleOrOp) {
     case Instruction::PHI: {
+      auto *PH = cast<PHINode>(VL0);
+
       TreeEntry *TE =
           newTreeEntry(VL, Bundle, S, UserTreeIdx, ReuseShuffleIndices);
       LLVM_DEBUG(dbgs() << "SLP: added a new TreeEntry (PHINode).\n";
                  TE->dump());
 
-      TE->setOperands(Operands);
+      // Keeps the reordered operands to avoid code duplication.
+      PHIHandler Handler(*DT, PH, VL);
+      Handler.buildOperands();
+      for (unsigned I : seq<unsigned>(PH->getNumOperands()))
+        TE->setOperand(I, Handler.getOperands(I));
+      SmallVector<ArrayRef<Value *>> Operands(PH->getNumOperands());
+      for (unsigned I : seq<unsigned>(PH->getNumOperands()))
+        Operands[I] = Handler.getOperands(I);
       CreateOperandNodes(TE, Operands);
       return;
     }
@@ -10365,7 +10201,7 @@ void BoUpSLP::buildTreeRec(ArrayRef<Value *> VLRef, unsigned Depth,
                  TE->dump());
       // This is a special case, as it does not gather, but at the same time
       // we are not extending buildTreeRec() towards the operands.
-      TE->setOperands(Operands);
+      TE->setOperand(*this);
       return;
     }
     case Instruction::InsertElement: {
@@ -10396,7 +10232,7 @@ void BoUpSLP::buildTreeRec(ArrayRef<Value *> VLRef, unsigned Depth,
       LLVM_DEBUG(dbgs() << "SLP: added a new TreeEntry (InsertElementInst).\n";
                  TE->dump());
 
-      TE->setOperands(Operands);
+      TE->setOperand(*this);
       buildTreeRec(TE->getOperand(1), Depth + 1, {TE, 1});
       return;
     }
@@ -10451,7 +10287,7 @@ void BoUpSLP::buildTreeRec(ArrayRef<Value *> VLRef, unsigned Depth,
       case TreeEntry::NeedToGather:
         llvm_unreachable("Unexpected loads state.");
       }
-      TE->setOperands(Operands);
+      TE->setOperand(*this);
       if (State == TreeEntry::ScatterVectorize)
         buildTreeRec(PointerOps, Depth + 1, {TE, 0});
       return;
@@ -10492,7 +10328,7 @@ void BoUpSLP::buildTreeRec(ArrayRef<Value *> VLRef, unsigned Depth,
       LLVM_DEBUG(dbgs() << "SLP: added a new TreeEntry (CastInst).\n";
                  TE->dump());
 
-      TE->setOperands(Operands);
+      TE->setOperand(*this);
       for (unsigned I : seq<unsigned>(VL0->getNumOperands()))
         buildTreeRec(TE->getOperand(I), Depth + 1, {TE, I});
       if (ShuffleOrOp == Instruction::Trunc) {
@@ -10520,28 +10356,37 @@ void BoUpSLP::buildTreeRec(ArrayRef<Value *> VLRef, unsigned Depth,
       LLVM_DEBUG(dbgs() << "SLP: added a new TreeEntry (CmpInst).\n";
                  TE->dump());
 
-      VLOperands Ops(VL, Operands, S, *this);
+      ValueList Left, Right;
+      VLOperands Ops(VL, S, *this);
       if (cast<CmpInst>(VL0)->isCommutative()) {
         // Commutative predicate - collect + sort operands of the instructions
         // so that each side is more likely to have the same opcode.
         assert(P0 == CmpInst::getSwappedPredicate(P0) &&
                "Commutative Predicate mismatch");
         Ops.reorder();
-        Operands.front() = Ops.getVL(0);
-        Operands.back() = Ops.getVL(1);
+        Left = Ops.getVL(0);
+        Right = Ops.getVL(1);
       } else {
         // Collect operands - commute if it uses the swapped predicate.
-        for (auto [Idx, V] : enumerate(VL)) {
-          if (isa<PoisonValue>(V))
+        for (Value *V : VL) {
+          if (isa<PoisonValue>(V)) {
+            Left.push_back(PoisonValue::get(VL0->getOperand(0)->getType()));
+            Right.push_back(PoisonValue::get(VL0->getOperand(1)->getType()));
             continue;
+          }
           auto *Cmp = cast<CmpInst>(V);
+          Value *LHS = Cmp->getOperand(0);
+          Value *RHS = Cmp->getOperand(1);
           if (Cmp->getPredicate() != P0)
-            std::swap(Operands.front()[Idx], Operands.back()[Idx]);
+            std::swap(LHS, RHS);
+          Left.push_back(LHS);
+          Right.push_back(RHS);
         }
       }
-      TE->setOperands(Operands);
-      buildTreeRec(Operands.front(), Depth + 1, {TE, 0});
-      buildTreeRec(Operands.back(), Depth + 1, {TE, 1});
+      TE->setOperand(0, Left);
+      TE->setOperand(1, Right);
+      buildTreeRec(Left, Depth + 1, {TE, 0});
+      buildTreeRec(Right, Depth + 1, {TE, 1});
       if (ShuffleOrOp == Instruction::ICmp) {
         unsigned NumSignBits0 =
             ComputeNumSignBits(VL0->getOperand(0), *DL, 0, AC, nullptr, DT);
@@ -10584,13 +10429,7 @@ void BoUpSLP::buildTreeRec(ArrayRef<Value *> VLRef, unsigned Depth,
                     "(SelectInst/UnaryOperator/BinaryOperator/FreezeInst).\n";
           TE->dump());
 
-      if (isa<BinaryOperator>(VL0) && isCommutative(VL0)) {
-        VLOperands Ops(VL, Operands, S, *this);
-        Ops.reorder();
-        Operands[0] = Ops.getVL(0);
-        Operands[1] = Ops.getVL(1);
-      }
-      TE->setOperands(Operands);
+      TE->setOperand(*this, isa<BinaryOperator>(VL0) && isCommutative(VL0));
       for (unsigned I : seq<unsigned>(VL0->getNumOperands()))
         buildTreeRec(TE->getOperand(I), Depth + 1, {TE, I});
       return;
@@ -10600,7 +10439,52 @@ void BoUpSLP::buildTreeRec(ArrayRef<Value *> VLRef, unsigned Depth,
                                    ReuseShuffleIndices);
       LLVM_DEBUG(dbgs() << "SLP: added a new TreeEntry (GetElementPtrInst).\n";
                  TE->dump());
-      TE->setOperands(Operands);
+      SmallVector<ValueList, 2> Operands(2);
+      // Prepare the operand vector for pointer operands.
+      for (Value *V : VL) {
+        auto *GEP = dyn_cast<GetElementPtrInst>(V);
+        if (!GEP) {
+          Operands.front().push_back(V);
+          continue;
+        }
+        Operands.front().push_back(GEP->getPointerOperand());
+      }
+      TE->setOperand(0, Operands.front());
+      // Need to cast all indices to the same type before vectorization to
+      // avoid crash.
+      // Required to be able to find correct matches between 
diff erent gather
+      // nodes and reuse the vectorized values rather than trying to gather them
+      // again.
+      int IndexIdx = 1;
+      Type *VL0Ty = VL0->getOperand(IndexIdx)->getType();
+      Type *Ty = all_of(VL,
+                        [VL0Ty, IndexIdx](Value *V) {
+                          auto *GEP = dyn_cast<GetElementPtrInst>(V);
+                          if (!GEP)
+                            return true;
+                          return VL0Ty == GEP->getOperand(IndexIdx)->getType();
+                        })
+                     ? VL0Ty
+                     : DL->getIndexType(cast<GetElementPtrInst>(VL0)
+                                            ->getPointerOperandType()
+                                            ->getScalarType());
+      // Prepare the operand vector.
+      for (Value *V : VL) {
+        auto *I = dyn_cast<GetElementPtrInst>(V);
+        if (!I) {
+          Operands.back().push_back(
+              ConstantInt::get(Ty, 0, /*isSigned=*/false));
+          continue;
+        }
+        auto *Op = I->getOperand(IndexIdx);
+        auto *CI = dyn_cast<ConstantInt>(Op);
+        if (!CI)
+          Operands.back().push_back(Op);
+        else
+          Operands.back().push_back(ConstantFoldIntegerCast(
+              CI, Ty, CI->getValue().isSignBitSet(), *DL));
+      }
+      TE->setOperand(IndexIdx, Operands.back());
 
       for (unsigned I = 0, Ops = Operands.size(); I < Ops; ++I)
         buildTreeRec(Operands[I], Depth + 1, {TE, I});
@@ -10619,7 +10503,7 @@ void BoUpSLP::buildTreeRec(ArrayRef<Value *> VLRef, unsigned Depth,
         LLVM_DEBUG(
             dbgs() << "SLP: added a new TreeEntry (jumbled StoreInst).\n";
             TE->dump());
-      TE->setOperands(Operands);
+      TE->setOperand(*this);
       buildTreeRec(TE->getOperand(0), Depth + 1, {TE, 0});
       return;
     }
@@ -10633,13 +10517,7 @@ void BoUpSLP::buildTreeRec(ArrayRef<Value *> VLRef, unsigned Depth,
                                    ReuseShuffleIndices);
       LLVM_DEBUG(dbgs() << "SLP: added a new TreeEntry (CallInst).\n";
                  TE->dump());
-      if (isCommutative(VL0)) {
-        VLOperands Ops(VL, Operands, S, *this);
-        Ops.reorder();
-        Operands[0] = Ops.getVL(0);
-        Operands[1] = Ops.getVL(1);
-      }
-      TE->setOperands(Operands);
+      TE->setOperand(*this, isCommutative(VL0));
       for (unsigned I : seq<unsigned>(CI->arg_size())) {
         // For scalar operands no need to create an entry since no need to
         // vectorize it.
@@ -10673,34 +10551,37 @@ void BoUpSLP::buildTreeRec(ArrayRef<Value *> VLRef, unsigned Depth,
         CmpInst::Predicate AltP = AltCI->getPredicate();
         assert(MainP != AltP &&
                "Expected 
diff erent main/alternate predicates.");
+        ValueList Left, Right;
         // Collect operands - commute if it uses the swapped predicate or
         // alternate operation.
-        for (auto [Idx, V] : enumerate(VL)) {
-          if (isa<PoisonValue>(V))
+        for (Value *V : VL) {
+          if (isa<PoisonValue>(V)) {
+            Left.push_back(PoisonValue::get(MainCI->getOperand(0)->getType()));
+            Right.push_back(PoisonValue::get(MainCI->getOperand(1)->getType()));
             continue;
+          }
           auto *Cmp = cast<CmpInst>(V);
+          Value *LHS = Cmp->getOperand(0);
+          Value *RHS = Cmp->getOperand(1);
 
           if (isAlternateInstruction(Cmp, MainCI, AltCI, *TLI)) {
             if (AltP == CmpInst::getSwappedPredicate(Cmp->getPredicate()))
-              std::swap(Operands.front()[Idx], Operands.back()[Idx]);
+              std::swap(LHS, RHS);
           } else {
             if (MainP == CmpInst::getSwappedPredicate(Cmp->getPredicate()))
-              std::swap(Operands.front()[Idx], Operands.back()[Idx]);
+              std::swap(LHS, RHS);
           }
+          Left.push_back(LHS);
+          Right.push_back(RHS);
         }
-        TE->setOperands(Operands);
-        buildTreeRec(Operands.front(), Depth + 1, {TE, 0});
-        buildTreeRec(Operands.back(), Depth + 1, {TE, 1});
+        TE->setOperand(0, Left);
+        TE->setOperand(1, Right);
+        buildTreeRec(Left, Depth + 1, {TE, 0});
+        buildTreeRec(Right, Depth + 1, {TE, 1});
         return;
       }
 
-      if (isa<BinaryOperator>(VL0) || CI) {
-        VLOperands Ops(VL, Operands, S, *this);
-        Ops.reorder();
-        Operands[0] = Ops.getVL(0);
-        Operands[1] = Ops.getVL(1);
-      }
-      TE->setOperands(Operands);
+      TE->setOperand(*this, isa<BinaryOperator>(VL0) || CI);
       for (unsigned I : seq<unsigned>(VL0->getNumOperands()))
         buildTreeRec(TE->getOperand(I), Depth + 1, {TE, I});
       return;


        


More information about the llvm-commits mailing list