[Hexagon] Explicitly truncate constant in UAddSubO

[androm3da]

After #117558 landed, this code would assert "Value is not an N-bit unsigned value" in getConstant(), from a test case in zig.

Co-authored-by: Craig Topper [email protected]
Fixes #127296

[llvmbot]

@llvm/pr-subscribers-backend-hexagon

Author: Brian Cain (androm3da)

Changes

After #117558 landed, this code would assert "Value is not an N-bit unsigned value" in getConstant(), from a test case in zig.

Co-authored-by: Craig Topper <[email protected]>
Fixes #127296

---

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

2 Files Affected:

• (modified) llvm/lib/Target/Hexagon/HexagonISelLowering.cpp (+1-1)
• (added) llvm/test/CodeGen/Hexagon/iss127296.ll (+22)

diff --git a/llvm/lib/Target/Hexagon/HexagonISelLowering.cpp b/llvm/lib/Target/Hexagon/HexagonISelLowering.cpp
index 1a7667fe42fbc..b31360b4096da 100644
--- a/llvm/lib/Target/Hexagon/HexagonISelLowering.cpp
+++ b/llvm/lib/Target/Hexagon/HexagonISelLowering.cpp
@@ -3273,7 +3273,7 @@ HexagonTargetLowering::LowerUAddSubO(SDValue Op, SelectionDAG &DAG) const {
     if (Opc == ISD::USUBO) {
       SDValue Op = DAG.getNode(ISD::SUB, dl, VTs.VTs[0], {X, Y});
       SDValue Ov = DAG.getSetCC(dl, MVT::i1, Op,
-                                DAG.getConstant(-1, dl, ty(Op)), ISD::SETEQ);
+                                DAG.getAllOnesConstant(dl, ty(Op)), ISD::SETEQ);
       return DAG.getMergeValues({Op, Ov}, dl);
     }
   }
diff --git a/llvm/test/CodeGen/Hexagon/iss127296.ll b/llvm/test/CodeGen/Hexagon/iss127296.ll
new file mode 100644
index 0000000000000..ad2c5e40a9f02
--- /dev/null
+++ b/llvm/test/CodeGen/Hexagon/iss127296.ll
@@ -0,0 +1,22 @@
+; RUN: llc -mtriple=hexagon -O0 < %s | FileCheck %s
+; REQUIRES: asserts
+
+target datalayout = "e-m:e-p:32:32:32-a:0-n16:32-i64:64:64-i32:32:32-i16:16:16-i1:8:8-f32:32:32-f64:64:64-v32:32:32-v64:64:64-v512:512:512-v1024:1024:1024-v2048:2048:2048"
+target triple = "hexagon-unknown-linux4.19.0-musl"
+
+; CHECK:  r0 = #-1
+
+define fastcc void @os.linux.tls.initStatic() {
+	%1 = call { i32, i1 } @llvm.usub.with.overflow.i32(i32 0, i32 1)
+	br label %2
+
+	2:                                                ; preds = %0
+	%3 = extractvalue { i32, i1 } %1, 0
+	ret void
+}
+
+; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
+declare { i32, i1 } @llvm.usub.with.overflow.i32(i32, i32) #0
+
+attributes #0 = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }
+

[nikic]

Why does this need asserts?

[androm3da]

The original failure (#127296) happens with assertions enabled. I removed it from my latest patch. But I just tried a build without asserts and it looks like the codegen is no different with this fix. So IIUC that means I want the asserts back in this test case, right?

[nikic]

You want the test to always run, even if it only reproduces the original issue in some configuration. You should only use REQUIRES: asserts if you are checking for debug output only available in assertion-enabled builds.

[androm3da]

Oh really? Ok, good to know, thanks.

[nikic]

LGTM

[topperc]

LGTM