Skip to content

PostAssemblyScript: Handle balanced retains involving allocations #2833

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed

PostAssemblyScript: Handle balanced retains involving allocations #2833

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
11127d8
PostAssemblyScript: Handle balanced retains involving allocations
dcodeIO May 6, 2020
4e1bb5a
known to be a Call
dcodeIO May 6, 2020
ac4bd00
refactor
dcodeIO May 7, 2020
0af4dcd
Merge remote-tracking branch 'webassembly/master' into postas-balance…
dcodeIO May 9, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 41 additions & 41 deletions src/passes/PostAssemblyScript.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,8 +364,16 @@ struct OptimizeARC : public WalkerPass<PostWalker<OptimizeARC>> {
return (*releaseLocation)->cast<Call>()->operands[0]->cast<LocalGet>();
}

// Tests if the given retain is generally eliminable (does not retain an
// allocation and does not reach an escape)
bool testRetainEliminable(LocalSet* retain, AliasGraph& graph) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can the AliasGraph here be const AliasGraph&?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gave this a try, including updating the called functions herein, but the compiler then barks at me on [] accesses (no such operator) like graph.setInfluences[...] and graph.getSetses[...] in the called functions, as if std::map etc. can't be used this way. Might be missing something, as usual :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the issue is that map[key] can have side effects, it doesn't just do a read, but it also creates the entry (with a default value) if it isn't there.

You can do map.at(key) if you know the key must be there. that will not change the map, and it will also check it actually exists, so it's better (but again, only if you know the key must be there).

return !testRetainsAllocation(getRetainedExpression(retain), graph) &&
!testReachesEscape(retain, graph);
}

