This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] try to fold one-demanded-bit-of-multiply
ClosedPublic

Authored by spatel on Jan 29 2022, 9:26 AM.

Details

Summary

This is a generalization of the icmp fold in D118061 (and that can probably be abandoned if this is sufficient).
We're looking for a disguised form of "odd * odd must be odd".
Some Alive2 proofs to show correctness:
https://alive2.llvm.org/ce/z/60Y8hz
https://alive2.llvm.org/ce/z/HfAP6R

Diff Detail

Event Timeline

spatel created this revision.Jan 29 2022, 9:26 AM
spatel requested review of this revision.Jan 29 2022, 9:26 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 29 2022, 9:26 AM

I don't suppose this can handle D117995? If we can do this in SimplifyDemandedBits then we can add the knownbits fold with the additional NeverUndefOrPoison check.

Otherwise I think I'll just add a (DemandedMask ==2 && X == Y) -> 0 special case.

I don't suppose this can handle D117995? If we can do this in SimplifyDemandedBits then we can add the knownbits fold with the additional NeverUndefOrPoison check.

Otherwise I think I'll just add a (DemandedMask ==2 && X == Y) -> 0 special case.

Hmm, not sure if I understood the question. I'm not seeing how we'd do it other than a special-case here. The knownbits fold isn't going to work in general because of the potential undef input?

I don't suppose this can handle D117995? If we can do this in SimplifyDemandedBits then we can add the knownbits fold with the additional NeverUndefOrPoison check.

Otherwise I think I'll just add a (DemandedMask ==2 && X == Y) -> 0 special case.

Hmm, not sure if I understood the question. I'm not seeing how we'd do it other than a special-case here. The knownbits fold isn't going to work in general because of the potential undef input?

Also, if we're adding the Mask==2 special-case for self-multiply, we could add the Mask==1 case as well:
https://alive2.llvm.org/ce/z/3HcD76

spatel added a comment.EditedFeb 4 2022, 6:30 AM

Ping.

Note that 2d1390efbe610 added one special-case for the backend. Presumably, we should keep these in sync as much as possible, so I can add that in follow-ups after adding tests.

This revision is now accepted and ready to land.Feb 4 2022, 7:31 AM
This revision was landed with ongoing or failed builds.Feb 4 2022, 8:42 AM
This revision was automatically updated to reflect the committed changes.

Ping.

Note that 2d1390efbe610 added one special-case for the backend. Presumably, we should keep these in sync as much as possible, so I can add that in follow-ups after adding tests.

Cheers @spatel !