[PATCH] D150221: Add option -fkeep-static-variables to emit all static variables

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: rjmccall, efriedma.
aaron.ballman added subscribers: efriedma, rjmccall.
aaron.ballman added a comment.

In D150221#4338500 <https://reviews.llvm.org/D150221#4338500>, @qianzhen wrote:

> This is useful in keeping the static variables in a patchable function (https://clang.llvm.org/docs/AttributeReference.html#patchable-function-entry), so that they can be directly addressed by a hot patch when the optimization to merge them is enabled (https://llvm.org/docs/doxygen/GlobalMerge_8cpp_source.html).

I would expect that the user could write `__attribute__((used))` themselves in that case rather than use the heavy hammer of keeping all statics in the TU, right?

In D150221#4332280 <https://reviews.llvm.org/D150221#4332280>, @hubert.reinterpretcast wrote:

> In D150221#4332142 <https://reviews.llvm.org/D150221#4332142>, @erichkeane wrote:
>
>>> This is intended to prevent "excessive transformation" to enable migration of existing applications (using a non-Clang compiler) where users further manipulate the object or assembly after compilation.
>>
>> I don't get what you mean by this?  I don't currently see motivation for this?
>
> The intent is to apply the `__attribute__((__used__))` semantic to static variables (since the front-end is likely to discard them). Additional reasons for using `__attribute__((__used__))` applies: The compiler cannot optimize with the assumption that it sees all direct accesses to the variable.

The part I'm having a hard time understanding is why this should be a TU-level option when the user can write `__attribute__((used))` on any static that matters to them that is being dropped? This feels like a heavy hammer.

Adding @rjmccall and @efriedma as codegen code owners.



================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2201-2210
+  if ((CodeGenOpts.KeepStaticConsts || CodeGenOpts.KeepStaticVariables) && D &&
+      isa<VarDecl>(D)) {
     const auto *VD = cast<VarDecl>(D);
-    if (VD->getType().isConstQualified() &&
-        VD->getStorageDuration() == SD_Static)
-      addUsedOrCompilerUsedGlobal(GV);
+    if (VD->getStorageDuration() == SD_Static) {
+      if (CodeGenOpts.KeepStaticVariables)
+        addUsedOrCompilerUsedGlobal(GV);
+      else if (CodeGenOpts.KeepStaticConsts && VD->getType().isConstQualified())
----------------
Reformatted with whatever clang-format does for that, as I doubt I have the formatting correct.


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:3096
+        return true;
+      else if (CodeGenOpts.KeepStaticConsts && VD->getType().isConstQualified())
+        return true;
----------------
cebowleratibm wrote:
> Fold to a single return.
Yup, similar suggestion here as above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150221