[llvm] c87b5e7 - [WebAssembly] Fix subregion relationship in CFGSort
Heejin Ahn via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 1 08:13:01 PDT 2020
Author: Heejin Ahn
Date: 2020-04-01T08:12:41-07:00
New Revision: c87b5e7e22b2df92021ac5fcc69160901a5841a9
URL: https://github.com/llvm/llvm-project/commit/c87b5e7e22b2df92021ac5fcc69160901a5841a9
DIFF: https://github.com/llvm/llvm-project/commit/c87b5e7e22b2df92021ac5fcc69160901a5841a9.diff
LOG: [WebAssembly] Fix subregion relationship in CFGSort
Summary:
The previous code for determining the innermost region in CFGSort was
not correct. We determine subregion relationship by domination of their
headers, i.e., if region A's header dominates region B's header, B is a
subregion of A. Previously we assumed that if a BB belongs to both a
loop and an exception, the region with fewer number of BBs is the
innermost one. This may not be true, because while WebAssemblyException
contains BBs in all its subregions (loops or exceptions), MachineLoop
may not, because MachineLoop does not contain BBs that don't have a path
to its header even if they are dominated by its header.
Loop header <---|
| |
Exception header |
| \ |
A B |
| \ |
| C |
| |
Loop latch |
| |
-------------|
For example, in this CFG, the loop does not contain B and C, because
they don't have a path back to the loops header. But for CFGSort we
consider the exception here belongs to the loop and the exception should
be a subregion of the loop and scheduled together.
So here we should use `WE->contains(ML->getHeader())` (but not
`ML->contains(WE->getHeader())`, for the stated region above).
This also fixes some comments and deletes `Regions` vector in
`RegionInfo` class, which was not used anywere.
Reviewers: dschuff
Subscribers: sbc100, jgravelle-google, hiraditya, sunfish, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D77181
Added:
Modified:
llvm/lib/Target/WebAssembly/WebAssemblyCFGSort.cpp
llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll
Removed:
################################################################################
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyCFGSort.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyCFGSort.cpp
index b4354e852194..b186e32e788d 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyCFGSort.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyCFGSort.cpp
@@ -79,7 +79,6 @@ template <> bool ConcreteRegion<MachineLoop>::isLoop() const { return true; }
class RegionInfo {
const MachineLoopInfo &MLI;
const WebAssemblyExceptionInfo &WEI;
- std::vector<const Region *> Regions;
DenseMap<const MachineLoop *, std::unique_ptr<Region>> LoopMap;
DenseMap<const WebAssemblyException *, std::unique_ptr<Region>> ExceptionMap;
@@ -93,7 +92,14 @@ class RegionInfo {
const auto *WE = WEI.getExceptionFor(MBB);
if (!ML && !WE)
return nullptr;
- if ((ML && !WE) || (ML && WE && ML->getNumBlocks() < WE->getNumBlocks())) {
+ // We determine subregion relationship by domination of their headers, i.e.,
+ // if region A's header dominates region B's header, B is a subregion of A.
+ // WebAssemblyException contains BBs in all its subregions (loops or
+ // exceptions), but MachineLoop may not, because MachineLoop does not contain
+ // BBs that don't have a path to its header even if they are dominated by
+ // its header. So here we should use WE->contains(ML->getHeader()), but not
+ // ML->contains(WE->getHeader()).
+ if ((ML && !WE) || (ML && WE && WE->contains(ML->getHeader()))) {
// If the smallest region containing MBB is a loop
if (LoopMap.count(ML))
return LoopMap[ML].get();
@@ -368,6 +374,7 @@ static void sortBlocks(MachineFunction &MF, const MachineLoopInfo &MLI,
const Region *Region = RI.getRegionFor(&MBB);
if (Region && &MBB == Region->getHeader()) {
+ // Region header.
if (Region->isLoop()) {
// Loop header. The loop predecessor should be sorted above, and the
// other predecessors should be backedges below.
@@ -377,7 +384,7 @@ static void sortBlocks(MachineFunction &MF, const MachineLoopInfo &MLI,
"Loop header predecessors must be loop predecessors or "
"backedges");
} else {
- // Not a loop header. All predecessors should be sorted above.
+ // Exception header. All predecessors should be sorted above.
for (auto Pred : MBB.predecessors())
assert(Pred->getNumber() < MBB.getNumber() &&
"Non-loop-header predecessors should be topologically sorted");
@@ -386,7 +393,7 @@ static void sortBlocks(MachineFunction &MF, const MachineLoopInfo &MLI,
"Regions should be declared at most once.");
} else {
- // Not a loop header. All predecessors should be sorted above.
+ // Not a region header. All predecessors should be sorted above.
for (auto Pred : MBB.predecessors())
assert(Pred->getNumber() < MBB.getNumber() &&
"Non-loop-header predecessors should be topologically sorted");
diff --git a/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll b/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll
index efdb2c6d684c..a4d8537343ca 100644
--- a/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll
+++ b/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll
@@ -729,18 +729,98 @@ terminate: ; preds = %entry
unreachable
}
+%class.MyClass = type { i32 }
+
+; This crashed on debug mode (= when NDEBUG is not defined) when the logic for
+; computing the innermost region was not correct, in which a loop region
+; contains an exception region. This should pass CFGSort without crashing.
+define void @test12() personality i8* bitcast (i32 (...)* @__gxx_wasm_personality_v0 to i8*) {
+entry:
+ %e = alloca %class.MyClass, align 4
+ br label %for.cond
+
+for.cond: ; preds = %for.inc, %entry
+ %i.0 = phi i32 [ 0, %entry ], [ %inc, %for.inc ]
+ %cmp = icmp slt i32 %i.0, 9
+ br i1 %cmp, label %for.body, label %for.end
+
+for.body: ; preds = %for.cond
+ invoke void @quux(i32 %i.0)
+ to label %for.inc unwind label %catch.dispatch
+
+catch.dispatch: ; preds = %for.body
+ %0 = catchswitch within none [label %catch.start] unwind to caller
+
+catch.start: ; preds = %catch.dispatch
+ %1 = catchpad within %0 [i8* bitcast ({ i8*, i8* }* @_ZTI7MyClass to i8*)]
+ %2 = call i8* @llvm.wasm.get.exception(token %1)
+ %3 = call i32 @llvm.wasm.get.ehselector(token %1)
+ %4 = call i32 @llvm.eh.typeid.for(i8* bitcast ({ i8*, i8* }* @_ZTI7MyClass to i8*)) #3
+ %matches = icmp eq i32 %3, %4
+ br i1 %matches, label %catch, label %rethrow
+
+catch: ; preds = %catch.start
+ %5 = call i8* @__cxa_get_exception_ptr(i8* %2) #3 [ "funclet"(token %1) ]
+ %6 = bitcast i8* %5 to %class.MyClass*
+ %call = call %class.MyClass* @_ZN7MyClassC2ERKS_(%class.MyClass* %e, %class.MyClass* dereferenceable(4) %6) [ "funclet"(token %1) ]
+ %7 = call i8* @__cxa_begin_catch(i8* %2) #3 [ "funclet"(token %1) ]
+ %x = getelementptr inbounds %class.MyClass, %class.MyClass* %e, i32 0, i32 0
+ %8 = load i32, i32* %x, align 4
+ invoke void @quux(i32 %8) [ "funclet"(token %1) ]
+ to label %invoke.cont2 unwind label %ehcleanup
+
+invoke.cont2: ; preds = %catch
+ %call3 = call %class.MyClass* @_ZN7MyClassD2Ev(%class.MyClass* %e) #3 [ "funclet"(token %1) ]
+ call void @__cxa_end_catch() [ "funclet"(token %1) ]
+ catchret from %1 to label %for.inc
+
+rethrow: ; preds = %catch.start
+ call void @llvm.wasm.rethrow.in.catch() #6 [ "funclet"(token %1) ]
+ unreachable
+
+for.inc: ; preds = %invoke.cont2, %for.body
+ %inc = add nsw i32 %i.0, 1
+ br label %for.cond
+
+ehcleanup: ; preds = %catch
+ %9 = cleanuppad within %1 []
+ %call4 = call %class.MyClass* @_ZN7MyClassD2Ev(%class.MyClass* %e) #3 [ "funclet"(token %9) ]
+ invoke void @__cxa_end_catch() [ "funclet"(token %9) ]
+ to label %invoke.cont6 unwind label %terminate7
+
+invoke.cont6: ; preds = %ehcleanup
+ cleanupret from %9 unwind to caller
+
+for.end: ; preds = %for.cond
+ ret void
+
+terminate7: ; preds = %ehcleanup
+ %10 = cleanuppad within %9 []
+ %11 = call i8* @llvm.wasm.get.exception(token %10)
+ call void @__clang_call_terminate(i8* %11) #7 [ "funclet"(token %10) ]
+ unreachable
+}
+
; Check if the unwind destination mismatch stats are correct
-; NOSORT-STAT: 11 wasm-cfg-stackify - Number of EH pad unwind mismatches found
+; NOSORT-STAT: 14 wasm-cfg-stackify - Number of EH pad unwind mismatches found
declare void @foo()
declare void @bar()
declare i32 @baz()
+declare void @quux(i32)
declare void @fun(i32)
; Function Attrs: nounwind
declare void @nothrow(i32) #0
declare i32 @nothrow_i32() #0
+
; Function Attrs: nounwind
declare %class.Object* @_ZN6ObjectD2Ev(%class.Object* returned) #0
+ at _ZTI7MyClass = external constant { i8*, i8* }, align 4
+; Function Attrs: nounwind
+declare %class.MyClass* @_ZN7MyClassD2Ev(%class.MyClass* returned) #0
+; Function Attrs: nounwind
+declare %class.MyClass* @_ZN7MyClassC2ERKS_(%class.MyClass* returned, %class.MyClass* dereferenceable(4)) #0
+
declare i32 @__gxx_wasm_personality_v0(...)
declare i8* @llvm.wasm.get.exception(token)
declare i32 @llvm.wasm.get.ehselector(token)
@@ -748,6 +828,7 @@ declare void @llvm.wasm.rethrow.in.catch()
declare i32 @llvm.eh.typeid.for(i8*)
declare i8* @__cxa_begin_catch(i8*)
declare void @__cxa_end_catch()
+declare i8* @__cxa_get_exception_ptr(i8*)
declare void @__clang_call_terminate(i8*)
declare void @_ZSt9terminatev()
; Function Attrs: nounwind
More information about the llvm-commits
mailing list