This is an archive of the discontinued LLVM Phabricator instance.

[MergICmps] Make sure that the comparison only has one use.
ClosedPublic

Authored by courbet on Mar 5 2018, 2:05 AM.

Diff Detail

Event Timeline

courbet created this revision.Mar 5 2018, 2:05 AM
spatel added inline comments.Mar 12 2018, 6:43 AM
lib/Transforms/Scalar/MergeICmps.cpp
186–188

The change looks safe and fixes the bug. But I haven't looked at the whole pass, so it's not clear to me why we should have the restriction. Can you add something to the comment to explain?

spatel added inline comments.Mar 12 2018, 6:44 AM
test/Transforms/MergeICmps/X86/pr36557.ll
101–118

Should be able to remove some of this and still show the bug?

courbet updated this revision to Diff 138016.Mar 12 2018, 8:27 AM
courbet marked an inline comment as done.
  • Remove superfluous annotations in test.
  • add more comments
courbet marked an inline comment as done.Mar 12 2018, 8:28 AM
courbet added inline comments.
test/Transforms/MergeICmps/X86/pr36557.ll
101–118

All, actually :)

spatel accepted this revision.Mar 12 2018, 8:40 AM

LGTM.

lib/Transforms/Scalar/MergeICmps.cpp
190

at -> as

This revision is now accepted and ready to land.Mar 12 2018, 8:40 AM
This revision was automatically updated to reflect the committed changes.
courbet marked an inline comment as done.