From ac4006b0d69ff3349fdd6c0bf8e4dad9504d438a Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Fri, 10 Jun 2022 21:51:16 -0700 Subject: [PATCH] [InstCombine] Don't slice up PHIs when pred BB has catchswitch If an integer PHI has an illegal type (according to the data layout) and it is only used by `trunc` or `trunc(lshr)` operations, we split the PHI into various instructions in its predecessors: https://github.com/llvm/llvm-project/blob/6d1543a16797fa07eecea7e542df5b42422fc721/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp#L1536-L1543 So this can produce code like the following: Before: ``` pred: ... bb: %p = phi i8 [ %somevalue, %pred ], ... ... %tobool = trunc i8 %p to i1 use %tobool ... ``` In this code, `%p` has an illegal integer type, `i8`, and its only used in a `trunc` instruction later. In this case this pass puts extraction code in its predecessors: After: ``` pred: ... %t = and i8 %somevalue, 1 %extract = icmp ne i8 %t, 0 bb: %p.new = phi i1 [ %extract, %pred ], ... use %p.new instead of %tobool ``` But this doesn't work if `pred` is a `catchswitch` BB because it cannot have any non-PHI instructions. This CL ensures we bail out in that case. Fixes https://github.com/llvm/llvm-project/issues/55803. Reviewed By: dschuff Differential Revision: https://reviews.llvm.org/D127699 --- .../Transforms/InstCombine/InstCombinePHI.cpp | 7 ++ .../Transforms/InstCombine/catchswitch-phi.ll | 83 ++++++++++++++++++- 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp index 538e84e2efa6..83f7846bbfce 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp @@ -1121,6 +1121,13 @@ Instruction *InstCombinerImpl::SliceUpIllegalIntegerPHI(PHINode &FirstPhi) { return nullptr; } + // If the incoming value is a PHI node before a catchswitch, we cannot + // extract the value within that BB because we cannot insert any non-PHI + // instructions in the BB. + for (auto *Pred : PN->blocks()) + if (isa(Pred->getFirstNonPHI())) + return nullptr; + for (User *U : PN->users()) { Instruction *UserI = cast(U); diff --git a/llvm/test/Transforms/InstCombine/catchswitch-phi.ll b/llvm/test/Transforms/InstCombine/catchswitch-phi.ll index df5d3e00ab4b..54f922381065 100644 --- a/llvm/test/Transforms/InstCombine/catchswitch-phi.ll +++ b/llvm/test/Transforms/InstCombine/catchswitch-phi.ll @@ -8,9 +8,14 @@ target triple = "wasm32-unknown-unknown" declare void @foo() declare void @bar(%struct.quux*) +declare i32 @baz() declare i32 @__gxx_wasm_personality_v0(...) +; Function Attrs: noreturn +declare void @llvm.wasm.rethrow() #0 -define void @test(i1 %c1) personality i8* bitcast (i32 (...)* @__gxx_wasm_personality_v0 to i8*) { +; Test that a PHI in catchswitch BB are excluded from combining into a non-PHI +; instruction. +define void @test0(i1 %c1) personality i8* bitcast (i32 (...)* @__gxx_wasm_personality_v0 to i8*) { bb: %tmp0 = alloca %struct.blam, align 4 br i1 %c1, label %bb1, label %bb2 @@ -51,3 +56,79 @@ bb7: ; preds = %bb5, %bb4 call void @bar(%struct.quux* %tmp3) [ "funclet"(token %tmp6) ] unreachable } + +; Test that slicing-up of illegal integer type PHI does not happen in catchswitch +; BBs, which can't have any non-PHI instruction before the catchswitch. +define void @test1() personality i8* bitcast (i32 (...)* @__gxx_wasm_personality_v0 to i8*) { +entry: + invoke void @foo() + to label %invoke.cont unwind label %catch.dispatch1 + +invoke.cont: ; preds = %entry + %call = invoke i32 @baz() + to label %invoke.cont1 unwind label %catch.dispatch + +invoke.cont1: ; preds = %invoke.cont + %tobool = icmp ne i32 %call, 0 + br i1 %tobool, label %if.then, label %if.end + +if.then: ; preds = %invoke.cont1 + br label %if.end + +if.end: ; preds = %if.then, %invoke.cont1 + %ap.0 = phi i8 [ 1, %if.then ], [ 0, %invoke.cont1 ] + invoke void @foo() + to label %invoke.cont2 unwind label %catch.dispatch + +invoke.cont2: ; preds = %if.end + br label %try.cont + +catch.dispatch: ; preds = %if.end, %invoke.cont + ; %ap.2 in catch.dispatch1 BB has an illegal integer type (i8) in the data + ; layout, and it is only used by trunc or trunc(lshr) operations. In this case + ; InstCombine will split this PHI in its predecessors, which include this + ; catch.dispatch BB. This splitting involves creating non-PHI instructions, + ; such as 'and' or 'icmp' in this BB, which is not valid for a catchswitch BB. + ; So if one of sliced-up PHI's predecessor is a catchswitch block, we don't + ; optimize that case and bail out. This BB should be preserved intact after + ; InstCombine and the pass shouldn't produce invalid code. + ; CHECK: catch.dispatch: + ; CHECK-NEXT: phi + ; CHECK-NEXT: catchswitch + %ap.1 = phi i8 [ %ap.0, %if.end ], [ 0, %invoke.cont ] + %tmp0 = catchswitch within none [label %catch.start] unwind label %catch.dispatch1 + +catch.start: ; preds = %catch.dispatch + %tmp1 = catchpad within %tmp0 [i8* null] + br i1 0, label %catch, label %rethrow + +catch: ; preds = %catch.start + catchret from %tmp1 to label %try.cont + +rethrow: ; preds = %catch.start + invoke void @llvm.wasm.rethrow() #0 [ "funclet"(token %tmp1) ] + to label %unreachable unwind label %catch.dispatch1 + +catch.dispatch1: ; preds = %rethrow, %catch.dispatch, %entry + %ap.2 = phi i8 [ %ap.1, %catch.dispatch ], [ %ap.1, %rethrow ], [ 0, %entry ] + %tmp2 = catchswitch within none [label %catch.start1] unwind to caller + +catch.start1: ; preds = %catch.dispatch1 + %tmp3 = catchpad within %tmp2 [i8* null] + %tobool1 = trunc i8 %ap.2 to i1 + br i1 %tobool1, label %if.then1, label %if.end1 + +if.then1: ; preds = %catch.start1 + br label %if.end1 + +if.end1: ; preds = %if.then1, %catch.start1 + catchret from %tmp3 to label %try.cont + +try.cont: ; preds = %if.end1, %catch, %invoke.cont2 + ret void + +unreachable: ; preds = %rethrow + unreachable +} + +attributes #0 = { noreturn }