[PATCH] D40925: Add option -fkeep-static-consts

2018-10-25 Thread Elizabeth Andrews via Phabricator via cfe-commits 
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 emit the constants.

Erich has uploaded a possible fix here - https://reviews.llvm.org/D53718


Repository:
  rC Clang

https://reviews.llvm.org/D40925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40925: Add option -fkeep-static-consts

2018-10-25 Thread Erich Keane via Phabricator via cfe-commits 
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 int foo = 42;
>
>
> If I add `static` to it, it gets emitted with -fkeep-static-consts, but as 
> is, it doesn't get emitted.
>
> Do you think it's worth extending the logic to handle this case? GCC seems to 
> that global `foo` regardless of the flag setting.


I looked into it real quick, and think that checking storage class instead of 
duration is incorrect here.  See https://reviews.llvm.org/D53718 for the fix 
@zturner is asking for (I think!).


Repository:
  rC Clang

https://reviews.llvm.org/D40925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40925: Add option -fkeep-static-consts

2018-10-25 Thread Reid Kleckner via Phabricator via cfe-commits 
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, it gets emitted with -fkeep-static-consts, but as is, 
it doesn't get emitted.

Do you think it's worth extending the logic to handle this case? GCC seems to 
that global `foo` regardless of the flag setting.


Repository:
  rC Clang

https://reviews.llvm.org/D40925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40925: Add option -fkeep-static-consts

2018-08-22 Thread Elizabeth Andrews via Phabricator via cfe-commits 
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

Repository:
  rC Clang

https://reviews.llvm.org/D40925

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CodeGenModule.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/keep-static-consts.cpp


Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -893,7 +893,8 @@
 def fno_force_enable_int128 : Flag<["-"], "fno-force-enable-int128">,
   Group, Flags<[CC1Option]>,
   HelpText<"Disable support for int128_t type">;
-
+def fkeep_static_consts : Flag<["-"], "fkeep-static-consts">, Group, 
Flags<[CC1Option]>,
+  HelpText<"Keep static const variables even if unused">;
 def ffixed_point : Flag<["-"], "ffixed-point">, Group,
Flags<[CC1Option]>, HelpText<"Enable fixed point types">;
 def fno_fixed_point : Flag<["-"], "fno-fixed-point">, Group,
Index: include/clang/Frontend/CodeGenOptions.def
===
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -341,6 +341,9 @@
 
 ENUM_CODEGENOPT(SignReturnAddress, SignReturnAddressScope, 2, None)
 
+/// Whether to emit unused static constants.
+CODEGENOPT(KeepStaticConsts, 1, 0)
+
 #undef CODEGENOPT
 #undef ENUM_CODEGENOPT
 #undef VALUE_CODEGENOPT
Index: test/CodeGen/keep-static-consts.cpp
===
--- test/CodeGen/keep-static-consts.cpp
+++ test/CodeGen/keep-static-consts.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fkeep-static-consts -emit-llvm %s -o - 
-triple=x86_64-unknown-linux-gnu | FileCheck %s
+
+// CHECK: @_ZL7srcvers = internal constant [4 x i8] c"xyz\00", align 1
+// CHECK: @llvm.used = appending global [1 x i8*] [i8* getelementptr inbounds 
([4 x i8], [4 x i8]* @_ZL7srcvers, i32 0, i32 0)], section "llvm.metadata"
+static const char srcvers[] = "xyz";
+extern const int b = 1;
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1145,6 +1145,8 @@
   << A->getAsString(Args) << A->getValue();
   }
 
+  Opts.KeepStaticConsts = Args.hasArg(OPT_fkeep_static_consts);
+
   return Success;
 }
 
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -1350,6 +1350,12 @@
 
   if (D && D->hasAttr())
 addUsedGlobal(GV);
+
+  if (CodeGenOpts.KeepStaticConsts && D && isa(D)) {
+const auto *VD = cast(D);
+if (VD->getType().isConstQualified() && VD->getStorageClass() == SC_Static)
+  addUsedGlobal(GV);
+  }
 }
 
 bool CodeGenModule::GetCPUAndFeaturesAttributes(const Decl *D,
@@ -1985,6 +1991,13 @@
   if (LangOpts.EmitAllDecls)
 return true;
 
+  if (CodeGenOpts.KeepStaticConsts) {
+const auto *VD = dyn_cast(Global);
+if (VD && VD->getType().isConstQualified() &&
+VD->getStorageClass() == SC_Static)
+  return true;
+  }
+
   return getContext().DeclMustBeEmitted(Global);
 }
 
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4008,6 +4008,7 @@
   Args.AddLastArg(CmdArgs, options::OPT_fno_operator_names);
   Args.AddLastArg(CmdArgs, options::OPT_femulated_tls,
   options::OPT_fno_emulated_tls);
