[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: The well of confusion that is HIP_COMPILER



Hi Cory,

Cordell Bloor, on 2022-07-02:
> On 2022-07-02 09:05, Étienne Mollier wrote:
> > Cordell Bloor, on 2022-07-02:
> > > [1]: It's usable, but has sharp edges. The changes to HIP_COMPILER need to
> > > be removed from 0005-clang-14.patch; changing HIP_CXX_COMPILER should be
> > > sufficient. We also might want to patch --hip-version=<version> directly
> > > into the HIPCFLAGS / HIPCXXFLAGS in hipcc until we can figure out how to get
> > > that info to clang properly.
> > I agree there is room for improvement.  At the time of writing
> > that patch, clang-14 was not the default and it was the only way
> > I found to enable use of this particular version.  If I only
> > have clang-14 and corresponding toolset installed, then I don't
> > have the unversionned compiler commands, i.e. clang, clang++,
> > and llc, so the checks would fail if left unchanged.
> 
> I'm afraid you are mistaken. It's terribly confusing (and completely
> undocumented), but HIP_COMPILER is not used to store the name of the
> executable for compiling HIP programs. It is an enumeration that specifies
> the *type* of compiler being used. The actual executable name is stored in
> HIP_CXX_COMPILER [1].

Confusing indeed, I pushed your adjustments on the packaging
repository and verified builds of rocm-hipamd and rocrand
(without the debian patch) were running as expected.  Changes
should make it into the next upload of rocm-hipamd, once we have
feedback from ftpmaster on suitability of the package for the
archive.  I had some fuzz on the last hunk of your patch, but I
believe it is nothing.

> I don't think this makes a lot of sense, so I will try to start a
> conversation to see if the logic in HIP can be improved, but here's my
> understanding of how it works now:
> 
> HIP_COMPILER can have three values: "hcc", "clang" or "nvcc". The values
> "hcc" and "clang" always imply that the HIP platform is AMD, while the value
> of "nvcc" implies that the HIP platform is NVIDIA.
> 
> Frankly, this is bizarre. It seems to imply that if you are compiling for
> NVIDIA using clang, the value of HIP_COMPILER will be "nvcc". It's perhaps
> also worth noting that the hcc value is irrelevant these days. I would be
> very surprised if anything can still be built with hcc, as it was completely
> replaced by clang roughly two years ago.
> 
> In any case, I've attached a patch for rocm-hipamd that will allow you to
> drop cram-clang-14.patch from rocrand.
> 
> [1]: https://github.com/ROCmSoftwarePlatform/rocRAND/pull/274

Thanks for the fixes and clarifications, it's really
appreciated.  I'm also sorry for my confusion: I have grown
accustomed to various ways of separating the C from the C++
compiler in various packages, and ended up monkey-patching the
HIP_COMPILER as if they were a C variant of HIP_CXX_COMPILER.
I shouldn't trip on that carpet from now on.

Have a nice Sunday,  :)
-- 
Étienne Mollier <emollier@emlwks999.eu>
Fingerprint:  8f91 b227 c7d6 f2b1 948c  8236 793c f67e 8f0d 11da
Sent from /dev/pts/4, please excuse my verbosity.
On air: Rush - Yyz

Attachment: signature.asc
Description: PGP signature


Reply to: