This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] Support (X | C1) & C2 --> (X & C2^(C1&C2)) | (C1&C2) for vector splats
ClosedPublic

Authored by craig.topper on Aug 6 2017, 5:12 PM.

Details

Summary

Note the original code I deleted incorrectly listed this as (X | C1) & C2 --> (X & C2^(C1&C2)) | C1 Which is only valid if C1 is a subset of C2. This relied on SimplifyDemandedBits to remove any extra bits from C1 before we got to that code.

My new implementation avoids relying on that behavior so that it can be naively verified with alive.

Diff Detail

Event Timeline

craig.topper created this revision.Aug 6 2017, 5:12 PM
craig.topper added inline comments.
test/Transforms/InstCombine/or.ll
270–286

There's obviously still something missing here that keeps this from matching the non vector case.

davide added inline comments.Aug 6 2017, 5:15 PM
test/Transforms/InstCombine/or.ll
270–271

Why removing this test? cheers.

Fixed diff of tests. Somehow it was relative to an unsquashed commit that added test30vec with the wrong type or something.

spatel accepted this revision.Aug 7 2017, 10:11 AM

LGTM.

lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
1222

It's not unique to this transform, but I've wondered: are these calls completely eliminated in release builds?

This revision is now accepted and ready to land.Aug 7 2017, 10:11 AM
craig.topper added inline comments.Aug 7 2017, 10:39 AM
lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
1222

I highly doubt it because release builds should still support tracking names. It's only disabled because the clang driver passes a flag to the cc1 command that disables the name tracking.

This revision was automatically updated to reflect the committed changes.