#156 Add patch to allow packaging rocm-hip
Merged 2 years ago by sergesanspaille. Opened 2 years ago by mystro256.
rpms/ mystro256/clang rawhide  into  rawhide

@@ -0,0 +1,122 @@ 

+ From 4218b3ad4f7a444ab72bd50c15d2adca85ac23c9 Mon Sep 17 00:00:00 2001

+ From: "Yaxun (Sam) Liu" <yaxun.liu@amd.com>

+ Date: Wed, 9 Mar 2022 09:10:17 -0500

+ Subject: [PATCH] [HIP] Fix HIP include path

+ 

+ The clang compiler prepends the HIP header include paths to the search

+ list using -internal-isystem when building for the HIP language. This

+ prevents warnings related to things like reserved identifiers when

+ including the HIP headers even when ROCm is installed in a non-system

+ directory, such as /opt/rocm.

+ 

+ However, when HIP is installed in /usr, then the prepended include

+ path would be /usr/include. That is a problem, because the C standard

+ library headers are stored in /usr/include and the C++ standard

+ library headers must come before the C library headers in the search

+ path list (because the C++ standard library headers use #include_next

+ to include the C standard library headers).

+ 

+ While the HIP wrapper headers _do_ need to be earlier in the search

+ than the C++ headers, those headers get their own subdirectory and

+ their own explicit -internal-isystem argument. This include path is for

+ <hip/hip_runtime_api.h> and <hip/hip_runtime.h>, which do not require a

+ particular search ordering with respect to the C or C++ headers. Thus,

+ HIP include path is added after other system include paths.

+ 

+ With contribution from Cordell Bloor.

+ 

+ Reviewed by: Artem Belevich

+ 

+ Differential Revision: https://reviews.llvm.org/D120132

+ ---

+  clang/lib/Driver/ToolChains/AMDGPU.cpp |  2 +-

+  clang/test/Driver/hip-include-path.hip | 10 +++++-----

+  clang/test/Driver/rocm-detect.hip      |  6 +++---

+  3 files changed, 9 insertions(+), 9 deletions(-)

+ 

+ diff --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp b/clang/lib/Driver/ToolChains/AMDGPU.cpp

+ index 43ce33750eba..9638fa2ecfca 100644

+ --- a/clang/lib/Driver/ToolChains/AMDGPU.cpp

+ +++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp

+ @@ -510,7 +510,7 @@ void RocmInstallationDetector::AddHIPIncludeArgs(const ArgList &DriverArgs,

+      return;

+    }

+  

+ -  CC1Args.push_back("-internal-isystem");

+ +  CC1Args.push_back("-idirafter");

+    CC1Args.push_back(DriverArgs.MakeArgString(getIncludePath()));

+    if (UsesRuntimeWrapper)

+      CC1Args.append({"-include", "__clang_hip_runtime_wrapper.h"});

+ diff --git a/clang/test/Driver/hip-include-path.hip b/clang/test/Driver/hip-include-path.hip

+ index dce42f58fdf5..92f2fab6c24c 100644

+ --- a/clang/test/Driver/hip-include-path.hip

+ +++ b/clang/test/Driver/hip-include-path.hip

+ @@ -19,24 +19,24 @@

+  // COMMON-LABEL: "{{[^"]*}}clang{{[^"]*}}" "-cc1"

+  // CLANG-SAME: "-internal-isystem" "{{[^"]*}}/lib{{[^"]*}}/clang/{{[^"]*}}/include/cuda_wrappers"

+  // NOCLANG-NOT: "{{[^"]*}}/lib{{[^"]*}}/clang/{{[^"]*}}/include/cuda_wrappers"

+ -// HIP-SAME: "-internal-isystem" "{{[^"]*}}Inputs/rocm/include"

+ -// NOHIP-NOT: "{{.*}}Inputs/rocm/include"

+ +// HIP-SAME: "-idirafter" "{{[^"]*}}Inputs/rocm/include"

+  // HIP-SAME: "-include" "__clang_hip_runtime_wrapper.h"

+  // NOHIP-NOT: "-include" "__clang_hip_runtime_wrapper.h"

