[clang-offload-wrapper] Add data layout to the wrapper bitcode

[ABataev]

Why do we need this?

[sdmitriev]

In D70551#1755527, @ABataev wrote:

Why do we need this?

To pass wrapper bitcode directly to the linker when LTO is enabled. LTO plugin does not accept bc if there is no data layout.

[ABataev]

In D70551#1755553, @sdmitriev wrote:

In D70551#1755527, @ABataev wrote:

Why do we need this?

To pass wrapper bitcode directly to the linker when LTO is enabled. LTO plugin does not accept bc if there is no data layout.

Maybe it would better to pass it a parameter just like for opt?

[sdmitriev]

In D70551#1755583, @ABataev wrote:

In D70551#1755553, @sdmitriev wrote:

In D70551#1755527, @ABataev wrote:

Why do we need this?

To pass wrapper bitcode directly to the linker when LTO is enabled. LTO plugin does not accept bc if there is no data layout.

Maybe it would better to pass it a parameter just like for opt?

In this case clang driver would need to get it from somewhere in order to pass it to the wrapper tool. I do not know where it can get it from.

[ABataev]

In D70551#1755761, @sdmitriev wrote:

In D70551#1755583, @ABataev wrote:

In D70551#1755553, @sdmitriev wrote:

In D70551#1755527, @ABataev wrote:

Why do we need this?

To pass wrapper bitcode directly to the linker when LTO is enabled. LTO plugin does not accept bc if there is no data layout.

Maybe it would better to pass it a parameter just like for opt?

In this case clang driver would need to get it from somewhere in order to pass it to the wrapper tool. I do not know where it can get it from.

TargetInfo class is not accessible in the driver?

[sdmitriev]

In D70551#1755768, @ABataev wrote:

In D70551#1755761, @sdmitriev wrote:

In D70551#1755583, @ABataev wrote:

In D70551#1755553, @sdmitriev wrote:

In D70551#1755527, @ABataev wrote:

Why do we need this?

To pass wrapper bitcode directly to the linker when LTO is enabled. LTO plugin does not accept bc if there is no data layout.

Maybe it would better to pass it a parameter just like for opt?

In this case clang driver would need to get it from somewhere in order to pass it to the wrapper tool. I do not know where it can get it from.

TargetInfo class is not accessible in the driver?

Ah, I see, you mean clang's TargetInfo. Let me check if/how it will work.

[sdmitriev]

Ok, it is possible to do it like you suggested, but I think that teaching wrapper tool to set data layout without external help is more preferable. There is a certain difference between opt and wrapper tool – opt works on the existing .bc that is provided in command line and data-layout option just gives user an optional way to override input’s data layout while wrapper tool creates output .bc from scratch. With your proposal, data-layout would become sort of mandatory option for the wrapper tool which is not very convenient. I believe wrapper tool should be able to set it without external help, and we can always add an option to override data layout (similar to opt) if there would be a need for that.

[ABataev]

In D70551#1756997, @sdmitriev wrote:

Ok, it is possible to do it like you suggested, but I think that teaching wrapper tool to set data layout without external help is more preferable. There is a certain difference between opt and wrapper tool – opt works on the existing .bc that is provided in command line and data-layout option just gives user an optional way to override input’s data layout while wrapper tool creates output .bc from scratch. With your proposal, data-layout would become sort of mandatory option for the wrapper tool which is not very convenient. I believe wrapper tool should be able to set it without external help, and we can always add an option to override data layout (similar to opt) if there would be a need for that.

Why it is not convenient? Plus, when you're trying to build the data layout yourself, you end up with the default one for the given target rather than rely on the one specified by the user when the driver was invoked. So, I think, it would be better to get the data layout from the driver rather than use the default one.

[sdmitriev]

I would agree with you if data layout was indeed available in the driver, but unfortunately driver does not have access to any existing TargetInfo instance (or maybe I just have not found it). So, I have to create TargetInfo and build data layout even for the case when driver passes it to the wrapper tool. I guess that would also be a default data layout, so it would not differ much from what I have already done in this patch. BTW, I will probably upload an alternative patch where I have implemented your suggestion, just to compare))

