[PATCH] D59321: AMDGPU: Teach toolchain to link rocm device libs

Michael Liao via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 1 11:49:14 PDT 2020


hliao added a comment.

Do we have a better way to avoid adding those empty bitcode files?



================
Comment at: clang/lib/Driver/ToolChains/AMDGPU.h:29
+  struct ConditionalLibrary {
+    SmallString<0> On;
+    SmallString<0> Off;
----------------
may need to add `llvm` namespace prefix just like all other LLVM stuffs used in clang in case the same name is introduced later by accident.


================
Comment at: clang/lib/Driver/ToolChains/AMDGPU.h:44-46
+  //RocmVersion Version = RocmVersion::UNKNOWN;
+  SmallString<0> InstallPath;
+  //SmallString<0> BinPath;
----------------
sounds to me that both `Version` and `BinPath` should be added. They will be used eventually.


================
Comment at: clang/lib/Driver/ToolChains/HIP.h:76
 
-class LLVM_LIBRARY_VISIBILITY HIPToolChain final : public AMDGPUToolChain {
+class LLVM_LIBRARY_VISIBILITY HIPToolChain final : public ROCMToolChain {
 public:
----------------
Do you miss the change in HIP.cpp? That constructor needs revising as the base class is changed.


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

https://reviews.llvm.org/D59321





More information about the cfe-commits mailing list