[PATCH] D77248: [llvm][IR] Add dso_local_equivalent Constant

[Leonard Chan via Phabricator via llvm-commits]

leonardchan added a comment.

In D77248#2390009 <https://reviews.llvm.org/D77248#2390009>, @MaskRay wrote:

> Apologies for my late reply. A couple of comments inlined
>
>> See RFC for more details.
>
> Neither the RFC nor the patch mentions the immediate use case. It is possible that the merit is buries in a deep message but for a few feature the description itself should justify its value (dblaikie mentioned that:
>
>> We should do this because I need it" (we shouldn't be implementing features for especially niche use cases/if they don't generalize) isn't always a compelling motivation but "we should do this because someone might need it" isn't either (we shouldn't be implementing features that have no users).
>
> Please add the use cases.

No worries. Thanks for the reviews! I added the use case in the LangRef. Essentially the immediate use case is for using static relocations in vtables under the new relative vtables ABI <https://reviews.llvm.org/D72959>. This constant gives us a generic way to essentially "request" a PLT relocation from Clang without needing to explicitly say in the frontend "these targets support PLT entries".



================
Comment at: llvm/include/llvm/IR/Constants.h:892
+/// Wrapper for a function that represents a value that
+/// functionally represents the original variable. This can be a function,
+/// global alias to a function, or an ifunc.
----------------
MaskRay wrote:
> The comment says "function" then "variable". Is "variable" a typo?
Yup, it should still be function. Fixed.


================
Comment at: llvm/lib/CodeGen/DSOLocalEquivalentLowering.cpp:51
+    if (Triple(M.getTargetTriple()).supportsCOMDAT())
+      Stub->setComdat(M.getOrInsertComdat(StubName));
+  }
----------------
MaskRay wrote:
> ELF has comdat but does not need this pass.
> Mach-O needs this pass but does not have comdat.
> 
> I assume that this code is untested.
Added some cases for `COFF` where this pass is needed and comdats are supported.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77248