This is an archive of the discontinued LLVM Phabricator instance.

[SLP] Don't vectorize div/rem with undef denominators
Needs ReviewPublic

Authored by RKSimon on Dec 8 2021, 1:26 PM.

Details

Summary

If any of the elements contains a DIV/REM node that is likely to fold to undef, then don't vectorize with it as it'll poison the entire vector.

I'm not certain if this IR can actually occur going into SLP or not, but we have tests for it, and I encountered further cases while altering the costs to try and fix the regressions in D111968.

Diff Detail

Event Timeline

RKSimon created this revision.Dec 8 2021, 1:26 PM
RKSimon requested review of this revision.Dec 8 2021, 1:26 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 8 2021, 1:26 PM
lebedev.ri requested changes to this revision.Dec 8 2021, 1:30 PM
lebedev.ri added a subscriber: lebedev.ri.
lebedev.ri added inline comments.
llvm/test/Transforms/SLPVectorizer/X86/arith-div-undef.ll
16

This is an immediate UB https://alive2.llvm.org/ce/z/uKOdmW

This revision now requires changes to proceed.Dec 8 2021, 1:30 PM
lebedev.ri added inline comments.Dec 8 2021, 1:30 PM
llvm/test/Transforms/SLPVectorizer/X86/arith-div-undef.ll
16
RKSimon added inline comments.Dec 11 2021, 4:52 AM
llvm/test/Transforms/SLPVectorizer/X86/arith-div-undef.ll
16

Although https://alive2.llvm.org/ce/z/ma3VyM shows that this patch isn't incorrect either

lebedev.ri added inline comments.Dec 11 2021, 4:58 AM
llvm/test/Transforms/SLPVectorizer/X86/arith-div-undef.ll
16

Sure, like with all UB we are free to make any refinement to the UB.
My comment is that this test case is just broken and can be optimized into system("dd if=/dev/zero of=/dev/sda");.

ABataev added inline comments.Dec 11 2021, 5:14 AM
llvm/test/Transforms/SLPVectorizer/X86/arith-div-undef.ll
16

I think we can just terminate SLP pass in this case

lebedev.ri resigned from this revision.Jan 12 2023, 4:49 PM

This review seems to be stuck/dead, consider abandoning if no longer relevant.

This revision now requires review to proceed.Jan 12 2023, 4:49 PM