Skip to content

[AMDGPU] Try decoding instructions longest first. NFCI. #82014

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 20, 2024

Conversation

jayfoad
Copy link
Contributor

@jayfoad jayfoad commented Feb 16, 2024

AMDGPUDisassembler::getInstruction tries decoding instructions using
different DecoderTables in a confusing order: first 96-bit instructions,
then some 64-bit, then 32-bit, then some more 64-bit.

This patch changes it to always try longer encodings first. The
motivation is to make getInstruction easier to understand, and to pave
the way for combining some 64-bit tables that do not need to be
separate.

AMDGPUDisassembler::getInstruction tries decoding instructions using
different DecoderTables in a confusing order: first 96-bit instructions,
then some 64-bit, then 32-bit, then some more 64-bit.

This patch changes it to always try longer encodings first. The
motivation is to make getInstruction easier to understand, and to pave
the way for combining some 64-bit tables that do not need to be
separate.
@llvmbot
Copy link
Member

llvmbot commented Feb 16, 2024

@llvm/pr-subscribers-backend-amdgpu

Author: Jay Foad (jayfoad)

Changes

AMDGPUDisassembler::getInstruction tries decoding instructions using
different DecoderTables in a confusing order: first 96-bit instructions,
then some 64-bit, then 32-bit, then some more 64-bit.

This patch changes it to always try longer encodings first. The
motivation is to make getInstruction easier to understand, and to pave
the way for combining some 64-bit tables that do not need to be
separate.


Full diff: https://github.com/llvm/llvm-project/pull/82014.diff

2 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp (+46-43)
  • (modified) llvm/lib/Target/AMDGPU/SOPInstructions.td (+3)
diff --git a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
index 98988f881f1b44..dcd036248c4300 100644
--- a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
+++ b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
@@ -585,6 +585,52 @@ DecodeStatus AMDGPUDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
         if (Res)
           break;
       }
+
+      Res = tryDecodeInst(DecoderTableGFX864, MI, QW, Address, CS);
+      if (Res)
+        break;
+
+      Res = tryDecodeInst(DecoderTableGFX964, MI, QW, Address, CS);
+      if (Res)
+        break;
+
+      if (STI.hasFeature(AMDGPU::FeatureGFX940Insts)) {
+        Res = tryDecodeInst(DecoderTableGFX94064, MI, QW, Address, CS);
+        if (Res)
+          break;
+      }
+
+      if (STI.hasFeature(AMDGPU::FeatureGFX90AInsts)) {
+        Res = tryDecodeInst(DecoderTableGFX90A64, MI, QW, Address, CS);
+        if (Res)
+          break;
+      }
+
+      Res = tryDecodeInst(DecoderTableAMDGPU64, MI, QW, Address, CS);
+      if (Res)
+        break;
+
+      Res = tryDecodeInst(DecoderTableGFX1064, MI, QW, Address, CS);
+      if (Res)
+        break;
+
+      Res = tryDecodeInst(DecoderTableGFX1264, DecoderTableGFX12_FAKE1664, MI,
+                          QW, Address, CS);
+      if (Res)
+        break;
+
+      Res = tryDecodeInst(DecoderTableGFX1164, DecoderTableGFX11_FAKE1664, MI,
+                          QW, Address, CS);
+      if (Res)
+        break;
+
+      Res = tryDecodeInst(DecoderTableGFX11W6464, MI, QW, Address, CS);
+      if (Res)
+        break;
+
+      Res = tryDecodeInst(DecoderTableGFX12W6464, MI, QW, Address, CS);
+      if (Res)
+        break;
     }
 
     // Reinitialize Bytes as DPP64 could have eaten too much
@@ -624,49 +670,6 @@ DecodeStatus AMDGPUDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
                         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, CS);
-      if (Res)
-        break;
-    }
-
-    if (STI.hasFeature(AMDGPU::FeatureGFX90AInsts)) {
-      Res = tryDecodeInst(DecoderTableGFX90A64, MI, QW, Address, CS);
-      if (Res)
-        break;
-    }
-
-    Res = tryDecodeInst(DecoderTableGFX864, MI, QW, Address, CS);
-    if (Res) break;
-
-    Res = tryDecodeInst(DecoderTableAMDGPU64, MI, QW, Address, CS);
-    if (Res) break;
-
-    Res = tryDecodeInst(DecoderTableGFX964, MI, QW, Address, CS);
-    if (Res) break;
-
-    Res = tryDecodeInst(DecoderTableGFX1064, MI, QW, Address, CS);
-    if (Res) break;
-
-    Res = tryDecodeInst(DecoderTableGFX1264, DecoderTableGFX12_FAKE1664, MI, QW,
-                        Address, CS);
-    if (Res)
-      break;
-
-    Res = tryDecodeInst(DecoderTableGFX1164, DecoderTableGFX11_FAKE1664, MI, QW,
-                        Address, CS);
-    if (Res)
-      break;
-
-    Res = tryDecodeInst(DecoderTableGFX11W6464, MI, QW, Address, CS);
-    if (Res)
-      break;
-
-    Res = tryDecodeInst(DecoderTableGFX12W6464, MI, QW, Address, CS);
   } while (false);
 
   if (Res && AMDGPU::isMAC(MI.getOpcode())) {
diff --git a/llvm/lib/Target/AMDGPU/SOPInstructions.td b/llvm/lib/Target/AMDGPU/SOPInstructions.td
index 1486df04ed950b..0fe2845f8edc31 100644
--- a/llvm/lib/Target/AMDGPU/SOPInstructions.td
+++ b/llvm/lib/Target/AMDGPU/SOPInstructions.td
@@ -2571,11 +2571,13 @@ multiclass SOPP_Real_32_gfx11_Renamed_gfx12<bits<7> op, string gfx12_name> :
 
 multiclass SOPP_Real_With_Relaxation_gfx12<bits<7> op> {
   defm "" : SOPP_Real_32_gfx12<op>;
+  let isCodeGenOnly = 1 in
   defm _pad_s_nop : SOPP_Real_64_gfx12<op>;
 }
 
 multiclass SOPP_Real_With_Relaxation_gfx11<bits<7> op> {
   defm "" : SOPP_Real_32_gfx11<op>;
+  let isCodeGenOnly = 1 in
   defm _pad_s_nop : SOPP_Real_64_gfx11<op>;
 }
 
@@ -2697,6 +2699,7 @@ multiclass SOPP_Real_64_gfx6_gfx7_gfx8_gfx9_gfx10<bits<7> op> :
 //relaxation for insts with no operands not implemented
 multiclass SOPP_Real_With_Relaxation_gfx6_gfx7_gfx8_gfx9_gfx10<bits<7> op> {
   defm "" : SOPP_Real_32_gfx6_gfx7_gfx8_gfx9_gfx10<op>;
+  let isCodeGenOnly = 1 in
   defm _pad_s_nop : SOPP_Real_64_gfx6_gfx7_gfx8_gfx9_gfx10<op>;
 }
 

@jayfoad jayfoad merged commit a4d4615 into llvm:main Feb 20, 2024
@jayfoad jayfoad deleted the decoder-longest-first branch February 20, 2024 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants