Skip to content

[InstCombine] Add optimization to combine adds through zext nneg, and add testcase #157723

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

[InstCombine] Add optimization to combine adds through zext nneg, and add testcase #157723

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0e8a5fd
Combine adds through zext nneg, and add testcase
Aethezz Sep 9, 2025
edde233
added preconditions and adjusted testcases
Aethezz Sep 16, 2025
9361257
update review and added multiuse
Aethezz Sep 24, 2025
0f37733
fix extra case and multiuse
Aethezz Sep 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1910,6 +1910,32 @@ Instruction *InstCombinerImpl::visitAdd(BinaryOperator &I) {
if (Instruction *Res = foldBinOpOfSelectAndCastOfSelectCondition(I))
return Res;

{
Value *X;
const APInt *C1, *C2;
if (match(&I,
m_Add(m_NNegZExt(m_Add(m_Value(X), m_APInt(C1))), m_APInt(C2)))) {
// Check if inner constant C1 is negative C1 < 0 and outer constant C2 >=
// 0
if (!C1->isNegative() || C2->isNegative())
return nullptr;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you use early exits like this, the transform should be moved into a separate function. Otherwise there is a high risk of accidentally skipping transforms that get added after this point.


APInt Sum = C1->sext(C2->getBitWidth()) + *C2;
APInt newSum = Sum.trunc(C1->getBitWidth());

if (!Sum.isSignedIntN(C1->getBitWidth()))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per the alive proof in https://alive2.llvm.org/ce/z/pY63fh, you need to perform this check on C2, not on Sum. And if you do, then you don't need the additional check below.

return nullptr;

// X if sum is zero, else X + newSum
Value *Inner =
Sum.isZero()
? X
: Builder.CreateAdd(X, ConstantInt::get(X->getType(), newSum));

return new ZExtInst(Inner, I.getType());
}
}

// Re-enqueue users of the induction variable of add recurrence if we infer
// new nuw/nsw flags.
if (Changed) {
Expand Down
82 changes: 82 additions & 0 deletions llvm/test/Transforms/InstCombine/zext.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -976,3 +976,85 @@ entry:
%res = zext nneg i2 %x to i32
ret i32 %res
}

define i32 @zext_nneg_add_cancel(i8 %arg) {
; CHECK-LABEL: @zext_nneg_add_cancel(
; CHECK-NEXT: [[ADD2:%.*]] = zext i8 [[ARG:%.*]] to i32
; CHECK-NEXT: ret i32 [[ADD2]]
;
%add = add i8 %arg, -2
%zext = zext nneg i8 %add to i32
%add2 = add i32 %zext, 2
ret i32 %add2
}

define i32 @zext_nneg_add_cancel_multi_use(i8 %arg) {
; CHECK-LABEL: @zext_nneg_add_cancel_multi_use(
; CHECK-NEXT: [[ADD:%.*]] = add i8 [[ARG:%.*]], -2
; CHECK-NEXT: [[ZEXT:%.*]] = zext nneg i8 [[ADD]] to i32
; CHECK-NEXT: call void @use32(i32 [[ZEXT]])
; CHECK-NEXT: [[ADD2:%.*]] = zext i8 [[ARG]] to i32
; CHECK-NEXT: ret i32 [[ADD2]]
;
%add = add i8 %arg, -2
%zext = zext nneg i8 %add to i32
call void @use32(i32 %zext)
%add2 = add i32 %zext, 2
ret i32 %add2
}

define i32 @zext_nneg_no_cancel(i8 %arg) {
; CHECK-LABEL: @zext_nneg_no_cancel(
; CHECK-NEXT: [[TMP1:%.*]] = add i8 [[ARG:%.*]], -1
; CHECK-NEXT: [[ADD2:%.*]] = zext i8 [[TMP1]] to i32
; CHECK-NEXT: ret i32 [[ADD2]]
;
%add = add i8 %arg, -2
%zext = zext nneg i8 %add to i32
%add2 = add i32 %zext, 1
ret i32 %add2
}

define i32 @zext_nneg_no_cancel_multi_use(i8 %arg) {
; CHECK-LABEL: @zext_nneg_no_cancel_multi_use(
; CHECK-NEXT: [[ADD:%.*]] = add i8 [[ARG:%.*]], -2
; CHECK-NEXT: [[ZEXT:%.*]] = zext nneg i8 [[ADD]] to i32
; CHECK-NEXT: call void @use32(i32 [[ZEXT]])
; CHECK-NEXT: [[TMP1:%.*]] = add i8 [[ARG]], -1
; CHECK-NEXT: [[ADD2:%.*]] = zext i8 [[TMP1]] to i32
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This increases the number of instructions. The transform should not be performed here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also we don't need the constants to exactly cancel, something like this is also profitable: https://alive2.llvm.org/ce/z/pT89U9 (Though this one would need a one-use check.)

In this case, the multiuse shows that number of instructions increases, so are we supporting the constants not exactly cancelling?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should do this fold, but guarded by a hasOneUse check.

; CHECK-NEXT: ret i32 [[ADD2]]
;
%add = add i8 %arg, -2
%zext = zext nneg i8 %add to i32
call void @use32(i32 %zext)
%add2 = add i32 %zext, 1
ret i32 %add2
}

define i32 @zext_nneg_overflow(i8 %arg) {
; CHECK-LABEL: @zext_nneg_overflow(
; CHECK-NEXT: [[ADD:%.*]] = add i8 [[ARG:%.*]], -2
; CHECK-NEXT: [[ZEXT:%.*]] = zext nneg i8 [[ADD]] to i32
; CHECK-NEXT: [[ADD2:%.*]] = add nuw nsw i32 [[ZEXT]], 299
; CHECK-NEXT: ret i32 [[ADD2]]
;
%add = add i8 %arg, -2
%zext = zext nneg i8 %add to i32
%add2 = add i32 %zext, 299
ret i32 %add2
}

define i32 @zext_nneg_overflow_multi_use(i8 %arg) {
; CHECK-LABEL: @zext_nneg_overflow_multi_use(
; CHECK-NEXT: [[ADD:%.*]] = add i8 [[ARG:%.*]], -2
; CHECK-NEXT: [[ZEXT:%.*]] = zext nneg i8 [[ADD]] to i32
; CHECK-NEXT: call void @use32(i32 [[ZEXT]])
; CHECK-NEXT: [[ADD2:%.*]] = add nuw nsw i32 [[ZEXT]], 299
; CHECK-NEXT: ret i32 [[ADD2]]
;
%add = add i8 %arg, -2
%zext = zext nneg i8 %add to i32
call void @use32(i32 %zext)
%add2 = add i32 %zext, 299
ret i32 %add2
}