This is an archive of the discontinued LLVM Phabricator instance.

Differential D116756

[AVR] Do not clear r0 at interrupt entry
ClosedPublic

Authored by aykevl on Jan 6 2022, 9:29 AM.

Details

Reviewers
benshi001
dylanmckay
Commits
rGca27b026f990: [AVR] Do not clear r0 at interrupt entry
Summary

There is no reason to do this: it's a scratch register and can therefore
hold any arbitrary value. And because it is in an interrupt, this code
is performance critical so it should be as short as possible.

I believe r0 was cleared because of the following:

  1. There used to be a bug that the cleared register was r0, not r1 as it should have been.
  2. This was fixed in https://reviews.llvm.org/D99467, but left the code to clear r0.

This patch completes D99467 by removing the clr r0 instruction.

Diff Detail

Event Timeline

aykevl created this revision.Jan 6 2022, 9:29 AM
Herald added subscribers: Jim, hiraditya. · View Herald TranscriptJan 6 2022, 9:29 AM
aykevl requested review of this revision.Jan 6 2022, 9:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 6 2022, 9:29 AM
Herald added a subscriber: llvm-commits. · View Herald Transcript
Harbormaster completed remote builds in B141926: Diff 397918.Jan 6 2022, 9:29 AM
benshi001 accepted this revision.Jan 6 2022, 3:48 PM
This revision is now accepted and ready to land.Jan 6 2022, 3:48 PM
benshi001 added a comment.Jan 6 2022, 8:38 PM

avr-gcc also does not save/restore R0/__tmp_reg__ in interrupt/signal service function.

This revision was landed with ongoing or failed builds.Jan 19 2022, 5:23 AM
Closed by commit rGca27b026f990: [AVR] Do not clear r0 at interrupt entry (authored by aykevl). · Explain Why
This revision was automatically updated to reflect the committed changes.
aykevl added a commit: rGca27b026f990: [AVR] Do not clear r0 at interrupt entry.

Revision Contents

PathSize
llvm/
lib/
Target/
AVR/
5 lines
test/
CodeGen/
AVR/
7 lines

Diff 401187

llvm/lib/Target/AVR/AVRFrameLowering.cpp

