[AVR] Merge AVRRelaxMemOperations into AVRExpandPseudoInsts

This commit contains a refactoring that merges AVRRelaxMemOperations
into AVRExpandPseudoInsts, so that we have a single place in code that
expands the STDWPtrQRr opcode.

Seizing the day, I've also fixed a couple of potential bugs with our
previous implementation (e.g. when the destination register was killed,
the previous implementation would try to .addDef() that killed
register, crashing LLVM in the process - that's fixed now, as proved by
the test).

Reviewed By: benshi001

Differential Revision: https://reviews.llvm.org/D122533
This commit is contained in:
Patryk Wychowaniec 2022-04-11 02:21:45 +00:00 committed by Ben Shi
parent 505fce5a9e
commit d16a631c12
9 changed files with 86 additions and 205 deletions

View File

@ -6398,7 +6398,6 @@ llvm/lib/Target/AVR/AVRMCInstLower.cpp
llvm/lib/Target/AVR/AVRMCInstLower.h
llvm/lib/Target/AVR/AVRRegisterInfo.cpp
llvm/lib/Target/AVR/AVRRegisterInfo.h
llvm/lib/Target/AVR/AVRRelaxMemOperations.cpp
llvm/lib/Target/AVR/AVRSelectionDAGInfo.h
llvm/lib/Target/AVR/AVRShiftExpand.cpp
llvm/lib/Target/AVR/AVRSubtarget.cpp

View File

