[PATCH] D92954: [clang-offload-bundler] Add option -list

Artem Belevich via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 10 10:50:01 PST 2020


tra added inline comments.


================
Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:188
+
+      if (Error Err = Func())
+        return Err;
----------------
Now, if we could save the triple in a `BundleInfo` when it's parsed, and pass `BundleInfo` to `Func()` that would make the iterator more useful.



================
Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:320-321
       ReadChars += TripleSize;
+      if (ListBundleIDs)
+        llvm::outs() << Triple << '\n';
 
----------------
I'm still not quite happy with the fact that the listing is interleaving with reading the bundle.
I think the code would benefit from further refactoring that would separate bundle reading from what we do with the bundles we've read.



================
Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:641
+    // start of each bundle which is done by forEachBundle.
+    return forEachBundle(Input, []() { return Error::success(); });
+  }
----------------
Once BundleInfo carries the triple, `ListBundleIDs` could then be changed to print the bundle info and moved into the FileHandler class:

```
    if (Error Err = ReadHeader(Input))
        return Err;

    return forEachBundle(Input, [](BundleInfo bundle) { llvm::outs() << bundle.triple << '\n'});
```

All other printouts of the triple should no longer be necessary.


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

https://reviews.llvm.org/D92954



More information about the cfe-commits mailing list