libCEED icon indicating copy to clipboard operation
libCEED copied to clipboard

Guard hipblas header include for HIP_VERSION

Open jeremylt opened this issue 1 year ago • 1 comments

jeremylt avatar Aug 16 '22 20:08 jeremylt

LGTM. Is there a reason for the parentheses? We don't usually use them around compiler #if checks, do we?

I still see a lot of deprecated warnings on basic HIP stuff, like hip_runtime.h. It seems to be because hipconfig -C, which we use to get the flags for including HIP stuff with non-hipcc compilers, lists -I/opt/rocm-5.2.0/hip/include before -I/opt/rocm-5.2.0/include, so it's looking in the deprecated directory first. Do you see this locally?

I'm not sure if it's worth fixing, since I'd assume when the deprecated directory goes away, hipconfig will no longer list it...right?

nbeams avatar Aug 16 '22 22:08 nbeams

I don't see those warnings locally, so I think/hope the issue isn't present if you have only 5.2+ installed?

jeremylt avatar Aug 17 '22 16:08 jeremylt

Can you check the output of hipconfig -C on your machine?

nbeams avatar Aug 17 '22 16:08 nbeams

[jeremy@sinensis ~]$ hipconfig -C
 -D__HIP_PLATFORM_HCC__= -D__HIP_PLATFORM_AMD__= -I/opt/rocm/include -I/opt/rocm/llvm/bin/../lib/clang/14.0.0 -I/opt/rocm/hsa/include[jeremy@sinensis ~]$

jeremylt avatar Aug 17 '22 16:08 jeremylt

Do you have 5.2.1? It would be great if that issue is already fixed. Then I would definitely say we shouldn't worry about it, since it would only pop up with 5.2.0.

nbeams avatar Aug 17 '22 16:08 nbeams

5.2.2 I think

jeremylt avatar Aug 17 '22 16:08 jeremylt

Eh, close enough, I say 👍 Even if it's only 5.2.0 and 5.2.1, I think it's not a problem. (And of course, it could still be only 5.2.0 since neither of us currently has tested 5.2.1.)

nbeams avatar Aug 17 '22 16:08 nbeams