[llvm] f41d2d9 - [AVR] Remove redundant dynalloca SP save/restore pass

Ayke van Laethem via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 19 05:22:22 PST 2022


Author: Ayke van Laethem
Date: 2022-01-19T14:22:13+01:00
New Revision: f41d2d9469d66b92c033616c080e196bdb07363a

URL: https://github.com/llvm/llvm-project/commit/f41d2d9469d66b92c033616c080e196bdb07363a
DIFF: https://github.com/llvm/llvm-project/commit/f41d2d9469d66b92c033616c080e196bdb07363a.diff

LOG: [AVR] Remove redundant dynalloca SP save/restore pass

I think this pass was previously used under the assumption that most
functions would not need a frame pointer and it would be more efficient
to store the old stack pointer in a regular register pair.

Unfortunately, right now we're forced to always reserve the Y register
as a frame pointer: whether or not this is needed is only known after
regsiter allocation at which point it doesn't make sense anymore to mark
it as non-reserved. Therefore, it makes sense to use the Y register to
store the old stack pointer in functions with dynamic allocas (with a
variable size or not in the entry block). Knowing this can make the code
around dynamic allocas a lot simpler: simply save/restore the frame
pointer.

This is especially relevant in functions that have a frame pointer
anyway (for example, because they have stack spills). The stack restore
in the epilogue will implicitly restore the old stack pointer, so there
is no need to store the old stack pointer separately. It even reduces
register pressure as a side effect.

Differential Revision: https://reviews.llvm.org/D97815

Added: 
    

Modified: 
    llvm/lib/Target/AVR/AVR.h
    llvm/lib/Target/AVR/AVRFrameLowering.cpp
    llvm/lib/Target/AVR/AVRTargetMachine.cpp
    llvm/test/CodeGen/AVR/dynalloca.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/AVR/AVR.h b/llvm/lib/Target/AVR/AVR.h
