<div class="gmail_quote">2009/6/17 Török Edwin <span dir="ltr"><<a href="mailto:edwintorok@gmail.com">edwintorok@gmail.com</a>></span><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

On 2009-06-17 22:55, Nick Lewycky wrote:<br>
> 2009/6/17 Török Edwin <<a href="mailto:edwintorok@gmail.com">edwintorok@gmail.com</a> <mailto:<a href="mailto:edwintorok@gmail.com">edwintorok@gmail.com</a>>><br>
<div class="im">><br>
>     Hi,<br>
><br>
>     I noticed that umax (zext t1 %X to %t0, zext t1 %Y to %t0) isn't<br>
>     folded,<br>
>     the attached patch folds this into:<br>
>     umax (zext t1 %X to %t0, zext t1 %Y to %t0).<br>
><br>
>     It also folds umax (sext t1 %X to %t0, sext t1 %Y to %t0) ->  sext t1<br>
>     (umax (%X, %Y)) to %t0.<br>
><br>
><br>
> Unless I'm mistaken, we deliberately push them the other way so that<br>
> we can eliminate ext(constant) inside of the umax. Also, since umax<br>
> can take multiple operands we get to canonicalize "umax(zext(umax(a,<br>
> b)), c)" into "umax(zext(a), zext(b), c)".<br>
><br>
> There's an ordering defined in ScalarEvolutionExpressions.h<br>
> (scConstant to scUnknown) which should match the inside-to-outside<br>
> order that we want SCEVs to have when canonicalizing.<br>
<br>
</div>Ok, this is the wrong approach then, since I could possibly infloop if<br>
SCEV prefers the original ordering.<br>
<div class="im"></div></blockquote><div><br>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.<br><br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

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

<br>Nick</div></div><br>