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

paulkirth updated this revision to Diff 488834. paulkirth added a comment. Fix documentation string, since it was invalid formatting for the rst file. Also since code blocks don't render correctly in the html, write the documentation so that its usable without the code examples Repository: r

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

paulkirth updated this revision to Diff 488815. paulkirth added a comment. Address comments - Take a stab a ReleaseNotes - Update -Wframe-larger-than documentation to reference this pass - Enable filtering output from this pass by function name Repository: rG LLVM Github Monorepo CHANGES SIN

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

nickdesaulniers added a comment. @paulkirth please don't forget to update the commit description with that flag so that I don't have to read the contents of the commit to find this flag again. If you're using `arc diff` to upload the patch, the `--verbatim` flag will update the phab descriptio

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

nickdesaulniers added a comment. In D135488#4049075 , @thegameg wrote: > You can easily... I'll just note that v1 of this patch IIRC was a note on the single individual instance of the `-Wframe-larger-than=` diagnostic. No additional flags for optimiz

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

thegameg added a comment. In D135488#4049050 , @nickdesaulniers wrote: > In D135488#4049035 , @paulkirth > wrote: > >> Actually if we add >> >> if (!isFunctionInPrintList(MF.getName())) >>return false;

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

nickdesaulniers added a comment. Sorry, mind adding the documentation, too? Comment at: clang/test/Frontend/stack-layout-remark.c:8 +// RUN: mkdir -p %t +// RUN: %clang_cc1 %s -emit-codegen-only -triple x86_64-unknown-linux-gnu -target-cpu corei7 -Rpass-analysis=stack-frame-la

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

nickdesaulniers accepted this revision. nickdesaulniers added a comment. In D135488#4049035 , @paulkirth wrote: > Actually if we add > > if (!isFunctionInPrintList(MF.getName())) >return false; > > we can filter by name Does name mangling compl

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

paulkirth added a comment. Actually if we add if (!isFunctionInPrintList(MF.getName())) return false; we can filter by name Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/new/ https://reviews.llvm.org/D135488 _

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

paulkirth added a comment. In D135488#4048869 , @thegameg wrote: > In D135488#4048854 , @paulkirth > wrote: > >> In D135488#4048380 , >> @nickdesaulniers wrote: >> >>> I

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

paulkirth added inline comments. Comment at: llvm/test/CodeGen/AArch64/O0-pipeline.ll:76-77 ; CHECK-NEXT: Insert CFI remember/restore state instructions +; CHECK-NEXT: Lazy Machine Block Frequency Analysis +; CHECK-NEXT: Machine Optimization Remark Emitter +; C

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

thegameg added a comment. In D135488#4048854 , @paulkirth wrote: > In D135488#4048380 , > @nickdesaulniers wrote: > >> It would be really nice if we could limit this to a specific function >> somehow. > > I thin

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

paulkirth added inline comments. Comment at: clang/test/Frontend/stack-layout-remark.c:8 +// RUN: mkdir -p %t +// RUN: %clang_cc1 %s -emit-codegen-only -triple x86_64-unknown-linux-gnu -target-cpu corei7 -Rpass-analysis=stack-frame-layout -o /dev/null -O0 2>&1 | FileCheck %s

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

paulkirth added a comment. In D135488#4048380 , @nickdesaulniers wrote: > It would be really nice if we could limit this to a specific function somehow. I think you can do that, right ? see: https://llvm.org/docs/Remarks.html#cmdoption-pass-remarks-fil

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

paulkirth added inline comments. Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:44 + +const char *PassName = "stack-frame-layout"; + nickdesaulniers wrote: > Consider replacing uses of `PassName` with `DEBUG_TYPE` since they have the > same value.

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

paulkirth updated this revision to Diff 488784. paulkirth marked 21 inline comments as done. paulkirth added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/new/ https://reviews.llvm.org/D135488 Files: clang/test

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

nickdesaulniers added inline comments. Comment at: clang/test/Frontend/stack-layout-remark.c:8 +// RUN: mkdir -p %t +// RUN: %clang_cc1 %s -emit-codegen-only -triple x86_64-unknown-linux-gnu -target-cpu corei7 -Rpass-analysis=stack-frame-layout -o /dev/null -O0 2>&1 | FileChe

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

nickdesaulniers added inline comments. Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:44 + +const char *PassName = "stack-frame-layout"; + Consider replacing uses of `PassName` with `DEBUG_TYPE` since they have the same value. C

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

nickdesaulniers added a comment. It would be really nice if we could limit this to a specific function somehow. Quick feedback from giving Diff 488727 a spin on the Linux kernel: via `ARCH=arm make LLVM=1 -j128 -s allyesconfig all` I found: CC drivers/net/ethernet/mellanox/mlx5/core/en

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

paulkirth updated this revision to Diff 488727. paulkirth added a comment. Make YAML tests less brittle. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/new/ https://reviews.llvm.org/D135488 Files: clang/test/Frontend/stack-layout-remark.c

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

