[PATCH] D14481: [SCEV] Simplify adds to a canonical sum of product
Andrew Trick via llvm-commits
llvm-commits at lists.llvm.org
Sun Dec 27 12:53:23 PST 2015
atrick added a reviewer: mzolotukhin.
atrick added a comment.
Nice little SumOfProduct utility!
Exponential recursion needs to be prevented. Even without the simplification step, the visitor is exponential. Unlike normal SCEV construction, the recursive visitor is not memoizing results.
I guess the simplification is inherently exponential. I don't know how to avoid that without limitting size of the product. It does look like excess SCEVMulExprs may be created (for unit scale).
If you want to go forward with this, I would realy want to see motivating examples shown in the form of symbolic expressions. I couldn't understand how the utility worked without visualizing examples. It's not completely clear to my why this can't be done in place as a bottom-up SCEV canonicalization/simplification. I would need to work through some examples to understand the issue.
I guess the final question is wheter to use this as the canonical SCEV form rather an an on-the-side simplification. I would be fine with that if the Expander knew how to minimize multiplication by factoring terms.
Adding Michael Z. since I can't really do timely SCEV reviews.
================
Comment at: lib/Analysis/ScalarEvolution.cpp:1848
@@ +1847,3 @@
+class SumOfProduct {
+ DenseMap<const SCEV *, APInt> Terms;
+ unsigned BitWidth;
----------------
Did you consider SmallPtrSet?
http://reviews.llvm.org/D14481
More information about the llvm-commits
mailing list