Skip to content

Commit da0fe5d

Browse files
tclin914tclin914
committed
[AVR] Fix codegen for rotate instructions
Summary: This patch introduces the ROLBRd and RORBRd pseudo-instructions, which implemenent the "traditional" rotate operations; instead of the AVR rotate instructions that use the carry bit. The code is not optimized at all. Especially when dealing with loops of rotate instructions, this codegen should be improved some day. Related bug: 41358 <https://bugs.llvm.org/show_bug.cgi?id=41358> //Note//: This is my first submitted patch. Reviewers: dylanmckay, Jim Reviewed By: dylanmckay Subscribers: hiraditya, llvm-commits, dylanmckay, dsprenkels Tags: #llvm Patched by dsprenkels (Daan Sprenkels) Differential Revision: https://reviews.llvm.org/D60365
1 parent 9681dc9 commit da0fe5d

File tree

4 files changed

+110
-6
lines changed

4 files changed

+110
-6
lines changed

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

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ class AVRExpandPseudo : public MachineFunctionPass {
5252

5353
/// The register to be used for temporary storage.
5454
const unsigned SCRATCH_REGISTER = AVR::R0;
55+
/// The register that will always contain zero.
56+
const unsigned ZERO_REGISTER = AVR::R1;
5557
/// The IO address of the status register.
5658
const unsigned SREG_ADDR = 0x3f;
5759

@@ -1242,6 +1244,93 @@ bool AVRExpandPseudo::expand<AVR::POPWRd>(Block &MBB, BlockIt MBBI) {
12421244
return true;
12431245
}
12441246

1247+
template <>
1248+
bool AVRExpandPseudo::expand<AVR::ROLBRd>(Block &MBB, BlockIt MBBI) {
1249+
// In AVR, the rotate instructions behave quite unintuitively. They rotate
1250+
// bits through the carry bit in SREG, effectively rotating over 9 bits,
1251+
// instead of 8. This is useful when we are dealing with numbers over
1252+
// multiple registers, but when we actually need to rotate stuff, we have
1253+
// to explicitly add the carry bit.
1254+
1255+
MachineInstr &MI = *MBBI;
1256+
unsigned OpShift, OpCarry;
1257+
unsigned DstReg = MI.getOperand(0).getReg();
1258+
bool DstIsDead = MI.getOperand(0).isDead();
1259+
OpShift = AVR::ADDRdRr;
1260+
OpCarry = AVR::ADCRdRr;
1261+
1262+
// add r16, r16
1263+
// adc r16, r1
1264+
1265+
// Shift part
1266+
buildMI(MBB, MBBI, OpShift)
1267+
.addReg(DstReg, RegState::Define | getDeadRegState(DstIsDead))
1268+
.addReg(DstReg)
1269+
.addReg(DstReg);
1270+
1271+
// Add the carry bit
1272+
auto MIB = buildMI(MBB, MBBI, OpCarry)
1273+
.addReg(DstReg, RegState::Define | getDeadRegState(DstIsDead))
1274+
.addReg(DstReg)
1275+
.addReg(ZERO_REGISTER);
1276+
1277+
// SREG is always implicitly killed
1278+
MIB->getOperand(2).setIsKill();
1279+
1280+
MI.eraseFromParent();
1281+
return true;
1282+
}
1283+
1284+
template <>
1285+
bool AVRExpandPseudo::expand<AVR::RORBRd>(Block &MBB, BlockIt MBBI) {
1286+
// In AVR, the rotate instructions behave quite unintuitively. They rotate
1287+
// bits through the carry bit in SREG, effectively rotating over 9 bits,
1288+
// instead of 8. This is useful when we are dealing with numbers over
1289+
// multiple registers, but when we actually need to rotate stuff, we have
1290+
// to explicitly add the carry bit.
1291+
1292+
MachineInstr &MI = *MBBI;
1293+
unsigned OpShiftOut, OpLoad, OpShiftIn, OpAdd;
1294+
unsigned DstReg = MI.getOperand(0).getReg();
1295+
bool DstIsDead = MI.getOperand(0).isDead();
1296+
OpShiftOut = AVR::LSRRd;
1297+
OpLoad = AVR::LDIRdK;
1298+
OpShiftIn = AVR::RORRd;
1299+
OpAdd = AVR::ORRdRr;
1300+
1301+
// lsr r16
1302+
// ldi r0, 0
1303+
// ror r0
1304+
// or r16, r17
1305+
1306+
// Shift out
1307+
buildMI(MBB, MBBI, OpShiftOut)
1308+
.addReg(DstReg, RegState::Define | getDeadRegState(DstIsDead))
1309+
.addReg(DstReg);
1310+
1311+
// Put 0 in temporary register
1312+
buildMI(MBB, MBBI, OpLoad)
1313+
.addReg(SCRATCH_REGISTER, RegState::Define | getDeadRegState(true))
1314+
.addImm(0x00);
1315+
1316+
// Shift in
1317+
buildMI(MBB, MBBI, OpShiftIn)
1318+
.addReg(SCRATCH_REGISTER, RegState::Define | getDeadRegState(true))
1319+
.addReg(SCRATCH_REGISTER);
1320+
1321+
// Add the results together using an or-instruction
1322+
auto MIB = buildMI(MBB, MBBI, OpAdd)
1323+
.addReg(DstReg, RegState::Define | getDeadRegState(DstIsDead))
1324+
.addReg(DstReg)
1325+
.addReg(SCRATCH_REGISTER);
1326+
1327+
// SREG is always implicitly killed
1328+
MIB->getOperand(2).setIsKill();
1329+
1330+
MI.eraseFromParent();
1331+
return true;
1332+
}
1333+
12451334
template <>
12461335
bool AVRExpandPseudo::expand<AVR::LSLWRd>(Block &MBB, BlockIt MBBI) {
12471336
MachineInstr &MI = *MBBI;
@@ -1562,6 +1651,8 @@ bool AVRExpandPseudo::expandMI(Block &MBB, BlockIt MBBI) {
15621651
EXPAND(AVR::OUTWARr);
15631652
EXPAND(AVR::PUSHWRr);
15641653
EXPAND(AVR::POPWRd);
1654+
EXPAND(AVR::ROLBRd);
1655+
EXPAND(AVR::RORBRd);
15651656
EXPAND(AVR::LSLWRd);
15661657
EXPAND(AVR::LSRWRd);
15671658
EXPAND(AVR::RORWRd);

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1472,16 +1472,15 @@ MachineBasicBlock *AVRTargetLowering::insertShift(MachineInstr &MI,
14721472
RC = &AVR::DREGSRegClass;
14731473
break;
14741474
case AVR::Rol8:
1475-
Opc = AVR::ADCRdRr; // ROL is an alias of ADC Rd, Rd
1475+
Opc = AVR::ROLBRd;
14761476
RC = &AVR::GPR8RegClass;
1477-
HasRepeatedOperand = true;
14781477
break;
14791478
case AVR::Rol16:
14801479
Opc = AVR::ROLWRd;
14811480
RC = &AVR::DREGSRegClass;
14821481
break;
14831482
case AVR::Ror8:
1484-
Opc = AVR::RORRd;
1483+
Opc = AVR::RORBRd;
14851484
RC = &AVR::GPR8RegClass;
14861485
break;
14871486
case AVR::Ror16:

‎llvm/lib/Target/AVR/AVRInstrInfo.td

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1676,6 +1676,16 @@ Defs = [SREG] in
16761676
{
16771677
// 8-bit ROL is an alias of ADC Rd, Rd
16781678

1679+
def ROLBRd : Pseudo<(outs GPR8:$rd),
1680+
(ins GPR8:$src),
1681+
"rolb\t$rd",
1682+
[(set i8:$rd, (AVRrol i8:$src)), (implicit SREG)]>;
1683+
1684+
def RORBRd : Pseudo<(outs GPR8:$rd),
1685+
(ins GPR8:$src),
1686+
"rorb\t$rd",
1687+
[(set i8:$rd, (AVRror i8:$src)), (implicit SREG)]>;
1688+
16791689
def ROLWRd : Pseudo<(outs DREGS:$rd),
16801690
(ins DREGS:$src),
16811691
"rolw\t$rd",
@@ -1686,7 +1696,7 @@ Defs = [SREG] in
16861696
(outs GPR8:$rd),
16871697
(ins GPR8:$src),
16881698
"ror\t$rd",
1689-
[(set i8:$rd, (AVRror i8:$src)), (implicit SREG)]>;
1699+
[]>;
16901700

16911701
def RORWRd : Pseudo<(outs DREGS:$rd),
16921702
(ins DREGS:$src),

‎llvm/test/CodeGen/AVR/rot.ll

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ define i8 @rol8(i8 %val, i8 %amt) {
1010
; CHECK-NEXT: breq LBB0_2
1111

1212
; CHECK-NEXT: LBB0_1:
13-
; CHECK-NEXT: rol r24
13+
; CHECK-NEXT: lsl r24
14+
; CHECK-NEXT: adc r24, r1
1415
; CHECK-NEXT: subi r22, 1
1516
; CHECK-NEXT: brne LBB0_1
1617

@@ -36,7 +37,10 @@ define i8 @ror8(i8 %val, i8 %amt) {
3637
; CHECK-NEXT: breq LBB1_2
3738

3839
; CHECK-NEXT: LBB1_1:
39-
; CHECK-NEXT: ror r24
40+
; CHECK-NEXT: lsr r24
41+
; CHECK-NEXT: ldi r0, 0
42+
; CHECK-NEXT: ror r0
43+
; CHECK-NEXT: or r24, r0
4044
; CHECK-NEXT: subi r22, 1
4145
; CHECK-NEXT: brne LBB1_1
4246

0 commit comments

Comments
 (0)