[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:658
+  std::string S;
+  raw_string_ostream OS(S);
+  if (Arch == Triple::x86 || Arch == Triple::x86_64) {
----------------
Could be simpler to have a single `AsmOS` that everyone writes to, then a single `appendModuleInlineAsm` at the end?


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


================
Comment at: lib/Transforms/IPO/LowerTypeTests.cpp:700
 
+void LowerTypeTestsModule::addUsed(ArrayRef<Function *> Functions) {
+  GlobalVariable *GV = M.getGlobalVariable("llvm.used");
----------------
http://llvm-cs.pcc.me.uk/?q=setSection.*llvm.metadata

We have code like this all over the place. May I ask you to add a general utility function for this?


================
Comment at: lib/Transforms/IPO/LowerTypeTests.cpp:820
+  auto JumpTable =
+      new GlobalVariable(M, JumpTableType,
+                         /*isConstant=*/true, GlobalValue::ExternalLinkage, // FIXME: private
----------------
This could just be Int8Ty since the type isn't important any more.


Repository:
  rL LLVM

https://reviews.llvm.org/D25927