This is an archive of the discontinued LLVM Phabricator instance.

Differential D90242

[AMDGPU] Fix check prefix for VOP3 VI disassembler tests
ClosedPublic

Authored by foad on Oct 27 2020, 8:57 AM.

Details

Reviewers
dp
rampitec
vpykhtin
Commits
rG4b1ea84a1d04: [AMDGPU] Fix check prefix for VOP3 VI disassembler tests
Summary

Also, following D81841, don't try to encode f16 literals in i16/u16
instructions.

Diff Detail

Unit TestsFailed

TimeTest
360 mslinux > HWAddressSanitizer-x86_64.TestCases::sizes.cpp

Event Timeline

foad created this revision.Oct 27 2020, 8:57 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 27 2020, 8:57 AM
Herald added subscribers: llvm-commits, kerbowa, t-tye and 6 others. · View Herald Transcript
foad requested review of this revision.Oct 27 2020, 8:57 AM
Herald added a subscriber: wdng. · View Herald TranscriptOct 27 2020, 8:57 AM
foad added reviewers: dp, rampitec, vpykhtin.Oct 27 2020, 9:02 AM

These tests were added in D36322.

A similar fix for the VOP2 GFX9 tests was made in D36674.

llvm/test/MC/Disassembler/AMDGPU/vop3_vi.txt
300

The inline constant is disassembled as 0x3800 not 0.5.

SIMCCodeEmitter does not recognise this constant as an inline 16-bit integer, so the encoding: line shows 0xff not 0xf0.

What is the correct behaviour here?

rampitec added inline comments.Oct 27 2020, 9:36 AM
llvm/test/MC/Disassembler/AMDGPU/vop3_vi.txt
300

It should be f0.

dp added inline comments.Oct 27 2020, 9:55 AM
llvm/test/MC/Disassembler/AMDGPU/vop3_vi.txt
300

I believe 0xff is an expected result. Change 5f5f566 disabled use of fp inline constants with 16-bit integer operands. When disassembled, these inline constants are deliberately printed in hex to stress that the value is not an inline constant but a literal.

And yes, the produced dump is incorrect (missing literal) but so is the original code.

rampitec added inline comments.Oct 27 2020, 10:07 AM
llvm/test/MC/Disassembler/AMDGPU/vop3_vi.txt
300

I would probably say the expected result is an error message if 0.5 cannot really be supported. Isn't it?

Harbormaster completed remote builds in B76580: Diff 301021.Oct 27 2020, 10:18 AM
foad updated this revision to Diff 301051.Oct 27 2020, 10:24 AM

Update.

foad edited the summary of this revision. (Show Details)Oct 27 2020, 10:25 AM
dp added inline comments.Oct 27 2020, 10:26 AM
llvm/test/MC/Disassembler/AMDGPU/vop3_vi.txt
300

I agree, an error message would be helpful, but possibility to disassemble incorrect code is useful as well.

rampitec accepted this revision.Oct 27 2020, 10:30 AM

LGTM

This revision is now accepted and ready to land.Oct 27 2020, 10:30 AM
This revision was landed with ongoing or failed builds.Oct 27 2020, 11:45 AM
Closed by commit rG4b1ea84a1d04: [AMDGPU] Fix check prefix for VOP3 VI disassembler tests (authored by foad). · Explain Why
This revision was automatically updated to reflect the committed changes.
foad added a commit: rG4b1ea84a1d04: [AMDGPU] Fix check prefix for VOP3 VI disassembler tests.
Harbormaster completed remote builds in B76602: Diff 301051.Oct 27 2020, 11:55 AM

Revision Contents

PathSize
llvm/
test/
MC/
Disassembler/
AMDGPU/
36 lines

Diff 301021

llvm/test/MC/Disassembler/AMDGPU/vop3_vi.txt

