[PATCH] D25927: [cfi] Implement cfi-icall using inline assembly.

[Peter Collingbourne via llvm-commits]

pcc added inline comments.


================
Comment at: lib/Transforms/IPO/LowerTypeTests.cpp:676
+  OS << ".globl " << Dest->getName() << "\n";
+  OS << ".type " << Dest->getName() << ", function\n";
+  OS << Dest->getName() << " = " << JumpTable->getName() << " + "
----------------
eugenis wrote:
> pcc wrote:
> > Don't you want to copy the linkage from the original function here?
> done, I think
Also weak?


================
Comment at: lib/Transforms/IPO/LowerTypeTests.cpp:820
+  auto JumpTable =
+      new GlobalVariable(M, JumpTableType,
+                         /*isConstant=*/true, GlobalValue::ExternalLinkage, // FIXME: private
----------------
eugenis wrote:
> pcc wrote:
> > This could just be Int8Ty since the type isn't important any more.
> The type matters for the GEP at line 830
Sorry, yes, thanks.


================
Comment at: lib/Transforms/IPO/LowerTypeTests.cpp:669
+  if (!Dest->hasLocalLinkage())
+    OS << ".globl " << Dest->getName() << "\n";
+  OS << ".type " << Dest->getName() << ", function\n";
----------------
I wonder whether we want to escape these names? This appears to be the escaping we'd need to use:
http://llvm-cs.pcc.me.uk/lib/MC/MCSymbol.cpp#53

Maybe it's not necessary though since we pretty much expect all names we see to satisfy `MCAsmInfo::isValidUnquotedName`.


Repository:
  rL LLVM

https://reviews.llvm.org/D25927