This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU][MC][GFX9+] Corrected encoding of op_sel_hi for unused operands in VOP3P
ClosedPublic

Authored by dp on Mar 1 2021, 7:36 AM.

Details

Summary

Summary of changes:

  • VOP3P op_sel_hi is encoded as 1 for unused operands;
  • Disassembler can correctly decode VOP3P instructions regardless of op_sel_hi value for unused operands.

Fixed bug 49363.

Diff Detail

Event Timeline

dp created this revision.Mar 1 2021, 7:36 AM
dp requested review of this revision.Mar 1 2021, 7:36 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 1 2021, 7:36 AM
foad added a subscriber: Joe_Nash.Mar 1 2021, 7:55 AM
foad added a subscriber: foad.
dp added a comment.Mar 1 2021, 8:41 AM

I was surprised to discover that codegen tests include encodings. Is there any reason for that? Should I remove or update them?

In D97689#2594395, @dp wrote:

I was surprised to discover that codegen tests include encodings. Is there any reason for that? Should I remove or update them?

Yes, it is surprising. I am inclined to say remove the encodings.

In D97689#2594395, @dp wrote:

I was surprised to discover that codegen tests include encodings. Is there any reason for that? Should I remove or update them?

Yes, it is surprising. I am inclined to say remove the encodings.

Looking at the blame now, arsenm added the encoding checks that are failing in 2020, so perhaps those should be converted into assembler tests.

dp added a comment.Mar 1 2021, 9:11 AM

Thanks Joe. I decided to update the encodings because they were added on purpose.

dp updated this revision to Diff 327139.Mar 1 2021, 9:17 AM

Updated encodings in codegen tests; corrected code style as suggested by linter.

dp updated this revision to Diff 327222.Mar 1 2021, 11:47 AM

Corrections to silence the linter

rampitec accepted this revision.Mar 1 2021, 12:09 PM

Thanks!

This revision is now accepted and ready to land.Mar 1 2021, 12:09 PM