[PATCH] D78045: [XCOFF][AIX] Fix getSymbol to return the correct qualname when necessary

[David Tenty via Phabricator via llvm-commits]

daltenty marked an inline comment as done.
daltenty added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1712
-  // eventually.
-  const auto *GV = dyn_cast<const GlobalVariable>(GO);
-  if (GV) {
----------------
Does it matter we're losing this early error check? Will we end up creating symbols for unanticipated GV kinds before we hit the corresponding check in emitGlobalVariable (and do we care)?


================
Comment at: llvm/lib/Target/TargetMachine.cpp:263
+  if (getTargetTriple().isOSBinFormatXCOFF()) {
+    return TLOF->getTargetSymbol(GV, *this);
+  }
----------------
hubert.reinterpretcast wrote:
> jasonliu wrote:
> > **To reviewers**, 
> > I'm not sure if this is a preferred way to inject target specific behavior. Or we want to override TargetMachine::getSymbol directly instead.
> > Please let me know if you find the other way is more appealing, or you could think of a better way to inject this target specific behavior.
> > Thanks.
> Can `getTargetSymbol` return `nullptr` to indicate that "falling through" to the generic path is okay? This means that the three lines below don't need to be duplicated.
This seems the right way to go to me. Going through the TLOF seems appropriate since this is really an object format property, doing a override in the TargetMachine hierarchy doesn't really seem to capture that.


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

https://reviews.llvm.org/D78045