[PATCH] D150221: Add option -fkeep-persistent-storage-variables to emit all variables that have a persistent storage duration

2023-07-15 Thread Hubert Tong via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbb6ab91b1dcd: Add option -fkeep-persistent-storage-variables 
to emit all variables that haveā€¦ (authored by qianzhen, committed by 
hubert.reinterpretcast).

Changed prior to commit:
  https://reviews.llvm.org/D150221?vs=540060=540732#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150221

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/keep-persistent-storage-variables.cpp
  clang/test/Driver/fkeep-persistent-storage-variables.c

Index: clang/test/Driver/fkeep-persistent-storage-variables.c
===
--- /dev/null
+++ clang/test/Driver/fkeep-persistent-storage-variables.c
@@ -0,0 +1,5 @@
+// RUN: %clang -fkeep-persistent-storage-variables -c %s -### 2>&1 | FileCheck %s
+// RUN: %clang -fkeep-persistent-storage-variables -fno-keep-persistent-storage-variables -c %s -### 2>&1 | FileCheck %s -check-prefix=CHECK-NOKEEP
+
+// CHECK: "-fkeep-persistent-storage-variables"
+// CHECK-NOKEEP-NOT: "-fkeep-persistent-storage-variables"
Index: clang/test/CodeGen/keep-persistent-storage-variables.cpp
===
--- /dev/null
+++ clang/test/CodeGen/keep-persistent-storage-variables.cpp
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -fkeep-persistent-storage-variables -emit-llvm %s -o - -triple=x86_64-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 -fkeep-persistent-storage-variables -emit-llvm %s -o - -triple=powerpc64-ibm-aix-xcoff | FileCheck %s
+
+// CHECK: @_ZL2g1 = internal global i32 0, align 4
+// CHECK: @_ZL2g2 = internal global i32 1, align 4
+// CHECK: @tl1 = thread_local global i32 0, align 4
+// CHECK: @tl2 = thread_local global i32 3, align 4
+// CHECK: @_ZL3tl3 = internal thread_local global i32 0, align 4
+// CHECK: @_ZL3tl4 = internal thread_local global i32 4, align 4
+// CHECK: @g5 = global i32 0, align 4
+// CHECK: @g6 = global i32 6, align 4
+// CHECK: @_ZZ5test3vE2s3 = internal global i32 0, align 4
+// CHECK: @_ZN12_GLOBAL__N_12s4E = internal global i32 42, align 4
+// CHECK: @_ZZ5test5vE3tl5 = internal thread_local global i32 1, align 4
+// CHECK: @_ZN2ST2s6E = global i32 7, align 4
+// CHECK: @_Z2v7 = internal global %union.anon zeroinitializer, align 4
+// CHECK: @_ZDC2v8E = global %struct.ST8 zeroinitializer, align 4
+// CHECK: @llvm{{(\.compiler)?}}.used = appending global [14 x ptr] [ptr @_ZL2g1, ptr @_ZL2g2, ptr @tl1, ptr @tl2, ptr @_ZL3tl3, ptr @_ZL3tl4, ptr @g5, ptr @g6, ptr @_ZZ5test3vE2s3, ptr @_ZN12_GLOBAL__N_12s4E, ptr @_ZZ5test5vE3tl5, ptr @_ZN2ST2s6E, ptr @_Z2v7, ptr @_ZDC2v8E], section "llvm.metadata"
+
+static int g1;
+static int g2 = 1;
+__thread int tl1;
+__thread int tl2 = 3;
+static __thread int tl3;
+static __thread int tl4 = 4;
+int g5;
+int g6 = 6;
+
+int test3() {
+  static int s3 = 0;
+  ++s3;
+  return s3;
+}
+
+namespace {
+  int s4 = 42;
+}
+
+int test5() {
+  static __thread int tl5 = 1;
+  ++tl5;
+  return tl5;
+}
+
+struct ST {
+  static int s6;
+};
+int ST::s6 = 7;
+
+static union { int v7; };
+
+struct ST8 { int v8; };
+auto [v8] = ST8{0};
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7404,6 +7404,8 @@
 
   Args.addOptInFlag(CmdArgs, options::OPT_fkeep_static_consts,
 options::OPT_fno_keep_static_consts);
