[LV] Exclude loop-invariant inputs from scalar cost computation.

[fhahn]

Summary

Loop invariant operands do not need to be scalarized, as we are using
the values outside the loop. We should ignore them when computing the
scalarization overhead.

Fixes PR41294

[Ayal]

The culprit here is the assumption made by TTI.getOperandsScalarizationOverhead(Operands, VF) that all its Operands will be vectorized according to VF, and would thus require extraction to feed a scalarized/replicated user. But any such Operand might not get vectorized, and possibly must not get vectorized, e.g., due to an incompatible type as demonstrated by PR41294 and the testcase. In some cases an Operand will obviously not be vectorized, such as if it's loop-invariant or live-in. More generally, LV uses the following:

auto needsExtract = [&](Instruction *I) -> bool {
  return TheLoop->contains(I) && !isScalarAfterVectorization(I, VF);
};

which would require passing not only TheLoop into getScalarizationOverhead(I, VF, TTI) but also the CM --- better turn this static function into a method of CM?

Note that there's also CM.isProfitableToScalarize(I, VF)), but it relies on having computed costs, so difficult to use when computing the cost (of a User). Skipping it would only affect accuracy of resulting cost, considering Operands that can be vectorized but will not be due to profitability.

Fixing getVectorCallCost deserves another testcase.
Seems like getVectorIntrinsicCost also requires fixing?

Would be good to hoist such invariant instructions

%a = extractvalue { i64, i64 } %sv, 0
%b = extractvalue { i64, i64 } %sv, 1

out of the loop before LV, or at-least have LV recognize them as uniform.

llvm/test/Transforms/LoopVectorize/AArch64/instructions-with-struct-ops.ll

3

Record PR41294 in comment, file name, or testcase function name.

4

for extractelement >> for extractvalue

10

the extractelement >> the extractvalue

[fhahn]

Thanks for your comments Ayal! I'll update the patch in a bit.

[fhahn]

Split out moving various functions to LoopVectorizationCostModel to D61638.

Hoisted needsExtract to do the check if we need to extract/scalarize an operand
to LoopVectorizationCostModel and extend it to check for loop invariant operands.

[fhahn]

In D59995#1455952, @Ayal wrote:

The culprit here is the assumption made by TTI.getOperandsScalarizationOverhead(Operands, VF) that all its Operands will be vectorized according to VF, and would thus require extraction to feed a scalarized/replicated user. But any such Operand might not get vectorized, and possibly must not get vectorized, e.g., due to an incompatible type as demonstrated by PR41294 and the testcase. In some cases an Operand will obviously not be vectorized, such as if it's loop-invariant or live-in. More generally, LV uses the following:

auto needsExtract = [&](Instruction *I) -> bool {
  return TheLoop->contains(I) && !isScalarAfterVectorization(I, VF);
};

which would require passing not only TheLoop into getScalarizationOverhead(I, VF, TTI) but also the CM --- better turn this static function into a method of CM?

Done.

Note that there's also CM.isProfitableToScalarize(I, VF)), but it relies on having computed costs, so difficult to use when computing the cost (of a User). Skipping it would only affect accuracy of resulting cost, considering Operands that can be vectorized but will not be due to profitability.

Fixing getVectorCallCost deserves another testcase.

Added a test case with call. But thinking about it again, it does not really test the issue. Not sure it is actually possible to test getVectorCallCost, as there are no vector call functions that take struct values?

Seems like getVectorIntrinsicCost also requires fixing?

Yep, I'll update it in a second. In practice, I don't think there are any intrinsics that take struct types, but maybe in the future it might become a problem.

Would be good to hoist such invariant instructions

%a = extractvalue { i64, i64 } %sv, 0
%b = extractvalue { i64, i64 } %sv, 1

out of the loop before LV, or at-least have LV recognize them as uniform.

Yep, I can look into that as a follow up. LICM should hoist those things, but I think in general we cannot guarantee that all loop-invariant instructions are hoisted out before LoopVectorize (the test case came from a fuzzer). Do you think we should try to hoist them in LV? I would assume a later run of LICM will hoist them.

