[PATCH] D99551: [clang-offload-wrapper] Add standard notes for ELF offload images

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

It's a hack, but D108303  will unblock nvptx 
offloading. Alternative to reverting. Suggest we go with that then revisit in a 
couple of weeks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99551/new/

https://reviews.llvm.org/D99551

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99551: [clang-offload-wrapper] Add standard notes for ELF offload images

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added a comment.

In D99551#2952336 , @JonChesterfield 
wrote:

> Nvptx broken here too, amdgpu is fine. I'm guessing one of the cuda tools 
> does some overly aggressive input validation that we're running afoul of.
>
> There was a discussion about this on the call today - plan was to put it 
> behind a disabled boolean argument while fixing to avoid downstream churn. 
> Sadly the original patch was not authored with that in mind. I suggest if we 
> can't get that patch together asap we revert this and fix it offline (even if 
> the fix is adding said flag)

I am about to merge D108246  that is adding 
the switch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99551/new/

https://reviews.llvm.org/D99551

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99551: [clang-offload-wrapper] Add standard notes for ELF offload images

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

Nvptx broken here too, amdgpu is fine. I'm guessing one of the cuda tools does 
some overly aggressive input validation that we're running afoul of.

There was a discussion about this on the call today - plan was to put it behind 
a disabled boolean argument while fixing to avoid downstream churn. Sadly the 
original patch was not authored with that in mind. I suggest if we can't get 
that patch together asap we revert this and fix it offline (even if the fix is 
adding said flag)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99551/new/

https://reviews.llvm.org/D99551

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99551: [clang-offload-wrapper] Add standard notes for ELF offload images

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added a comment.

In D99551#2950522 , @jhuber6 wrote:

> In D99551#2950515 , @vzakhari wrote:
>
>> In D99551#2950464 , @jhuber6 wrote:
>>
>>> In D99551#2950281 , @vzakhari 
>>> wrote:
>>>
 @jhuber6, thank you for reporting this.  I do not have a properly setup 
 CUDA system currently.  Can you please invoke `clang-offload-wrapper` with 
 `-save-temps` and send the temporary files to me 
 ?
>>>
>>> I sent it to you, any luck or should we revert this upstream for the time 
>>> being.
>>
>> Unfortunately, I did not receive it.  Can you please check if the mail was 
>> blocked on your side?  Can you please try to archive it with a password and 
>> send it again?
>> I would like to prepare a patch to put this functionality under a switch 
>> that is off by default.  I think we want to have this working eventually 
>> (and it actually works with x86_64 offload), so it will be easier to just 
>> flip a switch, when I figure out what CUDA API does not like about the 
>> modified ELF image.  Does it sound appropriate to you?
>
> I put it in a .tar.gz file, I think your domain is blocking it. Anything else 
> I can use?

Can you please try sharing it via Google drive?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99551/new/

https://reviews.llvm.org/D99551

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99551: [clang-offload-wrapper] Add standard notes for ELF offload images

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D99551#2950515 , @vzakhari wrote:

> In D99551#2950464 , @jhuber6 wrote:
>
>> In D99551#2950281 , @vzakhari wrote:
>>
>>> @jhuber6, thank you for reporting this.  I do not have a properly setup 
>>> CUDA system currently.  Can you please invoke `clang-offload-wrapper` with 
>>> `-save-temps` and send the temporary files to me 
>>> ?
>>
>> I sent it to you, any luck or should we revert this upstream for the time 
>> being.
>
> Unfortunately, I did not receive it.  Can you please check if the mail was 
> blocked on your side?  Can you please try to archive it with a password and 
> send it again?
> I would like to prepare a patch to put this functionality under a switch that 
> is off by default.  I think we want to have this working eventually (and it 
> actually works with x86_64 offload), so it will be easier to just flip a 
> switch, when I figure out what CUDA API does not like about the modified ELF 
> image.  Does it sound appropriate to you?

I put it in a .tar.gz file, I think your domain is blocking it. Anything else I 
can use?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99551/new/

https://reviews.llvm.org/D99551

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99551: [clang-offload-wrapper] Add standard notes for ELF offload images

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added a comment.

In D99551#2950464 , @jhuber6 wrote:

> In D99551#2950281 , @vzakhari wrote:
>
>> @jhuber6, thank you for reporting this.  I do not have a properly setup CUDA 
>> system currently.  Can you please invoke `clang-offload-wrapper` with 
>> `-save-temps` and send the temporary files to me 
>> ?
>
> I sent it to you, any luck or should we revert this upstream for the time 
> being.

