[llvm] da0fe5d - [AVR] Fix codegen for rotate instructions
Jim Lin via llvm-commits
llvm-commits at lists.llvm.org
Sun Dec 22 19:41:14 PST 2019
Author: Jim Lin
Date: 2019-12-23T11:41:28+08:00
New Revision: da0fe5db999baa659c2e386e5b0636dadfbbf759
URL: https://github.com/llvm/llvm-project/commit/da0fe5db999baa659c2e386e5b0636dadfbbf759
DIFF: https://github.com/llvm/llvm-project/commit/da0fe5db999baa659c2e386e5b0636dadfbbf759.diff
LOG: [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
Added:
Modified:
llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp
llvm/lib/Target/AVR/AVRISelLowering.cpp
llvm/lib/Target/AVR/AVRInstrInfo.td
llvm/test/CodeGen/AVR/rot.ll
Removed:
################################################################################
diff --git a/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp b/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp
index 83d0f6845332..f466c5c053ad 100644
--- a/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp
+++ b/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp
@@ -52,6 +52,8 @@ class AVRExpandPseudo : public MachineFunctionPass {
/// The register to be used for temporary storage.
const unsigned SCRATCH_REGISTER = AVR::R0;
+ /// The register that will always contain zero.
+ const unsigned ZERO_REGISTER = AVR::R1;
/// The IO address of the status register.
const unsigned SREG_ADDR = 0x3f;
@@ -1242,6 +1244,93 @@ bool AVRExpandPseudo::expand<AVR::POPWRd>(Block &MBB, BlockIt MBBI) {
return true;
}
+template <>
+bool AVRExpandPseudo::expand<AVR::ROLBRd>(Block &MBB, BlockIt MBBI) {
+ // In AVR, the rotate instructions behave quite unintuitively. They rotate
+ // bits through the carry bit in SREG, effectively rotating over 9 bits,
+ // instead of 8. This is useful when we are dealing with numbers over
+ // multiple registers, but when we actually need to rotate stuff, we have
+ // to explicitly add the carry bit.
+
+ MachineInstr &MI = *MBBI;
+ unsigned OpShift, OpCarry;
+ unsigned DstReg = MI.getOperand(0).getReg();
+ bool DstIsDead = MI.getOperand(0).isDead();
+ OpShift = AVR::ADDRdRr;
+ OpCarry = AVR::ADCRdRr;
+
+ // add r16, r16
+ // adc r16, r1
+
+ // Shift part
+ buildMI(MBB, MBBI, OpShift)
+ .addReg(DstReg, RegState::Define | getDeadRegState(DstIsDead))
+ .addReg(DstReg)
+ .addReg(DstReg);
+
+ // Add the carry bit
+ auto MIB = buildMI(MBB, MBBI, OpCarry)
+ .addReg(DstReg, RegState::Define | getDeadRegState(DstIsDead))
+ .addReg(DstReg)
+ .addReg(ZERO_REGISTER);
+
+ // SREG is always implicitly killed
+ MIB->getOperand(2).setIsKill();
+
+ MI.eraseFromParent();
+ return true;
+}
+
+template <>
+bool AVRExpandPseudo::expand<AVR::RORBRd>(Block &MBB, BlockIt MBBI) {
+ // In AVR, the rotate instructions behave quite unintuitively. They rotate
+ // bits through the carry bit in SREG, effectively rotating over 9 bits,
+ // instead of 8. This is useful when we are dealing with numbers over
+ // multiple registers, but when we actually need to rotate stuff, we have
+ // to explicitly add the carry bit.
+
+ MachineInstr &MI = *MBBI;
+ unsigned OpShiftOut, OpLoad, OpShiftIn, OpAdd;
+ unsigned DstReg = MI.getOperand(0).getReg();
+ bool DstIsDead = MI.getOperand(0).isDead();
+ OpShiftOut = AVR::LSRRd;
+ OpLoad = AVR::LDIRdK;
+ OpShiftIn = AVR::RORRd;
+ OpAdd = AVR::ORRdRr;
+
+ // lsr r16
+ // ldi r0, 0
+ // ror r0
+ // or r16, r17
+
+ // Shift out
+ buildMI(MBB, MBBI, OpShiftOut)
+ .addReg(DstReg, RegState::Define | getDeadRegState(DstIsDead))
+ .addReg(DstReg);
+
+ // Put 0 in temporary register
+ buildMI(MBB, MBBI, OpLoad)
+ .addReg(SCRATCH_REGISTER, RegState::Define | getDeadRegState(true))
+ .addImm(0x00);
+
+ // Shift in
+ buildMI(MBB, MBBI, OpShiftIn)
+ .addReg(SCRATCH_REGISTER, RegState::Define | getDeadRegState(true))
+ .addReg(SCRATCH_REGISTER);
+
+ // Add the results together using an or-instruction
+ auto MIB = buildMI(MBB, MBBI, OpAdd)
+ .addReg(DstReg, RegState::Define | getDeadRegState(DstIsDead))
+ .addReg(DstReg)
+ .addReg(SCRATCH_REGISTER);
+
+ // SREG is always implicitly killed
+ MIB->getOperand(2).setIsKill();
+
+ MI.eraseFromParent();
+ return true;
+}
+
template <>
bool AVRExpandPseudo::expand<AVR::LSLWRd>(Block &MBB, BlockIt MBBI) {
MachineInstr &MI = *MBBI;
@@ -1562,6 +1651,8 @@ bool AVRExpandPseudo::expandMI(Block &MBB, BlockIt MBBI) {
EXPAND(AVR::OUTWARr);
EXPAND(AVR::PUSHWRr);
EXPAND(AVR::POPWRd);
+ EXPAND(AVR::ROLBRd);
+ EXPAND(AVR::RORBRd);
EXPAND(AVR::LSLWRd);
EXPAND(AVR::LSRWRd);
EXPAND(AVR::RORWRd);
diff --git a/llvm/lib/Target/AVR/AVRISelLowering.cpp b/llvm/lib/Target/AVR/AVRISelLowering.cpp
index f12c59b7d8c3..4dd843dbd527 100644
--- a/llvm/lib/Target/AVR/AVRISelLowering.cpp
+++ b/llvm/lib/Target/AVR/AVRISelLowering.cpp
@@ -1472,16 +1472,15 @@ MachineBasicBlock *AVRTargetLowering::insertShift(MachineInstr &MI,
RC = &AVR::DREGSRegClass;
break;
case AVR::Rol8:
- Opc = AVR::ADCRdRr; // ROL is an alias of ADC Rd, Rd
+ Opc = AVR::ROLBRd;
RC = &AVR::GPR8RegClass;
- HasRepeatedOperand = true;
break;
case AVR::Rol16:
Opc = AVR::ROLWRd;
RC = &AVR::DREGSRegClass;
break;
case AVR::Ror8:
- Opc = AVR::RORRd;
+ Opc = AVR::RORBRd;
RC = &AVR::GPR8RegClass;
break;
case AVR::Ror16:
diff --git a/llvm/lib/Target/AVR/AVRInstrInfo.td b/llvm/lib/Target/AVR/AVRInstrInfo.td
index caca9b617609..3de28ead4176 100644
--- a/llvm/lib/Target/AVR/AVRInstrInfo.td
+++ b/llvm/lib/Target/AVR/AVRInstrInfo.td
@@ -1676,6 +1676,16 @@ Defs = [SREG] in
{
// 8-bit ROL is an alias of ADC Rd, Rd
+ def ROLBRd : Pseudo<(outs GPR8:$rd),
+ (ins GPR8:$src),
+ "rolb\t$rd",
+ [(set i8:$rd, (AVRrol i8:$src)), (implicit SREG)]>;
+
+ def RORBRd : Pseudo<(outs GPR8:$rd),
+ (ins GPR8:$src),
+ "rorb\t$rd",
+ [(set i8:$rd, (AVRror i8:$src)), (implicit SREG)]>;
+
def ROLWRd : Pseudo<(outs DREGS:$rd),
(ins DREGS:$src),
"rolw\t$rd",
@@ -1686,7 +1696,7 @@ Defs = [SREG] in
(outs GPR8:$rd),
(ins GPR8:$src),
"ror\t$rd",
- [(set i8:$rd, (AVRror i8:$src)), (implicit SREG)]>;
+ []>;
def RORWRd : Pseudo<(outs DREGS:$rd),
(ins DREGS:$src),
diff --git a/llvm/test/CodeGen/AVR/rot.ll b/llvm/test/CodeGen/AVR/rot.ll
index a7b77d97ba69..49981aabe7c3 100644
--- a/llvm/test/CodeGen/AVR/rot.ll
+++ b/llvm/test/CodeGen/AVR/rot.ll
@@ -10,7 +10,8 @@ define i8 @rol8(i8 %val, i8 %amt) {
; CHECK-NEXT: breq LBB0_2
; CHECK-NEXT: LBB0_1:
- ; CHECK-NEXT: rol r24
+ ; CHECK-NEXT: lsl r24
+ ; CHECK-NEXT: adc r24, r1
; CHECK-NEXT: subi r22, 1
; CHECK-NEXT: brne LBB0_1
@@ -36,7 +37,10 @@ define i8 @ror8(i8 %val, i8 %amt) {
; CHECK-NEXT: breq LBB1_2
; CHECK-NEXT: LBB1_1:
- ; CHECK-NEXT: ror r24
+ ; CHECK-NEXT: lsr r24
+ ; CHECK-NEXT: ldi r0, 0
+ ; CHECK-NEXT: ror r0
+ ; CHECK-NEXT: or r24, r0
; CHECK-NEXT: subi r22, 1
; CHECK-NEXT: brne LBB1_1
More information about the llvm-commits
mailing list