Page MenuHomePhabricator

[Instcombine] Fix uses of undef (PR46940)
ClosedPublic

Authored by kazu on Aug 10 2020, 1:46 PM.

Details

Summary

Without this patch, we attempt to distribute And over Xor even in
unsafe circumstances like so:

undef & (true ^ true)  ==>  (undef & true) ^ (undef & true)

and evaluate it to undef instead of false. Note that "true ^ true"
may show up implicitly with one true being part of a PHI node.

This patch fixes the problem by teaching SimplifyAndInst and
SimplifyUsingDistributiveLaws to not use undef as part of
simplifications when they distribute And over Xor.

Diff Detail

Event Timeline

kazu created this revision.Aug 10 2020, 1:46 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 10 2020, 1:46 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
kazu requested review of this revision.Aug 10 2020, 1:46 PM
nikic added a comment.Aug 10 2020, 1:51 PM

Oops, looks like we were working on this in parallel. I just submitted D85684 for the InstSimplify part of the problem.

kazu added a comment.Aug 10 2020, 2:01 PM

Oops, looks like we were working on this in parallel. I just submitted D85684 for the InstSimplify part of the problem.

Oh, OK. In that case, once your patch lands. I will drop the InstructionSimplify.cpp portion of my patch. Note that for my testcase in this patch to pass, I need both changes, the one in InstructionSimplify.cpp and the other in InstructionCombining.cpp.

nikic added a comment.Aug 11 2020, 1:10 PM

D85684 has landed now. After rebasing, you can also use SQ.getWithoutUndef() to reduce the boilerplate for creating a new SimplifyQuery.

kazu updated this revision to Diff 284884.Aug 11 2020, 1:54 PM

I've rebased this patch on top of https://reviews.llvm.org/D85684.

Please thank a look. Thanks!

nikic accepted this revision.Aug 11 2020, 2:00 PM

LGTM

llvm/test/Transforms/InstCombine/dont-distribute-phi.ll
2

Please use llvm/util/update_test_checks.py to generate CHECK lines.

This revision is now accepted and ready to land.Aug 11 2020, 2:00 PM
kazu updated this revision to Diff 284894.Aug 11 2020, 2:23 PM

I've llvm/util/update_test_checks.py to update CHECK lines.

kazu marked an inline comment as done.Aug 11 2020, 2:23 PM

I've llvm/util/update_test_checks.py to update CHECK lines.

I don't think this worked though.

This revision was landed with ongoing or failed builds.Aug 11 2020, 2:25 PM
This revision was automatically updated to reflect the committed changes.