[PATCH] D147424: [MC][AMDGPU] Warn when disassembling a v_cmpx with non-exec destination

Anshil Gandhi via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Apr 2 22:28:41 PDT 2023


gandhi21299 updated this revision to Diff 510402.
gandhi21299 added a comment.

removed debug prints


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147424/new/

https://reviews.llvm.org/D147424

Files:
  llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
  llvm/lib/Target/AMDGPU/MCTargetDesc/SIMCCodeEmitter.cpp
  llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
  llvm/test/MC/Disassembler/AMDGPU/gfx10-vop3cx-errs.txt
  llvm/test/MC/Disassembler/AMDGPU/gfx10_vop3cx.txt


Index: llvm/test/MC/Disassembler/AMDGPU/gfx10_vop3cx.txt
===================================================================
--- llvm/test/MC/Disassembler/AMDGPU/gfx10_vop3cx.txt
+++ llvm/test/MC/Disassembler/AMDGPU/gfx10_vop3cx.txt
@@ -1,5 +1,5 @@
-# RUN: llvm-mc -arch=amdgcn -mcpu=gfx1010 -mattr=+wavefrontsize32,-wavefrontsize64 -disassemble -show-encoding < %s | FileCheck -strict-whitespace -check-prefix=GFX10 %s
-# RUN: llvm-mc -arch=amdgcn -mcpu=gfx1010 -mattr=-wavefrontsize32,+wavefrontsize64 -disassemble -show-encoding < %s | FileCheck -strict-whitespace -check-prefix=GFX10 %s
+# RUN: llvm-mc -arch=amdgcn -mcpu=gfx1010 -mattr=+wavefrontsize32,-wavefrontsize64 -disassemble -show-encoding < %s | FileCheck -strict-whitespace -check-prefix=GFX10 --implicit-check-not="Warning:" %s
+# RUN: llvm-mc -arch=amdgcn -mcpu=gfx1010 -mattr=-wavefrontsize32,+wavefrontsize64 -disassemble -show-encoding < %s | FileCheck -strict-whitespace -check-prefix=GFX10 --implicit-check-not="Warning:" %s
 
 
 # GFX10: v_cmpx_class_f16_e64 -1, v2             ; encoding: [0x7e,0x00,0x9f,0xd4,0xc1,0x04,0x02,0x00]
Index: llvm/test/MC/Disassembler/AMDGPU/gfx10-vop3cx-errs.txt
===================================================================
--- /dev/null
+++ llvm/test/MC/Disassembler/AMDGPU/gfx10-vop3cx-errs.txt
@@ -0,0 +1,11 @@
+# RUN: llvm-mc -arch=amdgcn -mcpu=gfx1010 -mattr=+wavefrontsize32,-wavefrontsize64 -disassemble < %s 2>&1 | FileCheck -check-prefix=GFX10 %s
+# RUN: llvm-mc -arch=amdgcn -mcpu=gfx1010 -mattr=-wavefrontsize32,+wavefrontsize64 -disassemble < %s 2>&1 | FileCheck -check-prefix=GFX10 %s
+
+# FIXME: Warning should be aligned with the instruction
+# GFX10: Warning: Non-exec destination operand
+
+# GFX10: v_cmpx_f_f64_e64 exec, v[1:2]
+0x7e,0x00,0x30,0xd4,0x7e,0x02,0x02,0x00
+
+# GFX10: v_cmpx_f_f64_e64 s[0:1], v[1:2]
+0x7e,0x00,0x30,0xd4,0x00,0x02,0x02,0x00
Index: llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
===================================================================
--- llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
+++ llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
@@ -9,6 +9,7 @@
 #ifndef LLVM_LIB_TARGET_AMDGPU_UTILS_AMDGPUBASEINFO_H
 #define LLVM_LIB_TARGET_AMDGPU_UTILS_AMDGPUBASEINFO_H
 
+#include "MCTargetDesc/AMDGPUMCTargetDesc.h"
 #include "SIDefines.h"
 #include "llvm/IR/CallingConv.h"
 #include "llvm/IR/InstrTypes.h"
@@ -1311,6 +1312,11 @@
 /// \returns true if the intrinsic is uniform
 bool isIntrinsicAlwaysUniform(unsigned IntrID);
 
+inline bool isVCMPX64(const MCInstrDesc &Desc) {
+  return (Desc.TSFlags & SIInstrFlags::VOP3) &&
+         Desc.hasImplicitDefOfPhysReg(llvm::AMDGPU::EXEC);
+}
+
 } // end namespace AMDGPU
 
 raw_ostream &operator<<(raw_ostream &OS,
Index: llvm/lib/Target/AMDGPU/MCTargetDesc/SIMCCodeEmitter.cpp
===================================================================
--- llvm/lib/Target/AMDGPU/MCTargetDesc/SIMCCodeEmitter.cpp
+++ llvm/lib/Target/AMDGPU/MCTargetDesc/SIMCCodeEmitter.cpp
@@ -311,11 +311,6 @@
   return OP_SEL_HI_0 | OP_SEL_HI_1 | OP_SEL_HI_2;
 }
 
-static bool isVCMPX64(const MCInstrDesc &Desc) {
-  return (Desc.TSFlags & SIInstrFlags::VOP3) &&
-         Desc.hasImplicitDefOfPhysReg(AMDGPU::EXEC);
-}
-
 void SIMCCodeEmitter::encodeInstruction(const MCInst &MI, raw_ostream &OS,
                                         SmallVectorImpl<MCFixup> &Fixups,
                                         const MCSubtargetInfo &STI) const {
@@ -339,7 +334,7 @@
   // is ignored by HW. It was decided to define dst as "do not care"
   // in td files to allow disassembler accept any dst value.
   // However, dst is encoded as EXEC for compatibility with SP3.
-  if (AMDGPU::isGFX10Plus(STI) && isVCMPX64(Desc)) {
+  if (AMDGPU::isGFX10Plus(STI) && AMDGPU::isVCMPX64(Desc)) {
     assert((Encoding & 0xFF) == 0);
     Encoding |= MRI.getEncodingValue(AMDGPU::EXEC_LO);
   }
Index: llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
===================================================================
--- llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
+++ llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
@@ -687,6 +687,18 @@
   // (unless there are fewer bytes left)
   Size = Res ? (MaxInstBytesNum - Bytes.size())
              : std::min((size_t)4, Bytes_.size());
+
+  const MCInstrDesc &Desc = MCII->get(MI.getOpcode());
+  auto OpIsZero = [](MCOperand &Op) {
+    return (Op.isImm() && Op.getImm() == 0);
+  };
+  auto OpIsExec = [](MCOperand &Op) {
+    return (Op.isReg() && Op.getReg() == AMDGPU::EXEC);
+  };
+  if (isGFX10Plus() && AMDGPU::isVCMPX64(Desc) && MI.getNumOperands() > 3 &&
+      !OpIsExec(MI.getOperand(1)) && !OpIsZero(MI.getOperand(3))) {
+    errs() << "Warning: Non-exec destination operand\n";
+  }
   return Res;
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D147424.510402.patch
Type: text/x-patch
Size: 4777 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230403/cc5700c4/attachment.bin>


More information about the llvm-commits mailing list