[PATCH] D13909: clang-offload-bundler - offload files bundling/unbundling tool

[Samuel Antao via cfe-commits]

sfantao added a comment.

Hi Alexey,

Thanks for the review!


================
Comment at: test/Driver/clang-offload-bundler.c:14
@@ +13,3 @@
+//
+// RUN: touch %t.empty
+
----------------
ABataev wrote:
> Hmm, will it work on Windows? Maybe it is better just to add an empty test file?
I see many other tests in clang using it. There used to be a directive `//requires-shell` but I don't think that is being used now. Should I go ahead and create an empty file? Unfortunately, it is not easy for me to try these things on Windows.

================
Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:477-478
@@ +476,4 @@
+  auto Input = InputBuffers.begin();
+  for (auto Triple = TargetNames.begin(); Triple < TargetNames.end();
+       ++Triple, ++Input) {
+    FH.get()->WriteBundleStart(OutputFile, *Triple);
----------------
ABataev wrote:
> for (auto &Triple : TargetNames)
> 
Ok, moved the iterator `Input` to the end of the loop body.

================
Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:513-514
@@ +512,4 @@
+  auto Output = OutputFileNames.begin();
+  for (auto Triple = TargetNames.begin(); Triple < TargetNames.end();
+       ++Triple, ++Output)
+    Worklist[*Triple] = *Output;
----------------
ABataev wrote:
> for (auto &Triple : TargetNames)
Ok, moved `Output` iterator to the bottom of the loop body.


http://reviews.llvm.org/D13909