Skip to content

[SLP] NFC. Make InstructionsState support default constructor. #121627

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

HanKuanChen
Copy link
Contributor

Previously, InstructionsState::invalid was used to create an InstructionsState. However, since all of the member functions of InstructionsState are const, we don't need to consider that the state will change after it is created. Using the default constructor to represent the state as invalid is much simpler.

Previously, InstructionsState::invalid was used to create an
InstructionsState. However, since all of the member functions of
InstructionsState are const, we don't need to consider that the state
will change after it is created. Using the default constructor to
represent the state as invalid is much simpler.
@llvmbot
Copy link
Member

llvmbot commented Jan 4, 2025

@llvm/pr-subscribers-vectorizers

@llvm/pr-subscribers-llvm-transforms

Author: Han-Kuan Chen (HanKuanChen)

Changes

Previously, InstructionsState::invalid was used to create an InstructionsState. However, since all of the member functions of InstructionsState are const, we don't need to consider that the state will change after it is created. Using the default constructor to represent the state as invalid is much simpler.


Full diff: https://github.com/llvm/llvm-project/pull/121627.diff

1 Files Affected:

  • (modified) llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp (+16-17)
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index c4582df89213d8..ddface5cef32bc 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -812,8 +812,8 @@ namespace {
 /// Main data required for vectorization of instructions.
 class InstructionsState {
   /// The main/alternate instruction. MainOp is also VL0.
-  Instruction *MainOp = nullptr;
-  Instruction *AltOp = nullptr;
+  Instruction *MainOp;
+  Instruction *AltOp;
 
 public:
   Instruction *getMainOp() const {
@@ -844,10 +844,9 @@ class InstructionsState {
 
   explicit operator bool() const { return valid(); }
 
-  InstructionsState() = delete;
+  InstructionsState() : InstructionsState(nullptr, nullptr) {}
   InstructionsState(Instruction *MainOp, Instruction *AltOp)
       : MainOp(MainOp), AltOp(AltOp) {}
-  static InstructionsState invalid() { return {nullptr, nullptr}; }
 };
 
 } // end anonymous namespace
@@ -909,17 +908,17 @@ static InstructionsState getSameOpcode(ArrayRef<Value *> VL,
                                        const TargetLibraryInfo &TLI) {
   // Make sure these are all Instructions.
   if (!all_of(VL, IsaPred<Instruction, PoisonValue>))
-    return InstructionsState::invalid();
+    return InstructionsState();
 
   auto *It = find_if(VL, IsaPred<Instruction>);
   if (It == VL.end())
-    return InstructionsState::invalid();
+    return InstructionsState();
 
   Value *V = *It;
   unsigned InstCnt = std::count_if(It, VL.end(), IsaPred<Instruction>);
   if ((VL.size() > 2 && !isa<PHINode>(V) && InstCnt < VL.size() / 2) ||
       (VL.size() == 2 && InstCnt < 2))
-    return InstructionsState::invalid();
+    return InstructionsState();
 
   bool IsCastOp = isa<CastInst>(V);
   bool IsBinOp = isa<BinaryOperator>(V);
@@ -962,7 +961,7 @@ static InstructionsState getSameOpcode(ArrayRef<Value *> VL,
     BaseID = getVectorIntrinsicIDForCall(CallBase, &TLI);
     BaseMappings = VFDatabase(*CallBase).getMappings(*CallBase);
     if (!isTriviallyVectorizable(BaseID) && BaseMappings.empty())
-      return InstructionsState::invalid();
+      return InstructionsState();
   }
   bool AnyPoison = InstCnt != VL.size();
   for (int Cnt = 0, E = VL.size(); Cnt < E; Cnt++) {
@@ -974,7 +973,7 @@ static InstructionsState getSameOpcode(ArrayRef<Value *> VL,
     // TODO: do some smart analysis of the CallInsts to exclude divide-like
     // intrinsics/functions only.
     if (AnyPoison && (I->isIntDivRem() || I->isFPDivRem() || isa<CallInst>(I)))
-      return InstructionsState::invalid();
+      return InstructionsState();
     unsigned InstOpcode = I->getOpcode();
     if (IsBinOp && isa<BinaryOperator>(I)) {
       if (InstOpcode == Opcode || InstOpcode == AltOpcode)
@@ -1046,28 +1045,28 @@ static InstructionsState getSameOpcode(ArrayRef<Value *> VL,
       if (auto *Gep = dyn_cast<GetElementPtrInst>(I)) {
         if (Gep->getNumOperands() != 2 ||
             Gep->getOperand(0)->getType() != IBase->getOperand(0)->getType())
-          return InstructionsState::invalid();
+          return InstructionsState();
       } else if (auto *EI = dyn_cast<ExtractElementInst>(I)) {
         if (!isVectorLikeInstWithConstOps(EI))
-          return InstructionsState::invalid();
+          return InstructionsState();
       } else if (auto *LI = dyn_cast<LoadInst>(I)) {
         auto *BaseLI = cast<LoadInst>(IBase);
         if (!LI->isSimple() || !BaseLI->isSimple())
-          return InstructionsState::invalid();
+          return InstructionsState();
       } else if (auto *Call = dyn_cast<CallInst>(I)) {
         auto *CallBase = cast<CallInst>(IBase);
         if (Call->getCalledFunction() != CallBase->getCalledFunction())
-          return InstructionsState::invalid();
+          return InstructionsState();
         if (Call->hasOperandBundles() &&
             (!CallBase->hasOperandBundles() ||
              !std::equal(Call->op_begin() + Call->getBundleOperandsStartIndex(),
                          Call->op_begin() + Call->getBundleOperandsEndIndex(),
                          CallBase->op_begin() +
                              CallBase->getBundleOperandsStartIndex())))
-          return InstructionsState::invalid();
+          return InstructionsState();
         Intrinsic::ID ID = getVectorIntrinsicIDForCall(Call, &TLI);
         if (ID != BaseID)
-          return InstructionsState::invalid();
+          return InstructionsState();
         if (!ID) {
           SmallVector<VFInfo> Mappings = VFDatabase(*Call).getMappings(*Call);
           if (Mappings.size() != BaseMappings.size() ||
@@ -1077,12 +1076,12 @@ static InstructionsState getSameOpcode(ArrayRef<Value *> VL,
               Mappings.front().Shape.VF != BaseMappings.front().Shape.VF ||
               Mappings.front().Shape.Parameters !=
                   BaseMappings.front().Shape.Parameters)
-            return InstructionsState::invalid();
+            return InstructionsState();
         }
       }
       continue;
     }
-    return InstructionsState::invalid();
+    return InstructionsState();
   }
 
   return InstructionsState(cast<Instruction>(V),

@alexey-bataev
Copy link
Member

Prefer to avoid doing this

@HanKuanChen
Copy link
Contributor Author

Prefer to avoid doing this

It also solves #120198.

@HanKuanChen HanKuanChen deleted the slp-nfc-InstructionsState-constructor branch January 10, 2025 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants