[LLVM-C] Audit Inline Assembly APIs for Consistency

[CodaFi]

Summary

• Add a missing getter for module-level inline assembly
• Add a missing append function for module-level inline assembly
• Deprecate LLVMSetModuleInlineAsm and replace it with LLVMSetModuleInlineAsm2 which takes an explicit length parameter
• Deprecate LLVMConstInlineAsm and replace it with LLVMGetInlineAsm, a function that allows passing a dialect and is not mis-classified as a constant operation

[CodaFi]

The original API for LLVMSetModuleInlineAsm implies that only C-style strings can be passed here, but the implementation is copying to a std::string under the hood anyhow. If that's the invariant that we want to maintain here, I will revert the deprecation and add documentation.

[whitequark]

Excellent work, thanks!

Regarding LLVMSetModuleInlineAsm2, I have two thoughts:

1. It is likely that actually passing a literal \0 in inline assembly will confuse the assembler. In fact, I think both internal and external assemblers will refuse to process it.
2. Silently truncating on \0 is likely to hide a bug elsewhere instead, so an explicit error is good.
3. Consistency with other APIs is good, and making the job of FFI bindings easier is also good (some of them autogenerate code from signatures).
4. Code churn is bad, and in this case even the above isn't a particularly strong justification for code churn.

As a result, I think we should merge this as-is, but do not take aggressive steps to remove LLVMSetModuleInlineAsm. It is enough that all new code will use LLVMSetModuleInlineAsm2.

[whitequark]

Oh sorry, I've missed one API issue.

include/llvm-c/Core.h

684

I think this should be const char *AsmString, size_t AsmStringSize, const char *Constraints, size_t ConstraintsSize.

[whitequark]

include/llvm-c/Core.h

684

Or rather const char *AsmString, size_t AsmStringLen, const char *Constraints, size_t ConstraintsLen.

[CodaFi]

include/llvm-c/Core.h

684

Aha, good catch.

include/llvm-c/Core.h

 
 typedef enum {
     LLVMDSError,
     LLVMDSWarning,
     LLVMDSRemark,
     LLVMDSNote
 } LLVMDiagnosticSeverity;
 
