MachineFrameInfo: Make StackSize Optional.

[arsenm]

Summary

Currently it's easy to make mistakes by checking getStackSize() == 0
when this function is called before frame finalization. Make users
have to explicitly acknowledge this problem.

Another option would be to default to -1, so before the frame size is
computed things will conservatively assume there is stack usage.

Just omits it from the MIR form of MFI if missing, although maybe an
explicit unknown marker might be helpful. To fix crashing in some MIR
testcases, it would help to have StackFrameFinalized property.

include/llvm/CodeGen/MIRYamlMapping.h

 /// attributes.
 /// It also doesn't serialize attributes like 'NumFixedObject' and
 /// 'HasVarSizedObjects' as they are determined by the frame objects themselves.
 struct MachineFrameInfo {
   bool IsFrameAddressTaken = false;
   bool IsReturnAddressTaken = false;
   bool HasStackMap = false;
   bool HasPatchPoint = false;
-  uint64_t StackSize = 0;
+  Optional<uint64_t> StackSize;
   int OffsetAdjustment = 0;
   unsigned MaxAlignment = 0;
   bool AdjustsStack = false;
   bool HasCalls = false;
   StringValue StackProtector;
   // TODO: Serialize FunctionContextIdx
   unsigned MaxCallFrameSize = ~0u; ///< ~0u means: not computed yet.
   unsigned CVBytesOfCalleeSavedRegisters = 0;

 };
 
 template <> struct MappingTraits<MachineFrameInfo> {
   static void mapping(IO &YamlIO, MachineFrameInfo &MFI) {
     YamlIO.mapOptional("isFrameAddressTaken", MFI.IsFrameAddressTaken, false);
     YamlIO.mapOptional("isReturnAddressTaken", MFI.IsReturnAddressTaken, false);
     YamlIO.mapOptional("hasStackMap", MFI.HasStackMap, false);
     YamlIO.mapOptional("hasPatchPoint", MFI.HasPatchPoint, false);
-    YamlIO.mapOptional("stackSize", MFI.StackSize, (uint64_t)0);
+    YamlIO.mapOptional("stackSize", MFI.StackSize);
     YamlIO.mapOptional("offsetAdjustment", MFI.OffsetAdjustment, (int)0);
     YamlIO.mapOptional("maxAlignment", MFI.MaxAlignment, (unsigned)0);
     YamlIO.mapOptional("adjustsStack", MFI.AdjustsStack, false);
     YamlIO.mapOptional("hasCalls", MFI.HasCalls, false);
     YamlIO.mapOptional("stackProtector", MFI.StackProtector,
                        StringValue()); // Don't print it out when it's empty.
     YamlIO.mapOptional("maxCallFrameSize", MFI.MaxCallFrameSize, (unsigned)~0);
     YamlIO.mapOptional("cvBytesOfCalleeSavedRegisters",

include/llvm/CodeGen/MachineFrameInfo.h

 //===-- CodeGen/MachineFrameInfo.h - Abstract Stack Frame Rep. --*- C++ -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 //
 // The file defines the MachineFrameInfo class.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CODEGEN_MACHINEFRAMEINFO_H
 #define LLVM_CODEGEN_MACHINEFRAMEINFO_H
 
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/DataTypes.h"
 #include <cassert>
 #include <vector>
 
 namespace llvm {
 class raw_ostream;
 class MachineFunction;
private:
   /// This boolean keeps track of whether there is a call
   /// to builtin \@llvm.experimental.patchpoint.
   bool HasPatchPoint = false;
 
   /// The prolog/epilog code inserter calculates the final stack
   /// offsets for all of the fixed size objects, updating the Objects list
   /// above.  It then updates StackSize to contain the number of bytes that need
   /// to be allocated on entry to the function.
-  uint64_t StackSize = 0;
+  Optional<uint64_t> StackSize;
 
   /// The amount that a frame offset needs to be adjusted to
   /// have the actual offset from the stack/frame pointer.  The exact usage of
   /// this is target-dependent, but it is typically used to adjust between
   /// SP-relative and FP-relative offsets.  E.G., if objects are accessed via
   /// SP then OffsetAdjustment is zero; if FP is used, OffsetAdjustment is set
   /// to the distance between the initial SP and the value in FP.  For many
   /// targets, this value is only used when generating debug info (via
void setObjectSSPLayout(int ObjectIdx, SSPLayoutKind Kind) {
     assert(!isDeadObjectIndex(ObjectIdx) &&
            "Setting SSP layout for a dead object?");
     Objects[ObjectIdx+NumFixedObjects].SSPLayout = Kind;
   }
 
   /// Return the number of bytes that must be allocated to hold
   /// all of the fixed size frame objects.  This is only valid after
   /// Prolog/Epilog code insertion has finalized the stack frame layout.
-  uint64_t getStackSize() const { return StackSize; }
+  uint64_t getStackSize() const { return StackSize.getValue(); }
+
+  /// Return if the stack layout is finalized.
+  bool stackSizeIsKnown() const { return StackSize.hasValue(); }
 
   /// Set the size of the stack.
   void setStackSize(uint64_t Size) { StackSize = Size; }
 
   /// Estimate and return the size of the stack frame.
   unsigned estimateStackSize(const MachineFunction &MF) const;
 
   /// Return the correction for frame offsets.

lib/CodeGen/MIRParser/MIRParser.cpp

bool MIRParserImpl::initializeFrameInfo(PerFunctionMIParsingState &PFS,
   MachineFrameInfo &MFI = MF.getFrameInfo();
   const TargetFrameLowering *TFI = MF.getSubtarget().getFrameLowering();
   const Function &F = MF.getFunction();
   const yaml::MachineFrameInfo &YamlMFI = YamlMF.FrameInfo;
   MFI.setFrameAddressIsTaken(YamlMFI.IsFrameAddressTaken);
   MFI.setReturnAddressIsTaken(YamlMFI.IsReturnAddressTaken);
   MFI.setHasStackMap(YamlMFI.HasStackMap);
   MFI.setHasPatchPoint(YamlMFI.HasPatchPoint);
-  MFI.setStackSize(YamlMFI.StackSize);
+  if (YamlMFI.StackSize.hasValue())
+    MFI.setStackSize(YamlMFI.StackSize.getValue());
   MFI.setOffsetAdjustment(YamlMFI.OffsetAdjustment);
   if (YamlMFI.MaxAlignment)
     MFI.ensureMaxAlignment(YamlMFI.MaxAlignment);
   MFI.setAdjustsStack(YamlMFI.AdjustsStack);
   MFI.setHasCalls(YamlMFI.HasCalls);
   if (YamlMFI.MaxCallFrameSize != ~0u)
     MFI.setMaxCallFrameSize(YamlMFI.MaxCallFrameSize);
   MFI.setCVBytesOfCalleeSavedRegisters(YamlMFI.CVBytesOfCalleeSavedRegisters);

lib/CodeGen/MIRPrinter.cpp

 
 void MIRPrinter::convert(ModuleSlotTracker &MST,
                          yaml::MachineFrameInfo &YamlMFI,
                          const MachineFrameInfo &MFI) {
   YamlMFI.IsFrameAddressTaken = MFI.isFrameAddressTaken();
   YamlMFI.IsReturnAddressTaken = MFI.isReturnAddressTaken();
   YamlMFI.HasStackMap = MFI.hasStackMap();
   YamlMFI.HasPatchPoint = MFI.hasPatchPoint();
+  if (MFI.stackSizeIsKnown())
     YamlMFI.StackSize = MFI.getStackSize();
   YamlMFI.OffsetAdjustment = MFI.getOffsetAdjustment();
   YamlMFI.MaxAlignment = MFI.getMaxAlignment();
   YamlMFI.AdjustsStack = MFI.adjustsStack();
   YamlMFI.HasCalls = MFI.hasCalls();
   YamlMFI.MaxCallFrameSize = MFI.isMaxCallFrameSizeComputed()
     ? MFI.getMaxCallFrameSize() : ~0u;
   YamlMFI.CVBytesOfCalleeSavedRegisters =
       MFI.getCVBytesOfCalleeSavedRegisters();

lib/CodeGen/RegisterScavenging.cpp

void llvm::scavengeFrameVirtualRegs(MachineFunction &MF, RegScavenger &RS) {
   }
 
   MRI.clearVirtRegs();
   MF.getProperties().set(MachineFunctionProperties::Property::NoVRegs);
 }
 
 namespace {
 
-/// This class runs register scavenging independ of the PrologEpilogInserter.
+/// This class runs register scavenging independent of the PrologEpilogInserter.
 /// This is used in for testing.
 class ScavengerTest : public MachineFunctionPass {
 public:
   static char ID;
 
   ScavengerTest() : MachineFunctionPass(ID) {}
 
   bool runOnMachineFunction(MachineFunction &MF) override {
     const TargetSubtargetInfo &STI = MF.getSubtarget();
     const TargetFrameLowering &TFL = *STI.getFrameLowering();
 
     RegScavenger RS;
     // Let's hope that calling those outside of PrologEpilogueInserter works
     // well enough to initialize the scavenger with some emergency spillslots
     // for the target.
     BitVector SavedRegs;
     TFL.determineCalleeSaves(MF, SavedRegs, &RS);
     TFL.processFunctionBeforeFrameFinalized(MF, &RS);
 
+    // FIXME: calculateFrameObjectOffsets not exposed here.
+    MF.getFrameInfo().setStackSize(0);
+
     // Let's scavenge the current function
     scavengeFrameVirtualRegs(MF, RS);
     return true;
   }
 };
 
 } // end anonymous namespace
 
 char ScavengerTest::ID;
 
 INITIALIZE_PASS(ScavengerTest, "scavenger-test",
                 "Scavenge virtual registers inside basic blocks", false, false)

lib/Target/AMDGPU/SIFrameLowering.cpp

MachineBasicBlock::iterator SIFrameLowering::eliminateCallFramePseudoInstr(
   return MBB.erase(I);
 }
 
 bool SIFrameLowering::hasFP(const MachineFunction &MF) const {
   const MachineFrameInfo &MFI = MF.getFrameInfo();
   if (MFI.hasCalls()) {
     // All offsets are unsigned, so need to be addressed in the same direction
     // as stack growth.
-    if (MFI.getStackSize() != 0)
+    if (MFI.stackSizeIsKnown() && MFI.getStackSize() != 0)
       return true;
 
     // For the entry point, the input wave scratch offset must be copied to the
     // API SP if there are calls.
     if (MF.getInfo<SIMachineFunctionInfo>()->isEntryFunction())
       return true;
   }
 
   return MFI.hasVarSizedObjects() || MFI.isFrameAddressTaken() ||
     MFI.hasStackMap() || MFI.hasPatchPoint() ||
     MF.getSubtarget<GCNSubtarget>().getRegisterInfo()->needsStackRealignment(MF) ||
     MF.getTarget().Options.DisableFramePointerElim(MF);
 }

lib/Target/Hexagon/HexagonFrameLowering.cpp

static bool enableAllocFrameElim(const MachineFunction &MF) {
   const auto &F = MF.getFunction();
   const auto &MFI = MF.getFrameInfo();
   const auto &HST = MF.getSubtarget<HexagonSubtarget>();
   assert(!MFI.hasVarSizedObjects() &&
          !HST.getRegisterInfo()->needsStackRealignment(MF));
   return F.hasFnAttribute(Attribute::NoReturn) &&
     F.hasFnAttribute(Attribute::NoUnwind) &&
     !F.hasFnAttribute(Attribute::UWTable) && HST.noreturnStackElim() &&
+    MFI.stackSizeIsKnown() &&
     MFI.getStackSize() == 0;
 }
 
 void HexagonFrameLowering::insertPrologueInBlock(MachineBasicBlock &MBB,
       bool PrologueStubs) const {
   MachineFunction &MF = *MBB.getParent();
   MachineFrameInfo &MFI = MF.getFrameInfo();
   auto &HST = MF.getSubtarget<HexagonSubtarget>();
bool HexagonFrameLowering::hasFP(const MachineFunction &MF) const {
   // By default we want to use SP (since it's always there). FP requires
   // some setup (i.e. ALLOCFRAME).
   // Both, alloca and stack alignment modify the stack pointer by an
   // undetermined value, so we need to save it at the entry to the function
   // (i.e. use allocframe).
   if (HasAlloca || HasExtraAlign)
     return true;
 
-  if (MFI.getStackSize() > 0) {
+  if (MFI.stackSizeIsKnown() && MFI.getStackSize() > 0) {
     // If FP-elimination is disabled, we have to use FP at this point.
     const TargetMachine &TM = MF.getTarget();
     if (TM.Options.DisableFramePointerElim(MF) || !EliminateFramePointer)
       return true;
     if (EnableStackOVFSanitizer)
       return true;
   }
 

lib/Target/PowerPC/PPCFrameLowering.cpp

 /// frame size.
 unsigned
 PPCFrameLowering::determineFrameLayout(const MachineFunction &MF,
                                        bool UseEstimate,
                                        unsigned *NewMaxCallFrameSize) const {
   const MachineFrameInfo &MFI = MF.getFrameInfo();
 
   // Get the number of bytes to allocate from the FrameInfo
-  unsigned FrameSize =
-    UseEstimate ? MFI.estimateStackSize(MF) : MFI.getStackSize();
+  unsigned FrameSize = (UseEstimate || !MFI.stackSizeIsKnown()) ?
+                        MFI.estimateStackSize(MF) : MFI.getStackSize();
 
   // Get stack alignments. The frame must be aligned to the greatest of these:
   unsigned TargetAlign = getStackAlignment(); // alignment required per the ABI
   unsigned MaxAlign = MFI.getMaxAlignment(); // algmt required by data in frame
   unsigned AlignMask = std::max(MaxAlign, TargetAlign) - 1;
 
   const PPCRegisterInfo *RegInfo = Subtarget.getRegisterInfo();
 

 
 // hasFP - Return true if the specified function actually has a dedicated frame
 // pointer register.
 bool PPCFrameLowering::hasFP(const MachineFunction &MF) const {
   const MachineFrameInfo &MFI = MF.getFrameInfo();
   // FIXME: This is pretty much broken by design: hasFP() might be called really
   // early, before the stack layout was calculated and thus hasFP() might return
   // true or false here depending on the time of call.
-  return (MFI.getStackSize()) && needsFP(MF);
+  return (MFI.stackSizeIsKnown() && MFI.getStackSize()) && needsFP(MF);
 }
 
 // needsFP - Return true if the specified function should have a dedicated frame
 // pointer register.  This is true if the function has variable sized allocas or
 // if frame pointer elimination is disabled.
 bool PPCFrameLowering::needsFP(const MachineFunction &MF) const {
   const MachineFrameInfo &MFI = MF.getFrameInfo();
 

lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp

bool WebAssemblyFrameLowering::hasFP(const MachineFunction &MF) const {
   const MachineFrameInfo &MFI = MF.getFrameInfo();
 
   // When we have var-sized objects, we move the stack pointer by an unknown
   // amount, and need to emit a frame pointer to restore the stack to where we
   // were on function entry.
   // If we already need a base pointer, we use that to fix up the stack pointer.
   // If there are no fixed-size objects, we would have no use of a frame
   // pointer, and thus should not emit one.
-  bool HasFixedSizedObjects = MFI.getStackSize() > 0;
+  bool HasFixedSizedObjects = MFI.stackSizeIsKnown() && MFI.getStackSize() > 0;
   bool NeedsFixedReference = !hasBP(MF) || HasFixedSizedObjects;
 
   return MFI.isFrameAddressTaken() ||
          (MFI.hasVarSizedObjects() && NeedsFixedReference) ||
          MFI.hasStackMap() || MFI.hasPatchPoint();
 }
 
 /// Under normal circumstances, when a frame pointer is not required, we reserve
 /// argument space for call sites in the function immediately on entry to the
 /// current function. This eliminates the need for add/sub sp brackets around
 /// call sites. Returns true if the call frame is included as part of the stack
 /// frame.
 bool WebAssemblyFrameLowering::hasReservedCallFrame(
     const MachineFunction &MF) const {
   return !MF.getFrameInfo().hasVarSizedObjects();
 }
 
 // Returns true if this function needs a local user-space stack pointer for its
 // local frame (not for exception handling).
 bool WebAssemblyFrameLowering::needsSPForLocalFrame(
     const MachineFunction &MF) const {
   auto &MFI = MF.getFrameInfo();
-  return MFI.getStackSize() || MFI.adjustsStack() || hasFP(MF);
+  return (MFI.stackSizeIsKnown() && MFI.getStackSize()) ||
+          MFI.adjustsStack() || hasFP(MF);
 }
 
 // In function with EH pads, we need to make a copy of the value of
 // __stack_pointer global in SP32 register, in order to use it when restoring
 // __stack_pointer after an exception is caught.
 bool WebAssemblyFrameLowering::needsPrologForEH(
     const MachineFunction &MF) const {
   auto EHType = MF.getTarget().getMCAsmInfo()->getExceptionHandlingType();
bool WebAssemblyFrameLowering::needsSPWriteback(
   assert(needsSP(MF));
   // When we don't need a local stack pointer for its local frame but only to
   // support EH, we don't need to write SP back in the epilog, because we don't
   // bump down the stack pointer in the prolog. We need to write SP back in the
   // epilog only if
   // 1. We need SP not only for EH support but also because we actually use
   // stack or we have a frame address taken.
   // 2. We cannot use the red zone.
-  bool CanUseRedZone = MFI.getStackSize() <= RedZoneSize && !MFI.hasCalls() &&
+  bool CanUseRedZone = MFI.stackSizeIsKnown() &&
+                       MFI.getStackSize() <= RedZoneSize && !MFI.hasCalls() &&
                        !MF.getFunction().hasFnAttribute(Attribute::NoRedZone);
   return needsSPForLocalFrame(MF) && !CanUseRedZone;
 }
 
 void WebAssemblyFrameLowering::writeSPToGlobal(
     unsigned SrcReg, MachineFunction &MF, MachineBasicBlock &MBB,
     MachineBasicBlock::iterator &InsertStore, const DebugLoc &DL) const {
   const auto *TII = MF.getSubtarget<WebAssemblySubtarget>().getInstrInfo();

test/CodeGen/ARM/scavenging.mir

 # RUN: llc -o - %s -mtriple=thumb-arm-none-eabi -mcpu=cortex-m0 -run-pass scavenger-test | FileCheck %s
 ---
 # CHECK-LABEL: name: scavengebug0
 # Make sure we are not spilling/using a physreg used in the very last
 # instruction of the scavenging range.
 # CHECK-NOT: tSTRi {{.*}}$r0,{{.*}}$r0
 # CHECK-NOT: tSTRi {{.*}}$r1,{{.*}}$r1
 # CHECK-NOT: tSTRi {{.*}}$r2,{{.*}}$r2
 # CHECK-NOT: tSTRi {{.*}}$r3,{{.*}}$r3
 # CHECK-NOT: tSTRi {{.*}}$r4,{{.*}}$r4
 # CHECK-NOT: tSTRi {{.*}}$r5,{{.*}}$r5
 # CHECK-NOT: tSTRi {{.*}}$r6,{{.*}}$r6
 # CHECK-NOT: tSTRi {{.*}}$r7,{{.*}}$r7
 name: scavengebug0
+frameInfo:
+  stackSize: 0
 body: |
   bb.0:
     ; Bring up register pressure to force emergency spilling
     $r0 = IMPLICIT_DEF
     $r1 = IMPLICIT_DEF
     $r2 = IMPLICIT_DEF
     $r3 = IMPLICIT_DEF
     $r4 = IMPLICIT_DEF

test/CodeGen/MIR/Generic/frame-info.mir

--- |
   define i32 @test2(i32 %a) {
   entry:
     %b = alloca i32
     store i32 %a, i32* %b
     %c = load i32, i32* %b
     ret i32 %c
   }
 
+  define i32 @test3(i32 %a) {
+  entry:
+    %b = alloca i32
+    store i32 %a, i32* %b
+    %c = load i32, i32* %b
+    ret i32 %c
+  }
+
 ...
 ---
+
 name:            test
 tracksRegLiveness: true
 
+# CHECK-LABEL: name: test{{$}}
 # CHECK: frameInfo:
 # CHECK-NEXT: isFrameAddressTaken: false
 # CHECK-NEXT: isReturnAddressTaken: false
 # CHECK-NEXT: hasStackMap: false
 # CHECK-NEXT: hasPatchPoint: false
-# CHECK-NEXT: stackSize: 0
 # CHECK-NEXT: offsetAdjustment: 0
 # Note: max alignment can be target specific when printed.
 # CHECK-NEXT: maxAlignment:
 # CHECK-NEXT: adjustsStack: false
 # CHECK-NEXT: hasCalls: false
 # CHECK-NEXT: stackProtector:  ''
 # CHECK-NEXT: maxCallFrameSize:
 # CHECK-NEXT: cvBytesOfCalleeSavedRegisters: 0
 # CHECK-NEXT: hasOpaqueSPAdjustment: false
 # CHECK-NEXT: hasVAStart: false
 # CHECK-NEXT: hasMustTailInVarArgFunc: false
 # CHECK-NEXT: localFrameSize: 0
 # CHECK-NEXT: savePoint:       ''
 # CHECK-NEXT: restorePoint:    ''
 # CHECK: body
 frameInfo:
   maxAlignment:    4
 body: |
   bb.0.entry:
 ...
 ---
 name:            test2
 tracksRegLiveness: true
 
-# CHECK: test2
+# CHECK-LABEL: name: test2{{$}}
 # CHECK: frameInfo:
 # CHECK-NEXT: isFrameAddressTaken: true
 # CHECK-NEXT: isReturnAddressTaken: true
 # CHECK-NEXT: hasStackMap: true
 # CHECK-NEXT: hasPatchPoint: true
 # CHECK-NEXT: stackSize: 4
 # CHECK-NEXT: offsetAdjustment: 4
 # Note: max alignment can be target specific when printed.
frameInfo:
   hasOpaqueSPAdjustment: true
   hasVAStart:      true
   hasMustTailInVarArgFunc: true
   localFrameSize: 256
 body: |
   bb.0.entry:
 ...
 
+# Test an explicit stack size of 0
+---
+name:            test3
+tracksRegLiveness: true
+
+# CHECK-LABEL: name: test3{{$}}
+# CHECK: frameInfo:
+# CHECK-NEXT: isFrameAddressTaken: true
+# CHECK-NEXT: isReturnAddressTaken: true
+# CHECK-NEXT: hasStackMap: true
+# CHECK-NEXT: hasPatchPoint: true
+# CHECK-NEXT: stackSize: 0
+# CHECK-NEXT: offsetAdjustment: 4
+# Note: max alignment can be target specific when printed.
+# CHECK-NEXT: maxAlignment:
+# CHECK-NEXT: adjustsStack: true
+# CHECK-NEXT: hasCalls: true
+# CHECK-NEXT: stackProtector:  ''
+# CHECK-NEXT: maxCallFrameSize: 4
+# CHECK-NEXT: cvBytesOfCalleeSavedRegisters: 8
+# CHECK-NEXT: hasOpaqueSPAdjustment: true
+# CHECK-NEXT: hasVAStart: true
+# CHECK-NEXT: hasMustTailInVarArgFunc: true
+# CHECK: body
+frameInfo:
+  isFrameAddressTaken: true
+  isReturnAddressTaken: true
+  hasStackMap:     true
+  hasPatchPoint:   true
+  stackSize:       0
+  offsetAdjustment: 4
+  maxAlignment:    4
+  adjustsStack:    true
+  hasCalls:        true
+  maxCallFrameSize: 4
+  cvBytesOfCalleeSavedRegisters: 8
+  hasOpaqueSPAdjustment: true
+  hasVAStart:      true
+  hasMustTailInVarArgFunc: true
+  localFrameSize: 256
+body: |
+  bb.0.entry:
+...
+