This is an archive of the discontinued LLVM Phabricator instance.

[AArch64]Fix bug in DAG combine for ADDV
AbandonedPublic

Authored by junbuml on Oct 5 2015, 1:23 PM.

Details

Summary

Added more constraint when checking the number of vector elements during DAG combine for across vector reductions.

This fix PR25056.

Diff Detail

Event Timeline

junbuml updated this revision to Diff 36543.Oct 5 2015, 1:23 PM
junbuml retitled this revision from to [AArch64]Fix bug in DAG combine for ADDV.
junbuml updated this object.
junbuml added reviewers: jmolloy, mcrosier, chatur01.
junbuml added a subscriber: llvm-commits.
jmolloy requested changes to this revision.Oct 5 2015, 1:29 PM
jmolloy edited edge metadata.

Hi Jun,

I don't think this is the right fix. We generate oversized vectors in the loop vectorizer, and we expect them to work well. The right way to solve this PR is by implementing vector splitting for the [US]ADDV and [SU][MIN,MAX]V nodes.

Cheers,

James

This revision now requires changes to proceed.Oct 5 2015, 1:29 PM

Do you mean combining for oversized input vector for ADDV could be combined, but it should be legalized after then by splitting the vector ?

Yes. This occurs for all other ISD nodes, see what happens for example if
you try a v8i32 ADD.

mcrosier edited edge metadata.Oct 6 2015, 9:34 AM

Jun,
In case you haven't seen it Charlie proposed a fix for PR25056 here: http://reviews.llvm.org/D13466.

Chad

Yes, looks like D13466 by Charlie tries to do what you mentioned.

junbuml abandoned this revision.Oct 12 2015, 8:43 AM