Show First 20 Lines • Show All 255 Lines • ▼ Show 20 Lines
0x05,0x00,0xee,0xd1,0xf0,0x04,0x0e,0x04 0x05,0x00,0xee,0xd1,0xf0,0x04,0x0e,0x04
# VI: v_fma_f16 v5, v1, v2, |v3| ; encoding: [0x05,0x04,0xee,0xd1,0x01,0x05,0x0e,0x04] # VI: v_fma_f16 v5, v1, v2, |v3| ; encoding: [0x05,0x04,0xee,0xd1,0x01,0x05,0x0e,0x04]
0x05,0x04,0xee,0xd1,0x01,0x05,0x0e,0x04 0x05,0x04,0xee,0xd1,0x01,0x05,0x0e,0x04
# VI: v_fma_f16 v5, v1, v2, v3 clamp ; encoding: [0x05,0x80,0xee,0xd1,0x01,0x05,0x0e,0x04] # VI: v_fma_f16 v5, v1, v2, v3 clamp ; encoding: [0x05,0x80,0xee,0xd1,0x01,0x05,0x0e,0x04]
0x05,0x80,0xee,0xd1,0x01,0x05,0x0e,0x04 0x05,0x80,0xee,0xd1,0x01,0x05,0x0e,0x04
# CHECK: v_div_fixup_f16 v5, 0.5, v2, v3 ; encoding: [0x05,0x00,0xef,0xd1,0xf0,0x04,0x0e,0x04] # VI: v_div_fixup_f16 v5, 0.5, v2, v3 ; encoding: [0x05,0x00,0xef,0xd1,0xf0,0x04,0x0e,0x04]
0x05,0x00,0xef,0xd1,0xf0,0x04,0x0e,0x04 0x05,0x00,0xef,0xd1,0xf0,0x04,0x0e,0x04
# CHECK: v_div_fixup_f16 v5, v1, 0.5, v3 ; encoding: [0x05,0x00,0xef,0xd1,0x01,0xe1,0x0d,0x04] # VI: v_div_fixup_f16 v5, v1, 0.5, v3 ; encoding: [0x05,0x00,0xef,0xd1,0x01,0xe1,0x0d,0x04]
0x05,0x00,0xef,0xd1,0x01,0xe1,0x0d,0x04 0x05,0x00,0xef,0xd1,0x01,0xe1,0x0d,0x04
# CHECK: v_div_fixup_f16 v5, v1, v2, 0.5 ; encoding: [0x05,0x00,0xef,0xd1,0x01,0x05,0xc2,0x03] # VI: v_div_fixup_f16 v5, v1, v2, 0.5 ; encoding: [0x05,0x00,0xef,0xd1,0x01,0x05,0xc2,0x03]
0x05,0x00,0xef,0xd1,0x01,0x05,0xc2,0x03 0x05,0x00,0xef,0xd1,0x01,0x05,0xc2,0x03
# CHECK: v_div_fixup_f16 v5, -v1, -v2, -v3 ; encoding: [0x05,0x00,0xef,0xd1,0x01,0x05,0x0e,0xe4] # VI: v_div_fixup_f16 v5, -v1, -v2, -v3 ; encoding: [0x05,0x00,0xef,0xd1,0x01,0x05,0x0e,0xe4]
0x05,0x00,0xef,0xd1,0x01,0x05,0x0e,0xe4 0x05,0x00,0xef,0xd1,0x01,0x05,0x0e,0xe4
# CHECK: v_div_fixup_f16 v5, |v1|, |v2|, |v3| ; encoding: [0x05,0x07,0xef,0xd1,0x01,0x05,0x0e,0x04] # VI: v_div_fixup_f16 v5, |v1|, |v2|, |v3| ; encoding: [0x05,0x07,0xef,0xd1,0x01,0x05,0x0e,0x04]
0x05,0x07,0xef,0xd1,0x01,0x05,0x0e,0x04 0x05,0x07,0xef,0xd1,0x01,0x05,0x0e,0x04
# CHECK: v_div_fixup_f16 v5, v1, v2, v3 clamp ; encoding: [0x05,0x80,0xef,0xd1,0x01,0x05,0x0e,0x04] # VI: v_div_fixup_f16 v5, v1, v2, v3 clamp ; encoding: [0x05,0x80,0xef,0xd1,0x01,0x05,0x0e,0x04]
0x05,0x80,0xef,0xd1,0x01,0x05,0x0e,0x04 0x05,0x80,0xef,0xd1,0x01,0x05,0x0e,0x04
# CHECK: v_mad_f16 v5, 0.5, v2, v3 ; encoding: [0x05,0x00,0xea,0xd1,0xf0,0x04,0x0e,0x04] # VI: v_mad_f16 v5, 0.5, v2, v3 ; encoding: [0x05,0x00,0xea,0xd1,0xf0,0x04,0x0e,0x04]
0x05,0x00,0xea,0xd1,0xf0,0x04,0x0e,0x04 0x05,0x00,0xea,0xd1,0xf0,0x04,0x0e,0x04
# CHECK: v_mad_f16 v5, v1, 0.5, v3 ; encoding: [0x05,0x00,0xea,0xd1,0x01,0xe1,0x0d,0x04] # VI: v_mad_f16 v5, v1, 0.5, v3 ; encoding: [0x05,0x00,0xea,0xd1,0x01,0xe1,0x0d,0x04]
0x05,0x00,0xea,0xd1,0x01,0xe1,0x0d,0x04 0x05,0x00,0xea,0xd1,0x01,0xe1,0x0d,0x04
# CHECK: v_mad_f16 v5, v1, v2, 0.5 ; encoding: [0x05,0x00,0xea,0xd1,0x01,0x05,0xc2,0x03] # VI: v_mad_f16 v5, v1, v2, 0.5 ; encoding: [0x05,0x00,0xea,0xd1,0x01,0x05,0xc2,0x03]
0x05,0x00,0xea,0xd1,0x01,0x05,0xc2,0x03 0x05,0x00,0xea,0xd1,0x01,0x05,0xc2,0x03
# CHECK: v_mad_f16 v5, -v1, -v2, -v3 ; encoding: [0x05,0x00,0xea,0xd1,0x01,0x05,0x0e,0xe4] # VI: v_mad_f16 v5, -v1, -v2, -v3 ; encoding: [0x05,0x00,0xea,0xd1,0x01,0x05,0x0e,0xe4]
0x05,0x00,0xea,0xd1,0x01,0x05,0x0e,0xe4 0x05,0x00,0xea,0xd1,0x01,0x05,0x0e,0xe4
# CHECK: v_mad_f16 v5, |v1|, |v2|, |v3| ; encoding: [0x05,0x07,0xea,0xd1,0x01,0x05,0x0e,0x04] # VI: v_mad_f16 v5, |v1|, |v2|, |v3| ; encoding: [0x05,0x07,0xea,0xd1,0x01,0x05,0x0e,0x04]
0x05,0x07,0xea,0xd1,0x01,0x05,0x0e,0x04 0x05,0x07,0xea,0xd1,0x01,0x05,0x0e,0x04
# CHECK: v_mad_f16 v5, v1, v2, v3 clamp ; encoding: [0x05,0x80,0xea,0xd1,0x01,0x05,0x0e,0x04] # VI: v_mad_f16 v5, v1, v2, v3 clamp ; encoding: [0x05,0x80,0xea,0xd1,0x01,0x05,0x0e,0x04]
0x05,0x80,0xea,0xd1,0x01,0x05,0x0e,0x04 0x05,0x80,0xea,0xd1,0x01,0x05,0x0e,0x04
# CHECK: v_mad_i16 v5, 0.5, v2, v3 ; encoding: [0x05,0x00,0xec,0xd1,0xf0,0x04,0x0e,0x04] # VI: v_mad_i16 v5, 0x3800, v2, v3 ; encoding: [0x05,0x00,0xec,0xd1,0xff,0x04,0x0e,0x04]
foadAuthorUnsubmitted
Done
ReplyInline Actions

