This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Recommit r314244 with refactoring and off by default
ClosedPublic

Authored by nemanjai on Oct 4 2017, 10:36 PM.

Details

Summary

This patch reimplements everything that was pulled out in r314244. The code is moved out of the main selection path and a single entry point is provided. At this time, the transformation is off by default and there are a number of knobs to control which patterns to transform. After adequate field-testing, the plan is to turn this on by default and remove some of the control knobs for turning it off.

Diff Detail

Repository
rL LLVM

Event Timeline

nemanjai created this revision.Oct 4 2017, 10:36 PM
nemanjai added inline comments.Oct 4 2017, 10:41 PM
lib/Target/PowerPC/PPCISelDAGToDAG.cpp
108 ↗(On Diff #117776)

The various values provided here do not have adequate testing as they are not meant to stay in - they're only provided to aid in field-testing this patch.

If we can settle on which of these we want to keep permanently (and if we want to add any), then I'll provide testing for those and make sure I keep them in even after we turn this on by default.

2106 ↗(On Diff #117776)

Feel free to chime in with suggestions if you don't like the name. Perhaps a more appropriate name would be IntegerCompareSelector or similar.

nemanjai updated this revision to Diff 123960.Nov 23 2017, 7:54 AM

Rebase and fix the behaviour in test cases that has changed due to other committed patches.

We have now done all the testing we can do with this patch - including everything that failed with the original patches. Furthermore, the series of patches that this refactors and reimplements have all been approved.
Can I draw the attention of the reviewers to this patch so that I can re-commit it?

echristo accepted this revision.Nov 29 2017, 10:22 AM

I'd really like to see this solved in a more generic way, but I think that's a bit far afield for a single backend so OK.

-eric

This revision is now accepted and ready to land.Nov 29 2017, 10:22 AM

I'd really like to see this solved in a more generic way, but I think that's a bit far afield for a single backend so OK.

-eric

I will happily pull this once we can get it all done in .td files. Maybe once GlobalISel fully replaces SDAG ISel, we will get around to fixing this limitation. At least it's now all encapsulated and easy to remove now.

This revision was automatically updated to reflect the committed changes.