Unfortunately, I did not receive it.  Can you please check if the mail was 
blocked on your side?  Can you please try to archive it with a password and 
send it again?
I would like to prepare a patch to put this functionality under a switch that 
is off by default.  I think we want to have this working eventually (and it 
actually works with x86_64 offload), so it will be easier to just flip a 
switch, when I figure out what CUDA API does not like about the modified ELF 
image.  Does it sound appropriate to you?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99551/new/

https://reviews.llvm.org/D99551

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99551: [clang-offload-wrapper] Add standard notes for ELF offload images

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D99551#2950281 , @vzakhari wrote:

> @jhuber6, thank you for reporting this.  I do not have a properly setup CUDA 
> system currently.  Can you please invoke `clang-offload-wrapper` with 
> `-save-temps` and send the temporary files to me 
> ?

I sent it to you, any luck or should we revert this upstream for the time being.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99551/new/

https://reviews.llvm.org/D99551

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99551: [clang-offload-wrapper] Add standard notes for ELF offload images

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added a comment.

@jhuber6, thank you for reporting this.  I do not have a properly setup CUDA 
system currently.  Can you please invoke `clang-offload-wrapper` with 
`-save-temps` and send the temporary files to me 
?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99551/new/

https://reviews.llvm.org/D99551

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99551: [clang-offload-wrapper] Add standard notes for ELF offload images

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

This patch broke offloading on my machine. If I compile a basic offloading 
application I get an invalid device image.

  #include 
  
  int main() {
int isHost;
  #pragma omp target map(from:isHost)
{ isHost = omp_is_initial_device(); }
return isHost;
  }

Compiling and running with `clang -fopenmp -fopenmp-targets=nvptx64` gives me 
the following results.

  CUDA error: Error returned from cuModuleLoadDataEx
  CUDA error: device kernel image is invalid 
  Libomptarget error: Unable to generate entries table for device id 0.
  Libomptarget error: Failed to init globals on device 0
  Libomptarget error: Run with LIBOMPTARGET_INFO=4 to dump host-target pointer 
mappings.
  Libomptarget fatal error 1: failure of target construct while offloading is 
mandatory

Reverting `93d08acaacec951dbb302f77eeae51974985b6b2` fixes it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99551/new/

https://reviews.llvm.org/D99551

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99551: [clang-offload-wrapper] Add standard notes for ELF offload images

[Vyacheslav Zakharin via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG93d08acaacec: [clang-offload-wrapper] Add standard notes for 
ELF offload images (authored by vzakhari).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99551/new/

https://reviews.llvm.org/D99551

Files:
  clang/test/Driver/Inputs/empty-elf-template.yaml
  clang/test/Driver/clang-offload-wrapper.c
  clang/tools/clang-offload-wrapper/CMakeLists.txt
  clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp

Index: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
===
--- clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
+++ clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
@@ -17,27 +17,37 @@
 #include "clang/Basic/Version.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/BinaryFormat/ELF.h"
 #include "llvm/Bitcode/BitcodeWriter.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
+#include "llvm/Object/ELFObjectFile.h"
+#include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/EndianStream.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/ToolOutputFile.h"
+#include "llvm/Support/VCSRevision.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 #include 
 #include 
 
+#define OPENMP_OFFLOAD_IMAGE_VERSION "1.0"
+
 using namespace llvm;
+using namespace llvm::object;
 
 static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden);
 
@@ -60,6 +70,12 @@
cl::desc("Target triple for the output module"),
cl::value_desc("triple"), cl::cat(ClangOffloadWrapperCategory));
 
