[PATCH] D112103: [WIP][RFC] Sample code for containerizing offload images into one ELF.

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari abandoned this revision.
vzakhari added a comment.
Herald added a project: All.

Not working on the project any more.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112103

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

[PATCH] D112103: [WIP] Sample code for containerizing offload images into one ELF.

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari created this revision.
vzakhari added reviewers: saiislam, jdoerfert.
Herald added a reviewer: alexander-shaposhnikov.
vzakhari requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

This code cannot currently work within llorg, because it is mostly based on 
https://github.com/intel/llvm state of clang-offload-wrapper.  This patch is 
uploaded to facilitate discussion at OpenMP at LLVM weekly meeting.

The code shows how to containerize several offload images passed to 
clang-offload-wrapper into a single ELF file.  It also re-implements the 
LLVMOMPOFFLOAD notes embedding that currently uses llvm-objcopy in llorg.

The implementation uses yaml::yaml2elf utility.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112103

Files:
  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
@@ -26,6 +26,8 @@
 #include "llvm/IR/Module.h"
 #include "llvm/Object/ELFObjectFile.h"
 #include "llvm/Object/ObjectFile.h"
+#include "llvm/ObjectYAML/ELFYAML.h"
+#include "llvm/ObjectYAML/yaml2obj.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/EndianStream.h"
 #include "llvm/Support/Errc.h"
@@ -388,6 +390,181 @@
 return M;
   }
 
