This is an archive of the discontinued LLVM Phabricator instance.

[TargetLowering][AMDGPU][X86] Improve SimplifyDemandedBits bitcast handling
ClosedPublic

Authored by RKSimon on Apr 9 2019, 6:39 AM.

Details

Summary

This patch adds support for BigBitWidth -> SmallBitWidth bitcasts, splitting the DemandedBits/Elts accordingly.

Re: the AMDGPU regression - @arsenm it looks there isn't much that generates BFE U32/S32 nodes in the DAG its mostly done in ISEL - in this case we need to match srl(and(shl(x,c1),c2),c1) - is this something that needs fixing first or are you OK with this change for now?

The X86 changes are all definite wins.

Diff Detail

Repository
rL LLVM

Event Timeline

RKSimon created this revision.Apr 9 2019, 6:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 9 2019, 6:39 AM
RKSimon marked an inline comment as done.Apr 12 2019, 7:50 AM
RKSimon added inline comments.
test/CodeGen/AMDGPU/store-weird-sizes.ll
49 ↗(On Diff #194306)

@arsenm Any suggestions on what to do here - is it better to add another isel pattern or start creating bfe_user dag nodes?

nhaehnle added inline comments.Apr 15 2019, 12:53 AM
test/CodeGen/AMDGPU/store-weird-sizes.ll
49 ↗(On Diff #194306)

Why doesn't the DAGCombiner simplify this to a single AND node?

RKSimon added inline comments.
test/CodeGen/AMDGPU/store-weird-sizes.ll
49 ↗(On Diff #194306)

The AND appears too late (from a bitcasted i64 -> v2i32) for visitShiftByConstant to be called from the SRL node - I'm working on a generic fix at the moment.

RKSimon updated this revision to Diff 196065.Apr 22 2019, 6:47 AM

Add AMDGPU srl(and(x,m),c) -> and(srl(x,c),srl(m,c)) canonicalization to improve BFE recognition

RKSimon marked an inline comment as done.Apr 22 2019, 6:49 AM
RKSimon added inline comments.
lib/Target/AMDGPU/AMDGPUISelLowering.cpp
3170 ↗(On Diff #196065)

@nhaehnle @arsenm I think this handles the BFE issue - I investigated putting this in DAGCombine but it caused a lot of noise on other targets - some improvements, some regressions.

arsenm accepted this revision.Apr 22 2019, 6:50 AM

LGTM

lib/Target/AMDGPU/AMDGPUISelLowering.cpp
3162 ↗(On Diff #196065)

Braecs around this

This revision is now accepted and ready to land.Apr 22 2019, 6:50 AM
This revision was automatically updated to reflect the committed changes.