[DAG] ComputeNumSignBits - ISD::EXTRACT_ELEMENT needs to return at least 1

[Saldivarcher]

When going through the ISD::EXTRACT_ELEMENT case, KnownSign - rIndex * BitWidth
could produce a negative. When a negative is produced, the lower bound
of the std::clamp is returned. Change that lower bound to one to avoid
potential underflows, because the expectation is that ComputeNumSignBits
should always return at least 1.

Fixes #155452.

[llvmbot]

@llvm/pr-subscribers-llvm-selectiondag

@llvm/pr-subscribers-backend-amdgpu

Author: Miguel Saldivar (Saldivarcher)

Changes

If the value for KnownSign - rIndex * BitWidth produces a negative, we need ComputeNumSignBits to return a 1. Changing the std::clamp lower bound value should fix that.

This is a fix for #155452.

---

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

2 Files Affected:

• (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (+1-1)
• (added) llvm/test/CodeGen/AMDGPU/pr155452.ll (+22)

diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index 3672a91e33a30..078825f2a9a22 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -5127,7 +5127,7 @@ unsigned SelectionDAG::ComputeNumSignBits(SDValue Op, const APInt &DemandedElts,
 
     // If the sign portion ends in our element the subtraction gives correct
     // result. Otherwise it gives either negative or > bitwidth result
-    return std::clamp(KnownSign - rIndex * BitWidth, 0, BitWidth);
+    return std::clamp(KnownSign - rIndex * BitWidth, 1, BitWidth);
   }
   case ISD::INSERT_VECTOR_ELT: {
     if (VT.isScalableVector())
diff --git a/llvm/test/CodeGen/AMDGPU/pr155452.ll b/llvm/test/CodeGen/AMDGPU/pr155452.ll
new file mode 100644
index 0000000000000..f4be1b4922162
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/pr155452.ll
@@ -0,0 +1,22 @@
+; RUN: llc %s -march=amdgcn  -o - | FileCheck %s
+
+target triple = "amdgcn-amd-amdhsa"
+
+define amdgpu_kernel void @my_kernel(i64 %foo, i32 %bar) {
+entry:
+  br label %loop
+
+loop: ; preds = %entry, %loop
+  %i = phi i64 [ 1, %entry ], [ 0, %loop ]
+  %mul = mul i64 %foo, %i
+  %add = add i64 %mul, 1
+  %trunc = trunc i64 %add to i32
+  %div = sdiv i32 %trunc, %bar
+  %sext = sext i32 %div to i64
+  %or = or i64 %add, %sext
+  %inttoptr = inttoptr i64 %or to ptr
+  %addrspacecast = addrspacecast ptr %inttoptr to ptr addrspace(1)
+  %val = load double, ptr addrspace(1) %addrspacecast, align 8
+  store double %val, ptr addrspace(1) null, align 8
+  br label %loop
+}

[topperc]

Description and/or title should mention the affected opcode.

[Saldivarcher]

Description and/or title should mention the affected opcode.

Thanks for the tip, I didn't know that! Let me know how that looks now.

[topperc]

LGTM

[Saldivarcher]

@topperc do you think you can merge this for me, please? I don't have commit access. And thanks again for the review!