This is an archive of the discontinued LLVM Phabricator instance.

[DAGCombiner] Set the right SDLoc on a newly-created sextload
ClosedPublic

Authored by vsk on Apr 26 2018, 5:11 PM.

Details

Summary

This teaches tryToFoldExtOfLoad to set the right location on a
newly-created extload. With that in place, the logic for performing a
certain ([s|z]ext (load ...)) combine becomes identical for sexts and
zexts, and we can get rid of one copy of the logic.

The test case churn is due to dependencies on IROrders inherited from
the wrong SDLoc.

Part of: llvm.org/PR37262

Depends on: https://reviews.llvm.org/D46157

Diff Detail

Repository
rL LLVM

Event Timeline

vsk created this revision.Apr 26 2018, 5:11 PM

The additional register pressure is unfortunate but is unrelated. Before I LGTM, I just wanted an additional agreement that the regression is vector-shuffle-variable tests isn't justification to delay.

vsk edited the summary of this revision. (Show Details)Apr 27 2018, 11:18 AM
vsk retitled this revision from [DAGCombiner] Set the right SDLoc on a newly-created sextload (4/N) to [DAGCombiner] Set the right SDLoc on a newly-created sextload.May 1 2018, 12:51 PM
vsk updated this revision to Diff 145976.May 9 2018, 11:50 AM
vsk edited the summary of this revision. (Show Details)

Ping. I think there was still a question about register allocation changes which result from specifying the right IROrder. Is this something to be addressed in the scheduler, or should I be retaining the old IROrder here?

aprantl accepted this revision.May 9 2018, 12:06 PM

This patch is making it so the order of the generated MIR instructions is closer to the order of the IR instructions they were generated from, which is the intended / documented behavior of SelectionDag. My vote is to land this change unless one of the stakeholders of the respective targets objects.

This revision is now accepted and ready to land.May 9 2018, 12:06 PM
This revision was automatically updated to reflect the committed changes.