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 (g...@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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
