[LoopVectorize] Don't create unnecessary vscale intrinsic calls

[david-arm]

Summary

In quite a few cases in LoopVectorize.cpp we call createStepForVF
with a step value of 0, which leads to unnecessary generation of
llvm.vscale intrinsic calls. I've optimised IRBuilder::CreateVScale
and createStepForVF to return 0 when attempting to multiply
vscale by 0.

[frasercrmck]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

1119

Not that this is wrong, but would it be inappropriate to make CreateVScale detect a zero step and return zero?

[frasercrmck]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

1119

Sorry I just realised that's what you've done. Do we need to make this change, then?

[david-arm]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

1119

It's not really necessary as the end result is the same - I was just trying to avoid the compiler doing more work for the Step=0 case, i.e. the creation of a ConstantInt, etc. I noticed there are a few places in the vectoriser where we call this function with a Step of 0 - it happens when widening a PHI instruction or vectorising induction variables.

If you prefer I can just revert this code?

[frasercrmck]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

1119

I don't feel particularly strongly. I suppose my initial comment was trying to avoid repeating ourselves and duplicating logic. But I also realise that this is how compile-time regressions creep in. I don't know how impactful the creation of a zero ConstantInt is? Presumably it's cached more often than not?

[sdesmalen]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

1119

The overhead of creating the ConstantInt is small, and this only happens on a small number of cases when the step value is actually zero. Personally I'd prefer this to be implemented only once in IRBuilder, and have this change removed to make the code simpler.

[david-arm]

• Reverted change to createStepForVF

[sdesmalen]

Is it worth adding a test to llvm/unittests/IR/IRBuilderTest.cpp for this change?

[david-arm]

• Added unit test for vscale * 0 case.

[sdesmalen]

LGTM, thanks for adding the test.

[frasercrmck]

LGTM too, thanks.

llvm/lib/IR/IRBuilder.cpp

static CallInst *createCallHelper(Function *Callee, ArrayRef<Value *> Ops,
                                   ArrayRef<OperandBundleDef> OpBundles = {}) {
   CallInst *CI = Builder->CreateCall(Callee, Ops, OpBundles, Name);
   if (FMFSource)
     CI->copyFastMathFlags(FMFSource);
   return CI;
 }
 
 Value *IRBuilderBase::CreateVScale(Constant *Scaling, const Twine &Name) {
-  Module *M = GetInsertBlock()->getParent()->getParent();
   assert(isa<ConstantInt>(Scaling) && "Expected constant integer");
+  if (cast<ConstantInt>(Scaling)->isZero())
+    return Scaling;
+  Module *M = GetInsertBlock()->getParent()->getParent();
   Function *TheFn =
       Intrinsic::getDeclaration(M, Intrinsic::vscale, {Scaling->getType()});
   CallInst *CI = createCallHelper(TheFn, {}, this, Name);
   return cast<ConstantInt>(Scaling)->getSExtValue() == 1
              ? CI
              : CreateMul(CI, Scaling);
 }
 

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

static Value *createStepForVF(IRBuilder<> &B, Constant *Step, ElementCount VF) {
   Constant *StepVal = ConstantInt::get(
       Step->getType(),
       cast<ConstantInt>(Step)->getSExtValue() * VF.getKnownMinValue());
   return VF.isScalable() ? B.CreateVScale(StepVal) : StepVal;
 }
 
 namespace llvm {
 
 /// Return the runtime value for VF.
 Value *getRuntimeVF(IRBuilder<> &B, Type *Ty, ElementCount VF) {
   Constant *EC = ConstantInt::get(Ty, VF.getKnownMinValue());
   return VF.isScalable() ? B.CreateVScale(EC) : EC;
 }
 
 void reportVectorizationFailure(const StringRef DebugMsg,
     const StringRef OREMsg, const StringRef ORETag,
     OptimizationRemarkEmitter *ORE, Loop *TheLoop, Instruction *I) {
if (Cost->isScalarAfterVectorization(P, State.VF)) {
       // Determine the number of scalars we need to generate for each unroll
       // iteration. If the instruction is uniform, we only need to generate the
       // first lane. Otherwise, we generate all VF values.
       bool IsUniform = Cost->isUniformAfterVectorization(P, State.VF);
       assert((IsUniform || !VF.isScalable()) &&
              "Currently unsupported for scalable vectors");
       unsigned Lanes = IsUniform ? 1 : State.VF.getFixedValue();
 
-      Value *RuntimeVF = getRuntimeVF(Builder, PtrInd->getType(), VF);
       for (unsigned Part = 0; Part < UF; ++Part) {
-        Value *PartStart = Builder.CreateMul(
-            RuntimeVF, ConstantInt::get(PtrInd->getType(), Part));
+        Value *PartStart = createStepForVF(
+            Builder, ConstantInt::get(PtrInd->getType(), Part), VF);
         for (unsigned Lane = 0; Lane < Lanes; ++Lane) {
           Value *Idx = Builder.CreateAdd(
               PartStart, ConstantInt::get(PtrInd->getType(), Lane));
           Value *GlobalIdx = Builder.CreateAdd(PtrInd, Idx);
           Value *SclrGep =
               emitTransformedIndex(Builder, GlobalIdx, PSE.getSE(), DL, II);
           SclrGep->setName("next.gep");
           State.set(PhiR, SclrGep, VPIteration(Part, Lane));

llvm/unittests/IR/IRBuilderTest.cpp

Call = Builder.CreateIntrinsic(Intrinsic::masked_load,
                                  {VecTy, PtrToVecTy}, ArgTys,
                                  nullptr, "masked.load");
   FTy = Call->getFunctionType();
   EXPECT_EQ(FTy->getReturnType(), VecTy);
   for (unsigned i = 0; i != ArgTys.size(); ++i)
     EXPECT_EQ(FTy->getParamType(i), ArgTys[i]->getType());
 }
 
+TEST_F(IRBuilderTest, CreateVScale) {
+  IRBuilder<> Builder(BB);
+
+  Constant *Zero = Builder.getInt32(0);
+  Value *VScale = Builder.CreateVScale(Zero);
+  EXPECT_TRUE(isa<ConstantInt>(VScale) && cast<ConstantInt>(VScale)->isZero());
+}
+
 TEST_F(IRBuilderTest, CreateStepVector) {
   IRBuilder<> Builder(BB);
 
   // Fixed width vectors
   Type *DstVecTy = VectorType::get(Builder.getInt32Ty(), 4, false);
   Value *StepVec = Builder.CreateStepVector(DstVecTy);
   EXPECT_TRUE(isa<Constant>(StepVec));
   EXPECT_EQ(StepVec->getType(), DstVecTy);