[PATCH] D93525: [clang-offload-bundler] Add unbundling of archives containing bundled object files into device specific archives

[Saiyedul Islam via Phabricator via cfe-commits]

saiislam added inline comments.


================
Comment at: clang/docs/ClangOffloadBundler.rst:128
+
+  <Architecture>-<Vendor>-<OS>-<Environment>
+
----------------
grokos wrote:
> A bit of wordplay, but it's weird that a *triple* now has 4 elements...
I think llvm::Triple it is named Triple because of historical reasons. Otherwise, it already has these components (including the environment).
As llvm::Triple API's don't force presence of all components it is not an issue in most cases, but in our case of Bundle Entry ID we need a way to differentiate between 4th component of Triple and a GPU arch, hence this rule.


================
Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:147
+    Target.split(Components, '-', 5);
+    Components.resize(6);
+    this->OffloadKind = Components[0];
----------------
grokos wrote:
> Leftover? `Components` is already 6 elements long.
Not necessarily. It is possible that target has less than 6 elements. For example all bundling/unbundling cases which do not require GPUArch field.
E.g. "openmp-powerpc64le-ibm-linux-gnu"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93525