InstCombine: improve optimizations for ceiling division with no overflow #142869
+289
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
llvm:instcombine
|
@llvm/pr-subscribers-llvm-transforms Author: None (gaynor-anthropic) Changesfixes #142497 The contents of this pull request were substantially written using claude-code. I've reviewed to the best of my ability (it's been years since I did any compilers work). Full diff: https://github.com/llvm/llvm-project/pull/142869.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
index a9ac5ff9b9c89..16ebd7bceff63 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -1787,6 +1787,50 @@ Instruction *InstCombinerImpl::visitAdd(BinaryOperator &I) {
if (Instruction *Ashr = foldAddToAshr(I))
return Ashr;
+ // Ceiling division by power-of-2:
+ // (X >> log2(N)) + zext(X & (N-1) != 0) --> (X + (N-1)) >> log2(N)
+ // This is valid when adding (N-1) to X doesn't overflow.
+ {
+ Value *X = nullptr, *Cmp = nullptr;
+ const APInt *ShiftAmt = nullptr, *Mask = nullptr;
+ CmpPredicate Pred;
+
+ // Match: (X >> C) + zext((X & Mask) != 0)
+ // or: zext((X & Mask) != 0) + (X >> C)
+ Value *Op0 = I.getOperand(0);
+ Value *Op1 = I.getOperand(1);
+
+ // Try matching with shift on left, zext on right
+ bool Matched = false;
+ if (match(Op0, m_LShr(m_Value(X), m_APInt(ShiftAmt))) &&
+ match(Op1, m_ZExt(m_Value(Cmp)))) {
+ Matched = match(Cmp, m_ICmp(Pred, m_And(m_Specific(X), m_APInt(Mask)),
+ m_ZeroInt()));
+ } else if (match(Op1, m_LShr(m_Value(X), m_APInt(ShiftAmt))) &&
+ match(Op0, m_ZExt(m_Value(Cmp)))) {
+ Matched = match(Cmp, m_ICmp(Pred, m_And(m_Specific(X), m_APInt(Mask)),
+ m_ZeroInt()));
+ }
+
+ if (Matched &&
+ Pred == ICmpInst::ICMP_NE &&
+ ShiftAmt && ShiftAmt->uge(1) && ShiftAmt->ult(BitWidth) &&
+ Mask && *Mask == (APInt(BitWidth, 1) << *ShiftAmt) - 1) {
+
+ // Check if X + Mask doesn't overflow
+ Constant *MaskC = ConstantInt::get(X->getType(), *Mask);
+ bool WillNotOverflowUnsigned = willNotOverflowUnsignedAdd(X, MaskC, I);
+
+ if (WillNotOverflowUnsigned) {
+ // (X + Mask) >> ShiftAmt
+ bool WillNotOverflowSigned = willNotOverflowSignedAdd(X, MaskC, I);
+ Value *Add = Builder.CreateAdd(X, MaskC, "", WillNotOverflowUnsigned,
+ WillNotOverflowSigned);
+ return BinaryOperator::CreateLShr(Add, ConstantInt::get(X->getType(), *ShiftAmt));
+ }
+ }
+ }
+
// (~X) + (~Y) --> -2 - (X + Y)
{
// To ensure we can save instructions we need to ensure that we consume both
diff --git a/llvm/test/Transforms/InstCombine/add.ll b/llvm/test/Transforms/InstCombine/add.ll
index 495f99824652d..d364082eab317 100644
--- a/llvm/test/Transforms/InstCombine/add.ll
+++ b/llvm/test/Transforms/InstCombine/add.ll
@@ -4273,4 +4273,161 @@ define i32 @fold_zext_nneg_add_const_fail2(i8 %x) {
}
declare void @llvm.assume(i1)
+declare i32 @llvm.ctlz.i32(i32, i1)
+
+; Ceiling division by power-of-2: (x >> log2(N)) + ((x & (N-1)) != 0) -> (x + (N-1)) >> log2(N)
+; This is only valid when x + (N-1) doesn't overflow
+
+; Test with known range that prevents overflow
+define noundef range(i32 0, 100) i32 @ceil_div_by_8_known_range(i32 noundef range(i32 0, 100) %x) {
+; CHECK-LABEL: @ceil_div_by_8_known_range(
+; CHECK-NEXT: [[TMP1:%.*]] = add nuw nsw i32 [[X:%.*]], 7
+; CHECK-NEXT: [[R:%.*]] = lshr i32 [[TMP1]], 3
+; CHECK-NEXT: ret i32 [[R]]
+;
+ %shr = lshr i32 %x, 3
+ %and = and i32 %x, 7
+ %cmp = icmp ne i32 %and, 0
+ %ext = zext i1 %cmp to i32
+ %r = add i32 %shr, %ext
+ ret i32 %r
+}
+
+; Test with the exact IR from the original testcase
+define noundef range(i32 0, 6) i32 @ceil_div_from_clz(i32 noundef %v) {
+; CHECK-LABEL: @ceil_div_from_clz(
+; CHECK-NEXT: [[CTLZ:%.*]] = tail call range(i32 0, 33) i32 @llvm.ctlz.i32(i32 [[V:%.*]], i1 false)
+; CHECK-NEXT: [[TMP1:%.*]] = sub nuw nsw i32 39, [[CTLZ]]
+; CHECK-NEXT: [[R:%.*]] = lshr i32 [[TMP1]], 3
+; CHECK-NEXT: ret i32 [[R]]
+;
+ %ctlz = tail call range(i32 0, 33) i32 @llvm.ctlz.i32(i32 %v, i1 false)
+ %sub = sub nuw nsw i32 32, %ctlz
+ %shr = lshr i32 %sub, 3
+ %and = and i32 %sub, 7
+ %cmp = icmp ne i32 %and, 0
+ %ext = zext i1 %cmp to i32
+ %r = add nuw nsw i32 %shr, %ext
+ ret i32 %r
+}
+
+; Vector version with known range
+define <2 x i32> @ceil_div_by_8_vec_range(<2 x i32> range(i32 0, 1000) %x) {
+; CHECK-LABEL: @ceil_div_by_8_vec_range(
+; CHECK-NEXT: [[TMP1:%.*]] = add nuw nsw <2 x i32> [[X:%.*]], splat (i32 7)
+; CHECK-NEXT: [[R:%.*]] = lshr <2 x i32> [[TMP1]], splat (i32 3)
+; CHECK-NEXT: ret <2 x i32> [[R]]
+;
+ %shr = lshr <2 x i32> %x, <i32 3, i32 3>
+ %and = and <2 x i32> %x, <i32 7, i32 7>
+ %cmp = icmp ne <2 x i32> %and, <i32 0, i32 0>
+ %ext = zext <2 x i1> %cmp to <2 x i32>
+ %r = add <2 x i32> %shr, %ext
+ ret <2 x i32> %r
+}
+
+; Ceiling division by 16 with known range
+define i16 @ceil_div_by_16_i16(i16 range(i16 0, 1000) %x) {
+; CHECK-LABEL: @ceil_div_by_16_i16(
+; CHECK-NEXT: [[TMP1:%.*]] = add nuw nsw i16 [[X:%.*]], 15
+; CHECK-NEXT: [[R:%.*]] = lshr i16 [[TMP1]], 4
+; CHECK-NEXT: ret i16 [[R]]
+;
+ %shr = lshr i16 %x, 4
+ %and = and i16 %x, 15
+ %cmp = icmp ne i16 %and, 0
+ %ext = zext i1 %cmp to i16
+ %r = add i16 %shr, %ext
+ ret i16 %r
+}
+
+; Negative test: no overflow guarantee - should NOT optimize
+define i32 @ceil_div_by_8_no_overflow_info(i32 %x) {
+; CHECK-LABEL: @ceil_div_by_8_no_overflow_info(
+; CHECK-NEXT: [[SHR:%.*]] = lshr i32 [[X:%.*]], 3
+; CHECK-NEXT: [[AND:%.*]] = and i32 [[X]], 7
+; CHECK-NEXT: [[CMP:%.*]] = icmp ne i32 [[AND]], 0
+; CHECK-NEXT: [[EXT:%.*]] = zext i1 [[CMP]] to i32
+; CHECK-NEXT: [[R:%.*]] = add nuw nsw i32 [[SHR]], [[EXT]]
+; CHECK-NEXT: ret i32 [[R]]
+;
+ %shr = lshr i32 %x, 3
+ %and = and i32 %x, 7
+ %cmp = icmp ne i32 %and, 0
+ %ext = zext i1 %cmp to i32
+ %r = add i32 %shr, %ext
+ ret i32 %r
+}
+
+; Negative test: nuw on final add doesn't help
+define i32 @ceil_div_by_8_only_nuw_on_add(i32 %x) {
+; CHECK-LABEL: @ceil_div_by_8_only_nuw_on_add(
+; CHECK-NEXT: [[SHR:%.*]] = lshr i32 [[X:%.*]], 3
+; CHECK-NEXT: [[AND:%.*]] = and i32 [[X]], 7
+; CHECK-NEXT: [[CMP:%.*]] = icmp ne i32 [[AND]], 0
+; CHECK-NEXT: [[EXT:%.*]] = zext i1 [[CMP]] to i32
+; CHECK-NEXT: [[R:%.*]] = add nuw nsw i32 [[SHR]], [[EXT]]
+; CHECK-NEXT: ret i32 [[R]]
+;
+ %shr = lshr i32 %x, 3
+ %and = and i32 %x, 7
+ %cmp = icmp ne i32 %and, 0
+ %ext = zext i1 %cmp to i32
+ %r = add nuw i32 %shr, %ext ; nuw here doesn't prove x+7 won't overflow
+ ret i32 %r
+}
+
+; Negative test: wrong mask
+define i32 @ceil_div_wrong_mask(i32 range(i32 0, 100) %x) {
+; CHECK-LABEL: @ceil_div_wrong_mask(
+; CHECK-NEXT: [[SHR:%.*]] = lshr i32 [[X:%.*]], 3
+; CHECK-NEXT: [[AND:%.*]] = and i32 [[X]], 6
+; CHECK-NEXT: [[CMP:%.*]] = icmp ne i32 [[AND]], 0
+; CHECK-NEXT: [[EXT:%.*]] = zext i1 [[CMP]] to i32
+; CHECK-NEXT: [[R:%.*]] = add nuw nsw i32 [[SHR]], [[EXT]]
+; CHECK-NEXT: ret i32 [[R]]
+;
+ %shr = lshr i32 %x, 3
+ %and = and i32 %x, 6 ; Wrong mask: should be 7
+ %cmp = icmp ne i32 %and, 0
+ %ext = zext i1 %cmp to i32
+ %r = add i32 %shr, %ext
+ ret i32 %r
+}
+
+; Negative test: wrong shift amount
+define i32 @ceil_div_wrong_shift(i32 range(i32 0, 100) %x) {
+; CHECK-LABEL: @ceil_div_wrong_shift(
+; CHECK-NEXT: [[SHR:%.*]] = lshr i32 [[X:%.*]], 4
+; CHECK-NEXT: [[AND:%.*]] = and i32 [[X]], 7
+; CHECK-NEXT: [[CMP:%.*]] = icmp ne i32 [[AND]], 0
+; CHECK-NEXT: [[EXT:%.*]] = zext i1 [[CMP]] to i32
+; CHECK-NEXT: [[R:%.*]] = add nuw nsw i32 [[SHR]], [[EXT]]
+; CHECK-NEXT: ret i32 [[R]]
+;
+ %shr = lshr i32 %x, 4 ; Shift by 4, but mask is 7 (should be 15)
+ %and = and i32 %x, 7
+ %cmp = icmp ne i32 %and, 0
+ %ext = zext i1 %cmp to i32
+ %r = add i32 %shr, %ext
+ ret i32 %r
+}
+
+; Negative test: wrong comparison
+define i32 @ceil_div_wrong_cmp(i32 range(i32 0, 100) %x) {
+; CHECK-LABEL: @ceil_div_wrong_cmp(
+; CHECK-NEXT: [[SHR:%.*]] = lshr i32 [[X:%.*]], 3
+; CHECK-NEXT: [[AND:%.*]] = and i32 [[X]], 7
+; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[AND]], 0
+; CHECK-NEXT: [[EXT:%.*]] = zext i1 [[CMP]] to i32
+; CHECK-NEXT: [[R:%.*]] = add nuw nsw i32 [[SHR]], [[EXT]]
+; CHECK-NEXT: ret i32 [[R]]
+;
+ %shr = lshr i32 %x, 3
+ %and = and i32 %x, 7
+ %cmp = icmp eq i32 %and, 0 ; Wrong: should be ne
+ %ext = zext i1 %cmp to i32
+ %r = add i32 %shr, %ext
+ ret i32 %r
+}
declare void @fake_func(i32)
|
Hmm, unfortunately given the length of the logs, I'm not sure how to find the part that actually failed :-/ |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
dtcxzyw
reviewed
Jun 5, 2025
There was a problem hiding this comment.
Can you please provide generalized Alive2 proof?
See also https://llvm.org/docs/InstCombineContributorGuide.html#proofs.
|
Oops, sorry the alive2 proof was on the issue: https://alive2.llvm.org/ce/z/Ys4qAy Let me know if that's not sufficient. Will make the other improvements later this evening. |
I mean "generalized" proof. The proof should replace specific constants (excluding bitwidth) with parameters and preconditions. See https://llvm.org/docs/InstCombineContributorGuide.html#use-generic-values-in-proofs |
|
https://alive2.llvm.org/ce/z/HRxA3c generalized proof for all integer powers of 2. Hopefully that looks good. Will work on your code review notes next. Thank you! |
Updated: https://alive2.llvm.org/ce/z/CeaHaH |
dtcxzyw
reviewed
Jun 6, 2025
This was referenced Jun 6, 2025
|
IR diff looks good :) |
Co-authored-by: Yingwei Zheng <[email protected]>
|
thanks for your help on the generalized proof! |
|
(let me know if there's a preference to squash the commits or rebase them or anything) |
dtcxzyw
approved these changes
Jun 8, 2025
There was a problem hiding this comment.
LGTM. Please wait for additional approval from other reviewers :)
The contents of this pull request were substantially written using claude-code. I've reviewed to the best of my ability (it's been years since I did any compilers work).
I don't know the policy for AI-generated content in LLVM.
See also https://discourse.llvm.org/t/rfc-define-policy-on-ai-tool-usage-in-contributions/78758. IMO it is fine if contributors can ensure they have the right to contribute code under the terms of our license.
|
My reading of https://llvm.org/docs/DeveloperPolicy.html#ai-generated-contributions was that it was acceptable. Thanks for the review! |
|
Is there anything I could/should be doing to help get an additional reviewer, or should I just be more patient :-)? |
|
ping @nikic |
nikic
reviewed
Jun 16, 2025
Co-authored-by: Nikita Popov <[email protected]>
nikic
approved these changes
Jun 17, 2025
|
@gaynor-anthropic Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/51/builds/18071 Here is the relevant piece of the build log for the reference |
|
Thanks for the reviews! |
ajaden-codes
pushed a commit
to Jaddyen/llvm-project
that referenced
this pull request
Jun 17, 2025
…low (llvm#142869) Fixes llvm#142497. Alive2: https://alive2.llvm.org/ce/z/CeaHaH The contents of this pull request were substantially written using claude-code. I've reviewed to the best of my ability (it's been years since I did any compilers work). --------- Co-authored-by: Yingwei Zheng <[email protected]> Co-authored-by: Nikita Popov <[email protected]>
fschlimb
pushed a commit
to fschlimb/llvm-project
that referenced
this pull request
Jun 18, 2025
…low (llvm#142869) Fixes llvm#142497. Alive2: https://alive2.llvm.org/ce/z/CeaHaH The contents of this pull request were substantially written using claude-code. I've reviewed to the best of my ability (it's been years since I did any compilers work). --------- Co-authored-by: Yingwei Zheng <[email protected]> Co-authored-by: Nikita Popov <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #142497
Alive2: https://alive2.llvm.org/ce/z/CeaHaH
The contents of this pull request were substantially written using claude-code. I've reviewed to the best of my ability (it's been years since I did any compilers work).