+  // skip check of standard C++ include path

+  // CLANG-SAME: "-internal-isystem" "{{[^"]*}}/lib{{[^"]*}}/clang/{{[^"]*}}/include"

+  // NOCLANG-NOT: "{{[^"]*}}/lib{{[^"]*}}/clang/{{[^"]*}}/include"

+ +// NOHIP-NOT: "{{.*}}Inputs/rocm/include"

+  

+  // COMMON-LABEL: "{{[^"]*}}clang{{[^"]*}}" "-cc1"

+  // CLANG-SAME: "-internal-isystem" "{{[^"]*}}/lib{{[^"]*}}/clang/{{[^"]*}}/include/cuda_wrappers"

+  // NOCLANG-NOT: "{{[^"]*}}/lib{{[^"]*}}/clang/{{[^"]*}}/include/cuda_wrappers"

+ -// HIP-SAME: "-internal-isystem" "{{[^"]*}}Inputs/rocm/include"

+ -// NOHIP-NOT: "{{.*}}Inputs/rocm/include"

+ +// HIP-SAME: "-idirafter" "{{[^"]*}}Inputs/rocm/include"

+  // HIP-SAME: "-include" "__clang_hip_runtime_wrapper.h"

+  // NOHIP-NOT: "-include" "__clang_hip_runtime_wrapper.h"

+  // skip check of standard C++ include path

+  // CLANG-SAME: "-internal-isystem" "{{[^"]*}}/lib{{[^"]*}}/clang/{{[^"]*}}/include"

+  // NOCLANG-NOT: "{{[^"]*}}/lib{{[^"]*}}/clang/{{[^"]*}}/include"

+ +// NOHIP-NOT: "{{.*}}Inputs/rocm/include"

+  

+  // RUN: %clang -c -### -target x86_64-unknown-linux-gnu --cuda-gpu-arch=gfx900 \

+  // RUN:   -std=c++11 --rocm-path=%S/Inputs/rocm -nogpulib %s 2>&1 \

+ @@ -45,7 +45,7 @@

+  // ROCM35-LABEL: "{{[^"]*}}clang{{[^"]*}}" "-cc1"

+  // ROCM35-NOT: "{{[^"]*}}/lib{{[^"]*}}/clang/{{[^"]*}}/include/cuda_wrappers"

+  // ROCM35-SAME: "-internal-isystem" "{{[^"]*}}/lib{{[^"]*}}/clang/{{[^"]*}}"

+ -// ROCM35-SAME: "-internal-isystem" "{{[^"]*}}Inputs/rocm/include"

+ +// ROCM35-SAME: "-idirafter" "{{[^"]*}}Inputs/rocm/include"

+  // ROCM35-NOT: "-include" "__clang_hip_runtime_wrapper.h"

+  // skip check of standard C++ include path

+  // ROCM35-SAME: "-internal-isystem" "{{[^"]*}}/lib{{[^"]*}}/clang/{{[^"]*}}/include"

+ diff --git a/clang/test/Driver/rocm-detect.hip b/clang/test/Driver/rocm-detect.hip

+ index 3ab0175cf1f1..7166c42e9691 100644

+ --- a/clang/test/Driver/rocm-detect.hip

+ +++ b/clang/test/Driver/rocm-detect.hip

+ @@ -90,7 +90,7 @@

+  // SPACK: Found HIP installation: [[DIR]]/hip-4.0.0-5f63slrursbrvfe2txrrjkynbsywsob5, version 4.0.20214-a2917cd

+  // SPACK: "-triple" "amdgcn-amd-amdhsa"

+  // SPACK-SAME: "-mlink-builtin-bitcode" "[[DIR]]/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/hip.bc"

+ -// SPACK-SAME: "-internal-isystem" "[[DIR]]/hip-4.0.0-5f63slrursbrvfe2txrrjkynbsywsob5/include"

+ +// SPACK-SAME: "-idirafter" "[[DIR]]/hip-4.0.0-5f63slrursbrvfe2txrrjkynbsywsob5/include"

+  

