This is an archive of the discontinued LLVM Phabricator instance.

Added transform for ABS(NABS(X)) and NABS(ABS(X))
ClosedPublic

Authored by dinesh.d on Jun 6 2014, 12:53 AM.

Details

Diff Detail

Repository
rL LLVM

Event Timeline

dinesh.d updated this revision to Diff 10166.Jun 6 2014, 12:53 AM
dinesh.d retitled this revision from to Added transform for ABS(NABS(X)) and NABS(ABS(X)).
dinesh.d updated this object.
dinesh.d edited the test plan for this revision. (Show Details)
dinesh.d added reviewers: bkramer, rafael.
dinesh.d added a subscriber: Unknown Object (MLST).
chandlerc accepted this revision.Jun 12 2014, 5:19 AM
chandlerc added a reviewer: chandlerc.
chandlerc added a subscriber: chandlerc.

LGTM

I wouldn't bother re-testing all the permutations of the 'icmp' representation -- that code should already be well tested by existing checks? Still, not a big deal, just trying to minimize the redundancy in the testing.

This revision is now accepted and ready to land.Jun 12 2014, 5:19 AM

Thanks,

I had submitted similar patch [http://reviews.llvm.org/D3658] with all permutations of icmp.
This was just followup patch.

I am submitting it as it is. But if you care, I can remove few test cases for both changes.
In that case, we may not to add new file [abs_abs.ll], which I have added just because
there were too many test cases to add.

dinesh.d closed this revision.Jun 12 2014, 7:14 AM
dinesh.d updated this revision to Diff 10361.

Closed by commit rL210782 (authored by dinesh).