[PATCH] D76957: HIP: Merge builtin library handling

[Matt Arsenault via Phabricator via cfe-commits]

arsenm closed this revision.
arsenm added a comment.

14e184571139ba4c7347ea547074c6d9ec9c7b14 



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

https://reviews.llvm.org/D76957



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

[PATCH] D76957: HIP: Merge builtin library handling

[Yaxun Liu via Phabricator via cfe-commits]

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

LGTM. Thanks.


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

https://reviews.llvm.org/D76957



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

[PATCH] D76957: HIP: Merge builtin library handling

[Matt Arsenault via Phabricator via cfe-commits]

arsenm updated this revision to Diff 258562.
arsenm added a comment.

Switch default back for correct sqrt. Also add more checks for all of the 
linked libs, and fix duplicating wave64 logic


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

https://reviews.llvm.org/D76957

Files:
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/AMDGPU.h
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/test/Driver/hip-device-libs.hip
  clang/test/Driver/rocm-device-libs.cl

Index: clang/test/Driver/rocm-device-libs.cl
===
--- clang/test/Driver/rocm-device-libs.cl
+++ clang/test/Driver/rocm-device-libs.cl
@@ -121,6 +121,21 @@
 
 
 
+// Test --hip-device-lib-path format
+// RUN: %clang -### -target amdgcn-amd-amdhsa \
+// RUN:   -x cl -mcpu=gfx900 \
+// RUN:   --hip-device-lib-path=%S/Inputs/rocm-device-libs/amdgcn/bitcode \
+// RUN:   %S/opencl.cl \
+// RUN: 2>&1 | FileCheck -dump-input-on-failure --check-prefixes=COMMON,COMMON-DEFAULT,GFX900-DEFAULT,GFX900,WAVE64 %s
+
+// Test environment variable HIP_DEVICE_LIB_PATH
+// RUN: env HIP_DEVICE_LIB_PATH=%S/Inputs/rocm-device-libs/amdgcn/bitcode %clang -### -target amdgcn-amd-amdhsa \
+// RUN:   -x cl -mcpu=gfx900 \
+// RUN:   %S/opencl.cl \
+// RUN: 2>&1 | FileCheck -dump-input-on-failure --check-prefixes=COMMON,COMMON-DEFAULT,GFX900-DEFAULT,GFX900,WAVE64 %s
+
+
+
 // COMMON: "-triple" "amdgcn-amd-amdhsa"
 // COMMON-SAME: "-mlink-builtin-bitcode" "{{.*}}/amdgcn/bitcode/opencl.bc"
 // COMMON-SAME: "-mlink-builtin-bitcode" "{{.*}}/amdgcn/bitcode/ocml.bc"
Index: clang/test/Driver/hip-device-libs.hip
===
--- clang/test/Driver/hip-device-libs.hip
+++ clang/test/Driver/hip-device-libs.hip
@@ -7,16 +7,16 @@
 
 // Test subtarget with flushing on by default.
 // RUN: %clang -### -target x86_64-linux-gnu \
-// RUN:   --cuda-gpu-arch=gfx803 \
-// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib   \
+// RUN:  --cuda-gpu-arch=gfx803 \
+// RUN:  --rocm-path=%S/Inputs/rocm-device-libs   \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,FLUSHD
 
 
 // Test subtarget with flushing off by ddefault.
 // RUN: %clang -### -target x86_64-linux-gnu \
-// RUN:   --cuda-gpu-arch=gfx900 \
-// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib \
+// RUN:  --cuda-gpu-arch=gfx900 \
+// RUN:  --rocm-path=%S/Inputs/rocm-device-libs \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,NOFLUSHD
 
@@ -25,7 +25,7 @@
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx900 \
 // RUN:   -fcuda-flush-denormals-to-zero \
-// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,FLUSHD
 
@@ -34,7 +34,7 @@
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx803 \
 // RUN:   -fno-cuda-flush-denormals-to-zero \
-// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,NOFLUSHD
 
@@ -43,7 +43,7 @@
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx900 \
 // RUN:   -fno-cuda-flush-denormals-to-zero \
-// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,NOFLUSHD
 
@@ -52,7 +52,7 @@
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx803 \
 // RUN:   -fcuda-flush-denormals-to-zero \
-// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,FLUSHD
 
@@ -61,7 +61,7 @@
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx803 \
 // RUN:   -fcuda-flush-denormals-to-zero -fno-cuda-flush-denormals-to-zero \
-// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,NOFLUSHD
 
@@ -69,7 +69,7 @@
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx900 \
 // RUN:   -fcuda-flush-denormals-to-zero -fno-cuda-flush-denormals-to-zero \
-// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib   \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs   \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,NOFLUSHD
 
@@ -77,7 +77,7 @@
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx900 \
 // RUN:   

