[RISCV] Add a scavenge spill slot when use ADDI to compute scalable stack offset

[StephenFan]

Summary

Computing scalable offset needs up to two scrach registers. We add
scavenge spill slots according to the result of RISCV::isRVVSpill
and RVVStackSize. Since ADDI is not included in RISCV::isRVVSpill,
PEI doesn't add scavenge spill slots for scrach registers when using
ADDI to get scalable stack offsets.

The ADDI instruction has a destination register which can be used as
a scrach register. So one scavenge spil slot is sufficient for
computing scalable stack offsets.

[StephenFan]

Rename some variables.

[craig.topper]

Is it possible to add a test where we use the 2 slots for spills?

llvm/lib/Target/RISCV/RISCVFrameLowering.cpp

978

Why not use std::max here? Why separate local/global variable?

986

Should we stop searching when GlobalMostScavSlotsNum becomes 2?

[StephenFan]

Add a mir test

[StephenFan]

1. Early return if GlobalMostScavSlotsNum reaches the GlobalMostScavSlotsNumKnown
2. Remove LocalMostScavSlotsNum

[StephenFan]

In D128188#3599342, @craig.topper wrote:

Is it possible to add a test where we use the 2 slots for spills?

I added a mir test since it seems that .ll test can't reflect the number of scavenging slots very well.

llvm/lib/Target/RISCV/RISCVFrameLowering.cpp

978

You are right, the local variable is redundant and I should use std::max here

986

Yes.

[craig.topper]

llvm/lib/Target/RISCV/RISCVFrameLowering.cpp

968

Variable names should be capitalized

972

Variable names should be capitalized

[StephenFan]

Capitalize variable names.

[craig.topper]

llvm/lib/Target/RISCV/RISCVFrameLowering.cpp

975

Drop "Global". Without Local it doesn't mean anything.

Replace "Most" with "Max"

llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp

177

Is IsRVVSpill used anywhere other than the following if? If not can we call RICV::isRVVSpill as part of the if and remove the variable.

[StephenFan]

1. Drop global.
2. Remove local variable IsRVVSpill
3. Replace 'Most' with 'Max'

[StephenFan]

llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp

177

It is used on lines 262 and 272.

[StephenFan]

Undo changes.

[craig.topper]

LGTM

[frasercrmck]

llvm/test/CodeGen/RISCV/rvv/rvv-stack-align.mir

125–128

What's causing changes in these tests?

[craig.topper]

llvm/test/CodeGen/RISCV/rvv/rvv-stack-align.mir

125–128

We're allocating an emergency spill slot because there is an ADDI frame index reference to a scalable slot. We weren't considering ADDI before.

llvm/lib/Target/RISCV/RISCVFrameLowering.cpp

 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/CodeGen/RegisterScavenging.h"
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/MC/MCDwarf.h"
 
+#include <algorithm>
+
 using namespace llvm;
 
 // For now we use x18, a.k.a s2, as pointer to shadow call stack.
 // User should explicitly set -ffixed-x18 and not use x18 in their asm.
 static void emitSCSPrologue(MachineFunction &MF, MachineBasicBlock &MBB,
                             MachineBasicBlock::iterator MI,
                             const DebugLoc &DL) {
   if (!MF.getFunction().hasFnAttribute(Attribute::ShadowCallStack))
if (auto AlignmentPadding = offsetToAlignment(StackSize, RVVStackAlign)) {
     StackSize += AlignmentPadding;
     for (int FI : ObjectsToAllocate)
       MFI.setObjectOffset(FI, MFI.getObjectOffset(FI) - AlignmentPadding);
   }
 
   return std::make_pair(StackSize, RVVStackAlign);
 }
 
