Skip to content

Commit e23891a

Browse files
kosarevkosarev
committed
[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
1 parent c08dc8b commit e23891a

File tree

3 files changed

+45
-34
lines changed

3 files changed

+45
-34
lines changed

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

Lines changed: 28 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -413,7 +413,6 @@ DecodeStatus AMDGPUDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
413413
ArrayRef<uint8_t> Bytes_,
414414
uint64_t Address,
415415
raw_ostream &CS) const {
416-
CommentStream = &CS;
417416
bool IsSDWA = false;
418417

419418
unsigned MaxInstBytesNum = std::min((size_t)TargetMaxInstBytes, Bytes_.size());
@@ -428,13 +427,11 @@ DecodeStatus AMDGPUDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
428427
// encodings
429428
if (isGFX11Plus() && Bytes.size() >= 12 ) {
430429
DecoderUInt128 DecW = eat12Bytes(Bytes);
431-
Res = tryDecodeInst(DecoderTableDPP8GFX1196, MI, DecW,
432-
Address);
430+
Res = tryDecodeInst(DecoderTableDPP8GFX1196, MI, DecW, Address, CS);
433431
if (Res && convertDPP8Inst(MI) == MCDisassembler::Success)
434432
break;
435433
MI = MCInst(); // clear
436-
Res = tryDecodeInst(DecoderTableDPPGFX1196, MI, DecW,
437-
Address);
434+
Res = tryDecodeInst(DecoderTableDPPGFX1196, MI, DecW, Address, CS);
438435
if (Res) {
439436
if (MCII->get(MI.getOpcode()).TSFlags & SIInstrFlags::VOP3P)
440437
convertVOP3PDPPInst(MI);
@@ -446,7 +443,7 @@ DecodeStatus AMDGPUDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
446443
}
447444
break;
448445
}
449-
Res = tryDecodeInst(DecoderTableGFX1196, MI, DecW, Address);
446+
Res = tryDecodeInst(DecoderTableGFX1196, MI, DecW, Address, CS);
450447
if (Res)
451448
break;
452449
}
@@ -457,7 +454,7 @@ DecodeStatus AMDGPUDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
457454
const uint64_t QW = eatBytes<uint64_t>(Bytes);
458455

459456
if (STI.hasFeature(AMDGPU::FeatureGFX10_BEncoding)) {
460-
Res = tryDecodeInst(DecoderTableGFX10_B64, MI, QW, Address);
457+
Res = tryDecodeInst(DecoderTableGFX10_B64, MI, QW, Address, CS);
461458
if (Res) {
462459
if (AMDGPU::getNamedOperandIdx(MI.getOpcode(), AMDGPU::OpName::dpp8)
463460
== -1)
@@ -468,37 +465,37 @@ DecodeStatus AMDGPUDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
468465
}
469466
}
470467

471-
Res = tryDecodeInst(DecoderTableDPP864, MI, QW, Address);
468+
Res = tryDecodeInst(DecoderTableDPP864, MI, QW, Address, CS);
472469
if (Res && convertDPP8Inst(MI) == MCDisassembler::Success)
473470
break;
474471
MI = MCInst(); // clear
475472

476-
Res = tryDecodeInst(DecoderTableDPP8GFX1164, MI, QW, Address);
473+
Res = tryDecodeInst(DecoderTableDPP8GFX1164, MI, QW, Address, CS);
477474
if (Res && convertDPP8Inst(MI) == MCDisassembler::Success)
478475
break;
479476
MI = MCInst(); // clear
480477

481-
Res = tryDecodeInst(DecoderTableDPP64, MI, QW, Address);
478+
Res = tryDecodeInst(DecoderTableDPP64, MI, QW, Address, CS);
482479
if (Res) break;
483480

484-
Res = tryDecodeInst(DecoderTableDPPGFX1164, MI, QW, Address);
481+
Res = tryDecodeInst(DecoderTableDPPGFX1164, MI, QW, Address, CS);
485482
if (Res) {
486483
if (MCII->get(MI.getOpcode()).TSFlags & SIInstrFlags::VOPC)
487484
convertVOPCDPPInst(MI);
488485
break;
489486
}
490487

