[PATCH] D105176: [InstrProfiling] Use external linkage for bias variable

[Roland McGrath via Phabricator via llvm-commits]

mcgrathr added a comment.

It makes sense that the contract be that the runtime must define the variable.

I don't understand how this change is accomplishing that, however.  From the IR in the test case, it's become a hidden global definition in a COMDAT group.  AFAIK, that's what `linkonce_odr` actually translates to in ELF backends anyway.  What's the difference in the .s or .o file generated from that IR?  I would expect to just use a vanilla undefined external symbol here.



================
Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:693
       if (!Bias) {
-        Bias = new GlobalVariable(*M, Int64Ty, false, GlobalValue::LinkOnceODRLinkage,
                                   Constant::getNullValue(Int64Ty),
----------------
This merits a comment about the contract, e.g. "The runtime must define this variable."


================
Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:698
+        if (TT.supportsCOMDAT())
+            Bias->setComdat(M->getOrInsertComdat(Bias->getName()));
       }
----------------
What's the meaning of comdat on an undefined external?
There is no such concept at the object file level, at least not in ELF.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105176