This is an archive of the discontinued LLVM Phabricator instance.

[DAGCombiner] Don't reassociate operations that have the vector reduction flag set.
ClosedPublic

Authored by craig.topper on Aug 15 2018, 5:21 PM.

Details

Summary

When nodes are reassociated the vector-reduction flag gets lost.

The test case is here is what would happen if you had a sum of absolute differences loop that started with a non-zero but contant sum and that loop was unrolled. The vectorizer will generate a constant vector for the initial value. And DAGCombiner reassociate tries to move it down the addition tree erasing the vector-reduction flag. Interestingly this moves constants the opposite direction of the reassociate IR pass.

I've chosen to just punt on the reassociate, but I suppose we could maybe preserve the flag if both nodes have it set.

Diff Detail

Repository
rL LLVM

Event Timeline

craig.topper created this revision.Aug 15 2018, 5:21 PM
spatel added inline comments.Aug 16 2018, 5:07 AM
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
953–954 ↗(On Diff #160949)

Not sure if this is possible: what if the root node and N1 do not have the reduction flag set? In that case, we want to allow the 'if' clause at line 975 to fire even though N0 has the reduction flag set rather than bailing out?

test/CodeGen/X86/sad.ll
1401 ↗(On Diff #160949)

Please commit the test with baseline checks as a preliminary step.

craig.topper added inline comments.Aug 16 2018, 11:09 AM
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
953–954 ↗(On Diff #160949)

Oops. I'll fix that.

Rebase and fix review comment

spatel accepted this revision.Aug 16 2018, 2:27 PM

LGTM

This revision is now accepted and ready to land.Aug 16 2018, 2:27 PM
This revision was automatically updated to reflect the committed changes.