Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
26 changes: 26 additions & 0 deletions src/ir/intrinsics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

#include "ir/intrinsics.h"
#include "ir/find_all.h"
#include "wasm-builder.h"

namespace wasm {
Expand Down Expand Up @@ -100,4 +101,29 @@ std::vector<Name> Intrinsics::getConfigureAllFunctions(Call* call) {
return ret;
}

std::vector<Name> Intrinsics::getConfigureAllFunctions() {
// ConfigureAll in a start function makes its functions callable.
if (module.start) {
auto* start = module.getFunction(module.start);
if (!start->imported()) {
FindAll<Call> calls(start->body);
// Look for the (single) configureAll.
Call* configureAll = nullptr;
for (auto* call : calls.list) {
if (isConfigureAll(call)) {
if (configureAll) {
Fatal() << "Multiple configureAlls";
} else {
configureAll = call;
}
}
}
if (configureAll) {
return getConfigureAllFunctions(configureAll);
}
}
}
return {};
}

} // namespace wasm
2 changes: 2 additions & 0 deletions src/ir/intrinsics.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ class Intrinsics {
//
// where the segment $seg is of size N.
std::vector<Name> getConfigureAllFunctions(Call* call);
// As above, but looks through the module to find the configureAll.
std::vector<Name> getConfigureAllFunctions();
};

} // namespace wasm
Expand Down
21 changes: 0 additions & 21 deletions src/ir/module-utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/

#include "module-utils.h"
#include "ir/find_all.h"
#include "ir/intrinsics.h"
#include "ir/manipulation.h"
#include "ir/metadata.h"
Expand Down Expand Up @@ -693,26 +692,6 @@ std::vector<HeapType> getPublicHeapTypes(Module& wasm) {
WASM_UNREACHABLE("unexpected export kind");
}

// ConfigureAll in a start function makes its functions callable. They are
// only signature-called, so the heap type does not need to be public - nor
// types referred to - but for now we mark them as public to avoid breakage in
// several passes.
// TODO Specific fixes in those passes could replace this, and allow better
// optimization.
if (wasm.start) {
auto* start = wasm.getFunction(wasm.start);
if (!start->imported()) {
Intrinsics intrinsics(wasm);
for (auto* call : FindAll<Call>(start->body).list) {
if (intrinsics.isConfigureAll(call)) {
for (auto func : intrinsics.getConfigureAllFunctions(call)) {
notePublic(wasm.getFunction(func)->type.getHeapType());
}
}
}
}
}

