[llvm] d58cc08 - [RISCV] reorganize getFrameIndexReference to reduce code duplication [nfc]

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Thu May 26 09:45:11 PDT 2022


Author: Philip Reames
Date: 2022-05-26T09:44:58-07:00
New Revision: d58cc0839ee5935e649dbbc18c898a8e4bcd6724

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

LOG: [RISCV] reorganize getFrameIndexReference to reduce code duplication [nfc]

This change reorganizes the majority of frame index resolution into a two strep process.

    Step 1 - Select which base register we're going to use.
    Step 2 - Compute the offset from that base register.

The key point is that this allows us to share the step 2 logic for the SP case. This reduces the code duplication, and (I think) makes the code much easier to follow.

I also went ahead and added assertions into phase 2 to catch errors where we select an illegal base pointer. In general, we can't index from a base register to a stack location if that requires crossing a variable and unknown region. In practice, we have two such cases: dynamic stack realign and var sized objects. Note that crossing the scalable region is fine since while variable, it's a known variability which can be expressed in the offset.

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

Added: 
    

Modified: 
    llvm/lib/Target/RISCV/RISCVFrameLowering.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp b/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
index 5062d992c6cc..8bc5f54d9148 100644
--- a/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
@@ -687,7 +687,10 @@ RISCVFrameLowering::getFrameIndexReference(const MachineFunction &MF, int FI,
       Offset += StackOffset::getFixed(FirstSPAdjustAmount);
     else
       Offset += StackOffset::getFixed(getStackSizeWithRVVPadding(MF));
-  } else if (RI->hasStackRealignment(MF) && !MFI.isFixedObjectIndex(FI)) {
+    return Offset;
+  }
+
+  if (RI->hasStackRealignment(MF) && !MFI.isFixedObjectIndex(FI)) {
     // If the stack was realigned, the frame pointer is set in order to allow
     // SP to be restored, so we need another base register to record the stack
     // after realignment.
@@ -727,97 +730,94 @@ RISCVFrameLowering::getFrameIndexReference(const MachineFunction &MF, int FI,
       assert(!MFI.hasVarSizedObjects());
       FrameReg = RISCV::X2;
     }
-    // The total amount of padding surrounding RVV objects is described by
-    // RVV->getRVVPadding() and it can be zero. It allows us to align the RVV
-    // objects to the required alignment.
-    if (MFI.getStackID(FI) == TargetStackID::Default) {
-      Offset += StackOffset::getFixed(MFI.getStackSize());
-      assert(FI >= 0 && "Unhandled negative frame index");
-    } else if (MFI.getStackID(FI) == TargetStackID::ScalableVector) {
-      // Ensure the base of the RVV stack is correctly aligned: add on the
-      // alignment padding.
-      int ScalarLocalVarSize = MFI.getStackSize() -
-                               RVFI->getCalleeSavedStackSize() +
-                               RVFI->getRVVPadding();
-      Offset += StackOffset::get(ScalarLocalVarSize, RVFI->getRVVStackSize());
-    }
   } else {
     FrameReg = RI->getFrameRegister(MF);
-    if (hasFP(MF)) {
-      Offset += StackOffset::getFixed(RVFI->getVarArgsSaveSize());
-      if (FI >= 0)
-        Offset -= StackOffset::getFixed(RVFI->getLibCallStackSize());
-      // When using FP to access scalable vector objects, we need to minus
-      // the frame size.
-      //
-      // |--------------------------| -- <-- FP
-      // | callee-allocated save    | |
-      // | area for register varargs| |
-      // |--------------------------| |
-      // | callee-saved registers   | |
-      // |--------------------------| | MFI.getStackSize()
-      // | scalar local variables   | |
-      // |--------------------------| -- (Offset of RVV objects is from here.)
-      // | RVV objects              |
-      // |--------------------------|
-      // | VarSize objects          |
-      // |--------------------------| <-- SP
-      if (MFI.getStackID(FI) == TargetStackID::ScalableVector) {
-        // We don't expect any extra RVV alignment padding, as the stack size
-        // and RVV object sections should be correct aligned in their own
-        // right.
-        assert(MFI.getStackSize() == getStackSizeWithRVVPadding(MF) &&
-               "Inconsistent stack layout");
-        Offset -= StackOffset::getFixed(MFI.getStackSize());
-      }
-    } else {
-      // When using SP to access frame objects, we need to add RVV stack size.
-      //
-      // |--------------------------| -- <-- FP
-      // | callee-allocated save    | | <----|
-      // | area for register varargs| |      |
-      // |--------------------------| |      |
-      // | callee-saved registers   | |      |
-      // |--------------------------| --     |
-      // | RVV alignment padding    | |      |
-      // | (not counted in          | |      |
-      // | MFI.getStackSize() but   | |      |
-      // | counted in               | |      |
-      // | RVFI.getRVVStackSize())  | |      |
-      // |--------------------------| --     |
-      // | RVV objects              | |      |-- MFI.getStackSize()
-      // | (not counted in          | |      |
-      // | MFI.getStackSize())      | |      |
-      // |--------------------------| --     |
-      // | padding before RVV       | |      |
-      // | (not counted in          | |      |
-      // | MFI.getStackSize())      | |      |
-      // |--------------------------| --     |
-      // | scalar local variables   | | <----'
-      // |--------------------------| -- <-- SP
-      //
-      // The total amount of padding surrounding RVV objects is described by
-      // RVV->getRVVPadding() and it can be zero. It allows us to align the RVV
-      // objects to the required alignment.
-      if (MFI.getStackID(FI) == TargetStackID::Default) {
-        if (MFI.isFixedObjectIndex(FI)) {
-          Offset += StackOffset::get(getStackSizeWithRVVPadding(MF) +
-                                         RVFI->getLibCallStackSize(),
-                                     RVFI->getRVVStackSize());
-        } else {
-          Offset += StackOffset::getFixed(MFI.getStackSize());
-        }
-      } else if (MFI.getStackID(FI) == TargetStackID::ScalableVector) {
-        // Ensure the base of the RVV stack is correctly aligned: add on the
-        // alignment padding.
-        int ScalarLocalVarSize =
-            MFI.getStackSize() - RVFI->getCalleeSavedStackSize() -
-            RVFI->getVarArgsSaveSize() + RVFI->getRVVPadding();
-        Offset += StackOffset::get(ScalarLocalVarSize, RVFI->getRVVStackSize());
-      }
+  }
+
+  if (FrameReg == getFPReg(STI)) {
+    Offset += StackOffset::getFixed(RVFI->getVarArgsSaveSize());
+    if (FI >= 0)
+      Offset -= StackOffset::getFixed(RVFI->getLibCallStackSize());
+    // When using FP to access scalable vector objects, we need to minus
+    // the frame size.
+    //
+    // |--------------------------| -- <-- FP
+    // | callee-allocated save    | |
+    // | area for register varargs| |
+    // |--------------------------| |
+    // | callee-saved registers   | |
+    // |--------------------------| | MFI.getStackSize()
+    // | scalar local variables   | |
+    // |--------------------------| -- (Offset of RVV objects is from here.)
+    // | RVV objects              |
+    // |--------------------------|
+    // | VarSize objects          |
+    // |--------------------------| <-- SP
+    if (MFI.getStackID(FI) == TargetStackID::ScalableVector) {
+      assert(!RI->hasStackRealignment(MF) &&
+             "Can't index across variable sized realign");
+      // We don't expect any extra RVV alignment padding, as the stack size
+      // and RVV object sections should be correct aligned in their own
+      // right.
+      assert(MFI.getStackSize() == getStackSizeWithRVVPadding(MF) &&
+             "Inconsistent stack layout");
+      Offset -= StackOffset::getFixed(MFI.getStackSize());
     }
+    return Offset;
   }
 
+  // This case handles indexing off both SP and BP.
+  // If indexing off SP, there must not be any var sized objects
+  assert(FrameReg == RISCVABI::getBPReg() || !MFI.hasVarSizedObjects());
+
+  // When using SP to access frame objects, we need to add RVV stack size.
+  //
+  // |--------------------------| -- <-- FP
+  // | callee-allocated save    | | <----|
+  // | area for register varargs| |      |
+  // |--------------------------| |      |
+  // | callee-saved registers   | |      |
+  // |--------------------------| --     |
+  // | RVV alignment padding    | |      |
+  // | (not counted in          | |      |
+  // | MFI.getStackSize() but   | |      |
+  // | counted in               | |      |
+  // | RVFI.getRVVStackSize())  | |      |
+  // |--------------------------| --     |
+  // | RVV objects              | |      |-- MFI.getStackSize()
+  // | (not counted in          | |      |
+  // | MFI.getStackSize())      | |      |
+  // |--------------------------| --     |
+  // | padding before RVV       | |      |
+  // | (not counted in          | |      |
+  // | MFI.getStackSize())      | |      |
+  // |--------------------------| --     |
+  // | scalar local variables   | | <----'
+  // |--------------------------| -- <-- BP (if var sized objects present)
+  // | VarSize objects          | |
+  // |--------------------------| -- <-- SP
+  //
+  // The total amount of padding surrounding RVV objects is described by
+  // RVV->getRVVPadding() and it can be zero. It allows us to align the RVV
+  // objects to the required alignment.
+  if (MFI.getStackID(FI) == TargetStackID::Default) {
+    if (MFI.isFixedObjectIndex(FI)) {
+      assert(!RI->hasStackRealignment(MF) &&
+             "Can't index across variable sized realign");
+      Offset += StackOffset::get(getStackSizeWithRVVPadding(MF) +
+                                 RVFI->getLibCallStackSize(),
+                                 RVFI->getRVVStackSize());
+    } else {
+      Offset += StackOffset::getFixed(MFI.getStackSize());
+    }
+  } else if (MFI.getStackID(FI) == TargetStackID::ScalableVector) {
+    // Ensure the base of the RVV stack is correctly aligned: add on the
+    // alignment padding.
+    int ScalarLocalVarSize =
+      MFI.getStackSize() - RVFI->getCalleeSavedStackSize() -
+      RVFI->getVarArgsSaveSize() + RVFI->getRVVPadding();
+    Offset += StackOffset::get(ScalarLocalVarSize, RVFI->getRVVStackSize());
+  }
   return Offset;
 }
 


        


More information about the llvm-commits mailing list