[PATCH] D21851: [Driver][OpenMP][CUDA] Add capability to bundle object files in sections of the host binary format.

[Samuel Antao via cfe-commits]

sfantao added a comment.

Hi Alexey,

Thanks for the review!


================
Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:345
@@ +344,3 @@
+  /// return the triple by reference.
+  bool isOffloadSection(SectionRef CurSection, StringRef &OffloadTriple) {
+    StringRef SectionName;
----------------
ABataev wrote:
> 'const'? or 'static'?
It can be static.

================
Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:450
@@ +449,3 @@
+                                     BitcodeFileName))
+      llvm_unreachable("Error trying to create temporary file!");
+
----------------
ABataev wrote:
> Not sure that this a good solution to crash the compiler in this case. I think it must exit gracefully.
Ok, replaced the unreachable code with logic to emit an error and exist the tool.

================
Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:468
@@ +467,3 @@
+      sys::fs::remove(BitcodeFileName);
+      llvm_unreachable("Can't find clang in path!");
+    }
----------------
ABataev wrote:
> Again, maybe just emit an error an exit?
Emitting an error now.


http://reviews.llvm.org/D21851