[InstCombine] Fold uadd.sat(X, C) - C to umin(X, ~C) - #215130
Conversation
|
@llvm/pr-subscribers-llvm-transforms Author: Bagodiya (Bagodiya) Changes
The saturating add gives There was already a test documenting this miss in saturating-add-sub.ll Alive2: https://alive2.llvm.org/ce/z/3MdJCo Fixes #215103 Full diff: https://github.com/llvm/llvm-project/pull/215130.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
index 52a57e153bb4b..1d74547b5c6d0 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -1003,7 +1003,17 @@ Instruction *InstCombinerImpl::foldAddWithConstant(BinaryOperator &Add) {
return replaceInstUsesWith(
Add, Builder.CreateBinaryIntrinsic(
Intrinsic::usub_sat, X, ConstantInt::get(Add.getType(), -*C)));
-
+ // uadd.sat(X, C) + -C --> umin(X, ~C)
+ // The saturating add gives X + C or UMAX, so subtracting C leaves X or
+ // UMAX - C. Note UMAX - C == ~C.
+ {
+ APInt SatC = -*C;
+ if (!SatC.isZero() && match(Op0, m_OneUse(m_Intrinsic<Intrinsic::uadd_sat>(
+ m_Value(X), m_SpecificInt(SatC)))))
+ return replaceInstUsesWith(
+ Add, Builder.CreateBinaryIntrinsic(Intrinsic::umin, X,
+ ConstantInt::get(Ty, ~SatC)));
+ }
// Fold (add (zext (add X, -C)), C) -> (zext X) if X u>= C.
// Truncate C to the narrow type to avoid mismatched width comparisons.
{
diff --git a/llvm/test/Transforms/InstCombine/saturating-add-sub.ll b/llvm/test/Transforms/InstCombine/saturating-add-sub.ll
index f1cc997a41ba1..341459ed94c9d 100644
--- a/llvm/test/Transforms/InstCombine/saturating-add-sub.ll
+++ b/llvm/test/Transforms/InstCombine/saturating-add-sub.ll
@@ -1367,15 +1367,65 @@ define i8 @test_scalar_uadd_sub_commuted_wrong(i8 %a, i8 %b) {
define i8 @test_scalar_uadd_sub_const(i8 %a) {
; CHECK-LABEL: @test_scalar_uadd_sub_const(
+; CHECK-NEXT: [[RES:%.*]] = call i8 @llvm.umin.i8(i8 [[A:%.*]], i8 -43)
+; CHECK-NEXT: ret i8 [[RES]]
+;
+ %sat = call i8 @llvm.uadd.sat.i8(i8 %a, i8 42)
+ %res = sub i8 %sat, 42
+ ret i8 %res
+}
+
+define <2 x i8> @test_vector_uadd_sub_const(<2 x i8> %a) {
+; CHECK-LABEL: @test_vector_uadd_sub_const(
+; CHECK-NEXT: [[RES:%.*]] = call <2 x i8> @llvm.umin.v2i8(<2 x i8> [[A:%.*]], <2 x i8> splat (i8 -43))
+; CHECK-NEXT: ret <2 x i8> [[RES]]
+;
+ %sat = call <2 x i8> @llvm.uadd.sat.v2i8(<2 x i8> %a, <2 x i8> splat (i8 42))
+ %res = sub <2 x i8> %sat, splat (i8 42)
+ ret <2 x i8> %res
+}
+
+; negative test - the constants do not cancel
+
+define i8 @test_scalar_uadd_sub_const_mismatch(i8 %a) {
+; CHECK-LABEL: @test_scalar_uadd_sub_const_mismatch(
+; CHECK-NEXT: [[SAT:%.*]] = call i8 @llvm.uadd.sat.i8(i8 [[A:%.*]], i8 42)
+; CHECK-NEXT: [[RES:%.*]] = add i8 [[SAT]], -43
+; CHECK-NEXT: ret i8 [[RES]]
+;
+ %sat = call i8 @llvm.uadd.sat.i8(i8 %a, i8 42)
+ %res = sub i8 %sat, 43
+ ret i8 %res
+}
+
+; negative test - extra use of the saturating add
+
+define i8 @test_scalar_uadd_sub_const_multiuse(i8 %a) {
+; CHECK-LABEL: @test_scalar_uadd_sub_const_multiuse(
; CHECK-NEXT: [[SAT:%.*]] = call i8 @llvm.uadd.sat.i8(i8 [[A:%.*]], i8 42)
; CHECK-NEXT: [[RES:%.*]] = add i8 [[SAT]], -42
+; CHECK-NEXT: call void @usei8(i8 [[SAT]])
; CHECK-NEXT: ret i8 [[RES]]
;
%sat = call i8 @llvm.uadd.sat.i8(i8 %a, i8 42)
%res = sub i8 %sat, 42
+ call void @usei8(i8 %sat)
ret i8 %res
}
+; negative test - non-splat vector
+
+define <2 x i8> @test_vector_uadd_sub_const_nonsplat(<2 x i8> %a) {
+; CHECK-LABEL: @test_vector_uadd_sub_const_nonsplat(
+; CHECK-NEXT: [[SAT:%.*]] = call <2 x i8> @llvm.uadd.sat.v2i8(<2 x i8> [[A:%.*]], <2 x i8> <i8 42, i8 3>)
+; CHECK-NEXT: [[RES:%.*]] = add <2 x i8> [[SAT]], <i8 -42, i8 -3>
+; CHECK-NEXT: ret <2 x i8> [[RES]]
+;
+ %sat = call <2 x i8> @llvm.uadd.sat.v2i8(<2 x i8> %a, <2 x i8> <i8 42, i8 3>)
+ %res = sub <2 x i8> %sat, <i8 42, i8 3>
+ ret <2 x i8> %res
+}
+
define i1 @scalar_uadd_eq_zero(i8 %a, i8 %b) {
; CHECK-LABEL: @scalar_uadd_eq_zero(
; CHECK-NEXT: [[TMP1:%.*]] = or i8 [[A:%.*]], [[B:%.*]]
|
dtcxzyw
left a comment
There was a problem hiding this comment.
LGTM.
Generalized proof: https://alive2.llvm.org/ce/z/kLFWy7
| // UMAX - C. Note UMAX - C == ~C. | ||
| { | ||
| APInt SatC = -*C; | ||
| if (!SatC.isZero() && match(Op0, m_OneUse(m_Intrinsic<Intrinsic::uadd_sat>( |
There was a problem hiding this comment.
| if (!SatC.isZero() && match(Op0, m_OneUse(m_Intrinsic<Intrinsic::uadd_sat>( | |
| if (match(Op0, m_OneUse(m_Intrinsic<Intrinsic::uadd_sat>( |
Redundant check.
|
@Bagodiya Please follow https://llvm.org/docs/InstCombineContributorGuide.html#proofs to provide a generalized proof next time you create a new PR. |
bc47d06 to
bb6dc05
Compare
|
Thanks for the pointer, I'll provide generalized proofs going forward |
uadd.sat(X, C) - C --> umin(X, ~C)for nonzeroC.The saturating add gives
X + CorUMAX, so subtractingCleavesXorUMAX - C, which is the unsigned minimum.UMAX - C == ~C, so the constantis just the inverted
C.There was already a test documenting this miss in saturating-add-sub.ll
(
test_scalar_uadd_sub_const) - it folds now.https://alive2.llvm.org/ce/z/kLFWy7
Fixes #215103