[llvm] e291336 - [MachineCopyPropagation][RISCV] Fix D125335 accidentally change control flow.
Han-Kuan Chen via llvm-commits
llvm-commits at lists.llvm.org
Fri Jun 17 21:40:47 PDT 2022
Author: Han-Kuan Chen
Date: 2022-06-17T21:40:08-07:00
New Revision: e29133629b3d389b3c7cd9083874de5792747967
URL: https://github.com/llvm/llvm-project/commit/e29133629b3d389b3c7cd9083874de5792747967
DIFF: https://github.com/llvm/llvm-project/commit/e29133629b3d389b3c7cd9083874de5792747967.diff
LOG: [MachineCopyPropagation][RISCV] Fix D125335 accidentally change control flow.
D125335 makes regsOverlap skip following control flow, which is not entended
in the original code.
Differential Revision: https://reviews.llvm.org/D128039
Added:
Modified:
llvm/lib/CodeGen/MachineCopyPropagation.cpp
llvm/test/CodeGen/RISCV/machine-cp.mir
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/MachineCopyPropagation.cpp b/llvm/lib/CodeGen/MachineCopyPropagation.cpp
index a8909c20d7a2..66f0eb83e57c 100644
--- a/llvm/lib/CodeGen/MachineCopyPropagation.cpp
+++ b/llvm/lib/CodeGen/MachineCopyPropagation.cpp
@@ -660,77 +660,76 @@ void MachineCopyPropagation::ForwardCopyPropagateBlock(MachineBasicBlock &MBB) {
Register RegSrc = CopyOperands->Source->getReg();
Register RegDef = CopyOperands->Destination->getReg();
- if (TRI->regsOverlap(RegDef, RegSrc))
- continue;
-
- assert(RegDef.isPhysical() && RegSrc.isPhysical() &&
- "MachineCopyPropagation should be run after register allocation!");
-
- MCRegister Def = RegDef.asMCReg();
- MCRegister Src = RegSrc.asMCReg();
-
- // The two copies cancel out and the source of the first copy
- // hasn't been overridden, eliminate the second one. e.g.
- // %ecx = COPY %eax
- // ... nothing clobbered eax.
- // %eax = COPY %ecx
- // =>
- // %ecx = COPY %eax
- //
- // or
- //
- // %ecx = COPY %eax
- // ... nothing clobbered eax.
- // %ecx = COPY %eax
- // =>
- // %ecx = COPY %eax
- if (eraseIfRedundant(MI, Def, Src) || eraseIfRedundant(MI, Src, Def))
- continue;
+ if (!TRI->regsOverlap(RegDef, RegSrc)) {
+ assert(RegDef.isPhysical() && RegSrc.isPhysical() &&
+ "MachineCopyPropagation should be run after register allocation!");
+
+ MCRegister Def = RegDef.asMCReg();
+ MCRegister Src = RegSrc.asMCReg();
+
+ // The two copies cancel out and the source of the first copy
+ // hasn't been overridden, eliminate the second one. e.g.
+ // %ecx = COPY %eax
+ // ... nothing clobbered eax.
+ // %eax = COPY %ecx
+ // =>
+ // %ecx = COPY %eax
+ //
+ // or
+ //
+ // %ecx = COPY %eax
+ // ... nothing clobbered eax.
+ // %ecx = COPY %eax
+ // =>
+ // %ecx = COPY %eax
+ if (eraseIfRedundant(MI, Def, Src) || eraseIfRedundant(MI, Src, Def))
+ continue;
- forwardUses(MI);
+ forwardUses(MI);
+
+ // Src may have been changed by forwardUses()
+ CopyOperands = isCopyInstr(MI, *TII, UseCopyInstr);
+ Src = CopyOperands->Source->getReg().asMCReg();
+
+ // If Src is defined by a previous copy, the previous copy cannot be
+ // eliminated.
+ ReadRegister(Src, MI, RegularUse);
+ for (const MachineOperand &MO : MI.implicit_operands()) {
+ if (!MO.isReg() || !MO.readsReg())
+ continue;
+ MCRegister Reg = MO.getReg().asMCReg();
+ if (!Reg)
+ continue;
+ ReadRegister(Reg, MI, RegularUse);
+ }
- // Src may have been changed by forwardUses()
- CopyOperands = isCopyInstr(MI, *TII, UseCopyInstr);
- Src = CopyOperands->Source->getReg().asMCReg();
+ LLVM_DEBUG(dbgs() << "MCP: Copy is a deletion candidate: "; MI.dump());
+
+ // Copy is now a candidate for deletion.
+ if (!MRI->isReserved(Def))
+ MaybeDeadCopies.insert(&MI);
+
+ // If 'Def' is previously source of another copy, then this earlier copy's
+ // source is no longer available. e.g.
+ // %xmm9 = copy %xmm2
+ // ...
+ // %xmm2 = copy %xmm0
+ // ...
+ // %xmm2 = copy %xmm9
+ Tracker.clobberRegister(Def, *TRI, *TII, UseCopyInstr);
+ for (const MachineOperand &MO : MI.implicit_operands()) {
+ if (!MO.isReg() || !MO.isDef())
+ continue;
+ MCRegister Reg = MO.getReg().asMCReg();
+ if (!Reg)
+ continue;
+ Tracker.clobberRegister(Reg, *TRI, *TII, UseCopyInstr);
+ }
- // If Src is defined by a previous copy, the previous copy cannot be
- // eliminated.
- ReadRegister(Src, MI, RegularUse);
- for (const MachineOperand &MO : MI.implicit_operands()) {
- if (!MO.isReg() || !MO.readsReg())
- continue;
- MCRegister Reg = MO.getReg().asMCReg();
- if (!Reg)
- continue;
- ReadRegister(Reg, MI, RegularUse);
- }
+ Tracker.trackCopy(&MI, *TRI, *TII, UseCopyInstr);
- LLVM_DEBUG(dbgs() << "MCP: Copy is a deletion candidate: "; MI.dump());
-
- // Copy is now a candidate for deletion.
- if (!MRI->isReserved(Def))
- MaybeDeadCopies.insert(&MI);
-
- // If 'Def' is previously source of another copy, then this earlier copy's
- // source is no longer available. e.g.
- // %xmm9 = copy %xmm2
- // ...
- // %xmm2 = copy %xmm0
- // ...
- // %xmm2 = copy %xmm9
- Tracker.clobberRegister(Def, *TRI, *TII, UseCopyInstr);
- for (const MachineOperand &MO : MI.implicit_operands()) {
- if (!MO.isReg() || !MO.isDef())
- continue;
- MCRegister Reg = MO.getReg().asMCReg();
- if (!Reg)
- continue;
- Tracker.clobberRegister(Reg, *TRI, *TII, UseCopyInstr);
+ continue;
}
-
- Tracker.trackCopy(&MI, *TRI, *TII, UseCopyInstr);
-
- continue;
}
// Clobber any earlyclobber regs first.
@@ -927,23 +926,22 @@ void MachineCopyPropagation::BackwardCopyPropagateBlock(
for (MachineInstr &MI : llvm::make_early_inc_range(llvm::reverse(MBB))) {
// Ignore non-trivial COPYs.
Optional<DestSourcePair> CopyOperands = isCopyInstr(MI, *TII, UseCopyInstr);
- if (CopyOperands) {
+ if (CopyOperands && MI.getNumOperands() == 2) {
Register DefReg = CopyOperands->Destination->getReg();
Register SrcReg = CopyOperands->Source->getReg();
- if (TRI->regsOverlap(DefReg, SrcReg))
- continue;
-
- MCRegister Def = DefReg.asMCReg();
- MCRegister Src = SrcReg.asMCReg();
+ if (!TRI->regsOverlap(DefReg, SrcReg)) {
+ MCRegister Def = DefReg.asMCReg();
+ MCRegister Src = SrcReg.asMCReg();
- // Unlike forward cp, we don't invoke propagateDefs here,
- // just let forward cp do COPY-to-COPY propagation.
- if (isBackwardPropagatableCopy(MI, *MRI, *TII, UseCopyInstr)) {
- Tracker.invalidateRegister(Src, *TRI, *TII, UseCopyInstr);
- Tracker.invalidateRegister(Def, *TRI, *TII, UseCopyInstr);
- Tracker.trackCopy(&MI, *TRI, *TII, UseCopyInstr);
- continue;
+ // Unlike forward cp, we don't invoke propagateDefs here,
+ // just let forward cp do COPY-to-COPY propagation.
+ if (isBackwardPropagatableCopy(MI, *MRI, *TII, UseCopyInstr)) {
+ Tracker.invalidateRegister(Src, *TRI, *TII, UseCopyInstr);
+ Tracker.invalidateRegister(Def, *TRI, *TII, UseCopyInstr);
+ Tracker.trackCopy(&MI, *TRI, *TII, UseCopyInstr);
+ continue;
+ }
}
}
diff --git a/llvm/test/CodeGen/RISCV/machine-cp.mir b/llvm/test/CodeGen/RISCV/machine-cp.mir
index 5e4006415655..f3674f89cd91 100644
--- a/llvm/test/CodeGen/RISCV/machine-cp.mir
+++ b/llvm/test/CodeGen/RISCV/machine-cp.mir
@@ -18,14 +18,14 @@ body: |
; RV32-LABEL: name: foo
; RV32: liveins: $v28_v29_v30, $v8_v9, $v1
; RV32-NEXT: {{ $}}
- ; RV32-NEXT: renamable $v4_v5_v6_v7_v8_v9_v10_v11 = COPY renamable $v0_v1_v2_v3_v4_v5_v6_v7
- ; RV32-NEXT: renamable $v28 = COPY $v1, implicit killed $v28_v29_v30, implicit-def $v28_v29_v30
+ ; RV32-NEXT: renamable $v4_v5_v6_v7_v8_v9_v10_v11 = COPY killed renamable $v0_v1_v2_v3_v4_v5_v6_v7
+ ; RV32-NEXT: renamable $v28 = COPY renamable $v8, implicit killed $v28_v29_v30, implicit-def $v28_v29_v30
; RV32-NEXT: PseudoRET implicit $v28
; RV64-LABEL: name: foo
; RV64: liveins: $v28_v29_v30, $v8_v9, $v1
; RV64-NEXT: {{ $}}
- ; RV64-NEXT: renamable $v4_v5_v6_v7_v8_v9_v10_v11 = COPY renamable $v0_v1_v2_v3_v4_v5_v6_v7
- ; RV64-NEXT: renamable $v28 = COPY $v1, implicit killed $v28_v29_v30, implicit-def $v28_v29_v30
+ ; RV64-NEXT: renamable $v4_v5_v6_v7_v8_v9_v10_v11 = COPY killed renamable $v0_v1_v2_v3_v4_v5_v6_v7
+ ; RV64-NEXT: renamable $v28 = COPY renamable $v8, implicit killed $v28_v29_v30, implicit-def $v28_v29_v30
; RV64-NEXT: PseudoRET implicit $v28
renamable $v8 = COPY renamable $v1, implicit killed $v8_v9, implicit-def $v8_v9
renamable $v4_v5_v6_v7_v8_v9_v10_v11 = COPY killed renamable $v0_v1_v2_v3_v4_v5_v6_v7
More information about the llvm-commits
mailing list