This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] fixed divergence driven shift operations selection
ClosedPublic

Authored by alex-t on Jan 27 2020, 7:55 AM.

Details

Summary

change https://reviews.llvm.org/D64725 removes “isDivergent” predicates from the scalar shifts.
As a result we easy can select S_LSHL_B32 for divergent arguments.

Diff Detail

Event Timeline

alex-t created this revision.Jan 27 2020, 7:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 27 2020, 7:55 AM

Is there a test for scalar selection?

llvm/lib/Target/AMDGPU/VOP2Instructions.td
548

Any tests for GFX10 on this?

llvm/lib/Target/AMDGPU/VOP3Instructions.td
388

Any tests for GFX10 on this?

llvm/test/CodeGen/AMDGPU/GlobalISel/inst-select-add.mir
2 ↗(On Diff #240590)

Ugh... Fiji is GFX8. Instead of dropping test for tahiti just fix the check-prefix for fiji.

alex-t updated this revision to Diff 241120.Jan 29 2020, 5:17 AM
alex-t marked 2 inline comments as done.

divergence driven selection test for shift operations added
tests updated

llvm/lib/Target/AMDGPU/VOP2Instructions.td
548

Will be an assertion "Opcode does not exist on target"

llvm/lib/Target/AMDGPU/VOP3Instructions.td
388

Will be an assertion "Opcode does not exist on target"

rampitec added inline comments.Jan 29 2020, 10:35 AM
llvm/lib/Target/AMDGPU/VOP2Instructions.td
548

But we need a gfx10 test to check proper selection of IR shifts. I do not see these tests.

llvm/test/CodeGen/AMDGPU/GlobalISel/extractelement.ll
982 ↗(On Diff #241120)

It does not belong to this change. Please generate a proper diff.

llvm/test/CodeGen/AMDGPU/GlobalISel/inst-select-add.mir
2 ↗(On Diff #240590)

Please return the dropped tahiti run line.

llvm/test/CodeGen/AMDGPU/shift-select.ll
3

Please add another line for gfx1010.

58

You also need tests for i64.

135

Newline.

alex-t updated this revision to Diff 241451.Jan 30 2020, 7:29 AM

Diff corrected. GFX10 test added.

I expected this to fix the test XFAIL'd in c12f046eb96f8462b3fd3889344ba344de5ace1f, but it seems to to have?

rampitec added inline comments.Jan 30 2020, 9:57 AM
llvm/test/CodeGen/AMDGPU/shift-select.ll
58

Would be LGTM if there were i64 tests.

I expected this to fix the test XFAIL'd in c12f046eb96f8462b3fd3889344ba344de5ace1f, but it seems to to have?

I haven't seen any tests XFAIL'd by this c12f046eb96f8462b3fd3889344ba344de5ace1f commit.
What did you mean? Could you explain please?

alex-t updated this revision to Diff 241692.Jan 31 2020, 4:42 AM

i64 tests added

I expected this to fix the test XFAIL'd in c12f046eb96f8462b3fd3889344ba344de5ace1f, but it seems to to have?

I haven't seen any tests XFAIL'd by this c12f046eb96f8462b3fd3889344ba344de5ace1f commit.
What did you mean? Could you explain please?

test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.update.dpp.ll is now XFAILed

alex-t updated this revision to Diff 241723.Jan 31 2020, 6:58 AM

llvm.amdgcn.update.dpp test updated

arsenm accepted this revision.Jan 31 2020, 7:10 AM
This revision is now accepted and ready to land.Jan 31 2020, 7:10 AM
This revision was automatically updated to reflect the committed changes.