+typedef enum {
+  LLVMInlineAsmDialectATT,
+  LLVMInlineAsmDialectIntel
+} LLVMInlineAsmDialect;
+
 /**
  * Attribute index are either LLVMAttributeReturnIndex,
  * LLVMAttributeFunctionIndex or a parameter number from 1 to N.
  */
 enum {
   LLVMAttributeReturnIndex = 0U,
   // ISO C restricts enumerator values to range of 'int'
   // (4294967295 is too large)

  * Return a string representation of the module. Use
  * LLVMDisposeMessage to free the string.
  *
  * @see Module::print()
  */
 char *LLVMPrintModuleToString(LLVMModuleRef M);
 
 /**
+ * Get inline assembly for a module.
+ *
+ * @see Module::getModuleInlineAsm()
+ */
+const char *LLVMGetModuleInlineAsm(LLVMModuleRef M, size_t *Len);
+
+/**
  * Set inline assembly for a module.
  *
  * @see Module::setModuleInlineAsm()
  */
-void LLVMSetModuleInlineAsm(LLVMModuleRef M, const char *Asm);
+void LLVMSetModuleInlineAsm2(LLVMModuleRef M, const char *Asm, size_t Len);
+
+/**
+ * Append inline assembly to a module.
+ *
+ * @see Module::appendModuleInlineAsm()
+ */
+void LLVMAppendModuleInlineAsm(LLVMModuleRef M, const char *Asm, size_t Len);
+
+/**
+ * Create the specified uniqued inline asm string.
+ *
+ * @see InlineAsm::get()
+ */
+LLVMValueRef LLVMGetInlineAsm(LLVMTypeRef Ty,
+                              char *AsmString, size_t AsmStringSize,
+                              char *Constraints, size_t ConstraintsSize,
+                              LLVMBool HasSideEffects, LLVMBool IsAlignStack,
+                              LLVMInlineAsmDialect Dialect);
 
 /**
  * Obtain the context to which this module is associated.
  *
  * @see Module::getContext()
  */
 LLVMContextRef LLVMGetModuleContext(LLVMModuleRef M);
 

 /**
  * Decrement a Function iterator to the previous Function.
  *
  * Returns NULL if the iterator was already at the beginning and there are
  * no previous functions.
  */
 LLVMValueRef LLVMGetPreviousFunction(LLVMValueRef Fn);
 
+/** Deprecated: Use LLVMSetModuleInlineAsm2 instead. */
+void LLVMSetModuleInlineAsm(LLVMModuleRef M, const char *Asm);
+
 /**
  * @}
  */
 
 /**
  * @defgroup LLVMCCoreType Types
  *
  * Types represent the type of a value.

 LLVMValueRef LLVMConstShuffleVector(LLVMValueRef VectorAConstant,
                                     LLVMValueRef VectorBConstant,
                                     LLVMValueRef MaskConstant);
 LLVMValueRef LLVMConstExtractValue(LLVMValueRef AggConstant, unsigned *IdxList,
                                    unsigned NumIdx);
 LLVMValueRef LLVMConstInsertValue(LLVMValueRef AggConstant,
                                   LLVMValueRef ElementValueConstant,
                                   unsigned *IdxList, unsigned NumIdx);
+LLVMValueRef LLVMBlockAddress(LLVMValueRef F, LLVMBasicBlockRef BB);
+
+/** Deprecated: Use LLVMGetInlineAsm instead. */
 LLVMValueRef LLVMConstInlineAsm(LLVMTypeRef Ty,
                                 const char *AsmString, const char *Constraints,
                                 LLVMBool HasSideEffects, LLVMBool IsAlignStack);
-LLVMValueRef LLVMBlockAddress(LLVMValueRef F, LLVMBasicBlockRef BB);
 
 /**
  * @}
  */
 
 /**
  * @defgroup LLVMCCoreValueConstantGlobals Global Values
  *

lib/IR/Core.cpp

char *LLVMPrintModuleToString(LLVMModuleRef M) {
 
   unwrap(M)->print(os, nullptr);
   os.flush();
 
   return strdup(buf.c_str());
 }
 
 /*--.. Operations on inline assembler ......................................--*/
+void LLVMSetModuleInlineAsm2(LLVMModuleRef M, const char *Asm, size_t Len) {
+  unwrap(M)->setModuleInlineAsm(StringRef(Asm, Len));
+}
+
 void LLVMSetModuleInlineAsm(LLVMModuleRef M, const char *Asm) {
   unwrap(M)->setModuleInlineAsm(StringRef(Asm));
 }
 
+void LLVMAppendModuleInlineAsm(LLVMModuleRef M, const char *Asm, size_t Len) {
+  unwrap(M)->appendModuleInlineAsm(StringRef(Asm, Len));
+}
+
+const char *LLVMGetModuleInlineAsm(LLVMModuleRef M, size_t *Len) {
+  auto &Str = unwrap(M)->getModuleInlineAsm();
+  *Len = Str.length();
+  return Str.c_str();
+}
+
+LLVMValueRef LLVMGetInlineAsm(LLVMTypeRef Ty,
+                              char *AsmString, size_t AsmStringSize,
+                              char *Constraints, size_t ConstraintsSize,
+                              LLVMBool HasSideEffects, LLVMBool IsAlignStack,
+                              LLVMInlineAsmDialect Dialect) {
+  InlineAsm::AsmDialect AD;
+  switch (Dialect) {
+  case LLVMInlineAsmDialectATT:
+    AD = InlineAsm::AD_ATT;
+  case LLVMInlineAsmDialectIntel:
+    AD = InlineAsm::AD_Intel;
+  }
+  return wrap(InlineAsm::get(unwrap<FunctionType>(Ty),
+                             StringRef(AsmString, AsmStringSize),
+                             StringRef(Constraints, ConstraintsSize),
+                             HasSideEffects, IsAlignStack, AD));
+}
+
 
 /*--.. Operations on module contexts ......................................--*/
 LLVMContextRef LLVMGetModuleContext(LLVMModuleRef M) {
   return wrap(&unwrap(M)->getContext());
 }
 
 
 /*===-- Operations on types -----------------------------------------------===*/

test/Bindings/llvm-c/echo.ll

 ; RUN: llvm-as < %s | llvm-dis > %t.orig
 ; RUN: llvm-as < %s | llvm-c-test --echo > %t.echo
 ; RUN: diff -w %t.orig %t.echo
 
 source_filename = "/test/Bindings/echo.ll"
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.11.0"
 
+module asm "classical GAS"
+
 %S = type { i64, %S* }
 
 @var = global i32 42
 @ext = external global i32*
 @cst = constant %S { i64 1, %S* @cst }
 @tl = thread_local global { i64, %S* } { i64 1, %S* @cst }
 @arr = linkonce_odr global [5 x i8] [ i8 2, i8 3, i8 5, i8 7, i8 11 ]
 @str = private unnamed_addr constant [13 x i8] c"hello world\0A\00"

tools/llvm-c-test/echo.cpp

int llvm_echo(void) {
   LLVMSetSourceFileName(M, SourceFileName, SourceFileLen);
   LLVMSetModuleIdentifier(M, ModuleName, ModuleIdentLen);
 
   LLVMSetTarget(M, LLVMGetTarget(Src));
   LLVMSetModuleDataLayout(M, LLVMGetModuleDataLayout(Src));
   if (strcmp(LLVMGetDataLayoutStr(M), LLVMGetDataLayoutStr(Src)))
     report_fatal_error("Inconsistent DataLayout string representation");
 
+  size_t ModuleInlineAsmLen;
+  const char *ModuleAsm = LLVMGetModuleInlineAsm(Src, &ModuleInlineAsmLen);
+  LLVMSetModuleInlineAsm2(M, ModuleAsm, ModuleInlineAsmLen);
+
   declare_symbols(Src, M);
   clone_symbols(Src, M);
   char *Str = LLVMPrintModuleToString(M);
   fputs(Str, stdout);
 
   LLVMDisposeMessage(Str);
   LLVMDisposeModule(Src);
   LLVMDisposeModule(M);
   LLVMContextDispose(Ctx);
 
   return 0;
 }