Repository navigation
Conversation
ParkHanbum
reviewed
Sep 11, 2026
| } else if (!match(FalseVal, m_BinOp(m_Specific(TrueVal), m_Power2())) && | ||
| !match(TrueVal, m_BinOp(m_Specific(FalseVal), m_Power2()))) { | ||
| return nullptr; | ||
| } |
There was a problem hiding this comment.
Style-wise, I think this looks better, but the current version also seems perfectly fine to submit as-is.
const APInt *TC, *FC;
bool ConstArmsCanFold =
match(TrueVal, m_APInt(TC)) && match(FalseVal, m_APInt(FC)) &&
((*TC ^ *FC).isPowerOf2() || (*TC - *FC).isPowerOf2() ||
(*FC - *TC).isPowerOf2());
bool BinOpArmsCanFold =
match(FalseVal, m_BinOp(m_Specific(TrueVal), m_Power2())) ||
match(TrueVal, m_BinOp(m_Specific(FalseVal), m_Power2()));
if (!ConstArmsCanFold && !BinOpArmsCanFold)
return nullptr;Precommit tests for llvm#206708. Assisted-by: Claude Code
Both foldSelectICmpAnd and foldSelectICmpAndBinOp only handle selects whose arms are constants, or differ by a binop with a power-of-two constant. With constant arms, foldSelectICmpAnd additionally needs them to differ by a single bit (or/xor) or by a power of two (add/sub). Check all of that up front, before decomposing the condition and before the known-bits query on the decomposeBitTestICmp path. The check is symmetric in the arms, so the later arm swap for ICMP_NE does not affect it, and it only skips selects the folds reject anyway. Assisted-by: Claude Code
foldSelectBitTest only recognizes an equality bit test when the compared value is literally `and X, Pow2`. Known bits from an assume, a range attribute or a dominating branch can establish the same single-bit fact without the and being spelled out: %c = icmp ult i32 %a, 2 call void @llvm.assume(i1 %c) %cmp = icmp eq i32 %a, 0 %res = select i1 %cmp, i32 1, i32 2 => %res = add nuw nsw i32 %a, 1 Fall back to computeKnownBits when the and pattern does not match, the same way the decomposeBitTestICmp path below already refines its mask. The query only runs for selects whose arms passed the cheap checks at the top of the function, so the common `x == 0 ? C1 : C2` shapes that could never fold do not reach it. The fallback is limited to selects that already have the compared value's type. Without an and to remove, a narrower or wider select would need a cast plus a shift, which is no cheaper than icmp+select and blocks the later folds of the select with its users (seen as regressions in llvm-opt-benchmark on getRegState and qemu's csr). Pointer compares are rejected explicitly: computeKnownBits handles them, but the folds zext/trunc the compared value. Proofs: - add, eq and ne forms, symbolic C: https://alive2.llvm.org/ce/z/8oJ_7N - add/or/xor/sub, symbolic C and P: https://alive2.llvm.org/ce/z/q4Zeg2 - concrete shapes and flag boundaries: https://alive2.llvm.org/ce/z/vwT7Ke - negatives (range(0,3); nsw on the INT_MAX arm): https://alive2.llvm.org/ce/z/m5zjKW Closes llvm#206708 Assisted-by: Claude Code
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
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.
Fork PR for mentor review before opening upstream. Fixes llvm#206708.
foldSelectBitTestonly recognizes an equality bit test when the comparedvalue is literally
and X, Pow2. Known bits from anllvm.assume, arangeattribute, or a dominating branch can establish the same single-bit fact
without the
andbeing spelled out:This falls back to
computeKnownBitswhen theandpattern does not match,the same way the
decomposeBitTestICmppath directly below already refinesits mask with known bits. The query is only reached for selects whose arms
already passed the cheap structural checks hoisted to the function entry in
the preceding NFC commit, which also removes the pre-existing known-bits
query on the
decomposeBitTestICmppath for selects that can never fold.Proofs (generalized, symbolic C / single-bit P)
X u< 2 => select (icmp eq X, 0), C, C+1 -> add X, C(eq and ne forms, C symbolic): https://alive2.llvm.org/ce/z/8oJ_7NX in {0, P}, P = 2^k => select (icmp eq X, 0), C, C op P -> op C, Xfor add/or/xor/sub (C, P symbolic): https://alive2.llvm.org/ce/z/q4Zeg2C == INT_MAXyieldsadd nuwonly; sign bit; vector;andwith a non-power-of-two mask): https://alive2.llvm.org/ce/z/vwT7Kerange(i32 0, 3)must not fold (counterexamplea = 2), andnswmust not be added for theINT_MAXarm: https://alive2.llvm.org/ce/z/m5zjKWCompile-time and real-world impact
originalinputs, instcombine only): no IR changes, no regressions.Notes
isIntOrIntVectorTy): the consumerszext/truncthe compared value.CreateAndis false on this path, so the transform is instruction-count neutral even when the compare has other users (@select_icmp_bittest_range_multiuse_cmp).icmp eq X, 0totrunc nuwinfoldICmpUsingKnownBitswould only cover the bit-0 case;@select_icmp_bittest_assume_high_bit(the single known bit is the sign bit) is not expressible that way.Closes llvm#206708 (upstream).
Thanks to @Kristianerik, who described the same idea in the issue.
AI tool usage
Per the LLVM AI Tool Use Policy: AI assistance was used while writing parts of this patch and its tests. All transformations were verified with Alive2 (links above),
llvm-litpasses overllvm/test/Transforms/InstCombinewith an assertions-enabled build, and I reviewed the final implementation and every test expectation myself.