491-
Res = tryDecodeInst(DecoderTableSDWA64, MI, QW, Address);
488+
Res = tryDecodeInst(DecoderTableSDWA64, MI, QW, Address, CS);
492489
if (Res) { IsSDWA = true; break; }
493490

494-
Res = tryDecodeInst(DecoderTableSDWA964, MI, QW, Address);
491+
Res = tryDecodeInst(DecoderTableSDWA964, MI, QW, Address, CS);
495492
if (Res) { IsSDWA = true; break; }
496493

497-
Res = tryDecodeInst(DecoderTableSDWA1064, MI, QW, Address);
494+
Res = tryDecodeInst(DecoderTableSDWA1064, MI, QW, Address, CS);
498495
if (Res) { IsSDWA = true; break; }
499496

500497
if (STI.hasFeature(AMDGPU::FeatureUnpackedD16VMem)) {
501-
Res = tryDecodeInst(DecoderTableGFX80_UNPACKED64, MI, QW, Address);
498+
Res = tryDecodeInst(DecoderTableGFX80_UNPACKED64, MI, QW, Address, CS);
502499
if (Res)
503500
break;
504501
}
@@ -507,7 +504,7 @@ DecodeStatus AMDGPUDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
507504
// v_mad_mixhi_f16 for FMA variants. Try to decode using this special
508505
// table first so we print the correct name.
509506
if (STI.hasFeature(AMDGPU::FeatureFmaMixInsts)) {
510-
Res = tryDecodeInst(DecoderTableGFX9_DL64, MI, QW, Address);
507+
Res = tryDecodeInst(DecoderTableGFX9_DL64, MI, QW, Address, CS);
511508
if (Res)
512509
break;
513510
}
@@ -519,64 +516,64 @@ DecodeStatus AMDGPUDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
519516
// Try decode 32-bit instruction
520517
if (Bytes.size() < 4) break;
521518
const uint32_t DW = eatBytes<uint32_t>(Bytes);
522-
Res = tryDecodeInst(DecoderTableGFX832, MI, DW, Address);
519+
Res = tryDecodeInst(DecoderTableGFX832, MI, DW, Address, CS);
523520
if (Res) break;
524521

525-
Res = tryDecodeInst(DecoderTableAMDGPU32, MI, DW, Address);
522+
Res = tryDecodeInst(DecoderTableAMDGPU32, MI, DW, Address, CS);
526523
if (Res) break;
527524

528-
Res = tryDecodeInst(DecoderTableGFX932, MI, DW, Address);
525+
Res = tryDecodeInst(DecoderTableGFX932, MI, DW, Address, CS);
529526
if (Res) break;
530527

531528
if (STI.hasFeature(AMDGPU::FeatureGFX90AInsts)) {
532-
Res = tryDecodeInst(DecoderTableGFX90A32, MI, DW, Address);
529+
Res = tryDecodeInst(DecoderTableGFX90A32, MI, DW, Address, CS);
533530
if (Res)
534531
break;
535532
}
536533

537534
if (STI.hasFeature(AMDGPU::FeatureGFX10_BEncoding)) {
538-
Res = tryDecodeInst(DecoderTableGFX10_B32, MI, DW, Address);
535+
Res = tryDecodeInst(DecoderTableGFX10_B32, MI, DW, Address, CS);
539536
if (Res) break;
540537
}
541538

542-
Res = tryDecodeInst(DecoderTableGFX1032, MI, DW, Address);
539+
Res = tryDecodeInst(DecoderTableGFX1032, MI, DW, Address, CS);
543540
if (Res) break;
544541

545-
Res = tryDecodeInst(DecoderTableGFX1132, MI, DW, Address);
542+
Res = tryDecodeInst(DecoderTableGFX1132, MI, DW, Address, CS);
546543
if (Res) break;
547544

548545
if (Bytes.size() < 4) break;
549546
const uint64_t QW = ((uint64_t)eatBytes<uint32_t>(Bytes) << 32) | DW;
550547

