Skip to content

Commit 7bc2ed7

Browse files
kripkenkripken
committed
fix merge-blocks logic in call, call_indirect, select, we need to avoid any danger of moving something past a side effect ; also fix an asm2wasm bug with call_indirect fixups; the call target may be a block, which we need to look through
1 parent 908a74b commit 7bc2ed7

11 files changed

Lines changed: 307 additions & 60 deletions

‎src/asm2wasm.h‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1397,7 +1397,12 @@ void Asm2WasmBuilder::processAsm(Ref ast) {
13971397
void visitCallIndirect(CallIndirect* curr) {
13981398
// we already call into target = something + offset, where offset is a callImport with the name of the table. replace that with the table offset
13991399
// note that for an ftCall or mftCall, we have no asm.js mask, so have nothing to do here
1400-
auto* add = curr->target->dynCast<Binary>();
1400+
auto* target = curr->target;
1401+
// might be a block with a fallthrough
1402+
if (auto* block = target->dynCast<Block>()) {
1403+
target = block->list.back();
1404+
}
1405+
auto* add = target->dynCast<Binary>();
14011406
if (!add) return;
14021407
if (add->right->is<CallImport>()) {
14031408
auto* offset = add->right->cast<CallImport>();

‎src/passes/MergeBlocks.cpp‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -288,11 +288,14 @@ struct MergeBlocks : public WalkerPass<PostWalker<MergeBlocks>> {
288288
}
289289

290290
void visitSelect(Select* curr) {
291+
// TODO: for now, just stop when we see any side effect. instead, we could
292+
// check effects carefully for reordering
291293
Block* outer = nullptr;
292-
outer = optimize(curr, curr->ifTrue, outer);
293294
if (EffectAnalyzer(getPassOptions(), curr->ifTrue).hasSideEffects()) return;
294-
outer = optimize(curr, curr->ifFalse, outer);
295+
outer = optimize(curr, curr->ifTrue, outer);
295296
if (EffectAnalyzer(getPassOptions(), curr->ifFalse).hasSideEffects()) return;
297+
outer = optimize(curr, curr->ifFalse, outer);
298+
if (EffectAnalyzer(getPassOptions(), curr->condition).hasSideEffects()) return;
296299
optimize(curr, curr->condition, outer);
297300
}
298301

@@ -308,11 +311,13 @@ struct MergeBlocks : public WalkerPass<PostWalker<MergeBlocks>> {
308311
}
309312

310313
template<typename T>
311-
void handleCall(T* curr, Block* outer = nullptr) {
314+
void handleCall(T* curr) {
315+
Block* outer = nullptr;
312316
for (Index i = 0; i < curr->operands.size(); i++) {
313-
outer = optimize(curr, curr->operands[i], outer);
314317
if (EffectAnalyzer(getPassOptions(), curr->operands[i]).hasSideEffects()) return;
318+
outer = optimize(curr, curr->operands[i], outer);
315319
}
320+
return;
316321
}
317322

318323
void visitCall(Call* curr) {
@@ -324,9 +329,13 @@ struct MergeBlocks : public WalkerPass<PostWalker<MergeBlocks>> {
324329
}
325330

326331
void visitCallIndirect(CallIndirect* curr) {
327-
auto* outer = optimize(curr, curr->target);
332+
Block* outer = nullptr;
333+
for (Index i = 0; i < curr->operands.size(); i++) {
334+
if (EffectAnalyzer(getPassOptions(), curr->operands[i]).hasSideEffects()) return;
335+
outer = optimize(curr, curr->operands[i], outer);
336+
}
328337
if (EffectAnalyzer(getPassOptions(), curr->target).hasSideEffects()) return;
329-
handleCall(curr, outer);
338+
optimize(curr, curr->target, outer);
330339
}
331340
};
332341

‎test/passes/remove-unused-names_merge-blocks.txt‎

Lines changed: 121 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -412,26 +412,34 @@
412412
)
413413
)
414414
)
415-
(unreachable)
416-
(drop
417-
(i32.const 30)
418-
)
419-
(drop
420-
(i32.const 50)
421-
)
422415
(drop
423416
(select
424-
(i32.const 20)
425-
(i32.const 40)
426-
(i32.const 60)
417+
(block (result i32)
418+
(unreachable)
419+
(i32.const 20)
420+
)
421+
(block (result i32)
422+
(drop
423+
(i32.const 30)
424+
)
425+
(i32.const 40)
426+
)
427+
(block (result i32)
428+
(drop
429+
(i32.const 50)
430+
)
431+
(i32.const 60)
432+
)
427433
)
428434
)
429-
(drop
430-
(i32.const 10)
431-
)
432435
(drop
433436
(select
434-
(unreachable)
437+
(block (result i32)
438+
(drop
439+
(i32.const 10)
440+
)
441+
(unreachable)
442+
)
435443
(block (result i32)
436444
(drop
437445
(i32.const 30)
@@ -449,27 +457,33 @@
449457
(drop
450458
(i32.const 10)
451459
)
452-
(unreachable)
453-
(drop
454-
(i32.const 50)
455-
)
456460
(drop
457461
(select
458462
(i32.const 20)
459-
(i32.const 40)
460-
(i32.const 60)
463+
(block
464+
(unreachable)
465+
(i32.const 40)
466+
)
467+
(block (result i32)
468+
(drop
469+
(i32.const 50)
470+
)
471+
(i32.const 60)
472+
)
461473
)
462474
)
463475
(drop
464476
(i32.const 10)
465477
)
466-
(drop
467-
(i32.const 30)
468-
)
469478
(drop
470479
(select
471480
(i32.const 20)
472-
(unreachable)
481+
(block
482+
(drop
483+
(i32.const 30)
484+
)
485+
(unreachable)
486+
)
473487
(block (result i32)
474488
(drop
475489
(i32.const 50)
@@ -484,12 +498,14 @@
484498
(drop
485499
(i32.const 30)
486500
)
487-
(unreachable)
488501
(drop
489502
(select
490503
(i32.const 20)
491504
(i32.const 40)
492-
(i32.const 60)
505+
(block
506+
(unreachable)
507+
(i32.const 60)
508+
)
493509
)
494510
)
495511
(drop
@@ -498,14 +514,16 @@
498514
(drop
499515
(i32.const 30)
500516
)
501-
(drop
502-
(i32.const 50)
503-
)
504517
(drop
505518
(select
506519
(i32.const 20)
507520
(i32.const 40)
508-
(unreachable)
521+
(block
522+
(drop
523+
(i32.const 50)
524+
)
525+
(unreachable)
526+
)
509527
)
510528
)
511529
)
@@ -590,19 +608,25 @@
590608
(i32.const 20)
591609
(i32.const 40)
592610
)
593-
(unreachable)
594-
(drop
595-
(i32.const 20)
596-
)
597611
(call $call-ii
598-
(i32.const 10)
599-
(i32.const 30)
600-
)
601-
(drop
602-
(i32.const 10)
612+
(block (result i32)
613+
(unreachable)
614+
(i32.const 10)
615+
)
616+
(block (result i32)
617+
(drop
618+
(i32.const 20)
619+
)
620+
(i32.const 30)
621+
)
603622
)
604623
(call $call-ii
605-
(unreachable)
624+
(block (result i32)
625+
(drop
626+
(i32.const 10)
627+
)
628+
(unreachable)
629+
)
606630
(block (result i32)
607631
(drop
608632
(i32.const 20)
@@ -613,20 +637,24 @@
613637
(drop
614638
(i32.const 10)
615639
)
616-
(unreachable)
617640
(call $call-ii
618641
(i32.const 20)
619-
(i32.const 30)
642+
(block
643+
(unreachable)
644+
(i32.const 30)
645+
)
620646
)
621647
(drop
622648
(i32.const 10)
623649
)
624-
(drop
625-
(i32.const 30)
626-
)
627650
(call $call-ii
628651
(i32.const 20)
629-
(unreachable)
652+
(block
653+
(drop
654+
(i32.const 30)
655+
)
656+
(unreachable)
657+
)
630658
)
631659
(drop
632660
(i32.const 10)
@@ -653,23 +681,20 @@
653681
(i32.const 30)
654682
(i32.const 50)
655683
)
656-
(drop
657-
(i32.const 50)
658-
)
659684
(drop
660685
(i32.const 10)
661686
)
662687
(drop
663688
(i32.const 30)
664689
)
690+
(drop
691+
(i32.const 50)
692+
)
665693
(call_indirect $ii
666694
(i32.const 20)
667695
(i32.const 40)
668696
(i32.const 60)
669697
)
670-
(drop
671-
(i32.const 50)
672-
)
673698
(call_indirect $ii
674699
(unreachable)
675700
(block (result i32)
@@ -678,7 +703,50 @@
678703
)
679704
(i32.const 40)
680705
)
681-
(i32.const 60)
706+
(block (result i32)
707+
(drop
708+
(i32.const 50)
709+
)
710+
(i32.const 60)
711+
)
712+
)
713+
(drop
714+
(i32.const 31)
715+
)
716+
(call_indirect $ii
717+
(i32.const 41)
718+
(unreachable)
719+
(block (result i32)
720+
(drop
721+
(i32.const 51)
722+
)
723+
(i32.const 61)
724+
)
725+
)
726+
(drop
727+
(i32.const 32)
728+
)
729+
(drop
730+
(i32.const 52)
731+
)
732+
(call_indirect $ii
733+
(i32.const 42)
734+
(i32.const 62)
735+
(unreachable)
736+
)
737+
)
738+
(func $mix-select (type $i) (param $x i32)
739+
(drop
740+
(select
741+
(get_local $x)
742+
(get_local $x)
743+
(block (result i32)
744+
(set_local $x
745+
(i32.const 1)
746+
)
747+
(i32.const 2)
748+
)
749+
)
682750
)
683751
)
684752
(func $block-type-change (type $3)

‎test/passes/remove-unused-names_merge-blocks.wast‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -854,6 +854,50 @@
854854
(i32.const 60)
855855
)
856856
)
857+
(call_indirect $ii
858+
(block $block21 (result i32)
859+
(drop
860+
(i32.const 31)
861+
)
862+
(i32.const 41)
863+
)
864+
(unreachable)
865+
(block $block22 (result i32)
866+
(drop
867+
(i32.const 51)
868+
)
869+
(i32.const 61)
870+
)
871+
)
872+
(call_indirect $ii
873+
(block $block21 (result i32)
874+
(drop
875+
(i32.const 32)
876+
)
877+
(i32.const 42)
878+
)
879+
(block $block22 (result i32)
880+
(drop
881+
(i32.const 52)
882+
)
883+
(i32.const 62)
884+
)
885+
(unreachable)
886+
)
887+
)
888+
(func $mix-select (param $x i32)
889+
(drop
890+
(select
891+
(get_local $x)
892+
(get_local $x)
893+
(block (result i32)
894+
(set_local $x ;; cannot be moved before the gets
895+
(i32.const 1)
896+
)
897+
(i32.const 2)
898+
)
899+
)
900+
)
857901
)
858902
(func $block-type-change (type $3)
859903
(local $0 f64)

0 commit comments

Comments
 (0)