[fhahn]

Filter operands for getIntrinsicCallCost as well.

[fhahn]

Ping. Ayal, does this address your comments appropriately?

[Ayal]

In D59995#1493241, @fhahn wrote:

In D59995#1455952, @Ayal wrote:

The culprit here is the assumption made by TTI.getOperandsScalarizationOverhead(Operands, VF) that all its Operands will be vectorized according to VF, and would thus require extraction to feed a scalarized/replicated user. But any such Operand might not get vectorized, and possibly must not get vectorized, e.g., due to an incompatible type as demonstrated by PR41294 and the testcase. In some cases an Operand will obviously not be vectorized, such as if it's loop-invariant or live-in. More generally, LV uses the following:

auto needsExtract = [&](Instruction *I) -> bool {
  return TheLoop->contains(I) && !isScalarAfterVectorization(I, VF);
};

which would require passing not only TheLoop into getScalarizationOverhead(I, VF, TTI) but also the CM --- better turn this static function into a method of CM?

Done.

Very good, thanks for the refactoring.

Note that there's also CM.isProfitableToScalarize(I, VF)), but it relies on having computed costs, so difficult to use when computing the cost (of a User). Skipping it would only affect accuracy of resulting cost, considering Operands that can be vectorized but will not be due to profitability.

Fixing getVectorCallCost deserves another testcase.

Added a test case with call. But thinking about it again, it does not really test the issue. Not sure it is actually possible to test getVectorCallCost, as there are no vector call functions that take struct values?

Good point. Does the test case pass w/o this patch?

Seems like getVectorIntrinsicCost also requires fixing?

Yep, I'll update it in a second. In practice, I don't think there are any intrinsics that take struct types, but maybe in the future it might become a problem.

Would be good to hoist such invariant instructions

%a = extractvalue { i64, i64 } %sv, 0
%b = extractvalue { i64, i64 } %sv, 1

out of the loop before LV, or at-least have LV recognize them as uniform.

Yep, I can look into that as a follow up. LICM should hoist those things, but I think in general we cannot guarantee that all loop-invariant instructions are hoisted out before LoopVectorize (the test case came from a fuzzer). Do you think we should try to hoist them in LV? I would assume a later run of LICM will hoist them.

The thought was to check LICM, and check LV's uniform analysis.

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

1309

This is admittedly part of the original comment, but while we're here - "cannot be scalarized" >> "is expected to be vectorized" - every V can be scalarized. Furthermore, if we're unsure whether V will be vectorized or not it's safer to assume that it will be scalarized rather than vectorized, as the latter might not be possible. So if Scalars.find(VF) isn't set yet, which will cause isScalarAfterVectorization() to assert, better have needsExtract() return false.

5662

Yes, there's a phase ordering issue here, and also one regarding isProfitableToScalarize() as mentioned above (which would be good to note somewhere). But the instruction(s) that may trigger an assert, being loads or stores, are the operands of I rather than I itself, right? In any case, suggest to have FilterOperands()/needsExtract() understand if they can use isScalarAfterVectorization() or else be conservative, as noted above. Perhaps make Filter[Extracting]Operands() a method, to be reused.

[fhahn]

Addressed Ayal's comments, thanks and sorry for the long delay, it somehow dropped off my radar.

In D59995#1502129, @Ayal wrote:

Good point. Does the test case pass w/o this patch?

Yes both tests fail with the patch.

The only thing that cannot be tested is the combination of intrinsic taking struct
values, but that should be fine.

Seems like getVectorIntrinsicCost also requires fixing?

Yep, I'll update it in a second. In practice, I don't think there are any intrinsics that take struct types, but maybe in the future it might become a problem.

Would be good to hoist such invariant instructions

%a = extractvalue { i64, i64 } %sv, 0
%b = extractvalue { i64, i64 } %sv, 1

