Skip to content

Commit 46bd45f

Browse files
kripkenkripken
authored
Merge pull request WebAssembly#775 from WebAssembly/sl-fix
Fix simplify-locals when adding a value to a br_if
2 parents edd94ad + 7077f78 commit 46bd45f

4 files changed

Lines changed: 110 additions & 34 deletions

File tree

‎scripts/fuzz_passes_wast.py‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,11 @@
5353

5454
def run():
5555
try:
56-
print 'run', ['bin/wasm-opt', wast]
57-
subprocess.check_call(['bin/wasm-opt', wast])
56+
cmd = ['bin/wasm-opt', wast]
57+
print 'run', cmd
58+
subprocess.check_call(cmd, stderr=open('/dev/null'))
5859
except Exception, e:
59-
print ">>> !!! ", e, " !!!"
60+
return ">>> !!! ", e, " !!!"
6061
return 'ok'
6162

6263
original_wast = None
@@ -105,6 +106,7 @@ def simplify(passes):
105106
tested = set()
106107

107108
def pick_passes():
109+
# return '--waka'.split(' ')
108110
ret = []
109111
while 1:
110112
str_ret = str(ret)

‎src/passes/SimplifyLocals.cpp‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ struct SimplifyLocals : public WalkerPass<LinearExecutionWalker<SimplifyLocals,
9393
// sinkables. For the final exit from a block (falling off)
9494
// exitter is null.
9595
struct BlockBreak {
96-
Break* br;
96+
Expression** brp;
9797
Sinkables sinkables;
9898
};
9999

