[PATCH] D135488: [codegen] Display stack layouts in console

[Min-Yih Hsu via Phabricator via cfe-commits]

myhsu added a comment.

Just some drive-by comments



================
Comment at: llvm/lib/CodeGen/StackFramePrinterPass.cpp:79
+
+  void printBody(raw_ostream &OS, SlotData &D) const {
+    OS << formatv("{0,-11} {1,-5} {2,-9} {3,-10}\n", genStackOffset(D.Offset),
----------------
const SlotData &? If this is true please also update the call site (e.g. line 143)


================
Comment at: llvm/lib/CodeGen/StackFramePrinterPass.cpp:82
+                  D.IsSpillSlot ? "Spill" : "", D.Align, D.Size)
+              .str();
+  };
----------------
IIRC there is no need to materialize to std::string, formatv works out-of-box with raw_ostream


================
Comment at: llvm/lib/CodeGen/StackFramePrinterPass.cpp:123
+      // This is a dead slot, so skip it
+      if (Size == ~0ULL) {
+        continue;
----------------
format: remove curly braces for single-line body


================
Comment at: llvm/lib/CodeGen/StackFramePrinterPass.cpp:138
+    // sort the ordering, to match the actual layout in memory
+    std::sort(SlotInfo.begin(), SlotInfo.end(),
+              [](SlotData &A, SlotData &B) { return A.Offset > B.Offset; });
----------------
nit: use llvm::sort so that we can simply write `llvm::sort(SlotInfo, <comparer>)`


================
Comment at: llvm/lib/CodeGen/StackFramePrinterPass.cpp:139
+    std::sort(SlotInfo.begin(), SlotInfo.end(),
+              [](SlotData &A, SlotData &B) { return A.Offset > B.Offset; });
+
----------------
nit: using `const SlotData &` for both A and B


================
Comment at: llvm/lib/CodeGen/StackFramePrinterPass.cpp:145
+      printBody(OS, L);
+      for (const llvm::DILocalVariable *N : SlotMap[L.Slot])
+        printSourceLoc(OS, N);
----------------
remove llvm


================
Comment at: llvm/lib/CodeGen/StackFramePrinterPass.cpp:159
+    // add variables to the map
+    for (MachineFunction::VariableDbgInfo &DI : MF.getVariableDbgInfo()) {
+      SlotDebugMap[DI.Slot].insert(DI.Var);
----------------
format: remove curly braces for single-line loop body


================
Comment at: llvm/lib/CodeGen/StackFramePrinterPass.cpp:179
+
+          for (llvm::MachineInstr *MI : Dbg)
+            SlotDebugMap[FrameIdx].insert(MI->getDebugVariable());
----------------
remove llvm


================
Comment at: llvm/test/CodeGen/ARM/stack-frame-printer.ll:221
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "Fuchsia clang version 16.0.0 (git at github.com:llvm/llvm-project.git bb51a99e67747be81c9b523fd5ddcc8bf91a1ffb)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "frame-diags.c", directory: "/usr/local/google/home/paulkirth/llvm-sysroot/clang/test/Frontend", checksumkind: CSK_MD5, checksum: "01b5d69ce5387b02de2d1191b28a0b7f")
----------------
nit: is it possible to clean up some of the irrelevant strings like the producer and directory fields?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135488