+  Args.addOptInFlag(CmdArgs, options::OPT_fkeep_persistent_storage_variables,
+options::OPT_fno_keep_persistent_storage_variables);
   Args.addOptInFlag(CmdArgs, options::OPT_fcomplete_member_pointers,
 options::OPT_fno_complete_member_pointers);
   Args.addOptOutFlag(CmdArgs, options::OPT_fcxx_static_destructors,
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -2426,12 +2426,14 @@
   if (D && D->hasAttr())
 addUsedOrCompilerUsedGlobal(GV);
 
-  if (CodeGenOpts.KeepStaticConsts && D && isa(D)) {
-const auto *VD = cast(D);
-if (VD->getType().isConstQualified() &&
-VD->getStorageDuration() == SD_Static)
-  addUsedOrCompilerUsedGlobal(GV);
-  }
+  if (const auto *VD = dyn_cast_if_present(D);
+  VD &&
+  ((CodeGenOpts.KeepPersistentStorageVariables &&
+(VD->getStorageDuration() == SD_Static ||
+ VD->getStorageDuration() == SD_Thread)) ||
+   (CodeGenOpts.KeepStaticConsts && VD->getStorageDuration() == SD_Static &&
+VD->getType().isConstQualified(
+addUsedOrCompilerUsedGlobal(GV);
 }
 
 

[PATCH] D150221: Add option -fkeep-persistent-storage-variables to emit all variables that have a persistent storage duration

2023-07-13 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

> Furthermore, there is some discussion over covering more than just variables, 
> but also artifacts with reasonable mangled names such as the symbols for 
> temporaries bound to references with "persistent storage" and guard variables.

We can follow up with a separate patch to add the extra functionality. This 
patch LGTM for the scope it covers.




Comment at: clang/include/clang/Driver/Options.td:1703
+  PosFlag, NegFlag,
+  BothFlags<[NoXarchOption], " keeping all variables that have a persistent 
storage duration, including global, static and thread local variables, to 
guarantee that they can be directly addressed">>;
 defm fixed_point : BoolFOption<"fixed-point",

Minor nit: Use hyphen in "thread-local".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150221

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


[PATCH] D150221: Add option -fkeep-persistent-storage-variables to emit all variables that have a persistent storage duration

2023-07-13 Thread Zheng Qian via Phabricator via cfe-commits
qianzhen updated this revision to Diff 540060.
qianzhen added a comment.

Add two more test cases


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150221

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/keep-persistent-storage-variables.cpp
  clang/test/Driver/fkeep-persistent-storage-variables.c

Index: clang/test/Driver/fkeep-persistent-storage-variables.c
===
--- /dev/null
+++ clang/test/Driver/fkeep-persistent-storage-variables.c
@@ -0,0 +1,5 @@
+// RUN: %clang -fkeep-persistent-storage-variables -c %s -### 2>&1 | FileCheck %s
+// RUN: %clang -fkeep-persistent-storage-variables -fno-keep-persistent-storage-variables -c %s -### 2>&1 | FileCheck %s -check-prefix=CHECK-NOKEEP
+
+// CHECK: "-fkeep-persistent-storage-variables"
+// CHECK-NOKEEP-NOT: "-fkeep-persistent-storage-variables"
Index: clang/test/CodeGen/keep-persistent-storage-variables.cpp
===
--- /dev/null
+++ clang/test/CodeGen/keep-persistent-storage-variables.cpp
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -fkeep-persistent-storage-variables -emit-llvm %s -o - -triple=x86_64-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 -fkeep-persistent-storage-variables -emit-llvm %s -o - -triple=powerpc64-ibm-aix-xcoff | FileCheck %s
+
+// CHECK: @_ZL2g1 = internal global i32 0, align 4
+// CHECK: @_ZL2g2 = internal global i32 1, align 4
+// CHECK: @tl1 = thread_local global i32 0, align 4
+// CHECK: @tl2 = thread_local global i32 3, align 4
+// CHECK: @_ZL3tl3 = internal thread_local global i32 0, align 4
+// CHECK: @_ZL3tl4 = internal thread_local global i32 4, align 4
+// CHECK: @g5 = global i32 0, align 4
+// CHECK: @g6 = global i32 6, align 4
+// CHECK: @_ZZ5test3vE2s3 = internal global i32 0, align 4
+// CHECK: @_ZN12_GLOBAL__N_12s4E = internal global i32 42, align 4
+// CHECK: @_ZZ5test5vE3tl5 = internal thread_local global i32 1, align 4
+// CHECK: @_ZN2ST2s6E = global i32 7, align 4
+// CHECK: @_Z2v7 = internal global %union.anon zeroinitializer, align 4
+// CHECK: @_ZDC2v8E = global %struct.ST8 zeroinitializer, align 4
+// CHECK: @llvm{{(\.compiler)?}}.used = appending global [14 x ptr] [ptr @_ZL2g1, ptr @_ZL2g2, ptr @tl1, ptr @tl2, ptr @_ZL3tl3, ptr @_ZL3tl4, ptr @g5, ptr @g6, ptr @_ZZ5test3vE2s3, ptr @_ZN12_GLOBAL__N_12s4E, ptr @_ZZ5test5vE3tl5, ptr @_ZN2ST2s6E, ptr @_Z2v7, ptr @_ZDC2v8E], section "llvm.metadata"
+
+static int g1;
+static int g2 = 1;
+__thread int tl1;
+__thread int tl2 = 3;
+static __thread int tl3;
+static __thread int tl4 = 4;
+int g5;
+int g6 = 6;
+
+int test3() {
+  static int s3 = 0;
+  ++s3;
+  return s3;
+}
+
+namespace {
+  int s4 = 42;
+}
+
+int test5() {
+  static __thread int tl5 = 1;
+  ++tl5;
+  return tl5;
+}
+
+struct ST {
+  static int s6;
+};
+int ST::s6 = 7;
+
+static union { int v7; };
+
+struct ST8 { int v8; };
+auto [v8] = ST8{0};
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7294,6 +7294,8 @@
 
   Args.addOptInFlag(CmdArgs, options::OPT_fkeep_static_consts,
 options::OPT_fno_keep_static_consts);
+  Args.addOptInFlag(CmdArgs, options::OPT_fkeep_persistent_storage_variables,
+options::OPT_fno_keep_persistent_storage_variables);
   Args.addOptInFlag(CmdArgs, options::OPT_fcomplete_member_pointers,
 options::OPT_fno_complete_member_pointers);
   Args.addOptOutFlag(CmdArgs, options::OPT_fcxx_static_destructors,
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -2198,12 +2198,14 @@
   if (D && D->hasAttr())
 addUsedOrCompilerUsedGlobal(GV);
 
-  if (CodeGenOpts.KeepStaticConsts && D && isa(D)) {
-const auto *VD = cast(D);
-if (VD->getType().isConstQualified() &&
-VD->getStorageDuration() == SD_Static)
-  addUsedOrCompilerUsedGlobal(GV);
-  }
+  if (const auto *VD = dyn_cast_if_present(D);
+  VD &&
+  ((CodeGenOpts.KeepPersistentStorageVariables &&
+(VD->getStorageDuration() == SD_Static ||
+ VD->getStorageDuration() == SD_Thread)) ||
+   (CodeGenOpts.KeepStaticConsts && VD->getStorageDuration() == SD_Static &&
+VD->getType().isConstQualified(
+addUsedOrCompilerUsedGlobal(GV);
 }
 
 bool CodeGenModule::GetCPUAndFeaturesAttributes(GlobalDecl GD,
@@ -3084,12 +3086,14 @@
   if (LangOpts.EmitAllDecls)
 return true;
 
-  if (CodeGenOpts.KeepStaticConsts) {
-const auto *VD = dyn_cast(Global);
-if (VD && 

[PATCH] D150221: Add option -fkeep-persistent-storage-variables to emit all variables that have a persistent storage duration

2023-07-05 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment.

Update (looping back from offline discussion): The LTO use case is covered. 
There was some confusion over which meaning of "static" was meant by 
`-fkeep-static-consts`. The "static" meant was storage duration.

Furthermore, there is some discussion over covering more than just variables, 
but also artifacts with reasonable mangled names such as the symbols for 
temporaries bound to references with "persistent storage" and guard variables.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150221

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


[PATCH] D150221: Add option -fkeep-persistent-storage-variables to emit all variables that have a persistent storage duration

2023-07-04 Thread Zheng Qian via Phabricator via cfe-commits
qianzhen added inline comments.



Comment at: clang/test/CodeGen/keep-persistent-storage-variables.cpp:32-39
+int test1() {
+  g1 = 3;
+  return g1;
+}
+
+int test2() {
+  return g2;

hubert.reinterpretcast wrote:
> Why add functions that use `g1` and `g2`? Is there some reason to suspect 
> that using the variables will cause them not to be emitted as explicitly used?
> 
> If removing these functions, please also remove `g3` and `g4` as redundant.
Right, I don't think using the variables in a function will cause then not to 
be emitted as explicitly used. Patch updated accordingly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150221

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