Skip to content

Commit 343e204

Browse files
john-brawn-armjohn-brawn-arm
committed
[ARM] Replace TransferImpOps with copyImplicitOps
In most places where TransferImpOps is currently used we just have one machine instruction, so it's doing the same thing as copyImplicitOps anyway. In those cases where we have more than one machine instruction the destination is written to in each instruction so any implicit defs should appear on all of them (and we shouldn't see any implicit refs as these pseudo-instruction don't have any register inputs), meaning the current use of TransferImpOps is incorrect and we should be using copyImplicitOps on all of the generated instructions. Differential Revision: https://reviews.llvm.org/D155301
1 parent 46aec7b commit 343e204

File tree

3 files changed

+146
-77
lines changed

3 files changed

+146
-77
lines changed

‎llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp

Lines changed: 45 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,6 @@ namespace {
6060
}
6161

6262
private:
63-
void TransferImpOps(MachineInstr &OldMI,
64-
MachineInstrBuilder &UseMI, MachineInstrBuilder &DefMI);
6563
bool ExpandMI(MachineBasicBlock &MBB,
6664
MachineBasicBlock::iterator MBBI,
6765
MachineBasicBlock::iterator &NextMBBI);
@@ -123,22 +121,6 @@ namespace {
123121
INITIALIZE_PASS(ARMExpandPseudo, DEBUG_TYPE, ARM_EXPAND_PSEUDO_NAME, false,
124122
false)
125123

126-
/// TransferImpOps - Transfer implicit operands on the pseudo instruction to
127-
/// the instructions created from the expansion.
128-
void ARMExpandPseudo::TransferImpOps(MachineInstr &OldMI,
129-
MachineInstrBuilder &UseMI,
130-
MachineInstrBuilder &DefMI) {
131-
const MCInstrDesc &Desc = OldMI.getDesc();
132-
for (const MachineOperand &MO :
133-
llvm::drop_begin(OldMI.operands(), Desc.getNumOperands())) {
134-
assert(MO.isReg() && MO.getReg());
135-
if (MO.isUse())
136-
UseMI.add(MO);
137-
else
138-
DefMI.add(MO);
139-
}
140-
}
141-
142124
namespace {
143125
// Constants for register spacing in NEON load/store instructions.
144126
// For quad-register load-lane and store-lane pseudo instructors, the
@@ -678,7 +660,7 @@ void ARMExpandPseudo::ExpandVLD(MachineBasicBlock::iterator &MBBI) {
678660
}
679661
// Add an implicit def for the super-register.
680662
MIB.addReg(DstReg, RegState::ImplicitDefine | getDeadRegState(DstIsDead));
681-
TransferImpOps(MI, MIB, MIB);
663+
MIB.copyImplicitOps(MI);
682664

683665
// Transfer memoperands.
684666
MIB.cloneMemRefs(MI);
@@ -754,7 +736,7 @@ void ARMExpandPseudo::ExpandVST(MachineBasicBlock::iterator &MBBI) {
754736
MIB->addRegisterKilled(SrcReg, TRI, true);
755737
else if (!SrcIsUndef)
756738
MIB.addReg(SrcReg, RegState::Implicit); // Add implicit uses for src reg.
757-
TransferImpOps(MI, MIB, MIB);
739+
MIB.copyImplicitOps(MI);
758740

759741
// Transfer memoperands.
760742
MIB.cloneMemRefs(MI);
@@ -846,7 +828,7 @@ void ARMExpandPseudo::ExpandLaneOp(MachineBasicBlock::iterator &MBBI) {
846828
if (TableEntry->IsLoad)
847829
// Add an implicit def for the super-register.
848830
MIB.addReg(DstReg, RegState::ImplicitDefine | getDeadRegState(DstIsDead));
849-
TransferImpOps(MI, MIB, MIB);
831+
MIB.copyImplicitOps(MI);
850832
// Transfer memoperands.
851833
MIB.cloneMemRefs(MI);
852834
MI.eraseFromParent();
@@ -886,7 +868,7 @@ void ARMExpandPseudo::ExpandVTBL(MachineBasicBlock::iterator &MBBI,
886868

887869
// Add an implicit kill and use for the super-reg.
888870
MIB.addReg(SrcReg, RegState::Implicit | getKillRegState(SrcIsKill));
889-
TransferImpOps(MI, MIB, MIB);
871+
MIB.copyImplicitOps(MI);
890872
MI.eraseFromParent();
891873
LLVM_DEBUG(dbgs() << "To: "; MIB.getInstr()->dump(););
892874
}
@@ -923,7 +905,7 @@ void ARMExpandPseudo::ExpandMQQPRLoadStore(MachineBasicBlock::iterator &MBBI) {
923905
if (NewOpc == ARM::VSTMDIA)
924906
MIB.addReg(SrcReg, RegState::Implicit);
925907

926-
TransferImpOps(MI, MIB, MIB);
908+
MIB.copyImplicitOps(MI);
927909
MIB.cloneMemRefs(MI);
928910
MI.eraseFromParent();
929911
}
@@ -1132,7 +1114,8 @@ void ARMExpandPseudo::ExpandMOV32BitImm(MachineBasicBlock &MBB,
11321114
HI16.addImm(Pred).addReg(PredReg).add(condCodeOp());
11331115
if (isCC)
11341116
LO16.add(makeImplicit(MI.getOperand(1)));
1135-
TransferImpOps(MI, LO16, HI16);
1117+
LO16.copyImplicitOps(MI);
1118+
HI16.copyImplicitOps(MI);
11361119
MI.eraseFromParent();
11371120
return;
11381121
}
@@ -1191,7 +1174,8 @@ void ARMExpandPseudo::ExpandMOV32BitImm(MachineBasicBlock &MBB,
11911174

11921175
if (isCC)
11931176
LO16.add(makeImplicit(MI.getOperand(1)));
1194-
TransferImpOps(MI, LO16, HI16);
1177+
LO16.copyImplicitOps(MI);
1178+
HI16.copyImplicitOps(MI);
11951179
MI.eraseFromParent();
11961180
LLVM_DEBUG(dbgs() << "To: "; LO16.getInstr()->dump(););
11971181
LLVM_DEBUG(dbgs() << "And: "; HI16.getInstr()->dump(););
@@ -2584,14 +2568,13 @@ bool ARMExpandPseudo::ExpandMI(MachineBasicBlock &MBB,
25842568
}
25852569
case ARM::RRX: {
25862570
// This encodes as "MOVs Rd, Rm, rrx
2587-
MachineInstrBuilder MIB =
2588-
BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(ARM::MOVsi),
2589-
MI.getOperand(0).getReg())
2590-
.add(MI.getOperand(1))
2591-
.addImm(ARM_AM::getSORegOpc(ARM_AM::rrx, 0))
2592-
.add(predOps(ARMCC::AL))
2593-
.add(condCodeOp());
2594-
TransferImpOps(MI, MIB, MIB);
2571+
BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(ARM::MOVsi),
2572+
MI.getOperand(0).getReg())
2573+
.add(MI.getOperand(1))
2574+
.addImm(ARM_AM::getSORegOpc(ARM_AM::rrx, 0))
2575+
.add(predOps(ARMCC::AL))
2576+
.add(condCodeOp())
2577+
.copyImplicitOps(MI);
25952578
MI.eraseFromParent();
25962579
return true;
25972580
}
@@ -2631,7 +2614,7 @@ bool ARMExpandPseudo::ExpandMI(MachineBasicBlock &MBB,
26312614
}
26322615

26332616
MIB.cloneMemRefs(MI);
2634-
TransferImpOps(MI, MIB, MIB);
2617+
MIB.copyImplicitOps(MI);
26352618
// Update the call site info.
26362619
if (MI.isCandidateForCallSiteEntry())
26372620
MF->moveCallSiteInfo(&MI, &*MIB);
@@ -2644,17 +2627,16 @@ bool ARMExpandPseudo::ExpandMI(MachineBasicBlock &MBB,
26442627
? ARM::tLDRpci : ARM::t2LDRpci;
26452628
Register DstReg = MI.getOperand(0).getReg();
26462629
bool DstIsDead = MI.getOperand(0).isDead();
2647-
MachineInstrBuilder MIB1 =
2648-
BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(NewLdOpc), DstReg)
2649-
.add(MI.getOperand(1))
2650-
.add(predOps(ARMCC::AL));
2651-
MIB1.cloneMemRefs(MI);
2652-
MachineInstrBuilder MIB2 =
2653-
BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(ARM::tPICADD))
2654-
.addReg(DstReg, RegState::Define | getDeadRegState(DstIsDead))
2655-
.addReg(DstReg)
2656-
.add(MI.getOperand(2));
2657-
TransferImpOps(MI, MIB1, MIB2);
2630+
BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(NewLdOpc), DstReg)
2631+
.add(MI.getOperand(1))
2632+
.add(predOps(ARMCC::AL))
2633+
.cloneMemRefs(MI)
2634+
.copyImplicitOps(MI);
2635+
BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(ARM::tPICADD))
2636+
.addReg(DstReg, RegState::Define | getDeadRegState(DstIsDead))
2637+
.addReg(DstReg)
2638+
.add(MI.getOperand(2))
2639+
.copyImplicitOps(MI);
26582640
MI.eraseFromParent();
26592641
return true;
26602642
}
@@ -2739,15 +2721,16 @@ bool ARMExpandPseudo::ExpandMI(MachineBasicBlock &MBB,
27392721
unsigned PICAddOpc = isARM
27402722
? (Opcode == ARM::MOV_ga_pcrel_ldr ? ARM::PICLDR : ARM::PICADD)
27412723
: ARM::tPICADD;
2742-
MachineInstrBuilder MIB1 = BuildMI(MBB, MBBI, MI.getDebugLoc(),
2743-
TII->get(LO16Opc), DstReg)
2744-
.addGlobalAddress(GV, MO1.getOffset(), TF | LO16TF)
2745-
.addImm(LabelId);
2724+
BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(LO16Opc), DstReg)
2725+
.addGlobalAddress(GV, MO1.getOffset(), TF | LO16TF)
2726+
.addImm(LabelId)
2727+
.copyImplicitOps(MI);
27462728

