[llvm] ac4006b - [InstCombine] Don't slice up PHIs when pred BB has catchswitch

Heejin Ahn via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 13 18:32:19 PDT 2022


Author: Heejin Ahn
Date: 2022-06-13T18:32:09-07:00
New Revision: ac4006b0d69ff3349fdd6c0bf8e4dad9504d438a

URL: https://github.com/llvm/llvm-project/commit/ac4006b0d69ff3349fdd6c0bf8e4dad9504d438a
DIFF: https://github.com/llvm/llvm-project/commit/ac4006b0d69ff3349fdd6c0bf8e4dad9504d438a.diff

LOG: [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

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
    llvm/test/Transforms/InstCombine/catchswitch-phi.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
index 538e84e2efa66..83f7846bbfce5 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<CatchSwitchInst>(Pred->getFirstNonPHI()))
+        return nullptr;
+
     for (User *U : PN->users()) {
       Instruction *UserI = cast<Instruction>(U);
 

diff  --git a/llvm/test/Transforms/InstCombine/catchswitch-phi.ll b/llvm/test/Transforms/InstCombine/catchswitch-phi.ll
index df5d3e00ab4b2..54f922381065e 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 }


        


More information about the llvm-commits mailing list