[llvm] 3fe6ea4 - [WebAssembly] Fix a bug in removing unnecessary branches
Heejin Ahn via llvm-commits
llvm-commits at lists.llvm.org
Thu May 28 19:44:42 PDT 2020
Author: Heejin Ahn
Date: 2020-05-28T19:44:27-07:00
New Revision: 3fe6ea4641b20c3406e2ef10c0f3782788585030
URL: https://github.com/llvm/llvm-project/commit/3fe6ea4641b20c3406e2ef10c0f3782788585030
DIFF: https://github.com/llvm/llvm-project/commit/3fe6ea4641b20c3406e2ef10c0f3782788585030.diff
LOG: [WebAssembly] Fix a bug in removing unnecessary branches
Summary:
One of the things `removeUnnecessaryInstrs()` in CFGStackify does is to
remove an unnecessary unconditinal branch before an EH pad. When there
is an unconditional branch right before a catch instruction and it
branches to the end of `end_try` marker, we don't need the branch,
because it there is no exception, the control flow transfers to
that point anyway.
```
bb0:
try
...
br bb2 <- Not necessary
bb1:
catch
...
bb2:
end
```
This applies when we have a conditional branch followed by an
unconditional one, in which case we should only remove the unconditional
branch. For example:
```
bb0:
try
...
br_if someplace_else
br bb2 <- Not necessary
bb1:
catch
...
bb2:
end
```
But `TargetInstrInfo::removeBranch` we used removed all existing
branches when there are multiple ones. This patch fixes it by only
deleting the last (= unconditional) branch manually.
Also fixes some `preds` comments in the test file.
Reviewers: dschuff
Subscribers: sbc100, jgravelle-google, hiraditya, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D80572
Added:
Modified:
llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll
Removed:
################################################################################
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
index 23a5aa61daa9..103fe97c6e93 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
@@ -670,9 +670,27 @@ void WebAssemblyCFGStackify::removeUnnecessaryInstrs(MachineFunction &MF) {
MachineBasicBlock *EHPadLayoutPred = MBB.getPrevNode();
MachineBasicBlock *Cont = BeginToEnd[EHPadToTry[&MBB]]->getParent();
bool Analyzable = !TII.analyzeBranch(*EHPadLayoutPred, TBB, FBB, Cond);
+ // This condition means either
+ // 1. This BB ends with a single unconditional branch whose destinaion is
+ // Cont.
+ // 2. This BB ends with a conditional branch followed by an unconditional
+ // branch, and the unconditional branch's destination is Cont.
+ // In both cases, we want to remove the last (= unconditional) branch.
if (Analyzable && ((Cond.empty() && TBB && TBB == Cont) ||
- (!Cond.empty() && FBB && FBB == Cont)))
- TII.removeBranch(*EHPadLayoutPred);
+ (!Cond.empty() && FBB && FBB == Cont))) {
+ bool ErasedUncondBr = false;
+ for (auto I = EHPadLayoutPred->end(), E = EHPadLayoutPred->begin();
+ I != E; --I) {
+ auto PrevI = std::prev(I);
+ if (PrevI->isTerminator()) {
+ assert(PrevI->getOpcode() == WebAssembly::BR);
+ PrevI->eraseFromParent();
+ ErasedUncondBr = true;
+ break;
+ }
+ }
+ assert(ErasedUncondBr && "Unconditional branch not erased!");
+ }
}
// When there are block / end_block markers that overlap with try / end_try
diff --git a/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll b/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll
index 188ad22c89fc..fcc30466594e 100644
--- a/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll
+++ b/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll
@@ -1,5 +1,6 @@
; REQUIRES: asserts
; RUN: llc < %s -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-keep-registers -disable-block-placement -verify-machineinstrs -fast-isel=false -machine-sink-split-probability-threshold=0 -cgp-freq-ratio-to-skip-merge=1000 -exception-model=wasm -mattr=+exception-handling | FileCheck %s
+; RUN: llc < %s -disable-wasm-fallthrough-return-opt -disable-block-placement -verify-machineinstrs -fast-isel=false -machine-sink-split-probability-threshold=0 -cgp-freq-ratio-to-skip-merge=1000 -exception-model=wasm -mattr=+exception-handling
; RUN: llc < %s -O0 -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-keep-registers -verify-machineinstrs -exception-model=wasm -mattr=+exception-handling | FileCheck %s --check-prefix=NOOPT
; RUN: llc < %s -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-keep-registers -disable-block-placement -verify-machineinstrs -fast-isel=false -machine-sink-split-probability-threshold=0 -cgp-freq-ratio-to-skip-merge=1000 -exception-model=wasm -mattr=+exception-handling -wasm-disable-ehpad-sort | FileCheck %s --check-prefix=NOSORT
; RUN: llc < %s -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-keep-registers -disable-block-placement -verify-machineinstrs -fast-isel=false -machine-sink-split-probability-threshold=0 -cgp-freq-ratio-to-skip-merge=1000 -exception-model=wasm -mattr=+exception-handling -wasm-disable-ehpad-sort -stats 2>&1 | FileCheck %s --check-prefix=NOSORT-STAT
@@ -856,11 +857,11 @@ define void @test16(i32* %p, i32 %a, i32 %b) personality i8* bitcast (i32 (...)*
entry:
br label %loop
-loop:
+loop: ; preds = %try.cont, %entry
invoke void @foo()
to label %bb0 unwind label %catch.dispatch0
-bb0:
+bb0: ; preds = %loop
%cmp = icmp ne i32 %a, %b
br i1 %cmp, label %bb1, label %last
@@ -886,10 +887,50 @@ catch.start1: ; preds = %catch.dispatch1
%7 = call i32 @llvm.wasm.get.ehselector(token %5)
catchret from %5 to label %try.cont
-try.cont: ; preds = %catch.start, %loop
+try.cont: ; preds = %catch.start1, %catch.start0, %bb1
br label %loop
-last:
+last: ; preds = %bb0
+ ret void
+}
+
+; Tests if CFGStackify's removeUnnecessaryInstrs() removes unnecessary branches
+; correctly.
+; CHECK-LABEL: test17
+define void @test17(i32 %n) personality i8* bitcast (i32 (...)* @__gxx_wasm_personality_v0 to i8*) {
+entry:
+ invoke void @foo()
+ to label %for.body unwind label %catch.dispatch
+
+for.body: ; preds = %for.end, %entry
+ %i = phi i32 [ %inc, %for.end ], [ 0, %entry ]
+ invoke void @foo()
+ to label %for.end unwind label %catch.dispatch
+
+; Before going to CFGStackify, this BB will have a conditional branch followed
+; by an unconditional branch. CFGStackify should remove only the unconditional
+; one.
+for.end: ; preds = %for.body
+ %inc = add nuw nsw i32 %i, 1
+ %exitcond = icmp eq i32 %inc, %n
+ br i1 %exitcond, label %try.cont, label %for.body
+; CHECK: br_if
+; CHECK-NOT: br
+; CHECK: end_loop
+; CHECK: catch
+
+catch.dispatch: ; preds = %for.body, %entry
+ %0 = catchswitch within none [label %catch.start] unwind to caller
+
+catch.start: ; preds = %catch.dispatch
+ %1 = catchpad within %0 [i8* null]
+ %2 = call i8* @llvm.wasm.get.exception(token %1)
+ %3 = call i32 @llvm.wasm.get.ehselector(token %1)
+ %4 = call i8* @__cxa_begin_catch(i8* %2) #2 [ "funclet"(token %1) ]
+ call void @__cxa_end_catch() [ "funclet"(token %1) ]
+ catchret from %1 to label %try.cont
+
+try.cont: ; preds = %catch.start, %for.end
ret void
}
More information about the llvm-commits
mailing list