hubert.reinterpretcast added inline comments.
================
Comment at: clang/include/clang/Basic/CodeGenOptions.def:475
+/// Whether to emit all variables that have a persisent storage duration,
+/// including global, static and thread local variables.
----------------
Minor nit: Typo.
================
Comment at: clang/include/clang/Driver/Options.td:1700
BothFlags<[NoXarchOption], " static const variables if unused">>;
+defm keep_persistent_storage_variables :
BoolFOption<"keep-persistent-storage-variables",
+ CodeGenOpts<"KeepPersistentStorageVariables">, DefaultFalse,
----------------
Please update the patch title and description to match.
================
Comment at: clang/test/CodeGen/keep-persistent-storage-variables.cpp:18-19
+// CHECK: @_ZN2ST2s6E = global i32 7, align 4
+// CHECK-ELF: @llvm.compiler.used = appending global [14 x ptr] [ptr @_ZL2g1,
ptr @_ZL2g2, ptr @_ZL2g3, ptr @_ZL2g4, ptr @tl1, ptr @tl2, ptr @_ZL3tl3, ptr
@_ZL3tl4, ptr @g5, ptr @g6, ptr @_ZZ5test3vE2s3, ptr @_ZN12_GLOBAL__N_12s4E,
ptr @_ZZ5test5vE3tl5, ptr @_ZN2ST2s6E], section "llvm.metadata"
+// CHECK-NON-ELF: @llvm.used = appending global [14 x ptr] [ptr @_ZL2g1, ptr
@_ZL2g2, ptr @_ZL2g3, ptr @_ZL2g4, ptr @tl1, ptr @tl2, ptr @_ZL3tl3, ptr
@_ZL3tl4, ptr @g5, ptr @g6, ptr @_ZZ5test3vE2s3, ptr @_ZN12_GLOBAL__N_12s4E,
ptr @_ZZ5test5vE3tl5, ptr @_ZN2ST2s6E], section "llvm.metadata"
+
----------------
If we don't care which of `llvm.compiler.used` or `llvm.used` is used (no pun
intended), then maybe we can use a regex? For example,
`@llvm{{(\.compiler)?}}.used`.
================
Comment at: clang/test/CodeGen/keep-persistent-storage-variables.cpp:32-39
+int test1() {
+ g1 = 3;
+ return g1;
+}
+
+int test2() {
+ return g2;
----------------
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.
================
Comment at: clang/test/CodeGen/keep-persistent-storage-variables.cpp:50
+}
+void *test4() { return &s4; }
+
----------------
Same comment re: why add a function?
================
Comment at: clang/test/Driver/fkeep-persistent-storage-variables.c:1
+// RUN: %clang -fkeep-persistent-storage-variables -c %s -### 2>&1 | FileCheck
%s
+
----------------
Please add test for `-fkeep-persistent-storage-variables
-fno-keep-persistent-storage-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