+  Args.AddLastArg(CmdArgs, options::OPT_fkeep_static_consts);
 
   // AltiVec-like language extensions aren't relevant for assembling.
   if (!isa(JA) || Output.getType() != types::TY_PP_Asm)


Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -893,7 +893,8 @@
 def fno_force_enable_int128 : Flag<["-"], "fno-force-enable-int128">,
   Group, Flags<[CC1Option]>,
   HelpText<"Disable support for int128_t type">;
-
+def fkeep_static_consts : Flag<["-"], "fkeep-static-consts">, Group, Flags<[CC1Option]>,
+  HelpText<"Keep static const variables even if unused">;
 def ffixed_point : Flag<["-"], "ffixed-point">, Group,
Flags<[CC1Option]>, HelpText<"Enable fixed point types">;
 def fno_fixed_point : Flag<["-"], "fno-fixed-point">, Group,
Index: include/clang/Frontend/CodeGenOptions.def
===
--- include/clang/Frontend/CodeGenOptions.def
+++

[PATCH] D40925: Add option -fkeep-static-consts

2018-08-20 Thread Elizabeth Andrews via Phabricator via cfe-commits 
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
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40925: Add option -fkeep-static-consts

2018-08-20 Thread Reid Kleckner via Phabricator via cfe-commits 
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
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40925: Add option -fkeep-static-consts

2018-08-20 Thread Elizabeth Andrews via Phabricator via cfe-commits 
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 CodeGenOption, since only CodeGen needs to look at it.
Thanks for the feedback Reid! I've changed it


https://reviews.llvm.org/D40925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40925: Add option -fkeep-static-consts

2018-08-20 Thread Elizabeth Andrews via Phabricator via cfe-commits 
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
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/keep-static-consts.cpp


Index: test/CodeGen/keep-static-consts.cpp
===
--- /dev/null
+++ test/CodeGen/keep-static-consts.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fkeep-static-consts -emit-llvm %s -o - 
-triple=x86_64-unknown-linux-gnu | FileCheck %s
+
+// CHECK: @_ZL7srcvers = internal constant [4 x i8] c"xyz\00", align 1
+// CHECK: @llvm.used = appending global [1 x i8*] [i8* getelementptr inbounds 
([4 x i8], [4 x i8]* @_ZL7srcvers, i32 0, i32 0)], section "llvm.metadata"
+static const char srcvers[] = "xyz";
+extern const int b = 1;
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1125,6 +1125,8 @@
 
   Opts.Addrsig = Args.hasArg(OPT_faddrsig);
 
+  Opts.KeepStaticConsts = Args.hasArg(OPT_fkeep_static_consts);
+
   return Success;
 }
 
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3996,6 +3996,7 @@
   Args.AddLastArg(CmdArgs, options::OPT_fno_operator_names);
   Args.AddLastArg(CmdArgs, options::OPT_femulated_tls,
   options::OPT_fno_emulated_tls);
+  Args.AddLastArg(CmdArgs, options::OPT_fkeep_static_consts);
 
   // AltiVec-like language extensions aren't relevant for assembling.
   if (!isa(JA) || Output.getType() != types::TY_PP_Asm)
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -1350,6 +1350,12 @@
 
   if (D && D->hasAttr())
 addUsedGlobal(GV);
+
+  if (CodeGenOpts.KeepStaticConsts && D && isa(D)) {
+const auto *VD = cast(D);
+if (VD->getType().isConstQualified() && VD->getStorageClass() == SC_Static)
+  addUsedGlobal(GV);
+  }
 }
 
 bool CodeGenModule::GetCPUAndFeaturesAttributes(const Decl *D,
@@ -1985,6 +1991,13 @@
   if (LangOpts.EmitAllDecls)
 return true;
 
+  if (CodeGenOpts.KeepStaticConsts) {
+const auto *VD = dyn_cast(Global);
+if (VD && VD->getType().isConstQualified() &&
+VD->getStorageClass() == SC_Static)
+  return true;
+  }
+
   return getContext().DeclMustBeEmitted(Global);
 }
 