+  Expected getTmpFile(Twine NamePattern) {
+Expected TempFile =
+sys::fs::TempFile::create(NamePattern);
+if (!TempFile)
+  return TempFile.takeError();
+
+std::string FileName = TempFile->TmpName;
+
+if (Error E = TempFile->keep(FileName))
+  return std::move(E);
+
+TempFiles.push_back(std::move(FileName));
+return TempFiles.back();
+  }
+
+  void containerizeOpenMPImages() {
+// If there are no OpenMP images, there is nothing to do.
+auto OpenMPPackIt = Packs.find(OffloadKind::OpenMP);
+if (OpenMPPackIt == Packs.end())
+  return;
+SameKindPack *OpenMPPack = OpenMPPackIt->second.get();
+
+// Start creating notes for the ELF container.
+std::vector Notes;
+std::string ImgVersion = toHex(OPENMP_OFFLOAD_IMAGE_VERSION);
+Notes.emplace_back(ELFYAML::NoteEntry{"LLVMOMPOFFLOAD",
+yaml::BinaryRef(ImgVersion),
+ELF::NT_LLVM_OPENMP_OFFLOAD_VERSION});
+std::string Producer = toHex("LLVM");
+Notes.emplace_back(ELFYAML::NoteEntry{"LLVMOMPOFFLOAD",
+yaml::BinaryRef(Producer),
+ELF::NT_LLVM_OPENMP_OFFLOAD_PRODUCER});
+std::string ProdVersion = toHex(LLVM_VERSION_STRING
+#ifdef LLVM_REVISION
+" " LLVM_REVISION
+#endif
+);
+Notes.emplace_back(ELFYAML::NoteEntry{"LLVMOMPOFFLOAD",
+yaml::BinaryRef(ProdVersion),
+ELF::NT_LLVM_OPENMP_OFFLOAD_PRODUCER_VERSION});
+
+SmallVector ImageFiles;
+
+// Find SPIR-V images and create notes with auxiliary information
+// for each of them.
+unsigned ImageIdx = 0;
+for (auto I = OpenMPPack->begin(); I != OpenMPPack->end(); ++I) {
+  const BinaryWrapper::Image  = *(I->get());
+  if (Img.Tgt.find("spir") != 0)
+continue;
+
+  ImageFiles.push_back(Img.File);
+  ++ImageIdx;
+}
+
+// If there are no SPIR-V images, there is nothing to do.
+if (ImageIdx == 0)
+  return;
+
+StringRef ToolNameRef(ToolName);
+
+// Helper to emit warnings.
+auto warningOS = [ToolNameRef]() -> raw_ostream & {
+  return WithColor::warning(errs(), ToolNameRef);
+};
+auto handleErrorAsWarning = [](Error E) {
+  logAllUnhandledErrors(std::move(E), warningOS());
+};
+
+// Reserve  a temporary file for the ELF image.
+Expected ImageFileName =
+getTmpFile(Output + Twine(".spirv.elfimage.%%%.tmp"));
+if (!ImageFileName) {
+  logAllUnhandledErrors(ImageFileName.takeError(), warningOS());
+  return;
+}
+
+std::error_code EC;
+raw_fd_ostream ELFOS(ImageFileName->str(), EC);
+if (EC) {
+  warningOS() << "cannot open ELF file " << ImageFileName->str() << ": "
+  << EC.message() << "\n";
+  return;
+}
+
+// Write ELF file using yaml2elf instead of writing ELF by ourselves.
+// We use 64-bit little-endian ELF currently.
+ELFYAML::FileHeader Header{};
+Header.Class = ELF::ELFCLASS64;
+Header.Data = ELF::ELFDATA2LSB;
+Header.Type = ELF::ET_NONE;
+// Do not use any existing machine, otherwise, other plugins
+// may claim this image.
+Header.Machine = ELF::EM_NONE;
+
+// Create a section with notes.
+ELFYAML::NoteSection Section{};
+Section.Type = ELF::SHT_NOTE;
+Section.Name = ".note.openmp";
+Section.Notes.emplace(std::move(Notes));
+
+// The main ELFYAML structure describing our ELF container.
+ELFYAML::Object Object{};
+

[PATCH] D108673: [CodeExtractor] Preserve topological order for the return blocks.

[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 rG2e192ab1f457: [CodeExtractor] Preserve topological order for 
the return blocks. (authored by vzakhari).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108673

Files:
  clang/test/OpenMP/cancel_codegen.cpp
  clang/test/OpenMP/parallel_codegen.cpp
  llvm/lib/Transforms/IPO/IROutliner.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/test/Transforms/CodeExtractor/LoopExtractor.ll
  llvm/test/Transforms/CodeExtractor/LoopExtractor_crash.ll
  llvm/test/Transforms/CodeExtractor/LoopExtractor_infinite.ll
  llvm/test/Transforms/CodeExtractor/live_shrink_unsafe.ll
  llvm/test/Transforms/CodeExtractor/unreachable-block.ll
  llvm/test/Transforms/HotColdSplit/multiple-exits.ll
  llvm/test/Transforms/IROutliner/outlining-different-output-blocks.ll
  llvm/test/Transforms/IROutliner/outlining-same-output-blocks.ll
  llvm/test/Transforms/OpenMP/parallel_region_merging.ll
  llvm/test/tools/llvm-extract/extract-blocks-with-groups.ll

Index: llvm/test/tools/llvm-extract/extract-blocks-with-groups.ll
===
--- llvm/test/tools/llvm-extract/extract-blocks-with-groups.ll
+++ llvm/test/tools/llvm-extract/extract-blocks-with-groups.ll
@@ -8,9 +8,6 @@
 ; CHECK: newFuncRoot:
 ; CHECK:   br label %if.split
 ;
-; CHECK: end.exitStub: ; preds = %end.split
-; CHECK:   ret void
-;
 ; CHECK: then: ; preds = %if.split
 ; CHECK:   %tmp12 = shl i32 %arg1, 2
 ; CHECK:   %tmp13 = add nsw i32 %tmp12, %arg
@@ -32,6 +29,9 @@
 ; CHECK:   %tmp.0.ce = phi i32 [ %tmp13, %then ], [ %tmp25, %else ]
 ; CHECK:   store i32 %tmp.0.ce, i32* %tmp.0.ce.out
 ; CHECK:   br label %end.exitStub
+;
+; CHECK: end.exitStub: ; preds = %end.split
+; CHECK:   ret void
 ; CHECK: }
 
 ; The second extracted function is the region composed by the blocks
@@ -40,12 +40,6 @@
 ; CHECK: newFuncRoot:
 ; CHECK:   br label %bb14
 ;
-; CHECK: bb26.exitStub:; preds = %bb14
-; CHECK:   ret i1 true
-;
-; CHECK: bb30.exitStub:; preds = %bb20
-; CHECK:   ret i1 false
-;
 ; CHECK: bb14: ; preds = %newFuncRoot
 ; CHECK:   %tmp0 = and i32 %arg1, %arg
 ; CHECK:   %tmp1 = icmp slt i32 %tmp0, 0
@@ -57,6 +51,12 @@
 ; CHECK:   %tmp25 = add nsw i32 %tmp24, %tmp22
 ; CHECK:   store i32 %tmp25, i32* %tmp25.out
 ; CHECK:   br label %bb30.exitStub
+;
+; CHECK: bb26.exitStub:; preds = %bb14
+; CHECK:   ret i1 true
+;
+; CHECK: bb30.exitStub:; preds = %bb20
+; CHECK:   ret i1 false
 ; CHECK: }
 
 define i32 @foo(i32 %arg, i32 %arg1) {
Index: llvm/test/Transforms/OpenMP/parallel_region_merging.ll
===
--- llvm/test/Transforms/OpenMP/parallel_region_merging.ll
+++ llvm/test/Transforms/OpenMP/parallel_region_merging.ll
@@ -4715,8 +4715,6 @@
 ; CHECK1-NEXT:store i32 [[TMP0]], i32* [[TID_ADDR_LOCAL]], align 4
 ; CHECK1-NEXT:[[TID:%.*]] = load i32, i32* [[TID_ADDR_LOCAL]], align 4
 ; CHECK1-NEXT:br label [[OMP_PAR_REGION:%.*]]
-; CHECK1:   omp.par.outlined.exit.exitStub:
-; CHECK1-NEXT:ret void
 ; CHECK1:   omp.par.region:
 ; CHECK1-NEXT:br label [[OMP_PAR_MERGED:%.*]]
 ; CHECK1:   omp.par.merged:
@@ -4731,6 +4729,8 @@
 ; CHECK1-NEXT:br label [[OMP_PAR_PRE_FINALIZE:%.*]]
 ; CHECK1:   omp.par.pre_finalize:
 ; CHECK1-NEXT:br label [[OMP_PAR_OUTLINED_EXIT_EXITSTUB:%.*]]
+; CHECK1:   omp.par.outlined.exit.exitStub:
+; CHECK1-NEXT:ret void
 ;
 ;
 ; CHECK1-LABEL: define {{[^@]+}}@.omp_outlined.
@@ -4860,8 +4860,6 @@
 ; CHECK1-NEXT:store i32 [[TMP0]], i32* [[TID_ADDR_LOCAL]], align 4
 ; CHECK1-NEXT:[[TID:%.*]] = load i32, i32* [[TID_ADDR_LOCAL]], align 4
 ; CHECK1-NEXT:br label [[OMP_PAR_REGION:%.*]]
-; CHECK1:   omp.par.outlined.exit.exitStub:
-; CHECK1-NEXT:ret void
 ; CHECK1:   omp.par.region:
 ; CHECK1-NEXT:br label [[OMP_PAR_MERGED:%.*]]
 ; CHECK1:   omp.par.merged:
@@ -4897,6 +4895,8 @@
 ; CHECK1:   omp_region.body.split:
 ; CHECK1-NEXT:call void @__kmpc_end_master(%struct.ident_t* @[[GLOB1]], i32 [[OMP_GLOBAL_THREAD_NUM]])
 ; CHECK1-NEXT:br label [[OMP_REGION_END]]
+; CHECK1:   omp.par.outlined.exit.exitStub:
+; CHECK1-NEXT:ret void
 ;
 ;
 ; CHECK1-LABEL: define {{[^@]+}}@.omp_outlined..8
@@ -4944,8 +4944,6 @@
 ; CHECK1-NEXT:[[TID:%.*]] = load i32, i32* [[TID_ADDR_LOCAL]], align 4
 ; CHECK1-NEXT:[[TMP1:%.*]] = load float, float* [[F_RELOADED]], align 4
 ; 

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

[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 

[PATCH] D108303: [clang][openmp] Disable embedded elf notes

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added a comment.

In D108303#2952393 , @JonChesterfield 
wrote:

> I like D108246  more. None of the 
> offloading tests updated in D108246  failed 
> with the above patch, perhaps they're not run by `make check-openmp`

Right, the clang-offload-wrapper test is run by `check-clang`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108303

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

[PATCH] D108303: [clang][openmp] Disable embedded elf notes

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added a comment.

Hi Jon, I am about to merge D108246  that 
disables the notes embedding.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108303

___
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] D108246: [clang-offload-wrapper] Disabled ELF offload notes embedding by default.

[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 

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

[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.

[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 = 

[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

[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

[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

[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] D105375: [OPENMP]Remove const firstprivate allocation as a variable in a constant space.

[Vyacheslav Zakharin via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105375

___
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] D99360: [OpenMP][WIP] Add standard notes for ELF offload images

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari abandoned this revision.
vzakhari added a comment.

The revision was split into: D99612 , D99551 
, D99552 , 
D99553 


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

https://reviews.llvm.org/D99360

___
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 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] D99360: [OpenMP][WIP] Add standard notes for ELF offload images

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added a comment.

In D99360#2654244 , @MaskRay wrote:

>> It might make sense to do the llvm-readobj portions of this patch in a 
>> separate review, since they are somewhat independent.
>
> +1. Patches touching BinaryFormat/llvm-readobj/yaml2obj/etc part and others 
> (MC/CodeGen/etc) are often required to do 
> BinaryFormat/llvm-readobj/yaml2obj/etc separately.
> The part is usually very loosely connected with the MC/CodeGen/etc part code.

Thanks. I split the patches.

>> Decide how to test the LLVM ELF implementation of ELF light, since I expect 
>> the upstream builds will use libelf implementation.
>
> Does the current code use both llvm/Object and libelf? First, for in-tree 
> components we exclusively use llvm/Object - there should be no dependency on 
> the external libelf.

The current code uses only libelf.  I believe many OpenMP offload plugins have 
been using libelf for a long time, and they did not switch to LLVM/Object, when 
libomptarget was merged into the tree.  I am not going to change this now, 
because this may cause too much disturbance downstream.  The purpose of this 
patch is to make some ELF reading functionality available in OSes, where it is 
(historically) unreasonable to require libelf dependency for user environments.

> Having two implementations can make llvm/Object API refactoring difficult. 
> Other contributors will not know that an libelf implementation (a different 
> configuration) exists and can break the libelf implementation.

I think it will be actually vice versa :)  the upstream and many downstream 
repos will continue to use the libelf path, and the LLVM/Object path may be 
broken, as you say.  At the same time, we will have QA testing downstream for 
the LLVM/Object path, so I will be able to catch issues and upstream the fixes.




Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:327
+  // Look for llvm-objcopy.exe as well.
+  ObjcopyPathOrErr = sys::findProgramByName("llvm-objcopy.exe");
+  if (!ObjcopyPathOrErr) {

MaskRay wrote:
> `sys::findProgramByName` tests the filename with an `.exe` suffix appended, 
> no need to duplicate it in application code.
Thanks! Fixed in D99551.



Comment at: openmp/libomptarget/plugins/common/elf_common/elf_light.cpp:47
+//for 64-bit ELFs produced by LLVM.
+typedef struct {
+  uint32_t n_namesz; // Length of note's name.

MaskRay wrote:
> C++ does not require the use sites to prepend `struct` so `typedef struct` is 
> not used by convention.
Thanks! Fixed in D99553.


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

https://reviews.llvm.org/D99360

___
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 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 

[PATCH] D99360: [OpenMP][WIP] Add standard notes for ELF offload images

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added a comment.

In D99360#2652523 , @jhenderson wrote:

> It might make sense to do the llvm-readobj portions of this patch in a 
> separate review, since they are somewhat independent.

I agree.  I actually have them in two patches, but I squashed them together for 
the complete story.  If everyone agrees with this in general, I will upload two 
separate clang-formatted patches.


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

https://reviews.llvm.org/D99360

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

[PATCH] D99360: [OpenMP][WIP] Add standard notes for ELF offload images

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari updated this revision to Diff 333484.
vzakhari edited the summary of this revision.
vzakhari added a comment.

Fixed issues detected in Windows testing downstream.


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

https://reviews.llvm.org/D99360

Files:
  clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
  llvm/include/llvm/BinaryFormat/ELF.h
  llvm/tools/llvm-readobj/ELFDumper.cpp
  openmp/libomptarget/plugins/amdgpu/CMakeLists.txt
  openmp/libomptarget/plugins/common/elf_common/CMakeLists.txt
  openmp/libomptarget/plugins/common/elf_common/elf_common.cpp
  openmp/libomptarget/plugins/common/elf_common/elf_common.h
  openmp/libomptarget/plugins/common/elf_common/elf_constants.h
  openmp/libomptarget/plugins/common/elf_common/elf_light.cpp
  openmp/libomptarget/plugins/common/elf_common/elf_light.h
  openmp/libomptarget/plugins/remote/server/CMakeLists.txt

Index: openmp/libomptarget/plugins/remote/server/CMakeLists.txt
===
--- openmp/libomptarget/plugins/remote/server/CMakeLists.txt
+++ openmp/libomptarget/plugins/remote/server/CMakeLists.txt
@@ -10,7 +10,6 @@
 #
 ##===--===##
 
-include_directories(${LIBOMPTARGET_DEP_LIBELF_INCLUDE_DIRS})
 include_directories(${LIBOMPTARGET_SRC_DIR})
 include_directories(${LIBOMPTARGET_INCLUDE_DIR})
 include_directories(${GRPC_INCLUDE_DIR})
@@ -28,4 +27,4 @@
 grpc++
 protobuf
 ${OPENMP_PTHREAD_LIB}
-"-ldl" "-lomp" "-fopenmp" "-Wl,--version-script=${CMAKE_CURRENT_SOURCE_DIR}/../../exports" ${LIBOMPTARGET_DEP_LIBELF_LIBRARIES})
+"-ldl" "-lomp" "-fopenmp" "-Wl,--version-script=${CMAKE_CURRENT_SOURCE_DIR}/../../exports")
Index: openmp/libomptarget/plugins/common/elf_common/elf_light.h
===
--- /dev/null
+++ openmp/libomptarget/plugins/common/elf_common/elf_light.h
@@ -0,0 +1,132 @@
+//===-- elf_light.h - Basic ELF functionality ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Light ELF implementation provides basic ELF reading functionality.
+// It may be used in systems without libelf support, if the corresponding
+// LLVM ELF implementation is available.
+// The interface declared here must be independent of libelf.h/elf.h.
+//
+// NOTE: we can try to rely on https://github.com/WolfgangSt/libelf
+//   on Windows, if this implementation gets more complex.
+//
+//===--===//
+
+#ifndef LIBOMPTARGET_PLUGINS_ELF_LIGHT_H
+#define LIBOMPTARGET_PLUGINS_ELF_LIGHT_H
+
+#include 
+#include 
+#include 
+
+class ElfL;
+class ElfLSegmentNoteIterator;
+class ElfLSectionNoteIterator;
+class ElfNote;
+
+// Class representing NOTEs from PT_NOTE segments and SHT_NOTE sections.
+class ElfLNote {
+  const void *Impl = nullptr;
+
+  friend class ElfLSegmentNoteIterator;
+  friend class ElfLSectionNoteIterator;
+
+  // Only ElfLSectionNoteIterator is allowed to create notes via its operator*().
+  explicit ElfLNote(const void *I);
+  ElfLNote =(const ElfLNote &) = delete;
+
+public:
+  // FIXME: add move copy constructor and assignment operator.
+  ElfLNote(const ElfLNote &);
+  ~ElfLNote();
+  // Returns the note's name size not including the null terminator.
+  // Note that it may be illegal to access the getName() pointer
+  // beyond the returned size, i.e. the implementation may
+  // not guarantee that there is '\0' after getNameSize()
+  // characters of the name.
+  uint64_t getNameSize() const;
+  // Returns a pointer to the beginning of the note's name.
+  const char *getName() const;
+  // Returns the number of bytes in the descriptor.
+  uint64_t getDescSize() const;
+  // Returns a pointer to the beginning of the note's descriptor.
+  // It is illegal to access more that getDescSize() bytes
+  // via this pointer.
+  const uint8_t *getDesc() const;
+  uint64_t getType() const;
+};
+
+// Iterator over NOTEs in PT_NOTE segments.
+class ElfLSegmentNoteIterator
+: std::iterator {
+
+  void *Impl = nullptr;
+
+  friend class ElfL;
+
+  // Only ElfL is allowed to create iterators to itself.
+  ElfLSegmentNoteIterator(const void *I, bool IsEnd = false);
+  ElfLSectionNoteIterator =(const ElfLSegmentNoteIterator &) = delete;
+
+public:
+  // FIXME: add move copy constructor and assignment operator.
+  ElfLSegmentNoteIterator(const ElfLSegmentNoteIterator );
+  ~ElfLSegmentNoteIterator();
+  ElfLSegmentNoteIterator ++();
+  bool operator==(const ElfLSegmentNoteIterator Other) const;
+  bool operator!=(const ElfLSegmentNoteIterator Other) const;
+  ElfLNote operator*() 

[PATCH] D99360: [OpenMP][WIP] Add standard notes for ELF offload images

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari updated this revision to Diff 78.

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

https://reviews.llvm.org/D99360

Files:
  clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
  llvm/include/llvm/BinaryFormat/ELF.h
  llvm/tools/llvm-readobj/ELFDumper.cpp
  openmp/libomptarget/plugins/amdgpu/CMakeLists.txt
  openmp/libomptarget/plugins/common/elf_common/CMakeLists.txt
  openmp/libomptarget/plugins/common/elf_common/elf_common.cpp
  openmp/libomptarget/plugins/common/elf_common/elf_common.h
  openmp/libomptarget/plugins/common/elf_common/elf_constants.h
  openmp/libomptarget/plugins/common/elf_common/elf_light.cpp
  openmp/libomptarget/plugins/common/elf_common/elf_light.h
  openmp/libomptarget/plugins/remote/server/CMakeLists.txt

Index: openmp/libomptarget/plugins/remote/server/CMakeLists.txt
===
--- openmp/libomptarget/plugins/remote/server/CMakeLists.txt
+++ openmp/libomptarget/plugins/remote/server/CMakeLists.txt
@@ -10,7 +10,6 @@
 #
 ##===--===##
 
-include_directories(${LIBOMPTARGET_DEP_LIBELF_INCLUDE_DIRS})
 include_directories(${LIBOMPTARGET_SRC_DIR})
 include_directories(${LIBOMPTARGET_INCLUDE_DIR})
 include_directories(${GRPC_INCLUDE_DIR})
@@ -28,4 +27,4 @@
 grpc++
 protobuf
 ${OPENMP_PTHREAD_LIB}
-"-ldl" "-lomp" "-fopenmp" "-Wl,--version-script=${CMAKE_CURRENT_SOURCE_DIR}/../../exports" ${LIBOMPTARGET_DEP_LIBELF_LIBRARIES})
+"-ldl" "-lomp" "-fopenmp" "-Wl,--version-script=${CMAKE_CURRENT_SOURCE_DIR}/../../exports")
Index: openmp/libomptarget/plugins/common/elf_common/elf_light.h
===
--- /dev/null
+++ openmp/libomptarget/plugins/common/elf_common/elf_light.h
@@ -0,0 +1,122 @@
+//===-- elf_light.h - Basic ELF functionality ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Light ELF implementation provides basic ELF reading functionality.
+// It may be used in systems without libelf support, if the corresponding
+// LLVM ELF implementation is available.
+// The interface declared here must be independent of libelf.h/elf.h.
+//
+// NOTE: we can try to rely on https://github.com/WolfgangSt/libelf
+//   on Windows, if this implementation gets more complex.
+//
+//===--===//
+
+#ifndef LIBOMPTARGET_PLUGINS_ELF_LIGHT_H
+#define LIBOMPTARGET_PLUGINS_ELF_LIGHT_H
+
+#include 
+#include 
+#include 
+
+class ElfL;
+class ElfLSegmentNoteIterator;
+class ElfLSectionNoteIterator;
+class ElfNote;
+
+// Class representing NOTEs from PT_NOTE segments and SHT_NOTE sections.
+class ElfLNote {
+  const void *Impl = nullptr;
+
+  friend class ElfLSegmentNoteIterator;
+  friend class ElfLSectionNoteIterator;
+
+  // Only ElfLSectionNoteIterator is allowed to create notes via its operator*().
+  explicit ElfLNote(const void *I);
+  ElfLNote =(const ElfLNote &) = delete;
+
+public:
+  // FIXME: add move copy constructor and assignment operator.
+  ElfLNote(const ElfLNote &);
+  ~ElfLNote();
+  uint64_t getNameSize() const;
+  const char *getName() const;
+  uint64_t getDescSize() const;
+  const uint8_t *getDesc() const;
+  uint64_t getType() const;
+};
+
+// Iterator over NOTEs in PT_NOTE segments.
+class ElfLSegmentNoteIterator
+: std::iterator {
+
+  void *Impl = nullptr;
+
+  friend class ElfL;
+
+  // Only ElfL is allowed to create iterators to itself.
+  ElfLSegmentNoteIterator(const void *I, bool IsEnd = false);
+  ElfLSectionNoteIterator =(const ElfLSegmentNoteIterator &) = delete;
+
+public:
+  // FIXME: add move copy constructor and assignment operator.
+  ElfLSegmentNoteIterator(const ElfLSegmentNoteIterator );
+  ~ElfLSegmentNoteIterator();
+  ElfLSegmentNoteIterator ++();
+  bool operator==(const ElfLSegmentNoteIterator Other) const;
+  bool operator!=(const ElfLSegmentNoteIterator Other) const;
+  ElfLNote operator*() const;
+};
+
+// Iterator over NOTEs in SHT_NOTE sections.
+class ElfLSectionNoteIterator
+: std::iterator {
+
+  void *Impl = nullptr;
+
+  friend class ElfL;
+
+  // Only ElfL is allowed to create iterators to itself.
+  ElfLSectionNoteIterator(const void *I, bool IsEnd = false);
+  ElfLSectionNoteIterator =(const ElfLSectionNoteIterator &) = delete;
+
+public:
+  // FIXME: add move copy constructor and assignment operator.
+  ElfLSectionNoteIterator(const ElfLSectionNoteIterator );
+  ~ElfLSectionNoteIterator();
+  ElfLSectionNoteIterator ++();
+  bool operator==(const ElfLSectionNoteIterator Other) const;
+  bool operator!=(const ElfLSectionNoteIterator Other) 

[PATCH] D99360: [OpenMP][WIP] Add standard notes for ELF offload images

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari created this revision.
vzakhari added a project: OpenMP.
Herald added subscribers: kerbowa, rupprecht, guansong, yaxunl, mgorny, 
nhaehnle, jvesely.
Herald added a reviewer: alexshap.
Herald added a reviewer: jhenderson.
vzakhari requested review of this revision.
Herald added subscribers: llvm-commits, openmp-commits, cfe-commits, sstefan1, 
MaskRay.
Herald added a reviewer: jdoerfert.
Herald added projects: clang, LLVM.

This patch adds //standard// ELF notes into SHT_NOTE sections of ELF offload 
images passed to clang-offload-wrapper.  The notes then can be read by the 
offload plugins to get some extra information about the image.

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.

The second part of the patch implements an //ELF light// interface for the 
plugins to be able to ierate ELF notes in //SHT/PT_NOTE// sections/segments.  
One implementation is based on //libelf// and it can be used for platforms, 
where //libelf// depdency can be easily satisfied.  The second implementation 
is based on //LLVM ELFObjectFile// and requires in-tree build - this one can be 
used on Windows, etc.

Debug output from the plugins would look like this:

> TARGET Common ELF --> LLVMOMPOFFLOAD ELF note NT_LLVM_OPENMP_OFFLOAD_VERSION 
> with value: '1.0'
> TARGET Common ELF --> LLVMOMPOFFLOAD ELF note NT_LLVM_OPENMP_OFFLOAD_PRODUCER 
> with value: 'LLVM'
> TARGET Common ELF --> LLVMOMPOFFLOAD ELF note 
> NT_LLVM_OPENMP_OFFLOAD_PRODUCER_VERSION with value: '13.0.0git 
> 9f8975163c75b1f9f736f9a8e0a60e29ac062754'

**TODOs:**

- Find the right place to document //clang-offload-wrapper// behavior for ELF 
images.
- Write LIT tests.
- Decide how to test the //LLVM ELF// implementation of //ELF light//, since I 
expect the upstream builds will use //libelf// implementation.
- Perform thorough testing of //LLVM ELF// implementation on Windows downstream.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99360

Files:
  clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
  llvm/include/llvm/BinaryFormat/ELF.h
  llvm/tools/llvm-readobj/ELFDumper.cpp
  openmp/libomptarget/plugins/amdgpu/CMakeLists.txt
  openmp/libomptarget/plugins/common/elf_common/CMakeLists.txt
  openmp/libomptarget/plugins/common/elf_common/elf_common.cpp
  openmp/libomptarget/plugins/common/elf_common/elf_common.h
  openmp/libomptarget/plugins/common/elf_common/elf_constants.h
  openmp/libomptarget/plugins/common/elf_common/elf_light.cpp
  openmp/libomptarget/plugins/common/elf_common/elf_light.h
  openmp/libomptarget/plugins/remote/server/CMakeLists.txt

Index: openmp/libomptarget/plugins/remote/server/CMakeLists.txt
===
--- openmp/libomptarget/plugins/remote/server/CMakeLists.txt
+++ openmp/libomptarget/plugins/remote/server/CMakeLists.txt
@@ -10,7 +10,6 @@
 #
 ##===--===##
 
-include_directories(${LIBOMPTARGET_DEP_LIBELF_INCLUDE_DIRS})
 include_directories(${LIBOMPTARGET_SRC_DIR})
 include_directories(${LIBOMPTARGET_INCLUDE_DIR})
 include_directories(${GRPC_INCLUDE_DIR})
@@ -28,4 +27,4 @@
 grpc++
 protobuf
 ${OPENMP_PTHREAD_LIB}
-"-ldl" "-lomp" "-fopenmp" "-Wl,--version-script=${CMAKE_CURRENT_SOURCE_DIR}/../../exports" ${LIBOMPTARGET_DEP_LIBELF_LIBRARIES})
+"-ldl" "-lomp" "-fopenmp" "-Wl,--version-script=${CMAKE_CURRENT_SOURCE_DIR}/../../exports")
Index: openmp/libomptarget/plugins/common/elf_common/elf_light.h
===
--- /dev/null
+++ openmp/libomptarget/plugins/common/elf_common/elf_light.h
@@ -0,0 +1,122 @@
+//===-- elf_light.h - Basic ELF functionality ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Light ELF implementation provides basic ELF reading functionality.
+// It may be used in systems without libelf support, if the corresponding
+// LLVM ELF 

[PATCH] D80655: Define __SPIR__ macro for spir/spir64 targets.

[Vyacheslav Zakharin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3a1b07506c1f: Define __SPIR__ macro for spir/spir64 targets. 
(authored by vzakhari).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80655

Files:
  clang/lib/Basic/Targets/SPIR.cpp
  clang/test/Preprocessor/predefined-macros.c


Index: clang/test/Preprocessor/predefined-macros.c
===
--- clang/test/Preprocessor/predefined-macros.c
+++ clang/test/Preprocessor/predefined-macros.c
@@ -173,7 +173,17 @@
 
 // RUN: %clang_cc1 %s -E -dM -o - -x cl -triple spir-unknown-unknown \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-SPIR
-// CHECK-SPIR: #define __IMAGE_SUPPORT__ 1
+// CHECK-SPIR-DAG: #define __IMAGE_SUPPORT__ 1
+// CHECK-SPIR-DAG: #define __SPIR__ 1
+// CHECK-SPIR-DAG: #define __SPIR32__ 1
+// CHECK-SPIR-NOT: #define __SPIR64__ 1
+
+// RUN: %clang_cc1 %s -E -dM -o - -x cl -triple spir64-unknown-unknown \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-SPIR64
+// CHECK-SPIR64-DAG: #define __IMAGE_SUPPORT__ 1
+// CHECK-SPIR64-DAG: #define __SPIR__ 1
+// CHECK-SPIR64-DAG: #define __SPIR64__ 1
+// CHECK-SPIR64-NOT: #define __SPIR32__ 1
 
 // RUN: %clang_cc1 %s -E -dM -o - -x hip -triple amdgcn-amd-amdhsa \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-HIP
Index: clang/lib/Basic/Targets/SPIR.cpp
===
--- clang/lib/Basic/Targets/SPIR.cpp
+++ clang/lib/Basic/Targets/SPIR.cpp
@@ -23,10 +23,12 @@
 
 void SPIR32TargetInfo::getTargetDefines(const LangOptions ,
 MacroBuilder ) const {
+  SPIRTargetInfo::getTargetDefines(Opts, Builder);
   DefineStd(Builder, "SPIR32", Opts);
 }
 
 void SPIR64TargetInfo::getTargetDefines(const LangOptions ,
 MacroBuilder ) const {
+  SPIRTargetInfo::getTargetDefines(Opts, Builder);
   DefineStd(Builder, "SPIR64", Opts);
 }


Index: clang/test/Preprocessor/predefined-macros.c
===
--- clang/test/Preprocessor/predefined-macros.c
+++ clang/test/Preprocessor/predefined-macros.c
@@ -173,7 +173,17 @@
 
 // RUN: %clang_cc1 %s -E -dM -o - -x cl -triple spir-unknown-unknown \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-SPIR
-// CHECK-SPIR: #define __IMAGE_SUPPORT__ 1
+// CHECK-SPIR-DAG: #define __IMAGE_SUPPORT__ 1
+// CHECK-SPIR-DAG: #define __SPIR__ 1
+// CHECK-SPIR-DAG: #define __SPIR32__ 1
+// CHECK-SPIR-NOT: #define __SPIR64__ 1
+
+// RUN: %clang_cc1 %s -E -dM -o - -x cl -triple spir64-unknown-unknown \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-SPIR64
+// CHECK-SPIR64-DAG: #define __IMAGE_SUPPORT__ 1
+// CHECK-SPIR64-DAG: #define __SPIR__ 1
+// CHECK-SPIR64-DAG: #define __SPIR64__ 1
+// CHECK-SPIR64-NOT: #define __SPIR32__ 1
 
 // RUN: %clang_cc1 %s -E -dM -o - -x hip -triple amdgcn-amd-amdhsa \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-HIP
Index: clang/lib/Basic/Targets/SPIR.cpp
===
--- clang/lib/Basic/Targets/SPIR.cpp
+++ clang/lib/Basic/Targets/SPIR.cpp
@@ -23,10 +23,12 @@
 
 void SPIR32TargetInfo::getTargetDefines(const LangOptions ,
 MacroBuilder ) const {
+  SPIRTargetInfo::getTargetDefines(Opts, Builder);
   DefineStd(Builder, "SPIR32", Opts);
 }
 
 void SPIR64TargetInfo::getTargetDefines(const LangOptions ,
 MacroBuilder ) const {
+  SPIRTargetInfo::getTargetDefines(Opts, Builder);
   DefineStd(Builder, "SPIR64", Opts);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77184: Make it possible for lit.site.cfg to contain relative paths, and use it for llvm and clang

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added a comment.

In D77184#2056306 , @vzakhari wrote:

> It does not work on Windows (msbuild) for me, because `${pathlist_escaped}` 
> contains paths like `%(build_mode)s/bin` (caused by `set_llvm_build_mode`).  
> This seems to break something so that `python` process does not produce any 
> output at all: I checked both `OUTPUT_VARIABLE` and `ERROR_VARIABLE` and they 
> were empty on any invocation of `execute_process`.  @thakis, do you have any 
> ideas what might be wrong?
>
> FWIW, the code does not seem to work as expected for multi-configuration 
> environments, since the `%(build_mode)` substitution is not done yet and the 
> paths are not real filesystem paths.


Please ignore this.  It turned out the issue was in a python wrapper stuck in 
the middle.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77184



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

[PATCH] D77184: Make it possible for lit.site.cfg to contain relative paths, and use it for llvm and clang

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added a comment.
Herald added a project: LLVM.

It does not work on Windows (msbuild) for me, because `${pathlist_escaped}` 
contains paths like `%(build_mode)s/bin` (caused by `set_llvm_build_mode`).  
This seems to break something so that `python` process does not produce any 
output at all: I checked both `OUTPUT_VARIABLE` and `ERROR_VARIABLE` and they 
were empty on any invocation of `execute_process`.  @thakis, do you have any 
ideas what might be wrong?

FWIW, the code does not seem to work as expected for multi-configuration 
environments, since the `%(build_mode)` substitution is not done yet and the 
paths are not real filesystem paths.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77184



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

[PATCH] D75223: [clang-offload-wrapper] Lower priority of __tgt_register_lib in favor of __tgt_register_requires

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75223



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

[PATCH] D75223: [clang-offload-wrapper] Lower priority of __tgt_register_lib in favor of __tgt_register_requires

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added inline comments.



Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:291
 // Add this function to global destructors.
 appendToGlobalDtors(M, Func, 0);
   }

I think __tgt_unregister_lib should have a matching priority.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75223



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

[PATCH] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added a comment.

FYI, llvm.global_dtor fix is in https://reviews.llvm.org/D66373


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

https://reviews.llvm.org/D64943



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

[PATCH] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added a comment.

In D64943#1633372 , @lebedev.ri wrote:

> In D64943#1633357 , @vzakhari wrote:
>
> > In D64943#1633175 , @ABataev wrote:
> >
> > > In D64943#1633170 , @lebedev.ri 
> > > wrote:
> > >
> > > > In D64943#1633164 , @ABataev 
> > > > wrote:
> > > >
> > > > > In D64943#1619958 , 
> > > > > @sdmitriev wrote:
> > > > >
> > > > > > As I understand ‘atexit’ solution would be target dependent 
> > > > > > (‘__cxa_atexit’ on Linux and ‘atexit’ on Windows) whereas 
> > > > > > @llvm.global_ctors/dtors variables offer similar and platform 
> > > > > > neutral functionality 
> > > > > > (http://llvm.org/docs/LangRef.html#the-llvm-global-ctors-global-variable).
> > > > > >  Why do you think that ‘atexit’ is a better choice?
> > > > >
> > > > >
> > > > > Because it does not work on Windows, better to have portable 
> > > > > solution, if possible.
> > > >
> > > >
> > > > Is there a bug # ?
> > >
> > >
> > > @vzakhari?
> >
> >
> > I do not have bug #, but the issue was introduced with the following commit:
> >  commit f803b23879d9e1d9415ec1875713534dcc203df5 
> > 
> >  Author: Reid Kleckner 
> >  Date:   Fri Sep 7 23:07:55 2018 +
> >
> >   [COFF] Implement llvm.global_ctors priorities for MSVC COFF targets
> >   
> >   Summary:
> >   MSVC and LLD sort sections ASCII-betically, so we need to use section
> >   names that sort between .CRT$XCA (the start) and .CRT$XCU (the default
> >   priority).
> >   
> >   In the general case, use .CRT$XCT12345 as the section name, and let the
> >   linker sort the zero-padded digits.
> >   
> >   Users with low priorities typically want to initialize as early as
> >   possible, so use .CRT$XCA00199 for prioties less than 200. This number
> >   is arbitrary.
> >   
> >   Implements PR38552.
> >   
> >   Reviewers: majnemer, mstorsjo
> >   
> >   Subscribers: hiraditya, llvm-commits
> >   
> >   Differential Revision: https://reviews.llvm.org/D51820
> >   
> >   llvm-svn: 341727
> >   
> >   
> >
> > The destructors are still in .CRT$XT for default priority (65535) now, but 
> > for non-default priority they will go into .CRT$XC.  I will upload a fixing 
> > patch with a LIT test shortly.
> >
> > This clang-offload-wrapper commit will work properly, if we use default 
> > priority for the destructors.
>
>
> 'IMHO' if there is a problem with lowering of LLVM IR constructs for some
>  particular targets, that problem must be resolved instead of adding 
> workarounds.


I completely agree with you!  I am testing the patch for destructors.


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

https://reviews.llvm.org/D64943



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

[PATCH] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added a comment.

In D64943#1633175 , @ABataev wrote:

> In D64943#1633170 , @lebedev.ri 
> wrote:
>
> > In D64943#1633164 , @ABataev wrote:
> >
> > > In D64943#1619958 , @sdmitriev 
> > > wrote:
> > >
> > > > As I understand ‘atexit’ solution would be target dependent 
> > > > (‘__cxa_atexit’ on Linux and ‘atexit’ on Windows) whereas 
> > > > @llvm.global_ctors/dtors variables offer similar and platform neutral 
> > > > functionality 
> > > > (http://llvm.org/docs/LangRef.html#the-llvm-global-ctors-global-variable).
> > > >  Why do you think that ‘atexit’ is a better choice?
> > >
> > >
> > > Because it does not work on Windows, better to have portable solution, if 
> > > possible.
> >
> >
> > Is there a bug # ?
>
>
> @vzakhari?


I do not have bug #, but the issue was introduced with the following commit:
commit f803b23879d9e1d9415ec1875713534dcc203df5 

Author: Reid Kleckner 
Date:   Fri Sep 7 23:07:55 2018 +

  [COFF] Implement llvm.global_ctors priorities for MSVC COFF targets
  
  Summary:
  MSVC and LLD sort sections ASCII-betically, so we need to use section
  names that sort between .CRT$XCA (the start) and .CRT$XCU (the default
  priority).
  
  In the general case, use .CRT$XCT12345 as the section name, and let the
  linker sort the zero-padded digits.
  
  Users with low priorities typically want to initialize as early as
  possible, so use .CRT$XCA00199 for prioties less than 200. This number
  is arbitrary.
  
  Implements PR38552.
  
  Reviewers: majnemer, mstorsjo
  
  Subscribers: hiraditya, llvm-commits
  
  Differential Revision: https://reviews.llvm.org/D51820
  
  llvm-svn: 341727

The destructors are still in .CRT$XT for default priority (65535) now, but for 
non-default priority they will go into .CRT$XC.  I will upload a fixing patch 
with a LIT test shortly.

This clang-offload-wrapper commit will work properly, if we use default 
priority for the destructors.


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

https://reviews.llvm.org/D64943



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

[PATCH] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added inline comments.



Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:226
+// Add this function to global destructors.
+appendToGlobalDtors(M, Func, 0);
+  }

ABataev wrote:
> sdmitriev wrote:
> > vzakhari wrote:
> > > FYI, llvm.global_dtors does not work on Windows.  The symbol will be 
> > > placed into .CRT$XC[A-Z] portion of .CRT in 
> > > TargetLoweringObjectFileImpl.cpp, so, basically, __tgt_unregister_lib 
> > > will be called right after __tgt_register_lib.  We can either use a trick 
> > > from ASAN, i.e. put llvm.global_dtors into .CRT$XT[A-Z] (I am not sure 
> > > how solid this solution is) or call atexit() inside __tgt_register_lib to 
> > > register __tgt_unregister_lib terminator.
> > It works as expected on Linux, so I guess this is just a bug in lowering 
> > code for Windows that need to be fixed.
> Still, better to call atexit(), this is common solution to call a global 
> destructor/deinitializer
I agree.  One other thing: if `__tgt_register_lib` is never called do we want 
to call `__tgt_unregister_lib`?  It is possible with global_ctors/global_dtors, 
but not possible with atexit/cxa_atexit.


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

https://reviews.llvm.org/D64943



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

[PATCH] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added inline comments.



Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:226
+// Add this function to global destructors.
+appendToGlobalDtors(M, Func, 0);
+  }

FYI, llvm.global_dtors does not work on Windows.  The symbol will be placed 
into .CRT$XC[A-Z] portion of .CRT in TargetLoweringObjectFileImpl.cpp, so, 
basically, __tgt_unregister_lib will be called right after __tgt_register_lib.  
We can either use a trick from ASAN, i.e. put llvm.global_dtors into 
.CRT$XT[A-Z] (I am not sure how solid this solution is) or call atexit() inside 
__tgt_register_lib to register __tgt_unregister_lib terminator.


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

https://reviews.llvm.org/D64943



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

[PATCH] D63846: [clang] Preserve names of addrspacecast'ed values.

[Vyacheslav Zakharin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL365666: [clang] Preserve names of addrspacecasted 
values. (authored by vzakhari, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63846?vs=208564=209006#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63846

Files:
  cfe/trunk/lib/CodeGen/TargetInfo.cpp
  cfe/trunk/test/CodeGenCUDA/builtins-amdgcn.cu
  cfe/trunk/test/CodeGenOpenCL/address-spaces-conversions.cl
  cfe/trunk/test/CodeGenOpenCLCXX/address-space-deduction.cl
  cfe/trunk/test/CodeGenOpenCLCXX/addrspace-of-this.cl
  cfe/trunk/test/CodeGenOpenCLCXX/addrspace-operators.cl
  cfe/trunk/test/CodeGenOpenCLCXX/addrspace-references.cl
  cfe/trunk/test/CodeGenOpenCLCXX/template-address-spaces.cl

Index: cfe/trunk/lib/CodeGen/TargetInfo.cpp
===
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp
@@ -449,7 +449,9 @@
   // space, an address space conversion may end up as a bitcast.
   if (auto *C = dyn_cast(Src))
 return performAddrSpaceCast(CGF.CGM, C, SrcAddr, DestAddr, DestTy);
-  return CGF.Builder.CreatePointerBitCastOrAddrSpaceCast(Src, DestTy);
+  // Try to preserve the source's name to make IR more readable.
+  return CGF.Builder.CreatePointerBitCastOrAddrSpaceCast(
+  Src, DestTy, Src->hasName() ? Src->getName() + ".ascast" : "");
 }
 
 llvm::Constant *
Index: cfe/trunk/test/CodeGenOpenCL/address-spaces-conversions.cl
===
--- cfe/trunk/test/CodeGenOpenCL/address-spaces-conversions.cl
+++ cfe/trunk/test/CodeGenOpenCL/address-spaces-conversions.cl
@@ -13,7 +13,7 @@
   // CHECK-NOFAKE-NOT: addrspacecast
 
   arg_gen = _priv; // implicit cast with obtaining adr, private -> generic
-  // CHECK: %{{[0-9]+}} = addrspacecast i32* %var_priv to i32 addrspace(4)*
+  // CHECK: %{{[._a-z0-9]+}} = addrspacecast i32* %{{[._a-z0-9]+}} to i32 addrspace(4)*
   // CHECK-NOFAKE-NOT: addrspacecast
 
   arg_glob = (global int *)arg_gen; // explicit cast
Index: cfe/trunk/test/CodeGenCUDA/builtins-amdgcn.cu
===
--- cfe/trunk/test/CodeGenCUDA/builtins-amdgcn.cu
+++ cfe/trunk/test/CodeGenCUDA/builtins-amdgcn.cu
@@ -2,15 +2,15 @@
 #include "Inputs/cuda.h"
 
 // CHECK-LABEL: @_Z16use_dispatch_ptrPi(
-// CHECK: %2 = call i8 addrspace(4)* @llvm.amdgcn.dispatch.ptr()
-// CHECK: %3 = addrspacecast i8 addrspace(4)* %2 to i8 addrspace(4)**
+// CHECK: %[[PTR:.*]] = call i8 addrspace(4)* @llvm.amdgcn.dispatch.ptr()
+// CHECK: %{{.*}} = addrspacecast i8 addrspace(4)* %[[PTR]] to i8 addrspace(4)**
 __global__ void use_dispatch_ptr(int* out) {
   const int* dispatch_ptr = (const int*)__builtin_amdgcn_dispatch_ptr();
   *out = *dispatch_ptr;
 }
 
 // CHECK-LABEL: @_Z12test_ds_fmaxf(
-// CHECK: call float @llvm.amdgcn.ds.fmax(float addrspace(3)* @_ZZ12test_ds_fmaxfE6shared, float %2, i32 0, i32 0, i1 false)
+// CHECK: call float @llvm.amdgcn.ds.fmax(float addrspace(3)* @_ZZ12test_ds_fmaxfE6shared, float %{{[^,]*}}, i32 0, i32 0, i1 false)
 __global__
 void test_ds_fmax(float src) {
   __shared__ float shared;
Index: cfe/trunk/test/CodeGenOpenCLCXX/addrspace-operators.cl
===
--- cfe/trunk/test/CodeGenOpenCLCXX/addrspace-operators.cl
+++ cfe/trunk/test/CodeGenOpenCLCXX/addrspace-operators.cl
@@ -19,11 +19,11 @@
 //CHECK-LABEL: define spir_func void @_Z3barv()
 void bar() {
   C c;
-  //CHECK: addrspacecast %class.C* %c to %class.C addrspace(4)*
-  //CHECK: call void @_ZNU3AS41C6AssignE1E(%class.C addrspace(4)* %{{[0-9]+}}, i32 0)
+  //CHECK: [[A1:%[.a-z0-9]+]] = addrspacecast %class.C* [[C:%[a-z0-9]+]] to %class.C addrspace(4)*
+  //CHECK: call void @_ZNU3AS41C6AssignE1E(%class.C addrspace(4)* [[A1]], i32 0)
   c.Assign(a);
-  //CHECK: addrspacecast %class.C* %c to %class.C addrspace(4)*
-  //CHECK: call void @_ZNU3AS41C8OrAssignE1E(%class.C addrspace(4)* %{{[0-9]+}}, i32 0)
+  //CHECK: [[A2:%[.a-z0-9]+]] = addrspacecast %class.C* [[C]] to %class.C addrspace(4)*
+  //CHECK: call void @_ZNU3AS41C8OrAssignE1E(%class.C addrspace(4)* [[A2]], i32 0)
   c.OrAssign(a);
 
   E e;
@@ -35,19 +35,33 @@
   globI |= b;
   //CHECK: store i32 %add, i32 addrspace(1)* @globI
   globI += a;
-  //CHECK: store volatile i32 %and, i32 addrspace(1)* @globVI
+  //CHECK: [[GVIV1:%[0-9]+]] = load volatile i32, i32 addrspace(1)* @globVI
+  //CHECK: [[AND:%[a-z0-9]+]] = and i32 [[GVIV1]], 1
+  //CHECK: store volatile i32 [[AND]], i32 addrspace(1)* @globVI
   globVI &= b;
-  //CHECK: store volatile i32 %sub, i32 addrspace(1)* @globVI
+  //CHECK: [[GVIV2:%[0-9]+]] = load volatile i32, i32 addrspace(1)* @globVI
+  //CHECK: [[SUB:%[a-z0-9]+]] = sub nsw i32 [[GVIV2]], 0
+  //CHECK: 

[PATCH] D63846: [clang] Preserve names of addrspacecast'ed values.

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added a comment.

I changed the tests to use FileCheck variables whenever possible.


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

https://reviews.llvm.org/D63846



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

[PATCH] D63846: [clang] Preserve names of addrspacecast'ed values.

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari updated this revision to Diff 208564.

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

https://reviews.llvm.org/D63846

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGenCUDA/builtins-amdgcn.cu
  clang/test/CodeGenOpenCL/address-spaces-conversions.cl
  clang/test/CodeGenOpenCLCXX/address-space-deduction.cl
  clang/test/CodeGenOpenCLCXX/addrspace-of-this.cl
  clang/test/CodeGenOpenCLCXX/addrspace-operators.cl
  clang/test/CodeGenOpenCLCXX/addrspace-references.cl
  clang/test/CodeGenOpenCLCXX/template-address-spaces.cl

Index: clang/test/CodeGenOpenCLCXX/template-address-spaces.cl
===
--- clang/test/CodeGenOpenCLCXX/template-address-spaces.cl
+++ clang/test/CodeGenOpenCLCXX/template-address-spaces.cl
@@ -13,12 +13,12 @@
 // CHECK: %struct.S.0 = type { i32 addrspace(4)* }
 // CHECK: %struct.S.1 = type { i32 addrspace(1)* }
 
-// CHECK:  %0 = addrspacecast %struct.S* %sint to %struct.S addrspace(4)*
-// CHECK:  %call = call i32 @_ZNU3AS41SIiE3fooEv(%struct.S addrspace(4)* %0) #1
-// CHECK:  %1 = addrspacecast %struct.S.0* %sintptr to %struct.S.0 addrspace(4)*
-// CHECK:  %call1 = call i32 addrspace(4)* @_ZNU3AS41SIPU3AS4iE3fooEv(%struct.S.0 addrspace(4)* %1) #1
-// CHECK:  %2 = addrspacecast %struct.S.1* %sintptrgl to %struct.S.1 addrspace(4)*
-// CHECK:  %call2 = call i32 addrspace(1)* @_ZNU3AS41SIPU3AS1iE3fooEv(%struct.S.1 addrspace(4)* %2) #1
+// CHECK:  [[A1:%[.a-z0-9]+]] = addrspacecast %struct.S* %{{[a-z0-9]+}} to %struct.S addrspace(4)*
+// CHECK:  %call = call i32 @_ZNU3AS41SIiE3fooEv(%struct.S addrspace(4)* [[A1]]) #1
+// CHECK:  [[A2:%[.a-z0-9]+]] = addrspacecast %struct.S.0* %{{[a-z0-9]+}} to %struct.S.0 addrspace(4)*
+// CHECK:  %call1 = call i32 addrspace(4)* @_ZNU3AS41SIPU3AS4iE3fooEv(%struct.S.0 addrspace(4)* [[A2]]) #1
+// CHECK:  [[A3:%[.a-z0-9]+]] = addrspacecast %struct.S.1* %{{[a-z0-9]+}} to %struct.S.1 addrspace(4)*
+// CHECK:  %call2 = call i32 addrspace(1)* @_ZNU3AS41SIPU3AS1iE3fooEv(%struct.S.1 addrspace(4)* [[A3]]) #1
 
 void bar(){
   S sint;
Index: clang/test/CodeGenOpenCLCXX/addrspace-references.cl
===
--- clang/test/CodeGenOpenCLCXX/addrspace-references.cl
+++ clang/test/CodeGenOpenCLCXX/addrspace-references.cl
@@ -8,7 +8,7 @@
   // addrspacecast before passing the value to the function.
   // CHECK: [[REF:%.*]] = alloca i32
   // CHECK: store i32 1, i32* [[REF]]
-  // CHECK: [[REG:%[0-9]+]] = addrspacecast i32* [[REF]] to i32 addrspace(4)*
+  // CHECK: [[REG:%[.a-z0-9]+]] = addrspacecast i32* [[REF]] to i32 addrspace(4)*
   // CHECK: call spir_func i32 @_Z3barRU3AS4Kj(i32 addrspace(4)* dereferenceable(4) [[REG]])
   bar(1);
 }
Index: clang/test/CodeGenOpenCLCXX/addrspace-operators.cl
===
--- clang/test/CodeGenOpenCLCXX/addrspace-operators.cl
+++ clang/test/CodeGenOpenCLCXX/addrspace-operators.cl
@@ -19,11 +19,11 @@
 //CHECK-LABEL: define spir_func void @_Z3barv()
 void bar() {
   C c;
-  //CHECK: addrspacecast %class.C* %c to %class.C addrspace(4)*
-  //CHECK: call void @_ZNU3AS41C6AssignE1E(%class.C addrspace(4)* %{{[0-9]+}}, i32 0)
+  //CHECK: [[A1:%[.a-z0-9]+]] = addrspacecast %class.C* [[C:%[a-z0-9]+]] to %class.C addrspace(4)*
+  //CHECK: call void @_ZNU3AS41C6AssignE1E(%class.C addrspace(4)* [[A1]], i32 0)
   c.Assign(a);
-  //CHECK: addrspacecast %class.C* %c to %class.C addrspace(4)*
-  //CHECK: call void @_ZNU3AS41C8OrAssignE1E(%class.C addrspace(4)* %{{[0-9]+}}, i32 0)
+  //CHECK: [[A2:%[.a-z0-9]+]] = addrspacecast %class.C* [[C]] to %class.C addrspace(4)*
+  //CHECK: call void @_ZNU3AS41C8OrAssignE1E(%class.C addrspace(4)* [[A2]], i32 0)
   c.OrAssign(a);
 
   E e;
@@ -35,19 +35,33 @@
   globI |= b;
   //CHECK: store i32 %add, i32 addrspace(1)* @globI
   globI += a;
-  //CHECK: store volatile i32 %and, i32 addrspace(1)* @globVI
+  //CHECK: [[GVIV1:%[0-9]+]] = load volatile i32, i32 addrspace(1)* @globVI
+  //CHECK: [[AND:%[a-z0-9]+]] = and i32 [[GVIV1]], 1
+  //CHECK: store volatile i32 [[AND]], i32 addrspace(1)* @globVI
   globVI &= b;
-  //CHECK: store volatile i32 %sub, i32 addrspace(1)* @globVI
+  //CHECK: [[GVIV2:%[0-9]+]] = load volatile i32, i32 addrspace(1)* @globVI
+  //CHECK: [[SUB:%[a-z0-9]+]] = sub nsw i32 [[GVIV2]], 0
+  //CHECK: store volatile i32 [[SUB]], i32 addrspace(1)* @globVI
   globVI -= a;
 }
 
-//CHECK: define linkonce_odr void @_ZNU3AS41C6AssignE1E(%class.C addrspace(4)* %this, i32 %e)
-//CHECK: [[E:%[0-9]+]] = load i32, i32* %e.addr
-//CHECK: %me = getelementptr inbounds %class.C, %class.C addrspace(4)* %this1, i32 0, i32 0
-//CHECK: store i32 [[E]], i32 addrspace(4)* %me
+//CHECK: define linkonce_odr void @_ZNU3AS41C6AssignE1E(%class.C addrspace(4)*{{[ %a-z0-9]*}}, i32{{[ %a-z0-9]*}})
+//CHECK: [[THIS_ADDR:%[.a-z0-9]+]] = alloca %class.C addrspace(4)
+//CHECK: [[E_ADDR:%[.a-z0-9]+]] = alloca i32
+//CHECK: store 

[PATCH] D63846: [clang] Preserve names of addrspacecast'ed values.

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added a comment.

In D63846#1574302 , @rjmccall wrote:

> I don't know what I think about widespread use of `-fno-discard-value-names` 
> for now; please continue to use FileCheck variables, and we can make a 
> holistic decision about that flag later.


Sorry, I have one particular question about 
clang/test/CodeGenOpenCLCXX/addrspace-of-this.cl:
// Test the address space of 'this' when invoking copy-constructor.
// COMMON: [[C1GEN:%c1.ascast[0-9]*]] = addrspacecast %class.C* %c1 to %class.C 
addrspace(4)*

This check seems to rely on %c1 name already.  I guess the matching may go off, 
if we do not use actual names on the right hand side of the assignment.  Should 
I do anything about the right hand side, or just use a generic wildcard on the 
left hand side?


Repository:
  rC Clang

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

https://reviews.llvm.org/D63846



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

[PATCH] D63846: [clang] Preserve names of addrspacecast'ed values.

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari marked an inline comment as done.
vzakhari added a comment.

In D63846#1574102 , @rjmccall wrote:

> Please don't check IR names in test output.  That actually includes anonymous 
> names like `%2`; these should always be tested with FileCheck variables.  I 
> suggest using `%.*` as the pattern; if you're matching the LHS of an LLVM 
> assignment, that shouldn't have problems with accidentally matching too much.


I agree that checking IR names is a bad practice.  I can change all these tests 
so that they either use FileCheck variables or -fno-discard-value-names (e.g. 
for clang/test/CodeGenOpenCLCXX/addrspace-of-this.cl, which relies on the names 
for checking) - would that be an appropriate solution?




Comment at: clang/test/CodeGenOpenCL/address-spaces-conversions.cl:16
   arg_gen = _priv; // implicit cast with obtaining adr, private -> generic
-  // CHECK: %{{[0-9]+}} = addrspacecast i32* %var_priv to i32 addrspace(4)*
+  // CHECK: %var_priv.ascast = addrspacecast i32* %var_priv to i32 
addrspace(4)*
   // CHECK-NOFAKE-NOT: addrspacecast

erichkeane wrote:
> You probably don't want to remove the wildcard here.  If this is built 
> without names being saved, this change will fail.
In the tests that already use variable names I stick to checking the names, 
e.g. this test already assumes that the names are preserved (%var_priv in this 
check), so I am just checking that 'ascast' suffix is added.  I guess I will 
have to add -fno-discard-value-names for this test - do you agree?


Repository:
  rC Clang

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

https://reviews.llvm.org/D63846



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

[PATCH] D63846: [clang] Preserve names of addrspacecast'ed values.

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added a comment.

ping


Repository:
  rC Clang

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

https://reviews.llvm.org/D63846



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

[PATCH] D63846: [clang] Preserve names of addrspacecast'ed values.

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added reviewers: erichkeane, rsmith.
vzakhari added a comment.

Adding more reviewers.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63846



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

[PATCH] D63846: [clang] Preserve names of addrspacecast'ed values.

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added a comment.

ping


Repository:
  rC Clang

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

https://reviews.llvm.org/D63846



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

[PATCH] D63846: [clang] Preserve names of addrspacecast'ed values.

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari created this revision.
vzakhari added a reviewer: rjmccall.
Herald added subscribers: cfe-commits, jfb, jvesely.
Herald added a project: clang.

Attach ".ascast" suffix to a value name when generating addrspacecast for it.  
This improves IR readability, e.g. for alloca variables, since all users of the 
variable will be using the addrspacecast value instead of the original alloca.


Repository:
  rC Clang

https://reviews.llvm.org/D63846

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGenCUDA/builtins-amdgcn.cu
  clang/test/CodeGenOpenCL/address-spaces-conversions.cl
  clang/test/CodeGenOpenCL/atomic-ops-libcall.cl
  clang/test/CodeGenOpenCLCXX/address-space-deduction.cl
  clang/test/CodeGenOpenCLCXX/addrspace-of-this.cl
  clang/test/CodeGenOpenCLCXX/addrspace-operators.cl
  clang/test/CodeGenOpenCLCXX/addrspace-references.cl
  clang/test/CodeGenOpenCLCXX/template-address-spaces.cl

Index: clang/test/CodeGenOpenCLCXX/template-address-spaces.cl
===
--- clang/test/CodeGenOpenCLCXX/template-address-spaces.cl
+++ clang/test/CodeGenOpenCLCXX/template-address-spaces.cl
@@ -13,12 +13,12 @@
 // CHECK: %struct.S.0 = type { i32 addrspace(4)* }
 // CHECK: %struct.S.1 = type { i32 addrspace(1)* }
 
-// CHECK:  %0 = addrspacecast %struct.S* %sint to %struct.S addrspace(4)*
-// CHECK:  %call = call i32 @_ZNU3AS41SIiE3fooEv(%struct.S addrspace(4)* %0) #1
-// CHECK:  %1 = addrspacecast %struct.S.0* %sintptr to %struct.S.0 addrspace(4)*
-// CHECK:  %call1 = call i32 addrspace(4)* @_ZNU3AS41SIPU3AS4iE3fooEv(%struct.S.0 addrspace(4)* %1) #1
-// CHECK:  %2 = addrspacecast %struct.S.1* %sintptrgl to %struct.S.1 addrspace(4)*
-// CHECK:  %call2 = call i32 addrspace(1)* @_ZNU3AS41SIPU3AS1iE3fooEv(%struct.S.1 addrspace(4)* %2) #1
+// CHECK:  [[A1:%sint.ascast[0-9]*]] = addrspacecast %struct.S* %sint to %struct.S addrspace(4)*
+// CHECK:  %call = call i32 @_ZNU3AS41SIiE3fooEv(%struct.S addrspace(4)* [[A1]]) #1
+// CHECK:  [[A2:%sintptr.ascast[0-9]*]] = addrspacecast %struct.S.0* %sintptr to %struct.S.0 addrspace(4)*
+// CHECK:  %call1 = call i32 addrspace(4)* @_ZNU3AS41SIPU3AS4iE3fooEv(%struct.S.0 addrspace(4)* [[A2]]) #1
+// CHECK:  [[A3:%sintptrgl.ascast[0-9]*]] = addrspacecast %struct.S.1* %sintptrgl to %struct.S.1 addrspace(4)*
+// CHECK:  %call2 = call i32 addrspace(1)* @_ZNU3AS41SIPU3AS1iE3fooEv(%struct.S.1 addrspace(4)* [[A3]]) #1
 
 void bar(){
   S sint;
Index: clang/test/CodeGenOpenCLCXX/addrspace-references.cl
===
--- clang/test/CodeGenOpenCLCXX/addrspace-references.cl
+++ clang/test/CodeGenOpenCLCXX/addrspace-references.cl
@@ -8,7 +8,7 @@
   // addrspacecast before passing the value to the function.
   // CHECK: [[REF:%.*]] = alloca i32
   // CHECK: store i32 1, i32* [[REF]]
-  // CHECK: [[REG:%[0-9]+]] = addrspacecast i32* [[REF]] to i32 addrspace(4)*
+  // CHECK: [[REG:%[.a-z0-9]+]] = addrspacecast i32* [[REF]] to i32 addrspace(4)*
   // CHECK: call spir_func i32 @_Z3barRU3AS4Kj(i32 addrspace(4)* dereferenceable(4) [[REG]])
   bar(1);
 }
Index: clang/test/CodeGenOpenCLCXX/addrspace-operators.cl
===
--- clang/test/CodeGenOpenCLCXX/addrspace-operators.cl
+++ clang/test/CodeGenOpenCLCXX/addrspace-operators.cl
@@ -19,11 +19,11 @@
 //CHECK-LABEL: define spir_func void @_Z3barv()
 void bar() {
   C c;
-  //CHECK: addrspacecast %class.C* %c to %class.C addrspace(4)*
-  //CHECK: call void @_ZNU3AS41C6AssignE1E(%class.C addrspace(4)* %{{[0-9]+}}, i32 0)
+  //CHECK: [[A1:%c.ascast[0-9]*]] = addrspacecast %class.C* %c to %class.C addrspace(4)*
+  //CHECK: call void @_ZNU3AS41C6AssignE1E(%class.C addrspace(4)* [[A1]], i32 0)
   c.Assign(a);
-  //CHECK: addrspacecast %class.C* %c to %class.C addrspace(4)*
-  //CHECK: call void @_ZNU3AS41C8OrAssignE1E(%class.C addrspace(4)* %{{[0-9]+}}, i32 0)
+  //CHECK: [[A2:%c.ascast[0-9]*]] = addrspacecast %class.C* %c to %class.C addrspace(4)*
+  //CHECK: call void @_ZNU3AS41C8OrAssignE1E(%class.C addrspace(4)* [[A2]], i32 0)
   c.OrAssign(a);
 
   E e;
Index: clang/test/CodeGenOpenCLCXX/addrspace-of-this.cl
===
--- clang/test/CodeGenOpenCLCXX/addrspace-of-this.cl
+++ clang/test/CodeGenOpenCLCXX/addrspace-of-this.cl
@@ -94,30 +94,30 @@
 // COMMON: call i32 @_ZNU3AS41C7outsideEv(%class.C addrspace(4)* addrspacecast (%class.C addrspace(1)* @c to %class.C addrspace(4)*))
 
 // Test the address space of 'this' when invoking copy-constructor.
-// COMMON: [[C1GEN:%[0-9]+]] = addrspacecast %class.C* %c1 to %class.C addrspace(4)*
+// COMMON: [[C1GEN:%c1.ascast[0-9]*]] = addrspacecast %class.C* %c1 to %class.C addrspace(4)*
 // IMPL: [[C1VOID:%[0-9]+]] = bitcast %class.C* %c1 to i8*
 // IMPL: call void @llvm.memcpy.p0i8.p4i8.i32(i8* {{.*}}[[C1VOID]], i8 addrspace(4)* {{.*}}addrspacecast (i8 addrspace(1)* bitcast (%class.C 

[PATCH] D61220: lib/Header: Fix Visual Studio builds try #2

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added a comment.

OK, this should work while clang_generate_header() is used for plain files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61220



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

[PATCH] D61220: lib/Header: Fix Visual Studio builds try #2

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added inline comments.



Comment at: clang/lib/Headers/CMakeLists.txt:176
 install(
-  DIRECTORY ${output_dir}
+  FILES ${install_files}
   DESTINATION ${header_install_dir}

vzakhari wrote:
> This is going to flatten the headers' install directory structure.  install() 
> will put the files just by their name.
> 
> I had a local fix that use install() with RENAME, and it worked.  I can try 
> to find it.  Basically, we need to keep two lists:
> 1. The same as this ${install_files}
> 2. New list install_as, which holds all ${src} values passed to 
> copy_header_to_output_dir()
> 
> Walking the two lists together we may use install(FILES 
> ${elt_from_install_files} RENAME ${elt_from_install_as} ...)
Correction.

> New list install_as, which holds all ${src} values passed to 
> copy_header_to_output_dir()

Should be
New list install_as, which holds all ${file} values passed to 
copy_header_to_output_dir()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61220



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

[PATCH] D61220: lib/Header: Fix Visual Studio builds try #2

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added inline comments.



Comment at: clang/lib/Headers/CMakeLists.txt:173
 
 set(header_install_dir lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION})
 

Please add "/include" at the end.



Comment at: clang/lib/Headers/CMakeLists.txt:176
 install(
-  DIRECTORY ${output_dir}
+  FILES ${install_files}
   DESTINATION ${header_install_dir}

This is going to flatten the headers' install directory structure.  install() 
will put the files just by their name.

I had a local fix that use install() with RENAME, and it worked.  I can try to 
find it.  Basically, we need to keep two lists:
1. The same as this ${install_files}
2. New list install_as, which holds all ${src} values passed to 
copy_header_to_output_dir()

Walking the two lists together we may use install(FILES 
${elt_from_install_files} RENAME ${elt_from_install_as} ...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61220



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

[PATCH] D58537: lib/Header: Simplify CMakeLists.txt

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added a comment.

In D58537#1476570 , @tstellar wrote:

> Can you test D61054 ?


I will do it ASAP, but I am currently having problems with my build system.  I 
guess it is OK to proceed with this change.  I will send an update, when I try 
it.  Thank you for fixing this!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58537



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

[PATCH] D58537: lib/Header: Simplify CMakeLists.txt

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added inline comments.



Comment at: cfe/trunk/lib/Headers/CMakeLists.txt:168
 install(
-  FILES ${cuda_wrapper_files}
-  COMPONENT clang-headers
-  PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ
-  DESTINATION 
lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/include/cuda_wrappers)
+  DIRECTORY ${output_dir}
+  DESTINATION ${header_install_dir}

tstellar wrote:
> vzakhari wrote:
> > This change results in a use of CMAKE_CFG_INTDIR, which expands to 
> > $(Configuration) inside cmake_install.cmake, when using Visual Studio 
> > generator. CMake cannot reasonably expand $(Configuration), so Visual 
> > Studio builds are broken for me after this change-set.
> > 
> > Can we revert to installation from FILES relative to the current source 
> > dir?  This will require keeping a separate list of source files in 
> > copy_header_to_output_dir(), and using this list in the install() command.
> > 
> > I do understand that the intention was, probably, to copy headers files 
> > into output_dir along with creating some structure inside output_dir, and 
> > then installing the whole output_dir verbatim to the install dir.  It will 
> > be hard to achieve this with the change I suggest, but can we fix Visual 
> > Studio builds in any other way?
> > 
> > FWIW, vcproj invokes the installation with the following command: cmake 
> > -DBUILD_TYPE=$(Configuration) -P cmake_install.cmake
> > CMake could have expanded CMAKE_CFG_INTDIR as ${BUILD_TYPE} instead of 
> > $(Configuration) inside cmake_install.cmake.
> > This change results in a use of CMAKE_CFG_INTDIR, which expands to 
> > $(Configuration) inside cmake_install.cmake, when using Visual Studio 
> > generator. CMake cannot reasonably expand $(Configuration), so Visual 
> > Studio builds are broken for me after this change-set.
> 
> Prior to this change we were installing the arm generated files from 
> ${CMAKE_CURRENT_BINARY_DIR}.  Do we really need to use 
> ${LLVM_LIBRARY_OUTPUT_INTDIR} as the base for output_dir?  Could we use 
> ${CMAKE_CURRENT_BINARY_DIR} instead?  That seems like it would fix this issue.
> 
> > FWIW, vcproj invokes the installation with the following command: cmake 
> > -DBUILD_TYPE=$(Configuration) -P cmake_install.cmake
> CMake could have expanded CMAKE_CFG_INTDIR as ${BUILD_TYPE} instead of 
> $(Configuration) inside cmake_install.cmake.
> 
> Are the vc project files part of the LLVM source tree?
> 
> 
> 
> 
> 
As I understand, ${CMAKE_CURRENT_BINARY_DIR} is the same directory for all 
(Debug, Release) builds for the multiconfiguration builders.  I am not sure if 
it is possible to run Debug and Release builds in parallel, which would imply 
parallel access to the generated files.  Maybe it is a potential problem, but 
we have it even now inside clang_generate_header() function.

VC project files are generated by CMake, that is why I said CMake could have 
used ${BUILD_TYPE} in the cmake_install.cmake files instead of $(Configuration).

I guess just using ${CMAKE_CURRENT_BINARY_DIR} as the output_dir will solve the 
current problem.  It should work as long as Debug and Release versions of the 
header files are functionally equivalent, and noone runs Debug and Release 
builds in parallel.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58537



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

[PATCH] D58537: lib/Header: Simplify CMakeLists.txt

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added inline comments.



Comment at: cfe/trunk/lib/Headers/CMakeLists.txt:168
 install(
-  FILES ${cuda_wrapper_files}
-  COMPONENT clang-headers
-  PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ
-  DESTINATION 
lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/include/cuda_wrappers)
+  DIRECTORY ${output_dir}
+  DESTINATION ${header_install_dir}

This change results in a use of CMAKE_CFG_INTDIR, which expands to 
$(Configuration) inside cmake_install.cmake, when using Visual Studio 
generator. CMake cannot reasonably expand $(Configuration), so Visual Studio 
builds are broken for me after this change-set.

Can we revert to installation from FILES relative to the current source dir?  
This will require keeping a separate list of source files in 
copy_header_to_output_dir(), and using this list in the install() command.

I do understand that the intention was, probably, to copy headers files into 
output_dir along with creating some structure inside output_dir, and then 
installing the whole output_dir verbatim to the install dir.  It will be hard 
to achieve this with the change I suggest, but can we fix Visual Studio builds 
in any other way?

FWIW, vcproj invokes the installation with the following command: cmake 
-DBUILD_TYPE=$(Configuration) -P cmake_install.cmake
CMake could have expanded CMAKE_CFG_INTDIR as ${BUILD_TYPE} instead of 
$(Configuration) inside cmake_install.cmake.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58537



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

[PATCH] D56112: [clang-offload-bundler] Added install component

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added a comment.

ping


Repository:
  rC Clang

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

https://reviews.llvm.org/D56112



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

[PATCH] D56112: [clang-offload-bundler] Added install component

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari created this revision.
vzakhari added a reviewer: chandlerc.
Herald added subscribers: cfe-commits, mgorny.

Allow selective install of clang-offload-bundler


Repository:
  rC Clang

https://reviews.llvm.org/D56112

Files:
  CMakeLists.txt


Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -22,4 +22,7 @@
   ${CLANG_OFFLOAD_BUNDLER_LIB_DEPS}
   )

-install(TARGETS clang-offload-bundler RUNTIME DESTINATION bin)
+install(TARGETS clang-offload-bundler
+  COMPONENT clang-offload-bundler
+  RUNTIME DESTINATION bin
+  )


Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -22,4 +22,7 @@
   ${CLANG_OFFLOAD_BUNDLER_LIB_DEPS}
   )

-install(TARGETS clang-offload-bundler RUNTIME DESTINATION bin)
+install(TARGETS clang-offload-bundler
+  COMPONENT clang-offload-bundler
+  RUNTIME DESTINATION bin
+  )
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits