This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU][MC] Added missing VOP3P opcodes
ClosedPublic

Authored by dp on Jun 30 2017, 9:11 AM.

Details

Summary
  1. Added missing VOP3P opcodes (see https://bugs.llvm.org//show_bug.cgi?id=33593):
    • v_pk_sub_u16
    • v_pk_mad_i16
    • v_pk_mad_u16
  1. Corrected v_pk_sub_i16 (was labelled as commutable by mistake);
  1. Corrected parser to use MCRegAliasIterator instead of isRegIntersect (as suggested by Matt);
  1. Removed isRegIntersect.

Diff Detail

Repository
rL LLVM

Event Timeline

dp created this revision.Jun 30 2017, 9:11 AM
arsenm added inline comments.Jul 11 2017, 11:46 AM
lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
2191–2193 ↗(On Diff #104871)

This is an unrelated change

dp added inline comments.Jul 12 2017, 3:17 AM
lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
2191–2193 ↗(On Diff #104871)

Sure. Do you propose to remove 'isRegIntegsect' in a separate commit?

artem.tamazov accepted this revision.Jul 14 2017, 4:04 AM

LGTM except the comment about isRegIntersect() which needs to be addressed prior merging.

lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
2191–2193 ↗(On Diff #104871)

I recommend keeping the isRegIntersect() function and removing the comment. WRT separating this, I do not care, but this should be mentioned in the commit description.

This revision is now accepted and ready to land.Jul 14 2017, 4:04 AM
arsenm added inline comments.Jul 14 2017, 9:52 AM
lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
2191–2193 ↗(On Diff #104871)

Yes, this should be separate

dp added inline comments.Jul 14 2017, 11:11 AM
lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
2191–2193 ↗(On Diff #104871)

Are you suggesting to keep isRegIntegsect, but simplify its implementation using MCRegAliasIterator?

2191–2193 ↗(On Diff #104871)

Ok, I'll integrate this change in 2 commits

dp updated this revision to Diff 106766.Jul 15 2017, 4:25 AM
dp edited the summary of this revision. (Show Details)

Removed IsRegIntersect relevant changes

This revision was automatically updated to reflect the committed changes.