[PATCH] D108872: [IR] Refactor GlobalIFunc to inherit from GlobalObject, Remove GlobalIndirectSymbol

[Fangrui Song via Phabricator via llvm-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

Thanks. This makes sense. In the inheritance hierachy, `GlobalIndirectSymbol` does nothing useful now and can just be removed.

I'll wait a few days and commit on your behalf.



================
Comment at: llvm/docs/LangRef.rst:910
 
-    @<Name> = [Linkage] [Visibility] ifunc <IFuncTy>, <ResolverTy>* @<Resolver>
+    @<Name> = [Linkage] [PreemptionSpecifier] [Visibility] ifunc <IFuncTy>, <ResolverTy>* @<Resolver>
 
----------------
ibookstein wrote:
> MaskRay wrote:
> > Drop the change?
> > 
> > An ifunc definition can be interposed/preempted.
> The change only documents the existing situation, though, as far as I can tell.
> There are tests which check/specify a [PreemptionSpecifier]; If I remember correctly, if you'd drop the PrintDSOLocation/maybeSetDSOLocal you'd get test failures.
> I searched for "dso_local ifunc" and got some hits from the tests.
If that's the case. The documentation change can be pushed separately to reflect the fact that it is unrelated to this change.

Pushed f66b1b2717e84edef8a9e132638de6d866d01eab


================
Comment at: llvm/lib/AsmParser/LLParser.cpp:1067
-  // The module owns this now
-  GA.release();
 
----------------
Is this unneeded now?


================
Comment at: llvm/lib/Transforms/IPO/LowerTypeTests.cpp:373
+
+    for (auto &GI : M.ifuncs()) {
+      if (auto *F = dyn_cast<Function>(GI.getResolver()->stripPointerCasts()))
----------------
drop braces according to https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements


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

https://reviews.llvm.org/D108872