out of the loop before LV, or at-least have LV recognize them as uniform.

Yep, I can look into that as a follow up. LICM should hoist those things, but I think in general we cannot guarantee that all loop-invariant instructions are hoisted out before LoopVectorize (the test case came from a fuzzer). Do you think we should try to hoist them in LV? I would assume a later run of LICM will hoist them.

The thought was to check LICM, and check LV's uniform analysis.

I am not entirely sure I understand what you mean here. I checked LICM and it will
hoist the problematic case here, but I do not think we should rely on that, as
there might be other reasons why LICM decided against hoisting (or LICM is not run).

Currently they are not marked as uniform in LV, but I can fix that in a follow-up
patch.

[Ayal]

Some format typos, and clarifying if needsExtract() should assume vectorized or scalarized before scalars are computed.

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

1171

// >> /// on last line too ;-)

1173

indentation?

1317

Ahh, is it better to assume we can vectorize V, contrary to the above comment?

3138

overhed >> overhead.

4113

one line?

5661

two lines?

5678

Effectively initializing Ops to an empty range by default? This is an orthogonal refactoring; consider instead having another early-exit:

if (isa<StoreInst>(I) && TTI.supportsEfficientVectorElementLoadStore())
  return Cost;

and initializing Ops to I->operands() by default.

6621

one line?

[fhahn]

clang-format, small refactor

[fhahn]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

1317

Ahh, is it better to assume we can vectorize V, contrary to the above comment?

Ah sorry for the change, I missed adding an explanation here. Assuming vectorization here seems to be in line with the old behavior and switching it causes a bunch of unit-test failures. I agree it would be safer to assume scalarizing, but the current cost model seems to expect the optimistic choice here.

I might be worth investigating flipping it as a follow-up, but I think it will require some additional work to make sure other places in the cost model are in sync with the new assumption?

[Ayal]

LGTM, with some additional thoughts provoked by this fix.

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

1317

OK, fair enough. The current optimistic choice is ok, as long as V is of vectorizable type; and the latter seems to be the case given that setCostBasedWidenDecision() deals with loads and stores only, and LVLegality checks that their relevant operands (stored values) are of vectorizable types. Worth augmenting the explanation if agreed.

In any case, the optimistic choice can be confined to guard isScalarAfterVectorization() only, e.g.,:

if (VF <= 1)
  return false;
Instruction *I = dyn_cast<Instruction>(V);
if (!I || !TheLoop->contains(I) || TheLoop->isLoopInvariant(I))
  return false;
return (Scalars.find(VF) == Scalars.end() || !isScalarAfterVectorization(I, VF));

On a related note, notice that if %sv were a <2 x i64> vector instead of a {i64, i64} struct, LVLegality would bail out given that we can't vectorize its associated extractelement:

// Also, we can't vectorize extractelement instructions.
if ((!VectorType::isValidElementType(I.getType()) &&
     !I.getType()->isVoidTy()) ||
    isa<ExtractElementInst>(I)) {
  reportVectorizationFailure("Found unvectorizable type",
      "instruction return type cannot be vectorized",
      "CantVectorizeInstructionReturnType", &I);
  return false;
}

but it does allow vectorizing (loops with scalarizing) extractvalue. So a crude way of "fixing" the testcase would be to treat extractvalue similar to extractelement... but its clearly better to allow vectorizing such loops. OTOH, perhaps we could also allow vectorizing (loops with scalarizing) extractelement...? (Surely in a separate patch if so)

[fhahn]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

1317

OK, fair enough. The current optimistic choice is ok, as long as V is of vectorizable type; and the latter seems to be the case given that setCostBasedWidenDecision() deals with loads and stores only, and LVLegality checks that their relevant operands (stored values) are of vectorizable types. Worth augmenting the explanation if agreed.

In any case, the optimistic choice can be confined to guard isScalarAfterVectorization()

Thanks, I'll commit the change with the moved optimistic check as suggested and add a comment.

