[PATCH] D122485: [SimplifyCFG] Fold mutil cases to And mask

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:5646
 
-    // Early out if there are too many cases for this result.
-    if (NumCasesForResult > MaxCasesPerResult)
----------------
spatel wrote:
> bcl5980 wrote:
> > @spatel I'm a little worry about here. I remove the early out this version. It will cause compile time increase if we have some very large switch with many cases to the same result but not the pattern we can fold. But I have no detail data to show how much extra compile this change will be involved. Do we have some common compile time tests?
> > Another way to do is enlarge MaxCasesPerResult. The patch first version adjust MaxCasesPerResult to 16. But 16 is also a magic number. Maybe we can add a config for it.
> > How about your suggestions?
> > 
> @nikic has a system that is used to check for compile time changes with benchmarks from the test suite:
> https://llvm-compile-time-tracker.com/index.php
> 
> So we can run an experiment there.
> 
> But I think it is fine to use "16" as a limit for this analysis. You can give it a name and make it a debug option like the many others at the top of this file. For example:
> 
> ```
> static cl::opt<unsigned> MaxSpeculationDepth(
>     "max-speculation-depth", cl::Hidden, cl::init(10),
>     cl::desc("Limit maximum recursion depth when calculating costs of "
>              "speculatively executed instructions"));
> ```
We can reduce this patch by doing that part independently:
D123625


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

https://reviews.llvm.org/D122485