[PATCH] D44685: [AMDGPU] Improve disassembler error handling

[Tim Corringham via Phabricator via llvm-commits]

timcorringham created this revision.
Herald added subscribers: llvm-commits, t-tye, tpr, dstuttard, yaxunl, nhaehnle, wdng, kzhuravl, arsenm.

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.


Repository:
  rL LLVM

https://reviews.llvm.org/D44685

Files:
  lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
  test/MC/AMDGPU/data.s
  tools/llvm-objdump/llvm-objdump.cpp


Index: tools/llvm-objdump/llvm-objdump.cpp
===================================================================
--- tools/llvm-objdump/llvm-objdump.cpp
+++ tools/llvm-objdump/llvm-objdump.cpp
@@ -595,18 +595,24 @@
     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;
+      OS << left_justify(IS.str(), 60);
+    } else {
+      // an unrecognized encoding - this is probably data so represent it
+      // using the .long directive
+      OS << format("\t.long 0x%08" PRIx32 " ",
+                   static_cast<uint32_t>(*reinterpret_cast<const U32*>(Bytes.data())));
+      OS.indent(42);
+    }
+
+    OS << format("// %012" PRIX64 ": ", Address);
     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
Index: test/MC/AMDGPU/data.s
===================================================================
--- /dev/null
+++ test/MC/AMDGPU/data.s
@@ -0,0 +1,22 @@
+// We check that unrecognized opcodes are disassembled by llvm-objdump as data using the .long 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
+
+// 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-NOT: .long
Index: lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
===================================================================
--- lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
+++ lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
@@ -246,7 +246,8 @@
   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
+  Size = Res ? (MaxInstBytesNum - Bytes.size()) : 4;
   return Res;
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D44685.139139.patch
Type: text/x-patch
Size: 2747 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180320/33caea1a/attachment.bin>