[PATCH] D47555: [HIP] Fix unbundling

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 3 inline comments as done.
yaxunl added inline comments.


================
Comment at: lib/Driver/Driver.cpp:3895
+        if (UI.DependentOffloadKind == Action::OFK_Host)
+          Arch = StringRef();
+        else
----------------
tra wrote:
> Should it be something more descriptive? E.g. "all" or "combined".
> 
will use "all"


================
Comment at: test/Driver/hip-binding.hip:5-7
+// RUN: touch %t.o
+// RUN: %clang --hip-link -ccc-print-bindings -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %t.o\
----------------
tra wrote:
> Nit: you could probably just use /dev/null instead of a real temp file.
I tried /dev/null but I got 

error: no input files

I think we need a real input file here, even though an empty one.


================
Comment at: test/Driver/hip-binding.hip:11-12
+// CHECK: # "amdgcn-amd-amdhsa" - "offload bundler", inputs: ["[[IN:.*o]]"], outputs: ["[[OBJ1:.*o]]", "[[OBJ2:.*o]]", "[[OBJ3:.*o]]"] 
+// CHECK: # "amdgcn-amd-amdhsa" - "AMDGCN::Linker", inputs: ["[[OBJ2]]"], output: "[[IMG2:.*out]]"
+// CHECK: # "amdgcn-amd-amdhsa" - "AMDGCN::Linker", inputs: ["[[OBJ3]]"], output: "[[IMG3:.*out]]"
+// CHECK-NOT: offload bundler
----------------
tra wrote:
> There's currently a bundler invocation in-between these two linker commands. 
> I think you need a negative check for bundler here, too.
will do


https://reviews.llvm.org/D47555