[PATCH] D150221: Add option -fkeep-persistent-storage-variables to emit all variables that have a persistent storage duration
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
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
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
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
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