@ -29,12 +29,10 @@ FunctionPass *createAVRISelDag(AVRTargetMachine &TM,
CodeGenOpt::Level OptLevel);
FunctionPass *createAVRExpandPseudoPass();
FunctionPass *createAVRFrameAnalyzerPass();
FunctionPass *createAVRRelaxMemPass();
FunctionPass *createAVRBranchSelectionPass();
void initializeAVRShiftExpandPass(PassRegistry &);
void initializeAVRExpandPseudoPass(PassRegistry &);
void initializeAVRRelaxMemPass(PassRegistry &);
/// Contains the AVR backend.
namespace AVR {

View File

@ -1157,32 +1157,51 @@ bool AVRExpandPseudo::expand<AVR::STWPtrPdRr>(Block &MBB, BlockIt MBBI) {
template <>
bool AVRExpandPseudo::expand<AVR::STDWPtrQRr>(Block &MBB, BlockIt MBBI) {
MachineInstr &MI = *MBBI;
Register SrcLoReg, SrcHiReg;
Register DstReg = MI.getOperand(0).getReg();
Register SrcReg = MI.getOperand(2).getReg();
unsigned Imm = MI.getOperand(1).getImm();
bool DstIsKill = MI.getOperand(0).isKill();
unsigned Imm = MI.getOperand(1).getImm();
Register SrcReg = MI.getOperand(2).getReg();
bool SrcIsKill = MI.getOperand(2).isKill();
unsigned OpLo = AVR::STDPtrQRr;
unsigned OpHi = AVR::STDPtrQRr;
TRI->splitReg(SrcReg, SrcLoReg, SrcHiReg);
// 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.
assert(Imm <= 62 && "Offset is out of range");
// STD's maximum displacement is 63, so larger stores have to be split into a
// set of operations
if (Imm >= 63) {
if (!DstIsKill) {
buildMI(MBB, MBBI, AVR::PUSHWRr).addReg(DstReg);
}
auto MIBLO = buildMI(MBB, MBBI, OpLo)
.addReg(DstReg)
.addImm(Imm)
.addReg(SrcLoReg, getKillRegState(SrcIsKill));
buildMI(MBB, MBBI, AVR::SUBIWRdK)
.addReg(DstReg, RegState::Define)
.addReg(DstReg, RegState::Kill)
.addImm(-Imm);
auto MIBHI = buildMI(MBB, MBBI, OpHi)
.addReg(DstReg, getKillRegState(DstIsKill))
.addImm(Imm + 1)
.addReg(SrcHiReg, getKillRegState(SrcIsKill));
buildMI(MBB, MBBI, AVR::STWPtrRr)
.addReg(DstReg, RegState::Kill)
.addReg(SrcReg, getKillRegState(SrcIsKill));
MIBLO.setMemRefs(MI.memoperands());
MIBHI.setMemRefs(MI.memoperands());
if (!DstIsKill) {
buildMI(MBB, MBBI, AVR::POPWRd).addDef(DstReg, RegState::Define);
}
} else {
unsigned OpLo = AVR::STDPtrQRr;
unsigned OpHi = AVR::STDPtrQRr;
Register SrcLoReg, SrcHiReg;
TRI->splitReg(SrcReg, SrcLoReg, SrcHiReg);
auto MIBLO = buildMI(MBB, MBBI, OpLo)
.addReg(DstReg)
.addImm(Imm)
.addReg(SrcLoReg, getKillRegState(SrcIsKill));
auto MIBHI = buildMI(MBB, MBBI, OpHi)
.addReg(DstReg, getKillRegState(DstIsKill))
.addImm(Imm + 1)
.addReg(SrcHiReg, getKillRegState(SrcIsKill));
MIBLO.setMemRefs(MI.memoperands());
MIBHI.setMemRefs(MI.memoperands());
}
MI.eraseFromParent();
return true;

View File

@ -1,144 +0,0 @@
//===-- AVRRelaxMemOperations.cpp - Relax out of range loads/stores -------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
//
// This file contains a pass which relaxes out of range memory operations into
// equivalent operations which handle bigger addresses.
//
//===----------------------------------------------------------------------===//
#include "AVR.h"
#include "AVRInstrInfo.h"
#include "AVRTargetMachine.h"
#include "MCTargetDesc/AVRMCTargetDesc.h"
#include "llvm/CodeGen/MachineFunctionPass.h"
#include "llvm/CodeGen/MachineInstrBuilder.h"
#include "llvm/CodeGen/MachineRegisterInfo.h"
#include "llvm/CodeGen/TargetRegisterInfo.h"
using namespace llvm;
#define AVR_RELAX_MEM_OPS_NAME "AVR memory operation relaxation pass"
namespace {
class AVRRelaxMem : public MachineFunctionPass {
public:
static char ID;
AVRRelaxMem() : MachineFunctionPass(ID) {
initializeAVRRelaxMemPass(*PassRegistry::getPassRegistry());
}
bool runOnMachineFunction(MachineFunction &MF) override;
StringRef getPassName() const override { return AVR_RELAX_MEM_OPS_NAME; }
private:
typedef MachineBasicBlock Block;
typedef Block::iterator BlockIt;
const TargetInstrInfo *TII;
template <unsigned OP> bool relax(Block &MBB, BlockIt MBBI);
bool runOnBasicBlock(Block &MBB);
bool runOnInstruction(Block &MBB, BlockIt MBBI);
MachineInstrBuilder buildMI(Block &MBB, BlockIt MBBI, unsigned Opcode) {
return BuildMI(MBB, MBBI, MBBI->getDebugLoc(), TII->get(Opcode));
}
};
char AVRRelaxMem::ID = 0;
bool AVRRelaxMem::runOnMachineFunction(MachineFunction &MF) {
bool Modified = false;
const AVRSubtarget &STI = MF.getSubtarget<AVRSubtarget>();
TII = STI.getInstrInfo();
for (Block &MBB : MF) {
bool BlockModified = runOnBasicBlock(MBB);
Modified |= BlockModified;
}
return Modified;
}
bool AVRRelaxMem::runOnBasicBlock(Block &MBB) {
bool Modified = false;
BlockIt MBBI = MBB.begin(), E = MBB.end();
while (MBBI != E) {
BlockIt NMBBI = std::next(MBBI);
Modified |= runOnInstruction(MBB, MBBI);
MBBI = NMBBI;
}
return Modified;
}
template <> bool AVRRelaxMem::relax<AVR::STDWPtrQRr>(Block &MBB, BlockIt MBBI) {
MachineInstr &MI = *MBBI;
MachineOperand &Ptr = MI.getOperand(0);
MachineOperand &Src = MI.getOperand(2);
int64_t Imm = MI.getOperand(1).getImm();
// We can definitely optimise this better.
if (Imm > 63) {
// Push the previous state of the pointer register.
// This instruction must preserve the value.
buildMI(MBB, MBBI, AVR::PUSHWRr).addReg(Ptr.getReg());
// Add the immediate to the pointer register.
buildMI(MBB, MBBI, AVR::SBCIWRdK)
.addReg(Ptr.getReg(), RegState::Define)
.addReg(Ptr.getReg())
.addImm(-Imm);
// Store the value in the source register to the address
// pointed to by the pointer register.
buildMI(MBB, MBBI, AVR::STWPtrRr)
.addReg(Ptr.getReg())
.addReg(Src.getReg(), getKillRegState(Src.isKill()));
// Pop the original state of the pointer register.
buildMI(MBB, MBBI, AVR::POPWRd)
.addDef(Ptr.getReg(), getKillRegState(Ptr.isKill()));
MI.removeFromParent();
}
return false;
}
bool AVRRelaxMem::runOnInstruction(Block &MBB, BlockIt MBBI) {
MachineInstr &MI = *MBBI;
int Opcode = MBBI->getOpcode();
#define RELAX(Op) \
case Op: \
return relax<Op>(MBB, MI)
switch (Opcode) { RELAX(AVR::STDWPtrQRr); }
#undef RELAX
return false;
}
} // end of anonymous namespace
INITIALIZE_PASS(AVRRelaxMem, "avr-relax-mem", AVR_RELAX_MEM_OPS_NAME, false,
false)
namespace llvm {
FunctionPass *createAVRRelaxMemPass() { return new AVRRelaxMem(); }
} // end of namespace llvm

View File

@ -92,7 +92,6 @@ extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeAVRTarget() {
auto &PR = *PassRegistry::getPassRegistry();
initializeAVRExpandPseudoPass(PR);
initializeAVRRelaxMemPass(PR);
initializeAVRShiftExpandPass(PR);
}
@ -118,7 +117,6 @@ bool AVRPassConfig::addInstSelector() {
}
void AVRPassConfig::addPreSched2() {
addPass(createAVRRelaxMemPass());
addPass(createAVRExpandPseudoPass());
}

View File

@ -22,7 +22,6 @@ add_llvm_target(AVRCodeGen
AVRISelDAGToDAG.cpp
AVRISelLowering.cpp
AVRMCInstLower.cpp
AVRRelaxMemOperations.cpp
AVRRegisterInfo.cpp
AVRShiftExpand.cpp
AVRSubtarget.cpp

View File

@ -1,4 +1,4 @@
# RUN: llc -O0 -run-pass=avr-expand-pseudo %s -o - | FileCheck %s
# RUN: llc -O0 -run-pass=avr-expand-pseudo -verify-machineinstrs %s -o - | FileCheck %s
--- |
target triple = "avr--"
@ -15,8 +15,52 @@ body: |
; CHECK-LABEL: test
; CHECK: STDPtrQRr $r29r28, 10, $r0
; CHECK-NEXT: STDPtrQRr $r29r28, 11, $r1
; Small displacement (<63):
; CHECK: STDPtrQRr $r29r28, 3, $r0
; CHECK-NEXT: STDPtrQRr $r29r28, 4, $r1
STDWPtrQRr $r29r28, 3, $r1r0
STDWPtrQRr $r29r28, 10, $r1r0
; Small displacement where the destination register is killed:
; CHECK: STDPtrQRr $r29r28, 3, $r0
; CHECK-NEXT: STDPtrQRr killed $r29r28, 4, $r1
STDWPtrQRr killed $r29r28, 3, $r1r0
; Small displacement where the source register is killed:
; CHECK: STDPtrQRr $r29r28, 3, killed $r0
; CHECK-NEXT: STDPtrQRr $r29r28, 4, killed $r1
STDWPtrQRr $r29r28, 3, killed $r1r0
; Small displacement, near the limit (=62):
; CHECK: STDPtrQRr $r29r28, 62, $r0
; CHECK-NEXT: STDPtrQRr $r29r28, 63, $r1
STDWPtrQRr $r29r28, 62, $r1r0
; Large displacement (>=63):
; CHECK: PUSHRr $r28, implicit-def $sp, implicit $sp
; CHECK-NEXT: PUSHRr $r29, implicit-def $sp, implicit $sp
; CHECK-NEXT: $r28 = SUBIRdK killed $r28, 193, implicit-def $sreg
; CHECK-NEXT: $r29 = SBCIRdK killed $r29, 255, implicit-def $sreg, implicit killed $sreg
; CHECK-NEXT: STPtrRr $r29r28, $r0
; CHECK-NEXT: STDPtrQRr $r29r28, 1, $r1
; CHECK-NEXT: $r29 = POPRd implicit-def $sp, implicit $sp
; CHECK-NEXT: $r28 = POPRd implicit-def $sp, implicit $sp
STDWPtrQRr $r29r28, 63, $r1r0
; Large displacement where the destination register is killed:
; CHECK: $r28 = SUBIRdK killed $r28, 193, implicit-def $sreg
; CHECK-NEXT: $r29 = SBCIRdK killed $r29, 255, implicit-def $sreg, implicit killed $sreg
; CHECK-NEXT: STPtrRr $r29r28, $r0
; CHECK-NEXT: STDPtrQRr $r29r28, 1, $r1
STDWPtrQRr killed $r29r28, 63, $r1r0
; Large displacement where the source register is killed:
; CHECK: PUSHRr $r28, implicit-def $sp, implicit $sp
; CHECK-NEXT: PUSHRr $r29, implicit-def $sp, implicit $sp
; CHECK-NEXT: $r28 = SUBIRdK killed $r28, 193, implicit-def $sreg
; CHECK-NEXT: $r29 = SBCIRdK killed $r29, 255, implicit-def $sreg, implicit killed $sreg
; CHECK-NEXT: STPtrRr $r29r28, killed $r0
; CHECK-NEXT: STDPtrQRr $r29r28, 1, killed $r1
; CHECK-NEXT: $r29 = POPRd implicit-def $sp, implicit $sp
; CHECK-NEXT: $r28 = POPRd implicit-def $sp, implicit $sp
STDWPtrQRr $r29r28, 63, killed $r1r0
...

View File

@ -1,31 +0,0 @@
# RUN: llc -O0 -run-pass=avr-relax-mem %s -o - | FileCheck %s
--- |
target triple = "avr--"
define void @test() {
entry:
ret void
}
...
---
name: test
body: |
bb.0.entry:
; CHECK-LABEL: test
; We shouldn't expand things which already have 6-bit imms.
; CHECK: STDWPtrQRr $r29r28, 63, $r1r0
STDWPtrQRr $r29r28, 63, $r1r0
; We shouldn't expand things which already have 6-bit imms.
; CHECK-NEXT: STDWPtrQRr $r29r28, 0, $r1r0
STDWPtrQRr $r29r28, 0, $r1r0
; CHECK-NEXT: PUSHWRr $r29r28, implicit-def $sp, implicit $sp
; CHECK-NEXT: $r29r28 = SBCIWRdK $r29r28, -64, implicit-def $sreg, implicit $sreg
; CHECK-NEXT: STWPtrRr $r29r28, $r1r0
; CHECK-NEXT: $r29r28 = POPWRd implicit-def $sp, implicit $sp
STDWPtrQRr $r29r28, 64, $r1r0
...

View File

@ -37,7 +37,6 @@ static_library("LLVMAVRCodeGen") {
"AVRInstrInfo.cpp",
"AVRMCInstLower.cpp",
"AVRRegisterInfo.cpp",
"AVRRelaxMemOperations.cpp",
"AVRShiftExpand.cpp",
"AVRSubtarget.cpp",
"AVRTargetMachine.cpp",