Index: include/clang/Frontend/CodeGenOptions.def
===
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -339,6 +339,9 @@
 /// Whether to emit an address-significance table into the object file.
 CODEGENOPT(Addrsig, 1, 0)
 
+/// Whether to emit unused static constants.
+CODEGENOPT(KeepStaticConsts, 1, 0)
+
 
 #undef CODEGENOPT
 #undef ENUM_CODEGENOPT
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -893,7 +893,8 @@
 def fno_force_enable_int128 : Flag<["-"], "fno-force-enable-int128">,
   Group, Flags<[CC1Option]>,
   HelpText<"Disable support for int128_t type">;
-
+def fkeep_static_consts : Flag<["-"], "fkeep-static-consts">, Group, 
Flags<[CC1Option]>,
+  HelpText<"Keep static const variables even if unused">;
 def ffixed_point : Flag<["-"], "ffixed-point">, Group,
Flags<[CC1Option]>, HelpText<"Enable fixed point types">;
 def fno_fixed_point : Flag<["-"], "fno-fixed-point">, Group,


Index: test/CodeGen/keep-static-consts.cpp
===
--- /dev/null
+++ test/CodeGen/keep-static-consts.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fkeep-static-consts -emit-llvm %s -o - -triple=x86_64-unknown-linux-gnu | FileCheck %s
+
+// CHECK: @_ZL7srcvers = internal constant [4 x i8] c"xyz\00", align 1
+// CHECK: @llvm.used = appending global [1 x i8*] [i8* getelementptr inbounds ([4 x i8], [4 x i8]* @_ZL7srcvers, i32 0, i32 0)], section "llvm.metadata"
+static const char srcvers[] = "xyz";
+extern const int b = 1;
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1125,6 +1125,8 @@
 
   Opts.Addrsig = Args.hasArg(OPT_faddrsig);
 
+  Opts.KeepStaticConsts = Args.hasArg(OPT_fkeep_static_consts);
+
   return Success;
 }
 
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++

[PATCH] D40925: Add option -fkeep-static-consts

2018-08-17 Thread Reid Kleckner via Phabricator via cfe-commits 
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.


https://reviews.llvm.org/D40925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40925: Add option -fkeep-static-consts

2018-08-17 Thread Elizabeth Andrews via Phabricator via cfe-commits 
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 statements to ensure only static variables are considered.

I also compared the IR when using the option -fkeep-static-consts while 
compiling the program, vs adding __attribute__((used)) to individual variables 
inside the program. The generated IR had no difference. I assume this means all 
required information to prevent the variable from being removed in later stages 
are now being emitted?


https://reviews.llvm.org/D40925

Files:
  include/clang/Basic/LangOptions.def
  include/clang/Driver/Options.td
  lib/AST/ASTContext.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/keep-static-consts.cpp


Index: test/CodeGen/keep-static-consts.cpp
===
--- /dev/null
+++ test/CodeGen/keep-static-consts.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fkeep-static-consts -emit-llvm %s -o - 
-triple=x86_64-unknown-linux-gnu | FileCheck %s
+
+// CHECK: @_ZL7srcvers = internal constant [4 x i8] c"xyz\00", align 1
+// CHECK: @llvm.used = appending global [1 x i8*] [i8* getelementptr inbounds 
([4 x i8], [4 x i8]* @_ZL7srcvers, i32 0, i32 0)], section "llvm.metadata"
+static const char srcvers[] = "xyz";
+extern const int b = 1;
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2485,6 +2485,7 @@
   Opts.HalfArgsAndReturns = Args.hasArg(OPT_fallow_half_arguments_and_returns)
 | Opts.NativeHalfArgsAndReturns;
   Opts.GNUAsm = !Args.hasArg(OPT_fno_gnu_inline_asm);
+  Opts.KeepStaticConsts = Args.hasArg(OPT_fkeep_static_consts);
 
   // __declspec is enabled by default for the PS4 by the driver, and also
   // enabled for Microsoft Extensions or Borland Extensions, here.
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3996,6 +3996,7 @@
   Args.AddLastArg(CmdArgs, options::OPT_fno_operator_names);
   Args.AddLastArg(CmdArgs, options::OPT_femulated_tls,
   options::OPT_fno_emulated_tls);
