-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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.
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
[JumpThreading] Incorrect elimination of condition #70651
Labels
Comments
nikic
referenced
this issue
Oct 30, 2023
) Resolves #67916 . This patch folds `Ext(icmp (A, xxx)) Pred shr(A, BW - 1)` into `i1 Pred A s< 0`. [Alive2](https://alive2.llvm.org/ce/z/k53Xwa).
On the surface, the fix here is as simple as adding a |
nikic
added a commit
that referenced
this issue
Oct 30, 2023
nikic
added a commit
to nikic/llvm-project
that referenced
this issue
Oct 30, 2023
When evaluating comparisons in predecessors, phi operands are translated into the predecessor. If the translation is across a backedge, this means that the two operands of the icmp will be from two different loop iterations, resulting in incorrect simplification. Fix this by not performing the phi translation for phis in loop headers. Note: This is not a complete fix. If the jump-threading-across-loop-headers option is enabled, the LoopHeaders variable does not get populated. Additional changes will be needed to fix that case. Related to llvm#70651.
As this is actively causing issues, let's start with the simple part of the fix: #70664 |
nikic
added a commit
that referenced
this issue
Oct 31, 2023
When evaluating comparisons in predecessors, phi operands are translated into the predecessor. If the translation is across a backedge, this means that the two operands of the icmp will be from two different loop iterations, resulting in incorrect simplification. Fix this by not performing the phi translation for phis in loop headers. Note: This is not a complete fix. If the jump-threading-across-loop-headers option is enabled, the LoopHeaders variable does not get populated. Additional changes will be needed to fix that case. Related to #70651.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The following test case is miscomiled by JumpThreading (https://llvm.godbolt.org/z/o9W6efoTf):
The
%overflow
condition is folded tofalse
as part ofprocessBranchOnXOR()
.I believe the root cause is that
computeValueKnownInPredecessorsImpl()
performs phi translation of one icmp operand (the phi node), while leaving the other alone, and then trying to simplify it. This means that we are working on icmp operands from two different loop iterations.The text was updated successfully, but these errors were encountered: