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

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


ABataev added a comment.

Remove empty 'return' and ';' statements where they are not required.


================
Comment at: test/Driver/clang-offload-bundler.c:14
@@ +13,3 @@
+//
+// RUN: touch %t.empty
+
----------------
Hmm, will it work on Windows? Maybe it is better just to add an empty test file?

================
Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:174
@@ +173,3 @@
+  /// \brief Information about the bundles extracted from the header.
+  struct BundleInfo {
+    /// \brief Size of the bundle.
----------------
1. 'final'
2. Default field initializers instead of default constructor

================
Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:304-305
@@ +303,4 @@
+  }
+  void WriteBundleStart(raw_fd_ostream &OS, StringRef TargetTriple) { return; }
+  void WriteBundleEnd(raw_fd_ostream &OS, StringRef TargetTriple) { return; }
+  void WriteBundle(raw_fd_ostream &OS, MemoryBuffer &Input) {
----------------
remove empty 'return's

================
Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:308
@@ +307,3 @@
+    OS.write(Input.getBufferStart(), Input.getBufferSize());
+    return;
+  }
----------------
Remove it

================
Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:335
@@ +334,3 @@
+  /// \brief Number of chars read from input.
+  size_t ReadChars;
+
----------------
Default initializer

================
Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:453
@@ +452,3 @@
+  unsigned Idx = 0;
+  for (auto I : InputFileNames) {
+    ErrorOr<std::unique_ptr<MemoryBuffer>> CodeOrErr =
----------------
auto &File instead of auto I?

================
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);
----------------
for (auto &Triple : TargetNames)


================
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;
----------------
for (auto &Triple : TargetNames)

================
Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:677-680
@@ +676,6 @@
+
+  if (Unbundle)
+    return UnbundleFiles();
+  else
+    return BundleFiles();
+
----------------
Maybe 'return Unbundle ? UnbundleFiles() : BundleFiles();'?


http://reviews.llvm.org/D13909





More information about the cfe-commits mailing list