This is an archive of the discontinued LLVM Phabricator instance.

[x86] increment/decrement constant vector with min/max in vsetcc lowering (PR39859)
ClosedPublic

Authored by spatel on Dec 10 2018, 8:55 AM.

Details

Summary

This is part of fixing PR39859:
https://bugs.llvm.org/show_bug.cgi?id=39859

We have a crippled vector ISA, so we have to invert a typical fold and create min/max here.

As discussed in the bug report, we can probably do better by using saturating subtract in larger patterns when it's available, but we should have this improvement for the minimal patterns regardless.

Alive proofs:
https://rise4fun.com/Alive/zsf
https://rise4fun.com/Alive/Qrl

Diff Detail

Repository
rL LLVM

Event Timeline

spatel created this revision.Dec 10 2018, 8:55 AM
RKSimon added inline comments.Dec 10 2018, 9:09 AM
lib/Target/X86/X86ISelLowering.cpp
19394 ↗(On Diff #177534)

Can you use ISD::matchUnaryPredicate and get this TODO done straightaway?

spatel marked an inline comment as done.Dec 10 2018, 9:42 AM
spatel added inline comments.
lib/Target/X86/X86ISelLowering.cpp
19394 ↗(On Diff #177534)

I didn't think about using matchUnaryPredicate, but that is a possibility. We have this already:
rL348776
I was planning to generalize that (doesn't seem to handle undef elements either) and lift it out of x86. We have things like that in InstCombine called "AddOne" and "SubOne".

But I think it's best to make it a follow-up either way to avoid bug risk in that conversion.

nikic added a comment.Dec 10 2018, 2:23 PM

Are there any tests for the 0/max cases? (I guess they're probably just constant folded away anyway?)

Are there any tests for the 0/max cases? (I guess they're probably just constant folded away anyway?)

No - I can add those, but it's probably hard to get that pattern this deep into SDAG.
And yes, we would hope that those would always get folded, but nothing is certain in SDAG which is why I could not just assert that those cases are impossible.

spatel updated this revision to Diff 177729.Dec 11 2018, 9:10 AM

Patch updated:
Add tests for vsetcc simplification edge cases. These are currently folded early in generic DAGCombiner via SimplifySetCC().

RKSimon accepted this revision.Dec 13 2018, 1:54 PM

LGTM - hope to see the non-splat vector support soon.

This revision is now accepted and ready to land.Dec 13 2018, 1:54 PM
This revision was automatically updated to reflect the committed changes.