The inline constant is disassembled as 0x3800 not 0.5.

SIMCCodeEmitter does not recognise this constant as an inline 16-bit integer, so the encoding: line shows 0xff not 0xf0.

What is the correct behaviour here?

foad: The inline constant is disassembled as 0x3800 not 0.5. SIMCCodeEmitter does not recognise this…
rampitecUnsubmitted
Not Done
ReplyInline Actions

It should be f0.

rampitec: It should be f0.
dpUnsubmitted
Not Done
ReplyInline Actions

I believe 0xff is an expected result. Change 5f5f566 disabled use of fp inline constants with 16-bit integer operands. When disassembled, these inline constants are deliberately printed in hex to stress that the value is not an inline constant but a literal.

And yes, the produced dump is incorrect (missing literal) but so is the original code.

dp: I believe 0xff is an expected result. Change 5f5f566 disabled use of fp inline constants with…
rampitecUnsubmitted
Not Done
ReplyInline Actions

I would probably say the expected result is an error message if 0.5 cannot really be supported. Isn't it?

rampitec: I would probably say the expected result is an error message if 0.5 cannot really be supported.
dpUnsubmitted
Not Done
ReplyInline Actions

I agree, an error message would be helpful, but possibility to disassemble incorrect code is useful as well.

dp: I agree, an error message would be helpful, but possibility to disassemble incorrect code is…
0x05,0x00,0xec,0xd1,0xf0,0x04,0x0e,0x04 0x05,0x00,0xec,0xd1,0xf0,0x04,0x0e,0x04
# CHECK: v_mad_i16 v5, v1, 0.5, v3 ; encoding: [0x05,0x00,0xec,0xd1,0x01,0xe1,0x0d,0x04] # VI: v_mad_i16 v5, v1, 0x3800, v3 ; encoding: [0x05,0x00,0xec,0xd1,0x01,0xff,0x0d,0x04]
0x05,0x00,0xec,0xd1,0x01,0xe1,0x0d,0x04 0x05,0x00,0xec,0xd1,0x01,0xe1,0x0d,0x04
# CHECK: v_mad_i16 v5, v1, v2, 0.5 ; encoding: [0x05,0x00,0xec,0xd1,0x01,0x05,0xc2,0x03] # VI: v_mad_i16 v5, v1, v2, 0x3800 ; encoding: [0x05,0x00,0xec,0xd1,0x01,0x05,0xfe,0x03]
0x05,0x00,0xec,0xd1,0x01,0x05,0xc2,0x03 0x05,0x00,0xec,0xd1,0x01,0x05,0xc2,0x03
# CHECK: v_mad_u16 v5, 0, v2, v3 ; encoding: [0x05,0x00,0xeb,0xd1,0x80,0x04,0x0e,0x04] # VI: v_mad_u16 v5, 0, v2, v3 ; encoding: [0x05,0x00,0xeb,0xd1,0x80,0x04,0x0e,0x04]
0x05,0x00,0xeb,0xd1,0x80,0x04,0x0e,0x04 0x05,0x00,0xeb,0xd1,0x80,0x04,0x0e,0x04
# CHECK: v_mad_u16 v5, v1, -1, v3 ; encoding: [0x05,0x00,0xeb,0xd1,0x01,0x83,0x0d,0x04] # VI: v_mad_u16 v5, v1, -1, v3 ; encoding: [0x05,0x00,0xeb,0xd1,0x01,0x83,0x0d,0x04]
0x05,0x00,0xeb,0xd1,0x01,0x83,0x0d,0x04 0x05,0x00,0xeb,0xd1,0x01,0x83,0x0d,0x04
# CHECK: v_mad_u16 v5, v1, v2, -4.0 ; encoding: [0x05,0x00,0xeb,0xd1,0x01,0x05,0xde,0x03] # VI: v_mad_u16 v5, v1, v2, 0xc400 ; encoding: [0x05,0x00,0xeb,0xd1,0x01,0x05,0xfe,0x03]
0x05,0x00,0xeb,0xd1,0x01,0x05,0xde,0x03 0x05,0x00,0xeb,0xd1,0x01,0x05,0xde,0x03
# VI: v_interp_mov_f32_e64 v5, p10, attr0.x ; encoding: [0x05,0x00,0x72,0xd2,0x00,0x00,0x00,0x00] # VI: v_interp_mov_f32_e64 v5, p10, attr0.x ; encoding: [0x05,0x00,0x72,0xd2,0x00,0x00,0x00,0x00]
0x05,0x00,0x72,0xd2,0x00,0x00,0x00,0x00 0x05,0x00,0x72,0xd2,0x00,0x00,0x00,0x00
# VI: v_interp_mov_f32_e64 v5, p10, attr32.x ; encoding: [0x05,0x00,0x72,0xd2,0x20,0x00,0x00,0x00] # VI: v_interp_mov_f32_e64 v5, p10, attr32.x ; encoding: [0x05,0x00,0x72,0xd2,0x20,0x00,0x00,0x00]
0x05,0x00,0x72,0xd2,0x20,0x00,0x00,0x00 0x05,0x00,0x72,0xd2,0x20,0x00,0x00,0x00
▲ Show 20 Lines • Show All 189 Lines • Show Last 20 Lines