This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU: Added v_mad instruction.
ClosedPublic

Authored by wdng on Jun 14 2016, 8:25 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

wdng updated this revision to Diff 60788.Jun 14 2016, 8:25 PM
wdng retitled this revision from to Added v_mad instruction..
wdng updated this object.
wdng added reviewers: arsenm, tstellarAMD.
wdng set the repository for this revision to rL LLVM.
wdng added a project: Restricted Project.
tstellarAMD edited edge metadata.EditedJun 14 2016, 8:28 PM

The commit subject should be be prefixed with AMDGPU:

lib/Target/AMDGPU/VIInstructions.td
78–83

Can we also add a pattern and a test with this patch?

wdng retitled this revision from Added v_mad instruction. to AMDGPU: Added v_mad instruction..Jun 14 2016, 8:29 PM
wdng edited edge metadata.
wdng updated this revision to Diff 60789.EditedJun 14 2016, 8:34 PM

This pattern has been tested in LIT test mad_uint24.ll, which is a part of fp16 implementation.

arsenm added inline comments.Jun 14 2016, 8:36 PM
lib/Target/AMDGPU/VIInstructions.td
82

The unsigned version should be also added

281–284

This should be left for the i16 patch

wdng added a comment.Jun 14 2016, 8:37 PM

Pattern is added for LIT test mad_uint24.ll

wdng added inline comments.Jun 14 2016, 8:38 PM
lib/Target/AMDGPU/VIInstructions.td
281–284

Yes, we may not need the pattern. For this patch, we may only need to add the instruction definition for the time being.

arsenm added inline comments.Jun 14 2016, 8:49 PM
lib/Target/AMDGPU/VIInstructions.td
281–284

The pattern will do nothing here, so yes only the definitions

286

This is also trying to use half mul for integer mul

wdng updated this revision to Diff 60853.Jun 15 2016, 10:42 AM
wdng updated this object.
wdng marked 4 inline comments as done.

Added v_mad_u16 instruction definition.

wdng added inline comments.Jun 15 2016, 10:59 AM
lib/Target/AMDGPU/VIInstructions.td
87

Could you please say in more details? Thanks!

arsenm edited edge metadata.Jun 15 2016, 6:35 PM

The v_mad_f16 instruction is also missing

lib/Target/AMDGPU/VIInstructions.td
82–83

These aren't the opcode values I see in the manual

wdng updated this revision to Diff 60943.Jun 15 2016, 6:59 PM
wdng edited edge metadata.
wdng marked an inline comment as done.
  1. Modified opcode for v_mad_u16, v_mad_i16 to make it consistant with the GCN manual.
  2. Added v_mad_f16 instruction definition.
arsenm accepted this revision.Jun 15 2016, 7:04 PM
arsenm edited edge metadata.

With the capitalization fix LGTM

lib/Target/AMDGPU/VIInstructions.td
82

All of the other opcodes use lower case letters in the hex.

This revision is now accepted and ready to land.Jun 15 2016, 7:04 PM
This revision was automatically updated to reflect the committed changes.