[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

rjmccall added a comment. In D97446#3241828 , @JonChesterfield wrote: > I'd much refer we put variables in arrays corresponding to their intended > lifespan instead of the binary format they're destined for. I agree that LLVM features should generally m

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

MaskRay marked 2 inline comments as done. MaskRay added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2093 + "Only globals with definition can force usage."); + if (getTriple().isOSBinFormatELF()) +LLVMCompilerUsed.emplace_back(GV); ---

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

JonChesterfield added a comment. In D97446#3241735 , @MaskRay wrote: > For your comment it appears an issue in the internalisation pass. It is > orthogonal to this patch. > Do you have a reduced example with reproduce instructions for the issue? I > know

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

rjmccall added a comment. I can understand how we got here, but it's a bad place to have ended up. Toolchains that do dead-stripping need a way to prevent it for specific objects and functions. Windows and Darwin toolchains have historically done aggressive dead-stripping in both the compiler

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

MaskRay added a comment. In D97446#3241617 , @JonChesterfield wrote: >> Modulo optimizer bugs, __attribute__((used)) hasn't changed semantics. >> If your downstream project does not handle llvm.compiler.used, maybe handle >> it now :) > > There seems to

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

JonChesterfield added a comment. > Modulo optimizer bugs, __attribute__((used)) hasn't changed semantics. > If your downstream project does not handle llvm.compiler.used, maybe handle > it now :) There seems to be some confusion here. The 'downstream project' is openmp, which has worked around

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

MaskRay added a comment. In D97446#3241416 , @JonChesterfield wrote: > It looks deeply wrong to be marking globals as either llvm.used or > llvm.compiler.used based on the output file format. It should be based on the > (purpose of) the global. > > In D

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

JonChesterfield added a comment. It looks deeply wrong to be marking global as either llvm.used or llvm.compiler.used based on the output file format. It should be based on the (purpose of) the global. In D97446#3241142 , @rnk wrote: > This change was i

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

MaskRay added a comment. In D97446#3241142 , @rnk wrote: > In D97446#3240309 , @JonChesterfield > wrote: > >> If I had a red buildbot to reference I would have reverted - the commit >> message does not make it cle

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

rnk added a comment. In D97446#3240309 , @JonChesterfield wrote: > I don't know the context of this patch but changing attribute((used)) to put > things under compiler.used is definitely a breaking change. Please introduce > a new attribute if necessary

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

JonChesterfield reopened this revision. JonChesterfield added a comment. This revision is now accepted and ready to land. I don't know the context of this patch but changing attribute((used)) to put things under compiler.used is definitely a breaking change. Please introduce a new attribute if n

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG28cb620321f5: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

MaskRay updated this revision to Diff 326739. MaskRay added a comment. Simplify with `CodeGenModule::getTriple()` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97446/new/ https://reviews.llvm.org/D97446 Files: clang/lib/CodeGen/CGDecl.cpp clan

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm > 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, __

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

MaskRay updated this revision to Diff 326480. MaskRay edited the summary of this revision. MaskRay added a comment. Herald added a reviewer: jdoerfert. Herald added a subscriber: sstefan1. Adopted rnk's list (Thanks!) Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

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, `__at

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

probinson added a comment. In D97446#2587996 , @MaskRay wrote: > In D97446#2587806 , @probinson wrote: > >>> GNU ld has a rule "start_/stop_ references from a live input section retain >>> the associated C identifi

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

MaskRay added a comment. In D97446#2587806 , @probinson wrote: >> GNU ld has a rule "start_/stop_ references from a live input section retain >> the associated C identifier name sections", > > which LLD may change in the future (D96914

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

probinson added a comment. > GNU ld has a rule "start_/stop_ references from a live input section retain > the associated C identifier name sections", which LLD may change in the future (D96914 ). The phrasing "may change" implies LLD could eliminate the rule; i

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

MaskRay created this revision. MaskRay added reviewers: aaron.ballman, rjmccall, rnk, rsmith. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. An global value in the `llvm.used` list does not have GC root semantics on ELF targets. T