[PATCH] D108246: [clang-offload-wrapper] Disabled ELF offload notes embedding by default.

2021-08-18 Thread Vyacheslav Zakharin via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1ffbe8c04ff2: [clang-offload-wrapper] Disabled ELF offload 
notes embedding by default. (authored by vzakhari).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108246

Files:
  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
@@ -76,6 +76,10 @@
  "This option forces print-out of the temporary files' names."),
 cl::Hidden);
 
+static cl::opt AddOpenMPOffloadNotes(
+"add-omp-offload-notes",
+cl::desc("Add LLVMOMPOFFLOAD ELF notes to ELF device images."), 
cl::Hidden);
+
 namespace {
 
 class BinaryWrapper {
@@ -630,7 +634,7 @@
   return 1;
 }
 std::unique_ptr Buffer(std::move(*BufOrErr));
-if (File != "-") {
+if (File != "-" && AddOpenMPOffloadNotes) {
   // Adding ELF notes for STDIN is not supported yet.
   Buffer = Wrapper.addELFNotes(std::move(Buffer), File);
 }
Index: clang/test/Driver/clang-offload-wrapper.c
===
--- clang/test/Driver/clang-offload-wrapper.c
+++ clang/test/Driver/clang-offload-wrapper.c
@@ -19,7 +19,7 @@
 //
 // Check bitcode produced by the wrapper tool.
 //
-// RUN: clang-offload-wrapper -target=x86_64-pc-linux-gnu -o %t.wrapper.bc 
%t.tgt 2>&1 | FileCheck %s --check-prefix ELF-WARNING
+// RUN: clang-offload-wrapper -add-omp-offload-notes 
-target=x86_64-pc-linux-gnu -o %t.wrapper.bc %t.tgt 2>&1 | FileCheck %s 
--check-prefix ELF-WARNING
 // RUN: llvm-dis %t.wrapper.bc -o - | FileCheck %s --check-prefix CHECK-IR
 
 // ELF-WARNING: is not an ELF image, so notes cannot be added to it.
@@ -58,16 +58,16 @@
 // Check that clang-offload-wrapper adds LLVMOMPOFFLOAD notes
 // into the ELF offload images:
 // RUN: yaml2obj %S/Inputs/empty-elf-template.yaml -o %t.64le -DBITS=64 
-DENCODING=LSB
-// RUN: clang-offload-wrapper -target=x86_64-pc-linux-gnu -o 
%t.wrapper.elf64le.bc %t.64le
+// RUN: clang-offload-wrapper -add-omp-offload-notes 
-target=x86_64-pc-linux-gnu -o %t.wrapper.elf64le.bc %t.64le
 // RUN: llvm-dis %t.wrapper.elf64le.bc -o - | FileCheck %s --check-prefix 
OMPNOTES
 // RUN: yaml2obj %S/Inputs/empty-elf-template.yaml -o %t.64be -DBITS=64 
-DENCODING=MSB
-// RUN: clang-offload-wrapper -target=x86_64-pc-linux-gnu -o 
%t.wrapper.elf64be.bc %t.64be
+// RUN: clang-offload-wrapper -add-omp-offload-notes 
-target=x86_64-pc-linux-gnu -o %t.wrapper.elf64be.bc %t.64be
 // RUN: llvm-dis %t.wrapper.elf64be.bc -o - | FileCheck %s --check-prefix 
OMPNOTES
 // RUN: yaml2obj %S/Inputs/empty-elf-template.yaml -o %t.32le -DBITS=32 
-DENCODING=LSB
-// RUN: clang-offload-wrapper -target=x86_64-pc-linux-gnu -o 
%t.wrapper.elf32le.bc %t.32le
+// RUN: clang-offload-wrapper -add-omp-offload-notes 
-target=x86_64-pc-linux-gnu -o %t.wrapper.elf32le.bc %t.32le
 // RUN: llvm-dis %t.wrapper.elf32le.bc -o - | FileCheck %s --check-prefix 
OMPNOTES
 // RUN: yaml2obj %S/Inputs/empty-elf-template.yaml -o %t.32be -DBITS=32 
-DENCODING=MSB
-// RUN: clang-offload-wrapper -target=x86_64-pc-linux-gnu -o 
%t.wrapper.elf32be.bc %t.32be
+// RUN: clang-offload-wrapper -add-omp-offload-notes 
-target=x86_64-pc-linux-gnu -o %t.wrapper.elf32be.bc %t.32be
 // RUN: llvm-dis %t.wrapper.elf32be.bc -o - | FileCheck %s --check-prefix 
OMPNOTES
 
 // There is no clean way for extracting the offload image


Index: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
===
--- clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
+++ clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
@@ -76,6 +76,10 @@
  "This option forces print-out of the temporary files' names."),
 cl::Hidden);
 