index 143c339c0664e..a21d7d8c63c0a 100644
--- a/llvm/lib/Target/AVR/AVR.h
+++ b/llvm/lib/Target/AVR/AVR.h
@@ -28,7 +28,6 @@ FunctionPass *createAVRISelDag(AVRTargetMachine &TM,
 FunctionPass *createAVRExpandPseudoPass();
 FunctionPass *createAVRFrameAnalyzerPass();
 FunctionPass *createAVRRelaxMemPass();
-FunctionPass *createAVRDynAllocaSRPass();
 FunctionPass *createAVRBranchSelectionPass();
 
 void initializeAVRShiftExpandPass(PassRegistry &);

diff  --git a/llvm/lib/Target/AVR/AVRFrameLowering.cpp b/llvm/lib/Target/AVR/AVRFrameLowering.cpp
index 74c45d32136da..6f5765e7f4097 100644
--- a/llvm/lib/Target/AVR/AVRFrameLowering.cpp
+++ b/llvm/lib/Target/AVR/AVRFrameLowering.cpp
@@ -176,7 +176,7 @@ void AVRFrameLowering::emitEpilogue(MachineFunction &MF,
   const AVRInstrInfo &TII = *STI.getInstrInfo();
 
   // Early exit if there is no need to restore the frame pointer.
-  if (!FrameSize) {
+  if (!FrameSize && !MF.getFrameInfo().hasVarSizedObjects()) {
     restoreStatusRegister(MF, MBB);
     return;
   }
@@ -193,22 +193,24 @@ void AVRFrameLowering::emitEpilogue(MachineFunction &MF,
     --MBBI;
   }
 
-  unsigned Opcode;
+  if (FrameSize) {
+    unsigned Opcode;
 
-  // Select the optimal opcode depending on how big it is.
-  if (isUInt<6>(FrameSize)) {
-    Opcode = AVR::ADIWRdK;
-  } else {
-    Opcode = AVR::SUBIWRdK;
-    FrameSize = -FrameSize;
-  }
+    // Select the optimal opcode depending on how big it is.
+    if (isUInt<6>(FrameSize)) {
+      Opcode = AVR::ADIWRdK;
+    } else {
+      Opcode = AVR::SUBIWRdK;
+      FrameSize = -FrameSize;
+    }
 
-  // Restore the frame pointer by doing FP += <size>.
-  MachineInstr *MI = BuildMI(MBB, MBBI, DL, TII.get(Opcode), AVR::R29R28)
-                         .addReg(AVR::R29R28, RegState::Kill)
-                         .addImm(FrameSize);
-  // The SREG implicit def is dead.
-  MI->getOperand(3).setIsDead();
+    // Restore the frame pointer by doing FP += <size>.
+    MachineInstr *MI = BuildMI(MBB, MBBI, DL, TII.get(Opcode), AVR::R29R28)
+                          .addReg(AVR::R29R28, RegState::Kill)
+                          .addImm(FrameSize);
+    // The SREG implicit def is dead.
+    MI->getOperand(3).setIsDead();
+  }
 
   // Write back R29R28 to SP and temporarily disable interrupts.
   BuildMI(MBB, MBBI, DL, TII.get(AVR::SPWRITE), AVR::SP)
@@ -230,7 +232,8 @@ bool AVRFrameLowering::hasFP(const MachineFunction &MF) const {
   const AVRMachineFunctionInfo *FuncInfo = MF.getInfo<AVRMachineFunctionInfo>();
 
   return (FuncInfo->getHasSpills() || FuncInfo->getHasAllocas() ||
-          FuncInfo->getHasStackArgs());
+          FuncInfo->getHasStackArgs() ||
+          MF.getFrameInfo().hasVarSizedObjects());
 }
 
 bool AVRFrameLowering::spillCalleeSavedRegisters(
@@ -480,56 +483,4 @@ char AVRFrameAnalyzer::ID = 0;
 /// Creates instance of the frame analyzer pass.
 FunctionPass *createAVRFrameAnalyzerPass() { return new AVRFrameAnalyzer(); }
 
-/// Create the Dynalloca Stack Pointer Save/Restore pass.
-/// Insert a copy of SP before allocating the dynamic stack memory and restore
-/// it in function exit to restore the original SP state. This avoids the need
-/// of reserving a register pair for a frame pointer.
-struct AVRDynAllocaSR : public MachineFunctionPass {
-  static char ID;
-  AVRDynAllocaSR() : MachineFunctionPass(ID) {}
-
-  bool runOnMachineFunction(MachineFunction &MF) override {
-    // Early exit when there are no variable sized objects in the function.
-    if (!MF.getFrameInfo().hasVarSizedObjects()) {
-      return false;
-    }
-
-    const AVRSubtarget &STI = MF.getSubtarget<AVRSubtarget>();
-    const TargetInstrInfo &TII = *STI.getInstrInfo();
-    MachineBasicBlock &EntryMBB = MF.front();
-    MachineBasicBlock::iterator MBBI = EntryMBB.begin();
-    DebugLoc DL = EntryMBB.findDebugLoc(MBBI);
-
-    Register SPCopy =
-        MF.getRegInfo().createVirtualRegister(&AVR::DREGSRegClass);
-
-    // Create a copy of SP in function entry before any dynallocas are
-    // inserted.
-    BuildMI(EntryMBB, MBBI, DL, TII.get(AVR::COPY), SPCopy).addReg(AVR::SP);
-
-    // Restore SP in all exit basic blocks.
-    for (MachineBasicBlock &MBB : MF) {
-      // If last instruction is a return instruction, add a restore copy.
-      if (!MBB.empty() && MBB.back().isReturn()) {
-        MBBI = MBB.getLastNonDebugInstr();
-        DL = MBBI->getDebugLoc();
-        BuildMI(MBB, MBBI, DL, TII.get(AVR::COPY), AVR::SP)
-            .addReg(SPCopy, RegState::Kill);
-      }
-    }
-
-    return true;
-  }
-
-  StringRef getPassName() const override {
-    return "AVR dynalloca stack pointer save/restore";
-  }
-};
-
-char AVRDynAllocaSR::ID = 0;
-
-/// createAVRDynAllocaSRPass - returns an instance of the dynalloca stack
-/// pointer save/restore pass.
-FunctionPass *createAVRDynAllocaSRPass() { return new AVRDynAllocaSR(); }
-
 } // end of namespace llvm

diff  --git a/llvm/lib/Target/AVR/AVRTargetMachine.cpp b/llvm/lib/Target/AVR/AVRTargetMachine.cpp
index 65740f7c23062..22b9ba3ece071 100644
--- a/llvm/lib/Target/AVR/AVRTargetMachine.cpp
+++ b/llvm/lib/Target/AVR/AVRTargetMachine.cpp
@@ -70,7 +70,6 @@ class AVRPassConfig : public TargetPassConfig {
   bool addInstSelector() override;
   void addPreSched2() override;
   void addPreEmitPass() override;
-  void addPreRegAlloc() override;
 };
 } // namespace
 
@@ -118,11 +117,6 @@ bool AVRPassConfig::addInstSelector() {
   return false;
 }
 
-void AVRPassConfig::addPreRegAlloc() {
-  // Create the dynalloc SP save/restore pass to handle variable sized allocas.
-  addPass(createAVRDynAllocaSRPass());
-}
-
 void AVRPassConfig::addPreSched2() {
   addPass(createAVRRelaxMemPass());
   addPass(createAVRExpandPseudoPass());

diff  --git a/llvm/test/CodeGen/AVR/dynalloca.ll b/llvm/test/CodeGen/AVR/dynalloca.ll
index c5c36ab92283f..7f69966159761 100644
--- a/llvm/test/CodeGen/AVR/dynalloca.ll
+++ b/llvm/test/CodeGen/AVR/dynalloca.ll
@@ -4,10 +4,10 @@ declare void @foo(i16*, i16*, i8*)
 
 define void @test1(i16 %x) {
 ; CHECK-LABEL: test1:
+; Frame setup, with frame pointer
+; CHECK: in r28, 61
+; CHECK: in r29, 62
 ; CHECK: out 61, r28
-; SP copy
-; CHECK-NEXT: in [[SPCOPY1:r[0-9]+]], 61
-; CHECK-NEXT: in [[SPCOPY2:r[0-9]+]], 62
 ; allocate first dynalloca
 ; CHECK: in {{.*}}, 61
 ; CHECK: in {{.*}}, 62
@@ -26,9 +26,9 @@ define void @test1(i16 %x) {
 ; Test SP restore
 ; CHECK: in r0, 63
 ; CHECK-NEXT: cli
-; CHECK-NEXT: out 62, [[SPCOPY2]]
+; CHECK-NEXT: out 62, r29
 ; CHECK-NEXT: out 63, r0
-; CHECK-NEXT: out 61, [[SPCOPY1]]
+; CHECK-NEXT: out 61, r28
   %a = alloca [8 x i16]
   %vla = alloca i16, i16 %x
   %add = shl nsw i16 %x, 1
@@ -51,8 +51,8 @@ declare void @foo2(i16*, i64, i64, i64)
 ; after the call frame is restored and not before.
 define void @dynalloca2(i16 %x) {
 ; CHECK-LABEL: dynalloca2:
-; CHECK: in [[SPCOPY1:r[0-9]+]], 61
-; CHECK: in [[SPCOPY2:r[0-9]+]], 62
+; CHECK: in r28, 61
+; CHECK: in r29, 62
 ; Allocate stack space for call
 ; CHECK: in {{.*}}, 61
 ; CHECK: in {{.*}}, 62
@@ -87,10 +87,40 @@ define void @dynalloca2(i16 %x) {
 ; SP restore
 ; CHECK: in r0, 63
 ; CHECK-NEXT: cli
-; CHECK-NEXT: out 62, r7
+; CHECK-NEXT: out 62, r29
 ; CHECK-NEXT: out 63, r0
-; CHECK-NEXT: out 61, r6
+; CHECK-NEXT: out 61, r28
   %vla = alloca i16, i16 %x
   call void @foo2(i16* %vla, i64 0, i64 0, i64 0)
   ret void
 }
+
+; Test a function with a variable sized object but without any other need for a
+; frame pointer.
+; Allocas that are not placed in the entry block are considered variable sized
+; (they could be in a loop).
+define void @dynalloca3() {
+; CHECK-LABEL: dynalloca3:
+; Read frame pointer
+; CHECK:      in r28, 61
+; CHECK-NEXT: in r29, 62
+; Allocate memory for the alloca
+; CHECK-NEXT: in r24, 61
+; CHECK-NEXT: in r25, 62
+; CHECK-NEXT: sbiw r24, 8
+; CHECK-NEXT: in r0, 63
+; CHECK-NEXT: cli
+; CHECK-NEXT: out 62, r25
+; CHECK-NEXT: out 63, r0
+; CHECK-NEXT: out 61, r24
+; Restore frame pointer
+; CHECK-NEXT: in r0, 63
+; CHECK-NEXT: cli
+; CHECK-NEXT: out 62, r29
+; CHECK-NEXT: out 63, r0
+; CHECK-NEXT: out 61, r28
+  br label %1
+1:
+  %a = alloca i64
+  ret void
+}


        


More information about the llvm-commits mailing list