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

[Leonard Chan via Phabricator via llvm-commits]

leonardchan added a comment.

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

> `llc -filetype=obj` doesn't. Please fix it. It is a bit unfortunate but the ELFObjectWriter generally has more restrictions than the assembler writer.

Ah, didn't see this. Yeah I was using this incorrectly in some of my tests. Fixed.



================
Comment at: llvm/lib/CodeGen/DSOLocalEquivalentLowering.cpp:83
+  GlobalValue *GV = Equiv->getGlobalValue();
+  if (GV->hasLocalLinkage() || GV->hasHiddenVisibility() || GV->isDSOLocal())
+    return GV;
----------------
MaskRay wrote:
> Shouldn't protected visibility use GV as well?
> 
> Is isDSOLocal implied by other properties?
Yeah, it seems that there's actually a function that covers this already: `GlobalValue::isImplicitDSOLocal`.


================
Comment at: llvm/test/CodeGen/X86/dso_local_equivalent.ll:71
+; NO-PLT-NEXT:   .quad   _ifunc_func.stub
+ at data_ifunc = constant i32 (i32)* dso_local_equivalent @ifunc_func
+
----------------
MaskRay wrote:
> ifunc stub may seem strange. Can you add a comment?
Added. This is here mostly to ensure correctness since the `ifunc` may not be `dso_local` so we'd still need a stub to ensure that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77248