Page MenuHomePhabricator

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

Authored by inglorion on Mon, Dec 2, 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.Mon, Dec 2, 5:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptMon, Dec 2, 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.

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.Tue, Dec 3, 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.Tue, Dec 3, 3:57 PM

lgtm

This revision is now accepted and ready to land.Tue, Dec 3, 3:57 PM
This revision was automatically updated to reflect the committed changes.