[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

I think the main user-facing feature here to think about is `__attribute__((used))`. I see that it's currently listed as Undocumented. We should fix that while we're rehashing how this is supposed to work, and clarify what it does on each platform. As I understand it, `__attribute__((used))` globals will be GC roots on Mac/Win, but not ELF targets. The mismatch in behavior is unfortunate, but it matches the dominant system compilers on those platforms.

Here are all the existing addUsedGlobal call sites: https://reviews.llvm.org/P8257 I categorize them as:

- ObjC/blocks: this wants GC root semantics, since ObjC mainly runs on Mac.
- MS C++ ABI stuff: wants GC root semantics, no change
- OpenMP: unsure, but GC root semantics probably don't hurt
- CodeGenModule: affected in this patch to *not* use GC root semantics so that `__attribute__((used))` behavior remains the same on ELF, plus two other minor use cases that don't want GC semantics
- Coverage: Probably want GC root semantics
- CGExpr.cpp: refers to LTO, wants GC root
- CGDeclCXX.cpp: one is MS ABI specific, so yes GC root, one is some other C++ init functionality, which should form GC roots (C++ initializers can have side effects and must run)
- CGDecl.cpp: Changed in this patch for `__attribute__((used))`

So, looks good, I think you spotted all the things that should change.



================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1829
         VD->getStorageDuration() == SD_Static)
-      addUsedGlobal(GV);
+      addUsedOrCompilerUsedGlobal(GV);
   }
----------------
Should -fkeep-static-consts be getting GC root semantics on Mac/Win? We can leave this as behavior preserving, I don't think -fkeep-static-consts is a very important feature.


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5924
     if (Val && !getModule().getNamedValue(Name->getName()))
-      addUsedGlobal(llvm::GlobalAlias::create(Name->getName(), Val));
+      addUsedOrCompilerUsedGlobal(
+          llvm::GlobalAlias::create(Name->getName(), Val));
----------------
This probably doesn't need GC semantics, and should always used llvm.compiler.used on all platforms. This alias will be internal, this factory function it takes the linkage from the aliasee.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97446