This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine][NFC] Add test to show missing fold for icmp ult/uge (shl %x, C2), C1.
ClosedPublic

Authored by huihuiz on Jun 21 2019, 2:00 PM.

Details

Summary

'shl' inequality test

icmp ult/uge (shl %x, C2), C1 iff C1 is power of two

can be simplified as 'and' equality test

icmp eq/ne (and %x, (lshr -C1, C2)), 0.

Diff Detail

Repository
rL LLVM

Event Timeline

huihuiz created this revision.Jun 21 2019, 2:00 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 21 2019, 2:00 PM
huihuiz updated this revision to Diff 206078.Jun 21 2019, 2:17 PM

minor fix of comments in the test file

huihuiz updated this revision to Diff 206308.Jun 24 2019, 3:00 PM

remove test file for "shl-and-compare", as "shl-compare" test cases are sufficient.

lebedev.ri added inline comments.Jun 24 2019, 3:08 PM
llvm/test/Transforms/InstCombine/shl-unsigned-cmp-const.ll
83–87 ↗(On Diff #206308)

Yeah, it's not really possible to ensure that those folds happen, the predicate is canonicalized.
I'd recommend to add those tests with predicate variants using vectors: https://godbolt.org/z/69dzCf
^ Looks like those aren't being canonicalized, at least right now.

lebedev.ri accepted this revision.Jun 24 2019, 3:13 PM

Looks ok

This revision is now accepted and ready to land.Jun 24 2019, 3:13 PM
huihuiz updated this revision to Diff 206314.Jun 24 2019, 3:40 PM

Added vector test to check for predicate variance.
Canonicalization happens for ule -> ult , uge -> ugt

lebedev.ri accepted this revision.Jun 24 2019, 3:53 PM

Added vector test to check for predicate variance.
Canonicalization happens for ule -> ult , uge -> ugt

Right, i forgot to enable instcombine in my godbolt link :)
Well then, i guess we can't have test coverage for that half of the fold :/

huihuiz updated this revision to Diff 206319.Jun 24 2019, 4:11 PM

minor update for vector test,

for vector test

(X l<< 11) u<= 131071  , not 131072
}

similar for ugt test

sorry didn't catch this earlier

This revision was automatically updated to reflect the committed changes.