This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Combine and (srl) into shl (bfe)
ClosedPublic

Authored by rampitec on May 23 2017, 11:32 AM.

Details

Summary

Perform DAG combine:
and (srl x, c), mask => shl (bfe x, nb + c, mask >> nb), nb
Where nb is a number of trailing zeroes in mask.

It replaces two instructions with two and BFE is generally a more
expensive one. However this is only done if we are selecting a byte
or word at an aligned boundary which results in a proper SDWA
operand pattern. It is only done if SDWA is supported.

TODO: improve SDWA pass to actually convert this pattern. It is not
done now because we have an immediate in the instruction, which has
be moved into a VGPR.

Diff Detail

Repository
rL LLVM

Event Timeline

rampitec created this revision.May 23 2017, 11:32 AM
arsenm added inline comments.May 23 2017, 11:39 AM
lib/Target/AMDGPU/AMDGPUISelLowering.cpp
3482

There is an SDWA subtarget feature

lib/Target/AMDGPU/SIISelLowering.cpp
4242

s/finalizer/SDWA pass/

test/CodeGen/AMDGPU/bfe-combine.ll
4

s/CHECK/GCN

rampitec marked 2 inline comments as done.May 23 2017, 11:48 AM
rampitec added inline comments.
lib/Target/AMDGPU/AMDGPUISelLowering.cpp
3482

AMDGPUSubtarget does not know anything about SDWA, it is in SISubtarget.

rampitec updated this revision to Diff 99963.May 23 2017, 11:48 AM
rampitec added a reviewer: arsenm.

Changed comment and FileCheck prefix.

arsenm added inline comments.May 23 2017, 11:51 AM
lib/Target/AMDGPU/AMDGPUISelLowering.cpp
3482

The SDWA predicate could be moved into AMDGPUSUbtarget if needed here, there's nothing really requiring it be part of SISubtarget

rampitec updated this revision to Diff 99965.May 23 2017, 11:57 AM
rampitec marked 3 inline comments as done.

Moved hasSDWA() into AMDGPUSuntarget.

arsenm accepted this revision.May 23 2017, 12:04 PM

LGTM bu the other test would be good

test/CodeGen/AMDGPU/bfe-combine.ll
2

Should have a check for a target without SDWA showing this doesn't happen

This revision is now accepted and ready to land.May 23 2017, 12:04 PM
rampitec updated this revision to Diff 99969.May 23 2017, 12:21 PM
rampitec marked an inline comment as done.

Added check that we do not do it on CI.

rampitec updated this revision to Diff 99973.May 23 2017, 12:35 PM

Updated test.

This revision was automatically updated to reflect the committed changes.