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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay requested changes to this revision.
MaskRay added a comment.
This revision now requires changes to proceed.

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



================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2290
 
+  if (const auto *Equiv = dyn_cast<DSOLocalEquivalent>(CV)) {
+    return getObjFileLowering().lowerDSOLocalEquivalent(Equiv, TM);
----------------
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements


================
Comment at: llvm/lib/CodeGen/DSOLocalEquivalentLowering.cpp:24
+
+Function *GetOrCreateRelativeStub(GlobalValue *GV) {
+  SmallString<16> StubName(GV->getName());
----------------
Use static

https://llvm.org/docs/CodingStandards.html#anonymous-namespaces

lowerCase


================
Comment at: llvm/lib/CodeGen/DSOLocalEquivalentLowering.cpp:81
+
+Value *LowerDSOLocalEquivalent(DSOLocalEquivalent *Equiv) {
+  GlobalValue *GV = Equiv->getGlobalValue();
----------------
lowerCase.

This either needs `static` or should stick with https://llvm.org/docs/CodingStandards.html#use-namespace-qualifiers-to-implement-previously-declared-functions


================
Comment at: llvm/lib/CodeGen/DSOLocalEquivalentLowering.cpp:83
+  GlobalValue *GV = Equiv->getGlobalValue();
+  if (GV->hasLocalLinkage() || GV->hasHiddenVisibility() || GV->isDSOLocal())
+    return GV;
----------------
Shouldn't protected visibility use GV as well?

Is isDSOLocal implied by other properties?


================
Comment at: llvm/test/CodeGen/X86/dso_local_equivalent.ll:38
+; NO-PLT:      _data2:
+; NO-PLT-NEXT:   .quad   _hidden_func
+
----------------
`_hidden_func{{$}}`


================
Comment at: llvm/test/CodeGen/X86/dso_local_equivalent.ll:43
+; NO-PLT:      _data3:
+; NO-PLT-NEXT:   .quad   _dso_local_func
+
----------------
ditto


================
Comment at: llvm/test/CodeGen/X86/dso_local_equivalent.ll:48
+; NO-PLT:      _data4:
+; NO-PLT-NEXT:   .quad   _internal_func
+
----------------
ditto


================
Comment at: llvm/test/CodeGen/X86/dso_local_equivalent.ll:53
+; NO-PLT:      _data5:
+; NO-PLT-NEXT:   .quad   l_private_func
+ at data2 = constant void ()* dso_local_equivalent @hidden_func
----------------
ditto


================
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
+
----------------
ifunc stub may seem strange. Can you add a comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77248