[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D141381#4045215 <https://reviews.llvm.org/D141381#4045215>, @rjmccall wrote:

> That's about what I would expect.  One or two extra instructions per argument that are trivially removed in debug builds.  Very small overall impact because there just aren't very many of these arguments.
>
> I don't really see a need to post about this on Discourse, but it might be worth a release note.

Fair enough - if you're cool with it. Yeah, release note wouldn't hurt.



================
Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4826
+                                      unsigned ArgNo, CGBuilderTy &Builder,
+                                      const bool UsePointerValue) {
   assert(CGM.getCodeGenOpts().hasReducedDebugInfo());
----------------
We don't usually use top-level const on parameters/locals like this, for consistency (eg: with `ArgNo`) please remove it. Otherwise it can lead to confusion that maybe there was a `&` intended, etc.


================
Comment at: clang/lib/CodeGen/CGDebugInfo.h:494
+                           CGBuilderTy &Builder,
+                           const bool UsePointerValue = false);
 
----------------
fdeazeve wrote:
> FWIW I used a `const` bool here to match the style already present in this class
Examples of the things you were trying to be consistent with? Because the other by-value parameter here isn't const at the top level & const at the top level on function declaration parameters has no meaning, so especially here it should probably be omitted (& probably also in the definition, but it's a somewhat separate issue/has different tradeoffs)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141381