Page MenuHomePhabricator

[AMDGPU] Force qsads instrs to use different dest register than source registers
ClosedPublic

Authored by msearles on Jun 1 2017, 8:56 AM.

Details

Summary

The V_MQSAD_PK_U16_U8, V_QSAD_PK_U16_U8, and V_MQSAD_U32_U8 take more than 1 pass in hardware. For these three instructions, the destination registers must be different than all sources so that the first pass does not overwrite sources for the following passes.

Diff Detail

Repository
rL LLVM

Event Timeline

msearles created this revision.Jun 1 2017, 8:56 AM
arsenm added inline comments.Jun 1 2017, 9:08 AM
test/CodeGen/AMDGPU/llvm.amdgcn.mqsad.pk.u16.u8.ll
7 ↗(On Diff #101032)

You still need a positive check that the instruction was emitted with the proper register class. I also don't think FileCheck supports matching variables that appear later in the same line

msearles added inline comments.Jun 1 2017, 9:28 AM
test/CodeGen/AMDGPU/llvm.amdgcn.mqsad.pk.u16.u8.ll
7 ↗(On Diff #101032)

I'll update the test re: register class. As far as FileCheck goes: it seems to work and documentation seems to say that it should work ( "Variables can also be used later on the same line they were defined on" , which is from http://llvm.org/docs/CommandGuide/FileCheck.html#filecheck-variables ).

msearles updated this revision to Diff 101046.Jun 1 2017, 10:26 AM

Add the positive tests back in

rampitec added inline comments.
test/CodeGen/AMDGPU/llvm.amdgcn.mqsad.pk.u16.u8.ll
7 ↗(On Diff #101032)

Do not you need to switch order of checks? If FileCheck first will match positive pattern it will start matching from there, so negative check will not work.

arsenm added inline comments.Jun 1 2017, 11:49 AM
test/CodeGen/AMDGPU/llvm.amdgcn.mqsad.pk.u16.u8.ll
2 ↗(On Diff #101046)

It would be a good idea to add -enable-var-scope to FileCheck

msearles updated this revision to Diff 101806.Jun 7 2017, 1:26 PM
msearles added a reviewer: arsenm.

Update tests

dp added a subscriber: dp.Jun 8 2017, 3:54 AM
arsenm accepted this revision.Jun 8 2017, 10:51 AM

LGTM

This revision is now accepted and ready to land.Jun 8 2017, 10:51 AM
This revision was automatically updated to reflect the committed changes.