Skip to content

Commit e723e15

Browse files
authored
[MCP] Handle iterative simplification during forward copy prop (#140267)
This is the follow up I mentioned doing in the review of 52b345d. That change introduced an API for performing instruction simplifications following copy propagation (e.g. things like recognizing ORI a0, a1, zero is just a move). As noted in that review, we should be able to perform iterative simplification as we move forward through the block, but weren't because of the code structure. The majority of this code is just deleting the special casing for constant source and destination tracking, and merging the copy handling with the main path. By assumption, the properties of copies (in terms of register reads and writes), must be a subset of general instructions. Once we do that, the iterative bit basically falls out from having the tracking performed for copies which are recognized *after* we forward prior uses.
1 parent e383753 commit e723e15

File tree

6 files changed

+61
-104
lines changed

6 files changed

+61
-104
lines changed

llvm/lib/CodeGen/MachineCopyPropagation.cpp

Lines changed: 35 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -872,12 +872,6 @@ void MachineCopyPropagation::forwardUses(MachineInstr &MI) {
872872
++NumCopyForwards;
873873
Changed = true;
874874
}
875-
// Attempt to canonicalize/optimize the instruction now its arguments have
876-
// been mutated.
877-
if (TII->simplifyInstruction(MI)) {
878-
Changed = true;
879-
LLVM_DEBUG(dbgs() << "MCP: After optimizeInstruction: " << MI);
880-
}
881875
}
882876

