[PATCH] D72806: [HIP] fix paths for executables not in clang bin directory

Artem Belevich via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 16 16:02:42 PST 2020


tra added a comment.

In D72806#1825400 <https://reviews.llvm.org/D72806#1825400>, @DieGoldeneEnte wrote:

>




> Even in case we don't add the extra directories for llvm and lld, it would be a good idea to use the getProgramPath function instead of building it manually (so only the changes in HIP.cpp except lines 273-282). This was planned anyways according to the comments in HIP.cpp line 270-271:
> 
>>   // Lookup binaries into the driver directory, this is used to
>>   // discover the clang-offload-bundler executable.

Agreed. GetProgramPath() change makes sense on its own merits.

In the future it may help to keep intependent changes in separate patches. It makes it easier to review and land them that way. I.e. the patch with GetProgramPath()  would be stamped w/o problems as it's an obvious clean up local to HIP only. Changing build for everyone will need more scrutiny and would be easier to deal with if it's not commingled with other changes.

It's not too late to split this patch in two. :-)


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

https://reviews.llvm.org/D72806





More information about the cfe-commits mailing list