-static bool hasRVVSpillWithFIs(MachineFunction &MF) {
+static unsigned getScavSlotsNumForRVV(MachineFunction &MF) {
+  // For RVV spill, scalable stack offsets computing requires up to two scratch
+  // registers
+  static constexpr unsigned ScavSlotsNumRVVSpillScalableObject = 2;
+
+  // For RVV spill, non-scalable stack offsets computing requires up to one
+  // scratch register.
+  static constexpr unsigned ScavSlotsNumRVVSpillNonScalableObject = 1;
+
+  // ADDI instruction's destination register can be used for computing
+  // offsets. So Scalable stack offsets require up to one scratch register.
+  static constexpr unsigned ScavSlotsADDIScalableObject = 1;
+
+  static constexpr unsigned MaxScavSlotsNumKnown =
+      std::max({ScavSlotsADDIScalableObject, ScavSlotsNumRVVSpillScalableObject,
+                ScavSlotsNumRVVSpillNonScalableObject});
+
+  unsigned MaxScavSlotsNum = 0;
   if (!MF.getSubtarget<RISCVSubtarget>().hasVInstructions())
     return false;
   for (const MachineBasicBlock &MBB : MF)
-    for (const MachineInstr &MI : MBB)
-      if (RISCV::isRVVSpill(MI, /*CheckFIs*/ true))
-        return true;
-  return false;
+    for (const MachineInstr &MI : MBB) {
+      bool IsRVVSpill = RISCV::isRVVSpill(MI);
+      for (auto &MO : MI.operands()) {
+        if (!MO.isFI())
+          continue;
+        bool IsScalableVectorID = MF.getFrameInfo().getStackID(MO.getIndex()) ==
+                                  TargetStackID::ScalableVector;
+        if (IsRVVSpill) {
+          MaxScavSlotsNum = std::max(
+              MaxScavSlotsNum, IsScalableVectorID
+                                   ? ScavSlotsNumRVVSpillScalableObject
+                                   : ScavSlotsNumRVVSpillNonScalableObject);
+        } else if (MI.getOpcode() == RISCV::ADDI && IsScalableVectorID) {
+          MaxScavSlotsNum =
+              std::max(MaxScavSlotsNum, ScavSlotsADDIScalableObject);
+        }
+      }
+      if (MaxScavSlotsNum == MaxScavSlotsNumKnown)
+        return MaxScavSlotsNumKnown;
+    }
+  return MaxScavSlotsNum;
 }
 
 void RISCVFrameLowering::processFunctionBeforeFrameFinalized(
     MachineFunction &MF, RegScavenger *RS) const {
   const RISCVRegisterInfo *RegInfo =
       MF.getSubtarget<RISCVSubtarget>().getRegisterInfo();
   MachineFrameInfo &MFI = MF.getFrameInfo();
   const TargetRegisterClass *RC = &RISCV::GPRRegClass;
void RISCVFrameLowering::processFunctionBeforeFrameFinalized(
   // size, so give ourselves wiggle-room by checking for stack size
   // representable an 11-bit signed field rather than 12-bits.
   // FIXME: It may be possible to craft a function with a small stack that
   // still needs an emergency spill slot for branch relaxation. This case
   // would currently be missed.
   // RVV loads & stores have no capacity to hold the immediate address offsets
   // so we must always reserve an emergency spill slot if the MachineFunction
   // contains any RVV spills.
-  if (!isInt<11>(MFI.estimateStackSize(MF)) || hasRVVSpillWithFIs(MF)) {
-    int RegScavFI = MFI.CreateStackObject(RegInfo->getSpillSize(*RC),
-                                          RegInfo->getSpillAlign(*RC), false);
-    RS->addScavengingFrameIndex(RegScavFI);
-    // For RVV, scalable stack offsets require up to two scratch registers to
-    // compute the final offset. Reserve an additional emergency spill slot.
-    if (RVVStackSize != 0) {
-      int RVVRegScavFI = MFI.CreateStackObject(
-          RegInfo->getSpillSize(*RC), RegInfo->getSpillAlign(*RC), false);
-      RS->addScavengingFrameIndex(RVVRegScavFI);
-    }
+  unsigned ScavSlotsNum = 0;
+  if (!isInt<11>(MFI.estimateStackSize(MF)))
+    ScavSlotsNum = 1;
+
+  ScavSlotsNum = std::max(ScavSlotsNum, getScavSlotsNumForRVV(MF));
+  for (unsigned i = 0; i < ScavSlotsNum; i++) {
+    RS->addScavengingFrameIndex(MFI.CreateStackObject(
+        RegInfo->getSpillSize(*RC), RegInfo->getSpillAlign(*RC), false));
   }
 
   if (MFI.getCalleeSavedInfo().empty() || RVFI->useSaveRestoreLibCalls(MF)) {
     RVFI->setCalleeSavedStackSize(0);
     return;
   }
 
   unsigned Size = 0;

llvm/lib/Target/RISCV/RISCVInstrInfo.h

 
 protected:
   const RISCVSubtarget &STI;
 };
 
 namespace RISCV {
 
 // Returns true if the given MI is an RVV instruction opcode for which we may
-// expect to see a FrameIndex operand. When CheckFIs is true, the instruction
-// must contain at least one FrameIndex operand.
-bool isRVVSpill(const MachineInstr &MI, bool CheckFIs);
+// expect to see a FrameIndex operand.
+bool isRVVSpill(const MachineInstr &MI);
 
 Optional<std::pair<unsigned, unsigned>> isRVVSpillForZvlsseg(unsigned Opcode);
 
 bool isFaultFirstLoad(const MachineInstr &MI);
 
 // Implemented in RISCVGenInstrInfo.inc
 int16_t getNamedOperandIdx(uint16_t Opcode, uint16_t NamedIndex);
 

llvm/lib/Target/RISCV/RISCVInstrInfo.cpp

static bool isRVVWholeLoadStore(unsigned Opcode) {
   case RISCV::VL1RE64_V:
   case RISCV::VL2RE64_V:
   case RISCV::VL4RE64_V:
   case RISCV::VL8RE64_V:
     return true;
   }
 }
 
-bool RISCV::isRVVSpill(const MachineInstr &MI, bool CheckFIs) {
+bool RISCV::isRVVSpill(const MachineInstr &MI) {
   // RVV lacks any support for immediate addressing for stack addresses, so be
   // conservative.
   unsigned Opcode = MI.getOpcode();
   if (!RISCVVPseudosTable::getPseudoInfo(Opcode) &&
       !isRVVWholeLoadStore(Opcode) && !isRVVSpillForZvlsseg(Opcode))
     return false;
-  return !CheckFIs || any_of(MI.operands(), [](const MachineOperand &MO) {
-    return MO.isFI();
-  });
+  return true;
 }
 
 Optional<std::pair<unsigned, unsigned>>
 RISCV::isRVVSpillForZvlsseg(unsigned Opcode) {
   switch (Opcode) {
   default:
     return None;
   case RISCV::PseudoVSPILL2_M1:

llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp

void RISCVRegisterInfo::eliminateFrameIndex(MachineBasicBlock::iterator II,
   MachineRegisterInfo &MRI = MF.getRegInfo();
   const RISCVInstrInfo *TII = MF.getSubtarget<RISCVSubtarget>().getInstrInfo();
   DebugLoc DL = MI.getDebugLoc();
 
   int FrameIndex = MI.getOperand(FIOperandNum).getIndex();
   Register FrameReg;
   StackOffset Offset =
       getFrameLowering(MF)->getFrameIndexReference(MF, FrameIndex, FrameReg);
-  bool IsRVVSpill = RISCV::isRVVSpill(MI, /*CheckFIs*/ false);
+  bool IsRVVSpill = RISCV::isRVVSpill(MI);
   if (!IsRVVSpill)
     Offset += StackOffset::getFixed(MI.getOperand(FIOperandNum + 1).getImm());
 
   if (!isInt<32>(Offset.getFixed())) {
     report_fatal_error(
         "Frame offsets outside of the signed 32-bit range not supported");
   }
 

llvm/test/CodeGen/RISCV/rvv/addi-rvv-stack-object.mir

+# RUN: llc -mtriple=riscv64 -mattr=+v -run-pass prologepilog %s -o - | FileCheck %s
+--- |
+  declare void @extern(<vscale x 16 x i8>*) #0
+  
+  define void @addi_rvv_stack_object() #0 {
+    %local0 = alloca <vscale x 16 x i8>, align 16
+    call void @extern(<vscale x 16 x i8>* %local0)
+    ret void
+  }
+  
+  attributes #0 = { "target-features"="+v" }
+
+...
+---
+name:            addi_rvv_stack_object
+alignment:       4
+frameInfo:
+  isFrameAddressTaken: false
+  isReturnAddressTaken: false
+  hasStackMap:     false
+  hasPatchPoint:   false
+  stackSize:       0
+  offsetAdjustment: 0
+  maxAlignment:    16
+  adjustsStack:    false
+  hasCalls:        true
+  stackProtector:  ''
+  functionContext: ''
+  maxCallFrameSize: 4294967295
+  cvBytesOfCalleeSavedRegisters: 0
+  hasOpaqueSPAdjustment: false
+  hasVAStart:      false
+  hasMustTailInVarArgFunc: false
+  hasTailCall:     false
+  localFrameSize:  0
+  savePoint:       ''
+  restorePoint:    ''
+fixedStack:      []
+stack:
+  - { id: 0, name: local0, type: default, offset: 0, size: 16, alignment: 16, 
+      stack-id: scalable-vector, callee-saved-register: '', callee-saved-restored: true, 
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+# CHECK: - { id: 2, name: '', type: default, offset: -16, size: 8, alignment: 8,
+# CHECK:     stack-id: default, callee-saved-register: '', callee-saved-restored: true,
+# CHECK:     debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+callSites:       []
+debugValueSubstitutions: []
+constants:       []
+machineFunctionInfo:
+  varArgsFrameIndex: 0
+  varArgsSaveSize: 0
+body:             |
+  bb.0 (%ir-block.0):
+    ADJCALLSTACKDOWN 0, 0, implicit-def dead $x2, implicit $x2
+    $x10 = ADDI %stack.0.local0, 0
+    PseudoCALL target-flags(riscv-plt) @extern, csr_ilp32d_lp64d, implicit-def dead $x1, implicit killed $x10, implicit-def $x2
+    ADJCALLSTACKUP 0, 0, implicit-def dead $x2, implicit $x2
+    PseudoRET
+
+...

llvm/test/CodeGen/RISCV/rvv/rvv-stack-align.mir

--- |
   ; RV32-NEXT:    addi sp, s0, -48
   ; RV32-NEXT:    lw ra, 44(sp) # 4-byte Folded Reload
   ; RV32-NEXT:    lw s0, 40(sp) # 4-byte Folded Reload
   ; RV32-NEXT:    addi sp, sp, 48
   ; RV32-NEXT:    ret
   ;
   ; RV64-LABEL: rvv_stack_align32:
   ; RV64:       # %bb.0:
-  ; RV64-NEXT:    addi sp, sp, -48
-  ; RV64-NEXT:    sd ra, 40(sp) # 8-byte Folded Spill
-  ; RV64-NEXT:    sd s0, 32(sp) # 8-byte Folded Spill
-  ; RV64-NEXT:    addi s0, sp, 48
+  ; RV64-NEXT:    addi sp, sp, -80
+  ; RV64-NEXT:    sd ra, 72(sp) # 8-byte Folded Spill
+  ; RV64-NEXT:    sd s0, 64(sp) # 8-byte Folded Spill
+  ; RV64-NEXT:    addi s0, sp, 80
   ; RV64-NEXT:    csrr a0, vlenb
   ; RV64-NEXT:    slli a0, a0, 2
   ; RV64-NEXT:    sub sp, sp, a0
   ; RV64-NEXT:    andi sp, sp, -32
-  ; RV64-NEXT:    addi a0, sp, 32
-  ; RV64-NEXT:    addi a1, sp, 8
-  ; RV64-NEXT:    mv a2, sp
+  ; RV64-NEXT:    addi a0, sp, 64
+  ; RV64-NEXT:    addi a1, sp, 40
+  ; RV64-NEXT:    addi a2, sp, 32
   ; RV64-NEXT:    call extern@plt
-  ; RV64-NEXT:    addi sp, s0, -48
-  ; RV64-NEXT:    ld ra, 40(sp) # 8-byte Folded Reload
-  ; RV64-NEXT:    ld s0, 32(sp) # 8-byte Folded Reload
-  ; RV64-NEXT:    addi sp, sp, 48
+  ; RV64-NEXT:    addi sp, s0, -80
+  ; RV64-NEXT:    ld ra, 72(sp) # 8-byte Folded Reload
+  ; RV64-NEXT:    ld s0, 64(sp) # 8-byte Folded Reload
+  ; RV64-NEXT:    addi sp, sp, 80
   ; RV64-NEXT:    ret
     %a = alloca <vscale x 4 x i32>, align 32
     %b = alloca i64
     %c = alloca i64
     call void @extern(<vscale x 4 x i32>* %a)
     ret void
   }
 

llvm/test/CodeGen/RISCV/rvv/scalar-stack-align.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple=riscv32 -mattr=+zve64x -verify-machineinstrs < %s \
 ; RUN:   | FileCheck %s --check-prefix=RV32
 ; RUN: llc -mtriple=riscv64 -mattr=+zve64x -verify-machineinstrs < %s \
 ; RUN:   | FileCheck %s --check-prefix=RV64
 ; RUN: llc -mtriple=riscv32 -mattr=+v -verify-machineinstrs < %s \
 ; RUN:   | FileCheck %s --check-prefix=RV32
 ; RUN: llc -mtriple=riscv64 -mattr=+v -verify-machineinstrs < %s \
 ; RUN:   | FileCheck %s --check-prefix=RV64
 
 ; FIXME: We are over-aligning the stack on V, wasting stack space.
 
 define i64* @scalar_stack_align16() nounwind {
 ; RV32-LABEL: scalar_stack_align16:
 ; RV32:       # %bb.0:
-; RV32-NEXT:    addi sp, sp, -32
-; RV32-NEXT:    sw ra, 28(sp) # 4-byte Folded Spill
+; RV32-NEXT:    addi sp, sp, -48
+; RV32-NEXT:    sw ra, 44(sp) # 4-byte Folded Spill
 ; RV32-NEXT:    csrr a0, vlenb
 ; RV32-NEXT:    slli a0, a0, 1
 ; RV32-NEXT:    sub sp, sp, a0
-; RV32-NEXT:    addi a0, sp, 16
+; RV32-NEXT:    addi a0, sp, 32
 ; RV32-NEXT:    call extern@plt
-; RV32-NEXT:    mv a0, sp
+; RV32-NEXT:    addi a0, sp, 16
 ; RV32-NEXT:    csrr a1, vlenb
 ; RV32-NEXT:    slli a1, a1, 1
 ; RV32-NEXT:    add sp, sp, a1
-; RV32-NEXT:    lw ra, 28(sp) # 4-byte Folded Reload
-; RV32-NEXT:    addi sp, sp, 32
+; RV32-NEXT:    lw ra, 44(sp) # 4-byte Folded Reload
+; RV32-NEXT:    addi sp, sp, 48
 ; RV32-NEXT:    ret
 ;
 ; RV64-LABEL: scalar_stack_align16:
 ; RV64:       # %bb.0:
-; RV64-NEXT:    addi sp, sp, -32
-; RV64-NEXT:    sd ra, 24(sp) # 8-byte Folded Spill
+; RV64-NEXT:    addi sp, sp, -48
+; RV64-NEXT:    sd ra, 40(sp) # 8-byte Folded Spill
 ; RV64-NEXT:    csrr a0, vlenb
 ; RV64-NEXT:    slli a0, a0, 1
 ; RV64-NEXT:    sub sp, sp, a0
-; RV64-NEXT:    addi a0, sp, 16
+; RV64-NEXT:    addi a0, sp, 32
 ; RV64-NEXT:    call extern@plt
-; RV64-NEXT:    mv a0, sp
+; RV64-NEXT:    addi a0, sp, 16
 ; RV64-NEXT:    csrr a1, vlenb
 ; RV64-NEXT:    slli a1, a1, 1
 ; RV64-NEXT:    add sp, sp, a1
-; RV64-NEXT:    ld ra, 24(sp) # 8-byte Folded Reload
-; RV64-NEXT:    addi sp, sp, 32
+; RV64-NEXT:    ld ra, 40(sp) # 8-byte Folded Reload
+; RV64-NEXT:    addi sp, sp, 48
 ; RV64-NEXT:    ret
   %a = alloca <vscale x 2 x i32>
   %c = alloca i64, align 16
   call void @extern(<vscale x 2 x i32>* %a)
   ret i64* %c
 }
 
 declare void @extern(<vscale x 2 x i32>*)