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

Nick Desaulniers via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 12 16:23:55 PST 2023


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-layout -o /dev/null  -O0  2>&1 | FileCheck %s --check-prefix=O0-NODEBUG
+// 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  -debug-info-kind=constructor  -dwarf-version=5 -debugger-tuning=gdb 2>&1 | FileCheck %s --check-prefix=O0-DEBUG
----------------
paulkirth wrote:
> paulkirth wrote:
> > nickdesaulniers wrote:
> > > nickdesaulniers wrote:
> > > > Please update:
> > > > 1. the patch description/commit message
> > > > 2. clang/docs/ReleaseNotes.rst
> > > > 
> > > >  to mention this new flag. I kind of wish that `-Wstack-frame-larger-than=` alluded to this somehow.
> > > Perhaps in the documentation for `-Wframe-larger-than=`? i.e. adding a `code Documentation = [{}]` block to `BackendFrameLargerThan` record in clang/include/clang/Basic/DiagnosticGroups.td or something.
> > Will do on the ReleaseNotes, but I'm a bit unsure what you want in the summary. It mentions the motivation and describe what this pass is for/does using remarks. Is there something else it should say?
> > Perhaps in the documentation for `-Wframe-larger-than=`? i.e. adding a `code Documentation = [{}]` block to `BackendFrameLargerThan` record in clang/include/clang/Basic/DiagnosticGroups.td or something.
> 
> Hmm, I'll take a look there, but I'm not 100% sure I follow what you mean. 
> 
> are you after somethng like this when frame-larger-than diagnostics happen:
> ```
> you can debug this by adding -Rpass-analysis=stack-frame-layout -mllvm -pass-remarks-filter=<functionname>
> ```
> or something like that?
> Hmm, I'll take a look there, but I'm not 100% sure I follow what you mean.

> are you after somethng like this when frame-larger-than diagnostics happen:

> you can debug this by adding -Rpass-analysis=stack-frame-layout -mllvm -pass-remarks-filter=<functionname>
or something like that?


Basically, my concern is "how will other developers not cc'ed on this phab review ever find this nifty new flag?" If -Wframe-larger-than= doesn't print info about it, then we should at least have it in our docs.

The last sentence you suggested is exactly what I had in mind.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135488



More information about the llvm-commits mailing list