Skip to content

Fix simplify-locals when adding a value to a br_if #775

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
kripken merged 2 commits into from
Oct 13, 2016
Merged

Fix simplify-locals when adding a value to a br_if #775

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
98e7ff0
pass/wast fuzzer improvements
kripken Oct 13, 2016
7077f78
when simplify-locals give br_if a value, the br_if also returns it, s…
kripken Oct 13, 2016
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
8 changes: 5 additions & 3 deletions scripts/fuzz_passes_wast.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,11 @@

def run():
try:
print 'run', ['bin/wasm-opt', wast]
subprocess.check_call(['bin/wasm-opt', wast])
cmd = ['bin/wasm-opt', wast]
print 'run', cmd
subprocess.check_call(cmd, stderr=open('/dev/null'))
except Exception, e:
print ">>> !!! ", e, " !!!"
return ">>> !!! ", e, " !!!"
return 'ok'

original_wast = None
Expand Down Expand Up @@ -105,6 +106,7 @@ def simplify(passes):
tested = set()

def pick_passes():
# return '--waka'.split(' ')
ret = []
while 1:
str_ret = str(ret)
Expand Down
12 changes: 8 additions & 4 deletions src/passes/SimplifyLocals.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ struct SimplifyLocals : public WalkerPass<LinearExecutionWalker<SimplifyLocals,
// sinkables. For the final exit from a block (falling off)
// exitter is null.
struct BlockBreak {
Break* br;
Expression** brp;
Sinkables sinkables;
};

Expand Down Expand Up @@ -128,7 +128,7 @@ struct SimplifyLocals : public WalkerPass<LinearExecutionWalker<SimplifyLocals,
// value means the block already has a return value
self->unoptimizableBlocks.insert(br->name);
} else {
self->blockBreaks[br->name].push_back({ br, std::move(self->sinkables) });
self->blockBreaks[br->name].push_back({ currp, std::move(self->sinkables) });
}
} else if (curr->is<Block>()) {
return; // handled in visitBlock
Expand Down Expand Up @@ -290,7 +290,7 @@ struct SimplifyLocals : public WalkerPass<LinearExecutionWalker<SimplifyLocals,
auto breaks = std::move(blockBreaks[block->name]);
blockBreaks.erase(block->name);
if (breaks.size() == 0) return; // block has no branches TODO we might optimize trivial stuff here too
assert(!breaks[0].br->value); // block does not already have a return value (if one break has one, they all do)
assert(!(*breaks[0].brp)->cast<Break>()->value); // block does not already have a return value (if one break has one, they all do)
// look for a set_local that is present in them all
bool found = false;
Index sharedIndex = -1;
Expand Down Expand Up @@ -328,14 +328,18 @@ struct SimplifyLocals : public WalkerPass<LinearExecutionWalker<SimplifyLocals,
for (size_t j = 0; j < breaks.size(); j++) {
// move break set_local's value to the break
auto* breakSetLocalPointer = breaks[j].sinkables.at(sharedIndex).item;
auto* br = breaks[j].br;
auto* brp = breaks[j].brp;
auto* br = (*brp)->cast<Break>();
assert(!br->value);
// if the break is conditional, then we must set the value here - if the break is not taken, we must still have the new value in the local
auto* set = (*breakSetLocalPointer)->cast<SetLocal>();
if (br->condition) {
br->value = set;
set->setTee(true);
*breakSetLocalPointer = getModule()->allocator.alloc<Nop>();
// in addition, as this is a conditional br that now has a value, it now returns a value, so it must be dropped
br->finalize();
*brp = Builder(*getModule()).makeDrop(br);
} else {
br->value = set->value;
ExpressionManipulator::nop(set);
Expand Down
94 changes: 67 additions & 27 deletions test/passes/simplify-locals.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -578,21 +578,25 @@
(set_local $x
(block $out i32
(nop)
(br_if $out
(tee_local $x
(block $waka i32
(nop)
(br_if $waka
(tee_local $x
(i32.const 12)
(drop
(br_if $out
(tee_local $x
(block $waka i32
(nop)
(drop
(br_if $waka
(tee_local $x
(i32.const 12)
)
(i32.const 1)
)
)
(i32.const 1)
(nop)
(i32.const 34)
)
(nop)
(i32.const 34)
)
(i32.const 1)
)
(i32.const 1)
)
(drop
(get_local $x)
Expand All @@ -613,21 +617,23 @@
)
(nop)
)
(br_if $out
(tee_local $x
(if i32
(i32.const 1)
(block $block3 i32
(nop)
(i32.const 14)
)
(block $block5 i32
(nop)
(i32.const 25)
(drop
(br_if $out
(tee_local $x
(if i32
(i32.const 1)
(block $block3 i32
(nop)
(i32.const 14)
)
(block $block5 i32
(nop)
(i32.const 25)
)
)
)
(i32.const 1)
)
(i32.const 1)
)
(block $sink-out-of-me-i-have-but-one-exit
(nop)
Expand Down Expand Up @@ -720,11 +726,13 @@
(get_local $a)
)
(nop)
(br_if $while-out$0
(tee_local $a
(i32.const 4)
(drop
(br_if $while-out$0
(tee_local $a
(i32.const 4)
)
(get_local $e)
)
(get_local $e)
)
(nop)
(i32.add
Expand Down Expand Up @@ -764,4 +772,36 @@
(i32.const 0)
)
)
(func $drop-br_if (type $9) (param $label i32) (param $$cond2 i32) (param $$$0151 i32) (result i32)
(nop)
(tee_local $label
(block $label$break$L4 i32
(if
(i32.eq
(get_local $label)
(i32.const 15)
)
(block $block
(nop)
(nop)
(drop
(br_if $label$break$L4
(tee_local $label
(i32.const 0)
)
(i32.eqz
(i32.eq
(get_local $$$0151)
(i32.const 0)
)
)
)
)
)
)
(nop)
(i32.const 1)
)
)
)
)
30 changes: 30 additions & 0 deletions test/passes/simplify-locals.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -801,4 +801,34 @@
(i32.const 0)
)
)
(func $drop-br_if (param $label i32) (param $$cond2 i32) (param $$$0151 i32) (result i32)
(block $label$break$L4
(if
(i32.eq
(get_local $label)
(i32.const 15)
)
(block $block
(set_local $label
(i32.const 0)
)
(set_local $$cond2
(i32.eq
(get_local $$$0151)
(i32.const 0)
)
)
(br_if $label$break$L4 ;; when we add a value to this, its type changes as it returns the value too, so must be dropped
(i32.eqz
(get_local $$cond2)
)
)
)
)
(set_local $label
(i32.const 1)
)
)
(get_local $label)
)
)