Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Reassociation messes up SLPVectorizer reduction #35454

Open Quuxplusone opened 6 years ago

Quuxplusone commented 6 years ago
Bugzilla Link PR36481
Status CONFIRMED
Importance P enhancement
Reported by Tim Shen (timshen91@gmail.com)
Reported on 2018-02-22 15:41:18 -0800
Last modified on 2018-02-23 12:44:21 -0800
Version trunk
Hardware PC Linux
CC a.bataev@hotmail.com, arnold.schwaighofer@gmail.com, echristo@gmail.com, i@maskray.me, llvm-bugs@lists.llvm.org, llvm-dev@redking.me.uk, spatel+llvm@rotateright.com
Fixed by commit(s)
Attachments c.ll (3541 bytes, text/plain)
Blocks
Blocked by
See also

Created attachment 19931 Test

Running opt -S -slp-vectorizer $TEST_FILE, the function gets SLP-vectorized; Running opt -S -reassociate -slp-vectorizer $TEST_FILE, the function doesn't get SLP-vectorized.

This is because the reassociation pass re-associates instructions like acc = add ith_element, acc to acc = add acc, ith_element, but didn't re-assocate the first instruction acc = add first_element, second_element to acc = add second_element, first_element.

However, as add is associative, a smarter SLP vectorizer could have ignored the associations and vectorize the unordered adds anyway.

I'm not sure where to put the fix on.

Quuxplusone commented 6 years ago

Attached c.ll (3541 bytes, text/plain): Test

Quuxplusone commented 6 years ago

CC Arnold because he authored HorizontalReduction::matchAssociativeReduction().

Quuxplusone commented 6 years ago

(In reply to Tim Shen from comment #0)

but didn't re-assocate the first instruction acc = add first_element, second_element to acc = add second_element, first_element.

It should be phrased the other way around: but didn't re-assocate the first instruction acc = add second_elemnet, first_element to acc = add first_element, second_element.

As a result, the add sequence is v[1] + v[0] + v[2] + ... + v[31], which doesn't get vectorized.