Skip to content

Commit

Permalink
Use empty blocks instead of nops for empty scopes in IRBuilder (#7080)
Browse files Browse the repository at this point in the history
When IRBuilder builds an empty non-block scope such as a function body,
an if arm, a try block, etc, it needs to produce some expression to
represent the empty contents. Previously it produced a nop, but change
it to produce an empty block instead. The binary writer and printer have
special logic to elide empty blocks, so this produces smaller output.

Update J2CLOpts to recognize functions containing empty blocks as
trivial to avoid regressing one of its tests.
  • Loading branch information
tlively authored Nov 15, 2024
1 parent 6efd417 commit 49c45ac
Show file tree
Hide file tree
Showing 136 changed files with 53 additions and 520 deletions.
1 change: 1 addition & 0 deletions src/passes/J2CLOpts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ Expression* getTrivialFunctionBody(Function* func) {
// Only consider trivial the following instructions which can be safely
// inlined and note that their size is at most 2.
if (body->is<Nop>() || body->is<GlobalGet>() || body->is<Const>() ||
(body->is<Block>() && body->cast<Block>()->list.empty()) ||
// Call with no arguments.
(body->is<Call>() && body->dynCast<Call>()->operands.size() == 0) ||
// Simple global.set with a constant.
Expand Down
7 changes: 4 additions & 3 deletions src/wasm/wasm-ir-builder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ void IRBuilder::push(Expression* expr) {

Result<Expression*> IRBuilder::build() {
if (scopeStack.empty()) {
return builder.makeNop();
return builder.makeBlock();
}
if (scopeStack.size() > 1 || !scopeStack.back().isNone()) {
return Err{"unfinished block context"};
Expand Down Expand Up @@ -792,12 +792,13 @@ Result<Expression*> IRBuilder::finishScope(Block* block) {
Expression* ret = nullptr;
if (scope.exprStack.size() == 0) {
// No expressions for this scope, but we need something. If we were given a
// block, we can empty it out and return it, but otherwise we need a nop.
// block, we can empty it out and return it, but otherwise create a new
// empty block.
if (block) {
block->list.clear();
ret = block;
} else {
ret = builder.makeNop();
ret = builder.makeBlock();
}
} else if (scope.exprStack.size() == 1) {
// We can put our single expression directly into the surrounding scope.
Expand Down
3 changes: 0 additions & 3 deletions test/binaryen.js/debug-info.js.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ debugInfo=false
(memory $0 0)
(export "test" (func $0))
(func $0
(nop)
)
)

Expand All @@ -16,7 +15,6 @@ debugInfo=true
(memory $0 0)
(export "test" (func $test))
(func $test
(nop)
)
)

Expand All @@ -27,7 +25,6 @@ debugInfo=false
(memory $0 0)
(export "test" (func $0))
(func $0
(nop)
)
)

3 changes: 0 additions & 3 deletions test/binaryen.js/debug-names.js.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
(table $wor 0 0 funcref)
(func $of (param $wasm i32)
(local $!#$%&'*+-./:<=>?@\^_`|~ f64)
(nop)
)
)

Expand All @@ -28,7 +27,6 @@
(table $wor 0 0 funcref)
(func $of (param $js i32)
(local $!#$%&'*+-./:<=>?@\5c^_`|~ f64)
(nop)
)
)

Expand All @@ -40,7 +38,6 @@
(table $wor 0 0 funcref)
(func $of (param $js i32)
(local $!#$%&'*+-./:<=>?@\5c^_`|~ f64)
(nop)
)
)

4 changes: 0 additions & 4 deletions test/example/module-splitting.txt
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,6 @@ Before:
(elem $0 (global.get $base) $null $foo)
(export "foo" (func $foo))
(func $null (type $0)
(nop)
)
(func $foo (type $1) (param $0 i32) (result i32)
(local.get $0)
Expand All @@ -607,7 +606,6 @@ After:
(export "%table_2" (table $0))
(export "%global" (global $base))
(func $null (type $0)
(nop)
)
(func $foo (type $1) (param $0 i32) (result i32)
(call_indirect $0 (type $1)
Expand Down Expand Up @@ -1144,7 +1142,6 @@ Before:
(export "foo1" (func $foo))
(export "foo2" (func $foo))
(func $foo (type $0)
(nop)
)
)
Keeping: <none>
Expand All @@ -1169,7 +1166,6 @@ Secondary:
(import "primary" "%table" (table $0 1 funcref))
(elem $0 (i32.const 0) $foo)
(func $foo (type $0)
(nop)
)
)

Expand Down
2 changes: 1 addition & 1 deletion test/gtest/cfg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ TEST_F(CFGTest, Empty) {
;; entry
;; exit
0:
0: nop
0: block
)cfg";

Module wasm;
Expand Down
3 changes: 0 additions & 3 deletions test/lit/basic/complexTextNames.wast
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,10 @@
;; CHECK-TEXT: (type $0 (func))

;; CHECK-TEXT: (func $foo\20\28.bar\29 (type $0)
;; CHECK-TEXT-NEXT: (nop)
;; CHECK-TEXT-NEXT: )
;; CHECK-BIN: (type $0 (func))

;; CHECK-BIN: (func $foo\20\28.bar\29 (type $0)
;; CHECK-BIN-NEXT: (nop)
;; CHECK-BIN-NEXT: )
(func $foo\20\28.bar\29)

Expand All @@ -34,7 +32,6 @@
;; CHECK-BIN-NODEBUG: (type $0 (func))

;; CHECK-BIN-NODEBUG: (func $0 (type $0)
;; CHECK-BIN-NODEBUG-NEXT: (nop)
;; CHECK-BIN-NODEBUG-NEXT: )

;; CHECK-BIN-NODEBUG: (func $1 (type $0)
Expand Down
3 changes: 0 additions & 3 deletions test/lit/basic/duplicate_types.wast
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,10 @@
;; CHECK-TEXT: (type $1 (func (param i32) (result i32)))

;; CHECK-TEXT: (func $f0 (type $0) (param $0 i32)
;; CHECK-TEXT-NEXT: (nop)
;; CHECK-TEXT-NEXT: )
;; CHECK-BIN: (type $1 (func (param i32) (result i32)))

;; CHECK-BIN: (func $f0 (type $0) (param $0 i32)
;; CHECK-BIN-NEXT: (nop)
;; CHECK-BIN-NEXT: )
(func $f0 (param i32))

Expand All @@ -47,7 +45,6 @@
;; CHECK-BIN-NODEBUG: (type $1 (func (param i32) (result i32)))

;; CHECK-BIN-NODEBUG: (func $0 (type $0) (param $0 i32)
;; CHECK-BIN-NODEBUG-NEXT: (nop)
;; CHECK-BIN-NODEBUG-NEXT: )

;; CHECK-BIN-NODEBUG: (func $1 (type $1) (param $0 i32) (result i32)
Expand Down
Loading

0 comments on commit 49c45ac

Please sign in to comment.