+  // SPACK-MULT: InstalledDir: [[DIR:.*]]/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin

+  // SPACK-MULT-DAG: Cannot use SPACK package hip-4.0.0 at [[DIR]] due to multiple installations for the same version

+ @@ -101,12 +101,12 @@

+  // SPACK-SET: Found HIP installation: [[DIR]]/hip-4.0.0-abcd, version 4.0.20214-a2917cd

+  // SPACK-SET: "-triple" "amdgcn-amd-amdhsa"

+  // SPACK-SET-SAME: "-mlink-builtin-bitcode" "[[DIR]]/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/hip.bc"

+ -// SPACK-SET-SAME: "-internal-isystem" "[[DIR]]/hip-4.0.0-abcd/include"

+ +// SPACK-SET-SAME: "-idirafter" "[[DIR]]/hip-4.0.0-abcd/include"

+  

+  // SPACK-MISS: InstalledDir: [[DIR:.*]]/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin

+  // SPACK-MISS-DAG: SPACK package hip-4.0.0 not found at [[DIR]]

+  // SPACK-MISS-NOT: Found HIP installation: [[DIR]]/hip-4.0.0-5f63slrursbrvfe2txrrjkynbsywsob5, version 4.0.20214-a2917cd

+ -// SPACK-MISS-NOT: "-internal-isystem" "[[DIR]]/hip-4.0.0-5f63slrursbrvfe2txrrjkynbsywsob5/include"

+ +// SPACK-MISS-NOT: "-idirafter" "[[DIR]]/hip-4.0.0-5f63slrursbrvfe2txrrjkynbsywsob5/include"

+  

+  // SPACK-MISS-SILENT-NOT: SPACK package hip-{{.*}} not found at

+  // SPACK-MISS-SILENT-NOT: Found HIP installation

+ -- 

+ 2.35.1

+ 

file modified
+6 -1
@@ -41,7 +41,7 @@ 

  

  Name:		%pkg_name

  Version:	%{clang_version}%{?rc_ver:~rc%{rc_ver}}

- Release:	1%{?dist}

+ Release:	2%{?dist}

  Summary:	A C language family front-end for LLVM

  

  License:	NCSA
@@ -68,6 +68,8 @@ 

  # https://github.com/llvm/llvm-project/commit/fed96f31bb5b68f77dd617ee8e698dd8171ee71b

  Patch6:     m-branch-protection.patch

  Patch7:     0010-PATCH-clang-Produce-DWARF4-by-default.patch

+ # This is cherry-picked from trunk and can be dropped in LLVM 15

+ Patch8:		0001-HIP-Fix-HIP-include-path.patch

  

  # Patches for clang-tools-extra

  # See https://reviews.llvm.org/D120301
@@ -595,6 +597,9 @@ 

  

  %endif

  %changelog

+ * Mon Apr 04 2022 Jeremy Newton <alexjnewt AT hotmail DOT com> - 14.0.0-2

+ - Add patch for HIP (cherry-picked from llvm trunk, to be LLVM15)

+ 

  * Wed Mar 23 2022 Timm Bäder <tbaeder@redhat.com> - 14.0.0-1

  - Update to 14.0.0

  

I would like to get rocm-hip packaged in Fedora, but it doesn't work due to a bug, which is fixed by this patch straight from LLVM's upstream trunk (main branch).

I hope it would be ok to cherry-pick this for Fedora instead of waiting 6 months for LLVM 15. Ideally, if anyone is able to convince upstream to cherry-pick it into the 14.x branch, that might be the best case.

rebased onto 646da251d220cc1f43e944cf625d4cfe573d91be

2 years ago

I have a scratch build:
https://koji.fedoraproject.org/koji/taskinfo?taskID=85181825

As of the moment of writing this, it looks good, but I'm waiting for aarch64 and s390x to finish.

Build succeeded.

rebased onto c00fbc7

2 years ago

Looks good, I don't mind this cherry-pick.

Pull-Request has been merged by sergesanspaille

2 years ago

Build succeeded.

FYI, I'm told this was merged into 14.0.4, so feel free to revert when updating to 14.0.4