[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-12-21 Thread Henry Linjamäki via Phabricator via cfe-commits
linjamaki added a comment.

Thanks, @yaxunl.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

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


[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-12-20 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 rGa6786cdd5757: [HIPSPV][3/4] Enable SPIR-V emission for HIP 
(authored by yaxunl).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

Files:
  clang/include/clang/Basic/Cuda.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Cuda.cpp
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/Inputs/hipspv-dev-lib/a/a.bc
  clang/test/Driver/Inputs/hipspv-dev-lib/b/b.bc
  clang/test/Driver/Inputs/hipspv-dev-lib/hipspv-spirv64.bc
  clang/test/Driver/Inputs/hipspv/bin/.hipVersion
  clang/test/Driver/Inputs/hipspv/lib/hip-device-lib/hipspv-spirv64.bc
  clang/test/Driver/Inputs/hipspv/lib/libLLVMHipSpvPasses.so
  clang/test/Driver/Inputs/pass-plugin.so
  clang/test/Driver/hipspv-device-libs.hip
  clang/test/Driver/hipspv-pass-plugin.hip
  clang/test/Driver/hipspv-toolchain-rdc.hip
  clang/test/Driver/hipspv-toolchain.hip
  clang/test/Driver/invalid-offload-options.cpp

Index: clang/test/Driver/invalid-offload-options.cpp
===
--- /dev/null
+++ clang/test/Driver/invalid-offload-options.cpp
@@ -0,0 +1,31 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload= \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=INVALID-TARGET %s
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=foo \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=INVALID-TARGET %s
+
+// INVALID-TARGET: error: Invalid or unsupported offload target: '{{.*}}'
+
+// In the future we should be able to specify multiple targets for HIP
+// compilation but currently it is not supported.
+//
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=foo,bar \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=TOO-MANY-TARGETS %s
+// RUN: %clang -### -x hip -target x86_64-linux-gnu \
+// RUN:   --offload=foo --offload=bar \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=TOO-MANY-TARGETS %s
+
+// TOO-MANY-TARGETS: error: Only one offload target is supported in HIP.
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu -nogpuinc -nogpulib \
+// RUN:   --offload=amdgcn-amd-amdhsa --offload-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck --check-prefix=OFFLOAD-ARCH-MIX %s
+
+// OFFLOAD-ARCH-MIX: error: option '--offload-arch' cannot be specified with '--offload'
Index: clang/test/Driver/hipspv-toolchain.hip
===
--- /dev/null
+++ clang/test/Driver/hipspv-toolchain.hip
@@ -0,0 +1,37 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -target x86_64-linux-gnu --offload=spirv64 \
+// RUN:   --hip-path=%S/Inputs/hipspv -nohipwrapperinc %s \
+// RUN: 2>&1 | FileCheck %s
+
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "spirv64"
+// CHECK-SAME: "-aux-triple" "{{.*}}" "-emit-llvm-bc"
+// CHECK-SAME: "-fcuda-is-device"
+// CHECK-SAME: "-fcuda-allow-variadic-functions"
+// CHECK-SAME: "-mlink-builtin-bitcode" {{".*/hipspv/lib/hip-device-lib/hipspv-spirv64.bc"}}
+// CHECK-SAME: "-isystem" {{".*/hipspv/include"}}
+// CHECK-SAME: "-fhip-new-launch-api"
+// CHECK-SAME: "-o" [[DEV_BC:".*bc"]]
+// CHECK-SAME: "-x" "hip"
+
+// CHECK: {{".*llvm-link"}} [[DEV_BC]] "-o" [[LINK_BC:".*bc"]]
+
+// CHECK: {{".*opt"}} [[LINK_BC]] "-load-pass-plugin"
+// CHECK-SAME: {{".*/hipspv/lib/libLLVMHipSpvPasses.so"}}
+// CHECK-SAME: "-passes=hip-post-link-passes" "-o" [[LOWER_BC:".*bc"]]
+
+// CHECK: {{".*llvm-spirv"}} "--spirv-max-version=1.1" "--spirv-ext=+all"
+// CHECK-SAME: [[LOWER_BC]] "-o" "[[SPIRV_OUT:.*out]]"
+
+// CHECK: {{".*clang-offload-bundler"}} "-type=o" "-bundle-align=4096"
+// CHECK-SAME: "-targets=host-x86_64-unknown-linux,hip-spirv64generic"
+// CHECK-SAME: "-inputs={{.*}},[[SPIRV_OUT]]" "-outputs=[[BUNDLE:.*hipfb]]"
+
+// CHECK: [[CLANG]] "-cc1" "-triple" {{".*"}} "-aux-triple" "spirv64"
+// CHECK-SAME: "-emit-obj"
+// CHECK-SAME: "-fcuda-include-gpubinary" "[[BUNDLE]]"
+// CHECK-SAME: "-o" [[OBJ_HOST:".*o"]] "-x" "hip"
+
+// CHECK: {{".*ld.*"}} {{.*}}[[OBJ_HOST]]
Index: clang/test/Driver/hipspv-toolchain-rdc.hip
===
--- /dev/null
+++ clang/test/Driver/hipspv-toolchain-rdc.hip
@@ -0,0 +1,63 @@
+// REQUIRES: clang-driver
+// REQUIRES: 

[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-12-17 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment.

In D110622#3199233 , @linjamaki wrote:

> Assuming that this patch is ready to land. @tra or @yaxunl, could you please 
> commit this patch to the LLVM for us? Thanks.

I can help commit this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

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


[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-12-16 Thread Henry Linjamäki via Phabricator via cfe-commits
linjamaki added a comment.

Assuming that this patch is ready to land. @tra or @yaxunl, could you please 
commit this patch to the LLVM for us? Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

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


[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-12-16 Thread Henry Linjamäki via Phabricator via cfe-commits
linjamaki updated this revision to Diff 394789.
linjamaki added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

Files:
  clang/include/clang/Basic/Cuda.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Cuda.cpp
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/Inputs/hipspv-dev-lib/a/a.bc
  clang/test/Driver/Inputs/hipspv-dev-lib/b/b.bc
  clang/test/Driver/Inputs/hipspv-dev-lib/hipspv-spirv64.bc
  clang/test/Driver/Inputs/hipspv/bin/.hipVersion
  clang/test/Driver/Inputs/hipspv/lib/hip-device-lib/hipspv-spirv64.bc
  clang/test/Driver/Inputs/hipspv/lib/libLLVMHipSpvPasses.so
  clang/test/Driver/Inputs/pass-plugin.so
  clang/test/Driver/hipspv-device-libs.hip
  clang/test/Driver/hipspv-pass-plugin.hip
  clang/test/Driver/hipspv-toolchain-rdc.hip
  clang/test/Driver/hipspv-toolchain.hip
  clang/test/Driver/invalid-offload-options.cpp

Index: clang/test/Driver/invalid-offload-options.cpp
===
--- /dev/null
+++ clang/test/Driver/invalid-offload-options.cpp
@@ -0,0 +1,31 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload= \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=INVALID-TARGET %s
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=foo \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=INVALID-TARGET %s
+
+// INVALID-TARGET: error: Invalid or unsupported offload target: '{{.*}}'
+
+// In the future we should be able to specify multiple targets for HIP
+// compilation but currently it is not supported.
+//
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=foo,bar \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=TOO-MANY-TARGETS %s
+// RUN: %clang -### -x hip -target x86_64-linux-gnu \
+// RUN:   --offload=foo --offload=bar \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=TOO-MANY-TARGETS %s
+
+// TOO-MANY-TARGETS: error: Only one offload target is supported in HIP.
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu -nogpuinc -nogpulib \
+// RUN:   --offload=amdgcn-amd-amdhsa --offload-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck --check-prefix=OFFLOAD-ARCH-MIX %s
+
+// OFFLOAD-ARCH-MIX: error: option '--offload-arch' cannot be specified with '--offload'
Index: clang/test/Driver/hipspv-toolchain.hip
===
--- /dev/null
+++ clang/test/Driver/hipspv-toolchain.hip
@@ -0,0 +1,37 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -target x86_64-linux-gnu --offload=spirv64 \
+// RUN:   --hip-path=%S/Inputs/hipspv -nohipwrapperinc %s \
+// RUN: 2>&1 | FileCheck %s
+
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "spirv64"
+// CHECK-SAME: "-aux-triple" "{{.*}}" "-emit-llvm-bc"
+// CHECK-SAME: "-fcuda-is-device"
+// CHECK-SAME: "-fcuda-allow-variadic-functions"
+// CHECK-SAME: "-mlink-builtin-bitcode" {{".*/hipspv/lib/hip-device-lib/hipspv-spirv64.bc"}}
+// CHECK-SAME: "-isystem" {{".*/hipspv/include"}}
+// CHECK-SAME: "-fhip-new-launch-api"
+// CHECK-SAME: "-o" [[DEV_BC:".*bc"]]
+// CHECK-SAME: "-x" "hip"
+
+// CHECK: {{".*llvm-link"}} [[DEV_BC]] "-o" [[LINK_BC:".*bc"]]
+
+// CHECK: {{".*opt"}} [[LINK_BC]] "-load-pass-plugin"
+// CHECK-SAME: {{".*/hipspv/lib/libLLVMHipSpvPasses.so"}}
+// CHECK-SAME: "-passes=hip-post-link-passes" "-o" [[LOWER_BC:".*bc"]]
+
+// CHECK: {{".*llvm-spirv"}} "--spirv-max-version=1.1" "--spirv-ext=+all"
+// CHECK-SAME: [[LOWER_BC]] "-o" "[[SPIRV_OUT:.*out]]"
+
+// CHECK: {{".*clang-offload-bundler"}} "-type=o" "-bundle-align=4096"
+// CHECK-SAME: "-targets=host-x86_64-unknown-linux,hip-spirv64generic"
+// CHECK-SAME: "-inputs={{.*}},[[SPIRV_OUT]]" "-outputs=[[BUNDLE:.*hipfb]]"
+
+// CHECK: [[CLANG]] "-cc1" "-triple" {{".*"}} "-aux-triple" "spirv64"
+// CHECK-SAME: "-emit-obj"
+// CHECK-SAME: "-fcuda-include-gpubinary" "[[BUNDLE]]"
+// CHECK-SAME: "-o" [[OBJ_HOST:".*o"]] "-x" "hip"
+
+// CHECK: {{".*ld.*"}} {{.*}}[[OBJ_HOST]]
Index: clang/test/Driver/hipspv-toolchain-rdc.hip
===
--- /dev/null
+++ clang/test/Driver/hipspv-toolchain-rdc.hip
@@ -0,0 +1,63 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=spirv64 \
+// RUN:   -fgpu-rdc 

[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-12-08 Thread Henry Linjamäki via Phabricator via cfe-commits
linjamaki added a comment.

Thanks for the feedback. The `--offload` is meant to support multiple targets 
but right now it is restricted to one at most. The limitation comes from the 
HIPActionBuilder and CudaActionBuilderBase which currently expects a single 
target triple and toolchain for all offload devices. To relax the `--offload` 
target count cap we would need to adjust the action builders to support 
multiple target triples and create a separate toolchain for each (unique) 
target triple.

Details for  the `--offload` option for specifying multiple targets are left 
open in this patch. What this patch needs is at least an ability to specify a 
single target (e.g. `--offload=spirv64`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

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


[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-12-08 Thread Henry Linjamäki via Phabricator via cfe-commits
linjamaki updated this revision to Diff 392675.
linjamaki added a comment.

- Add comments to clarify the limitation of the `--offload` option to one 
target.

- Add test for multiple `--offload` option instances.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

Files:
  clang/include/clang/Basic/Cuda.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Cuda.cpp
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/Inputs/hipspv-dev-lib/a/a.bc
  clang/test/Driver/Inputs/hipspv-dev-lib/b/b.bc
  clang/test/Driver/Inputs/hipspv-dev-lib/hipspv-spirv64.bc
  clang/test/Driver/Inputs/hipspv/bin/.hipVersion
  clang/test/Driver/Inputs/hipspv/lib/hip-device-lib/hipspv-spirv64.bc
  clang/test/Driver/Inputs/hipspv/lib/libLLVMHipSpvPasses.so
  clang/test/Driver/Inputs/pass-plugin.so
  clang/test/Driver/hipspv-device-libs.hip
  clang/test/Driver/hipspv-pass-plugin.hip
  clang/test/Driver/hipspv-toolchain-rdc.hip
  clang/test/Driver/hipspv-toolchain.hip
  clang/test/Driver/invalid-offload-options.cpp

Index: clang/test/Driver/invalid-offload-options.cpp
===
--- /dev/null
+++ clang/test/Driver/invalid-offload-options.cpp
@@ -0,0 +1,31 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload= \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=INVALID-TARGET %s
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=foo \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=INVALID-TARGET %s
+
+// INVALID-TARGET: error: Invalid or unsupported offload target: '{{.*}}'
+
+// In the future we should be able to specify multiple targets for HIP
+// compilation but currently it is not supported.
+//
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=foo,bar \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=TOO-MANY-TARGETS %s
+// RUN: %clang -### -x hip -target x86_64-linux-gnu \
+// RUN:   --offload=foo --offload=bar \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=TOO-MANY-TARGETS %s
+
+// TOO-MANY-TARGETS: error: Only one offload target is supported in HIP.
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu -nogpuinc -nogpulib \
+// RUN:   --offload=amdgcn-amd-amdhsa --offload-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck --check-prefix=OFFLOAD-ARCH-MIX %s
+
+// OFFLOAD-ARCH-MIX: error: option '--offload-arch' cannot be specified with '--offload'
Index: clang/test/Driver/hipspv-toolchain.hip
===
--- /dev/null
+++ clang/test/Driver/hipspv-toolchain.hip
@@ -0,0 +1,37 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -target x86_64-linux-gnu --offload=spirv64 \
+// RUN:   --hip-path=%S/Inputs/hipspv -nohipwrapperinc %s \
+// RUN: 2>&1 | FileCheck %s
+
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "spirv64"
+// CHECK-SAME: "-aux-triple" "{{.*}}" "-emit-llvm-bc"
+// CHECK-SAME: "-fcuda-is-device"
+// CHECK-SAME: "-fcuda-allow-variadic-functions"
+// CHECK-SAME: "-mlink-builtin-bitcode" {{".*/hipspv/lib/hip-device-lib/hipspv-spirv64.bc"}}
+// CHECK-SAME: "-isystem" {{".*/hipspv/include"}}
+// CHECK-SAME: "-fhip-new-launch-api"
+// CHECK-SAME: "-o" [[DEV_BC:".*bc"]]
+// CHECK-SAME: "-x" "hip"
+
+// CHECK: {{".*llvm-link"}} [[DEV_BC]] "-o" [[LINK_BC:".*bc"]]
+
+// CHECK: {{".*opt"}} [[LINK_BC]] "-load-pass-plugin"
+// CHECK-SAME: {{".*/hipspv/lib/libLLVMHipSpvPasses.so"}}
+// CHECK-SAME: "-passes=hip-post-link-passes" "-o" [[LOWER_BC:".*bc"]]
+
+// CHECK: {{".*llvm-spirv"}} "--spirv-max-version=1.1" "--spirv-ext=+all"
+// CHECK-SAME: [[LOWER_BC]] "-o" "[[SPIRV_OUT:.*out]]"
+
+// CHECK: {{".*clang-offload-bundler"}} "-type=o" "-bundle-align=4096"
+// CHECK-SAME: "-targets=host-x86_64-unknown-linux,hip-spirv64generic"
+// CHECK-SAME: "-inputs={{.*}},[[SPIRV_OUT]]" "-outputs=[[BUNDLE:.*hipfb]]"
+
+// CHECK: [[CLANG]] "-cc1" "-triple" {{".*"}} "-aux-triple" "spirv64"
+// CHECK-SAME: "-emit-obj"
+// CHECK-SAME: "-fcuda-include-gpubinary" "[[BUNDLE]]"
+// CHECK-SAME: "-o" [[OBJ_HOST:".*o"]] "-x" "hip"
+
+// CHECK: {{".*ld.*"}} {{.*}}[[OBJ_HOST]]
Index: clang/test/Driver/hipspv-toolchain-rdc.hip
===
--- /dev/null
+++ clang/test/Driver/hipspv-toolchain-rdc.hip
@@ -0,0 +1,63 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target

[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-12-07 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:105-109
+auto HIPOffloadTargets = Args.getAllArgValues(options::OPT_offload_EQ);
+switch (HIPOffloadTargets.size()) {
+default:
+  D.Diag(diag::err_drv_only_one_offload_target_supported_in) << "HIP";
+  return llvm::None;

tra wrote:
> ^^^ 
> 
> This will cause issues in practice, as we're only allowed to specify 
> --offload once. 
> 
> I.e. we're neither allowed to override it (this goes contrary to how clang 
> options are handled conventionally), nor can we extend or modify the list of 
> offload variants as we can with --offload-arch.
> 
> This code initially used `getLastArg`, but that does not work for an option 
> that controls a list of values. Perhaps we should just make `--offload=` a 
> scalar value so it, at least, behaves consistently with other clang options.
You are right. I overlooked this part.

If multiple `--offload=` options are specified, they are supposed to be 
unioned. Since currently `--offload=` only accepts `amdgcn-amd-amdhsa` and 
`spirv64`, and they are mutually exclusive. I think it is OK.

In the future, `--offload=` will accept GPU archs, then this part needs to 
allow multiple `--offload=` options and more sophisticated compatibility check 
between different options.

I agree letting `--offload=` accept scalar value seems a good choice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

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


[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-12-07 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

In D110622#3177490 , @yaxunl wrote:

> I don't think `--offload=` is restricted to be specified only once. The test 
> checks `--offload-arch=` and `--offload=` are mutually exclusive.

It effectively is. See my inline comment.

  // RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=foo,bar \
  // RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
  // RUN: 2>&1 | FileCheck --check-prefix=TOO-MANY-TARGETS %s
  
  // TOO-MANY-TARGETS: error: Only one offload target is supported in HIP.




Comment at: clang/lib/Driver/Driver.cpp:105-109
+auto HIPOffloadTargets = Args.getAllArgValues(options::OPT_offload_EQ);
+switch (HIPOffloadTargets.size()) {
+default:
+  D.Diag(diag::err_drv_only_one_offload_target_supported_in) << "HIP";
+  return llvm::None;

^^^ 

This will cause issues in practice, as we're only allowed to specify --offload 
once. 

I.e. we're neither allowed to override it (this goes contrary to how clang 
options are handled conventionally), nor can we extend or modify the list of 
offload variants as we can with --offload-arch.

This code initially used `getLastArg`, but that does not work for an option 
that controls a list of values. Perhaps we should just make `--offload=` a 
scalar value so it, at least, behaves consistently with other clang options.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

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


[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-12-07 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment.

In D110622#3177111 , @tra wrote:

> In D110622#3176804 , @yaxunl wrote:
>
 So, the question is -- what's the right way to specify something like this 
 in a consistent manner? 
 `--offload` option proposed here does not seem to be a good fit. It was 
 intended as a more flexible way to create a single `-cc1` sub-compilation 
 and we're doing quite a bit more here.
>>>
>>> Does `--offload-arch=spirv*` fit better here? If I understand the goal of 
>>> this patch correctly, it tries to provide controls for changing offload 
>>> target for HIP application from default (AMDGCN) to SPIR-V.
>>
>> `--offload-arch=` only accepts GPU arch which is translated to processor 
>> option (-mcpu= or -march=) in clang -cc1. spirv is a target triple which is 
>> not suitable for `--offload-arch=`.
>>
>> `--offload=` is supposed to cover both target triple and processor with some 
>> flexibility. If only target triple is specified, it assumes default 
>> processor. If only processor is specified, it deduces target triple. It also 
>> allows both triple and processor. In this case, `--offload=spirv` translates 
>> to -triple spirv -mcpu=generic.
>
> So, one would expect that we should be able to specify it more than once to 
> target multiple GPU variants, if we were to use it as a more flexible 
> `--offload-arch`.
> If I read the tests correctly, using `--offload=` limits us to exactly one 
> variant now. Perhaps it should eventually be relaxed to only enforce single 
> `--offload=` variant if we're offloading to SPIR-V. It's not a showstopper 
> for this patch. We can relax it later.

I don't think `--offload=` is restricted to be specified only once. The test 
checks `--offload-arch=` and `--offload=` are mutually exclusive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

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


[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-12-07 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

In D110622#3176804 , @yaxunl wrote:

>>> So, the question is -- what's the right way to specify something like this 
>>> in a consistent manner? 
>>> `--offload` option proposed here does not seem to be a good fit. It was 
>>> intended as a more flexible way to create a single `-cc1` sub-compilation 
>>> and we're doing quite a bit more here.
>>
>> Does `--offload-arch=spirv*` fit better here? If I understand the goal of 
>> this patch correctly, it tries to provide controls for changing offload 
>> target for HIP application from default (AMDGCN) to SPIR-V.
>
> `--offload-arch=` only accepts GPU arch which is translated to processor 
> option (-mcpu= or -march=) in clang -cc1. spirv is a target triple which is 
> not suitable for `--offload-arch=`.
>
> `--offload=` is supposed to cover both target triple and processor with some 
> flexibility. If only target triple is specified, it assumes default 
> processor. If only processor is specified, it deduces target triple. It also 
> allows both triple and processor. In this case, `--offload=spirv` translates 
> to -triple spirv -mcpu=generic.

So, one would expect that we should be able to specify it more than once to 
target multiple GPU variants, if we were to use it as a more flexible 
`--offload-arch`.
If I read the tests correctly, using `--offload=` limits us to exactly one 
variant now. Perhaps it should eventually be relaxed to only enforce single 
`--offload=` variant if we're offloading to SPIR-V. It's not a showstopper for 
this patch. We can relax it later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

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


[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-12-07 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment.

>> So, the question is -- what's the right way to specify something like this 
>> in a consistent manner? 
>> `--offload` option proposed here does not seem to be a good fit. It was 
>> intended as a more flexible way to create a single `-cc1` sub-compilation 
>> and we're doing quite a bit more here.
>
> Does `--offload-arch=spirv*` fit better here? If I understand the goal of 
> this patch correctly, it tries to provide controls for changing offload 
> target for HIP application from default (AMDGCN) to SPIR-V.

`--offload-arch=` only accepts GPU arch which is translated to processor option 
(-mcpu= or -march=) in clang -cc1. spirv is a target triple which is not 
suitable for `--offload-arch=`.

`--offload=` is supposed to cover both target triple and processor with some 
flexibility. If only target triple is specified, it assumes default processor. 
If only processor is specified, it deduces target triple. It also allows both 
triple and processor. In this case, `--offload=spirv` translates to -triple 
spirv -mcpu=generic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

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


[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-12-07 Thread Alexey Bader via Phabricator via cfe-commits
bader added a comment.

In D110622#3174113 , @tra wrote:

> The patch looks OK for the time being. That said, I do have concerns that we 
> may be organically growing something that will be troublesome to deal with 
> long-term.
>
> TBH, I still can't quite make sense of where/how SPIR-V fits in the 
> offloading nomenclature.
>
> Right now we have multiple levels of offloading-related control points.
>
> - offload targets, specified by --offload-arch. Determines the ISA of the GPU 
> binary we produce.
> - offload mechanism: OpenMP, CUDA runtime, HSA. Determines how we 
> compile/pack/launch the GPU binaries.
> - front-end: CUDA/HIP/ C/C++ w/ OpenMP.
> - Driver: Determines compilation pipeline to glue everything together,
>
> SPIR-V in these patches appears to be wearing multiple hats. 
> It changes compilation pipeline, it changes offload mechanism and it changes 
> offload targets.

From my POV, SPIR-V is "the ISA of GPU binary we produce" and we might need 
some changes at offloading-related control points:

- offload mechanism: none of listed "offload mechanisms" (i.e. OpenMP, CUDA 
runtime, HSA) is able to handle SPIR-V natively. On the other hand, I'm not 
sure if there is a need in additional changes for all "offloading mechanisms". 
E.g. Intel's compiler implements OpenMP-offload to SPIR-V target using OpenMP 
runtime plug-in lowering OpenMP runtime calls to OpenCL/Level Zero. OpenCL and 
Level Zero  runtimes are 
able to compile and launch SPIR-V binaries.
- front-end: if we compare SPIR to other ISAs, they change compilation pipeline 
as well (e.g. add new built-ins to expose ISA, add CodeGen library changes to 
emit ISA specific metadata, etc.). AMDGPU ISA 
 or NVIDIA 
 GPU 
 ISA changed front-end/compilation 
pipeline as well. Do you refer to some other non-ISA specific changes? BTW, 
shameless plug, the patch adding built-in functions and types for SPIR-V ISA is 
under review here - https://reviews.llvm.org/D108034.
- Driver: front-end compiler doesn't support SPIR-V format yet (i.e. SPIR-V 
requires special encoding different from LLVM bitcode) , so Driver hooks up 
LLVM->SPIR-V translator tool to produce SPIR-V binary.

> To further complicate things, it appears to be a derivative of the HIP 
> compilation. I can't tell if it's an implementation detail at the moment, or 
> whether it will become a more generic offload mechanism that would be 
> expected to be used by other front- and back-ends. E.g. can we potentially 
> compile CUDA code to target SPIR-V? Can OpenMP offload to SPIR-V?

Intel's compiler compiles OpenMP offload and SYCL to SPIR-V, so we definitely 
would like to target SPIR-V using other front-ends.

> So, the question is -- what's the right way to specify something like this in 
> a consistent manner? 
> `--offload` option proposed here does not seem to be a good fit. It was 
> intended as a more flexible way to create a single `-cc1` sub-compilation and 
> we're doing quite a bit more here.

Does `--offload-arch=spirv*` fit better here? If I understand the goal of this 
patch correctly, it tries to provide controls for changing offload target for 
HIP application from default (AMDGCN) to SPIR-V.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

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


[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-12-06 Thread Artem Belevich via Phabricator via cfe-commits
tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.

Note to self: don't forget to hit "submit". The comments below have been left 
unsubmitted for two weeks. Sorry about that.

The patch looks OK for the time being. That said, I do have concerns that we 
may be organically growing something that will be troublesome to deal with 
long-term.

TBH, I still can't quite make sense of where/how SPIR-V fits in the offloading 
nomenclature.

Right now we have multiple levels of offloading-related control points.

- offload targets, specified by --offload-arch. Determines the ISA of the GPU 
binary we produce.
- offload mechanism: OpenMP, CUDA runtime, HSA. Determines how we 
compile/pack/launch the GPU binaries.
- front-end: CUDA/HIP/ C/C++ w/ OpenMP.
- Driver: Determines compilation pipeline to glue everything together,

SPIR-V in these patches appears to be wearing multiple hats. 
It changes compilation pipeline, it changes offload mechanism and it changes 
offload targets. To further complicate things, it appears to be a derivative of 
the HIP compilation. I can't tell if it's an implementation detail at the 
moment, or whether it will become a more generic offload mechanism that would 
be expected to be used by other front- and back-ends. E.g. can we potentially 
compile CUDA code to target SPIR-V? Can OpenMP offload to SPIR-V?

So, the question is -- what's the right way to specify something like this in a 
consistent manner? 
`--offload` option proposed here does not seem to be a good fit. It was 
intended as a more flexible way to create a single `-cc1` sub-compilation and 
we're doing quite a bit more here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

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


[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-12-01 Thread Henry Linjamäki via Phabricator via cfe-commits
linjamaki added a comment.

@tra, gentle ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

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


[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-11-23 Thread Henry Linjamäki via Phabricator via cfe-commits
linjamaki updated this revision to Diff 389114.
linjamaki marked 2 inline comments as done.
linjamaki added a comment.

Update a driver test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

Files:
  clang/include/clang/Basic/Cuda.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Cuda.cpp
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/Inputs/hipspv-dev-lib/a/a.bc
  clang/test/Driver/Inputs/hipspv-dev-lib/b/b.bc
  clang/test/Driver/Inputs/hipspv-dev-lib/hipspv-spirv64.bc
  clang/test/Driver/Inputs/hipspv/bin/.hipVersion
  clang/test/Driver/Inputs/hipspv/lib/hip-device-lib/hipspv-spirv64.bc
  clang/test/Driver/Inputs/hipspv/lib/libLLVMHipSpvPasses.so
  clang/test/Driver/Inputs/pass-plugin.so
  clang/test/Driver/hipspv-device-libs.hip
  clang/test/Driver/hipspv-pass-plugin.hip
  clang/test/Driver/hipspv-toolchain-rdc.hip
  clang/test/Driver/hipspv-toolchain.hip
  clang/test/Driver/invalid-offload-options.cpp

Index: clang/test/Driver/invalid-offload-options.cpp
===
--- /dev/null
+++ clang/test/Driver/invalid-offload-options.cpp
@@ -0,0 +1,24 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload= \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=INVALID-TARGET %s
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=foo \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=INVALID-TARGET %s
+
+// INVALID-TARGET: error: Invalid or unsupported offload target: '{{.*}}'
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=foo,bar \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=TOO-MANY-TARGETS %s
+
+// TOO-MANY-TARGETS: error: Only one offload target is supported in HIP.
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu -nogpuinc -nogpulib \
+// RUN:   --offload=amdgcn-amd-amdhsa --offload-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck --check-prefix=OFFLOAD-ARCH-MIX %s
+
+// OFFLOAD-ARCH-MIX: error: option '--offload-arch' cannot be specified with '--offload'
Index: clang/test/Driver/hipspv-toolchain.hip
===
--- /dev/null
+++ clang/test/Driver/hipspv-toolchain.hip
@@ -0,0 +1,37 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -target x86_64-linux-gnu --offload=spirv64 \
+// RUN:   --hip-path=%S/Inputs/hipspv -nohipwrapperinc %s \
+// RUN: 2>&1 | FileCheck %s
+
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "spirv64"
+// CHECK-SAME: "-aux-triple" "{{.*}}" "-emit-llvm-bc"
+// CHECK-SAME: "-fcuda-is-device"
+// CHECK-SAME: "-fcuda-allow-variadic-functions"
+// CHECK-SAME: "-mlink-builtin-bitcode" {{".*/hipspv/lib/hip-device-lib/hipspv-spirv64.bc"}}
+// CHECK-SAME: "-isystem" {{".*/hipspv/include"}}
+// CHECK-SAME: "-fhip-new-launch-api"
+// CHECK-SAME: "-o" [[DEV_BC:".*bc"]]
+// CHECK-SAME: "-x" "hip"
+
+// CHECK: {{".*llvm-link"}} [[DEV_BC]] "-o" [[LINK_BC:".*bc"]]
+
+// CHECK: {{".*opt"}} [[LINK_BC]] "-load-pass-plugin"
+// CHECK-SAME: {{".*/hipspv/lib/libLLVMHipSpvPasses.so"}}
+// CHECK-SAME: "-passes=hip-post-link-passes" "-o" [[LOWER_BC:".*bc"]]
+
+// CHECK: {{".*llvm-spirv"}} "--spirv-max-version=1.1" "--spirv-ext=+all"
+// CHECK-SAME: [[LOWER_BC]] "-o" "[[SPIRV_OUT:.*out]]"
+
+// CHECK: {{".*clang-offload-bundler"}} "-type=o" "-bundle-align=4096"
+// CHECK-SAME: "-targets=host-x86_64-unknown-linux,hip-spirv64generic"
+// CHECK-SAME: "-inputs={{.*}},[[SPIRV_OUT]]" "-outputs=[[BUNDLE:.*hipfb]]"
+
+// CHECK: [[CLANG]] "-cc1" "-triple" {{".*"}} "-aux-triple" "spirv64"
+// CHECK-SAME: "-emit-obj"
+// CHECK-SAME: "-fcuda-include-gpubinary" "[[BUNDLE]]"
+// CHECK-SAME: "-o" [[OBJ_HOST:".*o"]] "-x" "hip"
+
+// CHECK: {{".*ld.*"}} {{.*}}[[OBJ_HOST]]
Index: clang/test/Driver/hipspv-toolchain-rdc.hip
===
--- /dev/null
+++ clang/test/Driver/hipspv-toolchain-rdc.hip
@@ -0,0 +1,63 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=spirv64 \
+// RUN:   -fgpu-rdc --hip-path=%S/Inputs/hipspv -nohipwrapperinc \
+// RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck %s
+
+// Emit objects for host side path
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "x86_64-unknown-linux-gnu"

[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-11-18 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment.

LGTM. I will defer to @tra


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

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


[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-11-18 Thread Henry Linjamäki via Phabricator via cfe-commits
linjamaki marked 2 inline comments as done.
linjamaki added inline comments.



Comment at: clang/include/clang/Basic/Cuda.h:109
+  // Generic processor model is for testing only.
+  return A >= CudaArch::GFX600 && A <= CudaArch::GFX1035;
 }

yaxunl wrote:
> can we use A < CudaArch::Generic instead? to avoid updating this line each 
> time we add a new gfx arch.
Changed as suggested.



Comment at: clang/include/clang/Driver/Options.td:1136
+def offload_EQ : CommaJoined<["--"], "offload=">, Flags<[NoXarchOption]>,
+  HelpText<"Specify comma-separated list of offloading targets.">;
+

yaxunl wrote:
> linjamaki wrote:
> > tra wrote:
> > > `comma-separated list of offloading targets.` is, unfortunately, somewhat 
> > > ambiguous.
> > > Does it mean "how the offload will be done". I.e. HSA, OpenMP, SPIRV, 
> > > CUDA? 
> > > Or does it mean specific hardware we need to generate the code for? 
> > > The code suggests it's a variant of the former, but the option 
> > > description does not. 
> > > 
> > > E.g. `offload_arch_EQ ` also uses the term "offloading target" but with a 
> > > different meaning.
> > > 
> > I’m not sure how to rephrase the option description to be more clear. In 
> > the [1] the `--offload` option is envisioned to be quite 
> > flexible/expressive - it can take in target triples, offload kinds, 
> > processors, aliases for processor sets, etc.
> > 
> > FYI, I have imagined that the `--offload` option would take in explicit 
> > offload kind and target triple combinations as the basis. For example, 
> > something like this:
> > 
> > 
> > ```
> > --offload=hip-amdgcn-amd-amdhsa,openmp-x86_64-pc-linux-gnu
> > ```
> > 
> > And top of that, there would be predefined strings/shortcuts/aliases that 
> > expand to the basic form. For example, 
> > `--offload=sm_70,openmp-host` could expand to something like:
> > 
> > 
> > ```
> > --offload=cuda-nvptx64-nvidia-cuda,openmp-x86_64-pc-linux-gnu 
> > -Xoffload=cuda-nvptx64-nvidia-cuda -march=sm_70 ...
> > 
> > ```
> > Then there is a feature as discussed in [1] that the offload kind can be 
> > dropped if it can be inferred by other means (e.g. from `-x hip` option). 
> > 
> The description better matches the current implementation.
> 
> By this patch, `--offload=` only supports specifying target triple for HIP 
> and assumes default processor. The description should reflect that.
> 
> In the future, as `--offload=` supports more values, the description may be 
> updated.
> 
> Also, `--offload=` is designed to be mutually exclusive with 
> `--offload-arch=`. Probably we should check and diagnose that.
Thanks for the feedback. The option description has been changed to reflect the 
current state. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

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


[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-11-18 Thread Henry Linjamäki via Phabricator via cfe-commits
linjamaki updated this revision to Diff 388146.
linjamaki added a comment.

- Adjust `--offload` description: reflect the current state.
- Adjust enum range check in IsAMDGpuArch().
- Make `--offload` and `--offload-arch` options mutually exclusive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

Files:
  clang/include/clang/Basic/Cuda.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Cuda.cpp
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/Inputs/hipspv-dev-lib/a/a.bc
  clang/test/Driver/Inputs/hipspv-dev-lib/b/b.bc
  clang/test/Driver/Inputs/hipspv-dev-lib/hipspv-spirv64.bc
  clang/test/Driver/Inputs/hipspv/bin/.hipVersion
  clang/test/Driver/Inputs/hipspv/lib/hip-device-lib/hipspv-spirv64.bc
  clang/test/Driver/Inputs/hipspv/lib/libLLVMHipSpvPasses.so
  clang/test/Driver/Inputs/pass-plugin.so
  clang/test/Driver/hipspv-device-libs.hip
  clang/test/Driver/hipspv-pass-plugin.hip
  clang/test/Driver/hipspv-toolchain-rdc.hip
  clang/test/Driver/hipspv-toolchain.hip
  clang/test/Driver/invalid-offload-options.cpp

Index: clang/test/Driver/invalid-offload-options.cpp
===
--- /dev/null
+++ clang/test/Driver/invalid-offload-options.cpp
@@ -0,0 +1,24 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload= \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=INVALID-TARGET %s
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=foo \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=INVALID-TARGET %s
+
+// INVALID-TARGET: error: Invalid or unsupported offload target: '{{.*}}'
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=foo,bar \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=TOO-MANY-TARGETS %s
+
+// TOO-MANY-TARGETS: error: Only one offload target is supported in HIP.
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu -nogpuinc -nogpulib \
+// RUN:   --offload=amdgcn-amd-amdhsa --offload-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck --check-prefix=OFFLOAD-ARCH-MIX %s
+
+// OFFLOAD-ARCH-MIX: error: option '--offload-arch' cannot be specified with '--offload'
Index: clang/test/Driver/hipspv-toolchain.hip
===
--- /dev/null
+++ clang/test/Driver/hipspv-toolchain.hip
@@ -0,0 +1,37 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -target x86_64-linux-gnu --offload=spirv64 \
+// RUN:   --hip-path=%S/Inputs/hipspv -nohipwrapperinc %s \
+// RUN: 2>&1 | FileCheck %s
+
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "spirv64"
+// CHECK-SAME: "-aux-triple" "{{.*}}" "-emit-llvm-bc"
+// CHECK-SAME: "-fcuda-is-device"
+// CHECK-SAME: "-fcuda-allow-variadic-functions"
+// CHECK-SAME: "-mlink-builtin-bitcode" {{".*/hipspv/lib/hip-device-lib/hipspv-spirv64.bc"}}
+// CHECK-SAME: "-isystem" {{".*/hipspv/include"}}
+// CHECK-SAME: "-fhip-new-launch-api"
+// CHECK-SAME: "-o" [[DEV_BC:".*bc"]]
+// CHECK-SAME: "-x" "hip"
+
+// CHECK: {{".*llvm-link"}} [[DEV_BC]] "-o" [[LINK_BC:".*bc"]]
+
+// CHECK: {{".*opt"}} [[LINK_BC]] "-load-pass-plugin"
+// CHECK-SAME: {{".*/hipspv/lib/libLLVMHipSpvPasses.so"}}
+// CHECK-SAME: "-passes=hip-post-link-passes" "-o" [[LOWER_BC:".*bc"]]
+
+// CHECK: {{".*llvm-spirv"}} "--spirv-max-version=1.1" "--spirv-ext=+all"
+// CHECK-SAME: [[LOWER_BC]] "-o" "[[SPIRV_OUT:.*out]]"
+
+// CHECK: {{".*clang-offload-bundler"}} "-type=o" "-bundle-align=4096"
+// CHECK-SAME: "-targets=host-x86_64-unknown-linux,hip-spirv64generic"
+// CHECK-SAME: "-inputs={{.*}},[[SPIRV_OUT]]" "-outputs=[[BUNDLE:.*hipfb]]"
+
+// CHECK: [[CLANG]] "-cc1" "-triple" {{".*"}} "-aux-triple" "spirv64"
+// CHECK-SAME: "-emit-obj"
+// CHECK-SAME: "-fcuda-include-gpubinary" "[[BUNDLE]]"
+// CHECK-SAME: "-o" [[OBJ_HOST:".*o"]] "-x" "hip"
+
+// CHECK: {{".*ld.*"}} {{.*}}[[OBJ_HOST]]
Index: clang/test/Driver/hipspv-toolchain-rdc.hip
===
--- /dev/null
+++ clang/test/Driver/hipspv-toolchain-rdc.hip
@@ -0,0 +1,63 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=spirv64 \
+// RUN:   -fgpu-rdc --hip-path=%S/Inputs/hipspv -nohipwrapperinc \
+// RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck %s
+
+// Emit 

[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-11-17 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added inline comments.



Comment at: clang/include/clang/Basic/Cuda.h:109
+  // Generic processor model is for testing only.
+  return A >= CudaArch::GFX600 && A <= CudaArch::GFX1035;
 }

can we use A < CudaArch::Generic instead? to avoid updating this line each time 
we add a new gfx arch.



Comment at: clang/include/clang/Driver/Options.td:1136
+def offload_EQ : CommaJoined<["--"], "offload=">, Flags<[NoXarchOption]>,
+  HelpText<"Specify comma-separated list of offloading targets.">;
+

linjamaki wrote:
> tra wrote:
> > `comma-separated list of offloading targets.` is, unfortunately, somewhat 
> > ambiguous.
> > Does it mean "how the offload will be done". I.e. HSA, OpenMP, SPIRV, CUDA? 
> > Or does it mean specific hardware we need to generate the code for? 
> > The code suggests it's a variant of the former, but the option description 
> > does not. 
> > 
> > E.g. `offload_arch_EQ ` also uses the term "offloading target" but with a 
> > different meaning.
> > 
> I’m not sure how to rephrase the option description to be more clear. In the 
> [1] the `--offload` option is envisioned to be quite flexible/expressive - it 
> can take in target triples, offload kinds, processors, aliases for processor 
> sets, etc.
> 
> FYI, I have imagined that the `--offload` option would take in explicit 
> offload kind and target triple combinations as the basis. For example, 
> something like this:
> 
> 
> ```
> --offload=hip-amdgcn-amd-amdhsa,openmp-x86_64-pc-linux-gnu
> ```
> 
> And top of that, there would be predefined strings/shortcuts/aliases that 
> expand to the basic form. For example, 
> `--offload=sm_70,openmp-host` could expand to something like:
> 
> 
> ```
> --offload=cuda-nvptx64-nvidia-cuda,openmp-x86_64-pc-linux-gnu 
> -Xoffload=cuda-nvptx64-nvidia-cuda -march=sm_70 ...
> 
> ```
> Then there is a feature as discussed in [1] that the offload kind can be 
> dropped if it can be inferred by other means (e.g. from `-x hip` option). 
> 
The description better matches the current implementation.

By this patch, `--offload=` only supports specifying target triple for HIP and 
assumes default processor. The description should reflect that.

In the future, as `--offload=` supports more values, the description may be 
updated.

Also, `--offload=` is designed to be mutually exclusive with `--offload-arch=`. 
Probably we should check and diagnose that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

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


[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-11-16 Thread Henry Linjamäki via Phabricator via cfe-commits
linjamaki added a comment.

Gentle ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

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


[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-11-16 Thread Henry Linjamäki via Phabricator via cfe-commits
linjamaki updated this revision to Diff 387509.
linjamaki added a comment.
Herald added a subscriber: asavonic.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

Files:
  clang/include/clang/Basic/Cuda.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Cuda.cpp
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/Inputs/hipspv-dev-lib/a/a.bc
  clang/test/Driver/Inputs/hipspv-dev-lib/b/b.bc
  clang/test/Driver/Inputs/hipspv-dev-lib/hipspv-spirv64.bc
  clang/test/Driver/Inputs/hipspv/bin/.hipVersion
  clang/test/Driver/Inputs/hipspv/lib/hip-device-lib/hipspv-spirv64.bc
  clang/test/Driver/Inputs/hipspv/lib/libLLVMHipSpvPasses.so
  clang/test/Driver/Inputs/pass-plugin.so
  clang/test/Driver/hipspv-device-libs.hip
  clang/test/Driver/hipspv-pass-plugin.hip
  clang/test/Driver/hipspv-toolchain-rdc.hip
  clang/test/Driver/hipspv-toolchain.hip
  clang/test/Driver/invalid-offload-options.cpp

Index: clang/test/Driver/invalid-offload-options.cpp
===
--- /dev/null
+++ clang/test/Driver/invalid-offload-options.cpp
@@ -0,0 +1,18 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload= \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=INVALID-TARGET %s
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=foo   \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=INVALID-TARGET %s
+
+// INVALID-TARGET: error: Invalid or unsupported offload target: '{{.*}}'
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=foo,bar \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=TOO-MANY-TARGETS %s
+
+// TOO-MANY-TARGETS: error: Only one offload target is supported in HIP.
Index: clang/test/Driver/hipspv-toolchain.hip
===
--- /dev/null
+++ clang/test/Driver/hipspv-toolchain.hip
@@ -0,0 +1,37 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -target x86_64-linux-gnu --offload=spirv64 \
+// RUN:   --hip-path=%S/Inputs/hipspv -nohipwrapperinc %s \
+// RUN: 2>&1 | FileCheck %s
+
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "spirv64"
+// CHECK-SAME: "-aux-triple" "{{.*}}" "-emit-llvm-bc"
+// CHECK-SAME: "-fcuda-is-device"
+// CHECK-SAME: "-fcuda-allow-variadic-functions"
+// CHECK-SAME: "-mlink-builtin-bitcode" {{".*/hipspv/lib/hip-device-lib/hipspv-spirv64.bc"}}
+// CHECK-SAME: "-isystem" {{".*/hipspv/include"}}
+// CHECK-SAME: "-fhip-new-launch-api"
+// CHECK-SAME: "-o" [[DEV_BC:".*bc"]]
+// CHECK-SAME: "-x" "hip"
+
+// CHECK: {{".*llvm-link"}} [[DEV_BC]] "-o" [[LINK_BC:".*bc"]]
+
+// CHECK: {{".*opt"}} [[LINK_BC]] "-load-pass-plugin"
+// CHECK-SAME: {{".*/hipspv/lib/libLLVMHipSpvPasses.so"}}
+// CHECK-SAME: "-passes=hip-post-link-passes" "-o" [[LOWER_BC:".*bc"]]
+
+// CHECK: {{".*llvm-spirv"}} "--spirv-max-version=1.1" "--spirv-ext=+all"
+// CHECK-SAME: [[LOWER_BC]] "-o" "[[SPIRV_OUT:.*out]]"
+
+// CHECK: {{".*clang-offload-bundler"}} "-type=o" "-bundle-align=4096"
+// CHECK-SAME: "-targets=host-x86_64-unknown-linux,hip-spirv64generic"
+// CHECK-SAME: "-inputs={{.*}},[[SPIRV_OUT]]" "-outputs=[[BUNDLE:.*hipfb]]"
+
+// CHECK: [[CLANG]] "-cc1" "-triple" {{".*"}} "-aux-triple" "spirv64"
+// CHECK-SAME: "-emit-obj"
+// CHECK-SAME: "-fcuda-include-gpubinary" "[[BUNDLE]]"
+// CHECK-SAME: "-o" [[OBJ_HOST:".*o"]] "-x" "hip"
+
+// CHECK: {{".*ld.*"}} {{.*}}[[OBJ_HOST]]
Index: clang/test/Driver/hipspv-toolchain-rdc.hip
===
--- /dev/null
+++ clang/test/Driver/hipspv-toolchain-rdc.hip
@@ -0,0 +1,63 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=spirv64 \
+// RUN:   -fgpu-rdc --hip-path=%S/Inputs/hipspv -nohipwrapperinc \
+// RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck %s
+
+// Emit objects for host side path
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "x86_64-unknown-linux-gnu"
+// CHECK-SAME: "-aux-triple" "spirv64"
+// CHECK-SAME: "-emit-obj"
+// CHECK-SAME: "-fgpu-rdc"
+// CHECK-SAME: {{.*}} "-o" [[A_OBJ_HOST:".*o"]] "-x" "hip"
+// CHECK-SAME: {{.*}} [[A_SRC:".*a.cu"]]
+
+// CHECK: [[CLANG]] "-cc1" "-triple" "x86_64-unknown-linux-gnu"
+// CHECK-SAME: "-aux-triple" "spirv64"
+// 

[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-10-26 Thread Henry Linjamäki via Phabricator via cfe-commits
linjamaki updated this revision to Diff 382244.
linjamaki added a comment.

Improve `--offload` option description.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

Files:
  clang/include/clang/Basic/Cuda.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Cuda.cpp
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/Inputs/hipspv-dev-lib/a/a.bc
  clang/test/Driver/Inputs/hipspv-dev-lib/b/b.bc
  clang/test/Driver/Inputs/hipspv-dev-lib/hipspv-spirv64.bc
  clang/test/Driver/Inputs/hipspv/bin/.hipVersion
  clang/test/Driver/Inputs/hipspv/lib/hip-device-lib/hipspv-spirv64.bc
  clang/test/Driver/Inputs/hipspv/lib/libLLVMHipSpvPasses.so
  clang/test/Driver/Inputs/pass-plugin.so
  clang/test/Driver/hipspv-device-libs.hip
  clang/test/Driver/hipspv-pass-plugin.hip
  clang/test/Driver/hipspv-toolchain-rdc.hip
  clang/test/Driver/hipspv-toolchain.hip
  clang/test/Driver/invalid-offload-options.cpp

Index: clang/test/Driver/invalid-offload-options.cpp
===
--- /dev/null
+++ clang/test/Driver/invalid-offload-options.cpp
@@ -0,0 +1,18 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload= \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=INVALID-TARGET %s
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=foo   \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=INVALID-TARGET %s
+
+// INVALID-TARGET: error: Invalid or unsupported offload target: '{{.*}}'
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=foo,bar \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=TOO-MANY-TARGETS %s
+
+// TOO-MANY-TARGETS: error: Only one offload target is supported in HIP.
Index: clang/test/Driver/hipspv-toolchain.hip
===
--- /dev/null
+++ clang/test/Driver/hipspv-toolchain.hip
@@ -0,0 +1,37 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -target x86_64-linux-gnu --offload=spirv64 \
+// RUN:   --hip-path=%S/Inputs/hipspv -nohipwrapperinc %s \
+// RUN: 2>&1 | FileCheck %s
+
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "spirv64"
+// CHECK-SAME: "-aux-triple" "{{.*}}" "-emit-llvm-bc"
+// CHECK-SAME: "-fcuda-is-device"
+// CHECK-SAME: "-fcuda-allow-variadic-functions"
+// CHECK-SAME: "-mlink-builtin-bitcode" {{".*/hipspv/lib/hip-device-lib/hipspv-spirv64.bc"}}
+// CHECK-SAME: "-isystem" {{".*/hipspv/include"}}
+// CHECK-SAME: "-fhip-new-launch-api"
+// CHECK-SAME: "-o" [[DEV_BC:".*bc"]]
+// CHECK-SAME: "-x" "hip"
+
+// CHECK: {{".*llvm-link"}} [[DEV_BC]] "-o" [[LINK_BC:".*bc"]]
+
+// CHECK: {{".*opt"}} [[LINK_BC]] "-load-pass-plugin"
+// CHECK-SAME: {{".*/hipspv/lib/libLLVMHipSpvPasses.so"}}
+// CHECK-SAME: "-passes=hip-post-link-passes" "-o" [[LOWER_BC:".*bc"]]
+
+// CHECK: {{".*llvm-spirv"}} "--spirv-max-version=1.1" "--spirv-ext=+all"
+// CHECK-SAME: [[LOWER_BC]] "-o" "[[SPIRV_OUT:.*out]]"
+
+// CHECK: {{".*clang-offload-bundler"}} "-type=o" "-bundle-align=4096"
+// CHECK-SAME: "-targets=host-x86_64-unknown-linux,hip-spirv64generic"
+// CHECK-SAME: "-inputs={{.*}},[[SPIRV_OUT]]" "-outputs=[[BUNDLE:.*hipfb]]"
+
+// CHECK: [[CLANG]] "-cc1" "-triple" {{".*"}} "-aux-triple" "spirv64"
+// CHECK-SAME: "-emit-obj"
+// CHECK-SAME: "-fcuda-include-gpubinary" "[[BUNDLE]]"
+// CHECK-SAME: "-o" [[OBJ_HOST:".*o"]] "-x" "hip"
+
+// CHECK: {{".*ld.*"}} {{.*}}[[OBJ_HOST]]
Index: clang/test/Driver/hipspv-toolchain-rdc.hip
===
--- /dev/null
+++ clang/test/Driver/hipspv-toolchain-rdc.hip
@@ -0,0 +1,63 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=spirv64 \
+// RUN:   -fgpu-rdc --hip-path=%S/Inputs/hipspv -nohipwrapperinc \
+// RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck %s
+
+// Emit objects for host side path
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "x86_64-unknown-linux-gnu"
+// CHECK-SAME: "-aux-triple" "spirv64"
+// CHECK-SAME: "-emit-obj"
+// CHECK-SAME: "-fgpu-rdc"
+// CHECK-SAME: {{.*}} "-o" [[A_OBJ_HOST:".*o"]] "-x" "hip"
+// CHECK-SAME: {{.*}} [[A_SRC:".*a.cu"]]
+
+// CHECK: [[CLANG]] "-cc1" "-triple" "x86_64-unknown-linux-gnu"
+// CHECK-SAME: "-aux-triple" "spirv64"
+// CHECK-SAME: 

[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-10-25 Thread Henry Linjamäki via Phabricator via cfe-commits
linjamaki updated this revision to Diff 381855.
linjamaki added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

Files:
  clang/include/clang/Basic/Cuda.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Cuda.cpp
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/Inputs/hipspv-dev-lib/a/a.bc
  clang/test/Driver/Inputs/hipspv-dev-lib/b/b.bc
  clang/test/Driver/Inputs/hipspv-dev-lib/hipspv-spirv64.bc
  clang/test/Driver/Inputs/hipspv/bin/.hipVersion
  clang/test/Driver/Inputs/hipspv/lib/hip-device-lib/hipspv-spirv64.bc
  clang/test/Driver/Inputs/hipspv/lib/libLLVMHipSpvPasses.so
  clang/test/Driver/Inputs/pass-plugin.so
  clang/test/Driver/hipspv-device-libs.hip
  clang/test/Driver/hipspv-pass-plugin.hip
  clang/test/Driver/hipspv-toolchain-rdc.hip
  clang/test/Driver/hipspv-toolchain.hip
  clang/test/Driver/invalid-offload-options.cpp

Index: clang/test/Driver/invalid-offload-options.cpp
===
--- /dev/null
+++ clang/test/Driver/invalid-offload-options.cpp
@@ -0,0 +1,18 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload= \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=INVALID-TARGET %s
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=foo   \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=INVALID-TARGET %s
+
+// INVALID-TARGET: error: Invalid or unsupported offload target: '{{.*}}'
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=foo,bar \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=TOO-MANY-TARGETS %s
+
+// TOO-MANY-TARGETS: error: Only one offload target is supported in HIP.
Index: clang/test/Driver/hipspv-toolchain.hip
===
--- /dev/null
+++ clang/test/Driver/hipspv-toolchain.hip
@@ -0,0 +1,37 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -target x86_64-linux-gnu --offload=spirv64 \
+// RUN:   --hip-path=%S/Inputs/hipspv -nohipwrapperinc %s \
+// RUN: 2>&1 | FileCheck %s
+
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "spirv64"
+// CHECK-SAME: "-aux-triple" "{{.*}}" "-emit-llvm-bc"
+// CHECK-SAME: "-fcuda-is-device"
+// CHECK-SAME: "-fcuda-allow-variadic-functions"
+// CHECK-SAME: "-mlink-builtin-bitcode" {{".*/hipspv/lib/hip-device-lib/hipspv-spirv64.bc"}}
+// CHECK-SAME: "-isystem" {{".*/hipspv/include"}}
+// CHECK-SAME: "-fhip-new-launch-api"
+// CHECK-SAME: "-o" [[DEV_BC:".*bc"]]
+// CHECK-SAME: "-x" "hip"
+
+// CHECK: {{".*llvm-link"}} [[DEV_BC]] "-o" [[LINK_BC:".*bc"]]
+
+// CHECK: {{".*opt"}} [[LINK_BC]] "-load-pass-plugin"
+// CHECK-SAME: {{".*/hipspv/lib/libLLVMHipSpvPasses.so"}}
+// CHECK-SAME: "-passes=hip-post-link-passes" "-o" [[LOWER_BC:".*bc"]]
+
+// CHECK: {{".*llvm-spirv"}} "--spirv-max-version=1.1" "--spirv-ext=+all"
+// CHECK-SAME: [[LOWER_BC]] "-o" "[[SPIRV_OUT:.*out]]"
+
+// CHECK: {{".*clang-offload-bundler"}} "-type=o" "-bundle-align=4096"
+// CHECK-SAME: "-targets=host-x86_64-unknown-linux,hip-spirv64generic"
+// CHECK-SAME: "-inputs={{.*}},[[SPIRV_OUT]]" "-outputs=[[BUNDLE:.*hipfb]]"
+
+// CHECK: [[CLANG]] "-cc1" "-triple" {{".*"}} "-aux-triple" "spirv64"
+// CHECK-SAME: "-emit-obj"
+// CHECK-SAME: "-fcuda-include-gpubinary" "[[BUNDLE]]"
+// CHECK-SAME: "-o" [[OBJ_HOST:".*o"]] "-x" "hip"
+
+// CHECK: {{".*ld.*"}} {{.*}}[[OBJ_HOST]]
Index: clang/test/Driver/hipspv-toolchain-rdc.hip
===
--- /dev/null
+++ clang/test/Driver/hipspv-toolchain-rdc.hip
@@ -0,0 +1,63 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=spirv64 \
+// RUN:   -fgpu-rdc --hip-path=%S/Inputs/hipspv -nohipwrapperinc \
+// RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck %s
+
+// Emit objects for host side path
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "x86_64-unknown-linux-gnu"
+// CHECK-SAME: "-aux-triple" "spirv64"
+// CHECK-SAME: "-emit-obj"
+// CHECK-SAME: "-fgpu-rdc"
+// CHECK-SAME: {{.*}} "-o" [[A_OBJ_HOST:".*o"]] "-x" "hip"
+// CHECK-SAME: {{.*}} [[A_SRC:".*a.cu"]]
+
+// CHECK: [[CLANG]] "-cc1" "-triple" "x86_64-unknown-linux-gnu"
+// CHECK-SAME: "-aux-triple" "spirv64"
+// CHECK-SAME: "-emit-obj"
+// CHECK-SAME: 

[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-10-04 Thread Henry Linjamäki via Phabricator via cfe-commits
linjamaki updated this revision to Diff 376829.
linjamaki added a comment.

Repurpose 'Generic' CudaArch, Use getAllArgValues() for reading
--offload values and fix a enum range.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

Files:
  clang/include/clang/Basic/Cuda.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Cuda.cpp
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/Inputs/hipspv-dev-lib/a/a.bc
  clang/test/Driver/Inputs/hipspv-dev-lib/b/b.bc
  clang/test/Driver/Inputs/hipspv-dev-lib/hipspv-spirv64.bc
  clang/test/Driver/Inputs/hipspv/bin/.hipVersion
  clang/test/Driver/Inputs/hipspv/lib/hip-device-lib/hipspv-spirv64.bc
  clang/test/Driver/Inputs/hipspv/lib/libLLVMHipSpvPasses.so
  clang/test/Driver/Inputs/pass-plugin.so
  clang/test/Driver/hipspv-device-libs.hip
  clang/test/Driver/hipspv-pass-plugin.hip
  clang/test/Driver/hipspv-toolchain-rdc.hip
  clang/test/Driver/hipspv-toolchain.hip
  clang/test/Driver/invalid-offload-options.cpp

Index: clang/test/Driver/invalid-offload-options.cpp
===
--- /dev/null
+++ clang/test/Driver/invalid-offload-options.cpp
@@ -0,0 +1,18 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload= \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=INVALID-TARGET %s
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=foo   \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=INVALID-TARGET %s
+
+// INVALID-TARGET: error: Invalid or unsupported offload target: '{{.*}}'
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=foo,bar \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=TOO-MANY-TARGETS %s
+
+// TOO-MANY-TARGETS: error: Only one offload target is supported in HIP.
Index: clang/test/Driver/hipspv-toolchain.hip
===
--- /dev/null
+++ clang/test/Driver/hipspv-toolchain.hip
@@ -0,0 +1,37 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -target x86_64-linux-gnu --offload=spirv64 \
+// RUN:   --hip-path=%S/Inputs/hipspv -nohipwrapperinc %s \
+// RUN: 2>&1 | FileCheck %s
+
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "spirv64"
+// CHECK-SAME: "-aux-triple" "{{.*}}" "-emit-llvm-bc"
+// CHECK-SAME: "-fcuda-is-device"
+// CHECK-SAME: "-fcuda-allow-variadic-functions"
+// CHECK-SAME: "-mlink-builtin-bitcode" {{".*/hipspv/lib/hip-device-lib/hipspv-spirv64.bc"}}
+// CHECK-SAME: "-isystem" {{".*/hipspv/include"}}
+// CHECK-SAME: "-fhip-new-launch-api"
+// CHECK-SAME: "-o" [[DEV_BC:".*bc"]]
+// CHECK-SAME: "-x" "hip"
+
+// CHECK: {{".*llvm-link"}} [[DEV_BC]] "-o" [[LINK_BC:".*bc"]]
+
+// CHECK: {{".*opt"}} [[LINK_BC]] "-load-pass-plugin"
+// CHECK-SAME: {{".*/hipspv/lib/libLLVMHipSpvPasses.so"}}
+// CHECK-SAME: "-passes=hip-post-link-passes" "-o" [[LOWER_BC:".*bc"]]
+
+// CHECK: {{".*llvm-spirv"}} "-spirv-max-version=1.1" "--spirv-ext=+all"
+// CHECK-SAME: [[LOWER_BC]] "-o" "[[SPIRV_OUT:.*out]]"
+
+// CHECK: {{".*clang-offload-bundler"}} "-type=o" "-bundle-align=4096"
+// CHECK-SAME: "-targets=host-x86_64-unknown-linux,hip-spirv64generic"
+// CHECK-SAME: "-inputs={{.*}},[[SPIRV_OUT]]" "-outputs=[[BUNDLE:.*hipfb]]"
+
+// CHECK: [[CLANG]] "-cc1" "-triple" {{".*"}} "-aux-triple" "spirv64"
+// CHECK-SAME: "-emit-obj"
+// CHECK-SAME: "-fcuda-include-gpubinary" "[[BUNDLE]]"
+// CHECK-SAME: "-o" [[OBJ_HOST:".*o"]] "-x" "hip"
+
+// CHECK: {{".*ld.*"}} {{.*}}[[OBJ_HOST]]
Index: clang/test/Driver/hipspv-toolchain-rdc.hip
===
--- /dev/null
+++ clang/test/Driver/hipspv-toolchain-rdc.hip
@@ -0,0 +1,63 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=spirv64 \
+// RUN:   -fgpu-rdc --hip-path=%S/Inputs/hipspv -nohipwrapperinc \
+// RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck %s
+
+// Emit objects for host side path
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "x86_64-unknown-linux-gnu"
+// CHECK-SAME: "-aux-triple" "spirv64"
+// CHECK-SAME: "-emit-obj"
+// CHECK-SAME: "-fgpu-rdc"
+// CHECK-SAME: {{.*}} "-o" [[A_OBJ_HOST:".*o"]] "-x" "hip"
+// CHECK-SAME: {{.*}} [[A_SRC:".*a.cu"]]
+
+// CHECK: [[CLANG]] "-cc1" "-triple" "x86_64-unknown-linux-gnu"
+// CHECK-SAME: 

[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-10-04 Thread Henry Linjamäki via Phabricator via cfe-commits
linjamaki added a comment.

In D110622#3030792 , @tra wrote:

>> A Cuda GPU architecture ‘generic’ is added. The name is picked from the LLVM 
>> SPIR-V Backend. In the HIPSPV code path the architecture name is inserted to 
>> the bundle entry ID as target ID. Target ID is expected to be always present 
>> so a component in the target triple is not mistaken as target ID.
>
> How generic is 'generic'? If I understand the statement above correctly, it 
> should probably reflect that it's specific to spir-v.
> If it's the only possible spir-v variant, then calling it`spir-v` might be 
> more meaningful.
> If we expect to see other spir-v variants in the future it would allow us to 
> clearly differentiate between them later. 
> E.g. `--offload=spirv-a,spirv-b`. It would be rather odd if we had to use 
> `--offload=generic, spirv-b`.

In this patch the ‘generic’ is meant to be a processor model defined in the 
SPIR-V backend. Now to come to think of it a bit more, I think it should not be 
specific to the SPIR-V target but the target at hand if its backend defines 
one. What I’m seeing is that each entry in the CudaArch has a processor by the 
same name in the NVPTX and AMGPU backends.

If I need to set different processor other from the SPIR-V backend than what is 
set as the default in HIP compilation, I thought from the [1] it could be 
carried out with something like:

  --offload=spirv64 -Xoffload=spirv64 -march=other-spirv-cpu

[1]: https://lists.llvm.org/pipermail/cfe-dev/2020-December/067362.html

In D110622#3031010 , @tra wrote:

>> --offload’ option, which is envisioned in [1], is added for specifying 
>> offload targets. This option is used to override default device target 
>> (amdgcn-amd-amdhsa) for HIP compilation for emitting device code as SPIR-V 
>> binary. The option is handled in getHIPOffloadTargetTriple().
>
> Can you elaborate on what exactly this option does and how it's intended to 
> interact with the existing `--offload-arch`?

I think that the --offload-arch interaction question is for @yaxunl. What is 
being contributed here is a partial implementation for the unified offloading 
options. The --offload option in this patch is used to supply the offload 
device target triple (in HIP compilation mode) for retargeting the device code 
emission to SPIR-V instead of emitting HSA.

> In general a list of values, combined with the `getLastArg` will potentially 
> be an issue if/when more than one list value will be supported.
> In a large build it's fairly common for the build infrastructure to set the 
> default options and allowing users to extend/override them with *additional* 
> options. `getLastArg` works great for scalar options, not so much for the 
> lists.
> If an option is a list, modifying it requires prior knowledge of preceding 
> values and that may not always be easy.
> E.g. a build configuration may be set to  target gfx900 and gfx908. If I want 
> to *add* an option to target gfx1030, I would need to dig out the options for 
> the currently-enabled architectures and specify all of them again. It's 
> doable once, manually, but it does not scale if this option is expected to be 
> regularly tweaked by the end user, as is the case with `--offload-arch`. If 
> `--offload` is expected to have similar use patterns, you may need to 
> consider allowing it to be adjusted per-list-element.

The use of getLastArg() is an oversight. I’ll fix it with getAllArgValues().




Comment at: clang/include/clang/Basic/Cuda.h:106
 static inline bool IsAMDGpuArch(CudaArch A) {
   return A >= CudaArch::GFX600 && A < CudaArch::LAST;
 }

tra wrote:
> Does this need to be adjusted to exclude SPIR-V? If so, you may want to add 
> another enum range for SPIR-V.
> 
Didn't notice this. I'll fix this.



Comment at: clang/include/clang/Driver/Options.td:1136
+def offload_EQ : CommaJoined<["--"], "offload=">, Flags<[NoXarchOption]>,
+  HelpText<"Specify comma-separated list of offloading targets.">;
+

tra wrote:
> `comma-separated list of offloading targets.` is, unfortunately, somewhat 
> ambiguous.
> Does it mean "how the offload will be done". I.e. HSA, OpenMP, SPIRV, CUDA? 
> Or does it mean specific hardware we need to generate the code for? 
> The code suggests it's a variant of the former, but the option description 
> does not. 
> 
> E.g. `offload_arch_EQ ` also uses the term "offloading target" but with a 
> different meaning.
> 
I’m not sure how to rephrase the option description to be more clear. In the 
[1] the `--offload` option is envisioned to be quite flexible/expressive - it 
can take in target triples, offload kinds, processors, aliases for processor 
sets, etc.

FYI, I have imagined that the `--offload` option would take in explicit offload 
kind and target triple combinations as the basis. For example, something like 
this:


```

[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-09-29 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

> --offload’ option, which is envisioned in [1], is added for specifying 
> offload targets. This option is used to override default device target 
> (amdgcn-amd-amdhsa) for HIP compilation for emitting device code as SPIR-V 
> binary. The option is handled in getHIPOffloadTargetTriple().

Can you elaborate on what exactly this option does and how it's intended to 
interact with the existing `--offload-arch`?

In general a list of values, combined with the `getLastArg` will potentially be 
an issue if/when more than one list value will be supported.
In a large build it's fairly common for the build infrastructure to set the 
default options and allowing users to extend/override them with *additional* 
options. `getLastArg` works great for scalar options, not so much for the lists.
If an option is a list, modifying it requires prior knowledge of preceding 
values and that may not always be easy.
E.g. a build configuration may be set to  target gfx900 and gfx908. If I want 
to *add* an option to target gfx1030, I would need to dig out the options for 
the currently-enabled architectures and specify all of them again. It's doable 
once, manually, but it does not scale if this option is expected to be 
regularly tweaked by the end user, as is the case with `--offload-arch`. If 
`--offload` is expected to have similar use patterns, you may need to consider 
allowing it to be adjusted per-list-element.




Comment at: clang/include/clang/Basic/Cuda.h:106
 static inline bool IsAMDGpuArch(CudaArch A) {
   return A >= CudaArch::GFX600 && A < CudaArch::LAST;
 }

Does this need to be adjusted to exclude SPIR-V? If so, you may want to add 
another enum range for SPIR-V.




Comment at: clang/include/clang/Driver/Options.td:1136
+def offload_EQ : CommaJoined<["--"], "offload=">, Flags<[NoXarchOption]>,
+  HelpText<"Specify comma-separated list of offloading targets.">;
+

`comma-separated list of offloading targets.` is, unfortunately, somewhat 
ambiguous.
Does it mean "how the offload will be done". I.e. HSA, OpenMP, SPIRV, CUDA? 
Or does it mean specific hardware we need to generate the code for? 
The code suggests it's a variant of the former, but the option description does 
not. 

E.g. `offload_arch_EQ ` also uses the term "offloading target" but with a 
different meaning.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

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


[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-09-29 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

> A Cuda GPU architecture ‘generic’ is added. The name is picked from the LLVM 
> SPIR-V Backend. In the HIPSPV code path the architecture name is inserted to 
> the bundle entry ID as target ID. Target ID is expected to be always present 
> so a component in the target triple is not mistaken as target ID.

How generic is 'generic'? If I understand the statement above correctly, it 
should probably reflect that it's specific to spir-v.
If it's the only possible spir-v variant, then calling it`spir-v` might be more 
meaningful.
If we expect to see other spir-v variants in the future it would allow us to 
clearly differentiate between them later. 
E.g. `--offload=spirv-a,spirv-b`. It would be rather odd if we had to use 
`--offload=generic, spirv-b`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

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


[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

2021-09-29 Thread Henry Linjamäki via Phabricator via cfe-commits
linjamaki created this revision.
Herald added subscribers: Naghasan, dexonsmith, dang, Anastasia, yaxunl, 
jholewinski.
linjamaki updated this revision to Diff 375783.
linjamaki added a comment.
linjamaki updated this revision to Diff 375786.
linjamaki added reviewers: yaxunl, bader.
linjamaki published this revision for review.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

Style fixes.


linjamaki added a comment.

Remove noisy change.


This patch enables SPIR-V binary emission for HIP device code via the
HIPSPV tool chain.

- ‘--offload’ option, which is envisioned in [1], is added for specifying 
offload targets. This option is used to override default device target 
(amdgcn-amd-amdhsa) for HIP compilation for emitting device code as SPIR-V 
binary. The option is handled in getHIPOffloadTargetTriple().

- getOffloadingDeviceToolChain() function (based on the design in the SYCL 
repository) is added to select HIPSPVToolChain when HIP offload target is 
‘spirv64’.

- The HIPActionBuilder is modified to produce LLVM IR at the backend phase. 
HIPSPV tool chain expects to receive HIP device code as LLVM IR so it can run 
external LLVM passes over them. HIPSPV TC is also responsible for emitting the 
SPIR-V binary.

- A Cuda GPU architecture ‘generic’ is added. The name is picked from the LLVM 
SPIR-V Backend. In the HIPSPV code path the architecture name is inserted to 
the bundle entry ID as target ID. Target ID is expected to be always present so 
a component in the target triple is not mistaken as target ID.

- Tests are added for checking the HIPSPV tool chain.

[1]: https://lists.llvm.org/pipermail/cfe-dev/2020-December/067362.html


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110622

Files:
  clang/include/clang/Basic/Cuda.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Cuda.cpp
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/Inputs/hipspv-dev-lib/a/a.bc
  clang/test/Driver/Inputs/hipspv-dev-lib/b/b.bc
  clang/test/Driver/Inputs/hipspv-dev-lib/hipspv-spirv64.bc
  clang/test/Driver/Inputs/hipspv/bin/.hipVersion
  clang/test/Driver/Inputs/hipspv/lib/hip-device-lib/hipspv-spirv64.bc
  clang/test/Driver/Inputs/hipspv/lib/libLLVMHipSpvPasses.so
  clang/test/Driver/Inputs/pass-plugin.so
  clang/test/Driver/hipspv-device-libs.hip
  clang/test/Driver/hipspv-pass-plugin.hip
  clang/test/Driver/hipspv-toolchain-rdc.hip
  clang/test/Driver/hipspv-toolchain.hip
  clang/test/Driver/invalid-offload-options.cpp

Index: clang/test/Driver/invalid-offload-options.cpp
===
--- /dev/null
+++ clang/test/Driver/invalid-offload-options.cpp
@@ -0,0 +1,18 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload= \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=INVALID-TARGET %s
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=foo   \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=INVALID-TARGET %s
+
+// INVALID-TARGET: error: Invalid or unsupported offload target: '{{.*}}'
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=foo,bar \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=TOO-MANY-TARGETS %s
+
+// TOO-MANY-TARGETS: error: Only one offload target is supported in HIP.
Index: clang/test/Driver/hipspv-toolchain.hip
===
--- /dev/null
+++ clang/test/Driver/hipspv-toolchain.hip
@@ -0,0 +1,37 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -target x86_64-linux-gnu --offload=spirv64 \
+// RUN:   --hip-path=%S/Inputs/hipspv -nohipwrapperinc %s \
+// RUN: 2>&1 | FileCheck %s
+
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "spirv64"
+// CHECK-SAME: "-aux-triple" "{{.*}}" "-emit-llvm-bc"
+// CHECK-SAME: "-fcuda-is-device"
+// CHECK-SAME: "-fcuda-allow-variadic-functions"
+// CHECK-SAME: "-mlink-builtin-bitcode" {{".*/hipspv/lib/hip-device-lib/hipspv-spirv64.bc"}}
+// CHECK-SAME: "-isystem" {{".*/hipspv/include"}}
+// CHECK-SAME: "-fhip-new-launch-api"
+// CHECK-SAME: "-o" [[DEV_BC:".*bc"]]
+// CHECK-SAME: "-x" "hip"
+
+// CHECK: {{".*llvm-link"}} [[DEV_BC]] "-o" [[LINK_BC:".*bc"]]
+
+// CHECK: {{".*opt"}} [[LINK_BC]] "-load-pass-plugin"
+// CHECK-SAME: {{".*/hipspv/lib/libLLVMHipSpvPasses.so"}}
+// CHECK-SAME: "-passes=hip-post-link-passes" "-o" [[LOWER_BC:".*bc"]]
+
+// CHECK: {{".*llvm-spirv"}} "-spirv-max-version=1.1" "--spirv-ext=+all"
+//