Page MenuHomePhabricator

[DAGCombine] Match a greater range of rotate when not all bits are demanded.
Needs RevisionPublic

Authored by deadalnix on Sep 30 2019, 9:41 AM.

Details

Summary

Only match trunc for now, as integrating into the SimplifyDemandedBits proved to be a significant challenge.

Diff Detail

Event Timeline

deadalnix created this revision.Sep 30 2019, 9:41 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 30 2019, 9:41 AM
RKSimon added inline comments.Sep 30 2019, 10:08 AM
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
548

The change to returning SDValue looks like an NFC that you can do right away?

deadalnix marked an inline comment as done.Sep 30 2019, 1:23 PM
deadalnix added inline comments.
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
548

I can do that, indeed. Let me land this part and rebase.

deadalnix updated this revision to Diff 222500.Sep 30 2019, 2:41 PM

Rebase on top of NFC changes

vector tests?

lib/CodeGen/SelectionDAG/DAGCombiner.cpp
6135

Shouldn't this be: APInt::getMaxValue(LHS.getScalarValueSizeInBits()) ?

TBH I'd prefer getAllOnesValue as well as it avoids the signed/unsigned ambiguity of getMaxValue.

6234

isSubsetOf ?

deadalnix marked an inline comment as done.Oct 8 2019, 6:29 AM
deadalnix added inline comments.
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
6135

That wouldn't generate the proper pattern for vectors and would leave things such as only the last element is demanded.

deadalnix updated this revision to Diff 223844.Oct 8 2019, 7:08 AM

Add test for vector - but nothing changes for them (see rL374043 ) and use isSubsetOf and getAllOnesValue.

lebedev.ri requested changes to this revision.Wed, Oct 16, 11:44 AM
lebedev.ri added inline comments.
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
6226

Use getLimitedValue()

6230–6235

I don't understand the logic here.
This might need some comments.

6233

Should this use getAllOnesValue() directly?

6240–6243

Precommit.

This revision now requires changes to proceed.Wed, Oct 16, 11:44 AM