[llvm] [SystemZ] Fix Operand Retrieval for Vector Reduction Intrinsic in `shouldExpandReduction` (PR #88874)

Dominik Steenken via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 17 00:02:29 PDT 2024


================
@@ -1323,25 +1324,43 @@ SystemZTTIImpl::getIntrinsicInstrCost(const IntrinsicCostAttributes &ICA,
 }
 
 bool SystemZTTIImpl::shouldExpandReduction(const IntrinsicInst *II) const {
-  // Always expand on Subtargets without vector instructions
+  // Always expand on Subtargets without vector instructions.
   if (!ST->hasVector())
     return true;
 
-  // Always expand for operands that do not fill one vector reg
-  auto *Type = cast<FixedVectorType>(II->getOperand(0)->getType());
-  unsigned NumElts = Type->getNumElements();
-  unsigned ScalarSize = Type->getScalarSizeInBits();
+  // Find the type of the vector operand of the intrinsic
+  // This assumes that each vector reduction intrinsic only
+  // has one vector operand.
+  FixedVectorType *VType = 0x0;
+  for (unsigned I = 0; I < II->getNumOperands(); ++I) {
+    auto *T = II->getOperand(I)->getType();
+    if (T->isVectorTy()) {
+      VType = cast<FixedVectorType>(T);
+      break;
+    }
+  }
+
----------------
dominik-steenken wrote:

I'm just thinking that, whether or not the operand vector is full is probably going to be a factor in many of the reductions. So it seems wrong to have it just in the branch for `vector.reduce.add`. I do see your point though about keeping things as simple as possible. I'm going to refactor things a bit to make things simpler, but kepp some of the extensibility in case we need to handle more intrinsics.

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


More information about the llvm-commits mailing list