Page MenuHomePhabricator

[DAGCombiner] Enable SimplifyDemandedBits vector support for TRUNCATE (WIP)
Needs ReviewPublic

Authored by RKSimon on Jan 7 2019, 4:38 AM.



As discussed on D56118, currently there are a number of regressions when we enable root support for SimplifyDemandedBits to handle TRUNCATE vectors (note - it already accepts truncation of vectors further down the simplification call tree).

This patch is currently a placeholder to show the regressions and to help us work out what needs to be fixed.

Diff Detail

Event Timeline

RKSimon created this revision.Jan 7 2019, 4:38 AM
RKSimon added inline comments.Jan 7 2019, 5:18 AM
28 ↗(On Diff #180459)

This just looks like we're missing something for the ARMISD::VMULL lowering

53 ↗(On Diff #180459)

Simplifying to ANY_EXTEND prevents PACKSS/PACKUS from working

252 ↗(On Diff #180459)

We'd been relying on the v4i64 ashr expansion

956 ↗(On Diff #180459)

Haven't worked out the problem here yet

77 ↗(On Diff #180459)

We'd been relying on the v8i64 ashr expansion

craig.topper added inline comments.Jan 7 2019, 4:33 PM
956 ↗(On Diff #180459)

I think we need to call SimplifyDemandedBits on Conditions of SHRUNKBLEND. We only do it when we convert from VSELECT to SHRUNKBLEND.

craig.topper added inline comments.
956 ↗(On Diff #180459)

Patch here D56421

RKSimon updated this revision to Diff 181284.Jan 11 2019, 8:28 AM

rebase after D56421

huihuiz added inline comments.
28 ↗(On Diff #180459)

Using "CHECK-NEXT" and matching with the exact register names will make this test cast very sensitive to scheduling and register allocation changes.
Use pattern matching should be a better approach.

RKSimon added inline comments.Jan 23 2019, 1:20 PM
28 ↗(On Diff #180459)

But it stops people missing/hiding codegen changes that need to be kept an eye on, including register allocation changes.

This argument has been going on for years now, and we've tended to see that the benefits of outweighs any difficulties.

More importantly, do you have any insights as to how to improve ARMISD::VMULL lowering?

RKSimon updated this revision to Diff 196070.Apr 22 2019, 7:29 AM

rebase - still showing a number of regressions that are proving tricky to fix

Herald added a project: Restricted Project. · View Herald TranscriptApr 22 2019, 7:29 AM
RKSimon updated this revision to Diff 211912.Jul 26 2019, 4:56 AM

rebase + vector support for truncate(srl(x,c)) case

RKSimon updated this revision to Diff 214598.Aug 12 2019, 4:18 AM

rebase - most of the remaining x86 issues should be fixed by D66004