[llvm] [StructurizeCFG] Fix incorrect zero-cost hoisting in nested control flow (PR #183792)

[Yaxun Liu via llvm-commits]

yxsamliu wrote:

> Hi @yxsamliu , there is a related miscompilation bug:
> 
> ```llvm
> define void @test_switch(i32 %val) {
> entry:
>   switch i32 %val, label %default [
>     i32 0, label %case0
>     i32 1, label %case1
>   ]
> 
> case0:
>   %v0 = add i32 1, 2
>   br label %merge
> 
> case1:
>   %v1 = add i32 3, 4
>   br label %merge
> 
> default:
>   %vd = add i32 5, 6
>   br label %merge
> 
> merge:
>   %phi = phi i32 [ %v0, %case0 ], [ %v1, %case1 ], [ %vd, %default ]
>   ret void
> }
> ```
> 
> With opt built on this patch, it's transformed into:
> 
> ```llvm
> define void @test_switch(i32 %val) {
> entry:
>   br i1 undef, label %case1, label %Flow
> 
> Flow:                                             ; preds = %Flow1, %entry
>   %0 = phi i32 [ %1, %Flow1 ], [ poison, %entry ]
>   br label %merge
> 
> case0:                                            ; preds = %case1
>   %v0 = add i32 1, 2
>   br i1 undef, label %default, label %Flow2
> 
> case1:                                            ; preds = %entry
>   %v1 = add i32 3, 4
>   br i1 undef, label %case0, label %Flow1
> 
> Flow1:                                            ; preds = %Flow2, %case1
>   %1 = phi i32 [ %2, %Flow2 ], [ %v1, %case1 ]
>   br label %Flow
> 
> default:                                          ; preds = %case0
>   %vd = add i32 5, 6
>   br label %Flow2
> 
> Flow2:                                            ; preds = %default, %case0
>   %2 = phi i32 [ %vd, %default ], [ %v0, %case0 ]
>   br label %Flow1
> 
> merge:                                            ; preds = %Flow
>   ret void
> }
> ```
> 
> Alive2 proof: https://alive2.llvm.org/ce/z/Z9YwVt
> 
> **Command:**
> 
> ```shell
> opt -passes=lower-switch,fix-irreducible,unify-loop-exits,structurizecfg -S
> ```
> 
> > Note: This is a review assisted with a self-built agent. The reproducer was validated manually. Please let me know if anything is wrong.
> 
> **Bug Triggering Analysis:** The `test_switch` case triggers a bug in `structurizecfg` when combined with `lower-switch`. The `lower-switch` pass converts the `switch` into a series of conditional branches. Then `structurizecfg` attempts to structurize the resulting CFG. However, the transformation introduces `undef` values in branch conditions (e.g., `br i1 undef, label %case1, label %Flow`), which causes Alive2 to report "Source is more defined than target" and "UB triggered on br". This indicates that `structurizecfg` is incorrectly handling the CFG generated by `lower-switch`, possibly due to how it inserts conditions for the new flow blocks.
> 
> **Fix Weakness Analysis:** While the original patch fixed the issue of zero-cost hoisting in nested control flow by checking if the merge phi has exactly 2 incoming values, it does not address the broader issues of how `structurizecfg` handles complex control flow like switches (even after lowering). The fix prevents the specific miscompilation related to hoisting, but the underlying structurization logic still struggles with multi-way branches or the CFGs they produce, leading to undefined behavior in the transformed IR.

I couldn't reproduce the issue. It seems the problematic IR you got were missing lower-switch. StructurizeCFG doesn't handle `switch` instructions directly, so you cannot skip lower-switch. If you add lower-switch, the issue will disappear. Can you double check? Thanks.

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