+static cl::opt SaveTemps(
+"save-temps",
+cl::desc("Save temporary files that may be produced by the tool. "
+ "This option forces print-out of the temporary files' names."),
+cl::Hidden);
+
 namespace {
 
 class BinaryWrapper {
@@ -70,6 +86,15 @@
   StructType *ImageTy = nullptr;
   StructType *DescTy = nullptr;
 
+  std::string ToolName;
+  std::string ObjcopyPath;
+  // Temporary file names that may be created during adding notes
+  // to ELF offload images. Use -save-temps to keep them and also
+  // see their names. A temporary file's name includes the name
+  // of the original input ELF image, so you can easily match
+  // them, if you have multiple inputs.
+  std::vector TempFiles;
+
 private:
   IntegerType *getSizeTTy() {
 switch (M.getDataLayout().getPointerTypeSize(Type::getInt8PtrTy(C))) {
@@ -294,8 +319,61 @@
   }
 
 public:
-  BinaryWrapper(StringRef Target) : M("offload.wrapper.object", C) {
+  BinaryWrapper(StringRef Target, StringRef ToolName)
+  : M("offload.wrapper.object", C), ToolName(ToolName) {
 M.setTargetTriple(Target);
+// Look for llvm-objcopy in the same directory, from which
+// clang-offload-wrapper is invoked. This helps OpenMP offload
+// LIT tests.
+
+// This just needs to be some symbol in the binary; C++ doesn't
+// allow taking the address of ::main however.
+void *P = (void *)(intptr_t)
+std::string COWPath = sys::fs::getMainExecutable(ToolName.str().c_str(), P);
+if (!COWPath.empty()) {
+  auto COWDir = sys::path::parent_path(COWPath);
+  ErrorOr ObjcopyPathOrErr =
+  sys::findProgramByName("llvm-objcopy", {COWDir});
+  if (ObjcopyPathOrErr) {
+ObjcopyPath = *ObjcopyPathOrErr;
+return;
+  }
+
+  // Otherwise, look through PATH environment.
+}
+
+ErrorOr ObjcopyPathOrErr =
+sys::findProgramByName("llvm-objcopy");
+if (!ObjcopyPathOrErr) {
+  WithColor::warning(errs(), ToolName)
+  << "cannot find llvm-objcopy[.exe] in PATH; ELF notes cannot be "
+ "added.\n";
+  return;
+}
+
+ObjcopyPath = *ObjcopyPathOrErr;
+  }
+
+  ~BinaryWrapper() {
+if (TempFiles.empty())
+  return;
+
+StringRef ToolNameRef(ToolName);
+auto warningOS = [ToolNameRef]() -> raw_ostream & {
+  return WithColor::warning(errs(), ToolNameRef);
+};
+
+for (auto  : TempFiles) {
+  if (SaveTemps) {
+warningOS() << "keeping temporary file " << F << "\n";
+continue;
+  }
+
+  auto EC = sys::fs::remove(F, false);
+  if (EC)
+warningOS() << "cannot remove temporary file " << F << ": "
+<< EC.message().c_str() << "\n";
+}
   }
 
   const Module (ArrayRef> Binaries) {
@@ -305,6 

[PATCH] D99551: [clang-offload-wrapper] Add standard notes for ELF offload images

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari updated this revision to Diff 364193.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99551/new/

https://reviews.llvm.org/D99551

Files:
  clang/test/Driver/Inputs/empty-elf-template.yaml
  clang/test/Driver/clang-offload-wrapper.c
  clang/tools/clang-offload-wrapper/CMakeLists.txt
  clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp

Index: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
===
--- clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
+++ clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
@@ -17,27 +17,37 @@
 #include "clang/Basic/Version.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/BinaryFormat/ELF.h"
 #include "llvm/Bitcode/BitcodeWriter.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
+#include "llvm/Object/ELFObjectFile.h"
+#include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/EndianStream.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/ToolOutputFile.h"
+#include "llvm/Support/VCSRevision.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 #include 
 #include 
 
+#define OPENMP_OFFLOAD_IMAGE_VERSION "1.0"
+
 using namespace llvm;
+using namespace llvm::object;
 
 static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden);
 
@@ -60,6 +70,12 @@
cl::desc("Target triple for the output module"),
cl::value_desc("triple"), cl::cat(ClangOffloadWrapperCategory));
 
+static cl::opt SaveTemps(
+"save-temps",
+cl::desc("Save temporary files that may be produced by the tool. "
+ "This option forces print-out of the temporary files' names."),
+cl::Hidden);
+
 namespace {
 
 class BinaryWrapper {
@@ -70,6 +86,15 @@
   StructType *ImageTy = nullptr;
   StructType *DescTy = nullptr;
 
+  std::string ToolName;
+  std::string ObjcopyPath;
+  // Temporary file names that may be created during adding notes
+  // to ELF offload images. Use -save-temps to keep them and also
+  // see their names. A temporary file's name includes the name
+  // of the original input ELF image, so you can easily match
+  // them, if you have multiple inputs.
+  std::vector TempFiles;
+
 private:
   IntegerType *getSizeTTy() {
 switch (M.getDataLayout().getPointerTypeSize(Type::getInt8PtrTy(C))) {
@@ -294,8 +319,61 @@
   }
 
 public:
-  BinaryWrapper(StringRef Target) : M("offload.wrapper.object", C) {
+  BinaryWrapper(StringRef Target, StringRef ToolName)
+  : M("offload.wrapper.object", C), ToolName(ToolName) {
 M.setTargetTriple(Target);
+// Look for llvm-objcopy in the same directory, from which
+// clang-offload-wrapper is invoked. This helps OpenMP offload
+// LIT tests.
+
+// This just needs to be some symbol in the binary; C++ doesn't
+// allow taking the address of ::main however.
+void *P = (void *)(intptr_t)
+std::string COWPath = sys::fs::getMainExecutable(ToolName.str().c_str(), P);
+if (!COWPath.empty()) {
+  auto COWDir = sys::path::parent_path(COWPath);
+  ErrorOr ObjcopyPathOrErr =
+  sys::findProgramByName("llvm-objcopy", {COWDir});
+  if (ObjcopyPathOrErr) {
+ObjcopyPath = *ObjcopyPathOrErr;
+return;
+  }
+
+  // Otherwise, look through PATH environment.
+}
+
+ErrorOr ObjcopyPathOrErr =
+sys::findProgramByName("llvm-objcopy");
+if (!ObjcopyPathOrErr) {
+  WithColor::warning(errs(), ToolName)
+  << "cannot find llvm-objcopy[.exe] in PATH; ELF notes cannot be "
+ "added.\n";
+  return;
+}
+
+ObjcopyPath = *ObjcopyPathOrErr;
+  }
+
+  ~BinaryWrapper() {
+if (TempFiles.empty())
+  return;
+
+StringRef ToolNameRef(ToolName);
+auto warningOS = [ToolNameRef]() -> raw_ostream & {
+  return WithColor::warning(errs(), ToolNameRef);
+};
+
+for (auto  : TempFiles) {
+  if (SaveTemps) {
+warningOS() << "keeping temporary file " << F << "\n";
+continue;
+  }
+
+  auto EC = sys::fs::remove(F, false);
+  if (EC)
+warningOS() << "cannot remove temporary file " << F << ": "
+<< EC.message().c_str() << "\n";
+}
   }
 
   const Module (ArrayRef> Binaries) {
@@ -305,6 +383,205 @@
 createUnregisterFunction(Desc);
 return M;
   }
+
+  std::unique_ptr addELFNotes(std::unique_ptr Buf,
+StringRef OriginalFileName) {
+// Cannot add notes, if llvm-objcopy is not 

[PATCH] D99551: [clang-offload-wrapper] Add standard notes for ELF offload images

[George Rokos via Phabricator via cfe-commits]

grokos accepted this revision.
grokos added a comment.
This revision is now accepted and ready to land.

Change looks good, so it's accepted on my end. I'll let the other reviewers 
have a look and post their comments. Please do not commit until we have reached 
an agreement for all 4 patches together (D99551 
, D99552 , 
D99553 , D99612 
).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99551/new/

https://reviews.llvm.org/D99551

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99551: [clang-offload-wrapper] Add standard notes for ELF offload images

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari updated this revision to Diff 334292.
vzakhari added a comment.
Herald added a subscriber: mgorny.

Updated revision fixes //BUILD_SHARED_LIBS// build.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99551/new/

https://reviews.llvm.org/D99551

Files:
  clang/test/Driver/Inputs/empty-elf-template.yaml
  clang/test/Driver/clang-offload-wrapper.c
  clang/tools/clang-offload-wrapper/CMakeLists.txt
  clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp

Index: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
===
--- clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
+++ clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
@@ -17,26 +17,35 @@
 #include "clang/Basic/Version.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/BinaryFormat/ELF.h"
 #include "llvm/Bitcode/BitcodeWriter.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
+#include "llvm/Object/ELFObjectFile.h"
+#include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/EndianStream.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/ToolOutputFile.h"
+#include "llvm/Support/VCSRevision.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 #include 
 #include 
 
+#define OPENMP_OFFLOAD_IMAGE_VERSION "1.0"
+
 using namespace llvm;
+using namespace llvm::object;
 
 static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden);
 
@@ -59,6 +68,12 @@
cl::desc("Target triple for the output module"),
cl::value_desc("triple"), cl::cat(ClangOffloadWrapperCategory));
 
+static cl::opt SaveTemps(
+"save-temps",
+cl::desc("Save temporary files that may be produced by the tool. "
+ "This option forces print-out of the temporary files' names."),
+cl::Hidden);
+
 namespace {
 
 class BinaryWrapper {
@@ -69,6 +84,15 @@
   StructType *ImageTy = nullptr;
   StructType *DescTy = nullptr;
 
+  std::string ToolName;
+  std::string ObjcopyPath;
+  // Temporary file names that may be created during adding notes
+  // to ELF offload images. Use -save-temps to keep them and also
+  // see their names. A temporary file's name includes the name
+  // of the original input ELF image, so you can easily match
+  // them, if you have multiple inputs.
+  std::vector TempFiles;
+
 private:
   IntegerType *getSizeTTy() {
 switch (M.getDataLayout().getPointerTypeSize(Type::getInt8PtrTy(C))) {
@@ -293,8 +317,61 @@
   }
 
 public:
-  BinaryWrapper(StringRef Target) : M("offload.wrapper.object", C) {
+  BinaryWrapper(StringRef Target, StringRef ToolName)
+  : M("offload.wrapper.object", C), ToolName(ToolName) {
 M.setTargetTriple(Target);
+// Look for llvm-objcopy in the same directory, from which
+// clang-offload-wrapper is invoked. This helps OpenMP offload
+// LIT tests.
+
+// This just needs to be some symbol in the binary; C++ doesn't
+// allow taking the address of ::main however.
+void *P = (void *)(intptr_t)
+std::string COWPath = sys::fs::getMainExecutable(ToolName.str().c_str(), P);
+if (!COWPath.empty()) {
+  auto COWDir = sys::path::parent_path(COWPath);
+  ErrorOr ObjcopyPathOrErr =
+  sys::findProgramByName("llvm-objcopy", {COWDir});
+  if (ObjcopyPathOrErr) {
+ObjcopyPath = *ObjcopyPathOrErr;
+return;
+  }
+
+  // Otherwise, look through PATH environment.
+}
+
+ErrorOr ObjcopyPathOrErr =
+sys::findProgramByName("llvm-objcopy");
+if (!ObjcopyPathOrErr) {
+  WithColor::warning(errs(), ToolName)
+  << "cannot find llvm-objcopy[.exe] in PATH; ELF notes cannot be "
+ "added.\n";
+  return;
+}
+
+ObjcopyPath = *ObjcopyPathOrErr;
+  }
+
+  ~BinaryWrapper() {
+if (TempFiles.empty())
+  return;
+
+StringRef ToolNameRef(ToolName);
+auto warningOS = [ToolNameRef]() -> raw_ostream & {
+  return WithColor::warning(errs(), ToolNameRef);
+};
+
+for (auto  : TempFiles) {
+  if (SaveTemps) {
+warningOS() << "keeping temporary file " << F << "\n";
+continue;
+  }
+
+  auto EC = sys::fs::remove(F, false);
+  if (EC)
+warningOS() << "cannot remove temporary file " << F << ": "
+<< EC.message().c_str() << "\n";
+}
   }
 
   const Module (ArrayRef> Binaries) {
@@ -304,6 +381,205 @@
 createUnregisterFunction(Desc);
 return M;
   }
+
+  std::unique_ptr addELFNotes(std::unique_ptr Buf,
+StringRef OriginalFileName) {
+// 

[PATCH] D99551: [clang-offload-wrapper] Add standard notes for ELF offload images

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari updated this revision to Diff 334282.
vzakhari added a comment.

I am not sure what is wrong with the uploaded ELF files.  The test works 
locally.  Let's try it with a thinner YAML template.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99551/new/

https://reviews.llvm.org/D99551

Files:
  clang/test/Driver/Inputs/empty-elf-template.yaml
  clang/test/Driver/clang-offload-wrapper.c
  clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp

Index: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
===
--- clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
+++ clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
@@ -17,26 +17,35 @@
 #include "clang/Basic/Version.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/BinaryFormat/ELF.h"
 #include "llvm/Bitcode/BitcodeWriter.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
+#include "llvm/Object/ELFObjectFile.h"
+#include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/EndianStream.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/ToolOutputFile.h"
+#include "llvm/Support/VCSRevision.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 #include 
 #include 
 
+#define OPENMP_OFFLOAD_IMAGE_VERSION "1.0"
+
 using namespace llvm;
+using namespace llvm::object;
 
 static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden);
 
@@ -59,6 +68,12 @@
cl::desc("Target triple for the output module"),
cl::value_desc("triple"), cl::cat(ClangOffloadWrapperCategory));
 
+static cl::opt SaveTemps(
+"save-temps",
+cl::desc("Save temporary files that may be produced by the tool. "
+ "This option forces print-out of the temporary files' names."),
+cl::Hidden);
+
 namespace {
 
 class BinaryWrapper {
@@ -69,6 +84,15 @@
   StructType *ImageTy = nullptr;
   StructType *DescTy = nullptr;
 
+  std::string ToolName;
+  std::string ObjcopyPath;
+  // Temporary file names that may be created during adding notes
+  // to ELF offload images. Use -save-temps to keep them and also
+  // see their names. A temporary file's name includes the name
+  // of the original input ELF image, so you can easily match
+  // them, if you have multiple inputs.
+  std::vector TempFiles;
+
 private:
   IntegerType *getSizeTTy() {
 switch (M.getDataLayout().getPointerTypeSize(Type::getInt8PtrTy(C))) {
@@ -293,8 +317,61 @@
   }
 
 public:
-  BinaryWrapper(StringRef Target) : M("offload.wrapper.object", C) {
+  BinaryWrapper(StringRef Target, StringRef ToolName)
+  : M("offload.wrapper.object", C), ToolName(ToolName) {
 M.setTargetTriple(Target);
+// Look for llvm-objcopy in the same directory, from which
+// clang-offload-wrapper is invoked. This helps OpenMP offload
+// LIT tests.
+
+// This just needs to be some symbol in the binary; C++ doesn't
+// allow taking the address of ::main however.
+void *P = (void *)(intptr_t)
+std::string COWPath = sys::fs::getMainExecutable(ToolName.str().c_str(), P);
+if (!COWPath.empty()) {
+  auto COWDir = sys::path::parent_path(COWPath);
+  ErrorOr ObjcopyPathOrErr =
+  sys::findProgramByName("llvm-objcopy", {COWDir});
+  if (ObjcopyPathOrErr) {
+ObjcopyPath = *ObjcopyPathOrErr;
+return;
+  }
+
+  // Otherwise, look through PATH environment.
+}
+
+ErrorOr ObjcopyPathOrErr =
+sys::findProgramByName("llvm-objcopy");
+if (!ObjcopyPathOrErr) {
+  WithColor::warning(errs(), ToolName)
+  << "cannot find llvm-objcopy[.exe] in PATH; ELF notes cannot be "
+ "added.\n";
+  return;
+}
+
+ObjcopyPath = *ObjcopyPathOrErr;
+  }
+
+  ~BinaryWrapper() {
+if (TempFiles.empty())
+  return;
+
+StringRef ToolNameRef(ToolName);
+auto warningOS = [ToolNameRef]() -> raw_ostream & {
+  return WithColor::warning(errs(), ToolNameRef);
+};
+
+for (auto  : TempFiles) {
+  if (SaveTemps) {
+warningOS() << "keeping temporary file " << F << "\n";
+continue;
+  }
+
+  auto EC = sys::fs::remove(F, false);
+  if (EC)
+warningOS() << "cannot remove temporary file " << F << ": "
+<< EC.message().c_str() << "\n";
+}
   }
 
   const Module (ArrayRef> Binaries) {
@@ -304,6 +381,205 @@
 createUnregisterFunction(Desc);
 return M;
   }
+
+  std::unique_ptr addELFNotes(std::unique_ptr Buf,
+StringRef OriginalFileName) {
+// Cannot add 

[PATCH] D99551: [clang-offload-wrapper] Add standard notes for ELF offload images

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari updated this revision to Diff 334233.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99551/new/

https://reviews.llvm.org/D99551

Files:
  clang/test/Driver/Inputs/clang-offload-wrapper/elf32be
  clang/test/Driver/Inputs/clang-offload-wrapper/elf32le
  clang/test/Driver/Inputs/clang-offload-wrapper/elf64be
  clang/test/Driver/Inputs/clang-offload-wrapper/elf64le
  clang/test/Driver/clang-offload-wrapper.c
  clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp

Index: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
===
--- clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
+++ clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
@@ -17,26 +17,35 @@
 #include "clang/Basic/Version.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/BinaryFormat/ELF.h"
 #include "llvm/Bitcode/BitcodeWriter.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
+#include "llvm/Object/ELFObjectFile.h"
+#include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/EndianStream.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/ToolOutputFile.h"
+#include "llvm/Support/VCSRevision.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 #include 
 #include 
 
+#define OPENMP_OFFLOAD_IMAGE_VERSION "1.0"
+
 using namespace llvm;
+using namespace llvm::object;
 
 static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden);
 
@@ -59,6 +68,12 @@
cl::desc("Target triple for the output module"),
cl::value_desc("triple"), cl::cat(ClangOffloadWrapperCategory));
 
+static cl::opt SaveTemps(
+"save-temps",
+cl::desc("Save temporary files that may be produced by the tool. "
+ "This option forces print-out of the temporary files' names."),
+cl::Hidden);
+
 namespace {
 
 class BinaryWrapper {
@@ -69,6 +84,15 @@
   StructType *ImageTy = nullptr;
   StructType *DescTy = nullptr;
 
+  std::string ToolName;
+  std::string ObjcopyPath;
+  // Temporary file names that may be created during adding notes
+  // to ELF offload images. Use -save-temps to keep them and also
+  // see their names. A temporary file's name includes the name
+  // of the original input ELF image, so you can easily match
+  // them, if you have multiple inputs.
+  std::vector TempFiles;
+
 private:
   IntegerType *getSizeTTy() {
 switch (M.getDataLayout().getPointerTypeSize(Type::getInt8PtrTy(C))) {
@@ -293,8 +317,61 @@
   }
 
 public:
-  BinaryWrapper(StringRef Target) : M("offload.wrapper.object", C) {
+  BinaryWrapper(StringRef Target, StringRef ToolName)
+  : M("offload.wrapper.object", C), ToolName(ToolName) {
 M.setTargetTriple(Target);
+// Look for llvm-objcopy in the same directory, from which
+// clang-offload-wrapper is invoked. This helps OpenMP offload
+// LIT tests.
+
+// This just needs to be some symbol in the binary; C++ doesn't
+// allow taking the address of ::main however.
+void *P = (void *)(intptr_t)
+std::string COWPath = sys::fs::getMainExecutable(ToolName.str().c_str(), P);
+if (!COWPath.empty()) {
+  auto COWDir = sys::path::parent_path(COWPath);
+  ErrorOr ObjcopyPathOrErr =
+  sys::findProgramByName("llvm-objcopy", {COWDir});
+  if (ObjcopyPathOrErr) {
+ObjcopyPath = *ObjcopyPathOrErr;
+return;
+  }
+
+  // Otherwise, look through PATH environment.
+}
+
+ErrorOr ObjcopyPathOrErr =
+sys::findProgramByName("llvm-objcopy");
+if (!ObjcopyPathOrErr) {
+  WithColor::warning(errs(), ToolName)
+  << "cannot find llvm-objcopy[.exe] in PATH; ELF notes cannot be "
+ "added.\n";
+  return;
+}
+
+ObjcopyPath = *ObjcopyPathOrErr;
+  }
+
+  ~BinaryWrapper() {
+if (TempFiles.empty())
+  return;
+
+StringRef ToolNameRef(ToolName);
+auto warningOS = [ToolNameRef]() -> raw_ostream & {
+  return WithColor::warning(errs(), ToolNameRef);
+};
+
+for (auto  : TempFiles) {
+  if (SaveTemps) {
+warningOS() << "keeping temporary file " << F << "\n";
+continue;
+  }
+
+  auto EC = sys::fs::remove(F, false);
+  if (EC)
+warningOS() << "cannot remove temporary file " << F << ": "
+<< EC.message().c_str() << "\n";
+}
   }
 
   const Module (ArrayRef> Binaries) {
@@ -304,6 +381,205 @@
 createUnregisterFunction(Desc);
 return M;
   }
+
+  std::unique_ptr addELFNotes(std::unique_ptr Buf,
+StringRef 

[PATCH] D99551: [clang-offload-wrapper] Add standard notes for ELF offload images

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari created this revision.
vzakhari added reviewers: ABataev, sdmitriev, grokos.
vzakhari added a project: OpenMP.
Herald added a reviewer: alexshap.
vzakhari requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: llvm-commits, cfe-commits, sstefan1.
Herald added projects: clang, LLVM.

This patch is a piece of D99360 .

The patch adds ELF notes into SHT_NOTE sections of ELF offload images passed to 
clang-offload-wrapper.

The new notes use a null-terminated //"LLVMOMPOFFLOAD"// note name. There are 
currently three types of notes:

//VERSION//: a string (not null-terminated) representing the ELF offload image 
structure. The current version '1.0' does not put any restrictions on the 
structure of the image. If we ever need to come up with a common structure for 
ELF offload images (e.g. to be able to analyze the images in libomptarget in 
some standard way), then we will introduce new versions.
//PRODUCER//: a vendor specific name of the producing toolchain. Upstream LLVM 
uses //"LLVM"// (not null-terminated).
//PRODUCER_VERSION//: a vendor specific version of the producing toolchain. 
Upstream LLVM uses //LLVM_VERSION_STRING// with optional // 
LLVM_REVISION//.

All three notes are not mandatory currently.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99551

Files:
  clang/test/Driver/Inputs/clang-offload-wrapper/elf32be
  clang/test/Driver/Inputs/clang-offload-wrapper/elf32le
  clang/test/Driver/Inputs/clang-offload-wrapper/elf64be
  clang/test/Driver/Inputs/clang-offload-wrapper/elf64le
  clang/test/Driver/clang-offload-wrapper.c
  clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
  llvm/include/llvm/BinaryFormat/ELF.h

Index: llvm/include/llvm/BinaryFormat/ELF.h
===
--- llvm/include/llvm/BinaryFormat/ELF.h
+++ llvm/include/llvm/BinaryFormat/ELF.h
@@ -1606,6 +1606,13 @@
   NT_AMDGPU_METADATA = 32
 };
 
+// LLVMOMPOFFLOAD specific notes.
+enum : unsigned {
+  NT_LLVM_OPENMP_OFFLOAD_VERSION = 1,
+  NT_LLVM_OPENMP_OFFLOAD_PRODUCER = 2,
+  NT_LLVM_OPENMP_OFFLOAD_PRODUCER_VERSION = 3
+};
+
 enum {
   GNU_ABI_TAG_LINUX = 0,
   GNU_ABI_TAG_HURD = 1,
Index: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
===
--- clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
+++ clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
@@ -17,26 +17,35 @@
 #include "clang/Basic/Version.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/BinaryFormat/ELF.h"
 #include "llvm/Bitcode/BitcodeWriter.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
+#include "llvm/Object/ELFObjectFile.h"
+#include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/EndianStream.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/ToolOutputFile.h"
+#include "llvm/Support/VCSRevision.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 #include 
 #include 
 
+#define OPENMP_OFFLOAD_IMAGE_VERSION "1.0"
+
 using namespace llvm;
+using namespace llvm::object;
 
 static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden);
 
@@ -59,6 +68,12 @@
cl::desc("Target triple for the output module"),
cl::value_desc("triple"), cl::cat(ClangOffloadWrapperCategory));
 
+static cl::opt SaveTemps(
+"save-temps",
+cl::desc("Save temporary files that may be produced by the tool. "
+ "This option forces print-out of the temporary files' names."),
+cl::Hidden);
+
 namespace {
 
 class BinaryWrapper {
@@ -69,6 +84,15 @@
   StructType *ImageTy = nullptr;
   StructType *DescTy = nullptr;
 
+  std::string ToolName;
+  std::string ObjcopyPath;
+  // Temporary file names that may be created during adding notes
+  // to ELF offload images. Use -save-temps to keep them and also
+  // see their names. A temporary file's name includes the name
+  // of the original input ELF image, so you can easily match
+  // them, if you have multiple inputs.
+  std::vector TempFiles;
+
 private:
   IntegerType *getSizeTTy() {
 switch (M.getDataLayout().getPointerTypeSize(Type::getInt8PtrTy(C))) {
@@ -293,8 +317,62 @@
   }
 
 public:
-  BinaryWrapper(StringRef Target) : M("offload.wrapper.object", C) {
+  BinaryWrapper(StringRef Target, StringRef ToolName)
+  : M("offload.wrapper.object", C), ToolName(ToolName) {
 M.setTargetTriple(Target);
+// Look for llvm-objcopy in the same directory, from which
+// clang-offload-wrapper is