[PATCH] D109718: [HIP] Diagnose -fopenmp-targets for HIP programs

2021-09-15 Thread Yaxun Liu 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 rGab5f2b505a07: [HIP] Diagnose -fopenmp-targets for HIP 
programs (authored by yaxunl).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109718

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/hip-options.hip


Index: clang/test/Driver/hip-options.hip
===
--- clang/test/Driver/hip-options.hip
+++ clang/test/Driver/hip-options.hip
@@ -87,3 +87,15 @@
 // THINLTO-NOT: clang{{.*}}" "-triple" "amdgcn-amd-amdhsa" {{.*}} 
"-fwhole-program-vtables"
 // THINLTO: clang{{.*}}" "-triple" "x86_64-unknown-linux-gnu" {{.*}} 
"-flto=thin" {{.*}} "-fwhole-program-vtables"
 // THINLTO-NOT: clang{{.*}}" "-triple" "amdgcn-amd-amdhsa" {{.*}} 
"-fwhole-program-vtables"
+
+// Check -fopenmp is allowed with HIP but -fopenmp-targets= is not allowed.
+
+// RUN: %clang -### -target x86_64-unknown-linux-gnu -nogpuinc -nogpulib \
+// RUN:   --offload-arch=gfx906 -fopenmp %s 2>&1 | FileCheck -check-prefix=OMP 
%s
+// OMP-NOT: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} "-fopenmp"
+// OMP: clang{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}} "-fopenmp"
+
+// RUN: not %clang -target x86_64-unknown-linux-gnu -nogpuinc -nogpulib \
+// RUN:   --offload-arch=gfx906 -fopenmp -fopenmp-targets=amdgcn %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=OMPTGT %s
+// OMPTGT: unsupported option '-fopenmp-targets=' for language mode 'HIP'
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -680,6 +680,12 @@
 }
 C.addOffloadDeviceToolChain(CudaTC.get(), OFK);
   } else if (IsHIP) {
+if (auto *OMPTargetArg =
+C.getInputArgs().getLastArg(options::OPT_fopenmp_targets_EQ)) {
+  Diag(clang::diag::err_drv_unsupported_opt_for_language_mode)
+  << OMPTargetArg->getSpelling() << "HIP";
+  return;
+}
 const ToolChain *HostTC = C.getSingleOffloadToolChain();
 const llvm::Triple  = HostTC->getTriple();
 auto OFK = Action::OFK_HIP;
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -16,6 +16,8 @@
   "unsupported option '%0'; did you mean '%1'?">;
 def err_drv_unsupported_opt_for_target : Error<
   "unsupported option '%0' for target '%1'">;
+def err_drv_unsupported_opt_for_language_mode : Error<
+  "unsupported option '%0' for language mode '%1'">;
 def err_drv_unsupported_option_argument : Error<
   "unsupported argument '%1' to option '%0'">;
 def err_drv_unknown_stdin_type : Error<


Index: clang/test/Driver/hip-options.hip
===
--- clang/test/Driver/hip-options.hip
+++ clang/test/Driver/hip-options.hip
@@ -87,3 +87,15 @@
 // THINLTO-NOT: clang{{.*}}" "-triple" "amdgcn-amd-amdhsa" {{.*}} "-fwhole-program-vtables"
 // THINLTO: clang{{.*}}" "-triple" "x86_64-unknown-linux-gnu" {{.*}} "-flto=thin" {{.*}} "-fwhole-program-vtables"
 // THINLTO-NOT: clang{{.*}}" "-triple" "amdgcn-amd-amdhsa" {{.*}} "-fwhole-program-vtables"
+
+// Check -fopenmp is allowed with HIP but -fopenmp-targets= is not allowed.
+
+// RUN: %clang -### -target x86_64-unknown-linux-gnu -nogpuinc -nogpulib \
+// RUN:   --offload-arch=gfx906 -fopenmp %s 2>&1 | FileCheck -check-prefix=OMP %s
+// OMP-NOT: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} "-fopenmp"
+// OMP: clang{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}} "-fopenmp"
+
+// RUN: not %clang -target x86_64-unknown-linux-gnu -nogpuinc -nogpulib \
+// RUN:   --offload-arch=gfx906 -fopenmp -fopenmp-targets=amdgcn %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=OMPTGT %s
+// OMPTGT: unsupported option '-fopenmp-targets=' for language mode 'HIP'
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -680,6 +680,12 @@
 }
 C.addOffloadDeviceToolChain(CudaTC.get(), OFK);
   } else if (IsHIP) {
+if (auto *OMPTargetArg =
+C.getInputArgs().getLastArg(options::OPT_fopenmp_targets_EQ)) {
+  Diag(clang::diag::err_drv_unsupported_opt_for_language_mode)
+  << OMPTargetArg->getSpelling() << "HIP";
+  return;
+}
 const ToolChain *HostTC = C.getSingleOffloadToolChain();
 const llvm::Triple  = HostTC->getTriple();
 auto OFK = Action::OFK_HIP;
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- 

[PATCH] D109718: [HIP] Diagnose -fopenmp-targets for HIP programs

2021-09-14 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 372495.
yaxunl added a comment.

revise diag message


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

https://reviews.llvm.org/D109718

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/hip-options.hip


Index: clang/test/Driver/hip-options.hip
===
--- clang/test/Driver/hip-options.hip
+++ clang/test/Driver/hip-options.hip
@@ -87,3 +87,15 @@
 // THINLTO-NOT: clang{{.*}}" "-triple" "amdgcn-amd-amdhsa" {{.*}} 
"-fwhole-program-vtables"
 // THINLTO: clang{{.*}}" "-triple" "x86_64-unknown-linux-gnu" {{.*}} 
"-flto=thin" {{.*}} "-fwhole-program-vtables"
 // THINLTO-NOT: clang{{.*}}" "-triple" "amdgcn-amd-amdhsa" {{.*}} 
"-fwhole-program-vtables"
+
+// Check -fopenmp is allowed with HIP but -fopenmp-targets= is not allowed.
+
+// RUN: %clang -### -target x86_64-unknown-linux-gnu -nogpuinc -nogpulib \
+// RUN:   --offload-arch=gfx906 -fopenmp %s 2>&1 | FileCheck -check-prefix=OMP 
%s
+// OMP-NOT: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} "-fopenmp"
+// OMP: clang{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}} "-fopenmp"
+
+// RUN: not %clang -target x86_64-unknown-linux-gnu -nogpuinc -nogpulib \
+// RUN:   --offload-arch=gfx906 -fopenmp -fopenmp-targets=amdgcn %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=OMPTGT %s
+// OMPTGT: unsupported option '-fopenmp-targets=' for language mode 'HIP'
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -699,6 +699,12 @@
 }
 C.addOffloadDeviceToolChain(CudaTC.get(), OFK);
   } else if (IsHIP) {
+if (auto *OMPTargetArg =
+C.getInputArgs().getLastArg(options::OPT_fopenmp_targets_EQ)) {
+  Diag(clang::diag::err_drv_unsupported_opt_for_language_mode)
+  << OMPTargetArg->getSpelling() << "HIP";
+  return;
+}
 const ToolChain *HostTC = C.getSingleOffloadToolChain();
 const llvm::Triple  = HostTC->getTriple();
 auto OFK = Action::OFK_HIP;
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -16,6 +16,8 @@
   "unsupported option '%0'; did you mean '%1'?">;
 def err_drv_unsupported_opt_for_target : Error<
   "unsupported option '%0' for target '%1'">;
+def err_drv_unsupported_opt_for_language_mode : Error<
+  "unsupported option '%0' for language mode '%1'">;
 def err_drv_unsupported_option_argument : Error<
   "unsupported argument '%1' to option '%0'">;
 def err_drv_unknown_stdin_type : Error<


Index: clang/test/Driver/hip-options.hip
===
--- clang/test/Driver/hip-options.hip
+++ clang/test/Driver/hip-options.hip
@@ -87,3 +87,15 @@
 // THINLTO-NOT: clang{{.*}}" "-triple" "amdgcn-amd-amdhsa" {{.*}} "-fwhole-program-vtables"
 // THINLTO: clang{{.*}}" "-triple" "x86_64-unknown-linux-gnu" {{.*}} "-flto=thin" {{.*}} "-fwhole-program-vtables"
 // THINLTO-NOT: clang{{.*}}" "-triple" "amdgcn-amd-amdhsa" {{.*}} "-fwhole-program-vtables"
+
+// Check -fopenmp is allowed with HIP but -fopenmp-targets= is not allowed.
+
+// RUN: %clang -### -target x86_64-unknown-linux-gnu -nogpuinc -nogpulib \
+// RUN:   --offload-arch=gfx906 -fopenmp %s 2>&1 | FileCheck -check-prefix=OMP %s
+// OMP-NOT: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} "-fopenmp"
+// OMP: clang{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}} "-fopenmp"
+
+// RUN: not %clang -target x86_64-unknown-linux-gnu -nogpuinc -nogpulib \
+// RUN:   --offload-arch=gfx906 -fopenmp -fopenmp-targets=amdgcn %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=OMPTGT %s
+// OMPTGT: unsupported option '-fopenmp-targets=' for language mode 'HIP'
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -699,6 +699,12 @@
 }
 C.addOffloadDeviceToolChain(CudaTC.get(), OFK);
   } else if (IsHIP) {
+if (auto *OMPTargetArg =
+C.getInputArgs().getLastArg(options::OPT_fopenmp_targets_EQ)) {
+  Diag(clang::diag::err_drv_unsupported_opt_for_language_mode)
+  << OMPTargetArg->getSpelling() << "HIP";
+  return;
+}
 const ToolChain *HostTC = C.getSingleOffloadToolChain();
 const llvm::Triple  = HostTC->getTriple();
 auto OFK = Action::OFK_HIP;
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -16,6 +16,8 @@
   "unsupported option '%0'; did you mean '%1'?">;
 def err_drv_unsupported_opt_for_target : Error<
  

[PATCH] D109718: [HIP] Diagnose -fopenmp-targets for HIP programs

2021-09-14 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:704
+C.getInputArgs().getLastArg(options::OPT_fopenmp_targets_EQ)) {
+  Diag(clang::diag::err_drv_unsupported_opt) << 
OMPTargetArg->getSpelling();
+  return;

yaxunl wrote:
> tra wrote:
> > I think "unsupported" here is a bit misleading. We need something along the 
> > lines of "option X can't be used with option Y".
> > 
> > I think `err_drv_argument_not_allowed_with` might be a better choice.
> That is for two conflicting options. However, HIP may be specified by input 
> file extension instead of -x option. Another issue is that this is 
> unsupported option for HIP, not option argument.
> 
> How about
> 
> unsupported option '%1' for HIP language mode
SGTM.


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

https://reviews.llvm.org/D109718

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


[PATCH] D109718: [HIP] Diagnose -fopenmp-targets for HIP programs

2021-09-14 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:704
+C.getInputArgs().getLastArg(options::OPT_fopenmp_targets_EQ)) {
+  Diag(clang::diag::err_drv_unsupported_opt) << 
OMPTargetArg->getSpelling();
+  return;

tra wrote:
> I think "unsupported" here is a bit misleading. We need something along the 
> lines of "option X can't be used with option Y".
> 
> I think `err_drv_argument_not_allowed_with` might be a better choice.
That is for two conflicting options. However, HIP may be specified by input 
file extension instead of -x option. Another issue is that this is unsupported 
option for HIP, not option argument.

How about

unsupported option '%1' for HIP language mode


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

https://reviews.llvm.org/D109718

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


[PATCH] D109718: [HIP] Diagnose -fopenmp-targets for HIP programs

2021-09-13 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:704
+C.getInputArgs().getLastArg(options::OPT_fopenmp_targets_EQ)) {
+  Diag(clang::diag::err_drv_unsupported_opt) << 
OMPTargetArg->getSpelling();
+  return;

I think "unsupported" here is a bit misleading. We need something along the 
lines of "option X can't be used with option Y".

I think `err_drv_argument_not_allowed_with` might be a better choice.


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

https://reviews.llvm.org/D109718

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


[PATCH] D109718: [HIP] Diagnose -fopenmp-targets for HIP programs

2021-09-13 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl created this revision.
yaxunl added a reviewer: tra.
Herald added a subscriber: guansong.
yaxunl requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.

Diagnose -fopenmp-targets for HIP programs since dual HIP and OpenMP offloading 
in the same compilation
is currently not supported by HIP toolchain.


https://reviews.llvm.org/D109718

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/hip-options.hip


Index: clang/test/Driver/hip-options.hip
===
--- clang/test/Driver/hip-options.hip
+++ clang/test/Driver/hip-options.hip
@@ -87,3 +87,15 @@
 // THINLTO-NOT: clang{{.*}}" "-triple" "amdgcn-amd-amdhsa" {{.*}} 
"-fwhole-program-vtables"
 // THINLTO: clang{{.*}}" "-triple" "x86_64-unknown-linux-gnu" {{.*}} 
"-flto=thin" {{.*}} "-fwhole-program-vtables"
 // THINLTO-NOT: clang{{.*}}" "-triple" "amdgcn-amd-amdhsa" {{.*}} 
"-fwhole-program-vtables"
+
+// Check -fopenmp is allowed with HIP but -fopenmp-targets= is not allowed.
+
+// RUN: %clang -### -target x86_64-unknown-linux-gnu -nogpuinc -nogpulib \
+// RUN:   --offload-arch=gfx906 -fopenmp %s 2>&1 | FileCheck -check-prefix=OMP 
%s
+// OMP-NOT: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} "-fopenmp"
+// OMP: clang{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}} "-fopenmp"
+
+// RUN: not %clang -target x86_64-unknown-linux-gnu -nogpuinc -nogpulib \
+// RUN:   --offload-arch=gfx906 -fopenmp -fopenmp-targets=amdgcn %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=OMPTGT %s
+// OMPTGT: unsupported option '-fopenmp-targets='
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -699,6 +699,11 @@
 }
 C.addOffloadDeviceToolChain(CudaTC.get(), OFK);
   } else if (IsHIP) {
+if (auto *OMPTargetArg =
+C.getInputArgs().getLastArg(options::OPT_fopenmp_targets_EQ)) {
+  Diag(clang::diag::err_drv_unsupported_opt) << 
OMPTargetArg->getSpelling();
+  return;
+}
 const ToolChain *HostTC = C.getSingleOffloadToolChain();
 const llvm::Triple  = HostTC->getTriple();
 auto OFK = Action::OFK_HIP;


Index: clang/test/Driver/hip-options.hip
===
--- clang/test/Driver/hip-options.hip
+++ clang/test/Driver/hip-options.hip
@@ -87,3 +87,15 @@
 // THINLTO-NOT: clang{{.*}}" "-triple" "amdgcn-amd-amdhsa" {{.*}} "-fwhole-program-vtables"
 // THINLTO: clang{{.*}}" "-triple" "x86_64-unknown-linux-gnu" {{.*}} "-flto=thin" {{.*}} "-fwhole-program-vtables"
 // THINLTO-NOT: clang{{.*}}" "-triple" "amdgcn-amd-amdhsa" {{.*}} "-fwhole-program-vtables"
+
+// Check -fopenmp is allowed with HIP but -fopenmp-targets= is not allowed.
+
+// RUN: %clang -### -target x86_64-unknown-linux-gnu -nogpuinc -nogpulib \
+// RUN:   --offload-arch=gfx906 -fopenmp %s 2>&1 | FileCheck -check-prefix=OMP %s
+// OMP-NOT: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} "-fopenmp"
+// OMP: clang{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}} "-fopenmp"
+
+// RUN: not %clang -target x86_64-unknown-linux-gnu -nogpuinc -nogpulib \
+// RUN:   --offload-arch=gfx906 -fopenmp -fopenmp-targets=amdgcn %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=OMPTGT %s
+// OMPTGT: unsupported option '-fopenmp-targets='
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -699,6 +699,11 @@
 }
 C.addOffloadDeviceToolChain(CudaTC.get(), OFK);
   } else if (IsHIP) {
+if (auto *OMPTargetArg =
+C.getInputArgs().getLastArg(options::OPT_fopenmp_targets_EQ)) {
+  Diag(clang::diag::err_drv_unsupported_opt) << OMPTargetArg->getSpelling();
+  return;
+}
 const ToolChain *HostTC = C.getSingleOffloadToolChain();
 const llvm::Triple  = HostTC->getTriple();
 auto OFK = Action::OFK_HIP;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits