This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU][MC] Corrected definition of GATHER4 opcodes
ClosedPublic

Authored by dp on Feb 28 2018, 9:04 AM.

Details

Summary

See bug 36252: https://bugs.llvm.org/show_bug.cgi?id=36252

Generated tests (gfx*_asm_all.s) are not included in this review due to the large amount of changes.

Diff Detail

Repository
rL LLVM

Event Timeline

dp created this revision.Feb 28 2018, 9:04 AM
artem.tamazov added inline comments.Mar 1 2018, 4:41 AM
lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
2360 ↗(On Diff #136304)

Where these literals come from? Descriptive comment would be enough, I guess.

2360 ↗(On Diff #136304)

Aha, I see. Let's add something like

// See comment in MIMG_Gather_Helper class, in MIMGInstructions.td.
test/CodeGen/AMDGPU/llvm.amdgcn.image.gather4.d16.ll
6 ↗(On Diff #136304)

Is it possible to fix tests in .ll files rather than remove?

129 ↗(On Diff #136304)

Ditto.

test/CodeGen/AMDGPU/llvm.amdgcn.image.gather4.ll
323 ↗(On Diff #136304)

Ditto.

380 ↗(On Diff #136304)

Same.

dp added inline comments.Mar 1 2018, 5:38 AM
lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
2360 ↗(On Diff #136304)

Will do

test/CodeGen/AMDGPU/llvm.amdgcn.image.gather4.d16.ll
6 ↗(On Diff #136304)

These tests are incorrect - they assume that gather4 opcodes could have one or two image components in the destination. Actually, gather4 opcodes always return 4 components (but these may be packed).

I had to remove these. The remaining tests provide sufficient coverage.

129 ↗(On Diff #136304)

These declarations are also incorrect. Gather4 can return 4xhalf or 4xfloat only.

This revision is now accepted and ready to land.Mar 1 2018, 5:43 AM
This revision was automatically updated to reflect the committed changes.