// Tests if a release has balanced retains, that is it is being retained in
// any path leading to the release. For example
// any path leading to the release and that each path is generally eliminable.
// For example
//
// var c = somethingElse() || a;
// ...
Expand Down Expand Up @@ -395,7 +403,8 @@ struct OptimizeARC : public WalkerPass<PostWalker<OptimizeARC>> {
if (localSet == nullptr) {
return cache[release] = false;
}
if (retains.find(localSet) == retains.end()) {
auto it = retains.find(localSet);
if (it == retains.end()) {
if (auto* localGet = localSet->value->dynCast<LocalGet>()) {
if (seen.find(localGet) == seen.end()) {
seen.insert(localGet);
Expand All @@ -408,6 +417,8 @@ struct OptimizeARC : public WalkerPass<PostWalker<OptimizeARC>> {
} else {
return cache[release] = false;
}
} else if (!testRetainEliminable(it->first, graph)) {
return cache[release] = false;
}
}
return cache[release] = true;
Expand Down Expand Up @@ -441,71 +452,60 @@ struct OptimizeARC : public WalkerPass<PostWalker<OptimizeARC>> {
Set<Expression**> redundantReleases;
Map<LocalGet*, bool> balancedRetainsCache;

// For each retain, check that it
//
// * doesn't reach an escape
// * doesn't retain an allocation
// For each retain, check that it is
// * generally eliminable (does not retain an allocation / reach an escape)
// * reaches at least one release
// * reaches only releases with balanced retains
//
// * reaches only releases with balanced (generally eliminable) retains
for (auto& pair : retains) {
auto* retain = pair.first;
auto** retainLocation = pair.second;
if (!testReachesEscape(retain, graph)) {
if (!testRetainsAllocation(getRetainedExpression(retain), graph)) {
Set<Expression**> releaseLocations;
collectReleases(retain, graph, releaseLocations);
if (!releaseLocations.empty()) {
bool allBalanced = true;
for (auto** releaseLocation : releaseLocations) {
if (!testBalancedRetains(getReleaseByLocation(releaseLocation),
graph,
balancedRetainsCache)) {
allBalanced = false;
break;
}
if (testRetainEliminable(retain, graph)) {
Set<Expression**> releaseLocations;
collectReleases(retain, graph, releaseLocations);
if (!releaseLocations.empty()) {
bool allBalanced = true;
for (auto** releaseLocation : releaseLocations) {
if (!testBalancedRetains(getReleaseByLocation(releaseLocation),
graph,
balancedRetainsCache)) {
allBalanced = false;
break;
}
if (allBalanced) {
#ifdef POST_ASSEMBLYSCRIPT_DEBUG
std::cerr << " eliminating ";
WasmPrinter::printExpression(retain, std::cerr, true);
std::cerr << " reaching\n";
#endif
redundantRetains.insert(retainLocation);
for (auto** getLocation : releaseLocations) {
}
if (allBalanced) {
#ifdef POST_ASSEMBLYSCRIPT_DEBUG
std::cerr << " ";
WasmPrinter::printExpression(*getLocation, std::cerr, true);
std::cerr << "\n";
std::cerr << " eliminating ";
WasmPrinter::printExpression(retain, std::cerr, true);
std::cerr << " reaching\n";
#endif
redundantReleases.insert(getLocation);
}
redundantRetains.insert(retainLocation);
for (auto** getLocation : releaseLocations) {
#ifdef POST_ASSEMBLYSCRIPT_DEBUG
} else {
std::cerr << " cannot eliminate ";
WasmPrinter::printExpression(retain, std::cerr, true);
std::cerr << " - unbalanced\n";
std::cerr << " ";
WasmPrinter::printExpression(*getLocation, std::cerr, true);
std::cerr << "\n";
#endif
redundantReleases.insert(getLocation);
}
#ifdef POST_ASSEMBLYSCRIPT_DEBUG
} else {
std::cerr << " cannot eliminate ";
WasmPrinter::printExpression(retain, std::cerr, true);
std::cerr << " - zero releases\n";
std::cerr << " - unbalanced\n";
#endif
}
#ifdef POST_ASSEMBLYSCRIPT_DEBUG
} else {
std::cerr << " cannot eliminate ";
WasmPrinter::printExpression(retain, std::cerr, true);
std::cerr << " - retains allocation\n";
std::cerr << " - zero releases\n";
#endif
}
#ifdef POST_ASSEMBLYSCRIPT_DEBUG
} else {
std::cerr << " cannot eliminate ";
WasmPrinter::printExpression(retain, std::cerr, true);
std::cerr << " - reaches return\n";
std::cerr << " - not eliminable\n";
#endif
}
}
Expand Down
39 changes: 38 additions & 1 deletion test/passes/post-assemblyscript.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@
(type $i32_=>_none (func (param i32)))
(type $i32_i32_i32_=>_none (func (param i32 i32 i32)))
(type $i32_=>_i32 (func (param i32) (result i32)))
(type $none_=>_i32 (func (result i32)))
(type $i32_i32_=>_i32 (func (param i32 i32) (result i32)))
(type $none_=>_i32 (func (result i32)))
(import "rt" "retain" (func $~lib/rt/pure/__retain (param i32) (result i32)))
(import "rt" "release" (func $~lib/rt/pure/__release (param i32)))
(import "rt" "alloc" (func $~lib/rt/tlsf/__alloc (param i32 i32) (result i32)))
(import "rc" "getRetainedRef" (func $getRetainedRef (result i32)))
(func $eliminates.linearArgument (param $0 i32)
(local.set $0
Expand Down Expand Up @@ -260,4 +261,40 @@
)
)
)
(func $keeps.balancedRetainsWithAllocation (param $0 i32) (param $cond1 i32) (param $cond2 i32)
(local $3 i32)
(local $flat i32)
(if
(local.get $cond1)
(if
(local.get $cond2)
(local.set $3
(call $~lib/rt/pure/__retain
(local.get $0)
)
)
(block $block
(local.set $flat
(call $~lib/rt/tlsf/__alloc
(i32.const 16)
(i32.const 0)
)
)
(local.set $3
(call $~lib/rt/pure/__retain
(local.get $flat)
)
)
)
)
(local.set $3
(call $~lib/rt/pure/__retain
(local.get $0)
)
)
)
(call $~lib/rt/pure/__release
(local.get $3)
)
)
)
37 changes: 37 additions & 0 deletions test/passes/post-assemblyscript.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
(module
(import "rt" "retain" (func $~lib/rt/pure/__retain (param i32) (result i32)))
(import "rt" "release" (func $~lib/rt/pure/__release (param i32)))
(import "rt" "alloc" (func $~lib/rt/tlsf/__alloc (param i32 i32) (result i32)))
(func $eliminates.linearArgument (param $0 i32)
(local.set $0
(call $~lib/rt/pure/__retain
Expand Down Expand Up @@ -339,4 +340,40 @@
)
)
)
(func $keeps.balancedRetainsWithAllocation (param $0 i32) (param $cond1 i32) (param $cond2 i32)
(local $3 i32)
(local $flat i32)
(if
(local.get $cond1)
(if
(local.get $cond2)
(local.set $3
(call $~lib/rt/pure/__retain
(local.get $0)
)
)
(block
(local.set $flat
(call $~lib/rt/tlsf/__alloc
(i32.const 16)
(i32.const 0)
)
)
(local.set $3
(call $~lib/rt/pure/__retain
(local.get $flat)
)
)
)
)
(local.set $3
(call $~lib/rt/pure/__retain
(local.get $0)
)
)
)
(call $~lib/rt/pure/__release
(local.get $3)
)
)
)