[ABataev]

In D70551#1757065, @sdmitriev wrote:

I would agree with you if data layout was indeed available in the driver, but unfortunately driver does not have access to any existing TargetInfo instance (or maybe I just have not found it). So, I have to create TargetInfo and build data layout even for the case when driver passes it to the wrapper tool. I guess that would also be a default data layout, so it would not differ much from what I have already done in this patch. BTW, I will probably upload an alternative patch where I have implemented your suggestion, just to compare))

Of course, this is not a good solution. I was hoping that TargetInfo is available in driver. Otherwise, it would be good somehow to read the target data from the .bc file.

[sdmitriev]

Wrapper tool is invoked at link phase, therefore there just could be no .bc files available to read data layout from. Ok, looks like there is no good solution for the data layout problem, so I will drop the idea of passing wrapper .bc directly to the linker when LTO is enabled (at least for now:). I will abandon this patch.

clang/test/Driver/clang-offload-wrapper.c

 // RUN: echo 'Content of device file' > %t.tgt
 
 //
 // Check bitcode produced by the wrapper tool.
 //
 // RUN: clang-offload-wrapper -target=x86_64-pc-linux-gnu -o %t.wrapper.bc %t.tgt
 // RUN: llvm-dis %t.wrapper.bc -o - | FileCheck %s --check-prefix CHECK-IR
 
+// CHECK-IR: target datalayout =
 // CHECK-IR: target triple = "x86_64-pc-linux-gnu"
 
 // CHECK-IR-DAG: [[ENTTY:%.+]] = type { i8*, i8*, i{{32|64}}, i32, i32 }
 // CHECK-IR-DAG: [[IMAGETY:%.+]] = type { i8*, i8*, [[ENTTY]]*, [[ENTTY]]* }
 // CHECK-IR-DAG: [[DESCTY:%.+]] = type { i32, [[IMAGETY]]*, [[ENTTY]]*, [[ENTTY]]* }
 
 // CHECK-IR: [[ENTBEGIN:@.+]] = external hidden constant [[ENTTY]]
 // CHECK-IR: [[ENTEND:@.+]] = external hidden constant [[ENTTY]]

clang/tools/clang-offload-wrapper/CMakeLists.txt

