[llvm] 965a090 - Revert "[IVDescriptors] Add pointer InductionDescriptors with non-constant strides"
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 31 08:28:26 PDT 2023
Thank you for the revert. Will follow up in the commit thread, and a
new review. With this much fallout, this clearly isn't "obvious".
Also, JFYI, commenting on the phabricator commit is going to be slower
to get a response from me than replying to the commit thread. I watch
the commit threads on llvm-commits fairly closely, but the way
phabricator sends the notices on a commit thread get lost with all of
the other low value phab emails.
Philip
On 3/31/23 03:08, David Green via llvm-commits wrote:
> Author: David Green
> Date: 2023-03-31T11:08:50+01:00
> New Revision: 965a090f02cbab2dc08b78e274ce9d65e4d1221f
>
> URL: https://github.com/llvm/llvm-project/commit/965a090f02cbab2dc08b78e274ce9d65e4d1221f
> DIFF: https://github.com/llvm/llvm-project/commit/965a090f02cbab2dc08b78e274ce9d65e4d1221f.diff
>
> LOG: Revert "[IVDescriptors] Add pointer InductionDescriptors with non-constant strides"
>
> Multiple errors have being reported on
> https://reviews.llvm.org/rG498aa534f472d28db893aa9a8627d0b46e17f312
>
> Reverting until the correctness issues can be resolved.
>
> We are also seeing a lot of performance differences from the patch. Some are
> looking good, but some are looking pretty bad.
>
> Added:
>
>
> Modified:
> llvm/lib/Analysis/IVDescriptors.cpp
> llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
> llvm/test/Transforms/LoopVectorize/RISCV/strided-accesses.ll
>
> Removed:
>
>
>
> ################################################################################
> diff --git a/llvm/lib/Analysis/IVDescriptors.cpp b/llvm/lib/Analysis/IVDescriptors.cpp
> index 22b9f64ef88d8..156d141c461ed 100644
> --- a/llvm/lib/Analysis/IVDescriptors.cpp
> +++ b/llvm/lib/Analysis/IVDescriptors.cpp
> @@ -1288,6 +1288,8 @@ InductionDescriptor::InductionDescriptor(Value *Start, InductionKind K,
> assert((!getConstIntStepValue() || !getConstIntStepValue()->isZero()) &&
> "Step value is zero");
>
> + assert((IK != IK_PtrInduction || getConstIntStepValue()) &&
> + "Step value should be constant for pointer induction");
> assert((IK == IK_FpInduction || Step->getType()->isIntegerTy()) &&
> "StepValue is not an integer");
>
> @@ -1568,25 +1570,15 @@ bool InductionDescriptor::isInductionPHI(
> }
>
> assert(PhiTy->isPointerTy() && "The PHI must be a pointer");
> - PointerType *PtrTy = cast<PointerType>(PhiTy);
> -
> - // Always use i8 element type for opaque pointer inductions.
> - // This allows induction variables w/non-constant steps.
> - if (PtrTy->isOpaque()) {
> - D = InductionDescriptor(StartValue, IK_PtrInduction, Step,
> - /* BinOp */ nullptr,
> - Type::getInt8Ty(PtrTy->getContext()));
> - return true;
> - }
> -
> // Pointer induction should be a constant.
> - // TODO: This could be generalized, but should probably just
> - // be dropped instead once the migration to opaque ptrs is
> - // complete.
> if (!ConstStep)
> return false;
>
> - Type *ElementType = PtrTy->getNonOpaquePointerElementType();
> + // Always use i8 element type for opaque pointer inductions.
> + PointerType *PtrTy = cast<PointerType>(PhiTy);
> + Type *ElementType = PtrTy->isOpaque()
> + ? Type::getInt8Ty(PtrTy->getContext())
> + : PtrTy->getNonOpaquePointerElementType();
> if (!ElementType->isSized())
> return false;
>
>
> diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
> index df7c430d326af..21858cc1705b6 100644
> --- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
> +++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
> @@ -2492,6 +2492,8 @@ static Value *emitTransformedIndex(IRBuilderBase &B, Value *Index,
> return CreateAdd(StartValue, Offset);
> }
> case InductionDescriptor::IK_PtrInduction: {
> + assert(isa<Constant>(Step) &&
> + "Expected constant step for pointer induction");
> return B.CreateGEP(ID.getElementType(), StartValue, CreateMul(Index, Step));
> }
> case InductionDescriptor::IK_FpInduction: {
> @@ -8269,6 +8271,7 @@ VPRecipeBase *VPRecipeBuilder::tryToOptimizeInductionPHI(
> if (auto *II = Legal->getPointerInductionDescriptor(Phi)) {
> VPValue *Step = vputils::getOrCreateVPValueForSCEVExpr(Plan, II->getStep(),
> *PSE.getSE());
> + assert(isa<SCEVConstant>(II->getStep()));
> return new VPWidenPointerInductionRecipe(
> Phi, Operands[0], Step, *II,
> LoopVectorizationPlanner::getDecisionAndClampRange(
> @@ -9374,6 +9377,8 @@ void VPWidenPointerInductionRecipe::execute(VPTransformState &State) {
> return;
> }
>
> + assert(isa<SCEVConstant>(IndDesc.getStep()) &&
> + "Induction step not a SCEV constant!");
> Type *PhiType = IndDesc.getStep()->getType();
>
> // Build a pointer phi
>
> diff --git a/llvm/test/Transforms/LoopVectorize/RISCV/strided-accesses.ll b/llvm/test/Transforms/LoopVectorize/RISCV/strided-accesses.ll
> index 0ba05c1d621ae..a108e9b0129ce 100644
> --- a/llvm/test/Transforms/LoopVectorize/RISCV/strided-accesses.ll
> +++ b/llvm/test/Transforms/LoopVectorize/RISCV/strided-accesses.ll
> @@ -539,89 +539,19 @@ exit:
> define void @double_stride_ptr_iv(ptr %p, ptr %p2, i64 %stride) {
> ; CHECK-LABEL: @double_stride_ptr_iv(
> ; CHECK-NEXT: entry:
> -; CHECK-NEXT: [[TMP0:%.*]] = call i64 @llvm.vscale.i64()
> -; CHECK-NEXT: [[TMP1:%.*]] = mul i64 [[TMP0]], 4
> -; CHECK-NEXT: [[TMP2:%.*]] = call i64 @llvm.umax.i64(i64 8, i64 [[TMP1]])
> -; CHECK-NEXT: [[MIN_ITERS_CHECK:%.*]] = icmp ult i64 1024, [[TMP2]]
> -; CHECK-NEXT: br i1 [[MIN_ITERS_CHECK]], label [[SCALAR_PH:%.*]], label [[VECTOR_SCEVCHECK:%.*]]
> -; CHECK: vector.scevcheck:
> -; CHECK-NEXT: [[IDENT_CHECK:%.*]] = icmp ne i64 [[STRIDE:%.*]], 1
> -; CHECK-NEXT: br i1 [[IDENT_CHECK]], label [[SCALAR_PH]], label [[VECTOR_MEMCHECK:%.*]]
> -; CHECK: vector.memcheck:
> -; CHECK-NEXT: [[SCEVGEP:%.*]] = getelementptr i8, ptr [[P2:%.*]], i64 1027
> -; CHECK-NEXT: [[SCEVGEP1:%.*]] = getelementptr i8, ptr [[P:%.*]], i64 1027
> -; CHECK-NEXT: [[BOUND0:%.*]] = icmp ult ptr [[P2]], [[SCEVGEP1]]
> -; CHECK-NEXT: [[BOUND1:%.*]] = icmp ult ptr [[P]], [[SCEVGEP]]
> -; CHECK-NEXT: [[FOUND_CONFLICT:%.*]] = and i1 [[BOUND0]], [[BOUND1]]
> -; CHECK-NEXT: br i1 [[FOUND_CONFLICT]], label [[SCALAR_PH]], label [[VECTOR_PH:%.*]]
> -; CHECK: vector.ph:
> -; CHECK-NEXT: [[TMP3:%.*]] = call i64 @llvm.vscale.i64()
> -; CHECK-NEXT: [[TMP4:%.*]] = mul i64 [[TMP3]], 4
> -; CHECK-NEXT: [[N_MOD_VF:%.*]] = urem i64 1024, [[TMP4]]
> -; CHECK-NEXT: [[N_VEC:%.*]] = sub i64 1024, [[N_MOD_VF]]
> -; CHECK-NEXT: [[TMP5:%.*]] = mul i64 [[N_VEC]], [[STRIDE]]
> -; CHECK-NEXT: [[IND_END:%.*]] = getelementptr i8, ptr [[P]], i64 [[TMP5]]
> -; CHECK-NEXT: [[TMP6:%.*]] = mul i64 [[N_VEC]], [[STRIDE]]
> -; CHECK-NEXT: [[IND_END3:%.*]] = getelementptr i8, ptr [[P2]], i64 [[TMP6]]
> -; CHECK-NEXT: br label [[VECTOR_BODY:%.*]]
> -; CHECK: vector.body:
> -; CHECK-NEXT: [[POINTER_PHI:%.*]] = phi ptr [ [[P]], [[VECTOR_PH]] ], [ [[PTR_IND:%.*]], [[VECTOR_BODY]] ]
> -; CHECK-NEXT: [[POINTER_PHI7:%.*]] = phi ptr [ [[P2]], [[VECTOR_PH]] ], [ [[PTR_IND8:%.*]], [[VECTOR_BODY]] ]
> -; CHECK-NEXT: [[INDEX:%.*]] = phi i64 [ 0, [[VECTOR_PH]] ], [ [[INDEX_NEXT:%.*]], [[VECTOR_BODY]] ]
> -; CHECK-NEXT: [[TMP7:%.*]] = call i64 @llvm.vscale.i64()
> -; CHECK-NEXT: [[TMP8:%.*]] = mul i64 [[TMP7]], 4
> -; CHECK-NEXT: [[TMP9:%.*]] = mul i64 [[TMP8]], 1
> -; CHECK-NEXT: [[TMP10:%.*]] = mul i64 [[STRIDE]], [[TMP9]]
> -; CHECK-NEXT: [[TMP11:%.*]] = mul i64 [[TMP8]], 0
> -; CHECK-NEXT: [[DOTSPLATINSERT:%.*]] = insertelement <vscale x 4 x i64> poison, i64 [[TMP11]], i64 0
> -; CHECK-NEXT: [[DOTSPLAT:%.*]] = shufflevector <vscale x 4 x i64> [[DOTSPLATINSERT]], <vscale x 4 x i64> poison, <vscale x 4 x i32> zeroinitializer
> -; CHECK-NEXT: [[TMP12:%.*]] = call <vscale x 4 x i64> @llvm.experimental.stepvector.nxv4i64()
> -; CHECK-NEXT: [[TMP13:%.*]] = add <vscale x 4 x i64> [[DOTSPLAT]], [[TMP12]]
> -; CHECK-NEXT: [[DOTSPLATINSERT5:%.*]] = insertelement <vscale x 4 x i64> poison, i64 [[STRIDE]], i64 0
> -; CHECK-NEXT: [[DOTSPLAT6:%.*]] = shufflevector <vscale x 4 x i64> [[DOTSPLATINSERT5]], <vscale x 4 x i64> poison, <vscale x 4 x i32> zeroinitializer
> -; CHECK-NEXT: [[VECTOR_GEP:%.*]] = mul <vscale x 4 x i64> [[TMP13]], [[DOTSPLAT6]]
> -; CHECK-NEXT: [[TMP14:%.*]] = getelementptr i8, ptr [[POINTER_PHI]], <vscale x 4 x i64> [[VECTOR_GEP]]
> -; CHECK-NEXT: [[TMP15:%.*]] = call i64 @llvm.vscale.i64()
> -; CHECK-NEXT: [[TMP16:%.*]] = mul i64 [[TMP15]], 4
> -; CHECK-NEXT: [[TMP17:%.*]] = mul i64 [[TMP16]], 1
> -; CHECK-NEXT: [[TMP18:%.*]] = mul i64 [[STRIDE]], [[TMP17]]
> -; CHECK-NEXT: [[TMP19:%.*]] = mul i64 [[TMP16]], 0
> -; CHECK-NEXT: [[DOTSPLATINSERT9:%.*]] = insertelement <vscale x 4 x i64> poison, i64 [[TMP19]], i64 0
> -; CHECK-NEXT: [[DOTSPLAT10:%.*]] = shufflevector <vscale x 4 x i64> [[DOTSPLATINSERT9]], <vscale x 4 x i64> poison, <vscale x 4 x i32> zeroinitializer
> -; CHECK-NEXT: [[TMP20:%.*]] = call <vscale x 4 x i64> @llvm.experimental.stepvector.nxv4i64()
> -; CHECK-NEXT: [[TMP21:%.*]] = add <vscale x 4 x i64> [[DOTSPLAT10]], [[TMP20]]
> -; CHECK-NEXT: [[VECTOR_GEP13:%.*]] = mul <vscale x 4 x i64> [[TMP21]], [[DOTSPLAT6]]
> -; CHECK-NEXT: [[TMP22:%.*]] = getelementptr i8, ptr [[POINTER_PHI7]], <vscale x 4 x i64> [[VECTOR_GEP13]]
> -; CHECK-NEXT: [[WIDE_MASKED_GATHER:%.*]] = call <vscale x 4 x i32> @llvm.masked.gather.nxv4i32.nxv4p0(<vscale x 4 x ptr> [[TMP14]], i32 4, <vscale x 4 x i1> shufflevector (<vscale x 4 x i1> insertelement (<vscale x 4 x i1> poison, i1 true, i64 0), <vscale x 4 x i1> poison, <vscale x 4 x i32> zeroinitializer), <vscale x 4 x i32> poison), !alias.scope !16
> -; CHECK-NEXT: [[TMP23:%.*]] = add <vscale x 4 x i32> [[WIDE_MASKED_GATHER]], shufflevector (<vscale x 4 x i32> insertelement (<vscale x 4 x i32> poison, i32 1, i64 0), <vscale x 4 x i32> poison, <vscale x 4 x i32> zeroinitializer)
> -; CHECK-NEXT: call void @llvm.masked.scatter.nxv4i32.nxv4p0(<vscale x 4 x i32> [[TMP23]], <vscale x 4 x ptr> [[TMP22]], i32 4, <vscale x 4 x i1> shufflevector (<vscale x 4 x i1> insertelement (<vscale x 4 x i1> poison, i1 true, i64 0), <vscale x 4 x i1> poison, <vscale x 4 x i32> zeroinitializer)), !alias.scope !19, !noalias !16
> -; CHECK-NEXT: [[TMP24:%.*]] = call i64 @llvm.vscale.i64()
> -; CHECK-NEXT: [[TMP25:%.*]] = mul i64 [[TMP24]], 4
> -; CHECK-NEXT: [[INDEX_NEXT]] = add nuw i64 [[INDEX]], [[TMP25]]
> -; CHECK-NEXT: [[PTR_IND]] = getelementptr i8, ptr [[POINTER_PHI]], i64 [[TMP10]]
> -; CHECK-NEXT: [[PTR_IND8]] = getelementptr i8, ptr [[POINTER_PHI7]], i64 [[TMP18]]
> -; CHECK-NEXT: [[TMP26:%.*]] = icmp eq i64 [[INDEX_NEXT]], [[N_VEC]]
> -; CHECK-NEXT: br i1 [[TMP26]], label [[MIDDLE_BLOCK:%.*]], label [[VECTOR_BODY]], !llvm.loop [[LOOP21:![0-9]+]]
> -; CHECK: middle.block:
> -; CHECK-NEXT: [[CMP_N:%.*]] = icmp eq i64 1024, [[N_VEC]]
> -; CHECK-NEXT: br i1 [[CMP_N]], label [[EXIT:%.*]], label [[SCALAR_PH]]
> -; CHECK: scalar.ph:
> -; CHECK-NEXT: [[BC_RESUME_VAL:%.*]] = phi i64 [ [[N_VEC]], [[MIDDLE_BLOCK]] ], [ 0, [[ENTRY:%.*]] ], [ 0, [[VECTOR_SCEVCHECK]] ], [ 0, [[VECTOR_MEMCHECK]] ]
> -; CHECK-NEXT: [[BC_RESUME_VAL2:%.*]] = phi ptr [ [[IND_END]], [[MIDDLE_BLOCK]] ], [ [[P]], [[ENTRY]] ], [ [[P]], [[VECTOR_SCEVCHECK]] ], [ [[P]], [[VECTOR_MEMCHECK]] ]
> -; CHECK-NEXT: [[BC_RESUME_VAL4:%.*]] = phi ptr [ [[IND_END3]], [[MIDDLE_BLOCK]] ], [ [[P2]], [[ENTRY]] ], [ [[P2]], [[VECTOR_SCEVCHECK]] ], [ [[P2]], [[VECTOR_MEMCHECK]] ]
> ; CHECK-NEXT: br label [[LOOP:%.*]]
> ; CHECK: loop:
> -; CHECK-NEXT: [[I:%.*]] = phi i64 [ [[BC_RESUME_VAL]], [[SCALAR_PH]] ], [ [[NEXTI:%.*]], [[LOOP]] ]
> -; CHECK-NEXT: [[PTR:%.*]] = phi ptr [ [[BC_RESUME_VAL2]], [[SCALAR_PH]] ], [ [[PTR_NEXT:%.*]], [[LOOP]] ]
> -; CHECK-NEXT: [[PTR2:%.*]] = phi ptr [ [[BC_RESUME_VAL4]], [[SCALAR_PH]] ], [ [[PTR2_NEXT:%.*]], [[LOOP]] ]
> +; CHECK-NEXT: [[I:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ [[NEXTI:%.*]], [[LOOP]] ]
> +; CHECK-NEXT: [[PTR:%.*]] = phi ptr [ [[P:%.*]], [[ENTRY]] ], [ [[PTR_NEXT:%.*]], [[LOOP]] ]
> +; CHECK-NEXT: [[PTR2:%.*]] = phi ptr [ [[P2:%.*]], [[ENTRY]] ], [ [[PTR2_NEXT:%.*]], [[LOOP]] ]
> ; CHECK-NEXT: [[X0:%.*]] = load i32, ptr [[PTR]], align 4
> ; CHECK-NEXT: [[Y0:%.*]] = add i32 [[X0]], 1
> ; CHECK-NEXT: store i32 [[Y0]], ptr [[PTR2]], align 4
> -; CHECK-NEXT: [[PTR_NEXT]] = getelementptr inbounds i8, ptr [[PTR]], i64 [[STRIDE]]
> +; CHECK-NEXT: [[PTR_NEXT]] = getelementptr inbounds i8, ptr [[PTR]], i64 [[STRIDE:%.*]]
> ; CHECK-NEXT: [[PTR2_NEXT]] = getelementptr inbounds i8, ptr [[PTR2]], i64 [[STRIDE]]
> ; CHECK-NEXT: [[NEXTI]] = add i64 [[I]], 1
> ; CHECK-NEXT: [[DONE:%.*]] = icmp eq i64 [[NEXTI]], 1024
> -; CHECK-NEXT: br i1 [[DONE]], label [[EXIT]], label [[LOOP]], !llvm.loop [[LOOP22:![0-9]+]]
> +; CHECK-NEXT: br i1 [[DONE]], label [[EXIT:%.*]], label [[LOOP]]
> ; CHECK: exit:
> ; CHECK-NEXT: ret void
> ;
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list