// Ignorable public types are public.
for (auto type : getIgnorablePublicTypes()) {
notePublic(type);
Expand Down
3 changes: 2 additions & 1 deletion src/passes/AbstractTypeRefining.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,8 @@ struct AbstractTypeRefining : public Pass {

for (auto type : subTypes.types) {
if (!type.isStruct()) {
// TODO: support arrays and funcs
// TODO: Support arrays and functions (for functions we will need to
// handle configureAll).
continue;
}

Expand Down
7 changes: 7 additions & 0 deletions src/passes/SignaturePruning.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,13 @@ struct SignaturePruning : public Pass {
allInfo[tag->type].optimizable = false;
}

// configureAll functions are signature-called, and must also not be
// modified.
for (auto func : Intrinsics(*module).getConfigureAllFunctions()) {
allInfo[module->getFunction(func)->type.getHeapType()].optimizable =
false;
}

// A type must have the same number of parameters and results as its
// supertypes and subtypes, so we only attempt to modify types without
// supertypes or subtypes.
Expand Down
6 changes: 6 additions & 0 deletions src/passes/SignatureRefining.cpp
Copy link
Member

Choose a reason for hiding this comment

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

Is there a way we can allow refining the result types but not the parameter types?

Copy link
Member Author

Choose a reason for hiding this comment

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

We could with extra logic. Do you think it would be worth it?

Copy link
Member

Choose a reason for hiding this comment

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

I think it might be worth it. No reason to lose type information unnecessarily in the returns, and I assume it won't take much extra logic to implement.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I'll look into a followup.

Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,12 @@ struct SignatureRefining : public Pass {
}
}

// configureAll functions are signature-called, and must also not be
// modified.
for (auto func : Intrinsics(*module).getConfigureAllFunctions()) {
allInfo[module->getFunction(func)->type.getHeapType()].canModify = false;
}

// Also skip modifying types used in tags, even private tags, since we don't
// analyze exception handling or stack switching instructions. TODO: Analyze
// and optimize exception handling and stack switching instructions.
Expand Down
2 changes: 2 additions & 0 deletions src/passes/TypeFinalizing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ struct TypeFinalizing : public Pass {
subTypes = SubTypes(*module);
}

// Note we don't need to worry about signature-called functions here
// (configureAll) because such calls don't care about finality.
auto privateTypes = ModuleUtils::getPrivateHeapTypes(*module);
for (auto type : privateTypes) {
// If we are finalizing types then we can only do that to leaf types. If
Expand Down
117 changes: 117 additions & 0 deletions test/lit/passes/signature-pruning-configureAll.wast
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited.

;; RUN: foreach %s %t wasm-opt --signature-pruning --closed-world -all -S -o - | filecheck %s

;; Test that configureAll is respected: referred functions are not pruned.

(module
;; CHECK: (type $externs (array (mut externref)))
(type $externs (array (mut externref)))

;; CHECK: (type $funcs (array (mut funcref)))
(type $funcs (array (mut funcref)))

;; CHECK: (type $bytes (array (mut i8)))
(type $bytes (array (mut i8)))


;; CHECK: (type $configureAll (func (param (ref null $externs) (ref null $funcs) (ref null $bytes) externref)))
(type $configureAll (func (param (ref null $externs)) (param (ref null $funcs)) (param (ref null $bytes)) (param externref)))

(type $struct (struct))

(rec
;; CHECK: (rec
;; CHECK-NEXT: (type $any-2 (func))

;; CHECK: (type $any-1 (func (param anyref)))
(type $any-1 (func (param anyref)))

;; use brands to allow $any-1/2 to be optimized separately.
(type $brand1 (struct))
)

(rec
(type $any-2 (func (param anyref)))

(type $brand2 (struct))
(type $brand3 (struct))
)

;; CHECK: (type $6 (func (result i32)))

;; CHECK: (type $7 (func))

;; CHECK: (import "wasm:js-prototypes" "configureAll" (func $configureAll (type $configureAll) (param (ref null $externs) (ref null $funcs) (ref null $bytes) externref)))
(import "wasm:js-prototypes" "configureAll" (func $configureAll (type $configureAll)))


;; CHECK: (data $bytes "12345678")

;; CHECK: (elem $externs externref (item (ref.null noextern)))
(elem $externs externref
(ref.null extern)
)

;; CHECK: (elem $funcs func $foo $bar)
(elem $funcs funcref
(ref.func $foo)
(ref.func $bar)
)

(data $bytes "12345678")

;; CHECK: (start $start)
(start $start)

;; CHECK: (func $start (type $7)
;; CHECK-NEXT: (call $configureAll
;; CHECK-NEXT: (array.new_elem $externs $externs
;; CHECK-NEXT: (i32.const 0)
;; CHECK-NEXT: (i32.const 1)
;; CHECK-NEXT: )
;; CHECK-NEXT: (array.new_elem $funcs $funcs
;; CHECK-NEXT: (i32.const 0)
;; CHECK-NEXT: (i32.const 2)
;; CHECK-NEXT: )
;; CHECK-NEXT: (array.new_data $bytes $bytes
;; CHECK-NEXT: (i32.const 0)
;; CHECK-NEXT: (i32.const 8)
;; CHECK-NEXT: )
;; CHECK-NEXT: (ref.null noextern)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $start
(call $configureAll
(array.new_elem $externs $externs
(i32.const 0) (i32.const 1))
(array.new_elem $funcs $funcs
(i32.const 0) (i32.const 2))
(array.new_data $bytes $bytes
(i32.const 0) (i32.const 8))
(ref.null extern)
)
)

;; CHECK: (func $foo (type $6) (result i32)
;; CHECK-NEXT: (i32.const 42)
;; CHECK-NEXT: )
(func $foo (result i32)
;; Nothing to do here anyhow, but do not error.
(i32.const 42)
)

;; CHECK: (func $bar (type $any-1) (param $x anyref)
;; CHECK-NEXT: )
(func $bar (type $any-1) (param $x anyref)
;; The param is unused, but will not be pruned (turned into a local) due to
;; configureAll.
)

;; CHECK: (func $unconfigured (type $any-2)
;; CHECK-NEXT: (local $0 anyref)
;; CHECK-NEXT: )
(func $unconfigured (type $any-2) (param $x anyref)
;; This is not referred to by configureAll, and can be pruned.
)
)
30 changes: 14 additions & 16 deletions test/lit/passes/signature-refining-configureAll.wast
Original file line number Diff line number Diff line change
Expand Up @@ -24,35 +24,33 @@

;; CHECK: (type $struct (struct))

;; CHECK: (type $5 (func))
;; CHECK: (type $ret-any-1 (func (result anyref)))

;; CHECK: (type $6 (func (result i32)))

;; CHECK: (type $7 (func))

;; CHECK: (type $configureAll (func (param (ref null $externs) (ref null $funcs) (ref null $bytes) externref)))
;; OPEN_WORLD: (rec
;; OPEN_WORLD-NEXT: (type $ret-any-2 (func (result (ref (exact $struct)))))

;; OPEN_WORLD: (type $struct (struct))

;; OPEN_WORLD: (type $5 (func))
;; OPEN_WORLD: (type $ret-any-1 (func (result anyref)))

;; OPEN_WORLD: (type $6 (func (result i32)))

;; OPEN_WORLD: (type $7 (func))

;; OPEN_WORLD: (type $configureAll (func (param (ref null $externs) (ref null $funcs) (ref null $bytes) externref)))
(type $configureAll (func (param (ref null $externs)) (param (ref null $funcs)) (param (ref null $bytes)) (param externref)))

(type $struct (struct))

(rec
;; CHECK: (type $7 (func (result i32)))

;; CHECK: (rec
;; CHECK-NEXT: (type $ret-any-1 (func (result anyref)))
;; OPEN_WORLD: (type $7 (func (result i32)))

;; OPEN_WORLD: (rec
;; OPEN_WORLD-NEXT: (type $ret-any-1 (func (result anyref)))
(type $ret-any-1 (func (result anyref)))

;; use brands to allow $ret-any-1/2 to be optimized separately.
;; CHECK: (type $brand1 (struct))
;; OPEN_WORLD: (type $brand1 (struct))
(type $brand1 (struct))
)

Expand Down Expand Up @@ -90,7 +88,7 @@
;; OPEN_WORLD: (start $start)
(start $start)

;; CHECK: (func $start (type $5)
;; CHECK: (func $start (type $7)
;; CHECK-NEXT: (call $configureAll
;; CHECK-NEXT: (array.new_elem $externs $externs
;; CHECK-NEXT: (i32.const 0)
Expand All @@ -107,7 +105,7 @@
;; CHECK-NEXT: (ref.null noextern)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; OPEN_WORLD: (func $start (type $5)
;; OPEN_WORLD: (func $start (type $7)
;; OPEN_WORLD-NEXT: (call $configureAll
;; OPEN_WORLD-NEXT: (array.new_elem $externs $externs
;; OPEN_WORLD-NEXT: (i32.const 0)
Expand Down Expand Up @@ -136,10 +134,10 @@
)
)

;; CHECK: (func $foo (type $7) (result i32)
;; CHECK: (func $foo (type $6) (result i32)
;; CHECK-NEXT: (i32.const 42)
;; CHECK-NEXT: )
;; OPEN_WORLD: (func $foo (type $7) (result i32)
;; OPEN_WORLD: (func $foo (type $6) (result i32)
;; OPEN_WORLD-NEXT: (i32.const 42)
;; OPEN_WORLD-NEXT: )
(func $foo (result i32)
Expand Down
Loading