[PATCH] D38631: [SimplifyCFG] use pass options and remove the latesimplifycfg pass

Hal Finkel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 27 12:27:15 PDT 2017


hfinkel added a comment.

In https://reviews.llvm.org/D38631#909235, @spatel wrote:

> Patch updated:
>
> 1. Keep default SimplifyCFGPass constructor and use it where appropriate (reminder: this patch is NFCI, so I preserved the non-default instantiations in the new PM with one FIXME comment).


Okay. I understand that you want to switch the defaults in a follow-up patch. Why, however, don't you make `SimplifyCFGOptions() .forwardSwitchCondToPhi(true).convertSwitchToLookupTable(true).needCanonicalLoops(false)` the default, and then change the default in a follow-up patch?

> 2. Add methods to SimplifyCFGOptions to allow builder constructor pattern which provide easier-to-read instantiations of SimplifyCFGPass (please make sure I did that correctly because I've never tried that before!).





================
Comment at: include/llvm/Transforms/Scalar.h:263
+    bool ConvertSwitch = false, bool KeepLoops = true,
+    std::function<bool(const Function &)> Ftor = nullptr);
 
----------------
spatel wrote:
> bmakam wrote:
> > We still pass boolean and values to createCFGSimplificationPass(1, false, false, true) Don't we need builder API here as well?
> This is only used by the legacy PM unless I'm not seeing it correctly. Fixing this will require more futzing, possibly adding includes for the header or moving the struct definition from its current location. So I was hoping that we'd let that go given that the new PM seems relatively close to becoming default.
Okay (although this is slightly suboptimal for the backends which I imagine will use the legacy pass manager longer).


https://reviews.llvm.org/D38631





More information about the llvm-commits mailing list