Restore the capability to manipulate datalayout from the C API

[deadalnix]

Summary

This consist in variosu addition to the C API:

LLVMTargetDataRef LLVMGetModuleDataLayout(LLVMModuleRef M);
void LLVMSetModuleDataLayout(LLVMModuleRef M, LLVMTargetDataRef DL);
LLVMTargetDataRef LLVMCreateTargetMachineData(LLVMTargetMachineRef T);

This also removes LLVMAddTargetData, which does nothing and is historical cruft.

[mehdi_amini]

include/llvm-c/Target.h

189–190

Separate change?

191

Not clear why it isn't at the same place as LLVMGetDataLayout

195

comment seems wrong

203

split code motion in a separate patch (you may commit it and rebase this one).

include/llvm-c/TargetMachine.h

91

remove

include/llvm/IR/DataLayout.h

471 ↗

(On Diff #47957)

do not mix formatting change with other changes please

lib/IR/Core.cpp

166 ↗

(On Diff #47957)

Are you planning to deprecate this? Why a new duplicated API?

173 ↗

(On Diff #47957)

You can commit this change separately now.

lib/Target/TargetMachineC.cpp

177

I don't really like how we're not consistent on the naming. "Data" instead of "DataLayout" has always bothered me.
(Just mentioning, not your fault).

[mehdi_amini]

Oh, and since it is your domain: what about the tests?
Can you add a triple and a random data layout (not the default one) to the echo test?

[deadalnix]

include/llvm-c/Target.h

191

LLVMGetDataLayout really should be named LLVMGetDataLayoutStr as it maps getDataLayourStr, which is the string representation of the layout.

lib/IR/Core.cpp

166 ↗

(On Diff #47957)

Yes, the name is not only plain wrong but is also the name of anther method that does something else, which is confusing.

173 ↗

(On Diff #47957)

Got you. Will do.

lib/Target/TargetMachineC.cpp

177

Agreed. Problem is, the good names are taken here already, so I had to come up with something.

[mehdi_amini]

include/llvm-c/Target.h

191

This is a getter on the module right? I'm asking about the location in the header: why here and not in include/llvm-c/Core.h where the other getter for Module are?

lib/IR/Core.cpp

166 ↗

(On Diff #47957)

So mark it deprecated with a mention to use the other one. Also this sounds like a separate change to the C API (i.e. I'd say a separate review).

lib/Target/TargetMachineC.cpp

177

LLVMCreateTargetDataLayout ? LLVMCreateDataLayoutForTarget ?

[deadalnix]

include/llvm-c/Target.h

191

Yes, but LLVMTargetDataRef is not available from Core.h and making it so would be a bigger change. Also, all LLVMTargetDataRef related method are here, so I don't think this is that much of an issue.

[deadalnix]

Update as per comments

[mehdi_amini]

include/llvm-c/Target.h

191

LLVMTargetDataRef is not available from Core.h

That's a good reason :)

195

comment is still wrong?

lib/Target/TargetMachineC.cpp

177

What about one of these name? (you haven't answered so I'm not even sure you saw it)

[mehdi_amini]

I also asked earlier about adding a data layout and a triple in the "echo" test?

[deadalnix]

Fix comment
Update name

[deadalnix]

Add check in the echo C API test

[mehdi_amini]

LGTM.

include/llvm-c/Target.h

#ifdef LLVM_NATIVE_DISASSEMBLER
   return 0;
 #else
   return 1;
 #endif
 }
 
 /*===-- Target Data -------------------------------------------------------===*/
 
+/**
+ * Obtain the data layout for a module.
+ *
+ * @see Module::getDataLayout()
+ */
+LLVMTargetDataRef LLVMGetModuleDataLayout(LLVMModuleRef M);
+
+/**
+ * Set the data layout for a module.
+ *
+ * @see Module::setDataLayout()
+ */
+void LLVMSetModuleDataLayout(LLVMModuleRef M, LLVMTargetDataRef DL);
+
 /** Creates target data from a target layout string.
     See the constructor llvm::DataLayout::DataLayout. */
 LLVMTargetDataRef LLVMCreateTargetData(const char *StringRep);
 
 /** Adds target library information to a pass manager. This does not take
     ownership of the target library info.
     See the method llvm::PassManagerBase::add. */
 void LLVMAddTargetLibraryInfo(LLVMTargetLibraryInfoRef TLI,
                               LLVMPassManagerRef PM);
 
 /** Converts target data to a target layout string. The string must be disposed
     with LLVMDisposeMessage.
     See the constructor llvm::DataLayout::DataLayout. */

include/llvm-c/TargetMachine.h

 
 /** Returns if the target has a TargetMachine associated */
 LLVMBool LLVMTargetHasTargetMachine(LLVMTargetRef T);
 
 /** Returns if the target as an ASM backend (required for emitting output) */
 LLVMBool LLVMTargetHasAsmBackend(LLVMTargetRef T);
 
 /*===-- Target Machine ----------------------------------------------------===*/
 /** Creates a new llvm::TargetMachine. See llvm::Target::createTargetMachine */
 LLVMTargetMachineRef LLVMCreateTargetMachine(LLVMTargetRef T,
   const char *Triple, const char *CPU, const char *Features,
   LLVMCodeGenOptLevel Level, LLVMRelocMode Reloc, LLVMCodeModel CodeModel);
 
 /** Dispose the LLVMTargetMachineRef instance generated by
   LLVMCreateTargetMachine. */
 void LLVMDisposeTargetMachine(LLVMTargetMachineRef T);
 
/** Returns the cpu used creating this target machine. See
   LLVMDisposeMessage. */
 char *LLVMGetTargetMachineCPU(LLVMTargetMachineRef T);
 
 /** Returns the feature string used creating this target machine. See
   llvm::TargetMachine::getFeatureString. The result needs to be disposed with
   LLVMDisposeMessage. */
 char *LLVMGetTargetMachineFeatureString(LLVMTargetMachineRef T);
 
+/** Create a DataLayout based on the targetMachine. */
+LLVMTargetDataRef LLVMCreateTargetDataLayout(LLVMTargetMachineRef T);
+
 /** Set the target machine's ASM verbosity. */
 void LLVMSetTargetMachineAsmVerbosity(LLVMTargetMachineRef T,
                                       LLVMBool VerboseAsm);
 
 /** Emits an asm or object file for the given module to the filename. This
   wraps several c++ only classes (among them a file stream). Returns any
   error in ErrorMessage. Use LLVMDisposeMessage to dispose the message. */
 LLVMBool LLVMTargetMachineEmitToFile(LLVMTargetMachineRef T, LLVMModuleRef M,

lib/Target/Target.cpp

void llvm::initializeTarget(PassRegistry &Registry) {
   initializeTargetLibraryInfoWrapperPassPass(Registry);
   initializeTargetTransformInfoWrapperPassPass(Registry);
 }
 
 void LLVMInitializeTarget(LLVMPassRegistryRef R) {
   initializeTarget(*unwrap(R));
 }
 
+LLVMTargetDataRef LLVMGetModuleDataLayout(LLVMModuleRef M) {
+  return wrap(&unwrap(M)->getDataLayout());
+}
+
+void LLVMSetModuleDataLayout(LLVMModuleRef M, LLVMTargetDataRef DL) {
+  unwrap(M)->setDataLayout(*unwrap(DL));
+}
+
 LLVMTargetDataRef LLVMCreateTargetData(const char *StringRep) {
   return wrap(new DataLayout(StringRep));
 }
 
 void LLVMAddTargetLibraryInfo(LLVMTargetLibraryInfoRef TLI,
                               LLVMPassManagerRef PM) {
   unwrap(PM)->add(new TargetLibraryInfoWrapperPass(*unwrap(TLI)));
 }

lib/Target/TargetMachineC.cpp

char* LLVMGetTargetMachineFeatureString(LLVMTargetMachineRef T) {
   return strdup(StringRep.c_str());
 }
 
 void LLVMSetTargetMachineAsmVerbosity(LLVMTargetMachineRef T,
                                       LLVMBool VerboseAsm) {
   unwrap(T)->Options.MCOptions.AsmVerbose = VerboseAsm;
 }
 
+LLVMTargetDataRef LLVMCreateTargetDataLayout(LLVMTargetMachineRef T) {
+  return wrap(new DataLayout(unwrap(T)->createDataLayout()));
+}
+
 static LLVMBool LLVMTargetMachineEmit(LLVMTargetMachineRef T, LLVMModuleRef M,
                                       raw_pwrite_stream &OS,
                                       LLVMCodeGenFileType codegen,
                                       char **ErrorMessage) {
   TargetMachine* TM = unwrap(T);
   Module* Mod = unwrap(M);
 
   legacy::PassManager pass;

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
 
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.11.0"
+
 %S = type { i64, %S* }
 
 define { i64, %S* } @unpackrepack(%S %s) {
   %1 = extractvalue %S %s, 0
   %2 = extractvalue %S %s, 1
   %3 = insertvalue { i64, %S* } undef, %S* %2, 1
   %4 = insertvalue { i64, %S* } %3, i64 %1, 0
   ret { i64, %S* } %4

tools/llvm-c-test/echo.cpp

 //
 // This command uses the C API to read a module and output an exact copy of it
 // as output. It is used to check that the resulting module matches the input
 // to validate that the C API can read and write modules properly.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm-c-test.h"
+#include "llvm-c/Target.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/Support/ErrorHandling.h"
 
 #include <stdio.h>
 #include <stdlib.h>
 
 using namespace llvm;
 

 int llvm_echo(void) {
   LLVMEnablePrettyStackTrace();
 
   LLVMModuleRef Src = llvm_load_module(false, true);
 
   LLVMContextRef Ctx = LLVMContextCreate();
   LLVMModuleRef M = LLVMModuleCreateWithNameInContext("<stdin>", Ctx);
 
+  LLVMSetTarget(M, LLVMGetTarget(Src));
+  LLVMSetModuleDataLayout(M, LLVMGetModuleDataLayout(Src));
+  if (strcmp(LLVMGetDataLayoutStr(M), LLVMGetDataLayoutStr(Src)))
+    report_fatal_error("Inconsistent DataLayout string representation");
+
   clone_functions(Src, M);
   char *Str = LLVMPrintModuleToString(M);
   fputs(Str, stdout);
 
   LLVMDisposeMessage(Str);
   LLVMDisposeModule(M);
   LLVMContextDispose(Ctx);
 
   return 0;
 }