[PATCH] D21462: [PM] Make the the new pass manageg support fully generic extra arguments to run methods, both for transform passes and analysis passes.

Sanjoy Das via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 19 01:17:27 PDT 2016


sanjoy added a subscriber: sanjoy.
sanjoy accepted this revision.
sanjoy added a reviewer: sanjoy.
sanjoy added a comment.
This revision is now accepted and ready to land.

lgtm with some minor questions / comments inline.


================
Comment at: include/llvm/IR/PassManager.h:970
@@ +969,3 @@
+/// extra arguments to the analysis manager's \c getResult routine. We can't
+/// guess how to effectively map the arguements from one to the other, and so
+/// only the specialization with no extra arguments is provided generically.
----------------
s/arguements/arguments/

================
Comment at: include/llvm/IR/PassManager.h:975
@@ +974,3 @@
+template <typename AnalysisT, typename IRUnitT>
+struct RequireAnalysisPass<AnalysisT, IRUnitT, AnalysisManager<IRUnitT>>
+    : PassInfoMixin<
----------------
Have you considered making this building block instead be:

```
template <typename AnalysisK, typename IRUnitT, typename... ExtraArgTs>
struct RequireAnalysisPass<AnalysisK<IRUnitT, ExtraArgTs>, IRUnitT, AnalysisManager<IRUnitT, ExtraArgTs>> {
  PreservedAnalyses run(IRUnitT &Arg, AnalysisManager<IRUnitT> &AM, ExtraArgTs... &&args) {
    (void)AM.template getResult<AnalysisT>(Arg, std::forward(args));

    return PreservedAnalyses::all();
  }
};
```

and use cases that want something other than `std::forward` can specialize this differently?

================
Comment at: unittests/IR/PassManagerTest.cpp:342
@@ +341,3 @@
+class CustomizedAnalysis : public AnalysisInfoMixin<CustomizedAnalysis> {
+public:
+  struct Result {
----------------
Minor: just make this a `struct`?


https://reviews.llvm.org/D21462





More information about the llvm-commits mailing list