[llvm] [IR] Avoid self-referencing values caused by PHI node removal (PR #129501)

Robert Imschweiler via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 19 02:03:23 PDT 2025


ro-i wrote:

The error occurs while handling a self-referencing icmp instruction, `%C4`, as I described [above](https://github.com/llvm/llvm-project/pull/129501#issuecomment-2706869329). Initially, the instruction is not self-referencing. It becomes that way by actions of the pass. This leads to recursion and segfault. Of course, I could just prevent the recursion (that would be pretty easy), but that wouldn't fix the problem, right? Because it's only the symptom, not the disease...

> Simplest fix would be to not to try to cleanup unreachable code in JumpThreading. [...] Or call EliminateUnreachableBlocks at the end.

That sounds reasonable. I implemented this yesterday and added an `EliminateUnreachableBlocks` at the end of the pass (and also added a flag to disable the elimination so that the tests relying on unreachable blocks don't become meaningless). However, when I recompiled llvm in release mode to run all the tests before pushing, I noticed some compiler errors. They happen during compilation of the offload runtimes. I guess that is what the "We must remove BB to prevent invalid IR." comment was about...
I researched the origin of that comment and found https://reviews.llvm.org/D44177 (from 2018). Even back then, they were aware that the handling of (un)reachability is essentially broken in this pass. They concluded:
> The only way I know of preventing any wasted work done on unreachable regions is to check if BB is to maintain a set of blocks that are reachable from entry. We'd need to verify this state with calls to DT on almost every iteration inside ProcessBlock. It's not practical as it greatly lengthens compile time. For large functions like those found in sqlite3.c I was seeing a slowdown of 20% or more to track the 3,000+ blocks in the main REPL function when I initially tried this approach.

A few months before that (so in 2017), there was https://reviews.llvm.org/D34135. They also found that the JumpThreading pass has a fundamental problem, so they just prevented the bug they were dealing with instead of fixing the root cause :/

I mean, all this unreachable mess *could* be solved by materializing the lazy domtree info and just use the dominator tree to check for reachability, but that would be expensive. Also, I *could* call `EliminateUnreachableBlocks` on every inner iteration of the pass, but that would be expensive, too, I guess.
(Someone [here](https://reviews.llvm.org/D34135#780608) mentioned something about a "dynamic" domtree, but ig that never made it into upstream?)

TLDR:
- I fix the recursion by just preventing the recursion and not fixing the root cause of the recursion. (Easy)
- I fix the handling of unreachable blocks by either materializing the domtree info or by eliminating unreachable blocks on every inner iteration of the pass. (Not so difficult to implement, but may potentially increase compile time)
- Someone rewrites the pass. (I currently cannot do that.)

https://github.com/llvm/llvm-project/pull/129501


More information about the llvm-commits mailing list