Skip to content

Commit 31ca698

Browse files
Patryk27nikic
Patryk27
authored and
nikic
committed
[AVR] Merge AVRRelaxMemOperations into AVRExpandPseudoInsts
This commit contains a refactoring that merges AVRRelaxMemOperations into AVRExpandPseudoInsts, so that we have a single place in code that expands the STDWPtrQRr opcode. Seizing the day, I've also fixed a couple of potential bugs with our previous implementation (e.g. when the destination register was killed, the previous implementation would try to .addDef() that killed register, crashing LLVM in the process - that's fixed now, as proved by the test). Reviewed By: benshi001 Differential Revision: https://reviews.llvm.org/D122533
1 parent 0004a38 commit 31ca698

File tree

9 files changed

+86
-205
lines changed

9 files changed

+86
-205
lines changed

‎clang/docs/tools/clang-formatted-files.txt

-1
Original file line numberDiff line numberDiff line change
@@ -5990,7 +5990,6 @@ llvm/lib/Target/AVR/AVRMCInstLower.cpp
59905990
llvm/lib/Target/AVR/AVRMCInstLower.h
59915991
llvm/lib/Target/AVR/AVRRegisterInfo.cpp
59925992
llvm/lib/Target/AVR/AVRRegisterInfo.h
5993-
llvm/lib/Target/AVR/AVRRelaxMemOperations.cpp
59945993
llvm/lib/Target/AVR/AVRSelectionDAGInfo.h
59955994
llvm/lib/Target/AVR/AVRShiftExpand.cpp
59965995
llvm/lib/Target/AVR/AVRSubtarget.cpp

‎llvm/lib/Target/AVR/AVR.h

-2
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,10 @@ FunctionPass *createAVRISelDag(AVRTargetMachine &TM,
2727
CodeGenOpt::Level OptLevel);
2828
FunctionPass *createAVRExpandPseudoPass();
2929
FunctionPass *createAVRFrameAnalyzerPass();
30-
FunctionPass *createAVRRelaxMemPass();
3130
FunctionPass *createAVRBranchSelectionPass();
3231

3332
void initializeAVRShiftExpandPass(PassRegistry &);
3433
void initializeAVRExpandPseudoPass(PassRegistry &);
35-
void initializeAVRRelaxMemPass(PassRegistry &);
3634

