[PATCH] D135488: [codegen] Add a remarks based Stack Layout Analysis pass

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg added a comment.

This looks great, thanks for updating this! A few more comments inline.



================
Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:55
+    Variable,       // a Slot used to store a local data (could be a tmp)
+    Error           // Its an error for a slot to have this type
+  };
----------------



================
Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:65
+
+    SlotData(const MachineFrameInfo &MFI, const int ValOffset, const int Idx) {
+      Slot = Idx;
----------------



================
Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:110
+
+  std::string genStackOffsetPrefix(int I) const {
+    // Negative offsets will print a leading `-`, so only add `+`
----------------
Is this still worth being a separate function?


================
Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:128
+
+  void emitLayoutRemark(const MachineFunction &MF, StringRef RemarkName,
+                        StringRef RemarkLabel, StringRef Data,
----------------
Unused?


================
Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:179
+    const TargetFrameLowering *FI = MF.getSubtarget().getFrameLowering();
+    const int ValOffset = (FI ? FI->getOffsetOfLocalArea() : 0);
+    const unsigned int NumObj = MFI.getNumObjects();
----------------
Can you add a comment on what `ValOffset` is?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135488/new/

https://reviews.llvm.org/D135488