paulkirth updated this revision to Diff 488722. paulkirth added a comment. Add target triple to all RUN lines. Seems like the layout is the same on 64-bit windows, but for some reason clang.exe chooses i386 unless the triple is set. So just set the triple uniformly, and avoid any potential proble

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

paulkirth updated this revision to Diff 488427. paulkirth added a comment. Update clang test for windows file separators. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/new/ https://reviews.llvm.org/D135488 Files: clang/test/Frontend/stack

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

paulkirth updated this revision to Diff 488338. paulkirth added a comment. - Actually remove dead code - Update pass description to be more accurate - fix typo - update enum member name Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/new/ http

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

thegameg accepted this revision. thegameg added a comment. This revision is now accepted and ready to land. Looks great with the leftover minor changes, feel free to land this, thanks! I'll give this a try internally and provide feedback if any. Comment at: llvm/lib/CodeGen/St

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

paulkirth updated this revision to Diff 488314. paulkirth added a comment. Address comments. - document what ValOffset is used for - remove dead code - fix typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/new/ https://reviews.llvm.org/D13

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

thegameg added a comment. In D135488#4044437 , @paulkirth wrote: > BTW, is there a way to nest some of the items? Ideally we'd be able to have a > `Slot` in the YAML that contains all the various data, similar to how > `DebugLoc` is a more complex objec

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

paulkirth added a comment. @thegameg I think I finally understood what you meant re: multi-line remarks. Sorry for the back/forth on that, it just didn't click for me until you commented on the screenshot. BTW, is there a way to nest some of the items? Ideally we'd be able to have a `Slot` in

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

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

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

paulkirth updated this revision to Diff 488023. paulkirth added a comment. Add test for YAML output Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/new/ https://reviews.llvm.org/D135488 Files: clang/test/Frontend/stack-layout-remark.c llv

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

paulkirth updated this revision to Diff 487999. paulkirth added a comment. Rebase. - Switch to multi-line remarks - Update tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/new/ https://reviews.llvm.org/D135488 Files: clang/test/Fronte

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

thegameg added a comment. I don't think I understand why we can't achieve B with remarks? In C and D you generate one remark for each line, can't we generate a single multi-line remark instead? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/

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

paulkirth added a comment. I think ideally I'd like to surface something like this: A: F26139353: image.png or B: F26139363: image.png The first image uses stdout, and the second uses remarks, but prints everything from

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

thegameg added a comment. In D135488#4024713 , @paulkirth wrote: > @arsenm @thegameg @nickdesaulniers @dblaikie @phosek Can we reach a consensus > here on how to output this kind of information? I feel like I've been told to > move towards remarks as th

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

paulkirth added a comment. @arsenm @thegameg @nickdesaulniers @dblaikie @phosek Can we reach a consensus here on how to output this kind of information? I feel like I've been told to move towards remarks as the output method, but that the current diagnostic that tries to lay out the stack visua

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

thegameg added inline comments. Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:92 + &MF.front()) + << "FunctionName: " << ore::NV("FunctionName", MF.getName()); +}); Why are we emitting

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

paulkirth updated this revision to Diff 486112. paulkirth added a comment. Identify the stack protector in output Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/new/ https://reviews.llvm.org/D135488 Files: clang/test/Frontend/stack-layout-

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

paulkirth updated this revision to Diff 486095. paulkirth added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/new/ https://reviews.llvm.org/D135488 Files: clang/test/Frontend/stack-layout-remark.c llvm/include/llvm/Cod

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

paulkirth updated this revision to Diff 486094. paulkirth added a comment. Remove unnecesary null pointer check. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/new/ https://reviews.llvm.org/D135488 Files: clang/test/Frontend/stack-layout-r

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

paulkirth added a comment. @thegameg Maybe? It seems straightforward, but I'll need take a look. If it's easy(which I think it willbe), I'll try to update this patch, if it's more complex, I'll probably do a separate patch to add that feature. Comment at: llvm/lib/CodeGen/Sta

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

paulkirth updated this revision to Diff 486050. paulkirth added a comment. Avoid problems with path separators on windows, and ignore path prefix in diagnostic Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/new/ https://reviews.llvm.org/D135

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

thegameg added a comment. This is great! Any chance we can use `MachineFrameInfo::StackProtectorIdx` to annotate the slot that is reserved for the stack protector? Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:84 +ORE = &getAnalysis().getORE(); +if (!OR

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

paulkirth updated this revision to Diff 486030. paulkirth added a comment. Herald added subscribers: kosarev, kerbowa, jvesely, nemanjai. Add missing pipline test updates for PowerPC and AMDGPU Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/ne

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

paulkirth updated this revision to Diff 484694. paulkirth retitled this revision from "[codegen] Display stack layouts in console" to "[codegen] Add a remarks based Stack Layout Analysis pass". paulkirth edited the summary of this revision. paulkirth added a comment. Herald added subscribers: pcwa