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