Allow splitting a basic block before a PHI node

`split_block!` and the conditional splitting functions rejected a PHI node
as the split point, while LLVM accepts it and leaves the PHI nodes that
move to the tail for the caller to fix. Julia's LowerPTLS pass relies on
that: it creates an empty PHI node, splits the block before it, and adds
the incoming values from the new blocks afterwards. Accept PHI nodes, and
document what the caller has to fix; exception-handling pads remain
rejected, as no fix can make splitting before them valid.

Author: maleadt

src/analysis.jl

@@ -148,9 +148,11 @@ name:                            ; the returned block, with the rest of the orig
 
 Returns the new block. The PHI nodes in the successors of the original block are updated
 to refer to the new block. The block must have a terminator, and the split point must not be
-at the end of the block, before a PHI node or before an exception-handling pad. Debug
-records at the split point move along when the insertion point is at their head (e.g.,
-`LLVM.after(prev)`), and stay in the original block otherwise (e.g., `LLVM.before(inst)`).
+at the end of the block or before an exception-handling pad. It can be before a PHI node, as
+in LLVM, but the PHI nodes that move to the new block are not updated: their only
+predecessor is the original block now, so the caller has to fix them. Debug records at the
+split point move along when the insertion point is at their head (e.g., `LLVM.after(prev)`),
+and stay in the original block otherwise (e.g., `LLVM.before(inst)`).
 
 Analyses, like the dominator tree, are not updated.
 
@@ -162,9 +164,9 @@ function split_block!(pos::InsertionPoint{Instruction}, name::AbstractString="")
         throw(ArgumentError("Cannot split a basic block without a terminator"))
     pos.anchor == C_NULL &&
         throw(ArgumentError("Cannot split a basic block at its end"))
-    Instruction(pos.anchor) isa Union{PHIInst,LandingPadInst,CatchPadInst,CleanupPadInst,
+    Instruction(pos.anchor) isa Union{LandingPadInst,CatchPadInst,CleanupPadInst,
                                       CatchSwitchInst} &&
-        throw(ArgumentError("Cannot split a basic block before a PHI node or an exception-handling pad"))
+        throw(ArgumentError("Cannot split a basic block before an exception-handling pad"))
     BasicBlock(API.LLVMExtraSplitBasicBlock(bb, pos.anchor, pos.head, name))
 end
 split_block!(inst::Instruction, name::AbstractString="") = split_block!(before(inst), name)
@@ -187,8 +189,10 @@ tail:                            ; `at` and the rest of the original block
 ```
 
 Returns the terminator of the new block, before which instructions can be inserted. The
-instruction `at` must not be a PHI node or an exception-handling pad. The new branches get
-the debug location of `at`.
+instruction `at` must not be an exception-handling pad. It can be a PHI node, as in LLVM,
+but the PHI nodes in the tail are not updated for its new predecessors, so the caller has
+to fix them (e.g., by creating a PHI node first, to split before it and add its incoming
+values afterwards). The new branches get the debug location of `at`.
 
 The `weights` of the conditional branch, a pair of integers for when `cond` is true and
 when it is false (e.g., `(1, 9)` when the new block is rarely executed), are attached to it
@@ -264,8 +268,8 @@ function check_conditional_split(cond::Value, at::Instruction)
     bb === nothing && throw(ArgumentError("Instruction is not part of a basic block"))
     terminator(bb) === nothing &&
         throw(ArgumentError("Cannot split a basic block without a terminator"))
-    at isa Union{PHIInst,LandingPadInst,CatchPadInst,CleanupPadInst,CatchSwitchInst} &&
-        throw(ArgumentError("Cannot split a basic block before a PHI node or an exception-handling pad"))
+    at isa Union{LandingPadInst,CatchPadInst,CleanupPadInst,CatchSwitchInst} &&
+        throw(ArgumentError("Cannot split a basic block before an exception-handling pad"))
     return
 end
 

test/analysis.jl

@@ -599,8 +599,6 @@ end
         end
 
         @test_throws ArgumentError split_block_and_insert_if_then!(f.parameters[1], d)
-        @test_throws ArgumentError split_block_and_insert_if_then!(
-            ConstantInt(true), first(exit.instructions))
     end
 
     # the dominator tree must be one of the function that is changed
@@ -733,6 +731,60 @@ end
         @test_throws ArgumentError split_block_and_insert_if_then_else!(c, d; weights=(1,))
     end
 
+    # before a PHI node, created first to be given its incoming values afterwards (like
+    # Julia's LowerPTLS pass does), or before an exception-handling pad, which is invalid
+    @dispose ctx=Context() builder=IRBuilder() begin
+        pers = supports_typed_pointers(ctx) ? "i8* bitcast (i32 (...)* @pers to i8*)" :
+                                              "ptr @pers"
+        lp = supports_typed_pointers(ctx) ? "{ i8*, i32 }" : "{ ptr, i32 }"
+        mod = parse(LLVM.Module, """
+            declare void @g()
+            declare i32 @pers(...)
+
+            define i32 @f(i32 %x, i1 %c) personality $pers {
+            entry:
+              %a = add i32 %x, 1
+              invoke void @g() to label %exit unwind label %pad
+            exit:
+              ret i32 %a
+            pad:
+              %lp = landingpad $lp cleanup
+              ret i32 0
+            }""")
+        f = mod.functions["f"]
+        entry, exit, pad = f.blocks
+        a = first(entry.instructions)
+        ret = exit.terminator
+        c = f.parameters[2]
+        position!(builder, LLVM.before(ret))
+        p = phi!(builder, LLVM.Int32Type())
+        replace_uses!(a, p)
+        then_term, else_term = split_block_and_insert_if_then_else!(c, p)
+        @test p.parent == only(then_term.successors)
+        push!(p.incoming, (a, then_term.parent))
+        push!(p.incoming, (ConstantInt(Int32(0)), else_term.parent))
+        @test ret.operands[1] == p
+        verify(f)
+
+        # the same with a single new block
+        position!(builder, LLVM.before(ret))
+        q = phi!(builder, LLVM.Int32Type())
+        replace_uses!(p, q)
+        head = p.parent
+        term = split_block_and_insert_if_then!(c, q)
+        @test q.parent == only(term.successors)
+        push!(q.incoming, (ConstantInt(Int32(1)), term.parent))
+        push!(q.incoming, (p, head))
+        @test ret.operands[1] == q
+        verify(f)
+
+        @test_throws ArgumentError split_block_and_insert_if_then_else!(
+            c, first(pad.instructions))
+        @test_throws ArgumentError split_block_and_insert_if_then!(c, first(pad.instructions))
+        @test_throws ArgumentError split_block!(first(pad.instructions))
+        dispose(mod)
+    end
+
     # the dominator tree must be one of the function that is changed
     @dispose ctx=Context() mod=parse(LLVM.Module, """
         define void @f(i1 %c) {
@@ -1015,8 +1067,15 @@ end
         @test only(tail2.instructions) == br
         verify(f)
 
+        # before a PHI node, which moves to the new block without being updated
+        tail3 = split_block!(r)
+        @test collect(tail3.instructions)[1] == r
+        @test only(exit.terminator.successors) == tail3
+        @test verification_error(f) !== nothing
+        move!(r, LLVM.before(exit.terminator))
+        verify(f)
+
         @test_throws ArgumentError split_block!(LLVM.at_end(tail2))
-        @test_throws ArgumentError split_block!(r)
         detached = BasicBlock("detached")
         @test_throws ArgumentError split_block!(LLVM.at_begin(detached))
         erase!(detached)