[PATCH] D47555: [HIP] Fix unbundling

[Artem Belevich via Phabricator via cfe-commits]

tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.

Few minor nits/suggestions. LGTM otherwise.



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



================
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\
----------------
Nit: you could probably just use /dev/null instead of a real temp file.


================
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
----------------
There's currently a bundler invocation in-between these two linker commands. 
I think you need a negative check for bundler here, too.


https://reviews.llvm.org/D47555