[PATCH] D76957: HIP: Merge builtin library handling

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/lib/Driver/ToolChains/HIP.cpp:364
+bool FastRelaxedMath = false;
+bool CorrectSqrt = false;
+

By default this was on. We should keep the old behavior.


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

https://reviews.llvm.org/D76957



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

[PATCH] D76957: HIP: Merge builtin library handling

[Matt Arsenault via Phabricator via cfe-commits]

arsenm updated this revision to Diff 257694.
arsenm added a comment.

Update for new library structure


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

https://reviews.llvm.org/D76957

Files:
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/AMDGPU.h
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/test/Driver/hip-device-libs.hip
  clang/test/Driver/rocm-device-libs.cl

Index: clang/test/Driver/rocm-device-libs.cl
===
--- clang/test/Driver/rocm-device-libs.cl
+++ clang/test/Driver/rocm-device-libs.cl
@@ -121,6 +121,21 @@
 
 
 
+// Test --hip-device-lib-path format
+// RUN: %clang -### -target amdgcn-amd-amdhsa \
+// RUN:   -x cl -mcpu=gfx900 \
+// RUN:   --hip-device-lib-path=%S/Inputs/rocm-device-libs/amdgcn/bitcode \
+// RUN:   %S/opencl.cl \
+// RUN: 2>&1 | FileCheck -dump-input-on-failure --check-prefixes=COMMON,COMMON-DEFAULT,GFX900-DEFAULT,GFX900,WAVE64 %s
+
+// Test environment variable HIP_DEVICE_LIB_PATH
+// RUN: env HIP_DEVICE_LIB_PATH=%S/Inputs/rocm-device-libs/amdgcn/bitcode %clang -### -target amdgcn-amd-amdhsa \
+// RUN:   -x cl -mcpu=gfx900 \
+// RUN:   %S/opencl.cl \
+// RUN: 2>&1 | FileCheck -dump-input-on-failure --check-prefixes=COMMON,COMMON-DEFAULT,GFX900-DEFAULT,GFX900,WAVE64 %s
+
+
+
 // COMMON: "-triple" "amdgcn-amd-amdhsa"
 // COMMON-SAME: "-mlink-builtin-bitcode" "{{.*}}/amdgcn/bitcode/opencl.bc"
 // COMMON-SAME: "-mlink-builtin-bitcode" "{{.*}}/amdgcn/bitcode/ocml.bc"
Index: clang/test/Driver/hip-device-libs.hip
===
--- clang/test/Driver/hip-device-libs.hip
+++ clang/test/Driver/hip-device-libs.hip
@@ -7,16 +7,16 @@
 
 // Test subtarget with flushing on by default.
 // RUN: %clang -### -target x86_64-linux-gnu \
-// RUN:   --cuda-gpu-arch=gfx803 \
-// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib   \
+// RUN:  --cuda-gpu-arch=gfx803 \
+// RUN:  --rocm-path=%S/Inputs/rocm-device-libs   \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,FLUSHD
 
 
 // Test subtarget with flushing off by ddefault.
 // RUN: %clang -### -target x86_64-linux-gnu \
-// RUN:   --cuda-gpu-arch=gfx900 \
-// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib \
+// RUN:  --cuda-gpu-arch=gfx900 \
+// RUN:  --rocm-path=%S/Inputs/rocm-device-libs \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,NOFLUSHD
 
@@ -25,7 +25,7 @@
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx900 \
 // RUN:   -fcuda-flush-denormals-to-zero \
-// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,FLUSHD
 
@@ -34,7 +34,7 @@
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx803 \
 // RUN:   -fno-cuda-flush-denormals-to-zero \
-// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,NOFLUSHD
 
@@ -43,7 +43,7 @@
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx900 \
 // RUN:   -fno-cuda-flush-denormals-to-zero \
-// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,NOFLUSHD
 
@@ -52,7 +52,7 @@
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx803 \
 // RUN:   -fcuda-flush-denormals-to-zero \
-// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,FLUSHD
 
@@ -61,7 +61,7 @@
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx803 \
 // RUN:   -fcuda-flush-denormals-to-zero -fno-cuda-flush-denormals-to-zero \
-// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,NOFLUSHD
 
@@ -69,7 +69,7 @@
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx900 \
 // RUN:   -fcuda-flush-denormals-to-zero -fno-cuda-flush-denormals-to-zero \
-// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib   \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs   \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,NOFLUSHD
 
@@ -77,7 +77,7 @@
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx900 \
 // RUN:   -fno-cuda-flush-denormals-to-zero -fcuda-flush-denormals-to-zero \
-// RUN:   

[PATCH] D76957: HIP: Merge builtin library handling

[Matt Arsenault via Phabricator via cfe-commits]