-set(LLVM_LINK_COMPONENTS BitWriter Core Support TransformUtils)
+set(LLVM_LINK_COMPONENTS
+  AllTargetsAsmParsers
+  AllTargetsCodeGens
+  AllTargetsDescs
+  AllTargetsInfos
+  BitWriter
+  Core
+  MC
+  Support
+  Target
+  TransformUtils
+  )
 
 if(NOT CLANG_BUILT_STANDALONE)
   set(tablegen_deps intrinsics_gen)
 endif()
 
 add_clang_tool(clang-offload-wrapper
   ClangOffloadWrapper.cpp
 

clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp

 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Signals.h"
+#include "llvm/Support/TargetRegistry.h"
+#include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/ToolOutputFile.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Target/TargetMachine.h"
+#include "llvm/Target/TargetOptions.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 #include <cassert>
 #include <cstdint>
 
 using namespace llvm;
 
 static cl::opt<bool> Help("h", cl::desc("Alias for -help"), cl::Hidden);
 
 // Mark all our options with this category, everything else (except for -version
 // and -help) will be hidden.
 static cl::OptionCategory
     ClangOffloadWrapperCategory("clang-offload-wrapper options");
 
 static cl::opt<std::string> Output("o", cl::Required,
                                    cl::desc("Output filename"),
                                    cl::value_desc("filename"),
                                    cl::cat(ClangOffloadWrapperCategory));
 
 static cl::list<std::string> Inputs(cl::Positional, cl::OneOrMore,
                                     cl::desc("<input files>"),
                                     cl::cat(ClangOffloadWrapperCategory));
 
 static cl::opt<std::string>
-    Target("target", cl::Required,
+TargetTriple("target", cl::Required,
              cl::desc("Target triple for the output module"),
              cl::value_desc("triple"), cl::cat(ClangOffloadWrapperCategory));
 
 namespace {
 
 class BinaryWrapper {
   LLVMContext C;
   Module M;
 
   StructType *EntryTy = nullptr;
void createUnregisterFunction(GlobalVariable *BinDesc) {
     Builder.CreateCall(UnRegFuncC, BinDesc);
     Builder.CreateRetVoid();
 
     // Add this function to global destructors.
     appendToGlobalDtors(M, Func, 0);
   }
 
 public:
-  BinaryWrapper(StringRef Target) : M("offload.wrapper.object", C) {
-    M.setTargetTriple(Target);
+  BinaryWrapper(const TargetMachine *TM) : M("offload.wrapper.object", C) {
+    M.setTargetTriple(TM->getTargetTriple().getTriple());
+    M.setDataLayout(TM->createDataLayout());
   }
 
   const Module &wrapBinaries(ArrayRef<ArrayRef<char>> Binaries) {
     GlobalVariable *Desc = createBinDesc(Binaries);
     assert(Desc && "no binary descriptor");
     createRegisterFunction(Desc);
     createUnregisterFunction(Desc);
     return M;
cl::ParseCommandLineOptions(
       "target binaries packaged\nas data and initialization code which "
       "registers target binaries in offload runtime.\n");
 
   if (Help) {
     cl::PrintHelpMessage();
     return 0;
   }
 
+  InitializeAllTargets();
+  InitializeAllTargetMCs();
+
   auto reportError = [argv](Error E) {
     logAllUnhandledErrors(std::move(E), WithColor::error(errs(), argv[0]));
   };
 
-  if (Triple(Target).getArch() == Triple::UnknownArch) {
-    reportError(createStringError(
-        errc::invalid_argument, "'" + Target + "': unsupported target triple"));
+  if (Triple(TargetTriple).getArch() == Triple::UnknownArch) {
+    reportError(
+        createStringError(errc::invalid_argument,
+                          "'" + TargetTriple + "': unsupported target triple"));
     return 1;
   }
 
+  // Lookup and create target machine. We need it for setting data layout for
+  // the wrapper module.
+  std::string Error;
+  const Target *TheTarget = TargetRegistry::lookupTarget(TargetTriple, Error);
+  if (!TheTarget) {
+    reportError(createStringError(errc::invalid_argument, Error));
+    return 1;
+  }
+
+  std::unique_ptr<TargetMachine> TM(TheTarget->createTargetMachine(
+      TargetTriple, "", "", TargetOptions(), None));
+  assert(TM && "Could not allocate target machine!");
+
   // Read device binaries.
   SmallVector<std::unique_ptr<MemoryBuffer>, 4u> Buffers;
   SmallVector<ArrayRef<char>, 4u> Images;
   Buffers.reserve(Inputs.size());
   Images.reserve(Inputs.size());
   for (const std::string &File : Inputs) {
     ErrorOr<std::unique_ptr<MemoryBuffer>> BufOrErr =
         MemoryBuffer::getFileOrSTDIN(File);
int main(int argc, const char **argv) {
   std::error_code EC;
   ToolOutputFile Out(Output, EC, sys::fs::OF_None);
   if (EC) {
     reportError(createFileError(Output, EC));
     return 1;
   }
 
   // Create a wrapper for device binaries and write its bitcode to the file.
-  WriteBitcodeToFile(BinaryWrapper(Target).wrapBinaries(
+  WriteBitcodeToFile(BinaryWrapper(TM.get()).wrapBinaries(
                          makeArrayRef(Images.data(), Images.size())),
                      Out.os());
   if (Out.os().has_error()) {
     reportError(createFileError(Output, Out.os().error()));
     return 1;
   }
 
   // Success.
   Out.keep();
   return 0;
 }