551548
if (STI.hasFeature(AMDGPU::FeatureGFX940Insts)) {
552-
Res = tryDecodeInst(DecoderTableGFX94064, MI, QW, Address);
549+
Res = tryDecodeInst(DecoderTableGFX94064, MI, QW, Address, CS);
553550
if (Res)
554551
break;
555552
}
556553

557554
if (STI.hasFeature(AMDGPU::FeatureGFX90AInsts)) {
558-
Res = tryDecodeInst(DecoderTableGFX90A64, MI, QW, Address);
555+
Res = tryDecodeInst(DecoderTableGFX90A64, MI, QW, Address, CS);
559556
if (Res)
560557
break;
561558
}
562559

563-
Res = tryDecodeInst(DecoderTableGFX864, MI, QW, Address);
560+
Res = tryDecodeInst(DecoderTableGFX864, MI, QW, Address, CS);
564561
if (Res) break;
565562

566-
Res = tryDecodeInst(DecoderTableAMDGPU64, MI, QW, Address);
563+
Res = tryDecodeInst(DecoderTableAMDGPU64, MI, QW, Address, CS);
567564
if (Res) break;
568565

569-
Res = tryDecodeInst(DecoderTableGFX964, MI, QW, Address);
566+
Res = tryDecodeInst(DecoderTableGFX964, MI, QW, Address, CS);
570567
if (Res) break;
571568

572-
Res = tryDecodeInst(DecoderTableGFX1064, MI, QW, Address);
569+
Res = tryDecodeInst(DecoderTableGFX1064, MI, QW, Address, CS);
573570
if (Res) break;
574571

575-
Res = tryDecodeInst(DecoderTableGFX1164, MI, QW, Address);
572+
Res = tryDecodeInst(DecoderTableGFX1164, MI, QW, Address, CS);
576573
if (Res)
577574
break;
578575

579-
Res = tryDecodeInst(DecoderTableWMMAGFX1164, MI, QW, Address);
576+
Res = tryDecodeInst(DecoderTableWMMAGFX1164, MI, QW, Address, CS);
580577
} while (false);
581578

582579
if (Res && AMDGPU::isMAC(MI.getOpcode())) {

‎llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,14 +115,25 @@ class AMDGPUDisassembler : public MCDisassembler {
115115

116116
template <typename InsnType>
117117
DecodeStatus tryDecodeInst(const uint8_t *Table, MCInst &MI, InsnType Inst,
118-
uint64_t Address) const {
118+
uint64_t Address, raw_ostream &Comments) const {
119119
assert(MI.getOpcode() == 0);
120120
assert(MI.getNumOperands() == 0);
121121
MCInst TmpInst;
122122
HasLiteral = false;
123123
const auto SavedBytes = Bytes;
124-
if (decodeInstruction(Table, TmpInst, Inst, Address, this, STI)) {
124+
125+
SmallString<64> LocalComments;
126+
raw_svector_ostream LocalCommentStream(LocalComments);
127+
CommentStream = &LocalCommentStream;
128+
129+
DecodeStatus Res =
130+
decodeInstruction(Table, TmpInst, Inst, Address, this, STI);
131+
132+
CommentStream = nullptr;
133+
134+
if (Res != Fail) {
125135
MI = TmpInst;
136+
Comments << LocalComments;
126137
return MCDisassembler::Success;
127138
}
128139
Bytes = SavedBytes;

‎llvm/test/MC/Disassembler/AMDGPU/comments.txt

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
# RUN: llvm-mc -assemble -triple=amdgcn--amdhsa -mcpu=gfx1100 -filetype=obj %s -o - | \
22
# RUN: llvm-objdump -d - | FileCheck %s
33

4-
; CHECK: v_perm_b32 v14, v52, v51, 0x5040100 // 000000000000: D644000E 03FE6734 05040100 ; Error: cannot read literal, inst bytes left 0{{$}}
4+
; Make sure disassembling of this instruction does not cause any errors
5+
; generated in the comment. For this test to do its job, the instruction
6+
; has to be the last or the only one in the object file.
7+
; CHECK: v_perm_b32 v14, v52, v51, 0x5040100 // 000000000000: D644000E 03FE6734 05040100{{$}}
58
v_perm_b32 v14, v52, v51, 0x5040100

0 commit comments

Comments
 (0)