This is an archive of the discontinued LLVM Phabricator instance.

Fix PR22222
ClosedPublic

Authored by sanjoy on Jan 14 2015, 2:38 PM.

Details

Summary

The bug was introduced in r225282. r225282 assumed that sub X, Y is the same as add X, -Y. This is not correct if we are going to upgrade the sub to sub nuw. This change fixes the issue by making the optimization ignore sub instructions.

Diff Detail

Event Timeline

sanjoy updated this revision to Diff 18186.Jan 14 2015, 2:38 PM
sanjoy retitled this revision from to Fix PR22222.
sanjoy updated this object.
sanjoy edited the test plan for this revision. (Show Details)
sanjoy added reviewers: majnemer, hfinkel, atrick.
sanjoy added a subscriber: Unknown Object (MLST).
majnemer accepted this revision.Jan 14 2015, 4:46 PM
majnemer edited edge metadata.
This revision is now accepted and ready to land.Jan 14 2015, 4:46 PM
This revision was automatically updated to reflect the committed changes.

In retrospect, that would have been the right way to do it. Directly checking zext(A + B) == zext(A) + zext(B) does not catch test.unsigned.add.0 (in the test file), but I think that is fixable.

Moreover, the approach you pointed out can be easily generalized to subtraction. I don't think you can easily generalize it to multiplication (since checking for BitWidth + 1 may not be sufficient in that case); but we're not handling multiplication anyway.

I'll try to implement the approach LinearFunctionTestReplace and send in a patch for review sometime this week.