MachineFrameInfo: Track whether MaxCallFrameSize is computed yet

[MatzeB]

Summary

This tracks whether MaxCallFrameSize is computed yet. Ideally we would
assert and fail when the value is queried before it is computed, however
this fails various targets that need to be fixed first.

[MatzeB]

Let's make the assert a TODO but keep adding MachineFrameInfo::isMaxCallFrameInfo() computed. As it turned out to be useful to break some cycles in situations where querying wrong maxcallframesize is not a correctness issue but just a small performance one.

[MatzeB]

bring back a TODO comment that was accidentally deleted.

[MatzeB]

bring back TODO comment that was deleted by accident.

[qcolombet]

Hi Matthias,

however this fails various targets that need to be fixed first.

What are the PRs for that?

Is this something you plan to fix? (just asking :))

LGTM.

Cheers,
-Quentin

[MatzeB]

In D32570#742191, @qcolombet wrote:

Hi Matthias,

however this fails various targets that need to be fixed first.

What are the PRs for that?

Is this something you plan to fix? (just asking :))

I fixed ARM (see dependent patches). I do not plan to fix the other targets but will file PRs.

Thanks.

include/llvm/CodeGen/MIRYamlMapping.h

struct MachineFrameInfo {
   bool HasStackMap = false;
   bool HasPatchPoint = false;
   uint64_t StackSize = 0;
   int OffsetAdjustment = 0;
   unsigned MaxAlignment = 0;
   bool AdjustsStack = false;
   bool HasCalls = false;
   StringValue StackProtector;
-  // TODO: Serialize FunctionContextIdx
-  unsigned MaxCallFrameSize = 0;
+  unsigned MaxCallFrameSize = ~0u; ///< ~0u means: not computed yet.
   bool HasOpaqueSPAdjustment = false;
   bool HasVAStart = false;
   bool HasMustTailInVarArgFunc = false;
   StringValue SavePoint;
   StringValue RestorePoint;
 };
 
 template <> struct MappingTraits<MachineFrameInfo> {
   static void mapping(IO &YamlIO, MachineFrameInfo &MFI) {
     YamlIO.mapOptional("isFrameAddressTaken", MFI.IsFrameAddressTaken);
     YamlIO.mapOptional("isReturnAddressTaken", MFI.IsReturnAddressTaken);
     YamlIO.mapOptional("hasStackMap", MFI.HasStackMap);
     YamlIO.mapOptional("hasPatchPoint", MFI.HasPatchPoint);
     YamlIO.mapOptional("stackSize", MFI.StackSize);
     YamlIO.mapOptional("offsetAdjustment", MFI.OffsetAdjustment);
     YamlIO.mapOptional("maxAlignment", MFI.MaxAlignment);
     YamlIO.mapOptional("adjustsStack", MFI.AdjustsStack);
     YamlIO.mapOptional("hasCalls", MFI.HasCalls);
     YamlIO.mapOptional("stackProtector", MFI.StackProtector,
                        StringValue()); // Don't print it out when it's empty.
-    YamlIO.mapOptional("maxCallFrameSize", MFI.MaxCallFrameSize);
+    YamlIO.mapOptional("maxCallFrameSize", MFI.MaxCallFrameSize, ~0u);
     YamlIO.mapOptional("hasOpaqueSPAdjustment", MFI.HasOpaqueSPAdjustment);
     YamlIO.mapOptional("hasVAStart", MFI.HasVAStart);
     YamlIO.mapOptional("hasMustTailInVarArgFunc", MFI.HasMustTailInVarArgFunc);
     YamlIO.mapOptional("savePoint", MFI.SavePoint,
                        StringValue()); // Don't print it out when it's empty.
     YamlIO.mapOptional("restorePoint", MFI.RestorePoint,
                        StringValue()); // Don't print it out when it's empty.
   }

include/llvm/CodeGen/MachineFrameInfo.h

class MachineFrameInfo {
 
   /// The frame index for the function context. Used for SjLj exceptions.
   int FunctionContextIdx = -1;
 
   /// This contains the size of the largest call frame if the target uses frame
   /// setup/destroy pseudo instructions (as defined in the TargetFrameInfo
   /// class).  This information is important for frame pointer elimination.
   /// It is only valid during and after prolog/epilog code insertion.
-  unsigned MaxCallFrameSize = 0;
+  unsigned MaxCallFrameSize = ~0u;
 
   /// The prolog/epilog code inserter fills in this vector with each
   /// callee saved register saved in the frame.  Beyond its use by the prolog/
   /// epilog code inserter, this data used for debug info and exception
   /// handling.
   std::vector<CalleeSavedInfo> CSInfo;
 
   /// Has CSInfo been set yet?
public:
   bool hasTailCall() const { return HasTailCall; }
   void setHasTailCall() { HasTailCall = true; }
 
   /// Return the maximum size of a call frame that must be
   /// allocated for an outgoing function call.  This is only available if
   /// CallFrameSetup/Destroy pseudo instructions are used by the target, and
   /// then only during or after prolog/epilog code insertion.
   ///
-  unsigned getMaxCallFrameSize() const { return MaxCallFrameSize; }
+  unsigned getMaxCallFrameSize() const {
+    // TODO: Enable this assert when targets are fixed.
+    //assert(isMaxCallFrameSizeComputed() && "MaxCallFrameSize not computed yet");
+    return MaxCallFrameSize;
+  }
+  bool isMaxCallFrameSizeComputed() const {
+    return MaxCallFrameSize != ~0u;
+  }
   void setMaxCallFrameSize(unsigned S) { MaxCallFrameSize = S; }
 
   /// Create a new object at a fixed location on the stack.
   /// All fixed objects should be created before other objects are created for
   /// efficiency. By default, fixed objects are not pointed to by LLVM IR
   /// values. This returns an index with a negative value.
   int CreateFixedObject(uint64_t Size, int64_t SPOffset, bool Immutable,
                         bool isAliased = false);

lib/CodeGen/MIRParser/MIRParser.cpp

bool MIRParserImpl::initializeFrameInfo(PerFunctionMIParsingState &PFS,
   MFI.setHasStackMap(YamlMFI.HasStackMap);
   MFI.setHasPatchPoint(YamlMFI.HasPatchPoint);
   MFI.setStackSize(YamlMFI.StackSize);
   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.setHasOpaqueSPAdjustment(YamlMFI.HasOpaqueSPAdjustment);
   MFI.setHasVAStart(YamlMFI.HasVAStart);
   MFI.setHasMustTailInVarArgFunc(YamlMFI.HasMustTailInVarArgFunc);
   if (!YamlMFI.SavePoint.Value.empty()) {
     MachineBasicBlock *MBB = nullptr;
     if (parseMBBReference(PFS, MBB, YamlMFI.SavePoint))
       return true;
     MFI.setSavePoint(MBB);

lib/CodeGen/MIRPrinter.cpp

void MIRPrinter::convert(ModuleSlotTracker &MST,
   YamlMFI.IsReturnAddressTaken = MFI.isReturnAddressTaken();
   YamlMFI.HasStackMap = MFI.hasStackMap();
   YamlMFI.HasPatchPoint = MFI.hasPatchPoint();
   YamlMFI.StackSize = MFI.getStackSize();
   YamlMFI.OffsetAdjustment = MFI.getOffsetAdjustment();
   YamlMFI.MaxAlignment = MFI.getMaxAlignment();
   YamlMFI.AdjustsStack = MFI.adjustsStack();
   YamlMFI.HasCalls = MFI.hasCalls();
-  YamlMFI.MaxCallFrameSize = MFI.getMaxCallFrameSize();
+  YamlMFI.MaxCallFrameSize = MFI.isMaxCallFrameSizeComputed()
+    ? MFI.getMaxCallFrameSize() : ~0u;
   YamlMFI.HasOpaqueSPAdjustment = MFI.hasOpaqueSPAdjustment();
   YamlMFI.HasVAStart = MFI.hasVAStart();
   YamlMFI.HasMustTailInVarArgFunc = MFI.hasMustTailInVarArgFunc();
   if (MFI.getSavePoint()) {
     raw_string_ostream StrOS(YamlMFI.SavePoint.Value);
     MIPrinter(StrOS, MST, RegisterMaskIds, StackObjectOperandMapping)
         .printMBBReference(*MFI.getSavePoint());
   }