[VPlan] Extract reverse operation for reverse accesses#146525
Merged
[VPlan] Extract reverse operation for reverse accesses#146525
Conversation
Mel-Chen
requested review from
ElvisWang123,
alexey-bataev,
ayalz,
david-arm,
fhahn and
preames
Member
|
@llvm/pr-subscribers-backend-powerpc @llvm/pr-subscribers-backend-risc-v Author: Mel Chen (Mel-Chen) ChangesThis patch introduces VPInstruction::Reverse and extracts the reverse operations of loaded/stored values from reverse memory accesses. This extraction facilitates future support for permutation elimination within VPlan. Patch is 69.62 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/146525.diff 18 Files Affected:
diff --git a/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp b/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
index 67a51c12b508e..d5aeb4feb19ba 100644
--- a/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
@@ -1541,6 +1541,12 @@ RISCVTTIImpl::getIntrinsicInstrCost(const IntrinsicCostAttributes &ICA,
cast<VectorType>(ICA.getArgTypes()[0]), {}, CostKind,
0, cast<VectorType>(ICA.getReturnType()));
}
+ case Intrinsic::experimental_vp_reverse: {
+ return getShuffleCost(TTI::SK_Reverse,
+ cast<VectorType>(ICA.getReturnType()),
+ cast<VectorType>(ICA.getArgTypes()[0]), {}, CostKind,
+ 0, cast<VectorType>(ICA.getReturnType()));
+ }
}
if (ST->hasVInstructions() && RetTy->isVectorTy()) {
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index b01c8b02ec66a..94782c33f5bda 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -8880,6 +8880,10 @@ VPlanPtr LoopVectorizationPlanner::tryToBuildVPlanWithVPRecipes(
// bring the VPlan to its final state.
// ---------------------------------------------------------------------------
+ // Adjust the result of reverse memory accesses.
+ VPlanTransforms::runPass(VPlanTransforms::adjustRecipesForReverseAccesses,
+ *Plan);
+
// Adjust the recipes for any inloop reductions.
adjustRecipesForReductions(Plan, RecipeBuilder, Range.Start);
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index 61b5ccd85bc6e..55175a889d0e0 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -970,6 +970,8 @@ class VPInstruction : public VPRecipeWithIRFlags,
// It produces the lane index across all unrolled iterations. Unrolling will
// add all copies of its original operand as additional operands.
FirstActiveLane,
+ // Returns a reversed vector for the operand.
+ Reverse,
// The opcodes below are used for VPInstructionWithType.
//
diff --git a/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp b/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
index f3b99fe34c069..f87b6de42c8b8 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
@@ -126,6 +126,7 @@ Type *VPTypeAnalysis::inferScalarTypeForRecipe(const VPInstruction *R) {
return IntegerType::get(Ctx, 1);
case VPInstruction::Broadcast:
case VPInstruction::PtrAdd:
+ case VPInstruction::Reverse:
// Return the type based on first operand.
return inferScalarType(R->getOperand(0));
case VPInstruction::BranchOnCond:
diff --git a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
index 1a38932ef99fe..b4ed4ef3147c6 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
@@ -444,6 +444,7 @@ unsigned VPInstruction::getNumOperandsForOpcode(unsigned Opcode) {
case VPInstruction::ExtractPenultimateElement:
case VPInstruction::FirstActiveLane:
case VPInstruction::Not:
+ case VPInstruction::Reverse:
return 1;
case Instruction::ICmp:
case Instruction::FCmp:
@@ -873,6 +874,9 @@ Value *VPInstruction::generate(VPTransformState &State) {
return Res;
}
+ case VPInstruction::Reverse: {
+ return Builder.CreateVectorReverse(State.get(getOperand(0)), "reverse");
+ }
default:
llvm_unreachable("Unsupported opcode for instruction");
}
@@ -948,6 +952,13 @@ InstructionCost VPInstruction::computeCost(ElementCount VF,
I32Ty, {Arg0Ty, I32Ty, I1Ty});
return Ctx.TTI.getIntrinsicInstrCost(Attrs, Ctx.CostKind);
}
+ case VPInstruction::Reverse: {
+ assert(VF.isVector() && "Reverse operation must be vector type");
+ Type *VectorTy = toVectorTy(Ctx.Types.inferScalarType(this), VF);
+ return Ctx.TTI.getShuffleCost(
+ TargetTransformInfo::SK_Reverse, cast<VectorType>(VectorTy),
+ cast<VectorType>(VectorTy), {}, Ctx.CostKind, 0);
+ }
case VPInstruction::ExtractPenultimateElement:
if (VF == ElementCount::getScalable(1))
return InstructionCost::getInvalid();
@@ -1033,6 +1044,7 @@ bool VPInstruction::opcodeMayReadOrWriteFromMemory() const {
case VPInstruction::WideIVStep:
case VPInstruction::StepVector:
case VPInstruction::ReductionStartVector:
+ case VPInstruction::Reverse:
return false;
default:
return true;
@@ -1179,6 +1191,9 @@ void VPInstruction::print(raw_ostream &O, const Twine &Indent,
case VPInstruction::ReductionStartVector:
O << "reduction-start-vector";
break;
+ case VPInstruction::Reverse:
+ O << "reverse";
+ break;
default:
O << Instruction::getOpcodeName(getOpcode());
}
@@ -2967,12 +2982,7 @@ InstructionCost VPWidenMemoryRecipe::computeCost(ElementCount VF,
Cost += Ctx.TTI.getMemoryOpCost(Opcode, Ty, Alignment, AS, Ctx.CostKind,
OpInfo, &Ingredient);
}
- if (!Reverse)
- return Cost;
-
- return Cost += Ctx.TTI.getShuffleCost(
- TargetTransformInfo::SK_Reverse, cast<VectorType>(Ty),
- cast<VectorType>(Ty), {}, Ctx.CostKind, 0);
+ return Cost;
}
void VPWidenLoadRecipe::execute(VPTransformState &State) {
@@ -3004,8 +3014,6 @@ void VPWidenLoadRecipe::execute(VPTransformState &State) {
NewLI = Builder.CreateAlignedLoad(DataTy, Addr, Alignment, "wide.load");
}
applyMetadata(*cast<Instruction>(NewLI));
- if (Reverse)
- NewLI = Builder.CreateVectorReverse(NewLI, "reverse");
State.set(this, NewLI);
}
@@ -3061,8 +3069,6 @@ void VPWidenLoadEVLRecipe::execute(VPTransformState &State) {
0, Attribute::getWithAlignment(NewLI->getContext(), Alignment));
applyMetadata(*NewLI);
Instruction *Res = NewLI;
- if (isReverse())
- Res = createReverseEVL(Builder, Res, EVL, "vp.reverse");
State.set(this, Res);
}
@@ -3083,12 +3089,8 @@ InstructionCost VPWidenLoadEVLRecipe::computeCost(ElementCount VF,
getLoadStoreAddressSpace(const_cast<Instruction *>(&Ingredient));
InstructionCost Cost = Ctx.TTI.getMaskedMemoryOpCost(
Instruction::Load, Ty, Alignment, AS, Ctx.CostKind);
- if (!Reverse)
- return Cost;
- return Cost + Ctx.TTI.getShuffleCost(
- TargetTransformInfo::SK_Reverse, cast<VectorType>(Ty),
- cast<VectorType>(Ty), {}, Ctx.CostKind, 0);
+ return Cost;
}
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
@@ -3118,13 +3120,6 @@ void VPWidenStoreRecipe::execute(VPTransformState &State) {
}
Value *StoredVal = State.get(StoredVPValue);
- if (isReverse()) {
- // If we store to reverse consecutive memory locations, then we need
- // to reverse the order of elements in the stored value.
- StoredVal = Builder.CreateVectorReverse(StoredVal, "reverse");
- // We don't want to update the value in the map as it might be used in
- // another expression. So don't call resetVectorValue(StoredVal).
- }
Value *Addr = State.get(getAddr(), /*IsScalar*/ !CreateScatter);
Instruction *NewSI = nullptr;
if (CreateScatter)
@@ -3154,8 +3149,6 @@ void VPWidenStoreEVLRecipe::execute(VPTransformState &State) {
CallInst *NewSI = nullptr;
Value *StoredVal = State.get(StoredValue);
Value *EVL = State.get(getEVL(), VPLane(0));
- if (isReverse())
- StoredVal = createReverseEVL(Builder, StoredVal, EVL, "vp.reverse");
Value *Mask = nullptr;
if (VPValue *VPMask = getMask()) {
Mask = State.get(VPMask);
@@ -3196,12 +3189,8 @@ InstructionCost VPWidenStoreEVLRecipe::computeCost(ElementCount VF,
getLoadStoreAddressSpace(const_cast<Instruction *>(&Ingredient));
InstructionCost Cost = Ctx.TTI.getMaskedMemoryOpCost(
Instruction::Store, Ty, Alignment, AS, Ctx.CostKind);
- if (!Reverse)
- return Cost;
- return Cost + Ctx.TTI.getShuffleCost(
- TargetTransformInfo::SK_Reverse, cast<VectorType>(Ty),
- cast<VectorType>(Ty), {}, Ctx.CostKind, 0);
+ return Cost;
}
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
index 730deb0686b2a..cf41b6d00f285 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
@@ -2172,6 +2172,14 @@ static VPRecipeBase *createEVLRecipe(VPValue *HeaderMask,
VPI->getDebugLoc());
}
+ if (VPI->getOpcode() == VPInstruction::Reverse) {
+ SmallVector<VPValue *> Ops(VPI->operands());
+ Ops.append({&AllOneMask, &EVL});
+ return new VPWidenIntrinsicRecipe(Intrinsic::experimental_vp_reverse,
+ Ops, TypeInfo.inferScalarType(VPI),
+ VPI->getDebugLoc());
+ }
+
VPValue *LHS, *RHS;
// Transform select with a header mask condition
// select(header_mask, LHS, RHS)
@@ -3347,3 +3355,34 @@ void VPlanTransforms::addBranchWeightToMiddleTerminator(VPlan &Plan,
MDB.createBranchWeights({1, VectorStep - 1}, /*IsExpected=*/false);
MiddleTerm->addMetadata(LLVMContext::MD_prof, BranchWeights);
}
+
+void VPlanTransforms::adjustRecipesForReverseAccesses(VPlan &Plan) {
+ if (Plan.hasScalarVFOnly())
+ return;
+
+ for (VPBasicBlock *VPBB : VPBlockUtils::blocksOnly<VPBasicBlock>(
+ vp_depth_first_deep(Plan.getVectorLoopRegion()))) {
+ for (VPRecipeBase &R : *VPBB) {
+ auto *MemR = dyn_cast<VPWidenMemoryRecipe>(&R);
+ if (!MemR || !MemR->isReverse())
+ continue;
+
+ if (auto *L = dyn_cast<VPWidenLoadRecipe>(MemR)) {
+ auto *Reverse =
+ new VPInstruction(VPInstruction::Reverse, {L}, L->getDebugLoc());
+ Reverse->insertAfter(L);
+ L->replaceAllUsesWith(Reverse);
+ Reverse->setOperand(0, L);
+ continue;
+ }
+
+ if (auto *S = dyn_cast<VPWidenStoreRecipe>(MemR)) {
+ VPValue *StoredVal = S->getStoredValue();
+ auto *Reverse = new VPInstruction(VPInstruction::Reverse, {StoredVal},
+ S->getDebugLoc());
+ Reverse->insertBefore(S);
+ S->setOperand(1, Reverse);
+ }
+ }
+ }
+}
diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.h b/llvm/lib/Transforms/Vectorize/VPlanTransforms.h
index 40885cd52a127..abe592247e2de 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.h
+++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.h
@@ -239,6 +239,20 @@ struct VPlanTransforms {
/// Add branch weight metadata, if the \p Plan's middle block is terminated by
/// a BranchOnCond recipe.
static void addBranchWeightToMiddleTerminator(VPlan &Plan, ElementCount VF);
+
+ /// Add reverse recipes for reverse memory accesses.
+ /// For reverse loads, transform
+ /// WIDEN ir<%L> = load vp<%addr>
+ /// into
+ /// WIDEN ir<%L> = load vp<%addr>
+ /// EMIT vp<%RevL> = reverse ir<%L>
+ ///
+ /// For reverse stores, transform
+ /// WIDEN store vp<%addr>, ir<%SVal>
+ /// into
+ /// EMIT vp<%RevS> = reverse ir<%SVal>
+ /// WIDEN store vp<%addr>, vp<%RevS>
+ static void adjustRecipesForReverseAccesses(VPlan &Plan);
};
} // namespace llvm
diff --git a/llvm/test/Transforms/LoopVectorize/AArch64/sve-vector-reverse-mask4.ll b/llvm/test/Transforms/LoopVectorize/AArch64/sve-vector-reverse-mask4.ll
index 9485d827ced40..c838c63545341 100644
--- a/llvm/test/Transforms/LoopVectorize/AArch64/sve-vector-reverse-mask4.ll
+++ b/llvm/test/Transforms/LoopVectorize/AArch64/sve-vector-reverse-mask4.ll
@@ -22,8 +22,8 @@ define void @vector_reverse_mask_nxv4i1(ptr %a, ptr %cond, i64 %N) #0 {
; CHECK: %[[WIDEMSKLOAD:.*]] = call <vscale x 4 x double> @llvm.masked.load.nxv4f64.p0(ptr %{{.*}}, i32 8, <vscale x 4 x i1> %[[REVERSE6]], <vscale x 4 x double> poison)
; CHECK: %[[REVERSE7:.*]] = call <vscale x 4 x double> @llvm.vector.reverse.nxv4f64(<vscale x 4 x double> %[[WIDEMSKLOAD]])
; CHECK: %[[FADD:.*]] = fadd <vscale x 4 x double> %[[REVERSE7]]
-; CHECK: %[[REVERSE9:.*]] = call <vscale x 4 x i1> @llvm.vector.reverse.nxv4i1(<vscale x 4 x i1> %{{.*}})
; CHECK: %[[REVERSE8:.*]] = call <vscale x 4 x double> @llvm.vector.reverse.nxv4f64(<vscale x 4 x double> %[[FADD]])
+; CHECK: %[[REVERSE9:.*]] = call <vscale x 4 x i1> @llvm.vector.reverse.nxv4i1(<vscale x 4 x i1> %{{.*}})
; CHECK: call void @llvm.masked.store.nxv4f64.p0(<vscale x 4 x double> %[[REVERSE8]], ptr %{{.*}}, i32 8, <vscale x 4 x i1> %[[REVERSE9]]
entry:
diff --git a/llvm/test/Transforms/LoopVectorize/AArch64/vector-reverse-mask4.ll b/llvm/test/Transforms/LoopVectorize/AArch64/vector-reverse-mask4.ll
index 1dd49ecf85b81..d6f619cce54a0 100644
--- a/llvm/test/Transforms/LoopVectorize/AArch64/vector-reverse-mask4.ll
+++ b/llvm/test/Transforms/LoopVectorize/AArch64/vector-reverse-mask4.ll
@@ -37,8 +37,8 @@ define void @vector_reverse_mask_v4i1(ptr noalias %a, ptr noalias %cond, i64 %N)
; CHECK-NEXT: [[TMP3:%.*]] = getelementptr inbounds i8, ptr [[TMP2]], i64 -24
; CHECK-NEXT: [[TMP4:%.*]] = getelementptr inbounds i8, ptr [[TMP2]], i64 -56
; CHECK-NEXT: [[WIDE_LOAD:%.*]] = load <4 x double>, ptr [[TMP3]], align 8
-; CHECK-NEXT: [[REVERSE:%.*]] = shufflevector <4 x double> [[WIDE_LOAD]], <4 x double> poison, <4 x i32> <i32 3, i32 2, i32 1, i32 0>
; CHECK-NEXT: [[WIDE_LOAD1:%.*]] = load <4 x double>, ptr [[TMP4]], align 8
+; CHECK-NEXT: [[REVERSE:%.*]] = shufflevector <4 x double> [[WIDE_LOAD]], <4 x double> poison, <4 x i32> <i32 3, i32 2, i32 1, i32 0>
; CHECK-NEXT: [[REVERSE2:%.*]] = shufflevector <4 x double> [[WIDE_LOAD1]], <4 x double> poison, <4 x i32> <i32 3, i32 2, i32 1, i32 0>
; CHECK-NEXT: [[TMP5:%.*]] = fcmp une <4 x double> [[REVERSE]], zeroinitializer
; CHECK-NEXT: [[TMP6:%.*]] = fcmp une <4 x double> [[REVERSE2]], zeroinitializer
diff --git a/llvm/test/Transforms/LoopVectorize/RISCV/riscv-vector-reverse-output.ll b/llvm/test/Transforms/LoopVectorize/RISCV/riscv-vector-reverse-output.ll
index 09b274de30214..6d55f7369f01e 100644
--- a/llvm/test/Transforms/LoopVectorize/RISCV/riscv-vector-reverse-output.ll
+++ b/llvm/test/Transforms/LoopVectorize/RISCV/riscv-vector-reverse-output.ll
@@ -165,8 +165,8 @@ define void @vector_reverse_i32(ptr noalias %A, ptr noalias %B) {
; RV64-UF2-NEXT: [[TMP17:%.*]] = getelementptr inbounds i32, ptr [[TMP10]], i64 [[TMP15]]
; RV64-UF2-NEXT: [[TMP18:%.*]] = getelementptr inbounds i32, ptr [[TMP17]], i64 [[TMP16]]
; RV64-UF2-NEXT: [[WIDE_LOAD:%.*]] = load <vscale x 4 x i32>, ptr [[TMP14]], align 4
-; RV64-UF2-NEXT: [[REVERSE:%.*]] = call <vscale x 4 x i32> @llvm.vector.reverse.nxv4i32(<vscale x 4 x i32> [[WIDE_LOAD]])
; RV64-UF2-NEXT: [[WIDE_LOAD1:%.*]] = load <vscale x 4 x i32>, ptr [[TMP18]], align 4
+; RV64-UF2-NEXT: [[REVERSE:%.*]] = call <vscale x 4 x i32> @llvm.vector.reverse.nxv4i32(<vscale x 4 x i32> [[WIDE_LOAD]])
; RV64-UF2-NEXT: [[REVERSE2:%.*]] = call <vscale x 4 x i32> @llvm.vector.reverse.nxv4i32(<vscale x 4 x i32> [[WIDE_LOAD1]])
; RV64-UF2-NEXT: [[TMP19:%.*]] = add <vscale x 4 x i32> [[REVERSE]], splat (i32 1)
; RV64-UF2-NEXT: [[TMP20:%.*]] = add <vscale x 4 x i32> [[REVERSE2]], splat (i32 1)
@@ -180,8 +180,8 @@ define void @vector_reverse_i32(ptr noalias %A, ptr noalias %B) {
; RV64-UF2-NEXT: [[TMP28:%.*]] = getelementptr inbounds i32, ptr [[TMP21]], i64 [[TMP26]]
; RV64-UF2-NEXT: [[TMP29:%.*]] = getelementptr inbounds i32, ptr [[TMP28]], i64 [[TMP27]]
; RV64-UF2-NEXT: [[REVERSE3:%.*]] = call <vscale x 4 x i32> @llvm.vector.reverse.nxv4i32(<vscale x 4 x i32> [[TMP19]])
-; RV64-UF2-NEXT: store <vscale x 4 x i32> [[REVERSE3]], ptr [[TMP25]], align 4
; RV64-UF2-NEXT: [[REVERSE4:%.*]] = call <vscale x 4 x i32> @llvm.vector.reverse.nxv4i32(<vscale x 4 x i32> [[TMP20]])
+; RV64-UF2-NEXT: store <vscale x 4 x i32> [[REVERSE3]], ptr [[TMP25]], align 4
; RV64-UF2-NEXT: store <vscale x 4 x i32> [[REVERSE4]], ptr [[TMP29]], align 4
; RV64-UF2-NEXT: [[INDEX_NEXT]] = add nuw i64 [[INDEX]], [[TMP6]]
; RV64-UF2-NEXT: [[TMP30:%.*]] = icmp eq i64 [[INDEX_NEXT]], [[N_VEC]]
@@ -371,8 +371,8 @@ define void @vector_reverse_f32(ptr noalias %A, ptr noalias %B) {
; RV64-UF2-NEXT: [[TMP17:%.*]] = getelementptr inbounds float, ptr [[TMP10]], i64 [[TMP15]]
; RV64-UF2-NEXT: [[TMP18:%.*]] = getelementptr inbounds float, ptr [[TMP17]], i64 [[TMP16]]
; RV64-UF2-NEXT: [[WIDE_LOAD:%.*]] = load <vscale x 4 x float>, ptr [[TMP14]], align 4
-; RV64-UF2-NEXT: [[REVERSE:%.*]] = call <vscale x 4 x float> @llvm.vector.reverse.nxv4f32(<vscale x 4 x float> [[WIDE_LOAD]])
; RV64-UF2-NEXT: [[WIDE_LOAD1:%.*]] = load <vscale x 4 x float>, ptr [[TMP18]], align 4
+; RV64-UF2-NEXT: [[REVERSE:%.*]] = call <vscale x 4 x float> @llvm.vector.reverse.nxv4f32(<vscale x 4 x float> [[WIDE_LOAD]])
; RV64-UF2-NEXT: [[REVERSE2:%.*]] = call <vscale x 4 x float> @llvm.vector.reverse.nxv4f32(<vscale x 4 x float> [[WIDE_LOAD1]])
; RV64-UF2-NEXT: [[TMP19:%.*]] = fadd <vscale x 4 x float> [[REVERSE]], splat (float 1.000000e+00)
; RV64-UF2-NEXT: [[TMP20:%.*]] = fadd <vscale x 4 x float> [[REVERSE2]], splat (float 1.000000e+00)
@@ -386,8 +386,8 @@ define void @vector_reverse_f32(ptr noalias %A, ptr noalias %B) {
; RV64-UF2-NEXT: [[TMP28:%.*]] = getelementptr inbounds float, ptr [[TMP21]], i64 [[TMP26]]
; RV64-UF2-NEXT: [[TMP29:%.*]] = getelementptr inbounds float, ptr [[TMP28]], i64 [[TMP27]]
; RV64-UF2-NEXT: [[REVERSE3:%.*]] = call <vscale x 4 x float> @llvm.vector.reverse.nxv4f32(<vscale x 4 x float> [[TMP19]])
-; RV64-UF2-NEXT: store <vscale x 4 x float> [[REVERSE3]], ptr [[TMP25]], align 4
; RV64-UF2-NEXT: [[REVERSE4:%.*]] = call <vscale x 4 x float> @llvm.vector.reverse.nxv4f32(<vscale x 4 x float> [[TMP20]])
+; RV64-UF2-NEXT: store <vscale x 4 x float> [[REVERSE3]], ptr [[TMP25]], align 4
; RV64-UF2-NEXT: store <vscale x 4 x float> [[REVERSE4]], ptr [[TMP29]], align 4
; RV64-UF2-NEXT: [[INDEX_NEXT]] = add nuw i64 [[INDEX]], [[TMP6]]
; RV64-UF2-NEXT: [[TMP30:%.*]] = icmp eq i64 [[INDEX_NEXT]], [[N_VEC]]
diff --git a/llvm/test/Transforms/LoopVectorize/RISCV/riscv-vector-reverse.ll b/llvm/test/Transforms/LoopVectorize/RISCV/riscv-vector-reverse.ll
index dd8b7d6ea7e42..6d49a7fc16ad5 100644
--- a/llvm/test/Transforms/LoopVectorize/RISCV/riscv-vector-reverse.ll
+++ b/llvm/test/Transforms/LoopVectorize/RISCV/riscv-vector-reverse.ll
@@ -105,10 +105,12 @@ define void @vector_reverse_i64(ptr nocapture noundef writeonly %A, ptr nocaptur
; CHECK-NEXT: CLONE ir<%arrayidx> = getelementptr inbounds ir<%B>, ir<%idxprom>
; CHECK-NEXT: vp<%9> = vector-end-pointer inbounds ir<%arrayidx>, vp<%0>
; CHECK-NEXT: WIDEN ir<%1> = load vp<%9>
-; CHECK-NEXT: WIDEN ir<%add9> = add ir<%1>, ir<1>
+; CHECK-NEXT: EMIT vp<%10> = reverse ir<%1>
+; CHECK-NEXT: WIDEN ir<%add9> = add vp<%10>, ir<1>
; CHECK-NEXT: CLONE ir<%arrayidx3> = getelementptr inbounds ir<%A>, ir<%idxprom>
-; CHECK-NEXT: vp<%10> = vector-end-pointer inbounds ir<%arrayidx3>, vp<%0>
-; CHECK-NEXT: WIDEN store vp<%10>, ir<%add9>
+; CHECK-NEXT: vp<%11> = vector-end-pointer inbounds ir<%arrayidx3>, vp<%0>
+; CHECK-NEXT: EMIT vp<%12> = reverse ir<%add9>
+; CHECK-NEXT: WIDEN store vp<%11>, vp<%12>
; CHECK-NEXT: EMIT vp<%index.next> = add nuw vp<%6>, vp<%1>
; CHECK-NEXT: EMIT branch-on-count vp<%index.next>, vp<%2>
; CHECK-NEXT: No successors
@@ -167,8 +169,10 @@ define void @vector_reverse_i64(ptr nocapture noundef writeonly %A, ptr nocaptur
; CHECK-NEXT: LV(REG): At #9 Interval # 3
; CHECK-NEXT: LV(REG): At #10 Interval # 3
; CHECK-NEXT: LV(REG): At #11 Interval # 3
-; CHECK-NEXT: LV(REG): At #12 Interval # 2
-; CHECK-NEXT: LV(REG): At #13 Interval # 2
+; CHECK-NEXT: LV(REG): At #12 Interval # 3
+; CHECK-NEXT: LV(REG): At #13 Interval # 3
+; CHECK-NEXT: LV(REG): At #14 Interval # 2
+; CHECK-NEXT: LV(REG): At #15 Interval # 2
; CHECK-NEXT: LV(REG): VF = vscale x 4
; CHECK-NEXT: LV(REG): Found max...
[truncated]
|
This was referenced Jul 1, 2025
lukel97
reviewed
Jul 10, 2025
lukel97
reviewed
Jul 17, 2025
Contributor
lukel97
left a comment
lukel97
left a comment
There was a problem hiding this comment.
+1 on splitting this out, I think this works well with the direction of splitting up big recipes into smaller ones. Just an idea about possibly inserting the reverses in tryToWiden but otherwise generally LGTM
Contributor
There was a problem hiding this comment.
This is a cool optimisation, I guess the LICM transform pulls the VPInstruction::Reverse out of the loop body so convertToEVLRecipes doesn't see it?
Contributor
Author
There was a problem hiding this comment.
Yes. But I think it's fine that this isn't converted into vp.reverse here, since the operand of reverse that can be hoisted by LICM should be uniform. We could even remove the reverse operation entirely in this case in the future.
Contributor
There was a problem hiding this comment.
I just did a quick scan and it looks like the only places where reverse is set is in VPRecipeBuilder::tryToWidenMemory. Instead of introducing another transform, should we just insert the VPInstruction::Reverses there to avoid having to iterate over all the recipes again?
This way would mean we could also remove the Reverse field from VPWidenMemoryRecipe
Contributor
Author
There was a problem hiding this comment.
I was initially worried this might affect interleaved accesses, but I was overthinking it. So far, generating the reverse directly in tryToWidenMemory seems fine.
9d6136b7a21a91fe5c479b9071c113b7802f062f
This way would mean we could also remove the Reverse field from VPWidenMemoryRecipe
We can't remove the Reverse field from VPWidenMemoryRecipe. We still need the reverse mask if it's a reverse access. I also don’t plan to separate the reverse mask either, as I think it would not bring benefit.
Contributor
There was a problem hiding this comment.
Can we also just reverse the mask too at construction?
Contributor
Author
There was a problem hiding this comment.
That’s possible, but the reason I’m not doing it for now is that, in case some reverse operations can't be eliminated, we might want to convert reverse accesses into strided accesses with a stride of -1. Keeping the Reverse field could make it easier to identify the target recipes that need conversion. Also, reverse masks generally can not do permutation elimination, I think. So that’s why I haven’t done it this way.
Contributor
There was a problem hiding this comment.
What I guess I would eventually like to see is that our optimisations to remove the header masks just become plain old peepholes, written pattern match style like in similarRecipes.
E.g. for a regular load we would try and match:
(load ptr, header-mask) -> (vp.load ptr, all-true, evl)
And if we were to split out the reverses for both the data and mask into separate recipes, and add a VF operand like what we currently do for the end pointer, we would also have:
(reverse (load (end-ptr p, vf), (reverse header-mask, vf)), vf)
->
(reverse (vp.load (end-ptr p, evl), all-true, evl), evl)
OR
(vp.strided-load p, all-true, stride=-1, evl)
I think these patterns seem simple enough, and we could probably write them with the VPlanPatternMatch. Most importantly, these transformations don't change the semantics and are just optimisations. So if somehow we miss one of these transforms it will still be correct.
For permutation elimination we would just need to have:
(reverse (reverse x N) N) -> x
Contributor
Author
There was a problem hiding this comment.
Reverse access can indeed be pattern-matched.
1392a872f0b69310340088d6323f1ae3735838c6
I’ve separated out the reverse mask, but this is not easy as we thought. :(
In addition to affecting the cost model since the cost of the reverse mask is currently not computed. In EVL lowering, it also require extra handling for the reverse mask.
lukel97
reviewed
Jul 30, 2025
fhahn
reviewed
Jul 31, 2025
Contributor
There was a problem hiding this comment.
Now that the load/store doesn't handle reversing, it should not need the flag to indicate it is reversing
Contributor
Author
There was a problem hiding this comment.
There was a similar discussion earlier: #146525 (comment)
I think it would be good to continue the discussion in the same comment thread.
Contributor
There was a problem hiding this comment.
The code structure in the function seems inconsistent; converting FirstOrderRecurrenceSplice is handled inline, while handling Reverse is handled in the function. Can we merge the loops, as now we need to unconditionally iterate over all recipes in the loop region anyways?
Contributor
Author
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
fhahn
reviewed
Aug 14, 2025
llvm/test/Transforms/LoopVectorize/AArch64/sve-vector-reverse-mask4.ll
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
now not vectorized any longer?
Contributor
Author
There was a problem hiding this comment.
Yes, also caused by separating the reverse mask from reverse access recipes.
The change is moved to #155579, and will investigate it.
mahesh-attarde
pushed a commit
to mahesh-attarde/llvm-project
that referenced
this pull request
Dec 19, 2025
This patch introduces VPInstruction::Reverse and extracts the reverse operations of loaded/stored values from reverse memory accesses. This extraction facilitates future support for permutation elimination within VPlan.
valadaptive
pushed a commit
to valadaptive/llvm-project
that referenced
this pull request
Dec 24, 2025
This patch introduces VPInstruction::Reverse and extracts the reverse operations of loaded/stored values from reverse memory accesses. This extraction facilitates future support for permutation elimination within VPlan.
Member
|
Hi Mel, looks like this patch causes crashes |
Contributor
Author
Hi Alexey, I hope #173856 can fix the issue. Thanks for reporting the bug, and have a great holiday. |
fhahn
added a commit
to fhahn/llvm-project
that referenced
this pull request
Dec 30, 2025
Currently we need to precompute costs for exit conditions, to match the legacy cost, as they will get replaced by a compare against the canonical IV (or others, like active-lane-mask or EVL based) and the original compare will get removed. This is not true for instructions with users other than the exit condition. Those will remain, and we can just use the VPlan-based cost model to get more accurate results. This improves results in some cases, like @test_value_in_exit_compare_chain_used_outside because the IV increment user outside the loop is replaced by computing the final value outside the loop. It also fixes a crash introduced by f196b1d (llvm#146525).
fhahn
added a commit
that referenced
this pull request
Dec 31, 2025
#174029) Currently we need to precompute costs for exit conditions, to match the legacy cost, as they will get replaced by a compare against the canonical IV (or others, like active-lane-mask or EVL based) and the original compare will get removed. This is not true for instructions with users other than the exit condition. Those will remain, and we can just use the VPlan-based cost model to get more accurate results. This improves results in some cases, like @test_value_in_exit_compare_chain_used_outside because the IV increment user outside the loop is replaced by computing the final value outside the loop. It also fixes a crash introduced by f196b1d (#146525). PR: #174029
mahesh-attarde
pushed a commit
to mahesh-attarde/llvm-project
that referenced
this pull request
Jan 6, 2026
llvm#174029) Currently we need to precompute costs for exit conditions, to match the legacy cost, as they will get replaced by a compare against the canonical IV (or others, like active-lane-mask or EVL based) and the original compare will get removed. This is not true for instructions with users other than the exit condition. Those will remain, and we can just use the VPlan-based cost model to get more accurate results. This improves results in some cases, like @test_value_in_exit_compare_chain_used_outside because the IV increment user outside the loop is replaced by computing the final value outside the loop. It also fixes a crash introduced by f196b1d (llvm#146525). PR: llvm#174029
Mel-Chen
added a commit
that referenced
this pull request
Mar 31, 2026
Following #146525, separate the reverse mask from reverse access recipes. At the same time, remove the unused member variable `Reverse` from `VPWidenMemoryRecipe`. This will help to reduce redundant reverse mask computations by VPlan-based common subexpression elimination.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This patch introduces VPInstruction::Reverse and extracts the reverse operations of loaded/stored values from reverse memory accesses. This extraction facilitates future support for permutation elimination within VPlan.