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

paulkirth added a comment. Sorry, it took me a while to circle back to this. In D135488#3931603 , @dblaikie wrote: > I'm not sure we'd need/want to optimize a diagnostic experience for a > codebase that's got a lot of latent warnings. If people have lot

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

arsenm added a comment. In D135488#3929537 , @arsenm wrote: > The kernel resource remarks added in 67357739c6d36a61972c1fc0e829e35cb5375279 > are > probably the current heaviest remar

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

dblaikie added a comment. > This pass prints a TON of (helpful) information...we have a lot of > -Wframe-larger-than= instances triggered in our codebase...I think having > this on by default would blow our logs significantly. That's why it might be > nice to have a Note suggest a flag (default

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

arsenm added a comment. The kernel resource remarks added in 67357739c6d36a61972c1fc0e829e35cb5375279 are probably the current heaviest remarks. I believe there were some changes to newline handling for it Repository: rG

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

paulkirth added a comment. As a quick test, I hacked the printer pass to generate an output string, and passed that into the remarks emitter. From opt or llc things look as expected. There's some additional output, but its limited. I see a more serious issue when using it from Clang, as the out

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

nickdesaulniers added a comment. In D135488#3928831 , @dblaikie wrote: > Any chance we could ... emit it as part of the > frame-larger-than/warn-stack-size diagnostic? This pass prints a TON of (helpful) information...we have a lot of `-Wframe-larger-t

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

paulkirth added a comment. In D135488#3928975 , @dblaikie wrote: > Fair enough - could the warn-stack-size warning be moved to there, then, and > then the information included in the warning? It could have both a warning > and remark form, so folks coul

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

paulkirth updated this revision to Diff 475602. paulkirth added a comment. Rebase and address comments - Replace magic comparison with `MFI.isDeadIbjectIndex()` - Small code improvement by using a constructor w/ `emplace_back` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

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

dblaikie added a comment. In D135488#3928963 , @paulkirth wrote: > In D135488#3928831 , @dblaikie > wrote: > >> Any chance we could squirrel the info away (I assume there's a reason we >> can't compute the info

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

paulkirth added a comment. In D135488#3928831 , @dblaikie wrote: > Any chance we could squirrel the info away (I assume there's a reason we > can't compute the info where the warn-stack-size LLVM feature is implemented > in PrologEpilogInserter.cpp) som

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

dblaikie added a comment. > as a more general diagnostic output, like the other printing passes. As an aside: I don't think any "printing pass" is designed to be used beyond LLVM compiler engineers - they're implementation details of the compiler, even/much moreso than the optimization remarks

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

dblaikie added a comment. Any chance we could squirrel the info away (I assume there's a reason we can't compute the info where the warn-stack-size LLVM feature is implemented in PrologEpilogInserter.cpp) somewhere, and emit it as part of the frame-larger-than/warn-stack-size diagnostic? (also

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

paulkirth added a comment. Thanks for the feedback! I have a few questions I'm hoping you can answer. In D135488#3928559 , @arsenm wrote: > I don't think we should be pointing users to -mllvm flags. Plus, I don't > really think random dbgs() printing is

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

arsenm added a comment. In D135488#3928567 , @nickdesaulniers wrote: > In D135488#3928559 , @arsenm wrote: > >> In D135488#3928557 , >> @nickdesaulniers wrote: >> >>> Ca

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

nickdesaulniers added a comment. In D135488#3928567 , @nickdesaulniers wrote: > In D135488#3928559 , @arsenm wrote: > >> In D135488#3928557 , >> @nickdesaulniers wrote:

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

nickdesaulniers added a comment. In D135488#3928559 , @arsenm wrote: > In D135488#3928557 , > @nickdesaulniers wrote: > >> Can we add a `Note` diagnostic when we emit a `-Wframe-larger-than=` that >> alludes to

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

arsenm added a comment. In D135488#3928557 , @nickdesaulniers wrote: > Can we add a `Note` diagnostic when we emit a `-Wframe-larger-than=` that > alludes to re-running with `-mllvm -print-stack-frame` to get additional info? > > We should update the re

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

nickdesaulniers added a comment. Can we add a `Note` diagnostic when we emit a `-Wframe-larger-than=` that alludes to re-running with `-mllvm -print-stack-frame` to get additional info? We should update the release notes of clang to mention this feature, too. Repository: rG LLVM Github Monor

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

paulkirth updated this revision to Diff 471235. paulkirth added a comment. Address comments - fix brancing style - update parameters to use const - use llvm::sort in place of std::sort - remove path strings from debug info in test files Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

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

paulkirth added a comment. Thanks for the feedback. Those are good catches. I'll send out a patch cleaning those up. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/new/ https://reviews.llvm.org/D135488 __

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

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

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

paulkirth updated this revision to Diff 470989. paulkirth added a comment. Fix up use of 'auto' Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/new/ https://reviews.llvm.org/D135488 Files: llvm/include/llvm/CodeGen/Passes.h llvm/include/l

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

paulkirth updated this revision to Diff 470984. paulkirth added a comment. Fix pass name in a comment block Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/new/ https://reviews.llvm.org/D135488 Files: llvm/include/llvm/CodeGen/Passes.h ll

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

paulkirth updated this revision to Diff 470983. paulkirth retitled this revision from "[codegen][WIP] Display stack layouts in console" to "[codegen] Display stack layouts in console". paulkirth edited the summary of this revision. paulkirth added a comment. Rebase and update description Reposi