Skip to content

[AMDGPU] Verify SdwaSel value range #128898

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 27, 2025
Merged

Conversation

frederik-h
Copy link
Contributor

Ensure that the MachineVerifier verifies that the value provided for an SDWA selection is a valid value for the SdwaSel enum.

Ensure that the MachineVerifier verifies that the
value provided for an SDWA selection is a valid
value for the SdwaSel enum.
@llvmbot
Copy link
Member

llvmbot commented Feb 26, 2025

@llvm/pr-subscribers-backend-amdgpu

Author: Frederik Harwath (frederik-h)

Changes

Ensure that the MachineVerifier verifies that the value provided for an SDWA selection is a valid value for the SdwaSel enum.


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

2 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/SIInstrInfo.cpp (+12)
  • (added) llvm/test/CodeGen/AMDGPU/verifier-sdwa-selection.mir (+22)
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index 2ef628a7c569c..ceadc81aa56ac 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -4916,6 +4916,18 @@ bool SIInstrInfo::verifyInstruction(const MachineInstr &MI,
       return false;
     }
 
+    for (auto Op : {AMDGPU::OpName::src0_sel, AMDGPU::OpName::src1_sel,
+                    AMDGPU::OpName::dst_sel}) {
+      const MachineOperand *MO = getNamedOperand(MI, Op);
+      if (!MO)
+        continue;
+      int64_t Imm = MO->getImm();
+      if (Imm < 0 || Imm > AMDGPU::SDWA::SdwaSel::DWORD) {
+        ErrInfo = "Invalid SDWA selection";
+        return false;
+      }
+    }
+
     int DstIdx = AMDGPU::getNamedOperandIdx(Opcode, AMDGPU::OpName::vdst);
 
     for (int OpIdx : {DstIdx, Src0Idx, Src1Idx, Src2Idx}) {
diff --git a/llvm/test/CodeGen/AMDGPU/verifier-sdwa-selection.mir b/llvm/test/CodeGen/AMDGPU/verifier-sdwa-selection.mir
new file mode 100644
index 0000000000000..8517b0ad53458
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/verifier-sdwa-selection.mir
@@ -0,0 +1,22 @@
+# RUN: not --crash llc -mtriple=amdgcn -mcpu=gfx1030 --verify-machineinstrs -o - %s 2>&1 | FileCheck %s
+
+# CHECK-COUNT-6: *** Bad machine code: Invalid SDWA selection ***
+# CHECK-NOT: *** Bad machine code
+# CHECK: LLVM ERROR: Found 6 machine code errors
+
+---
+name: invalid_sdwa_selection
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    liveins: $vgpr0
+    %0:vgpr_32 = COPY $vgpr0
+    %1:vgpr_32 = V_LSHRREV_B32_sdwa 0, %0:vgpr_32, 0, %0:vgpr_32, 0, 1, 0, 7, 0, implicit $exec
+    %2:vgpr_32 = V_LSHRREV_B32_sdwa 0, %0:vgpr_32, 0, %0:vgpr_32, 0, 1, 0, -1, 0, implicit $exec
+    %3:vgpr_32 = V_LSHRREV_B32_sdwa 0, %0:vgpr_32, 0, %0:vgpr_32, 0, 7, 0, 6, 0, implicit $exec
+    %4:vgpr_32 = V_LSHRREV_B32_sdwa 0, %0:vgpr_32, 0, %0:vgpr_32, 0, -1, 0, 6, 0, implicit $exec
+    %5:vgpr_32 = V_LSHRREV_B32_sdwa 0, %0:vgpr_32, 0, %0:vgpr_32, 0, 0, 0, 0, 7, implicit $exec
+    %6:vgpr_32 = V_LSHRREV_B32_sdwa 0, %0:vgpr_32, 0, %0:vgpr_32, 0, 0, 0, 0, -1, implicit $exec
+
+    S_ENDPGM 0
+...

@frederik-h
Copy link
Contributor Author

@arsenm Thanks for the review!

@frederik-h frederik-h merged commit 50b508c into llvm:main Feb 27, 2025
11 checks passed
@frederik-h frederik-h deleted the verify-SdwaSel-range branch February 27, 2025 07:11
joaosaffran pushed a commit to joaosaffran/llvm-project that referenced this pull request Mar 3, 2025
Make the MachineVerifier check that the value provided for an SDWA selection is a
valid value for the SdwaSel enum.
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