This is an archive of the discontinued LLVM Phabricator instance.

[DAGCombine] Don't combine sext with extload if sextload is not supported and extload has multi users
ClosedPublic

Authored by Carrot on Oct 19 2017, 2:36 PM.

Details

Summary

In function DAGCombiner::visitSIGN_EXTEND_INREG, sext can be combined with extload even if sextload is not supported by target, then

  • if sext is the only user of extload, there is no big difference, no harm no benefit.
  • if extload has more than one user, the combined sextload may block extload from combining with other zext, causes extra zext instructions generated. As demonstrated by the attached test case.

This patch add the constraint that when sextload is not supported by target, sext can only be combined with extload if it is the only user of extload.

Diff Detail

Repository
rL LLVM

Event Timeline

Carrot created this revision.Oct 19 2017, 2:36 PM
hfinkel added inline comments.
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
8278 ↗(On Diff #119616)

How about adding to the comment:

Doing otherwise can block folding the extload with other extends that the target does support.
test/CodeGen/PowerPC/selectiondag-sextload.ll
14 ↗(On Diff #119616)

Please check for the code you expect, not just the absence of the extra zext. We want to make sure we continue to generate efficient code.

Carrot updated this revision to Diff 120662.Oct 27 2017, 11:30 AM
Carrot marked 2 inline comments as done.
This revision is now accepted and ready to land.Oct 27 2017, 12:10 PM
This revision was automatically updated to reflect the committed changes.