From e89379856acd52e0535bfef3123bd72e1704485c Mon Sep 17 00:00:00 2001 From: Florian Hahn Date: Wed, 29 Apr 2020 10:22:30 +0100 Subject: [PATCH] Recommit "[VPlan] Add & use VPValue operands for VPWidenRecipe (NFC)." The crash that caused the original revert has been fixed in a3c964a278b4. I also added a reduced version of the crash reproducer. This reverts the revert commit 2107af9ccfdfe67a90ea9ed4f3bfd7c72c4e29ac. --- llvm/lib/Transforms/Vectorize/LoopVectorize.cpp | 27 ++++++++++------------ llvm/lib/Transforms/Vectorize/VPlan.h | 17 +++++++++++++- llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp | 3 ++- .../test/Transforms/LoopVectorize/icmp-uniforms.ll | 24 +++++++++++++++++++ 4 files changed, 54 insertions(+), 17 deletions(-) diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp index c884548..df80203 100644 --- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp +++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp @@ -407,7 +407,8 @@ public: BasicBlock *createVectorizedLoopSkeleton(); /// Widen a single instruction within the innermost loop. - void widenInstruction(Instruction &I); + void widenInstruction(Instruction &I, VPUser &Operands, + VPTransformState &State); /// Widen a single call instruction within the innermost loop. void widenCallInstruction(CallInst &I, VPUser &ArgOperands, @@ -4231,7 +4232,8 @@ static bool mayDivideByZero(Instruction &I) { return !CInt || CInt->isZero(); } -void InnerLoopVectorizer::widenInstruction(Instruction &I) { +void InnerLoopVectorizer::widenInstruction(Instruction &I, VPUser &User, + VPTransformState &State) { switch (I.getOpcode()) { case Instruction::Call: case Instruction::Br: @@ -4263,8 +4265,8 @@ void InnerLoopVectorizer::widenInstruction(Instruction &I) { for (unsigned Part = 0; Part < UF; ++Part) { SmallVector Ops; - for (Value *Op : I.operands()) - Ops.push_back(getOrCreateVectorValue(Op, Part)); + for (VPValue *VPOp : User.operands()) + Ops.push_back(State.get(VPOp, Part)); Value *V = Builder.CreateNAryOp(I.getOpcode(), Ops); @@ -4285,8 +4287,8 @@ void InnerLoopVectorizer::widenInstruction(Instruction &I) { auto *Cmp = cast(&I); setDebugLocFromInst(Builder, Cmp); for (unsigned Part = 0; Part < UF; ++Part) { - Value *A = getOrCreateVectorValue(Cmp->getOperand(0), Part); - Value *B = getOrCreateVectorValue(Cmp->getOperand(1), Part); + Value *A = State.get(User.getOperand(0), Part); + Value *B = State.get(User.getOperand(1), Part); Value *C = nullptr; if (FCmp) { // Propagate fast math flags. @@ -4323,7 +4325,7 @@ void InnerLoopVectorizer::widenInstruction(Instruction &I) { (VF == 1) ? CI->getType() : VectorType::get(CI->getType(), VF); for (unsigned Part = 0; Part < UF; ++Part) { - Value *A = getOrCreateVectorValue(CI->getOperand(0), Part); + Value *A = State.get(User.getOperand(0), Part); Value *Cast = Builder.CreateCast(CI->getOpcode(), A, DestTy); VectorLoopValueMap.setVectorValue(&I, Part, Cast); addMetadata(Cast, &I); @@ -6936,12 +6938,7 @@ VPWidenCallRecipe *VPRecipeBuilder::tryToWidenCall(CallInst *CI, VFRange &Range, if (!LoopVectorizationPlanner::getDecisionAndClampRange(willWiden, Range)) return nullptr; - // Success: widen this call. - auto VPValues = map_range(CI->arg_operands(), [&Plan](Value *Op) { - return Plan.getOrAddVPValue(Op); - }); - - return new VPWidenCallRecipe(*CI, VPValues); + return new VPWidenCallRecipe(*CI, Plan.mapToVPValues(CI->arg_operands())); } bool VPRecipeBuilder::shouldWiden(Instruction *I, VFRange &Range) const { @@ -7004,7 +7001,7 @@ VPWidenRecipe *VPRecipeBuilder::tryToWiden(Instruction *I, VPlan &Plan) const { return nullptr; // Success: widen this instruction. - return new VPWidenRecipe(*I); + return new VPWidenRecipe(*I, Plan.mapToVPValues(I->operands())); } VPBasicBlock *VPRecipeBuilder::handleReplication( @@ -7411,7 +7408,7 @@ void VPWidenSelectRecipe::execute(VPTransformState &State) { } void VPWidenRecipe::execute(VPTransformState &State) { - State.ILV->widenInstruction(Ingredient); + State.ILV->widenInstruction(Ingredient, User, State); } void VPWidenGEPRecipe::execute(VPTransformState &State) { diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h index 1a112d3..9da379a 100644 --- a/llvm/lib/Transforms/Vectorize/VPlan.h +++ b/llvm/lib/Transforms/Vectorize/VPlan.h @@ -767,8 +767,13 @@ class VPWidenRecipe : public VPRecipeBase { /// Hold the instruction to be widened. Instruction &Ingredient; + /// Hold VPValues for the operands of the ingredient. + VPUser User; + public: - VPWidenRecipe(Instruction &I) : VPRecipeBase(VPWidenSC), Ingredient(I) {} + template + VPWidenRecipe(Instruction &I, iterator_range Operands) + : VPRecipeBase(VPWidenSC), Ingredient(I), User(Operands) {} ~VPWidenRecipe() override = default; @@ -1617,6 +1622,16 @@ public: /// Dump the plan to stderr (for debugging). void dump() const; + /// Returns a range mapping the values the range \p Operands to their + /// corresponding VPValues. + iterator_range>> + mapToVPValues(User::op_range Operands) { + std::function Fn = [this](Value *Op) { + return getOrAddVPValue(Op); + }; + return map_range(Operands, Fn); + } + private: /// Add to the given dominator tree the header block and every new basic block /// that was created between it and the latch block, inclusive. diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp index fa86488..2b48b2b 100644 --- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp +++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp @@ -72,7 +72,8 @@ void VPlanTransforms::VPInstructionsToVPRecipes( } else if (GetElementPtrInst *GEP = dyn_cast(Inst)) { NewRecipe = new VPWidenGEPRecipe(GEP, OrigLoop); } else - NewRecipe = new VPWidenRecipe(*Inst); + NewRecipe = + new VPWidenRecipe(*Inst, Plan->mapToVPValues(Inst->operands())); NewRecipe->insertBefore(Ingredient); Ingredient->eraseFromParent(); diff --git a/llvm/test/Transforms/LoopVectorize/icmp-uniforms.ll b/llvm/test/Transforms/LoopVectorize/icmp-uniforms.ll index f8d20c3..3636446 100644 --- a/llvm/test/Transforms/LoopVectorize/icmp-uniforms.ll +++ b/llvm/test/Transforms/LoopVectorize/icmp-uniforms.ll @@ -33,3 +33,27 @@ for.end: %tmp4 = phi i32 [ %tmp3, %for.body ] ret i32 %tmp4 } + +; Check for crash exposed by D76992. +; CHECK: N0 [label = +; CHECK-NEXT: "loop:\n" + +; CHECK-NEXT: "WIDEN-INDUCTION %iv = phi 0, %iv.next\l" + +; CHECK-NEXT: "WIDEN\l"" %cond0 = icmp %iv, 13\l" + +; CHECK-NEXT: "WIDEN-SELECT%s = select %cond0, 10, 20\l" + +; CHECK-NEXT: "EMIT vp<%1> = icmp ule ir<%iv> vp<%0>\l" +; CHECK-NEXT: ] +define void @test() { +entry: + br label %loop + +loop: ; preds = %loop, %entry + %iv = phi i64 [ 0, %entry ], [ %iv.next, %loop ] + %cond0 = icmp ult i64 %iv, 13 + %s = select i1 %cond0, i32 10, i32 20 + %iv.next = add nuw nsw i64 %iv, 1 + %exitcond = icmp eq i64 %iv.next, 14 + br i1 %exitcond, label %exit, label %loop + +exit: ; preds = %loop + ret void +} -- 2.7.4