[llvm] [AMDGPU] Relax SOPK immediate signed/unsigned check (PR #77015)

Stanislav Mekhanoshin via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 4 14:20:43 PST 2024


https://github.com/rampitec created https://github.com/llvm/llvm-project/pull/77015

Machine verifier complains about an invalid SOPK immediate based on it signedness. It does not seem we have a good sanity in the SOPK definitions. For instance there is no reason for S_SETREG_B32 immedaite to be defined as SIMM16. A useful value 63489 cannot be used and we force users to use -2047 instead which makes a little sense, not obvious, and on top of that only enforced in the debug compiler build. At the same time SP3 itself does not have this restriction and accepts both signed and unsigned value, so we also have SP3 compatibility issue.

Ralax the verifier and allow both signed and unsigned constants.

>From fc62c75d4ed2a564c3963700e87235169ec605d9 Mon Sep 17 00:00:00 2001
From: Stanislav Mekhanoshin <Stanislav.Mekhanoshin at amd.com>
Date: Thu, 4 Jan 2024 14:08:53 -0800
Subject: [PATCH] [AMDGPU] Relax SOPK immediate signed/unsigned check

Machine verifier complains about an invalid SOPK immediate based
on it signedness. It does not seem we have a good sanity in the
SOPK definitions. For instance there is no reason for S_SETREG_B32
immedaite to be defined as SIMM16. A useful value 63489 cannot be
used and we force users to use -2047 instead which makes a little
sense, not obvious, and on top of that only enforced in the debug
compiler build. At the same time SP3 itself does not have this
restriction and accepts both signed and unsigned value, so we also
have SP3 compatibility issue.

Ralax the verifier and allow both signed and unsigned constants.
---
 llvm/lib/Target/AMDGPU/SIInstrInfo.cpp        | 13 ++------
 .../CodeGen/AMDGPU/llvm.amdgcn.s.setreg.ll    | 33 +++++++++++++++++++
 2 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index 67992929ab3567..0dc2806fa30264 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -4891,16 +4891,9 @@ bool SIInstrInfo::verifyInstruction(const MachineInstr &MI,
       }
     } else {
       uint64_t Imm = Op->getImm();
-      if (sopkIsZext(MI)) {
-        if (!isUInt<16>(Imm)) {
-          ErrInfo = "invalid immediate for SOPK instruction";
-          return false;
-        }
-      } else {
-        if (!isInt<16>(Imm)) {
-          ErrInfo = "invalid immediate for SOPK instruction";
-          return false;
-        }
+      if (!isUInt<16>(Imm) && !isInt<16>(Imm)) {
+        ErrInfo = "invalid immediate for SOPK instruction";
+        return false;
       }
     }
   }
diff --git a/llvm/test/CodeGen/AMDGPU/llvm.amdgcn.s.setreg.ll b/llvm/test/CodeGen/AMDGPU/llvm.amdgcn.s.setreg.ll
index d2c14f2401fc35..b2e987ae37fd93 100644
--- a/llvm/test/CodeGen/AMDGPU/llvm.amdgcn.s.setreg.ll
+++ b/llvm/test/CodeGen/AMDGPU/llvm.amdgcn.s.setreg.ll
@@ -1433,6 +1433,39 @@ define amdgpu_kernel void @test_setreg_set_4_bits_straddles_round_and_denorm() {
   ret void
 }
 
+define amdgpu_ps void @test_63489(i32 inreg %var.mode) {
+; GFX6-LABEL: test_63489:
+; GFX6:       ; %bb.0:
+; GFX6-NEXT:    s_setreg_b32 hwreg(HW_REG_MODE), s0 ; encoding: [0x01,0xf8,0x80,0xb9]
+; GFX6-NEXT:    ;;#ASMSTART
+; GFX6-NEXT:    ;;#ASMEND
+; GFX6-NEXT:    s_endpgm ; encoding: [0x00,0x00,0x81,0xbf]
+;
+; GFX789-LABEL: test_63489:
+; GFX789:       ; %bb.0:
+; GFX789-NEXT:    s_setreg_b32 hwreg(HW_REG_MODE), s0 ; encoding: [0x01,0xf8,0x00,0xb9]
+; GFX789-NEXT:    ;;#ASMSTART
+; GFX789-NEXT:    ;;#ASMEND
+; GFX789-NEXT:    s_endpgm ; encoding: [0x00,0x00,0x81,0xbf]
+;
+; GFX10-LABEL: test_63489:
+; GFX10:       ; %bb.0:
+; GFX10-NEXT:    s_setreg_b32 hwreg(HW_REG_MODE), s0 ; encoding: [0x01,0xf8,0x80,0xb9]
+; GFX10-NEXT:    ;;#ASMSTART
+; GFX10-NEXT:    ;;#ASMEND
+; GFX10-NEXT:    s_endpgm ; encoding: [0x00,0x00,0x81,0xbf]
+;
+; GFX11-LABEL: test_63489:
+; GFX11:       ; %bb.0:
+; GFX11-NEXT:    s_setreg_b32 hwreg(HW_REG_MODE), s0 ; encoding: [0x01,0xf8,0x00,0xb9]
+; GFX11-NEXT:    ;;#ASMSTART
+; GFX11-NEXT:    ;;#ASMEND
+; GFX11-NEXT:    s_endpgm ; encoding: [0x00,0x00,0xb0,0xbf]
+  call void @llvm.amdgcn.s.setreg(i32 63489, i32 %var.mode)
+  call void asm sideeffect "", ""()
+  ret void
+}
+
 ; FIXME: Broken for DAG
 ; define void @test_setreg_roundingmode_var_vgpr(i32 %var.mode) {
 ;   call void @llvm.amdgcn.s.setreg(i32 4097, i32 %var.mode)



More information about the llvm-commits mailing list