[LV] Pass compare predicate to getCmpSelInstrCost.

[sdesmalen]

Summary

If the condition of a select is a compare, pass its predicate to
TTI::getCmpSelInstrCost to get a more accurate cost value instead
of passing BAD_ICMP_PREDICATE.

I noticed that the commit message from D90070 had a comment about the
vectorized select predicate possibly being composed of other compares with
different predicate values, but I wasn't able to construct an example
where this was an actual issue. If this is an issue, I guess we could
add another check that the block isn't predicated for any reason.

[dmgreen]

Makes sense to me, but I'm not sure what the original comment about being composed of different predicates meant. The aarch64 backend is already doing this from I in some situations, but won't if the vector and scalar types do not match.

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

7588–7589

Should this use the correct predicate too?

[sdesmalen]

Pass predicate to getCmpSelInstrCost for ICmp/FCmp as well.

[sdesmalen]

In D114646#3160680, @dmgreen wrote:

Makes sense to me, but I'm not sure what the original comment about being composed of different predicates meant.

From what I can see, there's no tests that fail if we pass in the extra information, so even if this was the desired behaviour it isn't protected by any tests.
Without clarification of that comment, I think we should pursue the thing in code that seems most sensible.

[fhahn]

LGTM, thanks! D90070 already mentioned that this should also be used in LV.

In D114646#3172854, @sdesmalen wrote:

In D114646#3160680, @dmgreen wrote:

Makes sense to me, but I'm not sure what the original comment about being composed of different predicates meant.

From what I can see, there's no tests that fail if we pass in the extra information, so even if this was the desired behaviour it isn't protected by any tests.
Without clarification of that comment, I think we should pursue the thing in code that seems most sensible.

The comment referred to cases where there may not exist a context instruction (passed to getCmpSelInstrCost) or it may use a different predicate, but this shouldn't apply to the cases here.

[dmgreen]

Cheers

[sdesmalen]

Thanks both!

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

if (!ScalarCond && (match(I, m_LogicalAnd(m_Value(Op0), m_Value(Op1))) ||
       return TTI.getArithmeticInstrCost(
           match(I, m_LogicalOr()) ? Instruction::Or : Instruction::And, VectorTy,
           CostKind, Op1VK, Op2VK, Op1VP, Op2VP, Operands, I);
     }
 
     Type *CondTy = SI->getCondition()->getType();
     if (!ScalarCond)
       CondTy = VectorType::get(CondTy, VF);
-    return TTI.getCmpSelInstrCost(I->getOpcode(), VectorTy, CondTy,
-                                  CmpInst::BAD_ICMP_PREDICATE, CostKind, I);
+
+    CmpInst::Predicate Pred = CmpInst::BAD_ICMP_PREDICATE;
+    if (auto *Cmp = dyn_cast<CmpInst>(SI->getCondition()))
+      Pred = Cmp->getPredicate();
+    return TTI.getCmpSelInstrCost(I->getOpcode(), VectorTy, CondTy, Pred,
+                                  CostKind, I);
   }
   case Instruction::ICmp:
   case Instruction::FCmp: {
     Type *ValTy = I->getOperand(0)->getType();
     Instruction *Op0AsInstruction = dyn_cast<Instruction>(I->getOperand(0));
     if (canTruncateToMinimalBitwidth(Op0AsInstruction, VF))
       ValTy = IntegerType::get(ValTy->getContext(), MinBWs[Op0AsInstruction]);
     VectorTy = ToVectorTy(ValTy, VF);
     return TTI.getCmpSelInstrCost(I->getOpcode(), VectorTy, nullptr,
-                                  CmpInst::BAD_ICMP_PREDICATE, CostKind, I);
+                                  cast<CmpInst>(I)->getPredicate(), CostKind,
+                                  I);
   }
   case Instruction::Store:
   case Instruction::Load: {
     ElementCount Width = VF;
     if (Width.isVector()) {
       InstWidening Decision = getWideningDecision(I, Width);
       assert(Decision != CM_Unknown &&
              "CM decision should be taken at this point");

llvm/test/Transforms/LoopVectorize/AArch64/select-costs.ll

 ; REQUIRES: asserts
 ; RUN: opt < %s -loop-vectorize -debug-only=loop-vectorize -S 2>&1 | FileCheck %s
 
 target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
 target triple = "arm64-apple-ios5.0.0"
 
 define void @selects_1(i32* nocapture %dst, i32 %A, i32 %B, i32 %C, i32 %N) {
-; CHECK: LV: Found an estimated cost of 5 for VF 2 For instruction:   %cond = select i1 %cmp1, i32 10, i32 %and
-; CHECK: LV: Found an estimated cost of 5 for VF 2 For instruction:   %cond6 = select i1 %cmp2, i32 30, i32 %and
-; CHECK: LV: Found an estimated cost of 5 for VF 2 For instruction:   %cond11 = select i1 %cmp7, i32 %cond, i32 %cond6
-
-; CHECK: LV: Found an estimated cost of 13 for VF 4 For instruction:   %cond = select i1 %cmp1, i32 10, i32 %and
-; CHECK: LV: Found an estimated cost of 13 for VF 4 For instruction:   %cond6 = select i1 %cmp2, i32 30, i32 %and
-; CHECK: LV: Found an estimated cost of 13 for VF 4 For instruction:   %cond11 = select i1 %cmp7, i32 %cond, i32 %cond6
+; CHECK: LV: Found an estimated cost of 1 for VF 2 For instruction:   %cond = select i1 %cmp1, i32 10, i32 %and
+; CHECK: LV: Found an estimated cost of 1 for VF 2 For instruction:   %cond6 = select i1 %cmp2, i32 30, i32 %and
+; CHECK: LV: Found an estimated cost of 1 for VF 2 For instruction:   %cond11 = select i1 %cmp7, i32 %cond, i32 %cond6
+
+; CHECK: LV: Found an estimated cost of 1 for VF 4 For instruction:   %cond = select i1 %cmp1, i32 10, i32 %and
+; CHECK: LV: Found an estimated cost of 1 for VF 4 For instruction:   %cond6 = select i1 %cmp2, i32 30, i32 %and
+; CHECK: LV: Found an estimated cost of 1 for VF 4 For instruction:   %cond11 = select i1 %cmp7, i32 %cond, i32 %cond6
 
 ; CHECK-LABEL: define void @selects_1(
 ; CHECK:       vector.body:
-; CHECK:         select <2 x i1>
+; CHECK:         select <4 x i1>
 
 entry:
   %cmp26 = icmp sgt i32 %N, 0
   br i1 %cmp26, label %for.body.preheader, label %for.cond.cleanup
 
 for.body.preheader:                               ; preds = %entry
   %wide.trip.count = zext i32 %N to i64
   br label %for.body