This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Whole Quad Mode variant of mov.dpp intrinsic
AbandonedPublic

Authored by dstuttard on Jun 27 2017, 3:38 AM.

Details

Reviewers
arsenm
tpr
Summary

Added an extra llvm.amdgcn.mov.dpp.wqm intrinsic (based on the non-wqm version)
that enables WQM.
This is required for shaders to implement functions such as dpdx using dpp.

Added some simple tests based on mov.dpp intrinsic tests, plus a new dpdx
function style test.

Event Timeline

dstuttard created this revision.Jun 27 2017, 3:38 AM

I think we will require a more extensive change to properly support WQM and all it can do. However, this is a minor extension to an existing intrinsic that allows implementation of functions required in some shader implementations.
This can be removed if necessary if more fully featured support is implemented.

tpr accepted this revision.Jun 28 2017, 4:14 AM

Yes, lgtm as a temporary thing pending a decision on how we support dpp, wqm and wwm properly.

This revision is now accepted and ready to land.Jun 28 2017, 4:14 AM
arsenm added inline comments.Jun 28 2017, 10:05 AM
lib/Target/AMDGPU/VOP1Instructions.td
645–648

It would be better to structure this similarly to how the other WQM mode instructions are already handled. This is adding another encoded instruction which we don't want.

arsenm requested changes to this revision.Jun 28 2017, 10:05 AM
This revision now requires changes to proceed.Jun 28 2017, 10:05 AM
dstuttard added inline comments.Jun 30 2017, 8:39 AM
lib/Target/AMDGPU/VOP1Instructions.td
645–648

Sure - which WQM mode instruction were you thinking would be a better model?

I just posted D35167, which seems closer to how we want to deal with WQM and WWM in the long term, and should provide the same functionality as this intrinsic.

dstuttard abandoned this revision.Aug 4 2017, 6:41 AM

Agreed - @cwabbott your changes provide something much more complete than this change, plus remove the need for it. Thanks.