On a related note, notice that if %sv were a <2 x i64> vector instead of a {i64, i64} struct, LVLegality would bail out given that we can't vectorize its associated extractelement:

I'll look into that as a follow up, same as marking the invariant instructions as uniform.

[fhahn]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

1317

OK, fair enough. The current optimistic choice is ok, as long as V is of vectorizable type; and the latter seems to be the case given that setCostBasedWidenDecision() deals with loads and stores only, and LVLegality checks that their relevant operands (stored values) are of vectorizable types. Worth augmenting the explanation if agreed.

I think vectorization should be what is happening in most of those cases, but I think there could be scenarios where we would not vectorize the operands. I've adjusted the comment accordingly.

[mstorsjo]

This triggers failed asserts in some cases, reproable with https://martin.st/temp/loadimage-preproc.cpp.xz, with clang++ -target i686-w64-mingw32 -c -O3 loadimage-preproc.cpp. Will post a proper bug report later today, unless someone else beats me to it.

[fhahn]

In D59995#1584967, @mstorsjo wrote:

This triggers failed asserts in some cases, reproable with https://martin.st/temp/loadimage-preproc.cpp.xz, with clang++ -target i686-w64-mingw32 -c -O3 loadimage-preproc.cpp. Will post a proper bug report later today, unless someone else beats me to it.

Thanks for reporting the issue, should be fixed in rL366049.

@Ayal, the problem was that we are not computing the scalarized cost in getVectorIntrinsicCost and TTI::getIntrinsicInstrCost expects the full list of arguments, so we should not filter it. I'll check if there's a reason we do not consider the scalarized cost there.

[Ayal]

llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp

1175 ↗

(On Diff #209747)

Thanks @fhahn, just pointing out the above documentation is also inaccurate, along with the associated cost.

[mstorsjo]

In D59995#1585031, @fhahn wrote:

In D59995#1584967, @mstorsjo wrote:

This triggers failed asserts in some cases, reproable with https://martin.st/temp/loadimage-preproc.cpp.xz, with clang++ -target i686-w64-mingw32 -c -O3 loadimage-preproc.cpp. Will post a proper bug report later today, unless someone else beats me to it.

Thanks for reporting the issue, should be fixed in rL366049.

Thanks for the very quick fix - it seems to work fine now!

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

 private:
   unsigned NumPredStores = 0;
 
   /// \return An upper bound for the vectorization factor, larger than zero.
   /// One is returned if vectorization should best be avoided due to cost.
   unsigned computeFeasibleMaxVF(bool OptForSize, unsigned ConstTripCount);
 
   /// The vectorization cost is a combination of the cost itself and a boolean
   /// indicating whether any of the contributing operations will actually
   /// operate on
   /// vector values after type legalization in the backend. If this latter value
   /// is
   /// false, then all operations will be scalarized (i.e. no vectorization has
   /// actually taken place).
   using VectorizationCostTy = std::pair<unsigned, bool>;
 
   /// Returns the expected execution cost. The unit of the cost does
   /// not matter because we use the 'cost' units to compare different
   /// vector widths. The cost that is returned is *not* normalized by
private:
 
   /// Keeps cost model vectorization decision and cost for instructions.
   /// Right now it is used for memory instructions only.
   using DecisionList = DenseMap<std::pair<Instruction *, unsigned>,
                                 std::pair<InstWidening, unsigned>>;
 
   DecisionList WideningDecisions;
 
 public:
   /// The loop that we evaluate.
   Loop *TheLoop;
 
   /// Predicated scalar evolution analysis.
   PredicatedScalarEvolution &PSE;
 
   /// Loop Info analysis.
   LoopInfo *LI;
 
   /// Vectorization legality.
   LoopVectorizationLegality *Legal;
 
   /// Vector target information.
   const TargetTransformInfo &TTI;
 
   /// Target Library Info.
for (BasicBlock::iterator I = BB->begin(), E = BB->end(); I != E;) {
 
     CSEMap[In] = In;
   }
 }
 
 /// Estimate the overhead of scalarizing an instruction. This is a
 /// convenience wrapper for the type-based getScalarizationOverhead API.
 static unsigned getScalarizationOverhead(Instruction *I, unsigned VF,
+                                         Loop *OrigLoop,
                                          const TargetTransformInfo &TTI) {
   if (VF == 1)
     return 0;
 
   unsigned Cost = 0;
   Type *RetTy = ToVectorTy(I->getType(), VF);
   if (!RetTy->isVoidTy() &&
       (!isa<LoadInst>(I) ||
        !TTI.supportsEfficientVectorElementLoadStore()))
     Cost += TTI.getScalarizationOverhead(RetTy, true, false);
 
   // Some targets keep addresses scalar.
   if (isa<LoadInst>(I) && !TTI.prefersVectorizedAddressing())
     return Cost;
 
+  // Skip loop-invariant operands, as they do not incur any scalarization
+  // overhead.
+  auto FilterOperands =
+      [OrigLoop](
+          Instruction::const_op_range Ops) -> SmallVector<const Value *, 4> {
+    return SmallVector<const Value *, 4>(
+        make_filter_range(Ops, [OrigLoop](const Value *V) {
+          return !OrigLoop->isLoopInvariant(V);
+        }));
+  };
+
   if (CallInst *CI = dyn_cast<CallInst>(I)) {
-    SmallVector<const Value *, 4> Operands(CI->arg_operands());
-    Cost += TTI.getOperandsScalarizationOverhead(Operands, VF);
+    Cost += TTI.getOperandsScalarizationOverhead(
+        FilterOperands(CI->arg_operands()), VF);
   }
   else if (!isa<StoreInst>(I) ||
            !TTI.supportsEfficientVectorElementLoadStore()) {
-    SmallVector<const Value *, 4> Operands(I->operand_values());
-    Cost += TTI.getOperandsScalarizationOverhead(Operands, VF);
+    Cost +=
+        TTI.getOperandsScalarizationOverhead(FilterOperands(I->operands()), VF);
   }
 
   return Cost;
 }
 
 // Estimate cost of a call instruction CI if it were vectorized with factor VF.
 // Return the cost of the instruction, including scalarization overhead if it's
 // needed. The flag NeedToScalarize shows if the call needs to be scalarized -
 // i.e. either vector version isn't available, or is too expensive.
