[PATCH] D145393: [HIP] Make `--offload-add-rpath` alias of `-frtlib-add-rpath`

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D145393#4172429 <https://reviews.llvm.org/D145393#4172429>, @MaskRay wrote:

> Seems fine. Should we eventually remove `--offload-add-rpath` and `-fopenmp-implicit-rpath`?

I agree we should eventually remove them and keep -frtlib-add-rpath only.



================
Comment at: clang/include/clang/Driver/Options.td:4257
+  HelpText<"Add -rpath with architecture-specific resource directory to the linker flags. "
+  "When --hip-link is specified, also add -rpath with HIP runtime library directory to the linker flags">;
 def fno_rtlib_add_rpath: Flag<["-"], "fno-rtlib-add-rpath">, Flags<[NoArgumentUnused]>,
----------------
tra wrote:
> I'm not sure these HIP-specific details are needed here.
> It may be better to generalize the generic description along the lines of "adds required architecture-specific directories to RPATH".
currently, it only adds compiler-rt path and HIP runtime path to RPATH. Making the help message generic for all language runtime will cause an incorrect impression to the users that this option adds all language runtime lib paths to RPATH but it actually does not.


================
Comment at: clang/test/Driver/hip-runtime-libs-linux.hip:16
 // RUN: %clang -### --hip-link --target=x86_64-linux-gnu \
-// RUN:   --rocm-path=%S/Inputs/rocm %t.o --offload-add-rpath 2>&1 \
+// RUN:   --rocm-path=%S/Inputs/rocm %t.o -frtlib-add-rpath 2>&1 \
 // RUN:   | FileCheck -check-prefixes=ROCM-RPATH %s
----------------
tra wrote:
> I think you may still want to test with `--offload-add-rpath`, too.
> 
will do


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

https://reviews.llvm.org/D145393