Skip to content

Infinite/exponential loop unswitch due to invariant condition injection #66868

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

Closed
nikic opened this issue Sep 20, 2023 · 4 comments · Fixed by #66883 or llvm/llvm-project-release-prs#705
Labels
llvm:hang Compiler hang (infinite loop)

Comments

@nikic
Copy link
Contributor

nikic commented Sep 20, 2023

Running opt -passes='simple-loop-unswitch<nontrivial>' on https://gist.github.com/nikic/852fbcaa307231c3aa0a5cdc1f057ede does not finish after 5 minutes.

My attempt to reduce this resulted in https://gist.github.com/nikic/52d9fc05e986a4e9279620461fe54d26, which does finish after about 50 seconds, resulting in this output: https://gist.github.com/nikic/45fb4b1448caa7195618c5001874f8cc The first test case might not be infinite as well, just very slow.

This problem does not occur when -simple-loop-unswitch-inject-invariant-conditions=false is passed.

cc @serguei-katkov

@nikic
Copy link
Contributor Author

nikic commented Sep 20, 2023

I think what happens here is that we have a lot of branches where we can inject invariant conditions. Then we'll pick one of those and unswitch, creating two loops, where in one the relevant original condition is removed and in the other marked as llvm.invariant.condition.injection.disabled. However, we can then take the next-best injectable condition in each of those loops and repeat the process, increasing the number of loops by a factor of two in each iteration until we reach a cost threshold.

Possibly this should work more like partial unswitching, where we mark the entire old loop as llvm.loop.unswitch.partial.disable. In that case we should only end up with a linear number of loops (though even that seems potentially quite unprofitable?)

@nikic
Copy link
Contributor Author

nikic commented Sep 21, 2023

This cherry-pick also includes the fix for #63962 (a miscompile) in the same code.

/cherry-pick afd7db4 8362cae

@llvmbot
Copy link
Member

llvmbot commented Sep 21, 2023

/branch llvm/llvm-project-release-prs/issue66868

llvmbot pushed a commit to llvm/llvm-project-release-prs that referenced this issue Sep 21, 2023
When unswitching via invariant condition injection, we currently
mark the condition in the old loop, so that it does not get
unswitched again. However, if there are multiple branches for
which conditions can be injected, then we can do that for both
the old and new loop. This means that the number of unswitches
increases exponentially.

Change the handling to be more similar to partial unswitching,
where we instead mark the whole loop, rather than a single
condition. This means that we will only generate a linear number
of loops.

TBH I think even that is still highly undesirable, and we should
probably be unswitching all candidates at the same time, so that
we end up with only two loops. But at least this mitigates the
worst case.

The test case is a reduced variant that generates 1700 lines of IR
without this patch and 290 with it.

Fixes llvm/llvm-project#66868.

(cherry picked from commit 8362cae71b80bc43c8c680cdfb13c495705a622f)
@llvmbot
Copy link
Member

llvmbot commented Sep 21, 2023

/pull-request llvm/llvm-project-release-prs#705

@nikic nikic moved this from Needs Triage to Needs Review in LLVM Release Status Sep 21, 2023
tru pushed a commit to llvm/llvm-project-release-prs that referenced this issue Sep 27, 2023
When unswitching via invariant condition injection, we currently
mark the condition in the old loop, so that it does not get
unswitched again. However, if there are multiple branches for
which conditions can be injected, then we can do that for both
the old and new loop. This means that the number of unswitches
increases exponentially.

Change the handling to be more similar to partial unswitching,
where we instead mark the whole loop, rather than a single
condition. This means that we will only generate a linear number
of loops.

TBH I think even that is still highly undesirable, and we should
probably be unswitching all candidates at the same time, so that
we end up with only two loops. But at least this mitigates the
worst case.

The test case is a reduced variant that generates 1700 lines of IR
without this patch and 290 with it.

Fixes llvm/llvm-project#66868.

(cherry picked from commit 8362cae71b80bc43c8c680cdfb13c495705a622f)
@tru tru moved this from Needs Review to Done in LLVM Release Status Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:hang Compiler hang (infinite loop)
Projects
2 participants