Skip to content

Refactor removing module elements #2489

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

Merged
aheejin merged 2 commits into from
Dec 3, 2019
Merged

Refactor removing module elements #2489

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d14e822
Refactor removing module elements
aheejin Dec 2, 2019
176befe
Fix the sanitizer error
aheejin Dec 2, 2019
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
4 changes: 2 additions & 2 deletions auto_update_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ def update_example_tests():
'-I' + os.path.join(shared.options.binaryen_root, 'src'), '-g', '-L' + libdir, '-pthread']
print('build: ', ' '.join(extra))
if src.endswith('.cpp'):
extra += ['-std=c++11']
extra += ['-std=c++14']
print(os.getcwd())
subprocess.check_call(extra)
# Link against the binaryen C library DSO, using rpath
Expand All @@ -206,7 +206,7 @@ def update_example_tests():
if os.environ.get('COMPILER_FLAGS'):
for f in os.environ.get('COMPILER_FLAGS').split(' '):
cmd.append(f)
cmd = [os.environ.get('CXX') or 'g++', '-std=c++11'] + cmd
cmd = [os.environ.get('CXX') or 'g++', '-std=c++14'] + cmd
try:
print('link: ', ' '.join(cmd))
subprocess.check_call(cmd)
Expand Down
4 changes: 2 additions & 2 deletions check.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ def run_gcc_tests():
extra = [shared.NATIVECC, src, '-c', '-o', 'example.o',
'-I' + os.path.join(shared.options.binaryen_root, 'src'), '-g', '-L' + libpath, '-pthread']
if src.endswith('.cpp'):
extra += ['-std=c++11']
extra += ['-std=c++14']
if os.environ.get('COMPILER_FLAGS'):
for f in os.environ.get('COMPILER_FLAGS').split(' '):
extra.append(f)
Expand All @@ -402,7 +402,7 @@ def run_gcc_tests():
if os.environ.get('COMPILER_FLAGS'):
for f in os.environ.get('COMPILER_FLAGS').split(' '):
cmd.append(f)
cmd = [shared.NATIVEXX, '-std=c++11'] + cmd
cmd = [shared.NATIVEXX, '-std=c++14'] + cmd
print('link: ', ' '.join(cmd))
subprocess.check_call(cmd)
print('run...', output_file)
Expand Down
10 changes: 2 additions & 8 deletions src/passes/DuplicateFunctionElimination.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,14 +89,8 @@ struct DuplicateFunctionElimination : public Pass {
// perform replacements
if (replacements.size() > 0) {
// remove the duplicates
auto& v = module->functions;
v.erase(std::remove_if(v.begin(),
v.end(),
[&](const std::unique_ptr<Function>& curr) {
return duplicates.count(curr->name) > 0;
}),
v.end());
module->updateMaps();
module->removeFunctions(
[&](Function* func) { return duplicates.count(func->name) > 0; });
OptUtils::replaceFunctions(runner, *module, replacements);
} else {
break;
Expand Down
23 changes: 6 additions & 17 deletions src/passes/Inlining.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,23 +385,12 @@ struct Inlining : public Pass {
OptUtils::optimizeAfterInlining(inlinedInto, module, runner);
}
// remove functions that we no longer need after inlining
auto& funcs = module->functions;
funcs.erase(std::remove_if(funcs.begin(),
funcs.end(),
[&](const std::unique_ptr<Function>& curr) {
auto name = curr->name;
auto& info = infos[name];
bool canRemove =
inlinedUses.count(name) &&
inlinedUses[name] == info.calls &&
!info.usedGlobally;
#ifdef INLINING_DEBUG
if (canRemove)
std::cout << "removing " << name << '\n';
#endif
return canRemove;
}),
funcs.end());
module->removeFunctions([&](Function* func) {
auto name = func->name;
auto& info = infos[name];
return inlinedUses.count(name) && inlinedUses[name] == info.calls &&
!info.usedGlobally;
});
// return whether we did any work
return inlinedUses.size() > 0;
}
Expand Down
56 changes: 14 additions & 42 deletions src/passes/RemoveUnusedModuleElements.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,40 +211,18 @@ struct RemoveUnusedModuleElements : public Pass {
// Compute reachability starting from the root set.
ReachabilityAnalyzer analyzer(module, roots);
// Remove unreachable elements.
{
auto& v = module->functions;
v.erase(std::remove_if(v.begin(),
v.end(),
[&](const std::unique_ptr<Function>& curr) {
return analyzer.reachable.count(ModuleElement(
ModuleElementKind::Function,
curr->name)) == 0;
}),
v.end());
}
{
auto& v = module->globals;
v.erase(std::remove_if(v.begin(),
v.end(),
[&](const std::unique_ptr<Global>& curr) {
return analyzer.reachable.count(
ModuleElement(ModuleElementKind::Global,
curr->name)) == 0;
}),
v.end());
}
{
auto& v = module->events;
v.erase(std::remove_if(v.begin(),
v.end(),
[&](const std::unique_ptr<Event>& curr) {
return analyzer.reachable.count(
ModuleElement(ModuleElementKind::Event,
curr->name)) == 0;
}),
v.end());
}
module->updateMaps();
module->removeFunctions([&](Function* curr) {
return analyzer.reachable.count(
ModuleElement(ModuleElementKind::Function, curr->name)) == 0;
});
module->removeGlobals([&](Global* curr) {
return analyzer.reachable.count(
ModuleElement(ModuleElementKind::Global, curr->name)) == 0;
});
module->removeEvents([&](Event* curr) {
return analyzer.reachable.count(
ModuleElement(ModuleElementKind::Event, curr->name)) == 0;
});
// Handle the memory and table
if (!exportsMemory && !analyzer.usesMemory) {
if (!importsMemory) {
Expand Down Expand Up @@ -302,14 +280,8 @@ struct RemoveUnusedModuleElements : public Pass {
call->fullType = canonicalize(call->fullType);
}
// remove no-longer used types
module->functionTypes.erase(
std::remove_if(module->functionTypes.begin(),
module->functionTypes.end(),
[&needed](std::unique_ptr<FunctionType>& type) {
return needed.count(type.get()) == 0;
}),
module->functionTypes.end());
module->updateMaps();
module->removeFunctionTypes(
[&](FunctionType* type) { return needed.count(type) == 0; });
}
};

Expand Down
6 changes: 6 additions & 0 deletions src/wasm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1396,6 +1396,12 @@ class Module {
void removeGlobal(Name name);
void removeEvent(Name name);

void removeFunctionTypes(std::function<bool(FunctionType*)> pred);
void removeExports(std::function<bool(Export*)> pred);
void removeFunctions(std::function<bool(Function*)> pred);
void removeGlobals(std::function<bool(Global*)> pred);
void removeEvents(std::function<bool(Event*)> pred);

void updateMaps();

void clearDebugInfo();
Expand Down
77 changes: 42 additions & 35 deletions src/wasm/wasm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1155,57 +1155,64 @@ Event* Module::addEvent(Event* curr) {

void Module::addStart(const Name& s) { start = s; }

void Module::removeFunctionType(Name name) {
for (size_t i = 0; i < functionTypes.size(); i++) {
if (functionTypes[i]->name == name) {
functionTypes.erase(functionTypes.begin() + i);
template<typename Vector, typename Map>
void removeModuleElement(Vector& v, Map& m, Name name) {
Copy link
Copy Markdown
Member

@tlively tlively Dec 2, 2019

Choose a reason for hiding this comment

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

Would it make sense to implement this in terms of removeModuleElements?

Copy link
Copy Markdown
Member

@kripken kripken Dec 2, 2019

Choose a reason for hiding this comment

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

That might be slower, I worry...

Copy link
Copy Markdown
Member Author

@aheejin aheejin Dec 2, 2019

Choose a reason for hiding this comment

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

For the vector thing the effect would be more or less the same, for the deletion from the map, this can be done in constant time whereas we iterate through all elements in the map in removeModuleElements. Given that a lot of usage in the codebase is removing a single element, I guess it'd be better to keep this in the way it is?

m.erase(name);
for (size_t i = 0; i < v.size(); i++) {
if (v[i]->name == name) {
v.erase(v.begin() + i);
break;
}
}
functionTypesMap.erase(name);
}

void Module::removeFunctionType(Name name) {
removeModuleElement(functionTypes, functionTypesMap, name);
}
void Module::removeExport(Name name) {
for (size_t i = 0; i < exports.size(); i++) {
if (exports[i]->name == name) {
exports.erase(exports.begin() + i);
break;
}
}
exportsMap.erase(name);
removeModuleElement(exports, exportsMap, name);
}

void Module::removeFunction(Name name) {
for (size_t i = 0; i < functions.size(); i++) {
if (functions[i]->name == name) {
functions.erase(functions.begin() + i);
break;
}
}
functionsMap.erase(name);
removeModuleElement(functions, functionsMap, name);
}

void Module::removeGlobal(Name name) {
for (size_t i = 0; i < globals.size(); i++) {
if (globals[i]->name == name) {
globals.erase(globals.begin() + i);
break;
}
}
globalsMap.erase(name);
removeModuleElement(globals, globalsMap, name);
}

void Module::removeEvent(Name name) {
for (size_t i = 0; i < events.size(); i++) {
if (events[i]->name == name) {
events.erase(events.begin() + i);
break;
removeModuleElement(events, eventsMap, name);
}

template<typename Vector, typename Map, typename Elem>
void removeModuleElements(Vector& v,
Map& m,
std::function<bool(Elem* elem)> pred) {
for (auto it = m.begin(); it != m.end();) {
if (pred(it->second)) {
it = m.erase(it);
} else {
it++;
}
}
eventsMap.erase(name);
v.erase(
std::remove_if(v.begin(), v.end(), [&](auto& e) { return pred(e.get()); }),
v.end());
}

// TODO: remove* for other elements
void Module::removeFunctionTypes(std::function<bool(FunctionType*)> pred) {
removeModuleElements(functionTypes, functionTypesMap, pred);
}
void Module::removeExports(std::function<bool(Export*)> pred) {
removeModuleElements(exports, exportsMap, pred);
}
void Module::removeFunctions(std::function<bool(Function*)> pred) {
removeModuleElements(functions, functionsMap, pred);
}
void Module::removeGlobals(std::function<bool(Global*)> pred) {
removeModuleElements(globals, globalsMap, pred);
}
void Module::removeEvents(std::function<bool(Event*)> pred) {
removeModuleElements(events, eventsMap, pred);
}

void Module::updateMaps() {
functionsMap.clear();
Expand Down