-
Notifications
You must be signed in to change notification settings - Fork 12.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RISCV] Merge RISCVCoalesceVSETVLI back into RISCVInsertVSETVLI #92869
[RISCV] Merge RISCVCoalesceVSETVLI back into RISCVInsertVSETVLI #92869
Conversation
@llvm/pr-subscribers-backend-risc-v Author: Luke Lau (lukel97) ChangesWe no longer need to separate the passes now that #70549 is landed and this will unblock #89089. It's not strictly NFC because it will move coalescing before register allocation when -riscv-vsetvl-after-rvv-regalloc is disabled. But this makes it closer to the original behaviour. Full diff: https://github.com/llvm/llvm-project/pull/92869.diff 7 Files Affected:
diff --git a/llvm/lib/Target/RISCV/RISCV.h b/llvm/lib/Target/RISCV/RISCV.h
index 2b8688c5de61f..dcf4c65c44dff 100644
--- a/llvm/lib/Target/RISCV/RISCV.h
+++ b/llvm/lib/Target/RISCV/RISCV.h
@@ -62,9 +62,6 @@ FunctionPass *createRISCVInsertVSETVLIPass();
void initializeRISCVInsertVSETVLIPass(PassRegistry &);
extern char &RISCVInsertVSETVLIID;
-FunctionPass *createRISCVCoalesceVSETVLIPass();
-void initializeRISCVCoalesceVSETVLIPass(PassRegistry &);
-
FunctionPass *createRISCVPostRAExpandPseudoPass();
void initializeRISCVPostRAExpandPseudoPass(PassRegistry &);
FunctionPass *createRISCVInsertReadWriteCSRPass();
diff --git a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
index c0b2a695b8ea4..2d60cd9b3c567 100644
--- a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
@@ -378,10 +378,10 @@ static bool areCompatibleVTYPEs(uint64_t CurVType, uint64_t NewVType,
/// Return the fields and properties demanded by the provided instruction.
DemandedFields getDemanded(const MachineInstr &MI, const RISCVSubtarget *ST) {
- // This function works in RISCVCoalesceVSETVLI too. We can still use the value
- // of a SEW, VL, or Policy operand even though it might not be the exact value
- // in the VL or VTYPE, since we only care about what the instruction
- // originally demanded.
+ // This function works in coalesceVSETVLI too. We can still use the value of a
+ // SEW, VL, or Policy operand even though it might not be the exact value in
+ // the VL or VTYPE, since we only care about what the instruction originally
+ // demanded.
// Most instructions don't use any of these subfeilds.
DemandedFields Res;
@@ -900,36 +900,7 @@ class RISCVInsertVSETVLI : public MachineFunctionPass {
void emitVSETVLIs(MachineBasicBlock &MBB);
void doPRE(MachineBasicBlock &MBB);
void insertReadVL(MachineBasicBlock &MBB);
-};
-
-class RISCVCoalesceVSETVLI : public MachineFunctionPass {
-public:
- static char ID;
- const RISCVSubtarget *ST;
- const TargetInstrInfo *TII;
- MachineRegisterInfo *MRI;
- LiveIntervals *LIS;
-
- RISCVCoalesceVSETVLI() : MachineFunctionPass(ID) {}
- bool runOnMachineFunction(MachineFunction &MF) override;
-
- void getAnalysisUsage(AnalysisUsage &AU) const override {
- AU.setPreservesCFG();
-
- AU.addRequired<LiveIntervals>();
- AU.addPreserved<LiveIntervals>();
- AU.addRequired<SlotIndexes>();
- AU.addPreserved<SlotIndexes>();
- AU.addPreserved<LiveDebugVariables>();
- AU.addPreserved<LiveStacks>();
-
- MachineFunctionPass::getAnalysisUsage(AU);
- }
-
- StringRef getPassName() const override { return RISCV_COALESCE_VSETVLI_NAME; }
-
-private:
- bool coalesceVSETVLIs(MachineBasicBlock &MBB);
+ void coalesceVSETVLIs(MachineBasicBlock &MBB) const;
};
} // end anonymous namespace
@@ -940,11 +911,6 @@ char &llvm::RISCVInsertVSETVLIID = RISCVInsertVSETVLI::ID;
INITIALIZE_PASS(RISCVInsertVSETVLI, DEBUG_TYPE, RISCV_INSERT_VSETVLI_NAME,
false, false)
-char RISCVCoalesceVSETVLI::ID = 0;
-
-INITIALIZE_PASS(RISCVCoalesceVSETVLI, "riscv-coalesce-vsetvli",
- RISCV_COALESCE_VSETVLI_NAME, false, false)
-
// Return a VSETVLIInfo representing the changes made by this VSETVLI or
// VSETIVLI instruction.
static VSETVLIInfo getInfoForVSETVLI(const MachineInstr &MI,
@@ -1653,7 +1619,7 @@ static bool canMutatePriorConfig(const MachineInstr &PrevMI,
return areCompatibleVTYPEs(PriorVType, VType, Used);
}
-bool RISCVCoalesceVSETVLI::coalesceVSETVLIs(MachineBasicBlock &MBB) {
+void RISCVInsertVSETVLI::coalesceVSETVLIs(MachineBasicBlock &MBB) const {
MachineInstr *NextMI = nullptr;
// We can have arbitrary code in successors, so VL and VTYPE
// must be considered demanded.
@@ -1745,8 +1711,6 @@ bool RISCVCoalesceVSETVLI::coalesceVSETVLIs(MachineBasicBlock &MBB) {
LIS->RemoveMachineInstrFromMaps(*MI);
MI->eraseFromParent();
}
-
- return !ToDelete.empty();
}
void RISCVInsertVSETVLI::insertReadVL(MachineBasicBlock &MBB) {
@@ -1836,6 +1800,15 @@ bool RISCVInsertVSETVLI::runOnMachineFunction(MachineFunction &MF) {
for (MachineBasicBlock &MBB : MF)
insertReadVL(MBB);
+ // Now that all vsetvlis are explicit, go through and do block local
+ // DSE and peephole based demanded fields based transforms. Note that
+ // this *must* be done outside the main dataflow so long as we allow
+ // any cross block analysis within the dataflow. We can't have both
+ // demanded fields based mutation and non-local analysis in the
+ // dataflow at the same time without introducing inconsistencies.
+ for (MachineBasicBlock &MBB : MF)
+ coalesceVSETVLIs(MBB);
+
BlockInfo.clear();
return HaveVectorOp;
}
@@ -1844,29 +1817,3 @@ bool RISCVInsertVSETVLI::runOnMachineFunction(MachineFunction &MF) {
FunctionPass *llvm::createRISCVInsertVSETVLIPass() {
return new RISCVInsertVSETVLI();
}
-
-// Now that all vsetvlis are explicit, go through and do block local
-// DSE and peephole based demanded fields based transforms. Note that
-// this *must* be done outside the main dataflow so long as we allow
-// any cross block analysis within the dataflow. We can't have both
-// demanded fields based mutation and non-local analysis in the
-// dataflow at the same time without introducing inconsistencies.
-bool RISCVCoalesceVSETVLI::runOnMachineFunction(MachineFunction &MF) {
- // Skip if the vector extension is not enabled.
- ST = &MF.getSubtarget<RISCVSubtarget>();
- if (!ST->hasVInstructions())
- return false;
- TII = ST->getInstrInfo();
- MRI = &MF.getRegInfo();
- LIS = &getAnalysis<LiveIntervals>();
-
- bool Changed = false;
- for (MachineBasicBlock &MBB : MF)
- Changed |= coalesceVSETVLIs(MBB);
-
- return Changed;
-}
-
-FunctionPass *llvm::createRISCVCoalesceVSETVLIPass() {
- return new RISCVCoalesceVSETVLI();
-}
diff --git a/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp b/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
index d9f8222669cab..87ae2ee0d3791 100644
--- a/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
+++ b/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
@@ -121,7 +121,6 @@ extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeRISCVTarget() {
initializeRISCVExpandPseudoPass(*PR);
initializeRISCVFoldMasksPass(*PR);
initializeRISCVInsertVSETVLIPass(*PR);
- initializeRISCVCoalesceVSETVLIPass(*PR);
initializeRISCVInsertReadWriteCSRPass(*PR);
initializeRISCVInsertWriteVXRMPass(*PR);
initializeRISCVDAGToDAGISelPass(*PR);
@@ -396,7 +395,6 @@ bool RISCVPassConfig::addRegAssignAndRewriteFast() {
addPass(createRVVRegAllocPass(false));
if (EnableVSETVLIAfterRVVRegAlloc)
addPass(createRISCVInsertVSETVLIPass());
- addPass(createRISCVCoalesceVSETVLIPass());
if (TM->getOptLevel() != CodeGenOptLevel::None &&
EnableRISCVDeadRegisterElimination)
addPass(createRISCVDeadRegisterDefinitionsPass());
@@ -408,7 +406,6 @@ bool RISCVPassConfig::addRegAssignAndRewriteOptimized() {
addPass(createVirtRegRewriter(false));
if (EnableVSETVLIAfterRVVRegAlloc)
addPass(createRISCVInsertVSETVLIPass());
- addPass(createRISCVCoalesceVSETVLIPass());
if (TM->getOptLevel() != CodeGenOptLevel::None &&
EnableRISCVDeadRegisterElimination)
addPass(createRISCVDeadRegisterDefinitionsPass());
diff --git a/llvm/test/CodeGen/RISCV/O0-pipeline.ll b/llvm/test/CodeGen/RISCV/O0-pipeline.ll
index e4abc93d1a8a1..ef7a8f2c7bbee 100644
--- a/llvm/test/CodeGen/RISCV/O0-pipeline.ll
+++ b/llvm/test/CodeGen/RISCV/O0-pipeline.ll
@@ -50,7 +50,6 @@
; CHECK-NEXT: Slot index numbering
; CHECK-NEXT: Live Interval Analysis
; CHECK-NEXT: RISC-V Insert VSETVLI pass
-; CHECK-NEXT: RISC-V Coalesce VSETVLI pass
; CHECK-NEXT: Fast Register Allocator
; CHECK-NEXT: Remove Redundant DEBUG_VALUE analysis
; CHECK-NEXT: Fixup Statepoint Caller Saved
diff --git a/llvm/test/CodeGen/RISCV/O3-pipeline.ll b/llvm/test/CodeGen/RISCV/O3-pipeline.ll
index 0528b00d408b2..1d1c5942aa8e9 100644
--- a/llvm/test/CodeGen/RISCV/O3-pipeline.ll
+++ b/llvm/test/CodeGen/RISCV/O3-pipeline.ll
@@ -142,7 +142,6 @@
; CHECK-NEXT: Greedy Register Allocator
; CHECK-NEXT: Virtual Register Rewriter
; CHECK-NEXT: RISC-V Insert VSETVLI pass
-; CHECK-NEXT: RISC-V Coalesce VSETVLI pass
; CHECK-NEXT: RISC-V Dead register definitions
; CHECK-NEXT: Virtual Register Map
; CHECK-NEXT: Live Register Matrix
diff --git a/llvm/test/CodeGen/RISCV/rvv/coalesce-vsetvli.mir b/llvm/test/CodeGen/RISCV/rvv/coalesce-vsetvli.mir
deleted file mode 100644
index f888534ebc035..0000000000000
--- a/llvm/test/CodeGen/RISCV/rvv/coalesce-vsetvli.mir
+++ /dev/null
@@ -1,66 +0,0 @@
-# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 4
-# RUN: llc %s -o - -mtriple=riscv64 -mattr=v -run-pass=riscv-coalesce-vsetvli -verify-machineinstrs | FileCheck %s
-
----
-name: dead_avl_addi
-tracksRegLiveness: true
-body: |
- bb.0:
- ; CHECK-LABEL: name: dead_avl_addi
- ; CHECK: $x0 = PseudoVSETIVLI 3, 216 /* e64, m1, ta, ma */, implicit-def $vl, implicit-def $vtype
- ; CHECK-NEXT: dead %x:gpr = PseudoVMV_X_S $noreg, 6 /* e64 */
- ; CHECK-NEXT: $v0 = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, 3, 6 /* e64 */, 0 /* tu, mu */
- ; CHECK-NEXT: PseudoRET
- %avl:gprnox0 = ADDI $x0, 42
- dead $x0 = PseudoVSETVLI killed %avl, 216, implicit-def $vl, implicit-def $vtype
- %x:gpr = PseudoVMV_X_S $noreg, 6
- dead $x0 = PseudoVSETIVLI 3, 216, implicit-def $vl, implicit-def $vtype
- $v0 = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, 3, 6, 0
- PseudoRET
-...
----
-name: dead_avl_nonvolatile_load
-tracksRegLiveness: true
-body: |
- bb.0:
- liveins: $x1
- ; CHECK-LABEL: name: dead_avl_nonvolatile_load
- ; CHECK: liveins: $x1
- ; CHECK-NEXT: {{ $}}
- ; CHECK-NEXT: %ptr:gpr = COPY $x1
- ; CHECK-NEXT: dead %avl:gprnox0 = LW %ptr, 0 :: (dereferenceable load (s32))
- ; CHECK-NEXT: $x0 = PseudoVSETIVLI 3, 216 /* e64, m1, ta, ma */, implicit-def $vl, implicit-def $vtype
- ; CHECK-NEXT: dead %x:gpr = PseudoVMV_X_S $noreg, 6 /* e64 */
- ; CHECK-NEXT: $v0 = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, 3, 6 /* e64 */, 0 /* tu, mu */
- ; CHECK-NEXT: PseudoRET
- %ptr:gpr = COPY $x1
- %avl:gprnox0 = LW killed %ptr, 0 :: (dereferenceable load (s32))
- dead $x0 = PseudoVSETVLI killed %avl, 216, implicit-def $vl, implicit-def $vtype
- %x:gpr = PseudoVMV_X_S $noreg, 6
- dead $x0 = PseudoVSETIVLI 3, 216, implicit-def $vl, implicit-def $vtype
- $v0 = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, 3, 6, 0
- PseudoRET
-...
----
-name: dead_avl_volatile_load
-tracksRegLiveness: true
-body: |
- bb.0:
- liveins: $x1
- ; CHECK-LABEL: name: dead_avl_volatile_load
- ; CHECK: liveins: $x1
- ; CHECK-NEXT: {{ $}}
- ; CHECK-NEXT: %ptr:gpr = COPY $x1
- ; CHECK-NEXT: dead %avl:gprnox0 = LW %ptr, 0 :: (volatile dereferenceable load (s32))
- ; CHECK-NEXT: $x0 = PseudoVSETIVLI 3, 216 /* e64, m1, ta, ma */, implicit-def $vl, implicit-def $vtype
- ; CHECK-NEXT: dead %x:gpr = PseudoVMV_X_S $noreg, 6 /* e64 */
- ; CHECK-NEXT: $v0 = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, 3, 6 /* e64 */, 0 /* tu, mu */
- ; CHECK-NEXT: PseudoRET
- %ptr:gpr = COPY $x1
- %avl:gprnox0 = LW killed %ptr, 0 :: (volatile dereferenceable load (s32))
- dead $x0 = PseudoVSETVLI killed %avl, 216, implicit-def $vl, implicit-def $vtype
- %x:gpr = PseudoVMV_X_S $noreg, 6
- dead $x0 = PseudoVSETIVLI 3, 216, implicit-def $vl, implicit-def $vtype
- $v0 = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, 3, 6, 0
- PseudoRET
-...
diff --git a/llvm/test/CodeGen/RISCV/rvv/vsetvli-insert.mir b/llvm/test/CodeGen/RISCV/rvv/vsetvli-insert.mir
index 41a68ef9903e8..a4b374c8bb401 100644
--- a/llvm/test/CodeGen/RISCV/rvv/vsetvli-insert.mir
+++ b/llvm/test/CodeGen/RISCV/rvv/vsetvli-insert.mir
@@ -1,6 +1,6 @@
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
-# RUN: llc %s -o - -mtriple=riscv64 -mattr=v \
-# RUN: -run-pass=riscv-insert-vsetvli,riscv-coalesce-vsetvli | FileCheck %s
+# RUN: llc %s -o - -mtriple=riscv64 -mattr=v -run-pass=riscv-insert-vsetvli \
+# RUN: | FileCheck %s
--- |
source_filename = "vsetvli-insert.ll"
@@ -80,6 +80,18 @@
ret void
}
+ define void @coalesce_dead_avl_addi() {
+ ret void
+ }
+
+ define void @coalesce_dead_avl_nonvolatile_load() {
+ ret void
+ }
+
+ define void @coalesce_dead_avl_volatile_load() {
+ ret void
+ }
+
declare <vscale x 1 x i64> @llvm.riscv.vadd.nxv1i64.nxv1i64.i64(<vscale x 1 x i64>, <vscale x 1 x i64>, <vscale x 1 x i64>, i64) #1
declare <vscale x 1 x i64> @llvm.riscv.vle.nxv1i64.i64(<vscale x 1 x i64>, ptr nocapture, i64) #4
@@ -501,3 +513,66 @@ body: |
%4:vr = PseudoVADD_VV_M1 undef $noreg, undef $noreg, undef $noreg, 3, 6, 0
PseudoRET
...
+---
+name: coalesce_dead_avl_addi
+tracksRegLiveness: true
+body: |
+ bb.0:
+ ; CHECK-LABEL: name: coalesce_dead_avl_addi
+ ; CHECK: $x0 = PseudoVSETIVLI 3, 216 /* e64, m1, ta, ma */, implicit-def $vl, implicit-def $vtype
+ ; CHECK-NEXT: dead %x:gpr = PseudoVMV_X_S $noreg, 6 /* e64 */, implicit $vtype
+ ; CHECK-NEXT: $v0 = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, 3, 6 /* e64 */, 0 /* tu, mu */, implicit $vl, implicit $vtype
+ ; CHECK-NEXT: PseudoRET
+ %avl:gprnox0 = ADDI $x0, 42
+ dead $x0 = PseudoVSETVLI killed %avl, 216, implicit-def $vl, implicit-def $vtype
+ %x:gpr = PseudoVMV_X_S $noreg, 6
+ dead $x0 = PseudoVSETIVLI 3, 216, implicit-def $vl, implicit-def $vtype
+ $v0 = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, 3, 6, 0
+ PseudoRET
+...
+---
+name: coalesce_dead_avl_nonvolatile_load
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $x1
+ ; CHECK-LABEL: name: coalesce_dead_avl_nonvolatile_load
+ ; CHECK: liveins: $x1
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: %ptr:gpr = COPY $x1
+ ; CHECK-NEXT: dead %avl:gprnox0 = LW %ptr, 0 :: (dereferenceable load (s32))
+ ; CHECK-NEXT: $x0 = PseudoVSETIVLI 3, 216 /* e64, m1, ta, ma */, implicit-def $vl, implicit-def $vtype
+ ; CHECK-NEXT: dead %x:gpr = PseudoVMV_X_S $noreg, 6 /* e64 */, implicit $vtype
+ ; CHECK-NEXT: $v0 = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, 3, 6 /* e64 */, 0 /* tu, mu */, implicit $vl, implicit $vtype
+ ; CHECK-NEXT: PseudoRET
+ %ptr:gpr = COPY $x1
+ %avl:gprnox0 = LW killed %ptr, 0 :: (dereferenceable load (s32))
+ dead $x0 = PseudoVSETVLI killed %avl, 216, implicit-def $vl, implicit-def $vtype
+ %x:gpr = PseudoVMV_X_S $noreg, 6
+ dead $x0 = PseudoVSETIVLI 3, 216, implicit-def $vl, implicit-def $vtype
+ $v0 = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, 3, 6, 0
+ PseudoRET
+...
+---
+name: coalesce_dead_avl_volatile_load
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $x1
+ ; CHECK-LABEL: name: coalesce_dead_avl_volatile_load
+ ; CHECK: liveins: $x1
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: %ptr:gpr = COPY $x1
+ ; CHECK-NEXT: dead %avl:gprnox0 = LW %ptr, 0 :: (volatile dereferenceable load (s32))
+ ; CHECK-NEXT: $x0 = PseudoVSETIVLI 3, 216 /* e64, m1, ta, ma */, implicit-def $vl, implicit-def $vtype
+ ; CHECK-NEXT: dead %x:gpr = PseudoVMV_X_S $noreg, 6 /* e64 */, implicit $vtype
+ ; CHECK-NEXT: $v0 = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, 3, 6 /* e64 */, 0 /* tu, mu */, implicit $vl, implicit $vtype
+ ; CHECK-NEXT: PseudoRET
+ %ptr:gpr = COPY $x1
+ %avl:gprnox0 = LW killed %ptr, 0 :: (volatile dereferenceable load (s32))
+ dead $x0 = PseudoVSETVLI killed %avl, 216, implicit-def $vl, implicit-def $vtype
+ %x:gpr = PseudoVMV_X_S $noreg, 6
+ dead $x0 = PseudoVSETIVLI 3, 216, implicit-def $vl, implicit-def $vtype
+ $v0 = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, 3, 6, 0
+ PseudoRET
+...
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but please wait until early next week before landing. If we have any fallout from the prior big change, it's good to avoid needing to revert a bunch of follow on stuff.
We no longer need to separate the passes now that llvm#70549 is landed and this will unblock llvm#89089. It's not strictly NFC because it will move coalescing before register allocation when -riscv-vsetvl-after-rvv-regalloc is disabled. But this makes it closer to the original behaviour.
49ffca4
to
73cf472
Compare
…rtReadVL In llvm#88295 we split out coalesceVSETVLIs (f.k.a doLocalPostpass) into a separate pass, which meant moving it past the call to insertReadVL. Whenever we merged it back in llvm#92869, we accidentally moved it after insertReadVL. This patch moves it back to its original position.
We no longer need to separate the passes now that #70549 is landed and this will unblock #89089.
It's not strictly NFC because it will move coalescing before register allocation when -riscv-vsetvl-after-rvv-regalloc is disabled. But this makes it closer to the original behaviour.