[PATCH] D55046: Linker: Add support for GlobalIFunc.

Teresa Johnson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 28 22:39:07 PST 2018


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

LGTM with a few minor comments.



================
Comment at: llvm/include/llvm/Transforms/Utils/ValueMapper.h:185
+  void scheduleMapGlobalIndirectSymbol(GlobalIndirectSymbol &GIS,
+                                       Constant &Aliasee,
+                                       unsigned MappingContextID = 0);
----------------
Elsewhere this was changed to "Constant &Target", make this one consistent.


================
Comment at: llvm/lib/Linker/IRMover.cpp:590
 
   // When linking a global for an alias, it will always be linked. However we
   // need to check if it was not already scheduled to satisfy a reference from a
----------------
This comment block might need to be generalized since it talks about aliases


================
Comment at: llvm/test/LTO/Resolution/X86/ifunc2.ll:13
+; CHECK: define internal i32 @foo_resolver.2() {
+; CHECK-NEXT: ret i32 1
+
----------------
Nit: these 2 check lines correspond to the below @foo_resolver def, so it would make more sense to put them right before it, whereas the ones you check right before it are for the copy from the other module (which might make more sense shown after the below def).


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55046





More information about the llvm-commits mailing list