-static unsigned getVectorCallCost(CallInst *CI, unsigned VF,
+static unsigned getVectorCallCost(CallInst *CI, unsigned VF, Loop *TheLoop,
                                   const TargetTransformInfo &TTI,
                                   const TargetLibraryInfo *TLI,
                                   bool &NeedToScalarize) {
   Function *F = CI->getCalledFunction();
   StringRef FnName = CI->getCalledFunction()->getName();
   Type *ScalarRetTy = CI->getType();
   SmallVector<Type *, 4> Tys, ScalarTys;
   for (auto &ArgOp : CI->arg_operands())
static unsigned getVectorCallCost(CallInst *CI, unsigned VF, Loop *TheLoop,
 
   // Compute corresponding vector type for return value and arguments.
   Type *RetTy = ToVectorTy(ScalarRetTy, VF);
   for (Type *ScalarTy : ScalarTys)
     Tys.push_back(ToVectorTy(ScalarTy, VF));
 
   // Compute costs of unpacking argument values for the scalar calls and
   // packing the return values to a vector.
-  unsigned ScalarizationCost = getScalarizationOverhead(CI, VF, TTI);
+  unsigned ScalarizationCost = getScalarizationOverhead(CI, VF, TheLoop, TTI);
 
   unsigned Cost = ScalarCallCost * VF + ScalarizationCost;
 
   // If we can't emit a vector call for this function, then the currently found
   // cost is the cost we need to return.
   NeedToScalarize = true;
   if (!TLI || !TLI->isFunctionVectorizable(FnName, VF) || CI->isNoBuiltin())
     return Cost;
static unsigned getVectorIntrinsicCost(CallInst *CI, unsigned VF,
   assert(ID && "Expected intrinsic call!");
 
   FastMathFlags FMF;
   if (auto *FPMO = dyn_cast<FPMathOperator>(CI))
     FMF = FPMO->getFastMathFlags();
 
   SmallVector<Value *, 4> Operands(CI->arg_operands());
   return TTI.getIntrinsicInstrCost(ID, CI->getType(), Operands, FMF, VF);
 }
 
 static Type *smallestIntegerVectorType(Type *T1, Type *T2) {
   auto *I1 = cast<IntegerType>(T1->getVectorElementType());
   auto *I2 = cast<IntegerType>(T2->getVectorElementType());
   return I1->getBitWidth() < I2->getBitWidth() ? T1 : T2;
 }
 static Type *largestIntegerVectorType(Type *T1, Type *T2) {
   auto *I1 = cast<IntegerType>(T1->getVectorElementType());
for (Value *ArgOperand : CI->arg_operands())
       Tys.push_back(ToVectorTy(ArgOperand->getType(), VF));
 
     Intrinsic::ID ID = getVectorIntrinsicIDForCall(CI, TLI);
 
     // The flag shows whether we use Intrinsic or a usual Call for vectorized
     // version of the instruction.
     // Is it beneficial to perform intrinsic call compared to lib call?
     bool NeedToScalarize;
-    unsigned CallCost = getVectorCallCost(CI, VF, *TTI, TLI, NeedToScalarize);
+    unsigned CallCost =
+        getVectorCallCost(CI, VF, OrigLoop, *TTI, TLI, NeedToScalarize);
     bool UseVectorIntrinsic =
         ID && getVectorIntrinsicCost(CI, VF, *TTI, TLI) <= CallCost;
     assert((UseVectorIntrinsic || !NeedToScalarize) &&
            "Instruction should be scalarized elsewhere.");
 
     for (unsigned Part = 0; Part < UF; ++Part) {
       SmallVector<Value *, 4> Args;
       for (unsigned i = 0, ie = CI->getNumArgOperands(); i != ie; ++i) {
unsigned LoopVectorizationCostModel::getMemInstScalarizationCost(Instruction *I,
   // Don't pass *I here, since it is scalar but will actually be part of a
   // vectorized loop where the user of it is a vectorized instruction.
   Cost += VF *
           TTI.getMemoryOpCost(I->getOpcode(), ValTy->getScalarType(), Alignment,
                               AS);
 
   // Get the overhead of the extractelement and insertelement instructions
   // we might create due to scalarization.
-  Cost += getScalarizationOverhead(I, VF, TTI);
+  Cost += getScalarizationOverhead(I, VF, TheLoop, TTI);
 
   // If we have a predicated store, it may not be executed for each vector
   // lane. Scale the cost by the probability of executing the predicated
   // block.
   if (isPredicatedInst(I)) {
     Cost /= getReciprocalPredBlockProb();
 
     if (useEmulatedMaskMemRefHack(I))
LoopVectorizationCostModel::getInstructionCost(Instruction *I, unsigned VF) {
   Type *VectorTy;
   unsigned C = getInstructionCost(I, VF, VectorTy);
 
   bool TypeNotScalarized =
       VF > 1 && VectorTy->isVectorTy() && TTI.getNumberOfParts(VectorTy) < VF;
   return VectorizationCostTy(C, TypeNotScalarized);
 }
 
 void LoopVectorizationCostModel::setCostBasedWideningDecision(unsigned VF) {
   if (VF == 1)
     return;
   NumPredStores = 0;
   for (BasicBlock *BB : TheLoop->blocks()) {
     // For each instruction in the old loop.
     for (Instruction &I : *BB) {
       Value *Ptr =  getLoadStorePointerOperand(&I);
       if (!Ptr)
         continue;
 
       // TODO: We should generate better code and update the cost model for
       // predicated uniform stores. Today they are treated as any other
       // predicated store (see added test cases in
       // invariant-store-vectorization.ll).
       if (isa<StoreInst>(&I) && isScalarWithPredication(&I))
         NumPredStores++;
 
       if (Legal->isUniform(Ptr) &&
           // Conditional loads and stores should be scalarized and predicated.
           // isScalarWithPredication cannot be used here since masked
           // gather/scatters are not considered scalar with predication.
           !Legal->blockNeedsPredication(I.getParent())) {
         // TODO: Avoid replicating loads and stores instead of
         // relying on instcombine to remove them.
         // Load: Scalar load + broadcast
if (VF > 1 && isScalarWithPredication(I)) {
       // models a copy at the end of each predicated block.
       Cost += VF * TTI.getCFInstrCost(Instruction::PHI);
 
       // The cost of the non-predicated instruction.
       Cost += VF * TTI.getArithmeticInstrCost(I->getOpcode(), RetTy);
 
       // The cost of insertelement and extractelement instructions needed for
       // scalarization.
-      Cost += getScalarizationOverhead(I, VF, TTI);
+      Cost += getScalarizationOverhead(I, VF, TheLoop, TTI);
 
       // Scale the cost by the probability of executing the predicated blocks.
       // This assumes the predicated block for each vector lane is equally
       // likely.
       return Cost / getReciprocalPredBlockProb();
     }
     LLVM_FALLTHROUGH;
   case Instruction::Add:
case Instruction::BitCast: {
     }
 
     unsigned N = isScalarAfterVectorization(I, VF) ? VF : 1;
     return N * TTI.getCastInstrCost(I->getOpcode(), VectorTy, SrcVecTy, I);
   }
   case Instruction::Call: {
     bool NeedToScalarize;
     CallInst *CI = cast<CallInst>(I);
-    unsigned CallCost = getVectorCallCost(CI, VF, TTI, TLI, NeedToScalarize);
+    unsigned CallCost =
+        getVectorCallCost(CI, VF, TheLoop, TTI, TLI, NeedToScalarize);
     if (getVectorIntrinsicIDForCall(CI, TLI))
       return std::min(CallCost, getVectorIntrinsicCost(CI, VF, TTI, TLI));
     return CallCost;
   }
   default:
     // The cost of executing VF copies of the scalar instruction. This opcode
     // is unknown. Assume that it is the same as 'mul'.
     return VF * TTI.getArithmeticInstrCost(Instruction::Mul, VectorTy) +
-           getScalarizationOverhead(I, VF, TTI);
+           getScalarizationOverhead(I, VF, TheLoop, TTI);
   } // end of switch.
 }
 
 char LoopVectorize::ID = 0;
 
 static const char lv_name[] = "Loop Vectorization";
 
 INITIALIZE_PASS_BEGIN(LoopVectorize, LV_NAME, lv_name, false, false)
if (!isa<PHINode>(I) && (CM.isScalarAfterVectorization(I, VF) ||
       return false;
     if (CallInst *CI = dyn_cast<CallInst>(I)) {
       Intrinsic::ID ID = getVectorIntrinsicIDForCall(CI, TLI);
       // The following case may be scalarized depending on the VF.
       // The flag shows whether we use Intrinsic or a usual Call for vectorized
       // version of the instruction.
       // Is it beneficial to perform intrinsic call compared to lib call?
       bool NeedToScalarize;
-      unsigned CallCost = getVectorCallCost(CI, VF, *TTI, TLI, NeedToScalarize);
+      unsigned CallCost =
+          getVectorCallCost(CI, VF, OrigLoop, *TTI, TLI, NeedToScalarize);
       bool UseVectorIntrinsic =
           ID && getVectorIntrinsicCost(CI, VF, *TTI, TLI) <= CallCost;
       return UseVectorIntrinsic || !NeedToScalarize;
     }
     if (isa<LoadInst>(I) || isa<StoreInst>(I)) {
       assert(CM.getWideningDecision(I, VF) ==
                  LoopVectorizationCostModel::CM_Scalarize &&
              "Memory widening decisions should have been taken care by now");

llvm/test/Transforms/LoopVectorize/AArch64/instructions-with-struct-ops.ll

+; RUN: opt -loop-vectorize -mtriple=arm64-apple-ios %s -S -debug -disable-output 2>&1 | FileCheck --check-prefix=CM %s
+; RUN: opt -loop-vectorize -force-vector-width=2 -force-vector-interleave=1 %s -S | FileCheck --check-prefix=FORCED %s
+
+; Check scalar cost for extractelement. The constant and loop invariant operands are free,
+; leaving cost 3 for scalarizing the result + 2 for executing the op with VF 2.
+
+; CM: LV: Found an estimated cost of 5 for VF 2 For instruction:   %a = extractvalue { i64, i64 } %sv, 0
+; CM-NEXT: LV: Found an estimated cost of 5 for VF 2 For instruction:   %b = extractvalue { i64, i64 } %sv, 1
+
+; Check that the extractlement operands are actually free in vector code.
+
+; FORCED-LABEL: vector.body:                                      ; preds = %vector.body, %vector.ph
+; FORCED-NEXT:    %index = phi i32 [ 0, %vector.ph ], [ %index.next, %vector.body ]
+; FORCED-NEXT:    %broadcast.splatinsert = insertelement <2 x i32> undef, i32 %index, i32 0
+; FORCED-NEXT:    %broadcast.splat = shufflevector <2 x i32> %broadcast.splatinsert, <2 x i32> undef, <2 x i32> zeroinitializer
+; FORCED-NEXT:    %induction = add <2 x i32> %broadcast.splat, <i32 0, i32 1>
+; FORCED-NEXT:    %0 = add i32 %index, 0
+; FORCED-NEXT:    %1 = extractvalue { i64, i64 } %sv, 0
+; FORCED-NEXT:    %2 = extractvalue { i64, i64 } %sv, 0
+; FORCED-NEXT:    %3 = insertelement <2 x i64> undef, i64 %1, i32 0
+; FORCED-NEXT:    %4 = insertelement <2 x i64> %3, i64 %2, i32 1
+; FORCED-NEXT:    %5 = extractvalue { i64, i64 } %sv, 1
+; FORCED-NEXT:    %6 = extractvalue { i64, i64 } %sv, 1
+; FORCED-NEXT:    %7 = insertelement <2 x i64> undef, i64 %5, i32 0
+; FORCED-NEXT:    %8 = insertelement <2 x i64> %7, i64 %6, i32 1
+; FORCED-NEXT:    %9 = getelementptr i64, i64* %dst, i32 %0
+; FORCED-NEXT:    %10 = add <2 x i64> %4, %8
+; FORCED-NEXT:    %11 = getelementptr i64, i64* %9, i32 0
+; FORCED-NEXT:    %12 = bitcast i64* %11 to <2 x i64>*
+; FORCED-NEXT:    store <2 x i64> %10, <2 x i64>* %12, align 4
+; FORCED-NEXT:    %index.next = add i32 %index, 2
+; FORCED-NEXT:    %13 = icmp eq i32 %index.next, 0
+; FORCED-NEXT:    br i1 %13, label %middle.block, label %vector.body, !llvm.loop !0
+
+define void @test1(i64* %dst, {i64, i64} %sv) {
+entry:
+  br label %loop.body
+
+loop.body:
+  %iv = phi i32 [ 0, %entry ], [ %iv.next, %loop.body ]
+  %a = extractvalue { i64, i64 } %sv, 0
+  %b = extractvalue { i64, i64 } %sv, 1
+  %addr = getelementptr i64, i64* %dst, i32 %iv
+  %add = add i64 %a, %b
+  store i64 %add, i64* %addr
+  %iv.next = add nsw i32 %iv, 1
+  %cond = icmp ne i32 %iv.next, 0
+  br i1 %cond, label %loop.body, label %exit
+
+exit:
+  ret void
+}