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

Alexey Bataev via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 29 20:48:17 PDT 2016


ABataev added inline comments.

================
Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:324-334
@@ -314,1 +323,13 @@
 
+// Handler for object files. The bundles are organized by sections with a
+// designated name.
+//
+// In order to bundle we create an IR file with the content of each section and
+// use incremental linking to produce the resulting object. We also add section
+// with a single byte to state the name of the component the main object file
+// (the one we are bundling into) refers to.
+//
+// To unbundle, we use just copy the contents of the designated section. If the
+// requested bundle refer to the main object file, we just copy it with no
+// changes.
+class ObjectFileHandler : public FileHandler {
----------------
3 slashes, please

================
Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:335
@@ +334,3 @@
+// changes.
+class ObjectFileHandler : public FileHandler {
+
----------------
'final'?

================
Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:337
@@ +336,3 @@
+
+  /// \brief The object file we are currently dealing with.
+  ObjectFile &Obj;
----------------
No '\brief's

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

================
Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:450
@@ +449,3 @@
+                                     BitcodeFileName))
+      llvm_unreachable("Error trying to create temporary file!");
+
----------------
Not sure that this a good solution to crash the compiler in this case. I think it must exit gracefully.

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


http://reviews.llvm.org/D21851





More information about the cfe-commits mailing list