@@ -128,7 +128,7 @@ struct SimplifyLocals : public WalkerPass<LinearExecutionWalker<SimplifyLocals,
128128
// value means the block already has a return value
129129
self->unoptimizableBlocks.insert(br->name);
130130
} else {
131-
self->blockBreaks[br->name].push_back({ br, std::move(self->sinkables) });
131+
self->blockBreaks[br->name].push_back({ currp, std::move(self->sinkables) });
132132
}
133133
} else if (curr->is<Block>()) {
134134
return; // handled in visitBlock
@@ -290,7 +290,7 @@ struct SimplifyLocals : public WalkerPass<LinearExecutionWalker<SimplifyLocals,
290290
auto breaks = std::move(blockBreaks[block->name]);
291291
blockBreaks.erase(block->name);
292292
if (breaks.size() == 0) return; // block has no branches TODO we might optimize trivial stuff here too
293-
assert(!breaks[0].br->value); // block does not already have a return value (if one break has one, they all do)
293+
assert(!(*breaks[0].brp)->cast<Break>()->value); // block does not already have a return value (if one break has one, they all do)
294294
// look for a set_local that is present in them all
295295
bool found = false;
296296
Index sharedIndex = -1;
@@ -328,14 +328,18 @@ struct SimplifyLocals : public WalkerPass<LinearExecutionWalker<SimplifyLocals,
328328
for (size_t j = 0; j < breaks.size(); j++) {
329329
// move break set_local's value to the break
330330
auto* breakSetLocalPointer = breaks[j].sinkables.at(sharedIndex).item;
331-
auto* br = breaks[j].br;
331+
auto* brp = breaks[j].brp;
332+
auto* br = (*brp)->cast<Break>();
332333
assert(!br->value);
333334
// 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
334335
auto* set = (*breakSetLocalPointer)->cast<SetLocal>();
335336
if (br->condition) {
336337
br->value = set;
337338
set->setTee(true);
338339
*breakSetLocalPointer = getModule()->allocator.alloc<Nop>();
340+
// in addition, as this is a conditional br that now has a value, it now returns a value, so it must be dropped
341+
br->finalize();
342+
*brp = Builder(*getModule()).makeDrop(br);
339343
} else {
340344
br->value = set->value;
341345
ExpressionManipulator::nop(set);

‎test/passes/simplify-locals.txt‎

Lines changed: 67 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -578,21 +578,25 @@
578578
(set_local $x
579579
(block $out i32
580580
(nop)
581-
(br_if $out
582-
(tee_local $x
583-
(block $waka i32
584-
(nop)
585-
(br_if $waka
586-
(tee_local $x
587-
(i32.const 12)
581+
(drop
582+
(br_if $out
583+
(tee_local $x
584+
(block $waka i32
585+
(nop)
586+
(drop
587+
(br_if $waka
588+
(tee_local $x
589+
(i32.const 12)
590+
)
591+
(i32.const 1)
592+
)
588593
)
589-
(i32.const 1)
594+
(nop)
595+
(i32.const 34)
590596
)
591-
(nop)
592-
(i32.const 34)
593597
)
598+
(i32.const 1)
594599
)
595-
(i32.const 1)
596600
)
597601
(drop
598602
(get_local $x)
@@ -613,21 +617,23 @@
613617
)
614618
(nop)
615619
)
616-
(br_if $out
617-
(tee_local $x
618-
(if i32
619-
(i32.const 1)
620-
(block $block3 i32
621-
(nop)
622-
(i32.const 14)
623-
)
624-
(block $block5 i32
625-
(nop)
626-
(i32.const 25)
620+
(drop
621+
(br_if $out
622+
(tee_local $x
623+
(if i32
624+
(i32.const 1)
625+
(block $block3 i32
626+
(nop)
627+
(i32.const 14)
628+
)
629+
(block $block5 i32
630+
(nop)
631+
(i32.const 25)
632+
)
627633
)
628634
)
635+
(i32.const 1)
629636
)
630-
(i32.const 1)
631637
)
632638
(block $sink-out-of-me-i-have-but-one-exit
633639
(nop)
@@ -720,11 +726,13 @@
720726
(get_local $a)
721727
)
722728
(nop)
723-
(br_if $while-out$0
724-
(tee_local $a
725-
(i32.const 4)
729+
(drop
730+
(br_if $while-out$0
731+
(tee_local $a
732+
(i32.const 4)
733+
)
734+
(get_local $e)
726735
)
727-
(get_local $e)
728736
)
729737
(nop)
730738
(i32.add
@@ -764,4 +772,36 @@
764772
(i32.const 0)
765773
)
766774
)
775+
(func $drop-br_if (type $9) (param $label i32) (param $$cond2 i32) (param $$$0151 i32) (result i32)
776+
(nop)
777+
(tee_local $label
778+
(block $label$break$L4 i32
779+
(if
780+
(i32.eq
781+
(get_local $label)
782+
(i32.const 15)
783+
)
784+
(block $block
785+
(nop)
786+
(nop)
787+
(drop
788+
(br_if $label$break$L4
789+
(tee_local $label
790+
(i32.const 0)
791+
)
792+
(i32.eqz
793+
(i32.eq
794+
(get_local $$$0151)
795+
(i32.const 0)
796+
)
797+
)
798+
)
799+
)
800+
)
801+
)
802+
(nop)
803+
(i32.const 1)
804+
)
805+
)
806+
)
767807
)

‎test/passes/simplify-locals.wast‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -801,4 +801,34 @@
801801
(i32.const 0)
802802
)
803803
)
804+
(func $drop-br_if (param $label i32) (param $$cond2 i32) (param $$$0151 i32) (result i32)
805+
(block $label$break$L4
806+
(if
807+
(i32.eq
808+
(get_local $label)
809+
(i32.const 15)
810+
)
811+
(block $block
812+
(set_local $label
813+
(i32.const 0)
814+
)
815+
(set_local $$cond2
816+
(i32.eq
817+
(get_local $$$0151)
818+
(i32.const 0)
819+
)
820+
)
821+
(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
822+
(i32.eqz
823+
(get_local $$cond2)
824+
)
825+
)
826+
)
827+
)
828+
(set_local $label
829+
(i32.const 1)
830+
)
831+
)
832+
(get_local $label)
833+
)
804834
)

0 commit comments

Comments
 (0)