This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] gfx11 VOPD instructions MC support
ClosedPublic

Authored by Joe_Nash on Jun 20 2022, 11:15 AM.

Details

Reviewers
foad
rampitec
dp
arsenm
Group Reviewers
Restricted Project
Commits
rG07b7fada73da: [AMDGPU] gfx11 VOPD instructions MC support
Summary

VOPD is a new encoding for dual-issue instructions for use in wave32.
This patch includes MC layer support only.

A VOPD instruction is constituted of an X component (for which there are
13 possible opcodes) and a Y component (for which there are the 13 X
opcodes plus 3 more). Most of the complexity in defining and parsing
a VOPD operation arises from the possible different total numbers of
operands and deferred parsing of certain operands depending on the
constituent X and Y opcodes.

Diff Detail

Event Timeline

Joe_Nash created this revision.Jun 20 2022, 11:15 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 20 2022, 11:15 AM
Joe_Nash requested review of this revision.Jun 20 2022, 11:15 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 20 2022, 11:15 AM
Joe_Nash added reviewers: Restricted Project, foad, rampitec, dp, arsenm.Jun 20 2022, 11:18 AM
dp added a comment.Jun 23 2022, 5:04 AM

Looks good, except for some nitpicking.

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

Indent.

290

Is there a way to avoid code duplication here? VOP2eInst and VOP2eInst_VOPD are almost identical.

291

Indent.

llvm/lib/Target/AMDGPU/VOPDInstructions.td
29

Extra spaces in bit range.

49

Extra spaces in bit range.

59

Indent.
What does VC stand for?

86

Indent.

88

Extra spaces in bit range.

96

Extra spaces in bit range.

102

Extra space

125

Missing space after commas. Next 2 lines also need corrections.

llvm/lib/Target/AMDGPU/VOPInstructions.td
35

Missing space after comma.

37

Could we avoid hardcoding 13 here? Ideally it should be defined together with VOPDXPseudos and VOPDYPseudos.

Joe_Nash updated this revision to Diff 439492.Jun 23 2022, 12:15 PM
Joe_Nash marked 10 inline comments as done.

fixed formatting, deduplicated VOP2eInst, named a variable

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

Good point, I have deduplicated it.

llvm/lib/Target/AMDGPU/VOPDInstructions.td
59

VOPD_Component.

llvm/lib/Target/AMDGPU/VOPInstructions.td
37

Yes, I have made it a named variable instead, used here and in defvar VOPDXPseudos. In theory I could move VOPDXPseudos and VOPDY pseudos here and call !size on it, but that seems slightly worse for file encapsulation purposes

dp accepted this revision.Jun 24 2022, 8:10 AM

LGTM, thanks!

This revision is now accepted and ready to land.Jun 24 2022, 8:10 AM
This revision was landed with ongoing or failed builds.Jun 24 2022, 8:38 AM
This revision was automatically updated to reflect the committed changes.