Show First 20 Lines • Show All 73 Lines • ▼ Show 20 Lines if (AFI->isInterruptOrSignalHandler()) {
BuildMI(MBB, MBBI, DL, TII.get(AVR::INRdA), AVR::R0) BuildMI(MBB, MBBI, DL, TII.get(AVR::INRdA), AVR::R0)
.addImm(0x3f) .addImm(0x3f)
.setMIFlag(MachineInstr::FrameSetup); .setMIFlag(MachineInstr::FrameSetup);
BuildMI(MBB, MBBI, DL, TII.get(AVR::PUSHRr)) BuildMI(MBB, MBBI, DL, TII.get(AVR::PUSHRr))
.addReg(AVR::R0, RegState::Kill) .addReg(AVR::R0, RegState::Kill)
.setMIFlag(MachineInstr::FrameSetup); .setMIFlag(MachineInstr::FrameSetup);
BuildMI(MBB, MBBI, DL, TII.get(AVR::EORRdRr)) BuildMI(MBB, MBBI, DL, TII.get(AVR::EORRdRr))
.addReg(AVR::R0, RegState::Define)
.addReg(AVR::R0, RegState::Kill)
.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::Define)
.addReg(AVR::R1, RegState::Kill) .addReg(AVR::R1, RegState::Kill)
.addReg(AVR::R1, RegState::Kill) .addReg(AVR::R1, RegState::Kill)
.setMIFlag(MachineInstr::FrameSetup); .setMIFlag(MachineInstr::FrameSetup);
} }
// Early exit if the frame pointer is not needed in this function. // Early exit if the frame pointer is not needed in this function.
if (!HasFP) { if (!HasFP) {
▲ Show 20 Lines • Show All 392 Lines • Show Last 20 Lines

llvm/test/CodeGen/AVR/interrupts.ll

; RUN: llc < %s -march=avr | FileCheck %s ; RUN: llc < %s -march=avr | FileCheck %s
@count = global i8 0 @count = global i8 0
define avr_intrcc void @interrupt_handler() { define avr_intrcc void @interrupt_handler() {
; CHECK-LABEL: interrupt_handler: ; CHECK-LABEL: interrupt_handler:
; CHECK: sei ; CHECK: sei
; CHECK-NEXT: push r0 ; CHECK-NEXT: push r0
; CHECK-NEXT: push r1 ; CHECK-NEXT: push r1
; CHECK-NEXT: in r0, 63 ; CHECK-NEXT: in r0, 63
; CHECK-NEXT: push r0 ; CHECK-NEXT: push r0
; CHECK: clr r0
; CHECK-NEXT: clr r1 ; CHECK-NEXT: clr r1
; CHECK: pop r0 ; CHECK: pop r0
; CHECK-NEXT: out 63, r0 ; CHECK-NEXT: out 63, r0
; CHECK-NEXT: pop r1 ; CHECK-NEXT: pop r1
; CHECK-NEXT: pop r0 ; CHECK-NEXT: pop r0
; CHECK-NEXT: reti ; CHECK-NEXT: reti
ret void ret void
} }
define void @interrupt_handler_via_ir_attribute() #0 { define void @interrupt_handler_via_ir_attribute() #0 {
; CHECK-LABEL: interrupt_handler_via_ir_attribute: ; CHECK-LABEL: interrupt_handler_via_ir_attribute:
; CHECK: sei ; CHECK: sei
; CHECK-NEXT: push r0 ; CHECK-NEXT: push r0
; CHECK-NEXT: push r1 ; CHECK-NEXT: push r1
; CHECK-NEXT: in r0, 63 ; CHECK-NEXT: in r0, 63
; CHECK-NEXT: push r0 ; CHECK-NEXT: push r0
; CHECK: clr r0
; CHECK-NEXT: clr r1 ; CHECK-NEXT: clr r1
; CHECK: pop r0 ; CHECK: pop r0
; CHECK-NEXT: out 63, r0 ; CHECK-NEXT: out 63, r0
; CHECK-NEXT: pop r1 ; CHECK-NEXT: pop r1
; CHECK-NEXT: pop r0 ; CHECK-NEXT: pop r0
; CHECK-NEXT: reti ; CHECK-NEXT: reti
ret void ret void
} }
define avr_signalcc void @signal_handler() { define avr_signalcc void @signal_handler() {
; CHECK-LABEL: signal_handler: ; CHECK-LABEL: signal_handler:
; CHECK-NOT: sei ; CHECK-NOT: sei
; CHECK: push r0 ; CHECK: push r0
; CHECK-NEXT: push r1 ; CHECK-NEXT: push r1
; CHECK-NEXT: in r0, 63 ; CHECK-NEXT: in r0, 63
; CHECK-NEXT: push r0 ; CHECK-NEXT: push r0
; CHECK: clr r0
; CHECK-NEXT: clr r1 ; CHECK-NEXT: clr r1
; CHECK: pop r0 ; CHECK: pop r0
; CHECK-NEXT: out 63, r0 ; CHECK-NEXT: out 63, r0
; CHECK-NEXT: pop r1 ; CHECK-NEXT: pop r1
; CHECK-NEXT: pop r0 ; CHECK-NEXT: pop r0
; CHECK-NEXT: reti ; CHECK-NEXT: reti
ret void ret void
} }
define void @signal_handler_via_attribute() #1 { define void @signal_handler_via_attribute() #1 {
; CHECK-LABEL: signal_handler_via_attribute: ; CHECK-LABEL: signal_handler_via_attribute:
; CHECK-NOT: sei ; CHECK-NOT: sei
; CHECK: push r0 ; CHECK: push r0
; CHECK-NEXT: push r1 ; CHECK-NEXT: push r1
; CHECK-NEXT: in r0, 63 ; CHECK-NEXT: in r0, 63
; CHECK-NEXT: push r0 ; CHECK-NEXT: push r0
; CHECK: clr r0
; CHECK-NEXT: clr r1 ; CHECK-NEXT: clr r1
; CHECK: pop r0 ; CHECK: pop r0
; CHECK-NEXT: out 63, r0 ; CHECK-NEXT: out 63, r0
; CHECK-NEXT: pop r1 ; CHECK-NEXT: pop r1
; CHECK-NEXT: pop r0 ; CHECK-NEXT: pop r0
; CHECK-NEXT: reti ; CHECK-NEXT: reti
ret void ret void
} }
define avr_intrcc void @interrupt_alloca() { define avr_intrcc void @interrupt_alloca() {
; CHECK-LABEL: interrupt_alloca: ; CHECK-LABEL: interrupt_alloca:
; CHECK: sei ; CHECK: sei
; CHECK-NEXT: push r0 ; CHECK-NEXT: push r0
; CHECK-NEXT: push r1 ; CHECK-NEXT: push r1
; CHECK-NEXT: in r0, 63 ; CHECK-NEXT: in r0, 63
; CHECK-NEXT: push r0 ; CHECK-NEXT: push r0
; CHECK: clr r0
; CHECK-NEXT: clr r1 ; CHECK-NEXT: clr r1
; CHECK: push r28 ; CHECK: push r28
; CHECK-NEXT: push r29 ; CHECK-NEXT: push r29
; CHECK-NEXT: in r28, 61 ; CHECK-NEXT: in r28, 61
; CHECK-NEXT: in r29, 62 ; CHECK-NEXT: in r29, 62
; CHECK-NEXT: sbiw r28, 1 ; CHECK-NEXT: sbiw r28, 1
; CHECK-NEXT: in r0, 63 ; CHECK-NEXT: in r0, 63
; CHECK-NEXT: cli ; CHECK-NEXT: cli
Show All 18 Lines
} }
define void @signal_handler_with_increment() #1 { define void @signal_handler_with_increment() #1 {
; CHECK-LABEL: signal_handler_with_increment: ; CHECK-LABEL: signal_handler_with_increment:
; CHECK: push r0 ; CHECK: push r0
; CHECK-NEXT: push r1 ; CHECK-NEXT: push r1
; CHECK-NEXT: in r0, 63 ; CHECK-NEXT: in r0, 63
; CHECK-NEXT: push r0 ; CHECK-NEXT: push r0
; CHECK-NEXT: clr r0
; CHECK-NEXT: clr r1 ; CHECK-NEXT: clr r1
; CHECK-NEXT: push r24 ; CHECK-NEXT: push r24
; CHECK-NEXT: lds r24, count ; CHECK-NEXT: lds r24, count
; CHECK-NEXT: inc r24 ; CHECK-NEXT: inc r24
; CHECK-NEXT: sts count, r24 ; CHECK-NEXT: sts count, r24
; CHECK-NEXT: pop r24 ; CHECK-NEXT: pop r24
; CHECK-NEXT: pop r0 ; CHECK-NEXT: pop r0
; CHECK-NEXT: out 63, r0 ; CHECK-NEXT: out 63, r0
Show All 11 Lines
; When a signal handler calls a function, it must push/pop all call clobbered ; When a signal handler calls a function, it must push/pop all call clobbered
; registers. ; registers.
define void @signal_handler_with_call() #1 { define void @signal_handler_with_call() #1 {
; CHECK-LABEL: signal_handler_with_call: ; CHECK-LABEL: signal_handler_with_call:
; CHECK: push r0 ; CHECK: push r0
; CHECK-NEXT: push r1 ; CHECK-NEXT: push r1
; CHECK-NEXT: in r0, 63 ; CHECK-NEXT: in r0, 63
; CHECK-NEXT: push r0 ; CHECK-NEXT: push r0
; CHECK-NEXT: clr r0
; CHECK-NEXT: clr r1 ; CHECK-NEXT: clr r1
; CHECK-NEXT: push r18 ; CHECK-NEXT: push r18
; CHECK-NEXT: push r19 ; CHECK-NEXT: push r19
; CHECK-NEXT: push r20 ; CHECK-NEXT: push r20
; CHECK-NEXT: push r21 ; CHECK-NEXT: push r21
; CHECK-NEXT: push r22 ; CHECK-NEXT: push r22
; CHECK-NEXT: push r23 ; CHECK-NEXT: push r23
; CHECK-NEXT: push r24 ; CHECK-NEXT: push r24
Show All 29 Lines