+  Args.AddLastArg(CmdArgs, options::OPT_fkeep_static_consts);
 
   // AltiVec-like language extensions aren't relevant for assembling.
   if (!isa(JA) || Output.getType() != types::TY_PP_Asm)
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -1350,6 +1350,12 @@
 
   if (D && D->hasAttr())
 addUsedGlobal(GV);
+
+  if (LangOpts.KeepStaticConsts && D && isa(D)) {
+const auto *VD = cast(D);
+if (VD->getType().isConstQualified() && VD->getStorageClass() == SC_Static)
+  addUsedGlobal(GV);
+  }
 }
 
 bool CodeGenModule::GetCPUAndFeaturesAttributes(const Decl *D,
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -9741,6 +9741,14 @@
   if (D->hasAttr() || D->hasAttr())
 return true;
 
+  // Emit static constants even if they are not used if KeepStaticConsts is 
set.
+  if (LangOpts.KeepStaticConsts) {
+const auto *VD = dyn_cast(D);
+if (VD && VD->getType().isConstQualified() &&
+VD->getStorageClass() == SC_Static)
+  return true;
+  }
+
   if (const auto *FD = dyn_cast(D)) {
 // Forward declarations aren't required.
 if (!FD->doesThisDeclarationHaveABody())
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -893,7 +893,8 @@
 def fno_force_enable_int128 : Flag<["-"], "fno-force-enable-int128">,
   Group, Flags<[CC1Option]>,
   HelpText<"Disable support for int128_t type">;
-
+def fkeep_static_consts : Flag<["-"], "fkeep-static-consts">, Group, 
Flags<[CC1Option]>,
+  HelpText<"Keep static const variables even if unused">;
 def ffixed_point : Flag<["-"], "ffixed-point">, Group,
Flags<[CC1Option]>, HelpText<"Enable fixed point types">;
 def fno_fixed_point : Flag<["-"], "fno-fixed-point">, Group,
Index: include/clang/Basic/LangOptions.def
===
--- include/clang/Basic/LangOptions.def
+++ include/clang/Basic/LangOptions.def
@@ -308,6 +308,8 @@
 LANGOPT(PaddingOnUnsignedFixedPoint, 1, 0,
 "unsigned fixed point types having one extra padding bit")
 
+BENIGN_LANGOPT(KeepStaticConsts  , 1, 0, "keep static const variables even 
if

[PATCH] D40925: Add option -fkeep-static-consts

2018-02-14 Thread Elizabeth Andrews via Phabricator via cfe-commits 
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
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40925: Add option -fkeep-static-consts

2018-02-13 Thread Erich Keane via Phabricator via cfe-commits 
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 the interface to avoid that user confusion.

Since the idea here is to have a global option for 'always emit these things', 
could a viable solution simply add all of these things to @llvm.used.  This 
wouldn't affect the usefulness of __attribute((used)), but could be implemented 
similarly, right?


https://reviews.llvm.org/D40925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40925: Add option -fkeep-static-consts

2018-01-18 Thread Elizabeth Andrews via Phabricator via cfe-commits 
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?


https://reviews.llvm.org/D40925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40925: Add option -fkeep-static-consts

2018-01-17 Thread Reid Kleckner via Phabricator via cfe-commits 
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 avoid that user confusion.


https://reviews.llvm.org/D40925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40925: Add option -fkeep-static-consts

2018-01-10 Thread Elizabeth Andrews via Phabricator via cfe-commits 
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 modify the source which may not always 
be possible/practical. Its also interesting to note that GCC actually retains 
unused static constants by default.  fno-keep-static-consts is used to remove 
unused static constants in GCC.


https://reviews.llvm.org/D40925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40925: Add option -fkeep-static-consts

2017-12-14 Thread Reid Kleckner via Phabricator via cfe-commits 
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 adds the 
global in question to `@llvm.used` and excludes it from -fdata-sections.


https://reviews.llvm.org/D40925



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40925: Add option -fkeep-static-consts

2017-12-11 Thread Elizabeth Andrews via Phabricator via cfe-commits 
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

[PATCH] D40925: Add option -fkeep-static-consts

2017-12-06 Thread Elizabeth Andrews via Phabricator via 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 identification strings like SVN identifiers 
inside the object file even though the string isn't used by the program.


https://reviews.llvm.org/D40925

Files:
  include/clang/Basic/LangOptions.def
  include/clang/Driver/Options.td
  lib/AST/ASTContext.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/keep-static-consts.cpp


Index: test/CodeGen/keep-static-consts.cpp
===
--- /dev/null
+++ test/CodeGen/keep-static-consts.cpp
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -fkeep-static-consts -emit-llvm %s -o - 
-triple=x86_64-unknown-linux-gnu | FileCheck %s
+
+// CHECK: @_ZL3var = internal constant i32 10, align 4
+static const int var = 10;
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2509,6 +2509,7 @@
   Opts.HalfArgsAndReturns = Args.hasArg(OPT_fallow_half_arguments_and_returns)
 | Opts.NativeHalfArgsAndReturns;
   Opts.GNUAsm = !Args.hasArg(OPT_fno_gnu_inline_asm);
+  Opts.KeepStaticConsts = Args.hasArg(OPT_fkeep_static_consts);
 
   // __declspec is enabled by default for the PS4 by the driver, and also
   // enabled for Microsoft Extensions or Borland Extensions, here.
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3803,6 +3803,7 @@
   Args.AddLastArg(CmdArgs, options::OPT_femit_all_decls);
   Args.AddLastArg(CmdArgs, options::OPT_fheinous_gnu_extensions);
   Args.AddLastArg(CmdArgs, options::OPT_fno_operator_names);
+  Args.AddLastArg(CmdArgs, options::OPT_fkeep_static_consts);
   // Emulated TLS is enabled by default on Android and OpenBSD, and can be 
enabled
   // manually with -femulated-tls.
   bool EmulatedTLSDefault = Triple.isAndroid() || Triple.isOSOpenBSD() ||
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -9314,6 +9314,12 @@
   if (D->hasAttr() || D->hasAttr())
 return true;
 
+  // Emit static constants even if they are not used if KeepStaticConsts is 
set.
+  if (const VarDecl *VD = dyn_cast(D)) {
+if (LangOpts.KeepStaticConsts && VD->getType().isConstQualified())
+  return true;
+  }
+
   if (const FunctionDecl *FD = dyn_cast(D)) {
 // Forward declarations aren't required.
 if (!FD->doesThisDeclarationHaveABody())
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -835,6 +835,8 @@
 def fjump_tables : Flag<["-"], "fjump-tables">, Group;
 def fno_jump_tables : Flag<["-"], "fno-jump-tables">, Group, 
Flags<[CC1Option]>,
   HelpText<"Do not use jump tables for lowering switches">;
+def fkeep_static_consts : Flag<["-"], "fkeep-static-consts">, Group, 
Flags<[CC1Option]>,
+  HelpText<"Keep static const variables even if unused">;
 
 // Begin sanitizer flags. These should all be core options exposed in all 
driver
 // modes.
Index: include/clang/Basic/LangOptions.def
===
--- include/clang/Basic/LangOptions.def
+++ include/clang/Basic/LangOptions.def
@@ -319,6 +319,8 @@
 BENIGN_LANGOPT(AllowEditorPlaceholders, 1, 0,
"allow editor placeholders in source")
 
+BENIGN_LANGOPT(KeepStaticConsts  , 1, 0, "keep static const variables even 
if unused")
+
 #undef LANGOPT
 #undef COMPATIBLE_LANGOPT
 #undef BENIGN_LANGOPT


Index: test/CodeGen/keep-static-consts.cpp
===
--- /dev/null
+++ test/CodeGen/keep-static-consts.cpp
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -fkeep-static-consts -emit-llvm %s -o - -triple=x86_64-unknown-linux-gnu | FileCheck %s
+
+// CHECK: @_ZL3var = internal constant i32 10, align 4
+static const int var = 10;
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2509,6 +2509,7 @@
   Opts.HalfArgsAndReturns = Args.hasArg(OPT_fallow_half_arguments_and_returns)
 | Opts.NativeHalfArgsAndReturns;
   Opts.GNUAsm = !Args.hasArg(OPT_fno_gnu_inline_asm);
+  Opts.KeepStaticConsts = Args.hasArg(OPT_fkeep_static_consts);
 
   // __declspec is enabled by default for the PS4 by the driver, and also
   // enabled for Microsoft Extensions or