[PATCH] Reassociate in favor of grouping previously paired operands

[Xuetian Weng]

On Mon, Jun 8, 2015 at 4:54 PM, Daniel Berlin <dberlin at dberlin.org> wrote:
>
> On Mon, Jun 8, 2015 at 4:23 PM, Xuetian Weng <xweng at google.com> wrote:
> > Related discussion and original patch on llvmdev.
> >
> > http://lists.cs.uiuc.edu/pipermail/llvmdev/2015-May/085246.html
> >
> > There are some differences between the original patch and this version:
> >
> > 1. Because OptimizeExpression has some assumptions about the order of linearized ops, in this version the order of operands is adjusted after calling function OptimizeExpression.
> > 2. Because RewriteExprTree can only pair the last two operands in Ops, in this patch the order of operands is not adjusted by rank but by simply moving one pair of operands to the end of Ops.
>
> This seems a bit more dangerous?
> In effect, you are changing the rank.  Tying this to rewritexprtree's
> current implementation seems like asking for bugs in the fuure

Actually it's less dangerous after tried several way to rank the operands on
the existing test case. In this patch, operands will be ranked with
their original
rank first. OptimizeExpression would do whatever old optimization it want like
constant combine and factorization. If we try to give them new rank directly
you would surprisingly find all other original optimizations in
Reassociate are broken.

For example,
b = a + 2
c = b - 2

c will not be eliminated because a and 2 are preferred to be paired together.

There's not much thing to be done between OptmizeExpression and
RewriteExprTree, so it's safe to reorder the optimized operands again there.

As for tying this to rewriteexprtree's implementation, the rank system itself
is tied to rewritexprtree's implementation. And we cannot implement this feature
without the knowledege of implementation of rewriteexprtree. For example, if
we already know that rewriteexprtree can only dump expression in the form of
left linearization, there would be no point to put more than one pair
of operands
together.

> > 3. Paired operands are indexed with their corresponding Opcode and Instruction. So (add a, b) will not make a, b together in a multiplication expression.
>
> What is the downside to trying to keep operands together that appear
> together in some other operation?
>
> By doing this, you are effectively giving an operand different ranks
> in different operations.
>
For example,
e  =  a | b
foo(e)
...
f = b + c
foo(f)
...
x = a + b
y = x + c
foo(y)

Pairing a,b together for y is pointless and would lose opportunity to
pair b,c together.

> > 7. Last pairing map is cleared after processing every basic block.
>
> Why?
> This just seems wrong?
To avoid dominance issue discussed here. For now I prefer to keep it simple
and conservative. Optimization related to dominance can be improved later.
http://lists.cs.uiuc.edu/pipermail/llvmdev/2015-May/085437.html
http://lists.cs.uiuc.edu/pipermail/llvmdev/2015-May/085441.html