From 982b7d16468e2276f265789afe6d6b16f045676a Mon Sep 17 00:00:00 2001 From: Patryk Wychowaniec Date: Mon, 11 Apr 2022 02:21:45 +0000 Subject: [PATCH 1/2] [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 --- clang/docs/tools/clang-formatted-files.txt | 1 - llvm/lib/Target/AVR/AVR.h | 2 - llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp | 57 ++++--- llvm/lib/Target/AVR/AVRRelaxMemOperations.cpp | 144 ------------------ llvm/lib/Target/AVR/AVRTargetMachine.cpp | 2 - llvm/lib/Target/AVR/CMakeLists.txt | 1 - llvm/test/CodeGen/AVR/pseudo/STDWPtrQRr.mir | 52 ++++++- .../test/CodeGen/AVR/relax-mem/STDWPtrQRr.mir | 31 ---- .../gn/secondary/llvm/lib/Target/AVR/BUILD.gn | 1 - 9 files changed, 86 insertions(+), 205 deletions(-) delete mode 100644 llvm/lib/Target/AVR/AVRRelaxMemOperations.cpp delete mode 100644 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 c7defa9cd88c6..02650d2c33010 100644 --- a/clang/docs/tools/clang-formatted-files.txt +++ b/clang/docs/tools/clang-formatted-files.txt @@ -5990,7 +5990,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 0b512172ba109..0190e6c1398f9 100644 --- a/llvm/lib/Target/AVR/AVR.h +++ b/llvm/lib/Target/AVR/AVR.h @@ -27,12 +27,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 144ae2b320f9c..32e5e6adf391b 100644 --- a/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp +++ b/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp @@ -1230,32 +1230,51 @@ bool AVRExpandPseudo::expand(Block &MBB, BlockIt MBBI) { template <> bool AVRExpandPseudo::expand(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 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(); - 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(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(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", From ac5145da99be8393f488c2a5e4d03a2f3e2d0396 Mon Sep 17 00:00:00 2001 From: Patryk Wychowaniec Date: Thu, 5 May 2022 03:07:41 +0000 Subject: [PATCH 2/2] [AVR] Always expand STDSPQRr & STDWSPQRr Currently, STDSPQRr and STDWSPQRr are expanded only during AVRFrameLowering - this means that if any of those instructions happen to appear _outside_ of the typical FrameSetup / FrameDestroy context, they wouldn't get substituted, eventually leading to a crash: ``` LLVM ERROR: Not supported instr: > ``` This commit fixes this issue by moving expansion of those two opcodes into AVRExpandPseudo. This bug was originally discovered due to the Rust compiler_builtins library. Its 0.1.37 release contained a 128-bit software division/remainder routine that exercised this buggy branch in the code. Reviewed By: benshi001 Differential Revision: https://reviews.llvm.org/D123528 --- llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp | 38 ++++++ llvm/lib/Target/AVR/AVRFrameLowering.cpp | 115 +++++++++---------- llvm/test/CodeGen/AVR/stdwstk.ll | 15 +++ 3 files changed, 109 insertions(+), 59 deletions(-) create mode 100644 llvm/test/CodeGen/AVR/stdwstk.ll diff --git a/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp b/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp index 32e5e6adf391b..ddd1a9f658179 100644 --- a/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp +++ b/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp @@ -1280,6 +1280,42 @@ bool AVRExpandPseudo::expand(Block &MBB, BlockIt MBBI) { return true; } +template <> +bool AVRExpandPseudo::expand(Block &MBB, BlockIt MBBI) { + MachineInstr &MI = *MBBI; + const MachineFunction &MF = *MBB.getParent(); + const AVRSubtarget &STI = MF.getSubtarget(); + + assert(MI.getOperand(0).getReg() == AVR::SP && + "SP is expected as base pointer"); + + assert(STI.getFrameLowering()->hasReservedCallFrame(MF) && + "unexpected STDSPQRr pseudo instruction"); + + MI.setDesc(TII->get(AVR::STDPtrQRr)); + MI.getOperand(0).setReg(AVR::R29R28); + + return true; +} + +template <> +bool AVRExpandPseudo::expand(Block &MBB, BlockIt MBBI) { + MachineInstr &MI = *MBBI; + const MachineFunction &MF = *MBB.getParent(); + const AVRSubtarget &STI = MF.getSubtarget(); + + assert(MI.getOperand(0).getReg() == AVR::SP && + "SP is expected as base pointer"); + + assert(STI.getFrameLowering()->hasReservedCallFrame(MF) && + "unexpected STDWSPQRr pseudo instruction"); + + MI.setDesc(TII->get(AVR::STDWPtrQRr)); + MI.getOperand(0).setReg(AVR::R29R28); + + return true; +} + template <> bool AVRExpandPseudo::expand(Block &MBB, BlockIt MBBI) { MachineInstr &MI = *MBBI; @@ -2365,6 +2401,8 @@ bool AVRExpandPseudo::expandMI(Block &MBB, BlockIt MBBI) { EXPAND(AVR::STWPtrPiRr); EXPAND(AVR::STWPtrPdRr); EXPAND(AVR::STDWPtrQRr); + EXPAND(AVR::STDSPQRr); + EXPAND(AVR::STDWSPQRr); EXPAND(AVR::INWRdA); EXPAND(AVR::OUTWARr); EXPAND(AVR::PUSHWRr); diff --git a/llvm/lib/Target/AVR/AVRFrameLowering.cpp b/llvm/lib/Target/AVR/AVRFrameLowering.cpp index 42a6e4f55dc86..03020e25bfe9b 100644 --- a/llvm/lib/Target/AVR/AVRFrameLowering.cpp +++ b/llvm/lib/Target/AVR/AVRFrameLowering.cpp @@ -201,8 +201,8 @@ void AVRFrameLowering::emitEpilogue(MachineFunction &MF, // Restore the frame pointer by doing FP += . MachineInstr *MI = BuildMI(MBB, MBBI, DL, TII.get(Opcode), AVR::R29R28) - .addReg(AVR::R29R28, RegState::Kill) - .addImm(FrameSize); + .addReg(AVR::R29R28, RegState::Kill) + .addImm(FrameSize); // The SREG implicit def is dead. MI->getOperand(3).setIsDead(); } @@ -299,7 +299,7 @@ bool AVRFrameLowering::restoreCalleeSavedRegisters( /// real instructions. static void fixStackStores(MachineBasicBlock &MBB, MachineBasicBlock::iterator StartMI, - const TargetInstrInfo &TII, Register FP) { + const TargetInstrInfo &TII) { // Iterate through the BB until we hit a call instruction or we reach the end. for (MachineInstr &MI : llvm::make_early_inc_range(llvm::make_range(StartMI, MBB.end()))) { @@ -313,7 +313,7 @@ static void fixStackStores(MachineBasicBlock &MBB, continue; assert(MI.getOperand(0).getReg() == AVR::SP && - "Invalid register, should be SP!"); + "SP is expected as base pointer"); // Replace this instruction with a regular store. Use Y as the base // pointer since it is guaranteed to contain a copy of SP. @@ -321,7 +321,7 @@ static void fixStackStores(MachineBasicBlock &MBB, (Opcode == AVR::STDWSPQRr) ? AVR::STDWPtrQRr : AVR::STDPtrQRr; MI.setDesc(TII.get(STOpc)); - MI.getOperand(0).setReg(FP); + MI.getOperand(0).setReg(AVR::R31R30); } } @@ -331,11 +331,7 @@ MachineBasicBlock::iterator AVRFrameLowering::eliminateCallFramePseudoInstr( const AVRSubtarget &STI = MF.getSubtarget(); const AVRInstrInfo &TII = *STI.getInstrInfo(); - // There is nothing to insert when the call frame memory is allocated during - // function entry. Delete the call frame pseudo and replace all pseudo stores - // with real store instructions. if (hasReservedCallFrame(MF)) { - fixStackStores(MBB, MI, TII, AVR::R29R28); return MBB.erase(MI); } @@ -343,57 +339,58 @@ MachineBasicBlock::iterator AVRFrameLowering::eliminateCallFramePseudoInstr( unsigned int Opcode = MI->getOpcode(); int Amount = TII.getFrameSize(*MI); - // ADJCALLSTACKUP and ADJCALLSTACKDOWN are converted to adiw/subi - // instructions to read and write the stack pointer in I/O space. - if (Amount != 0) { - assert(getStackAlign() == Align(1) && "Unsupported stack alignment"); - - if (Opcode == TII.getCallFrameSetupOpcode()) { - // Update the stack pointer. - // In many cases this can be done far more efficiently by pushing the - // relevant values directly to the stack. However, doing that correctly - // (in the right order, possibly skipping some empty space for undef - // values, etc) is tricky and thus left to be optimized in the future. - BuildMI(MBB, MI, DL, TII.get(AVR::SPREAD), AVR::R31R30).addReg(AVR::SP); - - MachineInstr *New = - BuildMI(MBB, MI, DL, TII.get(AVR::SUBIWRdK), AVR::R31R30) - .addReg(AVR::R31R30, RegState::Kill) - .addImm(Amount); - New->getOperand(3).setIsDead(); - - BuildMI(MBB, MI, DL, TII.get(AVR::SPWRITE), AVR::SP).addReg(AVR::R31R30); - - // Make sure the remaining stack stores are converted to real store - // instructions. - fixStackStores(MBB, MI, TII, AVR::R31R30); - } else { - assert(Opcode == TII.getCallFrameDestroyOpcode()); - - // Note that small stack changes could be implemented more efficiently - // with a few pop instructions instead of the 8-9 instructions now - // required. - - // Select the best opcode to adjust SP based on the offset size. - unsigned addOpcode; - if (isUInt<6>(Amount)) { - addOpcode = AVR::ADIWRdK; - } else { - addOpcode = AVR::SUBIWRdK; - Amount = -Amount; - } + if (Amount == 0) { + return MBB.erase(MI); + } + + assert(getStackAlign() == Align(1) && "Unsupported stack alignment"); + + if (Opcode == TII.getCallFrameSetupOpcode()) { + // Update the stack pointer. + // In many cases this can be done far more efficiently by pushing the + // relevant values directly to the stack. However, doing that correctly + // (in the right order, possibly skipping some empty space for undef + // values, etc) is tricky and thus left to be optimized in the future. + BuildMI(MBB, MI, DL, TII.get(AVR::SPREAD), AVR::R31R30).addReg(AVR::SP); + + MachineInstr *New = + BuildMI(MBB, MI, DL, TII.get(AVR::SUBIWRdK), AVR::R31R30) + .addReg(AVR::R31R30, RegState::Kill) + .addImm(Amount); + New->getOperand(3).setIsDead(); - // Build the instruction sequence. - BuildMI(MBB, MI, DL, TII.get(AVR::SPREAD), AVR::R31R30).addReg(AVR::SP); + BuildMI(MBB, MI, DL, TII.get(AVR::SPWRITE), AVR::SP).addReg(AVR::R31R30); - MachineInstr *New = BuildMI(MBB, MI, DL, TII.get(addOpcode), AVR::R31R30) - .addReg(AVR::R31R30, RegState::Kill) - .addImm(Amount); - New->getOperand(3).setIsDead(); + // Make sure the remaining stack stores are converted to real store + // instructions. + fixStackStores(MBB, MI, TII); + } else { + assert(Opcode == TII.getCallFrameDestroyOpcode()); - BuildMI(MBB, MI, DL, TII.get(AVR::SPWRITE), AVR::SP) - .addReg(AVR::R31R30, RegState::Kill); + // Note that small stack changes could be implemented more efficiently + // with a few pop instructions instead of the 8-9 instructions now + // required. + + // Select the best opcode to adjust SP based on the offset size. + unsigned AddOpcode; + + if (isUInt<6>(Amount)) { + AddOpcode = AVR::ADIWRdK; + } else { + AddOpcode = AVR::SUBIWRdK; + Amount = -Amount; } + + // Build the instruction sequence. + BuildMI(MBB, MI, DL, TII.get(AVR::SPREAD), AVR::R31R30).addReg(AVR::SP); + + MachineInstr *New = BuildMI(MBB, MI, DL, TII.get(AddOpcode), AVR::R31R30) + .addReg(AVR::R31R30, RegState::Kill) + .addImm(Amount); + New->getOperand(3).setIsDead(); + + BuildMI(MBB, MI, DL, TII.get(AVR::SPWRITE), AVR::SP) + .addReg(AVR::R31R30, RegState::Kill); } return MBB.erase(MI); @@ -420,7 +417,7 @@ struct AVRFrameAnalyzer : public MachineFunctionPass { bool runOnMachineFunction(MachineFunction &MF) override { const MachineFrameInfo &MFI = MF.getFrameInfo(); - AVRMachineFunctionInfo *FuncInfo = MF.getInfo(); + AVRMachineFunctionInfo *AFI = MF.getInfo(); // If there are no fixed frame indexes during this stage it means there // are allocas present in the function. @@ -431,7 +428,7 @@ struct AVRFrameAnalyzer : public MachineFunctionPass { for (unsigned i = 0, e = MFI.getObjectIndexEnd(); i != e; ++i) { // Variable sized objects have size 0. if (MFI.getObjectSize(i)) { - FuncInfo->setHasAllocas(true); + AFI->setHasAllocas(true); break; } } @@ -460,7 +457,7 @@ struct AVRFrameAnalyzer : public MachineFunctionPass { } if (MFI.isFixedObjectIndex(MO.getIndex())) { - FuncInfo->setHasStackArgs(true); + AFI->setHasStackArgs(true); return false; } } diff --git a/llvm/test/CodeGen/AVR/stdwstk.ll b/llvm/test/CodeGen/AVR/stdwstk.ll new file mode 100644 index 0000000000000..61b02235f2e49 --- /dev/null +++ b/llvm/test/CodeGen/AVR/stdwstk.ll @@ -0,0 +1,15 @@ +; RUN: llc < %s -march=avr -mcpu=atmega328 -O1 | FileCheck %s +; CHECK-NOT: stdwstk + +; Checks that we expand STDWSPQRr always - even if it appears outside of the +; FrameSetup/FrameDestroy context. + +declare { } @foo(i128, i128) addrspace(1) + +define i128 @bar(i128 %a, i128 %b) addrspace(1) { + %b_neg = icmp slt i128 %b, 0 + %divisor = select i1 %b_neg, i128 0, i128 %b + %result = tail call fastcc addrspace(1) { } @foo(i128 undef, i128 %divisor) + + ret i128 0 +}