[PATCH] D157438: [OpenMP] Ensure wrapper headers are included on both host and device

[Joseph Huber 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 rG61709bbae37a: [OpenMP] Ensure wrapper headers are included 
on both host and device (authored by jhuber6).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157438

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/gpu-libc-headers.c


Index: clang/test/Driver/gpu-libc-headers.c
===
--- clang/test/Driver/gpu-libc-headers.c
+++ clang/test/Driver/gpu-libc-headers.c
@@ -8,6 +8,7 @@
 // RUN: -fopenmp-targets=nvptx64-nvidia-cuda 
-Xopenmp-target=nvptx64-nvidia-cuda --offload-arch=sm_70  \
 // RUN: -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-HEADERS
 // CHECK-HEADERS: "-cc1"{{.*}}"-internal-isystem" 
"{{.*}}include{{.*}}llvm_libc_wrappers"{{.*}}"-isysroot" "./"
+// CHECK-HEADERS: "-cc1"{{.*}}"-internal-isystem" 
"{{.*}}include{{.*}}llvm_libc_wrappers"{{.*}}"-isysroot" "./"
 
 // RUN:   %clang -### --target=amdgcn-amd-amdhsa -mcpu=gfx1030 -nogpulib \
 // RUN: -nogpuinc %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-HEADERS-DISABLED
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -1183,14 +1183,13 @@
   // with ones created by the 'libc' project if present.
   if (!Args.hasArg(options::OPT_nostdinc) &&
   !Args.hasArg(options::OPT_nogpuinc) &&
-  !Args.hasArg(options::OPT_nobuiltininc) &&
-  (getToolChain().getTriple().isNVPTX() ||
-   getToolChain().getTriple().isAMDGCN())) {
-
+  !Args.hasArg(options::OPT_nobuiltininc)) {
 // Without an offloading language we will include these headers directly.
 // Offloading languages will instead only use the declarations stored in
 // the resource directory at clang/lib/Headers/llvm_libc_wrappers.
-if (C.getActiveOffloadKinds() == Action::OFK_None) {
+if ((getToolChain().getTriple().isNVPTX() ||
+ getToolChain().getTriple().isAMDGCN()) &&
+C.getActiveOffloadKinds() == Action::OFK_None) {
   SmallString<128> P(llvm::sys::path::parent_path(D.InstalledDir));
   llvm::sys::path::append(P, "include");
   llvm::sys::path::append(P, "gpu-none-llvm");


Index: clang/test/Driver/gpu-libc-headers.c
===
--- clang/test/Driver/gpu-libc-headers.c
+++ clang/test/Driver/gpu-libc-headers.c
@@ -8,6 +8,7 @@
 // RUN: -fopenmp-targets=nvptx64-nvidia-cuda -Xopenmp-target=nvptx64-nvidia-cuda --offload-arch=sm_70  \
 // RUN: -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-HEADERS
 // CHECK-HEADERS: "-cc1"{{.*}}"-internal-isystem" "{{.*}}include{{.*}}llvm_libc_wrappers"{{.*}}"-isysroot" "./"
+// CHECK-HEADERS: "-cc1"{{.*}}"-internal-isystem" "{{.*}}include{{.*}}llvm_libc_wrappers"{{.*}}"-isysroot" "./"
 
 // RUN:   %clang -### --target=amdgcn-amd-amdhsa -mcpu=gfx1030 -nogpulib \
 // RUN: -nogpuinc %s 2>&1 | FileCheck %s --check-prefix=CHECK-HEADERS-DISABLED
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -1183,14 +1183,13 @@
   // with ones created by the 'libc' project if present.
   if (!Args.hasArg(options::OPT_nostdinc) &&
   !Args.hasArg(options::OPT_nogpuinc) &&
-  !Args.hasArg(options::OPT_nobuiltininc) &&
-  (getToolChain().getTriple().isNVPTX() ||
-   getToolChain().getTriple().isAMDGCN())) {
-
+  !Args.hasArg(options::OPT_nobuiltininc)) {
 // Without an offloading language we will include these headers directly.
 // Offloading languages will instead only use the declarations stored in
 // the resource directory at clang/lib/Headers/llvm_libc_wrappers.
-if (C.getActiveOffloadKinds() == Action::OFK_None) {
+if ((getToolChain().getTriple().isNVPTX() ||
+ getToolChain().getTriple().isAMDGCN()) &&
+C.getActiveOffloadKinds() == Action::OFK_None) {
   SmallString<128> P(llvm::sys::path::parent_path(D.InstalledDir));
   llvm::sys::path::append(P, "include");
   llvm::sys::path::append(P, "gpu-none-llvm");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157438: [OpenMP] Ensure wrapper headers are included on both host and device

[Yaxun Liu via Phabricator via cfe-commits]

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

LGTM. Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157438

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

[PATCH] D157438: [OpenMP] Ensure wrapper headers are included on both host and device

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1190-1191
 // the resource directory at clang/lib/Headers/llvm_libc_wrappers.
-if (C.getActiveOffloadKinds() == Action::OFK_None) {
+if ((getToolChain().getTriple().isNVPTX() ||
+ getToolChain().getTriple().isAMDGCN()) &&
+C.getActiveOffloadKinds() == Action::OFK_None) {

yaxunl wrote:
> jhuber6 wrote:
> > arsenm wrote:
> > > can we do something better than this NVPTX||AMDGCN checks
> > This is more or less "Are we one of the GPUs `libc` supports". This is for 
> > cross-compiling so there's no existing infrastructure.
> maybe add a variable bool HasGPULibC as it is also used in other places below
I think this is the only use right now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157438

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

[PATCH] D157438: [OpenMP] Ensure wrapper headers are included on both host and device

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1190-1191
 // the resource directory at clang/lib/Headers/llvm_libc_wrappers.
-if (C.getActiveOffloadKinds() == Action::OFK_None) {
+if ((getToolChain().getTriple().isNVPTX() ||
+ getToolChain().getTriple().isAMDGCN()) &&
+C.getActiveOffloadKinds() == Action::OFK_None) {

jhuber6 wrote:
> arsenm wrote:
> > can we do something better than this NVPTX||AMDGCN checks
> This is more or less "Are we one of the GPUs `libc` supports". This is for 
> cross-compiling so there's no existing infrastructure.
maybe add a variable bool HasGPULibC as it is also used in other places below


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157438

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

[PATCH] D157438: [OpenMP] Ensure wrapper headers are included on both host and device

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1190-1191
 // the resource directory at clang/lib/Headers/llvm_libc_wrappers.
-if (C.getActiveOffloadKinds() == Action::OFK_None) {
+if ((getToolChain().getTriple().isNVPTX() ||
+ getToolChain().getTriple().isAMDGCN()) &&
+C.getActiveOffloadKinds() == Action::OFK_None) {

arsenm wrote:
> can we do something better than this NVPTX||AMDGCN checks
This is more or less "Are we one of the GPUs `libc` supports". This is for 
cross-compiling so there's no existing infrastructure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157438

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

[PATCH] D157438: [OpenMP] Ensure wrapper headers are included on both host and device

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1190-1191
 // the resource directory at clang/lib/Headers/llvm_libc_wrappers.
-if (C.getActiveOffloadKinds() == Action::OFK_None) {
+if ((getToolChain().getTriple().isNVPTX() ||
+ getToolChain().getTriple().isAMDGCN()) &&
+C.getActiveOffloadKinds() == Action::OFK_None) {

can we do something better than this NVPTX||AMDGCN checks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157438

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

[PATCH] D157438: [OpenMP] Ensure wrapper headers are included on both host and device

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 created this revision.
jhuber6 added reviewers: jdoerfert, tianshilei1992, tra, JonChesterfield, 
yaxunl, sivachandra.
Herald added a subscriber: guansong.
Herald added a project: All.
jhuber6 requested review of this revision.
Herald added subscribers: cfe-commits, jplehr, sstefan1, MaskRay.
Herald added a project: clang.

For the in-progress GPU `libc` project we are relying on overlay headers to
handle the interfacing between the `libc` project and the host `libc`.
We need this to be included on both the host and device so they agree
one what is present on the device, otherwise we will end up with random
errors. For whatever reason this was not being included on the host
although it previously worked. This patch ensures that it's included on
both.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157438

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/gpu-libc-headers.c


Index: clang/test/Driver/gpu-libc-headers.c
===
--- clang/test/Driver/gpu-libc-headers.c
+++ clang/test/Driver/gpu-libc-headers.c
@@ -8,6 +8,7 @@
 // RUN: -fopenmp-targets=nvptx64-nvidia-cuda 
-Xopenmp-target=nvptx64-nvidia-cuda --offload-arch=sm_70  \
 // RUN: -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-HEADERS
 // CHECK-HEADERS: "-cc1"{{.*}}"-internal-isystem" 
"{{.*}}include{{.*}}llvm_libc_wrappers"{{.*}}"-isysroot" "./"
+// CHECK-HEADERS: "-cc1"{{.*}}"-internal-isystem" 
"{{.*}}include{{.*}}llvm_libc_wrappers"{{.*}}"-isysroot" "./"
 
 // RUN:   %clang -### --target=amdgcn-amd-amdhsa -mcpu=gfx1030 -nogpulib \
 // RUN: -nogpuinc %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-HEADERS-DISABLED
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -1183,14 +1183,13 @@
   // with ones created by the 'libc' project if present.
   if (!Args.hasArg(options::OPT_nostdinc) &&
   !Args.hasArg(options::OPT_nogpuinc) &&
-  !Args.hasArg(options::OPT_nobuiltininc) &&
-  (getToolChain().getTriple().isNVPTX() ||
-   getToolChain().getTriple().isAMDGCN())) {
-
+  !Args.hasArg(options::OPT_nobuiltininc)) {
 // Without an offloading language we will include these headers directly.
 // Offloading languages will instead only use the declarations stored in
 // the resource directory at clang/lib/Headers/llvm_libc_wrappers.
-if (C.getActiveOffloadKinds() == Action::OFK_None) {
+if ((getToolChain().getTriple().isNVPTX() ||
+ getToolChain().getTriple().isAMDGCN()) &&
+C.getActiveOffloadKinds() == Action::OFK_None) {
   SmallString<128> P(llvm::sys::path::parent_path(D.InstalledDir));
   llvm::sys::path::append(P, "include");
   llvm::sys::path::append(P, "gpu-none-llvm");


Index: clang/test/Driver/gpu-libc-headers.c
===
--- clang/test/Driver/gpu-libc-headers.c
+++ clang/test/Driver/gpu-libc-headers.c
@@ -8,6 +8,7 @@
 // RUN: -fopenmp-targets=nvptx64-nvidia-cuda -Xopenmp-target=nvptx64-nvidia-cuda --offload-arch=sm_70  \
 // RUN: -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-HEADERS
 // CHECK-HEADERS: "-cc1"{{.*}}"-internal-isystem" "{{.*}}include{{.*}}llvm_libc_wrappers"{{.*}}"-isysroot" "./"
+// CHECK-HEADERS: "-cc1"{{.*}}"-internal-isystem" "{{.*}}include{{.*}}llvm_libc_wrappers"{{.*}}"-isysroot" "./"
 
 // RUN:   %clang -### --target=amdgcn-amd-amdhsa -mcpu=gfx1030 -nogpulib \
 // RUN: -nogpuinc %s 2>&1 | FileCheck %s --check-prefix=CHECK-HEADERS-DISABLED
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -1183,14 +1183,13 @@
   // with ones created by the 'libc' project if present.
   if (!Args.hasArg(options::OPT_nostdinc) &&
   !Args.hasArg(options::OPT_nogpuinc) &&
-  !Args.hasArg(options::OPT_nobuiltininc) &&
-  (getToolChain().getTriple().isNVPTX() ||
-   getToolChain().getTriple().isAMDGCN())) {
-
+  !Args.hasArg(options::OPT_nobuiltininc)) {
 // Without an offloading language we will include these headers directly.
 // Offloading languages will instead only use the declarations stored in
 // the resource directory at clang/lib/Headers/llvm_libc_wrappers.
-if (C.getActiveOffloadKinds() == Action::OFK_None) {
+if ((getToolChain().getTriple().isNVPTX() ||
+ getToolChain().getTriple().isAMDGCN()) &&
+C.getActiveOffloadKinds() == Action::OFK_None) {
   SmallString<128> P(llvm::sys::path::parent_path(D.InstalledDir));
   llvm::sys::path::append(P, "include");
   llvm::sys::path::append(P, "gpu-none-llvm");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org