arsenm created this revision.
arsenm added reviewers: yaxunl, gregrodgers.
Herald added subscribers: kerbowa, Anastasia, nhaehnle, wdng, jvesely.
arsenm added parent revisions: D76862: HIP: Ensure new denormal mode attributes 
are set, D76950: HIP: Link correct denormal mode library, D59321: AMDGPU: Teach 
toolchain to link rocm device libs.

Merge with the new --rocm-path handling used for OpenCL. This looks
for a usable set of device libraries upfront, rather than giving a
generic "no such file or directory error". If any of the required
bitcode libraries are missing, this will now produce a "cannot find
ROCm installation." error. This differs from the existing hip specific
flags by pointing to a rocm root install instead of a single directory
with bitcode files.

  

This tries to maintain compatibility with the existing the
--hip-device-lib and --hip-device-lib-path flags, as well as the
HIP_DEVICE_LIB_PATH environment variable, or at least the range of
uses with testcases. The existing range of uses and behavior doesn't
entirely make sense to me, so some of the untested edge cases change
 behavior. Currently the two path forms seem to have the double purpose
of a search path for an arbitrary --hip-device-lib, and for finding
he stock set of libraries. Since the stock set of libraries This also
changes the behavior when multiple paths are specified, and only takes
the last one (and the environment variable only handles a single
path).

  

If --hip-device-lib is used, it now only treats --hip-device-lib-path
as the search path for it, and does not attempt to find the rocm
installation. If not, --hip-device-lib-path and the environment
variable are used as the directory to search instead of the rocm root
based path.

  

This should also automatically fix handling of the options to use
wave64.


https://reviews.llvm.org/D76957

Files:
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/AMDGPU.h
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/test/Driver/hip-device-libs.hip
  clang/test/Driver/rocm-device-libs.cl

Index: clang/test/Driver/rocm-device-libs.cl
===
--- clang/test/Driver/rocm-device-libs.cl
+++ clang/test/Driver/rocm-device-libs.cl
@@ -120,6 +120,21 @@
 
 
 
+// Test --hip-device-lib-path format
+// RUN: %clang -### -target amdgcn-amd-amdhsa \
+// RUN:   -x cl -mcpu=gfx900 \
+// RUN:   --hip-device-lib-path=%S/Inputs/rocm-device-libs/lib \
+// RUN:   %S/opencl.cl \
+// RUN: 2>&1 | FileCheck -dump-input-on-failure --check-prefixes=COMMON,COMMON-DEFAULT,GFX900-DEFAULT,GFX900,WAVE64 %s
+
+// Test environment variable HIP_DEVICE_LIB_PATH
+// RUN: env HIP_DEVICE_LIB_PATH=%S/Inputs/rocm-device-libs/lib %clang -### -target amdgcn-amd-amdhsa \
+// RUN:   -x cl -mcpu=gfx900 \
+// RUN:   %S/opencl.cl \
+// RUN: 2>&1 | FileCheck -dump-input-on-failure --check-prefixes=COMMON,COMMON-DEFAULT,GFX900-DEFAULT,GFX900,WAVE64 %s
+
+
+
 // COMMON: "-triple" "amdgcn-amd-amdhsa"
 // COMMON-SAME: "-mlink-builtin-bitcode" "{{.*}}/lib/opencl.amdgcn.bc"
 // COMMON-SAME: "-mlink-builtin-bitcode" "{{.*}}/lib/ocml.amdgcn.bc"
Index: clang/test/Driver/hip-device-libs.hip
===
--- clang/test/Driver/hip-device-libs.hip
+++ clang/test/Driver/hip-device-libs.hip
@@ -7,16 +7,16 @@
 
 // Test subtarget with flushing on by default.
 // RUN: %clang -### -target x86_64-linux-gnu \
-// RUN:   --cuda-gpu-arch=gfx803 \
-// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib   \
+// RUN:  --cuda-gpu-arch=gfx803 \
+// RUN:  --rocm-path=%S/Inputs/rocm-device-libs   \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,FLUSHD
 
 
 // Test subtarget with flushing off by ddefault.
 // RUN: %clang -### -target x86_64-linux-gnu \
-// RUN:   --cuda-gpu-arch=gfx900 \
-// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib \
+// RUN:  --cuda-gpu-arch=gfx900 \
+// RUN:  --rocm-path=%S/Inputs/rocm-device-libs \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,NOFLUSHD
 
@@ -25,7 +25,7 @@
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx900 \
 // RUN:   -fcuda-flush-denormals-to-zero \
-// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,FLUSHD
 
@@ -34,7 +34,7 @@
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx803 \
 // RUN:   -fno-cuda-flush-denormals-to-zero \
-// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
 // RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=COM,NOFLUSHD
 
@@ -43,7 +43,7 @@
 // RUN: %clang -### -target x86_64-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx900 \
 // RUN: