[llvm-commits] patch: chrec * chrec = chrec.

Andrew Trick atrick at apple.com
Mon Oct 3 15:57:09 PDT 2011


On Oct 2, 2011, at 4:50 PM, Nick Lewycky wrote:
> This patch is a follow-up to r139133 which corrected SCEV's getMulExpr when given two chrec arguments. Here I extend it to correctly multiply chrecs of any length.
> 
> The meat of this patch is a pure blob of illegible math. If you want to do a hard review, I would suggest:
> - trying to derive the formula I used in the comment to calculate the terms of the new chrec
> - proving to yourself a coefficient that doesn't fit can safely be truncated
> 
> If you want to do an easier review, I would suggest running it over code. This will fire in 447.dealII for example.
> 
> I know that other work is going on in the loop optimizer right now which I don't want to interfere with, so please let me know when wouldn't be a bad time to land this.
> 
> Nick
> <scev-chrec-product.patch>

Nick,

I don't have a problem with you landing this whenever you can. As long as I don't have to vouch for correctness of the math. More comments would help in this regard. Not being a math major, it wasn't obvious to me that you're computing binomial coefficients here.

The reference to a benchmark is quite helpful, and the unit tests give me a good feeling. I'm aware of the problem with verifying SCEV equivalence. I would love to an equivalence checker that can run in assert mode, but it's pretty low on my todo list now.

-Andy



More information about the llvm-commits mailing list