]> granicus.if.org Git - llvm/commit
Merging r296003:
authorHans Wennborg <hans@hanshq.net>
Fri, 24 Feb 2017 18:53:54 +0000 (18:53 +0000)
committerHans Wennborg <hans@hanshq.net>
Fri, 24 Feb 2017 18:53:54 +0000 (18:53 +0000)
commit3ddbfabbacd30161abb6fb27e716f37abf1ad795
treefea64d00b8343c3fadc9f36debb917306473a8c3
parent96c0db119bcb6b9d6a4ed3eb95d0b992c086aef0
Merging r296003:
------------------------------------------------------------------------
r296003 | mcrosier | 2017-02-23 10:49:03 -0800 (Thu, 23 Feb 2017) | 32 lines

[Reassociate] Add negated value of negative constant to the Duplicates list.

In OptimizeAdd, we scan the operand list to see if there are any common factors
between operands that can be factored out to reduce the number of multiplies
(e.g., 'A*A+A*B*C+D' -> 'A*(A+B*C)+D'). For each operand of the operand list, we
only consider unique factors (which is tracked by the Duplicate set). Now if we
find a factor that is a negative constant, we add the negated value as a factor
as well, because we can percolate the negate out. However, we mistakenly don't
add this negated constant to the Duplicates set.

Consider the expression A*2*-2 + B. Obviously, nothing to factor.

For the added value A*2*-2 we over count 2 as a factor without this change,
which causes the assert reported in PR30256.  The problem is that this code is
assuming that all the multiply operands of the add are already reassociated.
This change avoids the issue by making OptimizeAdd tolerate multiplies which
haven't been completely optimized; this sort of works, but we're doing wasted
work: we'll end up revisiting the add later anyway.

Another possible approach would be to enforce RPO iteration order more strongly.
If we have RedoInsts, we process them immediately in RPO order, rather than
waiting until we've finished processing the whole function. Intuitively, it
seems like the natural approach: reassociation works on expression trees, so
the optimization only works in one direction. That said, I'm not sure how
practical that is given the current Reassociate; the "optimal" form for an
expression depends on its use list (see all the uses of "user_back()"), so
Reassociate is really an iterative optimization of sorts, so any changes here
would probably get messy.

PR30256

Differential Revision: https://reviews.llvm.org/D30228
------------------------------------------------------------------------

git-svn-id: https://llvm.org/svn/llvm-project/llvm/branches/release_40@296156 91177308-0d34-0410-b5e6-96231b3b80d8
lib/Transforms/Scalar/Reassociate.cpp
test/Transforms/Reassociate/basictest.ll