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

Georgi Mirazchiyski via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 24 03:46:49 PDT 2024


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

>From 478465dbf53c7172fb53dadfa861f3df6050709c 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 2e02bb4271adc7..06b2f181c276cd 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 c787edf7cfd11b..f5f367b2a4a7c6 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -9791,6 +9791,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