[AMDGPU] Improve disassembler error handling

Summary:
llvm-objdump now disassembles unrecognised opcodes as data, using
the .long directive. We treat unrecognised opcodes as being 32 bit
values, so move along 4 bytes rather than the single byte which
previously resulted in a cascade of bogus disassembly following an
unrecognised opcode.

While no solution can always disassemble code that contains
embedded data correctly this provides a significant improvement.

The disassembler will now cope with an arbitrary length section
as it no longer truncates it to a multiple of 4 bytes, and will
use the .byte directive for trailing bytes.

Subscribers: arsenm, kzhuravl, wdng, nhaehnle, yaxunl, dstuttard, tpr, t-tye, llvm-commits

Differential Revision: https://reviews.llvm.org/D44685

llvm-svn: 328553

diff --git a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
index 75356e9..9732d11 100644 (file)
--- a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
+++ b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
@@ -246,7 +246,10 @@ DecodeStatus AMDGPUDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
   if (Res && IsSDWA)
     Res = convertSDWAInst(MI);
 
-  Size = Res ? (MaxInstBytesNum - Bytes.size()) : 0;
+  // if the opcode was not recognized we'll assume a Size of 4 bytes
+  // (unless there are fewer bytes left)
+  Size = Res ? (MaxInstBytesNum - Bytes.size())
+             : std::min((size_t)4, Bytes_.size());
   return Res;
 }
 

diff --git a/llvm/test/MC/AMDGPU/data.s b/llvm/test/MC/AMDGPU/data.s
new file mode 100644 (file)
index 0000000..c967534
--- /dev/null
+++ b/llvm/test/MC/AMDGPU/data.s
@@ -0,0 +1,27 @@
+// We check that unrecognized opcodes are disassembled by llvm-objdump as data using the .long directive 
+// and any trailing bytes are disassembled using the .byte directive
+// RUN: llvm-mc -filetype=obj -triple=amdgcn--amdpal -mcpu=gfx900 -show-encoding %s | llvm-objdump -disassemble -mcpu=gfx900 - | FileCheck %s
+
+.text
+        v_mov_b32     v7, s24
+       v_mov_b32     v8, s25
+       .long         0xabadc0de
+       s_nop         0
+       s_endpgm                  
+       .long         0xbadc0de1, 0xbadc0de2, 0xbadc0de3, 0xbadc0de4
+       .byte         0x0a, 0x0b
+       .byte         0x0c
+
+// CHECK: .text
+// CHECK: v_mov_b32
+// CHECK: v_mov_b32
+// CHECK: .long 0xabadc0de
+// CHECK_SAME: : ABADC0DE
+// CHECK: s_endpgm
+// CHECK: .long 0xbadc0de1
+// CHECK: .long 0xbadc0de2
+// CHECK: .long 0xbadc0de3
+// CHECK: .long 0xbadc0de4
+// CHECK: .byte 0x0a, 0x0b, 0x0c
+// CHECK-SAME: : 0A 0B 0C
+// CHECK-NOT: .long

diff --git a/llvm/tools/llvm-objdump/llvm-objdump.cpp b/llvm/tools/llvm-objdump/llvm-objdump.cpp
index 7764c1d..53b720e 100644 (file)
--- a/llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ b/llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -595,23 +595,42 @@ public:
     if (SP && (PrintSource || PrintLines))
       SP->printSourceLine(OS, Address);
 
-    if (!MI) {
-      OS << " <unknown>";
-      return;
-    }
+    typedef support::ulittle32_t U32;
 
-    SmallString<40> InstStr;
-    raw_svector_ostream IS(InstStr);
+    if (MI) {
+      SmallString<40> InstStr;
+      raw_svector_ostream IS(InstStr);
 
-    IP.printInst(MI, IS, "", STI);
+      IP.printInst(MI, IS, "", STI);
 
-    OS << left_justify(IS.str(), 60) << format("// %012" PRIX64 ": ", Address);
-    typedef support::ulittle32_t U32;
-    for (auto D : makeArrayRef(reinterpret_cast<const U32*>(Bytes.data()),
-                               Bytes.size() / sizeof(U32)))
-      // D should be explicitly casted to uint32_t here as it is passed
-      // by format to snprintf as vararg.
-      OS << format("%08" PRIX32 " ", static_cast<uint32_t>(D));
+      OS << left_justify(IS.str(), 60);
+    } else {
+      // an unrecognized encoding - this is probably data so represent it
+      // using the .long directive, or .byte directive if fewer than 4 bytes
+      // remaining
+      if (Bytes.size() >= 4) {
+        OS << format("\t.long 0x%08" PRIx32 " ",
+                     static_cast<uint32_t>(*reinterpret_cast<const U32*>(Bytes.data())));
+        OS.indent(42);
+      } else {
+          OS << format("\t.byte 0x%02" PRIx8, Bytes[0]);
+          for (unsigned int i = 1; i < Bytes.size(); i++)
+            OS << format(", 0x%02" PRIx8, Bytes[i]);
+          OS.indent(55 - (6 * Bytes.size()));
+      }
+    }
+
+    OS << format("// %012" PRIX64 ": ", Address);
+    if (Bytes.size() >=4) {
+      for (auto D : makeArrayRef(reinterpret_cast<const U32*>(Bytes.data()),
+                                 Bytes.size() / sizeof(U32)))
+        // D should be explicitly casted to uint32_t here as it is passed
+        // by format to snprintf as vararg.
+        OS << format("%08" PRIX32 " ", static_cast<uint32_t>(D));
+    } else {
+      for (unsigned int i = 0; i < Bytes.size(); i++)
+        OS << format("%02" PRIX8 " ", Bytes[i]);
+    }
 
     if (!Annot.empty())
       OS << "// " << Annot;
@@ -1459,8 +1478,6 @@ static void DisassembleObject(const ObjectFile *Obj, bool InlineRelocs) {
         End = StopAddress - SectionAddr;
 
       if (Obj->isELF() && Obj->getArch() == Triple::amdgcn) {
-        // make size 4 bytes folded
-        End = Start + ((End - Start) & ~0x3ull);
         if (std::get<2>(Symbols[si]) == ELF::STT_AMDGPU_HSA_KERNEL) {
           // skip amd_kernel_code_t at the begining of kernel symbol (256 bytes)
           Start += 256;