This is an archive of the discontinued LLVM Phabricator instance.

Revert "[InstCombine] keep assumption before sinking calls"
ClosedPublic

Authored by inglorion on Dec 2 2019, 5:36 PM.

Details

Summary

This reverts commit c3b06d0c393e533eab712922911d14e5a079fa5d.

Reason for revert: Caused miscompiles when inserting assume for undef.

Also adds a test to prevent similar breakage in future.

Fixes PR44154.

Diff Detail

Event Timeline

inglorion created this revision.Dec 2 2019, 5:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 2 2019, 5:36 PM
Herald added a subscriber: hiraditya. · View Herald Transcript

@rnk suggested on D70749 to do a simple revert of D69477. That's what this is.

thakis added a subscriber: thakis.Dec 2 2019, 7:42 PM

Can you add a test for the case that's miscompiled with this not reverted?

Add the test case that miscompiled, maybe keep a modified assume-replacing-call.ll test case with a fixme, and then remove the assume code. (= I'm fine with this)

inglorion updated this revision to Diff 232003.Dec 3 2019, 3:30 PM
inglorion edited the summary of this revision. (Show Details)
inglorion removed a subscriber: thakis.

added test that fails without the revert

rnk accepted this revision.Dec 3 2019, 3:57 PM

lgtm

This revision is now accepted and ready to land.Dec 3 2019, 3:57 PM
This revision was automatically updated to reflect the committed changes.
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp