[clang] d16a631 - [AVR] Merge AVRRelaxMemOperations into AVRExpandPseudoInsts
Ben Shi via cfe-commits
cfe-commits at lists.llvm.org
Sun Apr 10 19:42:23 PDT 2022
Author: Patryk Wychowaniec
Date: 2022-04-11T02:42:13Z
New Revision: d16a631c124fdc27dd33037a826804ebf21dc582
URL: https://github.com/llvm/llvm-project/commit/d16a631c124fdc27dd33037a826804ebf21dc582
DIFF: https://github.com/llvm/llvm-project/commit/d16a631c124fdc27dd33037a826804ebf21dc582.diff
LOG: [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
Added:
Modified:
clang/docs/tools/clang-formatted-files.txt
llvm/lib/Target/AVR/AVR.h
llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp
llvm/lib/Target/AVR/AVRTargetMachine.cpp
llvm/lib/Target/AVR/CMakeLists.txt
llvm/test/CodeGen/AVR/pseudo/STDWPtrQRr.mir
llvm/utils/gn/secondary/llvm/lib/Target/AVR/BUILD.gn
Removed:
llvm/lib/Target/AVR/AVRRelaxMemOperations.cpp
llvm/test/CodeGen/AVR/relax-mem/STDWPtrQRr.mir
################################################################################
diff --git a/clang/docs/tools/clang-formatted-files.txt b/clang/docs/tools/clang-formatted-files.txt
index e0ce9b1c29228..e344c8faa7da2 100644
--- a/clang/docs/tools/clang-formatted-files.txt
+++ b/clang/docs/tools/clang-formatted-files.txt
@@ -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
diff --git a/llvm/lib/Target/AVR/AVR.h b/llvm/lib/Target/AVR/AVR.h
index 532a57af25157..d29dc5f70e72e 100644
--- a/llvm/lib/Target/AVR/AVR.h
+++ b/llvm/lib/Target/AVR/AVR.h
@@ -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 {
diff --git a/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp b/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp
index de93c863abd86..22f91f98a1644 100644
--- a/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp
+++ b/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp
@@ -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;
diff --git a/llvm/lib/Target/AVR/AVRRelaxMemOperations.cpp b/llvm/lib/Target/AVR/AVRRelaxMemOperations.cpp
deleted file mode 100644
index 76f29eb9f3697..0000000000000
--- a/llvm/lib/Target/AVR/AVRRelaxMemOperations.cpp
+++ /dev/null
@@ -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
diff --git a/llvm/lib/Target/AVR/AVRTargetMachine.cpp b/llvm/lib/Target/AVR/AVRTargetMachine.cpp
index 22b9ba3ece071..c8be3f150662d 100644
--- a/llvm/lib/Target/AVR/AVRTargetMachine.cpp
+++ b/llvm/lib/Target/AVR/AVRTargetMachine.cpp
@@ -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());
}
diff --git a/llvm/lib/Target/AVR/CMakeLists.txt b/llvm/lib/Target/AVR/CMakeLists.txt
index cde2138048dd4..ce8860ce2e3e4 100644
--- a/llvm/lib/Target/AVR/CMakeLists.txt
+++ b/llvm/lib/Target/AVR/CMakeLists.txt
@@ -22,7 +22,6 @@ add_llvm_target(AVRCodeGen
AVRISelDAGToDAG.cpp
AVRISelLowering.cpp
AVRMCInstLower.cpp
- AVRRelaxMemOperations.cpp
AVRRegisterInfo.cpp
AVRShiftExpand.cpp
AVRSubtarget.cpp
diff --git a/llvm/test/CodeGen/AVR/pseudo/STDWPtrQRr.mir b/llvm/test/CodeGen/AVR/pseudo/STDWPtrQRr.mir
index 8c06bf8214ea8..fbec684516ee7 100644
--- a/llvm/test/CodeGen/AVR/pseudo/STDWPtrQRr.mir
+++ b/llvm/test/CodeGen/AVR/pseudo/STDWPtrQRr.mir
@@ -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
...
diff --git a/llvm/test/CodeGen/AVR/relax-mem/STDWPtrQRr.mir b/llvm/test/CodeGen/AVR/relax-mem/STDWPtrQRr.mir
deleted file mode 100644
index 56f9e428f3651..0000000000000
--- a/llvm/test/CodeGen/AVR/relax-mem/STDWPtrQRr.mir
+++ /dev/null
@@ -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
-...
diff --git a/llvm/utils/gn/secondary/llvm/lib/Target/AVR/BUILD.gn b/llvm/utils/gn/secondary/llvm/lib/Target/AVR/BUILD.gn
index 223d1713a4981..28808d2cd0de0 100644
--- a/llvm/utils/gn/secondary/llvm/lib/Target/AVR/BUILD.gn
+++ b/llvm/utils/gn/secondary/llvm/lib/Target/AVR/BUILD.gn
@@ -37,7 +37,6 @@ static_library("LLVMAVRCodeGen") {
"AVRInstrInfo.cpp",
"AVRMCInstLower.cpp",
"AVRRegisterInfo.cpp",
- "AVRRelaxMemOperations.cpp",
"AVRShiftExpand.cpp",
"AVRSubtarget.cpp",
"AVRTargetMachine.cpp",
More information about the cfe-commits
mailing list