This is an archive of the discontinued LLVM Phabricator instance.

Differential D80637

[AMDGPU][MC][DISASSEMBLER] Corrected decoder to consume each code fragment once
ClosedPublic

Authored by dp on May 27 2020, 8:28 AM.

Details

Reviewers
arsenm
rampitec
Commits
rGbab5dadfcd0f: [AMDGPU][MC][DISASSEMBLER] Corrected decoder to consume each code fragment only…
Summary

Current implementation may decode some fragments of code twice. This result in other bugs being hidden.

See detailed bug description.

Decoding of v_writelane_b32 will be corrected by a separate change.

Diff Detail

Event Timeline

dp created this revision.May 27 2020, 8:28 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 27 2020, 8:28 AM
Herald added subscribers: llvm-commits, kerbowa, hiraditya and 8 others. · View Herald Transcript
dp added a child revision: D80636: [AMDGPU][MC] Corrected v_writelane_b32 to fix a decoding bug.May 27 2020, 8:36 AM
rampitec accepted this revision.May 27 2020, 9:56 AM
rampitec added inline comments.
llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
365

I assume this became unneeded with some other change.

This revision is now accepted and ready to land.May 27 2020, 9:56 AM
Closed by commit rGbab5dadfcd0f: [AMDGPU][MC][DISASSEMBLER] Corrected decoder to consume each code fragment only… (authored by dp). · Explain WhyMay 28 2020, 4:51 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
llvm/
lib/
Target/
AMDGPU/
Disassembler/
7 lines
test/
MC/
Disassembler/
AMDGPU/
4 lines

Diff 266818

llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp

Show First 20 Lines • Show All 356 Lines • ▼ Show 20 Lines do {
if (Res) break; if (Res) break;
Res = tryDecodeInst(DecoderTableGFX964, MI, QW, Address); Res = tryDecodeInst(DecoderTableGFX964, MI, QW, Address);
if (Res) break; if (Res) break;
Res = tryDecodeInst(DecoderTableGFX1064, MI, QW, Address); Res = tryDecodeInst(DecoderTableGFX1064, MI, QW, Address);
} while (false); } while (false);
if (Res && (MaxInstBytesNum - Bytes.size()) == 12 && (!HasLiteral ||
rampitecUnsubmitted
Not Done
ReplyInline Actions

I assume this became unneeded with some other change.

rampitec: I assume this became unneeded with some other change.
!(MCII->get(MI.getOpcode()).TSFlags & SIInstrFlags::VOP3))) {
MaxInstBytesNum = 8;
Bytes = Bytes_.slice(0, MaxInstBytesNum);
eatBytes<uint64_t>(Bytes);
}
if (Res && (MI.getOpcode() == AMDGPU::V_MAC_F32_e64_vi || if (Res && (MI.getOpcode() == AMDGPU::V_MAC_F32_e64_vi ||
MI.getOpcode() == AMDGPU::V_MAC_F32_e64_gfx6_gfx7 || MI.getOpcode() == AMDGPU::V_MAC_F32_e64_gfx6_gfx7 ||
MI.getOpcode() == AMDGPU::V_MAC_F32_e64_gfx10 || MI.getOpcode() == AMDGPU::V_MAC_F32_e64_gfx10 ||
MI.getOpcode() == AMDGPU::V_MAC_F16_e64_vi || MI.getOpcode() == AMDGPU::V_MAC_F16_e64_vi ||
MI.getOpcode() == AMDGPU::V_FMAC_F32_e64_vi || MI.getOpcode() == AMDGPU::V_FMAC_F32_e64_vi ||
MI.getOpcode() == AMDGPU::V_FMAC_F32_e64_gfx10 || MI.getOpcode() == AMDGPU::V_FMAC_F32_e64_gfx10 ||
MI.getOpcode() == AMDGPU::V_FMAC_F16_e64_gfx10)) { MI.getOpcode() == AMDGPU::V_FMAC_F16_e64_gfx10)) {
// Insert dummy unused src2_modifiers. // Insert dummy unused src2_modifiers.
▲ Show 20 Lines • Show All 891 Lines • Show Last 20 Lines

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

Show First 20 Lines • Show All 65,527 Lines • ▼ Show 20 Lines
0x05,0x00,0x97,0xd5,0xfe,0x01,0x00,0x00 0x05,0x00,0x97,0xd5,0xfe,0x01,0x00,0x00
# GFX10: v_trunc_f64_e64 v[5:6], vcc ; encoding: [0x05,0x00,0x97,0xd5,0x6a,0x00,0x00,0x00] # GFX10: v_trunc_f64_e64 v[5:6], vcc ; encoding: [0x05,0x00,0x97,0xd5,0x6a,0x00,0x00,0x00]
0x05,0x00,0x97,0xd5,0x6a,0x00,0x00,0x00 0x05,0x00,0x97,0xd5,0x6a,0x00,0x00,0x00
# GFX10: v_trunc_f64_e64 v[5:6], |v[1:2]| ; encoding: [0x05,0x01,0x97,0xd5,0x01,0x01,0x00,0x00] # GFX10: v_trunc_f64_e64 v[5:6], |v[1:2]| ; encoding: [0x05,0x01,0x97,0xd5,0x01,0x01,0x00,0x00]
0x05,0x01,0x97,0xd5,0x01,0x01,0x00,0x00 0x05,0x01,0x97,0xd5,0x01,0x01,0x00,0x00
# GFX10: v_writelane_b32 v255, 0, s2 ; encoding: [0xff,0x00,0x61,0xd7,0x80,0x04,0x00,0x00] # GFX10-FIXME: v_writelane_b32 v255, 0, s2 ; encoding: [0xff,0x00,0x61,0xd7,0x80,0x04,0x00,0x00]
0xff,0x00,0x61,0xd7,0x80,0x04,0x00,0x00 # 0xff,0x00,0x61,0xd7,0x80,0x04,0x00,0x00
# GFX10: v_writelane_b32 v5, -1, s2 ; encoding: [0x05,0x00,0x61,0xd7,0xc1,0x04,0x00,0x00] # GFX10: v_writelane_b32 v5, -1, s2 ; encoding: [0x05,0x00,0x61,0xd7,0xc1,0x04,0x00,0x00]
0x05,0x00,0x61,0xd7,0xc1,0x04,0x00,0x00 0x05,0x00,0x61,0xd7,0xc1,0x04,0x00,0x00
# GFX10: v_writelane_b32 v5, -4.0, s2 ; encoding: [0x05,0x00,0x61,0xd7,0xf7,0x04,0x00,0x00] # GFX10: v_writelane_b32 v5, -4.0, s2 ; encoding: [0x05,0x00,0x61,0xd7,0xf7,0x04,0x00,0x00]
0x05,0x00,0x61,0xd7,0xf7,0x04,0x00,0x00 0x05,0x00,0x61,0xd7,0xf7,0x04,0x00,0x00
# GFX10: v_writelane_b32 v5, 0, 0 ; encoding: [0x05,0x00,0x61,0xd7,0x80,0x00,0x01,0x00] # GFX10: v_writelane_b32 v5, 0, 0 ; encoding: [0x05,0x00,0x61,0xd7,0x80,0x00,0x01,0x00]
▲ Show 20 Lines • Show All 994 Lines • Show Last 20 Lines