[llvm] a560e57 - [AVR] Only push and clear R1 in interrupts when necessary
Ayke van Laethem via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 15 05:29:59 PDT 2022
Author: Ayke van Laethem
Date: 2022-08-15T14:29:38+02:00
New Revision: a560e57a7e32d7da3fd897b3d09ee541911eed65
URL: https://github.com/llvm/llvm-project/commit/a560e57a7e32d7da3fd897b3d09ee541911eed65
DIFF: https://github.com/llvm/llvm-project/commit/a560e57a7e32d7da3fd897b3d09ee541911eed65.diff
LOG: [AVR] Only push and clear R1 in interrupts when necessary
R1 is a reserved register, but LLVM gives the APIs to know when it is
used or not. So this patch uses these APIs to only save/clear/restore R1
in interrupts when necessary.
The main issue here was getting inline assembly to work. One could argue
that this is the job of Clang, but for consistency I've made sure that
R1 is always usable in inline assembly even if that means clearing it
when it might not be needed.
Information on inline assembly in AVR can be found here:
https://www.nongnu.org/avr-libc/user-manual/inline_asm.html#asm_code
Essentially, this seems to suggest that r1 can be freely used in avr-gcc
inline assembly, even without specifying it as an input operand.
Differential Revision: https://reviews.llvm.org/D117426
Added:
llvm/test/CodeGen/AVR/pseudo/ROLBrd.mir
llvm/test/CodeGen/AVR/zeroreg.ll
Modified:
llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp
llvm/lib/Target/AVR/AVRFrameLowering.cpp
llvm/lib/Target/AVR/AVRISelLowering.cpp
llvm/lib/Target/AVR/AVRInstrInfo.td
llvm/test/CodeGen/AVR/interrupts.ll
llvm/test/CodeGen/AVR/pseudo/NEGWRd.mir
Removed:
################################################################################
diff --git a/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp b/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp
index 78592d3c39a2e..076b3f62e0f7f 100644
--- a/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp
+++ b/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp
@@ -1348,8 +1348,8 @@ bool AVRExpandPseudo::expand<AVR::ROLBRd>(Block &MBB, BlockIt MBBI) {
.addReg(DstReg, getKillRegState(DstIsKill))
.addReg(ZERO_REGISTER);
- // SREG is always implicitly killed
- MIB->getOperand(2).setIsKill();
+ MIB->getOperand(3).setIsDead(); // SREG is always dead
+ MIB->getOperand(4).setIsKill(); // SREG is always implicitly killed
MI.eraseFromParent();
return true;
diff --git a/llvm/lib/Target/AVR/AVRFrameLowering.cpp b/llvm/lib/Target/AVR/AVRFrameLowering.cpp
index ec8b74e435ced..d7683b745e0a4 100644
--- a/llvm/lib/Target/AVR/AVRFrameLowering.cpp
+++ b/llvm/lib/Target/AVR/AVRFrameLowering.cpp
@@ -56,6 +56,7 @@ void AVRFrameLowering::emitPrologue(MachineFunction &MF,
const AVRSubtarget &STI = MF.getSubtarget<AVRSubtarget>();
const AVRInstrInfo &TII = *STI.getInstrInfo();
const AVRMachineFunctionInfo *AFI = MF.getInfo<AVRMachineFunctionInfo>();
+ const MachineRegisterInfo &MRI = MF.getRegInfo();
bool HasFP = hasFP(MF);
// Interrupt handlers re-enable interrupts in function entry.
@@ -68,8 +69,8 @@ void AVRFrameLowering::emitPrologue(MachineFunction &MF,
// Emit special prologue code to save R1, R0 and SREG in interrupt/signal
// handlers before saving any other registers.
if (AFI->isInterruptOrSignalHandler()) {
- BuildMI(MBB, MBBI, DL, TII.get(AVR::PUSHWRr))
- .addReg(AVR::R1R0, RegState::Kill)
+ BuildMI(MBB, MBBI, DL, TII.get(AVR::PUSHRr))
+ .addReg(AVR::R0, RegState::Kill)
.setMIFlag(MachineInstr::FrameSetup);
BuildMI(MBB, MBBI, DL, TII.get(AVR::INRdA), AVR::R0)
@@ -78,11 +79,16 @@ void AVRFrameLowering::emitPrologue(MachineFunction &MF,
BuildMI(MBB, MBBI, DL, TII.get(AVR::PUSHRr))
.addReg(AVR::R0, RegState::Kill)
.setMIFlag(MachineInstr::FrameSetup);
- BuildMI(MBB, MBBI, DL, TII.get(AVR::EORRdRr))
- .addReg(AVR::R1, RegState::Define)
- .addReg(AVR::R1, RegState::Kill)
- .addReg(AVR::R1, RegState::Kill)
- .setMIFlag(MachineInstr::FrameSetup);
+ if (!MRI.reg_empty(AVR::R1)) {
+ BuildMI(MBB, MBBI, DL, TII.get(AVR::PUSHRr))
+ .addReg(AVR::R1, RegState::Kill)
+ .setMIFlag(MachineInstr::FrameSetup);
+ BuildMI(MBB, MBBI, DL, TII.get(AVR::EORRdRr))
+ .addReg(AVR::R1, RegState::Define)
+ .addReg(AVR::R1, RegState::Kill)
+ .addReg(AVR::R1, RegState::Kill)
+ .setMIFlag(MachineInstr::FrameSetup);
+ }
}
// Early exit if the frame pointer is not needed in this function.
@@ -132,6 +138,7 @@ void AVRFrameLowering::emitPrologue(MachineFunction &MF,
static void restoreStatusRegister(MachineFunction &MF, MachineBasicBlock &MBB) {
const AVRMachineFunctionInfo *AFI = MF.getInfo<AVRMachineFunctionInfo>();
+ const MachineRegisterInfo &MRI = MF.getRegInfo();
MachineBasicBlock::iterator MBBI = MBB.getLastNonDebugInstr();
@@ -142,11 +149,14 @@ static void restoreStatusRegister(MachineFunction &MF, MachineBasicBlock &MBB) {
// Emit special epilogue code to restore R1, R0 and SREG in interrupt/signal
// handlers at the very end of the function, just before reti.
if (AFI->isInterruptOrSignalHandler()) {
+ if (!MRI.reg_empty(AVR::R1)) {
+ BuildMI(MBB, MBBI, DL, TII.get(AVR::POPRd), AVR::R1);
+ }
BuildMI(MBB, MBBI, DL, TII.get(AVR::POPRd), AVR::R0);
BuildMI(MBB, MBBI, DL, TII.get(AVR::OUTARr))
.addImm(STI.getIORegSREG())
.addReg(AVR::R0, RegState::Kill);
- BuildMI(MBB, MBBI, DL, TII.get(AVR::POPWRd), AVR::R1R0);
+ BuildMI(MBB, MBBI, DL, TII.get(AVR::POPRd), AVR::R0);
}
}
diff --git a/llvm/lib/Target/AVR/AVRISelLowering.cpp b/llvm/lib/Target/AVR/AVRISelLowering.cpp
index 7a1e7b1535a7a..7de585efddbf0 100644
--- a/llvm/lib/Target/AVR/AVRISelLowering.cpp
+++ b/llvm/lib/Target/AVR/AVRISelLowering.cpp
@@ -57,6 +57,8 @@ AVRTargetLowering::AVRTargetLowering(const AVRTargetMachine &TM,
setOperationAction(ISD::DYNAMIC_STACKALLOC, MVT::i8, Expand);
setOperationAction(ISD::DYNAMIC_STACKALLOC, MVT::i16, Expand);
+ setOperationAction(ISD::INLINEASM, MVT::Other, Custom);
+
for (MVT VT : MVT::integer_valuetypes()) {
for (auto N : {ISD::EXTLOAD, ISD::SEXTLOAD, ISD::ZEXTLOAD}) {
setLoadExtAction(N, VT, MVT::i1, Promote);
@@ -836,6 +838,52 @@ SDValue AVRTargetLowering::LowerVASTART(SDValue Op, SelectionDAG &DAG) const {
MachinePointerInfo(SV));
}
+// Modify the existing ISD::INLINEASM node to add the implicit register r1.
+SDValue AVRTargetLowering::LowerINLINEASM(SDValue Op, SelectionDAG &DAG) const {
+ SDValue R1Reg = DAG.getRegister(AVR::R1, MVT::i8);
+ if (Op.getOperand(Op.getNumOperands() - 1) == R1Reg ||
+ Op.getOperand(Op.getNumOperands() - 2) == R1Reg) {
+ // R1 has already been added. Don't add it again.
+ // If this isn't handled, we get called over and over again.
+ return Op;
+ }
+
+ // Get a list of operands to the new INLINEASM node. This is mostly a copy,
+ // with some edits.
+ // Add the following operands at the end (but before the glue node, if it's
+ // there):
+ // - The flags of the implicit R1 register operand.
+ // - The implicit R1 register operand itself.
+ SDLoc dl(Op);
+ SmallVector<SDValue, 8> Ops;
+ SDNode *N = Op.getNode();
+ SDValue Glue;
+ for (unsigned I = 0; I < N->getNumOperands(); I++) {
+ SDValue Operand = N->getOperand(I);
+ if (Operand.getValueType() == MVT::Glue) {
+ // The glue operand always needs to be at the end, so we need to treat it
+ // specially.
+ Glue = Operand;
+ } else {
+ Ops.push_back(Operand);
+ }
+ }
+ unsigned Flags = InlineAsm::getFlagWord(InlineAsm::Kind_RegUse, 1);
+ Ops.push_back(DAG.getTargetConstant(Flags, dl, MVT::i32));
+ Ops.push_back(R1Reg);
+ if (Glue) {
+ Ops.push_back(Glue);
+ }
+
+ // Replace the current INLINEASM node with a new one that has R1 as implicit
+ // parameter.
+ SDValue New = DAG.getNode(N->getOpcode(), dl, N->getVTList(), Ops);
+ DAG.ReplaceAllUsesOfValueWith(Op, New);
+ DAG.ReplaceAllUsesOfValueWith(Op.getValue(1), New.getValue(1));
+
+ return New;
+}
+
SDValue AVRTargetLowering::LowerOperation(SDValue Op, SelectionDAG &DAG) const {
switch (Op.getOpcode()) {
default:
@@ -861,6 +909,8 @@ SDValue AVRTargetLowering::LowerOperation(SDValue Op, SelectionDAG &DAG) const {
case ISD::SDIVREM:
case ISD::UDIVREM:
return LowerDivRem(Op, DAG);
+ case ISD::INLINEASM:
+ return LowerINLINEASM(Op, DAG);
}
return SDValue();
@@ -1451,6 +1501,10 @@ SDValue AVRTargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
Ops.push_back(DAG.getRegister(Reg.first, Reg.second.getValueType()));
}
+ // The R1 register must be passed as an implicit register so that R1 is
+ // correctly zeroed in interrupts.
+ Ops.push_back(DAG.getRegister(AVR::R1, MVT::i8));
+
// Add a register mask operand representing the call-preserved registers.
const TargetRegisterInfo *TRI = Subtarget.getRegisterInfo();
const uint32_t *Mask =
@@ -1572,6 +1626,14 @@ AVRTargetLowering::LowerReturn(SDValue Chain, CallingConv::ID CallConv,
const AVRMachineFunctionInfo *AFI = MF.getInfo<AVRMachineFunctionInfo>();
+ if (!AFI->isInterruptOrSignalHandler()) {
+ // The return instruction has an implicit R1 operand: it must contain zero
+ // on return.
+ // This is not needed in interrupts however, where R1 is handled specially
+ // (only pushed/popped when needed).
+ RetOps.push_back(DAG.getRegister(AVR::R1, MVT::i8));
+ }
+
unsigned RetOpc =
AFI->isInterruptOrSignalHandler() ? AVRISD::RETI_FLAG : AVRISD::RET_FLAG;
diff --git a/llvm/lib/Target/AVR/AVRInstrInfo.td b/llvm/lib/Target/AVR/AVRInstrInfo.td
index f20ba5edf208c..a5fcfc231b130 100644
--- a/llvm/lib/Target/AVR/AVRInstrInfo.td
+++ b/llvm/lib/Target/AVR/AVRInstrInfo.td
@@ -915,6 +915,7 @@ let Constraints = "$src = $rd", Defs = [SREG] in {
// neg Rd+1
// neg Rd
// sbc Rd+1, r1
+ let Uses = [R1] in
def NEGWRd : Pseudo<(outs DREGS
: $rd),
(ins DREGS
@@ -1986,6 +1987,7 @@ let Constraints = "$src = $rd", Defs = [SREG] in {
def ASRWLoRd : Pseudo<(outs DREGS:$rd), (ins DREGS:$src), "asrwlo\t$rd",
[(set i16:$rd, (AVRasrlo i16:$src)), (implicit SREG)]>;
+ let Uses = [R1] in
def ROLBRd : Pseudo<(outs GPR8
: $rd),
(ins GPR8
diff --git a/llvm/test/CodeGen/AVR/interrupts.ll b/llvm/test/CodeGen/AVR/interrupts.ll
index 2310413e75cfc..f85acbf63d6b0 100644
--- a/llvm/test/CodeGen/AVR/interrupts.ll
+++ b/llvm/test/CodeGen/AVR/interrupts.ll
@@ -1,18 +1,16 @@
; RUN: llc < %s -march=avr | FileCheck %s
@count = global i8 0
+ at funcptr = global void () addrspace(1)* null
define avr_intrcc void @interrupt_handler() {
; CHECK-LABEL: interrupt_handler:
; CHECK: sei
; CHECK-NEXT: push r0
-; CHECK-NEXT: push r1
; CHECK-NEXT: in r0, 63
; CHECK-NEXT: push r0
-; CHECK-NEXT: clr r1
; CHECK: pop r0
; CHECK-NEXT: out 63, r0
-; CHECK-NEXT: pop r1
; CHECK-NEXT: pop r0
; CHECK-NEXT: reti
ret void
@@ -22,13 +20,10 @@ define void @interrupt_handler_via_ir_attribute() #0 {
; CHECK-LABEL: interrupt_handler_via_ir_attribute:
; CHECK: sei
; CHECK-NEXT: push r0
-; CHECK-NEXT: push r1
; CHECK-NEXT: in r0, 63
; CHECK-NEXT: push r0
-; CHECK-NEXT: clr r1
; CHECK: pop r0
; CHECK-NEXT: out 63, r0
-; CHECK-NEXT: pop r1
; CHECK-NEXT: pop r0
; CHECK-NEXT: reti
ret void
@@ -38,13 +33,10 @@ define avr_signalcc void @signal_handler() {
; CHECK-LABEL: signal_handler:
; CHECK-NOT: sei
; CHECK: push r0
-; CHECK-NEXT: push r1
; CHECK-NEXT: in r0, 63
; CHECK-NEXT: push r0
-; CHECK-NEXT: clr r1
; CHECK: pop r0
; CHECK-NEXT: out 63, r0
-; CHECK-NEXT: pop r1
; CHECK-NEXT: pop r0
; CHECK-NEXT: reti
ret void
@@ -54,13 +46,10 @@ define void @signal_handler_via_attribute() #1 {
; CHECK-LABEL: signal_handler_via_attribute:
; CHECK-NOT: sei
; CHECK: push r0
-; CHECK-NEXT: push r1
; CHECK-NEXT: in r0, 63
; CHECK-NEXT: push r0
-; CHECK-NEXT: clr r1
; CHECK: pop r0
; CHECK-NEXT: out 63, r0
-; CHECK-NEXT: pop r1
; CHECK-NEXT: pop r0
; CHECK-NEXT: reti
ret void
@@ -70,10 +59,8 @@ define avr_intrcc void @interrupt_alloca() {
; CHECK-LABEL: interrupt_alloca:
; CHECK: sei
; CHECK-NEXT: push r0
-; CHECK-NEXT: push r1
; CHECK-NEXT: in r0, 63
; CHECK-NEXT: push r0
-; CHECK-NEXT: clr r1
; CHECK: push r28
; CHECK-NEXT: push r29
; CHECK-NEXT: in r28, 61
@@ -94,7 +81,6 @@ define avr_intrcc void @interrupt_alloca() {
; CHECK-NEXT: pop r28
; CHECK: pop r0
; CHECK-NEXT: out 63, r0
-; CHECK-NEXT: pop r1
; CHECK-NEXT: pop r0
; CHECK-NEXT: reti
alloca i8
@@ -104,10 +90,8 @@ define avr_intrcc void @interrupt_alloca() {
define void @signal_handler_with_increment() #1 {
; CHECK-LABEL: signal_handler_with_increment:
; CHECK: push r0
-; CHECK-NEXT: push r1
; CHECK-NEXT: in r0, 63
; CHECK-NEXT: push r0
-; CHECK-NEXT: clr r1
; CHECK-NEXT: push r24
; CHECK-NEXT: lds r24, count
; CHECK-NEXT: inc r24
@@ -115,7 +99,6 @@ define void @signal_handler_with_increment() #1 {
; CHECK-NEXT: pop r24
; CHECK-NEXT: pop r0
; CHECK-NEXT: out 63, r0
-; CHECK-NEXT: pop r1
; CHECK-NEXT: pop r0
; CHECK-NEXT: reti
%old = load volatile i8, i8* @count
@@ -124,6 +107,29 @@ define void @signal_handler_with_increment() #1 {
ret void
}
+; Check that r1 is saved/restored and set to 0 when using inline assembly.
+define void @signal_handler_with_asm() #1 {
+; CHECK-LABEL: signal_handler_with_asm:
+; CHECK: push r0
+; CHECK-NEXT: in r0, 63
+; CHECK-NEXT: push r0
+; CHECK-NEXT: push r1
+; CHECK-NEXT: clr r1
+; CHECK-NEXT: push r24
+; CHECK-NEXT: ldi
+; ;APP
+; CHECK: mov
+; ;NO_APP
+; CHECK: pop r24
+; CHECK-NEXT: pop r1
+; CHECK-NEXT: pop r0
+; CHECK-NEXT: out 63, r0
+; CHECK-NEXT: pop r0
+; CHECK-NEXT: reti
+ call i8 asm sideeffect "mov $0, $1", "=r,r"(i8 3) nounwind
+ ret void
+}
+
declare void @foo()
; When a signal handler calls a function, it must push/pop all call clobbered
@@ -131,9 +137,9 @@ declare void @foo()
define void @signal_handler_with_call() #1 {
; CHECK-LABEL: signal_handler_with_call:
; CHECK: push r0
-; CHECK-NEXT: push r1
; CHECK-NEXT: in r0, 63
; CHECK-NEXT: push r0
+; CHECK-NEXT: push r1
; CHECK-NEXT: clr r1
; CHECK-NEXT: push r18
; CHECK-NEXT: push r19
@@ -160,14 +166,58 @@ define void @signal_handler_with_call() #1 {
; CHECK-NEXT: pop r20
; CHECK-NEXT: pop r19
; CHECK-NEXT: pop r18
+; CHECK-NEXT: pop r1
; CHECK-NEXT: pop r0
; CHECK-NEXT: out 63, r0
-; CHECK-NEXT: pop r1
; CHECK-NEXT: pop r0
; CHECK-NEXT: reti
call void @foo()
ret void
}
+define void @signal_handler_with_icall() #1 {
+; CHECK-LABEL: signal_handler_with_icall:
+; CHECK: push r0
+; CHECK-NEXT: in r0, 63
+; CHECK-NEXT: push r0
+; CHECK-NEXT: push r1
+; CHECK-NEXT: clr r1
+; CHECK-NEXT: push r18
+; CHECK-NEXT: push r19
+; CHECK-NEXT: push r20
+; CHECK-NEXT: push r21
+; CHECK-NEXT: push r22
+; CHECK-NEXT: push r23
+; CHECK-NEXT: push r24
+; CHECK-NEXT: push r25
+; CHECK-NEXT: push r26
+; CHECK-NEXT: push r27
+; CHECK-NEXT: push r30
+; CHECK-NEXT: push r31
+; CHECK-NEXT: lds r30, funcptr
+; CHECK-NEXT: lds r31, funcptr+1
+; CHECK-NEXT: icall
+; CHECK-NEXT: pop r31
+; CHECK-NEXT: pop r30
+; CHECK-NEXT: pop r27
+; CHECK-NEXT: pop r26
+; CHECK-NEXT: pop r25
+; CHECK-NEXT: pop r24
+; CHECK-NEXT: pop r23
+; CHECK-NEXT: pop r22
+; CHECK-NEXT: pop r21
+; CHECK-NEXT: pop r20
+; CHECK-NEXT: pop r19
+; CHECK-NEXT: pop r18
+; CHECK-NEXT: pop r1
+; CHECK-NEXT: pop r0
+; CHECK-NEXT: out 63, r0
+; CHECK-NEXT: pop r0
+; CHECK-NEXT: reti
+ %ptr = load volatile void() addrspace(1)*, void() addrspace(1)** @funcptr
+ call void %ptr()
+ ret void
+}
+
attributes #0 = { "interrupt" }
attributes #1 = { "signal" }
diff --git a/llvm/test/CodeGen/AVR/pseudo/NEGWRd.mir b/llvm/test/CodeGen/AVR/pseudo/NEGWRd.mir
index bbddf7625fab7..3f30fc0ecaf1f 100644
--- a/llvm/test/CodeGen/AVR/pseudo/NEGWRd.mir
+++ b/llvm/test/CodeGen/AVR/pseudo/NEGWRd.mir
@@ -22,5 +22,5 @@ body: |
; CHECK-NEXT: $r14 = NEGRd $r14
; CHECK-NEXT: $r15 = SBCRdRr $r15, $r1, implicit-def $sreg, implicit killed $sreg
- $r15r14 = NEGWRd $r15r14, implicit-def $sreg
+ $r15r14 = NEGWRd $r15r14, implicit-def $sreg, implicit $r1
...
diff --git a/llvm/test/CodeGen/AVR/pseudo/ROLBrd.mir b/llvm/test/CodeGen/AVR/pseudo/ROLBrd.mir
new file mode 100644
index 0000000000000..a47d66fd60486
--- /dev/null
+++ b/llvm/test/CodeGen/AVR/pseudo/ROLBrd.mir
@@ -0,0 +1,25 @@
+# RUN: llc -O0 -run-pass=avr-expand-pseudo %s -o - | FileCheck %s
+
+# This test checks the expansion of the 8-bit ROLB (rotate) pseudo instruction.
+
+--- |
+ target triple = "avr--"
+ define void @test_rolbrd() {
+ entry:
+ ret void
+ }
+...
+
+---
+name: test_rolbrd
+body: |
+ bb.0.entry:
+ liveins: $r14
+
+ ; CHECK-LABEL: test_rolbrd
+
+ ; CHECK: $r14 = ADDRdRr killed $r14, killed $r14, implicit-def $sreg
+ ; CHECK-NEXT: $r14 = ADCRdRr $r14, $r1, implicit-def dead $sreg, implicit killed $sreg
+
+ $r14 = ROLBRd $r14, implicit-def $sreg, implicit $r1
+...
diff --git a/llvm/test/CodeGen/AVR/zeroreg.ll b/llvm/test/CodeGen/AVR/zeroreg.ll
new file mode 100644
index 0000000000000..7d0ea5b8f38c7
--- /dev/null
+++ b/llvm/test/CodeGen/AVR/zeroreg.ll
@@ -0,0 +1,27 @@
+; RUN: llc -mattr=avr6,sram < %s -march=avr | FileCheck %s
+
+; This file tests whether the compiler correctly works with the r1 register,
+; clearing it when needed.
+
+; Test regular use of r1 as a zero register.
+; CHECK-LABEL: store8zero:
+; CHECK: st {{[XYZ]}}, r1
+; CHECK-NEXT: mov r24, r1
+; CHECK-NEXT: ret
+define i8 @store8zero(i8* %x) {
+ store i8 0, i8* %x
+ ret i8 0
+}
+
+; Test that mulitplication instructions (mul, muls, etc) clobber r1 and require
+; a "clr r1" instruction.
+; CHECK-LABEL: mul:
+; CHECK: muls
+; CHECK-NEXT: clr r1
+; CHECK-NEXT: st {{[XYZ]}}, r0
+; CHECK-NEXT: ret
+define void @mul(i8* %ptr, i8 %n) {
+ %result = mul i8 %n, 3
+ store i8 %result, i8* %ptr
+ ret void
+}
More information about the llvm-commits
mailing list