This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] combine extractelement into several selects
ClosedPublic

Authored by rampitec on Nov 9 2018, 1:27 PM.

Details

Summary

An extractelement with non-constant index will be lowered either to
scratch or movrel loop in most cases. This patch converts such
instruction into a set of selects if vector size is not too big.

Diff Detail

Event Timeline

rampitec created this revision.Nov 9 2018, 1:27 PM
arsenm added inline comments.Nov 10 2018, 2:25 PM
lib/Target/AMDGPU/SIISelLowering.cpp
8057–8058

Is this a combine instead of custom lowering to handle illegal typed vectors?

8060–8061

Grammar, too many

test/CodeGen/AMDGPU/extract_vector_dynelt.ll
3

Should have some 8 and 16-bit element vectors (and 1-bit since those always break things)

rampitec added inline comments.Nov 10 2018, 5:13 PM
lib/Target/AMDGPU/SIISelLowering.cpp
8057–8058

This is to allow further combining of selects. Ideally I thought about an IR pass, though amdgpu codegen prepare works too early to catch extract vectors created by promote alloca. A DAG combine works just fine.

rampitec updated this revision to Diff 173723.Nov 12 2018, 11:34 AM
rampitec marked 2 inline comments as done.

Fixed comment and added tests.

test/CodeGen/AMDGPU/extract_vector_dynelt.ll
3

Added tests except for i1. Extract i1 element is already broken in the DAGTypeLegalizer::SplitVecOp_EXTRACT_VECTOR_ELT() before my patch. I have a testcase but that needs to be a separate independent patch wrt legalizer.

rampitec updated this revision to Diff 173734.Nov 12 2018, 12:45 PM

Fixed i1 split vector, added i1 tests.

rampitec marked an inline comment as done.Nov 12 2018, 12:47 PM
rampitec added inline comments.
test/CodeGen/AMDGPU/extract_vector_dynelt.ll
3

I have fixed i1 splitvector, it is here in the review.
I can commit it separately, but the testcase from it is in this revieq.

rampitec marked 2 inline comments as done.Nov 12 2018, 12:51 PM
rampitec added inline comments.
test/CodeGen/AMDGPU/extract_vector_dynelt.ll
3

See D54440.

rampitec updated this revision to Diff 173921.Nov 13 2018, 1:04 PM
rampitec marked an inline comment as done.

Rebased. Split vector fixes are already submitted.

arsenm accepted this revision.Nov 13 2018, 1:17 PM

LGTM

This revision is now accepted and ready to land.Nov 13 2018, 1:17 PM
This revision was automatically updated to reflect the committed changes.