+static cl::opt AddOpenMPOffloadNotes(
+"add-omp-offload-notes",
+cl::desc("Add LLVMOMPOFFLOAD ELF notes to ELF device images."), cl::Hidden);
+
 namespace {
 
 class BinaryWrapper {
@@ -630,7 +634,7 @@
   return 1;
 }
 std::unique_ptr Buffer(std::move(*BufOrErr));
-if (File != "-") {
+if (File != "-" && AddOpenMPOffloadNotes) {
   // Adding ELF notes for STDIN is not supported yet.
   Buffer = Wrapper.addELFNotes(std::move(Buffer), File);
 }
Index: clang/test/Driver/clang-offload-wrapper.c
===
--- clang/test/Driver/clang-offload-wrapper.c
+++ clang/test/Driver/clang-offload-wrapper.c
@@ -19,7 +19,7 @@
 //
 // Check bitcode produced by the wrapper tool.
 //
-// RUN: clang-offload-wrapper -target=x86_64-pc-linux-gnu -o %t.wrapper.bc %t.tgt 2>&1 | FileCheck %s --c

[PATCH] D108246: [clang-offload-wrapper] Disabled ELF offload notes embedding by default.

2021-08-18 Thread Jon Chesterfield via Phabricator via cfe-commits
JonChesterfield added inline comments.



Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:79
 
+static cl::opt AddOpenMPOffloadNotes(
+"add-omp-offload-notes",

I'd have probably gone with an explicit false here but it doesn't make much 
difference.


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

https://reviews.llvm.org/D108246

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


[PATCH] D108246: [clang-offload-wrapper] Disabled ELF offload notes embedding by default.

2021-08-18 Thread Jon Chesterfield via Phabricator via cfe-commits
JonChesterfield accepted this revision.
JonChesterfield added a comment.

Amusingly similar to D108303 . LGTM.


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

https://reviews.llvm.org/D108246

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


[PATCH] D108246: [clang-offload-wrapper] Disabled ELF offload notes embedding by default.

2021-08-18 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

As discussed, LG, we will look into the NVIDIA GPU problem now to get rid of 
this again.


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

https://reviews.llvm.org/D108246

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


[PATCH] D108246: [clang-offload-wrapper] Disabled ELF offload notes embedding by default.

2021-08-17 Thread Vyacheslav Zakharin via Phabricator via cfe-commits
vzakhari updated this revision to Diff 367057.

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

https://reviews.llvm.org/D108246

Files:
  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
@@ -76,6 +76,10 @@
  "This option forces print-out of the temporary files' names."),
 cl::Hidden);
 
+static cl::opt AddOpenMPOffloadNotes(
+"add-omp-offload-notes",
+cl::desc("Add LLVMOMPOFFLOAD ELF notes to ELF device images."), 
cl::Hidden);
+
 namespace {
 
 class BinaryWrapper {
@@ -630,7 +634,7 @@
   return 1;
 }
 std::unique_ptr Buffer(std::move(*BufOrErr));
-if (File != "-") {
+if (File != "-" && AddOpenMPOffloadNotes) {
   // Adding ELF notes for STDIN is not supported yet.
   Buffer = Wrapper.addELFNotes(std::move(Buffer), File);
 }
Index: clang/test/Driver/clang-offload-wrapper.c
===
--- clang/test/Driver/clang-offload-wrapper.c
+++ clang/test/Driver/clang-offload-wrapper.c
@@ -19,7 +19,7 @@
 //
 // Check bitcode produced by the wrapper tool.
 //
-// RUN: clang-offload-wrapper -target=x86_64-pc-linux-gnu -o %t.wrapper.bc 
%t.tgt 2>&1 | FileCheck %s --check-prefix ELF-WARNING
+// RUN: clang-offload-wrapper -add-omp-offload-notes 
-target=x86_64-pc-linux-gnu -o %t.wrapper.bc %t.tgt 2>&1 | FileCheck %s 
--check-prefix ELF-WARNING
 // RUN: llvm-dis %t.wrapper.bc -o - | FileCheck %s --check-prefix CHECK-IR
 
 // ELF-WARNING: is not an ELF image, so notes cannot be added to it.
@@ -58,16 +58,16 @@
 // Check that clang-offload-wrapper adds LLVMOMPOFFLOAD notes
 // into the ELF offload images:
 // RUN: yaml2obj %S/Inputs/empty-elf-template.yaml -o %t.64le -DBITS=64 
-DENCODING=LSB
-// RUN: clang-offload-wrapper -target=x86_64-pc-linux-gnu -o 
%t.wrapper.elf64le.bc %t.64le
+// RUN: clang-offload-wrapper -add-omp-offload-notes 
-target=x86_64-pc-linux-gnu -o %t.wrapper.elf64le.bc %t.64le
 // RUN: llvm-dis %t.wrapper.elf64le.bc -o - | FileCheck %s --check-prefix 
OMPNOTES
 // RUN: yaml2obj %S/Inputs/empty-elf-template.yaml -o %t.64be -DBITS=64 
-DENCODING=MSB
-// RUN: clang-offload-wrapper -target=x86_64-pc-linux-gnu -o 
%t.wrapper.elf64be.bc %t.64be
+// RUN: clang-offload-wrapper -add-omp-offload-notes 
-target=x86_64-pc-linux-gnu -o %t.wrapper.elf64be.bc %t.64be
 // RUN: llvm-dis %t.wrapper.elf64be.bc -o - | FileCheck %s --check-prefix 
OMPNOTES
 // RUN: yaml2obj %S/Inputs/empty-elf-template.yaml -o %t.32le -DBITS=32 
-DENCODING=LSB
-// RUN: clang-offload-wrapper -target=x86_64-pc-linux-gnu -o 
%t.wrapper.elf32le.bc %t.32le
+// RUN: clang-offload-wrapper -add-omp-offload-notes 
-target=x86_64-pc-linux-gnu -o %t.wrapper.elf32le.bc %t.32le
 // RUN: llvm-dis %t.wrapper.elf32le.bc -o - | FileCheck %s --check-prefix 
OMPNOTES
 // RUN: yaml2obj %S/Inputs/empty-elf-template.yaml -o %t.32be -DBITS=32 
-DENCODING=MSB
-// RUN: clang-offload-wrapper -target=x86_64-pc-linux-gnu -o 
%t.wrapper.elf32be.bc %t.32be
+// RUN: clang-offload-wrapper -add-omp-offload-notes 
-target=x86_64-pc-linux-gnu -o %t.wrapper.elf32be.bc %t.32be
 // RUN: llvm-dis %t.wrapper.elf32be.bc -o - | FileCheck %s --check-prefix 
OMPNOTES
 
 // There is no clean way for extracting the offload image


Index: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
===
--- clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
+++ clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
@@ -76,6 +76,10 @@
  "This option forces print-out of the temporary files' names."),
 cl::Hidden);
 
+static cl::opt AddOpenMPOffloadNotes(
+"add-omp-offload-notes",
+cl::desc("Add LLVMOMPOFFLOAD ELF notes to ELF device images."), cl::Hidden);
+
 namespace {
 
 class BinaryWrapper {
@@ -630,7 +634,7 @@
   return 1;
 }
 std::unique_ptr Buffer(std::move(*BufOrErr));
-if (File != "-") {
+if (File != "-" && AddOpenMPOffloadNotes) {
   // Adding ELF notes for STDIN is not supported yet.
   Buffer = Wrapper.addELFNotes(std::move(Buffer), File);
 }
Index: clang/test/Driver/clang-offload-wrapper.c
===
--- clang/test/Driver/clang-offload-wrapper.c
+++ clang/test/Driver/clang-offload-wrapper.c
@@ -19,7 +19,7 @@
 //
 // Check bitcode produced by the wrapper tool.
 //
-// RUN: clang-offload-wrapper -target=x86_64-pc-linux-gnu -o %t.wrapper.bc %t.tgt 2>&1 | FileCheck %s --check-prefix ELF-WARNING
+// RUN: clang-offload-wrapper -add-omp-offload-notes -target=x86_64-pc-linux-gnu -o %t.wrapper.bc %t.tgt 2>&1 | FileCheck %s --check-prefix ELF-WARNING
 // RUN: llvm-dis %t

[PATCH] D108246: [clang-offload-wrapper] Disabled ELF offload notes embedding by default.

2021-08-17 Thread Vyacheslav Zakharin via Phabricator via cfe-commits
vzakhari added a comment.

In D108246#2950650 , @jdoerfert wrote:

> I would prefer to revert 93d08acaacec951dbb302f77eeae51974985b6b2 
>  now and 
> then fix the CUDA toolchain rather than making it someone else's problem.

@jdoerfert, can we please keep the code upstream?  I've spent some time already 
to resolve the conflicts downstream, and I do not really want to do it once 
again.  I am taking the AR to triage the CUDA issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108246

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


[PATCH] D108246: [clang-offload-wrapper] Disabled ELF offload notes embedding by default.

2021-08-17 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert requested changes to this revision.
jdoerfert added a comment.
This revision now requires changes to proceed.

I would prefer to revert 93d08acaacec951dbb302f77eeae51974985b6b2 
 now and 
then fix the CUDA toolchain rather than making it someone else's problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108246

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


[PATCH] D108246: [clang-offload-wrapper] Disabled ELF offload notes embedding by default.

2021-08-17 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 accepted this revision.
jhuber6 added a comment.
This revision is now accepted and ready to land.

LGTM, applications compile and run now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108246

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


[PATCH] D108246: [clang-offload-wrapper] Disabled ELF offload notes embedding by default.

2021-08-17 Thread Vyacheslav Zakharin via Phabricator via cfe-commits
vzakhari created this revision.
vzakhari added a reviewer: jhuber6.
vzakhari added a project: OpenMP.
vzakhari requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

This change-set puts 93d08acaacec951dbb302f77eeae51974985b6b2 
 
functionality
under -add-omp-offload-notes switch that is OFF by default.
CUDA toolchain is not able to handle ELF images with LLVMOMPOFFLOAD
notes for unknown reason (see https://reviews.llvm.org/D99551#2950272).
I disable the ELF notes embedding until the CUDA issue is triaged and resolved.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108246

Files:
  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
@@ -76,6 +76,9 @@
  "This option forces print-out of the temporary files' names."),
 cl::Hidden);
 
+static cl::opt AddOpenMPOffloadNotes("add-omp-offload-notes",
+cl::desc("Add LLVMOMPOFFLOAD ELF notes to ELF device images."), 
cl::Hidden);
+
 namespace {
 
 class BinaryWrapper {
@@ -630,7 +633,7 @@
   return 1;
 }
 std::unique_ptr Buffer(std::move(*BufOrErr));
-if (File != "-") {
+if (File != "-" && AddOpenMPOffloadNotes) {
   // Adding ELF notes for STDIN is not supported yet.
   Buffer = Wrapper.addELFNotes(std::move(Buffer), File);
 }
Index: clang/test/Driver/clang-offload-wrapper.c
===
--- clang/test/Driver/clang-offload-wrapper.c
+++ clang/test/Driver/clang-offload-wrapper.c
@@ -19,7 +19,7 @@
 //
 // Check bitcode produced by the wrapper tool.
 //
-// RUN: clang-offload-wrapper -target=x86_64-pc-linux-gnu -o %t.wrapper.bc 
%t.tgt 2>&1 | FileCheck %s --check-prefix ELF-WARNING
+// RUN: clang-offload-wrapper -add-omp-offload-notes 
-target=x86_64-pc-linux-gnu -o %t.wrapper.bc %t.tgt 2>&1 | FileCheck %s 
--check-prefix ELF-WARNING
 // RUN: llvm-dis %t.wrapper.bc -o - | FileCheck %s --check-prefix CHECK-IR
 
 // ELF-WARNING: is not an ELF image, so notes cannot be added to it.
@@ -58,16 +58,16 @@
 // Check that clang-offload-wrapper adds LLVMOMPOFFLOAD notes
 // into the ELF offload images:
 // RUN: yaml2obj %S/Inputs/empty-elf-template.yaml -o %t.64le -DBITS=64 
-DENCODING=LSB
-// RUN: clang-offload-wrapper -target=x86_64-pc-linux-gnu -o 
%t.wrapper.elf64le.bc %t.64le
+// RUN: clang-offload-wrapper -add-omp-offload-notes 
-target=x86_64-pc-linux-gnu -o %t.wrapper.elf64le.bc %t.64le
 // RUN: llvm-dis %t.wrapper.elf64le.bc -o - | FileCheck %s --check-prefix 
OMPNOTES
 // RUN: yaml2obj %S/Inputs/empty-elf-template.yaml -o %t.64be -DBITS=64 
-DENCODING=MSB
-// RUN: clang-offload-wrapper -target=x86_64-pc-linux-gnu -o 
%t.wrapper.elf64be.bc %t.64be
+// RUN: clang-offload-wrapper -add-omp-offload-notes 
-target=x86_64-pc-linux-gnu -o %t.wrapper.elf64be.bc %t.64be
 // RUN: llvm-dis %t.wrapper.elf64be.bc -o - | FileCheck %s --check-prefix 
OMPNOTES
 // RUN: yaml2obj %S/Inputs/empty-elf-template.yaml -o %t.32le -DBITS=32 
-DENCODING=LSB
-// RUN: clang-offload-wrapper -target=x86_64-pc-linux-gnu -o 
%t.wrapper.elf32le.bc %t.32le
+// RUN: clang-offload-wrapper -add-omp-offload-notes 
-target=x86_64-pc-linux-gnu -o %t.wrapper.elf32le.bc %t.32le
 // RUN: llvm-dis %t.wrapper.elf32le.bc -o - | FileCheck %s --check-prefix 
OMPNOTES
 // RUN: yaml2obj %S/Inputs/empty-elf-template.yaml -o %t.32be -DBITS=32 
-DENCODING=MSB
-// RUN: clang-offload-wrapper -target=x86_64-pc-linux-gnu -o 
%t.wrapper.elf32be.bc %t.32be
+// RUN: clang-offload-wrapper -add-omp-offload-notes 
-target=x86_64-pc-linux-gnu -o %t.wrapper.elf32be.bc %t.32be
 // RUN: llvm-dis %t.wrapper.elf32be.bc -o - | FileCheck %s --check-prefix 
OMPNOTES
 
 // There is no clean way for extracting the offload image


Index: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
===
--- clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
+++ clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
@@ -76,6 +76,9 @@
  "This option forces print-out of the temporary files' names."),
 cl::Hidden);
 
+static cl::opt AddOpenMPOffloadNotes("add-omp-offload-notes",
+cl::desc("Add LLVMOMPOFFLOAD ELF notes to ELF device images."), cl::Hidden);
+
 namespace {
 
 class BinaryWrapper {
@@ -630,7 +633,7 @@
   return 1;
 }
 std::unique_ptr Buffer(std::move(*BufOrErr));
-if (File != "-") {
+if (File != "-" && AddOpenMPOffloadNotes) {
   // Adding ELF notes for STDIN is not supported yet.
   Buffer = Wrapper.addELFNotes(std::move(Buffer