3735
/// Contains the AVR backend.
3836
namespace AVR {

‎llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp

+38-19
Original file line numberDiff line numberDiff line change
@@ -1230,32 +1230,51 @@ bool AVRExpandPseudo::expand<AVR::STWPtrPdRr>(Block &MBB, BlockIt MBBI) {
12301230
template <>
12311231
bool AVRExpandPseudo::expand<AVR::STDWPtrQRr>(Block &MBB, BlockIt MBBI) {
12321232
MachineInstr &MI = *MBBI;
1233-
Register SrcLoReg, SrcHiReg;
1233+
12341234
Register DstReg = MI.getOperand(0).getReg();
1235-
Register SrcReg = MI.getOperand(2).getReg();
1236-
unsigned Imm = MI.getOperand(1).getImm();
12371235
bool DstIsKill = MI.getOperand(0).isKill();
1236+
unsigned Imm = MI.getOperand(1).getImm();
1237+
Register SrcReg = MI.getOperand(2).getReg();
12381238
bool SrcIsKill = MI.getOperand(2).isKill();
1239-
unsigned OpLo = AVR::STDPtrQRr;
1240-
unsigned OpHi = AVR::STDPtrQRr;
1241-
TRI->splitReg(SrcReg, SrcLoReg, SrcHiReg);
12421239

1243-
// Since we add 1 to the Imm value for the high byte below, and 63 is the
1244-
// highest Imm value allowed for the instruction, 62 is the limit here.
1245-
assert(Imm <= 62 && "Offset is out of range");
1240+
// STD's maximum displacement is 63, so larger stores have to be split into a
1241+
// set of operations
1242+
if (Imm >= 63) {
1243+
if (!DstIsKill) {
1244+
buildMI(MBB, MBBI, AVR::PUSHWRr).addReg(DstReg);
1245+
}
12461246

1247-
auto MIBLO = buildMI(MBB, MBBI, OpLo)
1248-
.addReg(DstReg)
1249-
.addImm(Imm)
1250-
.addReg(SrcLoReg, getKillRegState(SrcIsKill));
1247+
buildMI(MBB, MBBI, AVR::SUBIWRdK)
1248+
.addReg(DstReg, RegState::Define)
1249+
.addReg(DstReg, RegState::Kill)
1250+
.addImm(-Imm);
12511251

1252-
auto MIBHI = buildMI(MBB, MBBI, OpHi)
1253-
.addReg(DstReg, getKillRegState(DstIsKill))
1254-
.addImm(Imm + 1)
1255-
.addReg(SrcHiReg, getKillRegState(SrcIsKill));
1252+
buildMI(MBB, MBBI, AVR::STWPtrRr)
1253+
.addReg(DstReg, RegState::Kill)
1254+
.addReg(SrcReg, getKillRegState(SrcIsKill));
12561255

1257-
MIBLO.setMemRefs(MI.memoperands());
1258-
MIBHI.setMemRefs(MI.memoperands());
1256+
if (!DstIsKill) {
1257+
buildMI(MBB, MBBI, AVR::POPWRd).addDef(DstReg, RegState::Define);
1258+
}
1259+
} else {
1260+
unsigned OpLo = AVR::STDPtrQRr;
1261+
unsigned OpHi = AVR::STDPtrQRr;
1262+
Register SrcLoReg, SrcHiReg;
1263+
TRI->splitReg(SrcReg, SrcLoReg, SrcHiReg);
1264+
1265+
auto MIBLO = buildMI(MBB, MBBI, OpLo)
1266+
.addReg(DstReg)
1267+
.addImm(Imm)
1268+
.addReg(SrcLoReg, getKillRegState(SrcIsKill));
1269+
1270+
auto MIBHI = buildMI(MBB, MBBI, OpHi)
1271+
.addReg(DstReg, getKillRegState(DstIsKill))
1272+
.addImm(Imm + 1)
1273+
.addReg(SrcHiReg, getKillRegState(SrcIsKill));
1274+
1275+
MIBLO.setMemRefs(MI.memoperands());
1276+
MIBHI.setMemRefs(MI.memoperands());
1277+
}
12591278

12601279
MI.eraseFromParent();
12611280
return true;

‎llvm/lib/Target/AVR/AVRRelaxMemOperations.cpp

-144
This file was deleted.

‎llvm/lib/Target/AVR/AVRTargetMachine.cpp

-2
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,6 @@ extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeAVRTarget() {
9292

9393
auto &PR = *PassRegistry::getPassRegistry();
9494
initializeAVRExpandPseudoPass(PR);
95-
initializeAVRRelaxMemPass(PR);
9695
initializeAVRShiftExpandPass(PR);
9796
}
9897

@@ -118,7 +117,6 @@ bool AVRPassConfig::addInstSelector() {
118117
}
119118

120119
void AVRPassConfig::addPreSched2() {
121-
addPass(createAVRRelaxMemPass());
122120
addPass(createAVRExpandPseudoPass());
123121
}
124122

‎llvm/lib/Target/AVR/CMakeLists.txt

-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ add_llvm_target(AVRCodeGen
2222
AVRISelDAGToDAG.cpp
2323
AVRISelLowering.cpp
2424
AVRMCInstLower.cpp
25-
AVRRelaxMemOperations.cpp
2625
AVRRegisterInfo.cpp
2726
AVRShiftExpand.cpp
2827
AVRSubtarget.cpp

‎llvm/test/CodeGen/AVR/pseudo/STDWPtrQRr.mir

+48-4
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# RUN: llc -O0 -run-pass=avr-expand-pseudo %s -o - | FileCheck %s
1+
# RUN: llc -O0 -run-pass=avr-expand-pseudo -verify-machineinstrs %s -o - | FileCheck %s
22

33
--- |
44
target triple = "avr--"
@@ -15,8 +15,52 @@ body: |
1515
1616
; CHECK-LABEL: test
1717
18-
; CHECK: STDPtrQRr $r29r28, 10, $r0
19-
; CHECK-NEXT: STDPtrQRr $r29r28, 11, $r1
18+
; Small displacement (<63):
19+
; CHECK: STDPtrQRr $r29r28, 3, $r0
20+
; CHECK-NEXT: STDPtrQRr $r29r28, 4, $r1
21+
STDWPtrQRr $r29r28, 3, $r1r0
2022
21-
STDWPtrQRr $r29r28, 10, $r1r0
23+
; Small displacement where the destination register is killed:
24+
; CHECK: STDPtrQRr $r29r28, 3, $r0
25+
; CHECK-NEXT: STDPtrQRr killed $r29r28, 4, $r1
26+
STDWPtrQRr killed $r29r28, 3, $r1r0
27+
28+
; Small displacement where the source register is killed:
29+
; CHECK: STDPtrQRr $r29r28, 3, killed $r0
30+
; CHECK-NEXT: STDPtrQRr $r29r28, 4, killed $r1
31+
STDWPtrQRr $r29r28, 3, killed $r1r0
32+
33+
; Small displacement, near the limit (=62):
34+
; CHECK: STDPtrQRr $r29r28, 62, $r0
35+
; CHECK-NEXT: STDPtrQRr $r29r28, 63, $r1
36+
STDWPtrQRr $r29r28, 62, $r1r0
37+
38+
; Large displacement (>=63):
39+
; CHECK: PUSHRr $r28, implicit-def $sp, implicit $sp
40+
; CHECK-NEXT: PUSHRr $r29, implicit-def $sp, implicit $sp
41+
; CHECK-NEXT: $r28 = SUBIRdK killed $r28, 193, implicit-def $sreg
42+
; CHECK-NEXT: $r29 = SBCIRdK killed $r29, 255, implicit-def $sreg, implicit killed $sreg
43+
; CHECK-NEXT: STPtrRr $r29r28, $r0
44+
; CHECK-NEXT: STDPtrQRr $r29r28, 1, $r1
45+
; CHECK-NEXT: $r29 = POPRd implicit-def $sp, implicit $sp
46+
; CHECK-NEXT: $r28 = POPRd implicit-def $sp, implicit $sp
47+
STDWPtrQRr $r29r28, 63, $r1r0
48+
49+
; Large displacement where the destination register is killed:
50+
; CHECK: $r28 = SUBIRdK killed $r28, 193, implicit-def $sreg
51+
; CHECK-NEXT: $r29 = SBCIRdK killed $r29, 255, implicit-def $sreg, implicit killed $sreg
52+
; CHECK-NEXT: STPtrRr $r29r28, $r0
53+
; CHECK-NEXT: STDPtrQRr $r29r28, 1, $r1
54+
STDWPtrQRr killed $r29r28, 63, $r1r0
55+
56+
; Large displacement where the source register is killed:
57+
; CHECK: PUSHRr $r28, implicit-def $sp, implicit $sp
58+
; CHECK-NEXT: PUSHRr $r29, implicit-def $sp, implicit $sp
59+
; CHECK-NEXT: $r28 = SUBIRdK killed $r28, 193, implicit-def $sreg
60+
; CHECK-NEXT: $r29 = SBCIRdK killed $r29, 255, implicit-def $sreg, implicit killed $sreg
61+
; CHECK-NEXT: STPtrRr $r29r28, killed $r0
62+
; CHECK-NEXT: STDPtrQRr $r29r28, 1, killed $r1
63+
; CHECK-NEXT: $r29 = POPRd implicit-def $sp, implicit $sp
64+
; CHECK-NEXT: $r28 = POPRd implicit-def $sp, implicit $sp
65+
STDWPtrQRr $r29r28, 63, killed $r1r0
2266
...

‎llvm/test/CodeGen/AVR/relax-mem/STDWPtrQRr.mir

-31
This file was deleted.

‎llvm/utils/gn/secondary/llvm/lib/Target/AVR/BUILD.gn

-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ static_library("LLVMAVRCodeGen") {
3737
"AVRInstrInfo.cpp",
3838
"AVRMCInstLower.cpp",
3939
"AVRRegisterInfo.cpp",
40-
"AVRRelaxMemOperations.cpp",
4140
"AVRShiftExpand.cpp",
4241
"AVRSubtarget.cpp",
4342
"AVRTargetMachine.cpp",

0 commit comments

Comments
 (0)