[llvm] e23891a - [AMDGPU][Disassembler] Fix a spurious error message in an instruction comment.
Ivan Kosarev via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 26 04:50:40 PDT 2023
Author: Ivan Kosarev
Date: 2023-04-26T12:50:17+01:00
New Revision: e23891a3823e6bff11a68c08b75d2d45d11832cf
URL: https://github.com/llvm/llvm-project/commit/e23891a3823e6bff11a68c08b75d2d45d11832cf
DIFF: https://github.com/llvm/llvm-project/commit/e23891a3823e6bff11a68c08b75d2d45d11832cf.diff
LOG: [AMDGPU][Disassembler] Fix a spurious error message in an instruction comment.
The patch prevents pollution of instruction comments with error messages
generated during unsuccessful decoding attempts.
Reviewed By: foad
Differential Revision: https://reviews.llvm.org/D149049
Added:
Modified:
llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h
llvm/test/MC/Disassembler/AMDGPU/comments.txt
Removed:
################################################################################
diff --git a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
index 9fcc8d5480def..6ad59e026003e 100644
--- a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
+++ b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
@@ -413,7 +413,6 @@ DecodeStatus AMDGPUDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
ArrayRef<uint8_t> Bytes_,
uint64_t Address,
raw_ostream &CS) const {
- CommentStream = &CS;
bool IsSDWA = false;
unsigned MaxInstBytesNum = std::min((size_t)TargetMaxInstBytes, Bytes_.size());
@@ -428,13 +427,11 @@ DecodeStatus AMDGPUDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
// encodings
if (isGFX11Plus() && Bytes.size() >= 12 ) {
DecoderUInt128 DecW = eat12Bytes(Bytes);
- Res = tryDecodeInst(DecoderTableDPP8GFX1196, MI, DecW,
- Address);
+ Res = tryDecodeInst(DecoderTableDPP8GFX1196, MI, DecW, Address, CS);
if (Res && convertDPP8Inst(MI) == MCDisassembler::Success)
break;
MI = MCInst(); // clear
- Res = tryDecodeInst(DecoderTableDPPGFX1196, MI, DecW,
- Address);
+ Res = tryDecodeInst(DecoderTableDPPGFX1196, MI, DecW, Address, CS);
if (Res) {
if (MCII->get(MI.getOpcode()).TSFlags & SIInstrFlags::VOP3P)
convertVOP3PDPPInst(MI);
@@ -446,7 +443,7 @@ DecodeStatus AMDGPUDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
}
break;
}
- Res = tryDecodeInst(DecoderTableGFX1196, MI, DecW, Address);
+ Res = tryDecodeInst(DecoderTableGFX1196, MI, DecW, Address, CS);
if (Res)
break;
}
@@ -457,7 +454,7 @@ DecodeStatus AMDGPUDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
const uint64_t QW = eatBytes<uint64_t>(Bytes);
if (STI.hasFeature(AMDGPU::FeatureGFX10_BEncoding)) {
- Res = tryDecodeInst(DecoderTableGFX10_B64, MI, QW, Address);
+ Res = tryDecodeInst(DecoderTableGFX10_B64, MI, QW, Address, CS);
if (Res) {
if (AMDGPU::getNamedOperandIdx(MI.getOpcode(), AMDGPU::OpName::dpp8)
== -1)
@@ -468,37 +465,37 @@ DecodeStatus AMDGPUDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
}
}
- Res = tryDecodeInst(DecoderTableDPP864, MI, QW, Address);
+ Res = tryDecodeInst(DecoderTableDPP864, MI, QW, Address, CS);
if (Res && convertDPP8Inst(MI) == MCDisassembler::Success)
break;
MI = MCInst(); // clear
- Res = tryDecodeInst(DecoderTableDPP8GFX1164, MI, QW, Address);
+ Res = tryDecodeInst(DecoderTableDPP8GFX1164, MI, QW, Address, CS);
if (Res && convertDPP8Inst(MI) == MCDisassembler::Success)
break;
MI = MCInst(); // clear
- Res = tryDecodeInst(DecoderTableDPP64, MI, QW, Address);
+ Res = tryDecodeInst(DecoderTableDPP64, MI, QW, Address, CS);
if (Res) break;
- Res = tryDecodeInst(DecoderTableDPPGFX1164, MI, QW, Address);
+ Res = tryDecodeInst(DecoderTableDPPGFX1164, MI, QW, Address, CS);
if (Res) {
if (MCII->get(MI.getOpcode()).TSFlags & SIInstrFlags::VOPC)
convertVOPCDPPInst(MI);
break;
}
- Res = tryDecodeInst(DecoderTableSDWA64, MI, QW, Address);
+ Res = tryDecodeInst(DecoderTableSDWA64, MI, QW, Address, CS);
if (Res) { IsSDWA = true; break; }
- Res = tryDecodeInst(DecoderTableSDWA964, MI, QW, Address);
+ Res = tryDecodeInst(DecoderTableSDWA964, MI, QW, Address, CS);
if (Res) { IsSDWA = true; break; }
- Res = tryDecodeInst(DecoderTableSDWA1064, MI, QW, Address);
+ Res = tryDecodeInst(DecoderTableSDWA1064, MI, QW, Address, CS);
if (Res) { IsSDWA = true; break; }
if (STI.hasFeature(AMDGPU::FeatureUnpackedD16VMem)) {
- Res = tryDecodeInst(DecoderTableGFX80_UNPACKED64, MI, QW, Address);
+ Res = tryDecodeInst(DecoderTableGFX80_UNPACKED64, MI, QW, Address, CS);
if (Res)
break;
}
@@ -507,7 +504,7 @@ DecodeStatus AMDGPUDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
// v_mad_mixhi_f16 for FMA variants. Try to decode using this special
// table first so we print the correct name.
if (STI.hasFeature(AMDGPU::FeatureFmaMixInsts)) {
- Res = tryDecodeInst(DecoderTableGFX9_DL64, MI, QW, Address);
+ Res = tryDecodeInst(DecoderTableGFX9_DL64, MI, QW, Address, CS);
if (Res)
break;
}
@@ -519,64 +516,64 @@ DecodeStatus AMDGPUDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
// Try decode 32-bit instruction
if (Bytes.size() < 4) break;
const uint32_t DW = eatBytes<uint32_t>(Bytes);
- Res = tryDecodeInst(DecoderTableGFX832, MI, DW, Address);
+ Res = tryDecodeInst(DecoderTableGFX832, MI, DW, Address, CS);
if (Res) break;
- Res = tryDecodeInst(DecoderTableAMDGPU32, MI, DW, Address);
+ Res = tryDecodeInst(DecoderTableAMDGPU32, MI, DW, Address, CS);
if (Res) break;
- Res = tryDecodeInst(DecoderTableGFX932, MI, DW, Address);
+ Res = tryDecodeInst(DecoderTableGFX932, MI, DW, Address, CS);
if (Res) break;
if (STI.hasFeature(AMDGPU::FeatureGFX90AInsts)) {
- Res = tryDecodeInst(DecoderTableGFX90A32, MI, DW, Address);
+ Res = tryDecodeInst(DecoderTableGFX90A32, MI, DW, Address, CS);
if (Res)
break;
}
if (STI.hasFeature(AMDGPU::FeatureGFX10_BEncoding)) {
- Res = tryDecodeInst(DecoderTableGFX10_B32, MI, DW, Address);
+ Res = tryDecodeInst(DecoderTableGFX10_B32, MI, DW, Address, CS);
if (Res) break;
}
- Res = tryDecodeInst(DecoderTableGFX1032, MI, DW, Address);
+ Res = tryDecodeInst(DecoderTableGFX1032, MI, DW, Address, CS);
if (Res) break;
- Res = tryDecodeInst(DecoderTableGFX1132, MI, DW, Address);
+ Res = tryDecodeInst(DecoderTableGFX1132, MI, DW, Address, CS);
if (Res) break;
if (Bytes.size() < 4) break;
const uint64_t QW = ((uint64_t)eatBytes<uint32_t>(Bytes) << 32) | DW;
if (STI.hasFeature(AMDGPU::FeatureGFX940Insts)) {
- Res = tryDecodeInst(DecoderTableGFX94064, MI, QW, Address);
+ Res = tryDecodeInst(DecoderTableGFX94064, MI, QW, Address, CS);
if (Res)
break;
}
if (STI.hasFeature(AMDGPU::FeatureGFX90AInsts)) {
- Res = tryDecodeInst(DecoderTableGFX90A64, MI, QW, Address);
+ Res = tryDecodeInst(DecoderTableGFX90A64, MI, QW, Address, CS);
if (Res)
break;
}
- Res = tryDecodeInst(DecoderTableGFX864, MI, QW, Address);
+ Res = tryDecodeInst(DecoderTableGFX864, MI, QW, Address, CS);
if (Res) break;
- Res = tryDecodeInst(DecoderTableAMDGPU64, MI, QW, Address);
+ Res = tryDecodeInst(DecoderTableAMDGPU64, MI, QW, Address, CS);
if (Res) break;
- Res = tryDecodeInst(DecoderTableGFX964, MI, QW, Address);
+ Res = tryDecodeInst(DecoderTableGFX964, MI, QW, Address, CS);
if (Res) break;
- Res = tryDecodeInst(DecoderTableGFX1064, MI, QW, Address);
+ Res = tryDecodeInst(DecoderTableGFX1064, MI, QW, Address, CS);
if (Res) break;
- Res = tryDecodeInst(DecoderTableGFX1164, MI, QW, Address);
+ Res = tryDecodeInst(DecoderTableGFX1164, MI, QW, Address, CS);
if (Res)
break;
- Res = tryDecodeInst(DecoderTableWMMAGFX1164, MI, QW, Address);
+ Res = tryDecodeInst(DecoderTableWMMAGFX1164, MI, QW, Address, CS);
} while (false);
if (Res && AMDGPU::isMAC(MI.getOpcode())) {
diff --git a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h
index 5b9a1551a989e..490eb14c3396f 100644
--- a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h
+++ b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h
@@ -115,14 +115,25 @@ class AMDGPUDisassembler : public MCDisassembler {
template <typename InsnType>
DecodeStatus tryDecodeInst(const uint8_t *Table, MCInst &MI, InsnType Inst,
- uint64_t Address) const {
+ uint64_t Address, raw_ostream &Comments) const {
assert(MI.getOpcode() == 0);
assert(MI.getNumOperands() == 0);
MCInst TmpInst;
HasLiteral = false;
const auto SavedBytes = Bytes;
- if (decodeInstruction(Table, TmpInst, Inst, Address, this, STI)) {
+
+ SmallString<64> LocalComments;
+ raw_svector_ostream LocalCommentStream(LocalComments);
+ CommentStream = &LocalCommentStream;
+
+ DecodeStatus Res =
+ decodeInstruction(Table, TmpInst, Inst, Address, this, STI);
+
+ CommentStream = nullptr;
+
+ if (Res != Fail) {
MI = TmpInst;
+ Comments << LocalComments;
return MCDisassembler::Success;
}
Bytes = SavedBytes;
diff --git a/llvm/test/MC/Disassembler/AMDGPU/comments.txt b/llvm/test/MC/Disassembler/AMDGPU/comments.txt
index 36fd182a717ad..c47c8da79066f 100644
--- a/llvm/test/MC/Disassembler/AMDGPU/comments.txt
+++ b/llvm/test/MC/Disassembler/AMDGPU/comments.txt
@@ -1,5 +1,8 @@
# RUN: llvm-mc -assemble -triple=amdgcn--amdhsa -mcpu=gfx1100 -filetype=obj %s -o - | \
# RUN: llvm-objdump -d - | FileCheck %s
-; CHECK: v_perm_b32 v14, v52, v51, 0x5040100 // 000000000000: D644000E 03FE6734 05040100 ; Error: cannot read literal, inst bytes left 0{{$}}
+; Make sure disassembling of this instruction does not cause any errors
+; generated in the comment. For this test to do its job, the instruction
+; has to be the last or the only one in the object file.
+; CHECK: v_perm_b32 v14, v52, v51, 0x5040100 // 000000000000: D644000E 03FE6734 05040100{{$}}
v_perm_b32 v14, v52, v51, 0x5040100
More information about the llvm-commits
mailing list