RPCS3/llvm-mirror
1
0
Fork 0
mirror of https://github.com/RPCS3/llvm-mirror.git synced 2024-11-23 03:02:36 +01:00
Code Issues Projects Releases Wiki Activity

[WebAssembly] Don't analyze branches after CFGStackify

Browse Source 
Summary:
`WebAssembly::analyzeBranch` now does not analyze anything if the
function is CFG stackified. We were previously doing similar things by
checking if a branch's operand is whether an integer or an MBB, but this
failed to bail out when a BB did not have any terminators.

Consider this case:
```
bb0:
  try $label0
  call @foo    // unwinds to %ehpad
bb1:
  ...
  br $label0   // jumps to %cont. can be deleted
ehpad:
  catch
  ...
cont:
  end_try
```
Here `br $label0` will be deleted in CFGStackify's
`removeUnnecessaryInstrs` function, because we jump to the %cont block
even without the branch. But in this case, MachineVerifier fails to
verify this, because `ehpad` is not a successor of `bb1` even if `bb1`
does not have any terminators. MachineVerifier incorrectly thinks `bb1`
falls through to the next block.

This pass now consistently rejects all analysis after CFGStackify
whether a BB has terminators or not, also making the MachineVerifier
work. (MachineVerifier does not try to verify relationships between BBs
if `analyzeBranch` fails, the behavior we want after CFGStackify.)

This also adds a new option `-wasm-disable-ehpad-sort` for testing. This
option helps create the sorted order we want to test, and without the
fix in this patch, the tests in cfg-stackify-eh.ll fail at
MachineVerifier with `-wasm-disable-ehpad-sort`.

Reviewers: dschuff

Subscribers: sunfish, sbc100, jgravelle-google, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D59740

llvm-svn: 357015
This commit is contained in:
Heejin Ahn 2019-03-26 18:21:20 +00:00
parent f06f63f7b4
commit 59aee97e36
3 changed files with 28 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
lib/Target/WebAssembly/WebAssemblyCFGSort.cpp
View File

@ -34,6 +34,14 @@ using namespace llvm;
#define DEBUG_TYPE "wasm-cfg-sort"
// Option to disable EH pad first sorting. Only for testing unwind destination
// mismatches in CFGStackify.
static cl::opt<bool> WasmDisableEHPadSort(
"wasm-disable-ehpad-sort", cl::ReallyHidden,
cl::desc(
"WebAssembly: Disable EH pad-first sort order. Testing purpose only."),
cl::init(false));
namespace {
// Wrapper for loops and exceptions
@ -187,10 +195,12 @@ namespace {
struct CompareBlockNumbers {
bool operator()(const MachineBasicBlock *A,
const MachineBasicBlock *B) const {
if (!WasmDisableEHPadSort) {
if (A->isEHPad() && !B->isEHPad())
return false;
if (!A->isEHPad() && B->isEHPad())
return true;
}
return A->getNumber() > B->getNumber();
}
@ -199,11 +209,12 @@ struct CompareBlockNumbers {
struct CompareBlockNumbersBackwards {
bool operator()(const MachineBasicBlock *A,
const MachineBasicBlock *B) const {
// We give a higher priority to an EH pad
if (!WasmDisableEHPadSort) {
if (A->isEHPad() && !B->isEHPad())
return false;
if (!A->isEHPad() && B->isEHPad())
return true;
}
return A->getNumber() < B->getNumber();
}

19
lib/Target/WebAssembly/WebAssemblyInstrInfo.cpp
View File

@ -101,6 +101,13 @@ bool WebAssemblyInstrInfo::analyzeBranch(MachineBasicBlock &MBB,
MachineBasicBlock *&FBB,
SmallVectorImpl<MachineOperand> &Cond,
bool /*AllowModify*/) const {
const auto &MFI = *MBB.getParent()->getInfo<WebAssemblyFunctionInfo>();
// WebAssembly has control flow that doesn't have explicit branches or direct
// fallthrough (e.g. try/catch), which can't be modeled by analyzeBranch. It
// is created after CFGStackify.
if (MFI.isCFGStackified())
return true;
bool HaveCond = false;
for (MachineInstr &MI : MBB.terminators()) {
switch (MI.getOpcode()) {
@ -110,9 +117,6 @@ bool WebAssemblyInstrInfo::analyzeBranch(MachineBasicBlock &MBB,
case WebAssembly::BR_IF:
if (HaveCond)
return true;
// If we're running after CFGStackify, we can't optimize further.
if (!MI.getOperand(0).isMBB())
return true;
Cond.push_back(MachineOperand::CreateImm(true));
Cond.push_back(MI.getOperand(1));
TBB = MI.getOperand(0).getMBB();
@ -121,18 +125,12 @@ bool WebAssemblyInstrInfo::analyzeBranch(MachineBasicBlock &MBB,
case WebAssembly::BR_UNLESS:
if (HaveCond)
return true;
// If we're running after CFGStackify, we can't optimize further.
if (!MI.getOperand(0).isMBB())
return true;
Cond.push_back(MachineOperand::CreateImm(false));
Cond.push_back(MI.getOperand(1));
TBB = MI.getOperand(0).getMBB();
HaveCond = true;
break;
case WebAssembly::BR:
// If we're running after CFGStackify, we can't optimize further.
if (!MI.getOperand(0).isMBB())
return true;
if (!HaveCond)
TBB = MI.getOperand(0).getMBB();
else
@ -141,9 +139,6 @@ bool WebAssemblyInstrInfo::analyzeBranch(MachineBasicBlock &MBB,
case WebAssembly::BR_ON_EXN:
if (HaveCond)
return true;
// If we're running after CFGStackify, we can't optimize further.
if (!MI.getOperand(0).isMBB())
return true;
Cond.push_back(MachineOperand::CreateImm(true));
Cond.push_back(MI.getOperand(2));
TBB = MI.getOperand(0).getMBB();

1
test/CodeGen/WebAssembly/cfg-stackify-eh.ll
View File

@ -1,5 +1,6 @@
; 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 -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
target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
target triple = "wasm32-unknown-unknown"