883877
void MachineCopyPropagation::ForwardCopyPropagateBlock(MachineBasicBlock &MBB) {
@@ -889,10 +883,8 @@ void MachineCopyPropagation::ForwardCopyPropagateBlock(MachineBasicBlock &MBB) {
889883
std::optional<DestSourcePair> CopyOperands =
890884
isCopyInstr(MI, *TII, UseCopyInstr);
891885
if (CopyOperands) {
892-
893886
Register RegSrc = CopyOperands->Source->getReg();
894887
Register RegDef = CopyOperands->Destination->getReg();
895-
896888
if (!TRI->regsOverlap(RegDef, RegSrc)) {
897889
assert(RegDef.isPhysical() && RegSrc.isPhysical() &&
898890
"MachineCopyPropagation should be run after register allocation!");
@@ -917,51 +909,6 @@ void MachineCopyPropagation::ForwardCopyPropagateBlock(MachineBasicBlock &MBB) {
917909
// %ecx = COPY %eax
918910
if (eraseIfRedundant(MI, Def, Src) || eraseIfRedundant(MI, Src, Def))
919911
continue;
920-
921-
forwardUses(MI);
922-
923-
// Src may have been changed by forwardUses()
924-
CopyOperands = isCopyInstr(MI, *TII, UseCopyInstr);
925-
Src = CopyOperands->Source->getReg().asMCReg();
926-
927-
// If Src is defined by a previous copy, the previous copy cannot be
928-
// eliminated.
929-
ReadRegister(Src, MI, RegularUse);
930-
for (const MachineOperand &MO : MI.implicit_operands()) {
931-
if (!MO.isReg() || !MO.readsReg())
932-
continue;
933-
MCRegister Reg = MO.getReg().asMCReg();
934-
if (!Reg)
935-
continue;
936-
ReadRegister(Reg, MI, RegularUse);
937-
}
938-
939-
LLVM_DEBUG(dbgs() << "MCP: Copy is a deletion candidate: "; MI.dump());
940-
941-
// Copy is now a candidate for deletion.
942-
if (!MRI->isReserved(Def))
943-
MaybeDeadCopies.insert(&MI);
944-
945-
// If 'Def' is previously source of another copy, then this earlier copy's
946-
// source is no longer available. e.g.
947-
// %xmm9 = copy %xmm2
948-
// ...
949-
// %xmm2 = copy %xmm0
950-
// ...
951-
// %xmm2 = copy %xmm9
952-
Tracker.clobberRegister(Def, *TRI, *TII, UseCopyInstr);
953-
for (const MachineOperand &MO : MI.implicit_operands()) {
954-
if (!MO.isReg() || !MO.isDef())
955-
continue;
956-
MCRegister Reg = MO.getReg().asMCReg();
957-
if (!Reg)
958-
continue;
959-
Tracker.clobberRegister(Reg, *TRI, *TII, UseCopyInstr);
960-
}
961-
962-
Tracker.trackCopy(&MI, *TRI, *TII, UseCopyInstr);
963-
964-
continue;
965912
}
966913
}
967914

@@ -979,20 +926,36 @@ void MachineCopyPropagation::ForwardCopyPropagateBlock(MachineBasicBlock &MBB) {
979926

980927
forwardUses(MI);
981928

982-
// It's possible that the previous transformation has resulted in a no-op
983-
// register move (i.e. one where source and destination registers are the
984-
// same and are not referring to a reserved register). If so, delete it.
985-
CopyOperands = isCopyInstr(MI, *TII, UseCopyInstr);
986-
if (CopyOperands &&
987-
CopyOperands->Source->getReg() == CopyOperands->Destination->getReg() &&
988-
!MRI->isReserved(CopyOperands->Source->getReg())) {
989-
MI.eraseFromParent();
990-
NumDeletes++;
929+
// Attempt to canonicalize/optimize the instruction now its arguments have
930+
// been mutated. This may convert MI from a non-copy to a copy instruction.
931+
if (TII->simplifyInstruction(MI)) {
991932
Changed = true;
992-
continue;
933+
LLVM_DEBUG(dbgs() << "MCP: After simplifyInstruction: " << MI);
934+
}
935+
936+
CopyOperands = isCopyInstr(MI, *TII, UseCopyInstr);
937+
if (CopyOperands) {
938+
Register RegSrc = CopyOperands->Source->getReg();
939+
Register RegDef = CopyOperands->Destination->getReg();
940+
// It's possible that the previous transformations have resulted in a
941+
// no-op register move (i.e. one where source and destination registers
942+
// are the same and are not referring to a reserved register). If so,
943+
// delete it.
944+
if (RegSrc == RegDef && !MRI->isReserved(RegSrc)) {
945+
MI.eraseFromParent();
946+
NumDeletes++;
947+
Changed = true;
948+
continue;
949+
}
950+
951+
if (!TRI->regsOverlap(RegDef, RegSrc)) {
952+
// Copy is now a candidate for deletion.
953+
MCRegister Def = RegDef.asMCReg();
954+
if (!MRI->isReserved(Def))
955+
MaybeDeadCopies.insert(&MI);
956+
}
993957
}
994958

995-
// Not a copy.
996959
SmallVector<Register, 4> Defs;
997960
const MachineOperand *RegMask = nullptr;
998961
for (const MachineOperand &MO : MI.operands()) {
@@ -1072,6 +1035,14 @@ void MachineCopyPropagation::ForwardCopyPropagateBlock(MachineBasicBlock &MBB) {
10721035
// Any previous copy definition or reading the Defs is no longer available.
10731036
for (MCRegister Reg : Defs)
10741037
Tracker.clobberRegister(Reg, *TRI, *TII, UseCopyInstr);
1038+
1039+
if (CopyOperands) {
1040+
Register RegSrc = CopyOperands->Source->getReg();
1041+
Register RegDef = CopyOperands->Destination->getReg();
1042+
if (!TRI->regsOverlap(RegDef, RegSrc)) {
1043+
Tracker.trackCopy(&MI, *TRI, *TII, UseCopyInstr);
1044+
}
1045+
}
10751046
}
10761047

10771048
bool TracksLiveness = MRI->tracksLiveness();

llvm/test/CodeGen/RISCV/GlobalISel/constbarrier-rv32.ll

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -27,24 +27,14 @@ define void @constant_fold_barrier_i128(ptr %p) {
2727
; RV32-NEXT: lw a5, 12(a0)
2828
; RV32-NEXT: slli a1, a1, 11
2929
; RV32-NEXT: and a2, a2, a1
30-
; RV32-NEXT: and a3, a3, zero
31-
; RV32-NEXT: and a4, a4, zero
32-
; RV32-NEXT: and a5, a5, zero
3330
; RV32-NEXT: add a2, a2, a1
34-
; RV32-NEXT: add a6, a3, zero
3531
; RV32-NEXT: sltu a1, a2, a1
36-
; RV32-NEXT: sltu a3, a3, a3
37-
; RV32-NEXT: add a6, a6, a1
38-
; RV32-NEXT: seqz a7, a6
32+
; RV32-NEXT: mv a6, a1
33+
; RV32-NEXT: seqz a7, a1
3934
; RV32-NEXT: and a1, a7, a1
40-
; RV32-NEXT: add a7, a4, zero
41-
; RV32-NEXT: sltu a4, a4, a4
42-
; RV32-NEXT: or a1, a3, a1
43-
; RV32-NEXT: add a7, a7, a1
44-
; RV32-NEXT: seqz a3, a7
35+
; RV32-NEXT: mv a7, a1
36+
; RV32-NEXT: seqz a3, a1
4537
; RV32-NEXT: and a1, a3, a1
46-
; RV32-NEXT: or a1, a4, a1
47-
; RV32-NEXT: add a1, a5, a1
4838
; RV32-NEXT: sw a2, 0(a0)
4939
; RV32-NEXT: sw a6, 4(a0)
5040
; RV32-NEXT: sw a7, 8(a0)

llvm/test/CodeGen/RISCV/GlobalISel/constbarrier-rv64.ll

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,11 @@ define i128 @constant_fold_barrier_i128(i128 %x) {
2121
; RV64-LABEL: constant_fold_barrier_i128:
2222
; RV64: # %bb.0: # %entry
2323
; RV64-NEXT: li a2, 1
24-
; RV64-NEXT: and a1, a1, zero
2524
; RV64-NEXT: slli a2, a2, 11
2625
; RV64-NEXT: and a0, a0, a2
2726
; RV64-NEXT: add a0, a0, a2
2827
; RV64-NEXT: sltu a2, a0, a2
29-
; RV64-NEXT: add a1, a1, a2
28+
; RV64-NEXT: mv a1, a2
3029
; RV64-NEXT: ret
3130
entry:
3231
%and = and i128 %x, 2048

llvm/test/CodeGen/RISCV/GlobalISel/div-by-constant.ll

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -92,34 +92,26 @@ define i64 @udiv64_constant_no_add(i64 %a) nounwind {
9292
; RV32-LABEL: udiv64_constant_no_add:
9393
; RV32: # %bb.0:
9494
; RV32-NEXT: lui a2, 838861
95-
; RV32-NEXT: mulhu a3, a0, zero
9695
; RV32-NEXT: addi a4, a2, -819
9796
; RV32-NEXT: addi a2, a2, -820
9897
; RV32-NEXT: mul a5, a1, a4
9998
; RV32-NEXT: mul a6, a0, a2
10099
; RV32-NEXT: mulhu a7, a0, a4
101-
; RV32-NEXT: mul t0, zero, a4
102100
; RV32-NEXT: mul t1, a1, a2
103101
; RV32-NEXT: mulhu t2, a1, a4
104102
; RV32-NEXT: mulhu a0, a0, a2
105103
; RV32-NEXT: mulhu a1, a1, a2
106-
; RV32-NEXT: mul a2, zero, a2
107-
; RV32-NEXT: mulhu a4, zero, a4
108104
; RV32-NEXT: add a5, a5, a6
109-
; RV32-NEXT: add a2, t0, a2
110-
; RV32-NEXT: add t0, t0, t1
111-
; RV32-NEXT: add a1, a4, a1
105+
; RV32-NEXT: mv t0, t1
112106
; RV32-NEXT: sltu a4, a5, a6
113107
; RV32-NEXT: add a5, a5, a7
114-
; RV32-NEXT: sltu a6, t0, t1
115-
; RV32-NEXT: sltiu t1, t0, 0
108+
; RV32-NEXT: sltu a6, t1, t1
109+
; RV32-NEXT: sltiu t1, t1, 0
116110
; RV32-NEXT: add t0, t0, t2
117-
; RV32-NEXT: add a1, a2, a1
118111
; RV32-NEXT: sltu a2, a5, a7
119112
; RV32-NEXT: add a6, a6, t1
120113
; RV32-NEXT: sltu a5, t0, t2
121114
; RV32-NEXT: add t0, t0, a0
122-
; RV32-NEXT: add a1, a1, a3
123115
; RV32-NEXT: add a2, a4, a2
124116
; RV32-NEXT: add a5, a6, a5
125117
; RV32-NEXT: sltu a0, t0, a0
@@ -156,34 +148,27 @@ define i64 @udiv64_constant_add(i64 %a) nounwind {
156148
; RV32: # %bb.0:
157149
; RV32-NEXT: lui a2, 599186
158150
; RV32-NEXT: lui a3, 149797
159-
; RV32-NEXT: mulhu a4, a0, zero
160151
; RV32-NEXT: addi a2, a2, 1171
161152
; RV32-NEXT: addi a3, a3, -1756
162153
; RV32-NEXT: mul a5, a1, a2
163154
; RV32-NEXT: mul a6, a0, a3
164155
; RV32-NEXT: mulhu a7, a0, a2
165-
; RV32-NEXT: mul t0, zero, a2
166-
; RV32-NEXT: mulhu t1, zero, a2
167156
; RV32-NEXT: mulhu t2, a1, a3
168-
; RV32-NEXT: add t1, t1, t2
169-
; RV32-NEXT: mul t2, zero, a3
170-
; RV32-NEXT: add t2, t0, t2
171-
; RV32-NEXT: add t1, t2, t1
157+
; RV32-NEXT: mv t1, t2
172158
; RV32-NEXT: mul t2, a1, a3
173159
; RV32-NEXT: mulhu a2, a1, a2
174160
; RV32-NEXT: mulhu a3, a0, a3
175161
; RV32-NEXT: add a5, a5, a6
176-
; RV32-NEXT: add t0, t0, t2
162+
; RV32-NEXT: mv t0, t2
177163
; RV32-NEXT: sltu a6, a5, a6
178164
; RV32-NEXT: add a5, a5, a7
179-
; RV32-NEXT: sltu t2, t0, t2
165+
; RV32-NEXT: sltu t2, t2, t2
180166
; RV32-NEXT: sltu a5, a5, a7
181167
; RV32-NEXT: sltiu a7, t0, 0
182168
; RV32-NEXT: add t0, t0, a2
183169
; RV32-NEXT: add a7, t2, a7
184170
; RV32-NEXT: sltu a2, t0, a2
185171
; RV32-NEXT: add t0, t0, a3
186-
; RV32-NEXT: add a4, t1, a4
187172
; RV32-NEXT: add a5, a6, a5
188173
; RV32-NEXT: add a2, a7, a2
189174
; RV32-NEXT: sltu a3, t0, a3
@@ -195,7 +180,7 @@ define i64 @udiv64_constant_add(i64 %a) nounwind {
195180
; RV32-NEXT: add a2, a2, a3
196181
; RV32-NEXT: sub a1, a1, a0
197182
; RV32-NEXT: srli a5, a5, 1
198-
; RV32-NEXT: add a2, a4, a2
183+
; RV32-NEXT: add a2, t1, a2
199184
; RV32-NEXT: sub a1, a1, a2
200185
; RV32-NEXT: slli a0, a1, 31
201186
; RV32-NEXT: srli a1, a1, 1

llvm/test/CodeGen/RISCV/GlobalISel/rv32zbb.ll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,7 @@ define i1 @ctpop_i64_ugt_two(i64 %a) nounwind {
385385
; RV32ZBB: # %bb.0:
386386
; RV32ZBB-NEXT: j .LBB6_2
387387
; RV32ZBB-NEXT: # %bb.1:
388-
; RV32ZBB-NEXT: sltiu a0, zero, 0
388+
; RV32ZBB-NEXT: li a0, 0
389389
; RV32ZBB-NEXT: ret
390390
; RV32ZBB-NEXT: .LBB6_2:
391391
; RV32ZBB-NEXT: cpop a0, a0
@@ -415,7 +415,7 @@ define i1 @ctpop_i64_ugt_one(i64 %a) nounwind {
415415
; RV32ZBB: # %bb.0:
416416
; RV32ZBB-NEXT: j .LBB7_2
417417
; RV32ZBB-NEXT: # %bb.1:
418-
; RV32ZBB-NEXT: snez a0, zero
418+
; RV32ZBB-NEXT: li a0, 0
419419
; RV32ZBB-NEXT: ret
420420
; RV32ZBB-NEXT: .LBB7_2:
421421
; RV32ZBB-NEXT: cpop a0, a0

llvm/test/CodeGen/RISCV/machine-copyprop-simplifyinstruction.mir

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -743,6 +743,18 @@ body: |
743743
PseudoRET implicit $x10
744744
...
745745
---
746+
name: multipass
747+
body: |
748+
bb.0:
749+
; CHECK-LABEL: name: multipass
750+
; CHECK: renamable $x9 = ADDI $x0, 0
751+
; CHECK-NEXT: PseudoRET implicit $x9
752+
renamable $x11 = COPY $x0
753+
renamable $x10 = SLLI renamable $x11, 13
754+
renamable $x9 = SRLI renamable $x10, 13
755+
PseudoRET implicit $x9
756+
...
757+
---
746758
name: beq
747759
body: |
748760
; CHECK-LABEL: name: beq

0 commit comments

Comments
 (0)