[llvm-commits] [PATCH] fold umax(zext A, zext B) -> zext (umax(A, B))

[Nick Lewycky]

2009/6/17 Török Edwin <edwintorok at gmail.com>

> On 2009-06-17 22:55, Nick Lewycky wrote:
> > 2009/6/17 Török Edwin <edwintorok at gmail.com <mailto:edwintorok at gmail.com
> >>
> >
> >     Hi,
> >
> >     I noticed that umax (zext t1 %X to %t0, zext t1 %Y to %t0) isn't
> >     folded,
> >     the attached patch folds this into:
> >     umax (zext t1 %X to %t0, zext t1 %Y to %t0).
> >
> >     It also folds umax (sext t1 %X to %t0, sext t1 %Y to %t0) ->  sext t1
> >     (umax (%X, %Y)) to %t0.
> >
> >
> > Unless I'm mistaken, we deliberately push them the other way so that
> > we can eliminate ext(constant) inside of the umax. Also, since umax
> > can take multiple operands we get to canonicalize "umax(zext(umax(a,
> > b)), c)" into "umax(zext(a), zext(b), c)".
> >
> > There's an ordering defined in ScalarEvolutionExpressions.h
> > (scConstant to scUnknown) which should match the inside-to-outside
> > order that we want SCEVs to have when canonicalizing.
>
> Ok, this is the wrong approach then, since I could possibly infloop if
> SCEV prefers the original ordering.
>

Exactly. If your patch doesn't cause infinite loops it might mean that the
zext(umax) optimization never happens. Checking this would be a good sanity
check.

 >     zext is very often encountered in SCEV expressions on x86-64, since
> >     pointer indexes for GEP are i64.
> >
> >     Thoughts?
> >
> >
> > This is a real problem that deserves a fix, I'm just not sure I like
> > the one proposed. How hard would it be to add logic to detect this
> > case to the ScalarEvolutionExpander? Just look inside the umax and see
> > whether all its operands contains the same zext/sext/trunc and hoist
> > it then.
>
> Yes, that would be possible. However I would prefer a utility function
> is ScalarEvolution that given a SCEV* transforms it into a SCEV* with
> minimal operations.
> ScalarEvolutionExpander could use that, and so could other clients of
> ScalarEvolution.
>
> As initial implementation it could transform OP1(OP2(a1), OP2(a2), ...,
> OP2(aN)) -> OP2(OP1(a1, ..., aN)).
> Besides umax for OP1, and zext/sext for OP2 is there any other situation
> where this transform would be valid?


OP1 = smax and OP2 = sext. I think that's it.

The other transformations SEE wants to do can't be easily represented with
SCEVs, like mul(x, -1) to xor(x, -1) and add(x, 4) to or(x, 4).

Nick
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20090617/a4cad688/attachment.html>