[LV] ExtractValue instruction costs

[SjoerdMeijer]

Summary

Instruction ExtractValue wasn't handled in LoopVectorizationCostModel::getInstructionCost(). As a result, it was modeled as a mul which may not be really accurate. Since it typically takes 1 instruction to extract a value from a vector we now model this with a cost of 1.

This is a follow-up of D92208, that required changing this regression test.

[fhahn]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

6858

Better to use TTI.getVectorInstrCost? And perhaps also handle InsertValue?

[SjoerdMeijer]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

6858

Yeah, cheers, will look/do that.

[SjoerdMeijer]

Decided to use TTI.getInstructionCost because getVectorInstrCost doesn't work here; it's an aggregate type, not really a vector.
That means the cost for a ExtractValue will be Free now, which I think is actually more accurate (than 1, and also the original 4).
I haven't added InsertValue yet, because that is e.g. not supported by getInstructionCost as it seems a little bit of a different beast.

[fhahn]

In D92317#2423288, @SjoerdMeijer wrote:

Decided to use TTI.getInstructionCost because getVectorInstrCost doesn't work here; it's an aggregate type, not really a vector.
That means the cost for a ExtractValue will be Free now, which I think is actually more accurate (than 1, and also the original 4).
I haven't added InsertValue yet, because that is e.g. not supported by getInstructionCost as it seems a little bit of a different beast.

SGTM, I was thinking about *insertelement*... Sorry about that.

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

6858

Should this also handle InsertValue for completeness?

[SjoerdMeijer]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

6858

That doesn't seem to be supported by getInstructionCost, which means it returns -1 for InsertValue, which is probably not what we want. So, I think InsertValue needs a bit looking into, which probably best done separately? Although don't mind doing it part of this....

[fhahn]

LGTM, we should be able to rely on TTI for the cost. Also handling InsertValue as a follow-up would be great!

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

6858

oh yes, just saw that. Probably best to do it separately.

[SjoerdMeijer]

Thanks, and I will look into InsertValue too. That may take a few days, but will certainly do that.

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

unsigned LoopVectorizationCostModel::getInstructionCost(Instruction *I,
   case Instruction::Call: {
     bool NeedToScalarize;
     CallInst *CI = cast<CallInst>(I);
     unsigned CallCost = getVectorCallCost(CI, VF, NeedToScalarize);
     if (getVectorIntrinsicIDForCall(CI, TLI))
       return std::min(CallCost, getVectorIntrinsicCost(CI, VF));
     return CallCost;
   }
+  case Instruction::ExtractValue:
+    return TTI.getInstructionCost(I, TTI::TCK_RecipThroughput);
   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.getKnownMinValue() * TTI.getArithmeticInstrCost(
                                        Instruction::Mul, VectorTy, CostKind) +
            getScalarizationOverhead(I, VF);
   } // end of switch.
 }
 

llvm/test/Transforms/LoopVectorize/AArch64/extractvalue-no-scalarization-required.ll

 ; REQUIRES: asserts
 
 ; 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
 
 ; Test case from PR41294.
 
 ; Check scalar cost for extractvalue. 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: Scalar loop costs: 7.
-; CM: LV: Found an estimated cost of 19 for VF 2 For instruction:   %a = extractvalue { i64, i64 } %sv, 0
-; CM-NEXT: LV: Found an estimated cost of 19 for VF 2 For instruction:   %b = extractvalue { i64, i64 } %sv, 1
+; CM: LV: Scalar loop costs: 5.
+; CM: LV: Found an estimated cost of 0 for VF 2 For instruction:   %a = extractvalue { i64, i64 } %sv, 0
+; CM-NEXT: LV: Found an estimated cost of 0 for VF 2 For instruction:   %b = extractvalue { i64, i64 } %sv, 1
 
 ; Check that the extractvalue 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:    %0 = add i32 %index, 0
 ; FORCED-NEXT:    %1 = extractvalue { i64, i64 } %sv, 0
 ; FORCED-NEXT:    %2 = extractvalue { i64, i64 } %sv, 0

 exit:
   ret void
 }
 
 
 ; Similar to the test case above, but checks getVectorCallCost as well.
 declare float @pow(float, float) readnone nounwind
 
-; CM: LV: Scalar loop costs: 16.
-; CM: LV: Found an estimated cost of 5 for VF 2 For instruction:   %a = extractvalue { float, float } %sv, 0
-; CM-NEXT: LV: Found an estimated cost of 5 for VF 2 For instruction:   %b = extractvalue { float, float } %sv, 1
+; CM: LV: Scalar loop costs: 14.
+; CM: LV: Found an estimated cost of 0 for VF 2 For instruction:   %a = extractvalue { float, float } %sv, 0
+; CM-NEXT: LV: Found an estimated cost of 0 for VF 2 For instruction:   %b = extractvalue { float, float } %sv, 1
 
 ; FORCED-LABEL: define void @test_getVectorCallCost
 
 ; FORCED-LABEL: vector.body:                                      ; preds = %vector.body, %vector.ph
 ; FORCED-NEXT:    %index = phi i32 [ 0, %vector.ph ], [ %index.next, %vector.body ]
 ; FORCED-NEXT:    %0 = add i32 %index, 0
 ; FORCED-NEXT:    %1 = extractvalue { float, float } %sv, 0
 ; FORCED-NEXT:    %2 = extractvalue { float, float } %sv, 0
loop.body:
   store float %p, float* %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
 }
-
-
-