arsenm added a comment.

In D109885#3198340 <https://reviews.llvm.org/D109885#3198340>, @JonChesterfield 
wrote:

> In D109885#3198296 <https://reviews.llvm.org/D109885#3198296>, @arsenm wrote:
>
>> This isn't a feature, it's the introduction of a bug.
>
> It would regress people depending on the implicit pickup of /opt/rocm, 
> leaving them with a straightforward workaround of setting the cmake variable.

Exactly. Builds that don't find their dependencies in the default location 
without doing anything are bad builds. It adds extra work and discovery a 
casual builder doesn't actually care about.

> The inverse, where we look in /opt/rocm unless that's overridden (perhaps by 
> the empty string), achieves much the same effect without breaking anyone 
> using trunk with a rocm install.

Exactly, this is how the search process is supposed to work. HINTS are at the 
bottom of the search hierarchy. The mechanisms for finding a specific version 
win out over the basic hint to the default

> I'm very mistrusting of mixing a rocm toolchain with a trunk toolchain as 
> they're both quite keen on runpath and LD_LIBRARY_PATH to find internal 
> components. That makes it very easy to accidentally mix the two together into 
> something that doesn't work so personal preference is to rip out the 
> /opt/rocm search path for HSA entirely and encourage people to build it from 
> source.

There are a number of cmake crimes going on in both of these, but this isn't 
one of them. LD_LIBRARY_PATH should not be used for any builds to find 
components.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109885

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

Reply via email to