[llvm] [AMDGPU] Assert no bad shift operations will happen (PR #108416)

Georgi Mirazchiyski via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 12 09:49:53 PDT 2024


https://github.com/GeorgeWeb updated https://github.com/llvm/llvm-project/pull/108416

>From ecd2cd428faa92cb43a86f09d11789cafd546c2a Mon Sep 17 00:00:00 2001
From: Georgi Mirazchiyski <georgi.mirazchiyski at codeplay.com>
Date: Thu, 12 Sep 2024 17:21:46 +0100
Subject: [PATCH] [AMDGPU] Assert no bad shift operations will happen

The assumption in the asserts is based on the fact that no SGPR/VGPR register
Arg mask in the ISelLowering and Legalizer can equal zero. They are implicitly
set to ~0 by default (meaning non-masked) or explicitly to a non-zero value.
The optimizeCompareInstr case is different from the above described. It
requires the mask to be a power-of-two because it's a special-case
optimization, hence in this case we still cannot have an invalid shift.
This commit also silences static analysis tools wrt potential bad shifts.
---
 llvm/lib/Target/AMDGPU/AMDGPUArgumentUsageInfo.h | 2 ++
 llvm/lib/Target/AMDGPU/SIInstrInfo.cpp           | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/llvm/lib/Target/AMDGPU/AMDGPUArgumentUsageInfo.h b/llvm/lib/Target/AMDGPU/AMDGPUArgumentUsageInfo.h
index 42b33c50d9f8c4..1eb59350ba09cc 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUArgumentUsageInfo.h
+++ b/llvm/lib/Target/AMDGPU/AMDGPUArgumentUsageInfo.h
@@ -77,6 +77,8 @@ struct ArgDescriptor {
   }
 
   unsigned getMask() const {
+    // None of the target SGPRs or VGPRs are expected to have a 'zero' mask.
+    assert(Mask && "Invalid mask.");
     return Mask;
   }
 
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index c19c3c6017a7c8..967862b49a4052 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -9769,6 +9769,9 @@ bool SIInstrInfo::optimizeCompareInstr(MachineInstr &CmpInstr, Register SrcReg,
     else
       return false;
 
+    // A valid Mask is required to have a single bit set, hence a non-zero and
+    // power-of-two value. This verifies that we will not do 64-bit shift below.
+    assert(llvm::has_single_bit(Mask) && "Invalid mask.");
     unsigned BitNo = llvm::countr_zero((uint64_t)Mask);
     if (IsSigned && BitNo == SrcSize - 1)
       return false;



More information about the llvm-commits mailing list