This is an archive of the discontinued LLVM Phabricator instance.

Differential D131844

[AVR] Use @earlyclobber instead of register scavenging
ClosedPublic

Authored by aykevl on Aug 13 2022, 1:04 PM.

Details

Reviewers
dylanmckay
benshi001
Commits
rG43a8dbc5be8a: [AVR] Use @earlyclobber instead of register scavenging
Summary

The code to support the case when the register allocator has assigned
the same register to the src and the dst register operand isn't actually
needed:

  • LDWRdPtr and LDDWRdPtrQ have an @earlyclobber on the output register, so the register allocator will make sure to allocate a different register for the output register.
  • LDDWRdYQ does not have an @earlyclobber, but the pointer register is the fixed Y register which is reserved. The register allocator won't use reserved registers for the output value.

This removes a special case in the code that makes the pseudo
instruction expansion pass more complicated than it needs to be.

I tested this locally with TinyGo and all test cases still pass, so that gives some confidence that this doesn't break anything.

Diff Detail

Event Timeline

aykevl created this revision.Aug 13 2022, 1:04 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 13 2022, 1:04 PM
Herald added subscribers: Jim, hiraditya. · View Herald Transcript
aykevl requested review of this revision.Aug 13 2022, 1:04 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 13 2022, 1:04 PM
Herald added a subscriber: llvm-commits. · View Herald Transcript
aykevl edited the summary of this revision. (Show Details)Aug 13 2022, 1:05 PM
aykevl added a comment.Aug 13 2022, 1:11 PM

I forgot about D117957. This is a shorter version that doesn't include the LPM/ELPM changes.

Harbormaster completed remote builds in B181114: Diff 452451.Aug 13 2022, 2:50 PM
benshi001 accepted this revision.Aug 13 2022, 7:46 PM

LGTM

This revision is now accepted and ready to land.Aug 13 2022, 7:46 PM
benshi001 mentioned this in D117957: [AVR] Remove unused register scavenger.Aug 15 2022, 12:55 AM
Closed by commit rG43a8dbc5be8a: [AVR] Use @earlyclobber instead of register scavenging (authored by aykevl). · Explain WhyAug 15 2022, 5:30 AM
This revision was automatically updated to reflect the committed changes.
aykevl added a commit: rG43a8dbc5be8a: [AVR] Use @earlyclobber instead of register scavenging.
aykevl mentioned this in rG91ae1afd3cb9: [AVR] Remove unused register scavenger.Nov 27 2022, 6:31 AM

Revision Contents

PathSize
llvm/
lib/
Target/
AVR/
84 lines
test/
CodeGen/
AVR/
pseudo/

Diff 452639

llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp

