This is an archive of the discontinued LLVM Phabricator instance.

[DAGCombiner] After performing the division by constant optimization for a DIV or REM node, replace the users of the corresponding REM or DIV node if it exists.
ClosedPublic

Authored by craig.topper on Dec 28 2018, 11:29 PM.

Details

Summary

Currently we expand the two nodes separately. This gives DAG combiner an opportunity to optimize the expanded sequence taking into account only one set of users. When we expand the other node we'll create the expansion again, but might not be able to optimize it the same way. So the nodes won't CSE and we'll have two similarish sequences in the same basic block. By expanding both nodes at the same time we'll avoid prematurely optimizing the expansion until both the division and remainder have been replaced.

Improves the test case from PR38217. There may be additional opportunities after this.

Diff Detail

Event Timeline

craig.topper created this revision.Dec 28 2018, 11:29 PM
RKSimon added inline comments.Dec 31 2018, 7:23 AM
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
3437

Indentation is quite confusing here?

Fix bad indentation

RKSimon accepted this revision.Jan 2 2019, 2:49 AM

LGTM - there might be a minor code cleanup opportunity to move more of this inside visitSDIVLike/visitUDIVLike as a future NFC.

This revision is now accepted and ready to land.Jan 2 2019, 2:49 AM
This revision was automatically updated to reflect the committed changes.