eandrews added a comment.
I think it makes sense to include this case just to make the flag more
complete. Also, we don't really match GCC behavior for this flag. GCC emits all
static constants by default (when O0). When optimized, this flag has no effect
on GCC. The negation is used to not
erichkeane added a comment.
In https://reviews.llvm.org/D40925#1276114, @rnk wrote:
> My coworker, @zturner, asked if I knew a way to force emit all constants, and
> I mentioned this flag, but we noticed it doesn't work on cases when the
> constant is implicitly static, like this:
>
> const
rnk added a subscriber: zturner.
rnk added a comment.
My coworker, @zturner, asked if I knew a way to force emit all constants, and I
mentioned this flag, but we noticed it doesn't work on cases when the constant
is implicitly static, like this:
const int foo = 42;
If I add `static` to it,
This revision was automatically updated to reflect the committed changes.
Closed by commit rC340439: Currently clang does not emit unused static
constants. GCC emits these (authored by eandrews, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D40925?vs=161544=162022#toc
eandrews marked an inline comment as done.
eandrews added a comment.
In https://reviews.llvm.org/D40925#1206416, @rnk wrote:
> lgtm!
Thanks!
https://reviews.llvm.org/D40925
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm!
https://reviews.llvm.org/D40925
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
eandrews marked an inline comment as done.
eandrews added inline comments.
Comment at: include/clang/Basic/LangOptions.def:311
+BENIGN_LANGOPT(KeepStaticConsts , 1, 0, "keep static const variables even
if unused")
+
rnk wrote:
> Let's make this a
eandrews updated this revision to Diff 161544.
eandrews added a comment.
Based on Reid's feedback, I changed option to CodeGenOption
https://reviews.llvm.org/D40925
Files:
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.def
lib/CodeGen/CodeGenModule.cpp
rnk added inline comments.
Comment at: include/clang/Basic/LangOptions.def:311
+BENIGN_LANGOPT(KeepStaticConsts , 1, 0, "keep static const variables even
if unused")
+
Let's make this a CodeGenOption, since only CodeGen needs to look at it.
eandrews updated this revision to Diff 161307.
eandrews edited the summary of this revision.
eandrews added a comment.
This patch fell through the cracks earlier. I apologize. Based on Reid's and
Erich's feedback, I am now adding the variable to @llvm.used. Additionally I
modified the if
eandrews added a comment.
I think we can use CodeGenModule::addUsedGlobal for this purpose. I noticed
this is what attribute 'used' calls. I need to look into it though.
https://reviews.llvm.org/D40925
___
cfe-commits mailing list
erichkeane added a comment.
>> OK. My concern is that users need to use __attribute__((used)) or something
>> more robust if they want SVN identifiers to reliably appear in the output.
>> Adding this flag just creates a trap that will fail once they turn on >>-O2.
>> I'd rather not have it in
eandrews added a comment.
That makes sense. Is it not possible to implement the required functionality
using a flag vs an attribute? In an earlier comment you mentioned adding the
global to @llvm.used to prevent GlobalDCE from removing it. Can you not do that
when using a flag?
rnk added a comment.
OK. My concern is that users need to use `__attribute__((used))` or something
more robust if they want SVN identifiers to reliably appear in the output.
Adding this flag just creates a trap that will fail once they turn on `-O2`.
I'd rather not have it in the interface to
eandrews added a comment.
Thanks for the review Reid. Sorry for the delay in my response. I was OOO.
I am not sure if a new attribute is necessary. __ attribute __(used) is already
supported in Clang. While this attribute can be used to retain static
constants, it would require the user to
rnk added a comment.
This isn't sufficient, GlobalDCE will remove the internal constant. It's also
unlikely that the constant will survive `--gc-sections / -fdata-sections`. A
better solution would be to add a new attribute
(`__attribute__((nondiscardable))`? too close to `nodiscard`?) that
eandrews added a comment.
*ping*
https://reviews.llvm.org/D40925
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
eandrews created this revision.
Currently clang does not emit unused static constants declared globally. Local
static constants are emitted by default. -fkeep-static-consts can be used to
emit static constants declared globally even if they are not used.
This could be useful for producing
18 matches
Mail list logo