Show First 20 Lines • Show All 642 Lines • ▼ Show 20 Linesbool AVRExpandPseudo::expand<AVR::LDSWRdK>(Block &MBB, BlockIt MBBI) {
return true; return true;
} }
template <> template <>
bool AVRExpandPseudo::expand<AVR::LDWRdPtr>(Block &MBB, BlockIt MBBI) { bool AVRExpandPseudo::expand<AVR::LDWRdPtr>(Block &MBB, BlockIt MBBI) {
MachineInstr &MI = *MBBI; MachineInstr &MI = *MBBI;
Register DstLoReg, DstHiReg; Register DstLoReg, DstHiReg;
Register DstReg = MI.getOperand(0).getReg(); Register DstReg = MI.getOperand(0).getReg();
Register TmpReg = 0; // 0 for no temporary register
Register SrcReg = MI.getOperand(1).getReg(); Register SrcReg = MI.getOperand(1).getReg();
bool SrcIsKill = MI.getOperand(1).isKill(); bool SrcIsKill = MI.getOperand(1).isKill();
unsigned OpLo = AVR::LDRdPtr; unsigned OpLo = AVR::LDRdPtr;
unsigned OpHi = AVR::LDDRdPtrQ; unsigned OpHi = AVR::LDDRdPtrQ;
TRI->splitReg(DstReg, DstLoReg, DstHiReg); TRI->splitReg(DstReg, DstLoReg, DstHiReg);
// Use a temporary register if src and dst registers are the same. // DstReg has an earlyclobber so the register allocator will allocate them in
if (DstReg == SrcReg) // separate registers.
TmpReg = scavengeGPR8(MI); assert(DstReg != SrcReg && "Dst and Src registers are the same!");
Register CurDstLoReg = (DstReg == SrcReg) ? TmpReg : DstLoReg;
Register CurDstHiReg = (DstReg == SrcReg) ? TmpReg : DstHiReg;
// Load low byte. // Load low byte.
auto MIBLO = buildMI(MBB, MBBI, OpLo) buildMI(MBB, MBBI, OpLo)
.addReg(CurDstLoReg, RegState::Define) .addReg(DstLoReg, RegState::Define)
.addReg(SrcReg); .addReg(SrcReg)
.setMemRefs(MI.memoperands());
// Push low byte onto stack if necessary.
if (TmpReg)
buildMI(MBB, MBBI, AVR::PUSHRr).addReg(TmpReg);
// Load high byte. // Load high byte.
auto MIBHI = buildMI(MBB, MBBI, OpHi) buildMI(MBB, MBBI, OpHi)
.addReg(CurDstHiReg, RegState::Define) .addReg(DstHiReg, RegState::Define)
.addReg(SrcReg, getKillRegState(SrcIsKill)) .addReg(SrcReg, getKillRegState(SrcIsKill))
.addImm(1); .addImm(1)
.setMemRefs(MI.memoperands());
if (TmpReg) {
// Move the high byte into the final destination.
buildMI(MBB, MBBI, AVR::MOVRdRr, DstHiReg).addReg(TmpReg);
// Move the low byte from the scratch space into the final destination.
buildMI(MBB, MBBI, AVR::POPRd, DstLoReg);
}
MIBLO.setMemRefs(MI.memoperands());
MIBHI.setMemRefs(MI.memoperands());
MI.eraseFromParent(); MI.eraseFromParent();
return true; return true;
} }
template <> template <>
bool AVRExpandPseudo::expand<AVR::LDWRdPtrPi>(Block &MBB, BlockIt MBBI) { bool AVRExpandPseudo::expand<AVR::LDWRdPtrPi>(Block &MBB, BlockIt MBBI) {
MachineInstr &MI = *MBBI; MachineInstr &MI = *MBBI;
▲ Show 20 Lines • Show All 60 Lines • ▼ Show 20 Linesbool AVRExpandPseudo::expand<AVR::LDWRdPtrPd>(Block &MBB, BlockIt MBBI) {
return true; return true;
} }
template <> template <>
bool AVRExpandPseudo::expand<AVR::LDDWRdPtrQ>(Block &MBB, BlockIt MBBI) { bool AVRExpandPseudo::expand<AVR::LDDWRdPtrQ>(Block &MBB, BlockIt MBBI) {
MachineInstr &MI = *MBBI; MachineInstr &MI = *MBBI;
Register DstLoReg, DstHiReg; Register DstLoReg, DstHiReg;
Register DstReg = MI.getOperand(0).getReg(); Register DstReg = MI.getOperand(0).getReg();
Register TmpReg = 0; // 0 for no temporary register
Register SrcReg = MI.getOperand(1).getReg(); Register SrcReg = MI.getOperand(1).getReg();
unsigned Imm = MI.getOperand(2).getImm(); unsigned Imm = MI.getOperand(2).getImm();
bool SrcIsKill = MI.getOperand(1).isKill(); bool SrcIsKill = MI.getOperand(1).isKill();
unsigned OpLo = AVR::LDDRdPtrQ; unsigned OpLo = AVR::LDDRdPtrQ;
unsigned OpHi = AVR::LDDRdPtrQ; unsigned OpHi = AVR::LDDRdPtrQ;
TRI->splitReg(DstReg, DstLoReg, DstHiReg); TRI->splitReg(DstReg, DstLoReg, DstHiReg);
// Since we add 1 to the Imm value for the high byte below, and 63 is the // Since we add 1 to the Imm value for the high byte below, and 63 is the
// highest Imm value allowed for the instruction, 62 is the limit here. // highest Imm value allowed for the instruction, 62 is the limit here.
assert(Imm <= 62 && "Offset is out of range"); assert(Imm <= 62 && "Offset is out of range");
// Use a temporary register if src and dst registers are the same. // DstReg has an earlyclobber so the register allocator will allocate them in
if (DstReg == SrcReg) // separate registers.
TmpReg = scavengeGPR8(MI); assert(DstReg != SrcReg && "Dst and Src registers are the same!");
Register CurDstLoReg = (DstReg == SrcReg) ? TmpReg : DstLoReg;
Register CurDstHiReg = (DstReg == SrcReg) ? TmpReg : DstHiReg;
// Load low byte. // Load low byte.
auto MIBLO = buildMI(MBB, MBBI, OpLo) buildMI(MBB, MBBI, OpLo)
.addReg(CurDstLoReg, RegState::Define) .addReg(DstLoReg, RegState::Define)
.addReg(SrcReg) .addReg(SrcReg)
.addImm(Imm); .addImm(Imm)
.setMemRefs(MI.memoperands());
// Push low byte onto stack if necessary.
if (TmpReg)
buildMI(MBB, MBBI, AVR::PUSHRr).addReg(TmpReg);
// Load high byte. // Load high byte.
auto MIBHI = buildMI(MBB, MBBI, OpHi) buildMI(MBB, MBBI, OpHi)
.addReg(CurDstHiReg, RegState::Define) .addReg(DstHiReg, RegState::Define)
.addReg(SrcReg, getKillRegState(SrcIsKill)) .addReg(SrcReg, getKillRegState(SrcIsKill))
.addImm(Imm + 1); .addImm(Imm + 1)
.setMemRefs(MI.memoperands());
if (TmpReg) {
// Move the high byte into the final destination.
buildMI(MBB, MBBI, AVR::MOVRdRr, DstHiReg).addReg(TmpReg);
// Move the low byte from the scratch space into the final destination.
buildMI(MBB, MBBI, AVR::POPRd, DstLoReg);
}
MIBLO.setMemRefs(MI.memoperands());
MIBHI.setMemRefs(MI.memoperands());
MI.eraseFromParent(); MI.eraseFromParent();
return true; return true;
} }
bool AVRExpandPseudo::expandLPMWELPMW(Block &MBB, BlockIt MBBI, bool IsExt) { bool AVRExpandPseudo::expandLPMWELPMW(Block &MBB, BlockIt MBBI, bool IsExt) {
MachineInstr &MI = *MBBI; MachineInstr &MI = *MBBI;
Register DstLoReg, DstHiReg; Register DstLoReg, DstHiReg;
▲ Show 20 Lines • Show All 1,690 Lines • Show Last 20 Lines

llvm/test/CodeGen/AVR/pseudo/LDWRdPtr-same-src-dst.mir

  • This file was deleted.
# RUN: llc -O0 %s -o - | FileCheck %s
# This test checks the expansion of the 16-bit LDWRdPtr pseudo instruction.
--- |
target triple = "avr--"
define void @test_ldwrdptr() {
entry:
ret void
}
...
---
name: test_ldwrdptr
tracksRegLiveness: true
body: |
bb.0.entry:
liveins: $r31r30
; CHECK-LABEL: test_ldwrdptr
; CHECK: ld [[SCRATCH:r[0-9]+]], Z
; CHECK-NEXT: push [[SCRATCH]]
; CHECK-NEXT: ldd [[SCRATCH]], Z+1
; CHECK-NEXT: mov r31, [[SCRATCH]]
; CHECK-NEXT: pop r30
early-clobber $r31r30 = LDWRdPtr undef $r31r30
...
llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp