[SLP] Use vector types for cmp alt instructions costs

[dmgreen]

Summary

Similar to the other code that costs main/alt instructions, the cmp should be using the VecTy for the costs, not the ScalarTy.

One of the tests look like it gets worse just because it is not simplified to 0.

[vporpo]

The test patch won't apply cleanly to top of trunk.

llvm/test/Transforms/SLPVectorizer/AArch64/extracts-from-scalarizable-vector.ll

2

Perhaps set the -slp-threshold such that it gets vectorized?

[dmgreen]

The test patch won't apply cleanly to top of trunk.

Yeah I have not committed the updated tests yet. It is common practice to split them out to show just the differences, and I tend to keep them locally until the patch gets further into a review in case they need more adjustment. I can commit them if it's useful, but you can grab the new file using the View Options -> Show Raw File (Right) button. Or do you mean the code in the SLPVectorizer? I think that sound apply to a recent checkout.

llvm/test/Transforms/SLPVectorizer/AArch64/extracts-from-scalarizable-vector.ll

2

The idea is that these shouldn't be vectorized. The degenerate case would get simplified by instsimplify to ret 0, so is not interesting. The with_inputs case doesn't gain anything from vectorization (the f128 fcmp's will just be scalarized, so the vectorized version has are twice as many fcmp, plus extra shuffles and the reduce).

[vporpo]

Yeah I have not committed the updated tests yet. It is common practice to split them out to show just the differences, and I tend to keep them locally until the patch gets further into a review in case they need more adjustment. I can commit them if it's useful, but you can grab the new file using the View Options -> Show Raw File (Right) button. Or do you mean the code in the SLPVectorizer? I think that sound apply to a recent checkout.

I meant to say the test hunk, sorry about that. You don't need to commit the first part, but it sometimes helps if it is shared on phabricator. In this case I was a bit confused as I could not tell what changes were made in the test compared to the existing version. In particular, I could not see the original @test function, but I could see a function named @degenerate which looked very much like @test, but it was not immediately obvious if only the its name was changed or if there were changes in its body.

llvm/test/Transforms/SLPVectorizer/AArch64/extracts-from-scalarizable-vector.ll

2

I understand that this can be simplified, but isn't this test checking that SLP matches and generates the llvm.vector.reduce.and pattern? Or is there a separate test for that?

[dmgreen]

llvm/test/Transforms/SLPVectorizer/AArch64/extracts-from-scalarizable-vector.ll

2

According to https://reviews.llvm.org/rG403bd583a8cc1f041430ff1b236ab296a2acdc85 it was just added to protect against a crash. I left this version of the test as-is to make sure it remains valid.

[dmgreen]

Rebase

[vporpo]

LG

[dmgreen]

Thanks

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

auto GetVectorCost = [&](InstructionCost) {
         });
         // No need to add new vector costs here since we're going to reuse
         // same main/alternate vector ops, just do different shuffling.
       } else if (Instruction::isBinaryOp(E->getOpcode())) {
         VecCost = TTI->getArithmeticInstrCost(E->getOpcode(), VecTy, CostKind);
         VecCost +=
             TTI->getArithmeticInstrCost(E->getAltOpcode(), VecTy, CostKind);
       } else if (auto *CI0 = dyn_cast<CmpInst>(VL0)) {
-        VecCost = TTI->getCmpSelInstrCost(E->getOpcode(), ScalarTy,
-                                          Builder.getInt1Ty(),
+        auto *MaskTy = FixedVectorType::get(Builder.getInt1Ty(), VL.size());
+        VecCost = TTI->getCmpSelInstrCost(E->getOpcode(), VecTy, MaskTy,
                                           CI0->getPredicate(), CostKind, VL0);
         VecCost += TTI->getCmpSelInstrCost(
-            E->getOpcode(), ScalarTy, Builder.getInt1Ty(),
+            E->getOpcode(), VecTy, MaskTy,
             cast<CmpInst>(E->getAltOp())->getPredicate(), CostKind,
             E->getAltOp());
       } else {
         Type *Src0SclTy = E->getMainOp()->getOperand(0)->getType();
         Type *Src1SclTy = E->getAltOp()->getOperand(0)->getType();
         auto *Src0Ty = FixedVectorType::get(Src0SclTy, VL.size());
         auto *Src1Ty = FixedVectorType::get(Src1SclTy, VL.size());
         VecCost = TTI->getCastInstrCost(E->getOpcode(), VecTy, Src0Ty,

llvm/test/Transforms/SLPVectorizer/AArch64/extracts-from-scalarizable-vector.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 2
 ; RUN: opt -S -passes=slp-vectorizer -mtriple=aarch64-unknown-linux-gnu < %s | FileCheck %s
 
 define i1 @degenerate() {
 ; CHECK-LABEL: define i1 @degenerate() {
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[TMP0:%.*]] = freeze <4 x i1> zeroinitializer
-; CHECK-NEXT:    [[TMP1:%.*]] = call i1 @llvm.vector.reduce.and.v4i1(<4 x i1> [[TMP0]])
-; CHECK-NEXT:    ret i1 [[TMP1]]
+; CHECK-NEXT:    [[TMP0:%.*]] = extractelement <4 x fp128> zeroinitializer, i32 0
+; CHECK-NEXT:    [[CMP:%.*]] = fcmp ogt fp128 [[TMP0]], 0xL00000000000000000000000000000000
+; CHECK-NEXT:    [[CMP3:%.*]] = fcmp olt fp128 [[TMP0]], 0xL00000000000000000000000000000000
+; CHECK-NEXT:    [[OR_COND:%.*]] = and i1 [[CMP]], [[CMP3]]
+; CHECK-NEXT:    [[TMP1:%.*]] = extractelement <4 x fp128> zeroinitializer, i32 0
+; CHECK-NEXT:    [[CMP6:%.*]] = fcmp ogt fp128 [[TMP1]], 0xL00000000000000000000000000000000
+; CHECK-NEXT:    [[OR_COND29:%.*]] = select i1 [[OR_COND]], i1 [[CMP6]], i1 false
+; CHECK-NEXT:    [[CMP10:%.*]] = fcmp olt fp128 [[TMP1]], 0xL00000000000000000000000000000000
+; CHECK-NEXT:    [[OR_COND30:%.*]] = select i1 [[OR_COND29]], i1 [[CMP10]], i1 false
+; CHECK-NEXT:    ret i1 [[OR_COND30]]
 ;
 entry:
   %0 = extractelement <4 x fp128> zeroinitializer, i32 0
   %cmp = fcmp ogt fp128 %0, 0xL00000000000000000000000000000000
   %cmp3 = fcmp olt fp128 %0, 0xL00000000000000000000000000000000
   %or.cond = and i1 %cmp, %cmp3
   %1 = extractelement <4 x fp128> zeroinitializer, i32 0
   %cmp6 = fcmp ogt fp128 %1, 0xL00000000000000000000000000000000
   %or.cond29 = select i1 %or.cond, i1 %cmp6, i1 false
   %cmp10 = fcmp olt fp128 %1, 0xL00000000000000000000000000000000
   %or.cond30 = select i1 %or.cond29, i1 %cmp10, i1 false
   ret i1 %or.cond30
 }
 
 define i1 @with_inputs(<4 x fp128> %a) {
 ; CHECK-LABEL: define i1 @with_inputs
 ; CHECK-SAME: (<4 x fp128> [[A:%.*]]) {
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[TMP0:%.*]] = shufflevector <4 x fp128> [[A]], <4 x fp128> poison, <4 x i32> <i32 0, i32 1, i32 0, i32 1>
-; CHECK-NEXT:    [[TMP1:%.*]] = fcmp olt <4 x fp128> [[TMP0]], zeroinitializer
-; CHECK-NEXT:    [[TMP2:%.*]] = fcmp ogt <4 x fp128> [[TMP0]], zeroinitializer
-; CHECK-NEXT:    [[TMP3:%.*]] = shufflevector <4 x i1> [[TMP1]], <4 x i1> [[TMP2]], <4 x i32> <i32 0, i32 1, i32 6, i32 7>
-; CHECK-NEXT:    [[TMP4:%.*]] = freeze <4 x i1> [[TMP3]]
-; CHECK-NEXT:    [[TMP5:%.*]] = call i1 @llvm.vector.reduce.and.v4i1(<4 x i1> [[TMP4]])
-; CHECK-NEXT:    ret i1 [[TMP5]]
+; CHECK-NEXT:    [[TMP0:%.*]] = extractelement <4 x fp128> [[A]], i32 0
+; CHECK-NEXT:    [[CMP:%.*]] = fcmp ogt fp128 [[TMP0]], 0xL00000000000000000000000000000000
+; CHECK-NEXT:    [[CMP3:%.*]] = fcmp olt fp128 [[TMP0]], 0xL00000000000000000000000000000000
+; CHECK-NEXT:    [[OR_COND:%.*]] = and i1 [[CMP]], [[CMP3]]
+; CHECK-NEXT:    [[TMP1:%.*]] = extractelement <4 x fp128> [[A]], i32 1
+; CHECK-NEXT:    [[CMP6:%.*]] = fcmp ogt fp128 [[TMP1]], 0xL00000000000000000000000000000000
+; CHECK-NEXT:    [[OR_COND29:%.*]] = select i1 [[OR_COND]], i1 [[CMP6]], i1 false
+; CHECK-NEXT:    [[CMP10:%.*]] = fcmp olt fp128 [[TMP1]], 0xL00000000000000000000000000000000
+; CHECK-NEXT:    [[OR_COND30:%.*]] = select i1 [[OR_COND29]], i1 [[CMP10]], i1 false
+; CHECK-NEXT:    ret i1 [[OR_COND30]]
 ;
 entry:
   %0 = extractelement <4 x fp128> %a, i32 0
   %cmp = fcmp ogt fp128 %0, 0xL00000000000000000000000000000000
   %cmp3 = fcmp olt fp128 %0, 0xL00000000000000000000000000000000
   %or.cond = and i1 %cmp, %cmp3
   %1 = extractelement <4 x fp128> %a, i32 1
   %cmp6 = fcmp ogt fp128 %1, 0xL00000000000000000000000000000000
   %or.cond29 = select i1 %or.cond, i1 %cmp6, i1 false
   %cmp10 = fcmp olt fp128 %1, 0xL00000000000000000000000000000000
   %or.cond30 = select i1 %or.cond29, i1 %cmp10, i1 false
   ret i1 %or.cond30
 }