27472729
BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(HI16Opc), DstReg)
2748-
.addReg(DstReg)
2749-
.addGlobalAddress(GV, MO1.getOffset(), TF | HI16TF)
2750-
.addImm(LabelId);
2730+
.addReg(DstReg)
2731+
.addGlobalAddress(GV, MO1.getOffset(), TF | HI16TF)
2732+
.addImm(LabelId)
2733+
.copyImplicitOps(MI);
27512734

27522735
MachineInstrBuilder MIB3 = BuildMI(MBB, MBBI, MI.getDebugLoc(),
27532736
TII->get(PICAddOpc))
@@ -2758,7 +2741,7 @@ bool ARMExpandPseudo::ExpandMI(MachineBasicBlock &MBB,
27582741
if (Opcode == ARM::MOV_ga_pcrel_ldr)
27592742
MIB3.cloneMemRefs(MI);
27602743
}
2761-
TransferImpOps(MI, MIB1, MIB3);
2744+
MIB3.copyImplicitOps(MI);
27622745
MI.eraseFromParent();
27632746
return true;
27642747
}
@@ -2786,14 +2769,13 @@ bool ARMExpandPseudo::ExpandMI(MachineBasicBlock &MBB,
27862769
return true;
27872770

27882771
case ARM::SUBS_PC_LR: {
2789-
MachineInstrBuilder MIB =
2790-
BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(ARM::SUBri), ARM::PC)
2791-
.addReg(ARM::LR)
2792-
.add(MI.getOperand(0))
2793-
.add(MI.getOperand(1))
2794-
.add(MI.getOperand(2))
2795-
.addReg(ARM::CPSR, RegState::Undef);
2796-
TransferImpOps(MI, MIB, MIB);
2772+
BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(ARM::SUBri), ARM::PC)
2773+
.addReg(ARM::LR)
2774+
.add(MI.getOperand(0))
2775+
.add(MI.getOperand(1))
2776+
.add(MI.getOperand(2))
2777+
.addReg(ARM::CPSR, RegState::Undef)
2778+
.copyImplicitOps(MI);
27972779
MI.eraseFromParent();
27982780
return true;
27992781
}
@@ -2822,7 +2804,7 @@ bool ARMExpandPseudo::ExpandMI(MachineBasicBlock &MBB,
28222804

28232805
// Add an implicit def for the super-register.
28242806
MIB.addReg(DstReg, RegState::ImplicitDefine | getDeadRegState(DstIsDead));
2825-
TransferImpOps(MI, MIB, MIB);
2807+
MIB.copyImplicitOps(MI);
28262808
MIB.cloneMemRefs(MI);
28272809
MI.eraseFromParent();
28282810
return true;
@@ -2855,7 +2837,7 @@ bool ARMExpandPseudo::ExpandMI(MachineBasicBlock &MBB,
28552837
if (SrcIsKill) // Add an implicit kill for the Q register.
28562838
MIB->addRegisterKilled(SrcReg, TRI, true);
28572839

2858-
TransferImpOps(MI, MIB, MIB);
2840+
MIB.copyImplicitOps(MI);
28592841
MIB.cloneMemRefs(MI);
28602842
MI.eraseFromParent();
28612843
return true;

‎llvm/test/CodeGen/ARM/expand-pseudos.mir

Lines changed: 69 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,15 @@
1515
entry:
1616
unreachable
1717
}
18+
define i32 @test4(i32 %x) {
19+
entry:
20+
unreachable
21+
}
22+
@var = global i32 0
23+
define i32 @test5(i32 %x) {
24+
entry:
25+
unreachable
26+
}
1827
...
1928
---
2029
name: test1
@@ -28,11 +37,12 @@ body: |
2837
2938
; CHECK-LABEL: name: test1
3039
; CHECK: liveins: $r0
31-
; CHECK: $r1 = MOVi 2, 14 /* CC::al */, $noreg, $noreg
32-
; CHECK: CMPri killed $r0, 0, 14 /* CC::al */, $noreg, implicit-def $cpsr
33-
; CHECK: $r1 = MOVi16 500, 0 /* CC::eq */, killed $cpsr, implicit killed $r1
34-
; CHECK: $r0 = MOVr killed $r1, 14 /* CC::al */, $noreg, $noreg
35-
; CHECK: BX_RET 14 /* CC::al */, $noreg, implicit $r0
40+
; CHECK-NEXT: {{ $}}
41+
; CHECK-NEXT: $r1 = MOVi 2, 14 /* CC::al */, $noreg, $noreg
42+
; CHECK-NEXT: CMPri killed $r0, 0, 14 /* CC::al */, $noreg, implicit-def $cpsr
43+
; CHECK-NEXT: $r1 = MOVi16 500, 0 /* CC::eq */, killed $cpsr, implicit killed $r1
44+
; CHECK-NEXT: $r0 = MOVr killed $r1, 14 /* CC::al */, $noreg, $noreg
45+
; CHECK-NEXT: BX_RET 14 /* CC::al */, $noreg, implicit $r0
3646
$r1 = MOVi 2, 14, $noreg, $noreg
3747
CMPri killed $r0, 0, 14, $noreg, implicit-def $cpsr
3848
$r1 = MOVCCi16 killed $r1, 500, 0, killed $cpsr
@@ -52,12 +62,13 @@ body: |
5262
5363
; CHECK-LABEL: name: test2
5464
; CHECK: liveins: $r0
55-
; CHECK: $r1 = MOVi 2, 14 /* CC::al */, $noreg, $noreg
56-
; CHECK: CMPri killed $r0, 0, 14 /* CC::al */, $noreg, implicit-def $cpsr
57-
; CHECK: $r1 = MOVi16 2068, 0 /* CC::eq */, $cpsr, implicit killed $r1
58-
; CHECK: $r1 = MOVTi16 $r1, 7637, 0 /* CC::eq */, $cpsr
59-
; CHECK: $r0 = MOVr killed $r1, 14 /* CC::al */, $noreg, $noreg
60-
; CHECK: BX_RET 14 /* CC::al */, $noreg, implicit $r0
65+
; CHECK-NEXT: {{ $}}
66+
; CHECK-NEXT: $r1 = MOVi 2, 14 /* CC::al */, $noreg, $noreg
67+
; CHECK-NEXT: CMPri killed $r0, 0, 14 /* CC::al */, $noreg, implicit-def $cpsr
68+
; CHECK-NEXT: $r1 = MOVi16 2068, 0 /* CC::eq */, $cpsr, implicit killed $r1
69+
; CHECK-NEXT: $r1 = MOVTi16 $r1, 7637, 0 /* CC::eq */, $cpsr
70+
; CHECK-NEXT: $r0 = MOVr killed $r1, 14 /* CC::al */, $noreg, $noreg
71+
; CHECK-NEXT: BX_RET 14 /* CC::al */, $noreg, implicit $r0
6172
$r1 = MOVi 2, 14, $noreg, $noreg
6273
CMPri killed $r0, 0, 14, $noreg, implicit-def $cpsr
6374
$r1 = MOVCCi32imm killed $r1, 500500500, 0, killed $cpsr
@@ -78,11 +89,55 @@ body: |
7889
7990
; CHECK-LABEL: name: test3
8091
; CHECK: liveins: $r0, $r1
81-
; CHECK: CMPri $r1, 500, 14 /* CC::al */, $noreg, implicit-def $cpsr
82-
; CHECK: $r0 = MOVr killed $r1, 12 /* CC::gt */, killed $cpsr, $noreg, implicit killed $r0
83-
; CHECK: BX_RET 14 /* CC::al */, $noreg, implicit $r0
92+
; CHECK-NEXT: {{ $}}
93+
; CHECK-NEXT: CMPri $r1, 500, 14 /* CC::al */, $noreg, implicit-def $cpsr
94+
; CHECK-NEXT: $r0 = MOVr killed $r1, 12 /* CC::gt */, killed $cpsr, $noreg, implicit killed $r0
95+
; CHECK-NEXT: BX_RET 14 /* CC::al */, $noreg, implicit $r0
8496
CMPri $r1, 500, 14, $noreg, implicit-def $cpsr
8597
$r0 = MOVCCr killed $r0, killed $r1, 12, killed $cpsr
8698
BX_RET 14, $noreg, implicit $r0
8799
88100
...
101+
---
102+
name: test4
103+
alignment: 4
104+
tracksRegLiveness: true
105+
liveins:
106+
- { reg: '$r0', virtual-reg: '' }
107+
- { reg: '$r0_r1', virtual-reg: '' }
108+
body: |
109+
bb.0.entry:
110+
liveins: $r0, $r0_r1
111+
112+
; CHECK-LABEL: name: test4
113+
; CHECK: liveins: $r0, $r0_r1
114+
; CHECK-NEXT: {{ $}}
115+
; CHECK-NEXT: $r0 = MOVi16 51712, 14 /* CC::al */, $noreg, implicit-def $r0_r1
116+
; CHECK-NEXT: $r0 = MOVTi16 $r0, 15258, 14 /* CC::al */, $noreg, implicit-def $r0_r1
117+
; CHECK-NEXT: BX_RET 14 /* CC::al */, $noreg, implicit $r0
118+
$r0 = MOVi32imm 1000000000, implicit-def $r0_r1
119+
BX_RET 14, $noreg, implicit $r0
120+
121+
...
122+
---
123+
name: test5
124+
alignment: 4
125+
tracksRegLiveness: true
126+
liveins:
127+
- { reg: '$r0', virtual-reg: '' }
128+
- { reg: '$r0_r1', virtual-reg: '' }
129+
body: |
130+
bb.0.entry:
131+
liveins: $r0, $r0_r1
132+
133+
; CHECK-LABEL: name: test5
134+
; CHECK: liveins: $r0, $r0_r1
135+
; CHECK-NEXT: {{ $}}
136+
; CHECK-NEXT: $r0 = MOVi16_ga_pcrel target-flags(arm-lo16) @var, 0, implicit-def $r0_r1
137+
; CHECK-NEXT: $r0 = MOVTi16_ga_pcrel $r0, target-flags(arm-hi16) @var, 0, implicit-def $r0_r1
138+
; CHECK-NEXT: $r0 = PICLDR $r0, 0, 14 /* CC::al */, $noreg, implicit-def $r0_r1
139+
; CHECK-NEXT: BX_RET 14 /* CC::al */, $noreg, implicit $r0
140+
$r0 = MOV_ga_pcrel_ldr @var, implicit-def $r0_r1
141+
BX_RET 14, $noreg, implicit $r0
142+
143+
...

‎llvm/test/CodeGen/Thumb2/expand-pseudos.mir

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
2+
# RUN: llc -run-pass=arm-pseudo -verify-machineinstrs %s -o - | FileCheck %s
3+
--- |
4+
target triple = "thumbv7---gnueabi"
5+
6+
@var = global i32 0
7+
define i32 @test1(i32 %x) {
8+
entry:
9+
unreachable
10+
}
11+
...
12+
---
13+
name: test1
14+
alignment: 4
15+
tracksRegLiveness: true
16+
liveins:
17+
- { reg: '$r0', virtual-reg: '' }
18+
- { reg: '$r0_r1', virtual-reg: '' }
19+
body: |
20+
bb.0.entry:
21+
liveins: $r0, $r0_r1
22+
23+
; CHECK-LABEL: name: test1
24+
; CHECK: liveins: $r0, $r0_r1
25+
; CHECK-NEXT: {{ $}}
26+
; CHECK-NEXT: $r0 = t2LDRpci @var, 14 /* CC::al */, $noreg, implicit-def $r0_r1
27+
; CHECK-NEXT: $r0 = tPICADD $r0, 0, implicit-def $r0_r1
28+
; CHECK-NEXT: BX_RET 14 /* CC::al */, $noreg, implicit $r0
29+
$r0 = t2LDRpci_pic @var, 0, implicit-def $r0_r1
30+
BX_RET 14, $noreg, implicit $r0
31+
32+
...

0 commit comments

Comments
 (0)