[PATCH] D104058: ThinLTO: Fix inline assembly references to static functions with CFI

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp:77
+      // references from inline assembly.
+      std::string Alias = ".set \"" + OldName + "\",\"" + NewName + "\"\n";
+      ExportM.appendModuleInlineAsm(Alias);
----------------
nickdesaulniers wrote:
> nickdesaulniers wrote:
> > Is there more information about "promotion aliases with x86_64-pc-windows-msvc" from D106392? Can we quote these only for msvc target triples? Can we add a comment about the quoting be necessary for those targets?
> > 
> > It's still not clear to me how tests using explicit -linux-gnu triples could fail on -mscv hosts.
> Also, I think the quoting hurts the readability of the generated asm.  Maybe that doesn't matter for LTO, but I'd be curious if we could do such escaping only when necessary? Perhaps that's only when targeting -msvc triples?
I was going to say, the proper way to do this is what `MCSymbol::print` does:
https://github.com/llvm/llvm-project/blob/main/llvm/lib/MC/MCSymbol.cpp#L59

That code doesn't seem exhaustive, but at least it escapes quotes. We can't call MC from here due to library layering.

The LLVM IR readability is poor, but inline asm in IR is already hard to read. I wouldn't worry about that, I'd mainly worry about the output of -S.

Quoting always seems fine to me, I guess.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104058