Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions NOTICE
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ under the licensing terms detailed in LICENSE:
* Fabián Heredia Montiel <[email protected]>
* Jonas Minnberg <[email protected]>
* Kam Chehresa <[email protected]>
* Rui Jin <[email protected]>

Portions of this software are derived from third-party works licensed under
the following terms:
Expand Down
117 changes: 108 additions & 9 deletions src/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10026,6 +10026,95 @@ export class Compiler extends DiagnosticEmitter {
// === Specialized code generation ==============================================================

/** Makes a constant zero of the specified type. */
/** Checks if an expression evaluates to a zero value for the given type. */
shouldSkipZeroInit(expr: ExpressionRef, type: Type): bool {
let module = this.module;
// Try to evaluate the expression at compile time
let evaled = module.runExpression(expr, ExpressionRunnerFlags.Default);
if (!evaled) return false; // Can't evaluate at compile time

const evaledType = getExpressionType(evaled);
switch (type.kind) {
case TypeKind.Bool:
case TypeKind.I8:
case TypeKind.I16:
case TypeKind.I32:
case TypeKind.U8:
case TypeKind.U16:
case TypeKind.U32:
// All small integer types are represented as i32 in WebAssembly
// But we still check for safety and consistency
if (evaledType == TypeRef.I32) {
return getConstValueI32(evaled) === 0;
}
return false;
case TypeKind.I64:
case TypeKind.U64:
case TypeKind.Isize:
case TypeKind.Usize:
// Only call getConstValueI64* if the expression is actually i64
if (evaledType == TypeRef.I64) {
return getConstValueI64Low(evaled) === 0 && getConstValueI64High(evaled) === 0;
}
// For size types on 32-bit platforms, they might be i32
if (evaledType == TypeRef.I32 && (type.kind == TypeKind.Isize || type.kind == TypeKind.Usize)) {
return getConstValueI32(evaled) === 0;
}
return false;
case TypeKind.F32:
if (evaledType == TypeRef.F32) {
return getConstValueF32(evaled) === 0.0;
}
return false;
case TypeKind.F64:
if (evaledType == TypeRef.F64) {
return getConstValueF64(evaled) === 0.0;
}
return false;
default:
// For reference types, zero means null
if (type.isReference && type.is(TypeFlags.Nullable)) {
return getExpressionId(evaled) === ExpressionId.RefNull;
}
return false;
}
}

/** Checks if a field type can use the default zero-initialized memory value. */
canUseZeroDefault(type: Type): bool {
switch (type.kind) {
default: assert(false);
case TypeKind.Bool:
case TypeKind.I8:
case TypeKind.I16:
case TypeKind.I32:
case TypeKind.U8:
case TypeKind.U16:
case TypeKind.U32:
case TypeKind.I64:
case TypeKind.U64:
case TypeKind.Isize:
case TypeKind.Usize:
case TypeKind.F32:
case TypeKind.F64:
case TypeKind.V128:
return true; // Value types default to zero in zero-initialized memory
case TypeKind.Func:
case TypeKind.Extern:
case TypeKind.Any:
case TypeKind.Eq:
case TypeKind.Struct:
case TypeKind.Array:
case TypeKind.String:
case TypeKind.StringviewWTF8:
case TypeKind.StringviewWTF16:
case TypeKind.StringviewIter:
case TypeKind.I31:
// Reference types: only nullable refs can use zero (null) default
return type.is(TypeFlags.Nullable);
}
}

makeZero(type: Type): ExpressionRef {
let module = this.module;
switch (type.kind) {
Expand Down Expand Up @@ -10372,6 +10461,7 @@ export class Compiler extends DiagnosticEmitter {
let parameterIndex = fieldPrototype.parameterIndex;

// Defer non-parameter fields until parameter fields are initialized
// Since non-parameter may depend on parameter fields
if (parameterIndex < 0) {
if (!nonParameterFields) nonParameterFields = new Array();
nonParameterFields.push(property);
Expand Down Expand Up @@ -10407,16 +10497,25 @@ export class Compiler extends DiagnosticEmitter {
let initializerNode = fieldPrototype.initializerNode;
assert(fieldPrototype.parameterIndex < 0);
let setterInstance = assert(field.setterInstance);
let expr = this.makeCallDirect(setterInstance, [
module.local_get(thisLocalIndex, sizeTypeRef),
initializerNode // use initializer if present, otherwise initialize with zero
? this.compileExpression(initializerNode, fieldType, Constraints.ConvImplicit)
: this.makeZero(fieldType)
], field.identifierNode, true);
if (this.currentType != Type.void) { // in case
expr = module.drop(expr);

if (initializerNode) {
Copy link
Member

Choose a reason for hiding this comment

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

If there isn't an initializerNode, this code neglects to zero-initialize the field when stub runtime is used (and the stub runtime doesn't zero-initialize allocated memory!). The previous code's this.makeZero(fieldType) happened to cover this case.

Copy link
Author

Choose a reason for hiding this comment

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

Looks no need to zero-initialize because:

  1. memory will not be collected and reused in stub runtime.
  2. memory grow will default to initialized with 0 as wasm spec.

Copy link
Member

Choose a reason for hiding this comment

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

The exception is when the latest allocation is being freed. There's also the case when memory is imported (which means it's not necessarily zeroed), as well as the very niche case when the heap is reset (which also leaves the memory dirty upon future allocations).

You are correct in saying that memory.grow zeroes out the new memory, but the above cases still show why fields need to be zeroed when using the stub allocator.

Copy link
Author

Choose a reason for hiding this comment

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

Okay, I understand your point.
Perhaps the safest approach here is to simply disable this optimization in stub rt.

// Explicit initializer
// Check if we need to initialize this field
const valueExpr: ExpressionRef = this.compileExpression(initializerNode, fieldType, Constraints.ConvImplicit);
if (!this.shouldSkipZeroInit(valueExpr, fieldType)) {
let expr = this.makeCallDirect(setterInstance, [
module.local_get(thisLocalIndex, sizeTypeRef),
valueExpr
], field.identifierNode, true);
if (this.currentType != Type.void) { // in case
expr = module.drop(expr);
}
stmts.push(expr);
}
} else {
// No explicit initializer
assert(this.canUseZeroDefault(fieldType));
}
stmts.push(expr);
}
}

Expand Down
59 changes: 7 additions & 52 deletions tests/compiler/assignment-chain.debug.wat
Original file line number Diff line number Diff line change
Expand Up @@ -2250,35 +2250,30 @@
local.get $ptr
return
)
(func $assignment-chain/A#set:y (param $this i32) (param $y i64)
local.get $this
local.get $y
i64.store offset=8
)
(func $assignment-chain/A#set:x (param $this i32) (param $x i64)
local.get $this
local.get $x
i64.store
)
(func $assignment-chain/A#set:y (param $this i32) (param $y i64)
(func $assignment-chain/B#get:_setter_cnt (param $this i32) (result i32)
local.get $this
local.get $y
i64.store offset=8
i32.load
)
(func $assignment-chain/B#set:_setter_cnt (param $this i32) (param $_setter_cnt i32)
local.get $this
local.get $_setter_cnt
i32.store
)
(func $assignment-chain/B#set:_getter_cnt (param $this i32) (param $_getter_cnt i32)
local.get $this
local.get $_getter_cnt
i32.store offset=4
)
(func $assignment-chain/B#set:_y (param $this i32) (param $_y f64)
local.get $this
local.get $_y
f64.store offset=8
)
(func $assignment-chain/B#get:_setter_cnt (param $this i32) (result i32)
local.get $this
i32.load
)
(func $assignment-chain/B#get:_getter_cnt (param $this i32) (result i32)
local.get $this
i32.load offset=4
Expand Down Expand Up @@ -2434,22 +2429,6 @@
local.get $this
local.set $1
global.get $~lib/memory/__stack_pointer
local.get $1
i32.store offset=4
local.get $1
i64.const 0
call $assignment-chain/A#set:x
local.get $this
local.set $1
global.get $~lib/memory/__stack_pointer
local.get $1
i32.store offset=4
local.get $1
i64.const 0
call $assignment-chain/A#set:y
local.get $this
local.set $1
global.get $~lib/memory/__stack_pointer
i32.const 8
i32.add
global.set $~lib/memory/__stack_pointer
Expand Down Expand Up @@ -2552,30 +2531,6 @@
local.get $this
local.set $1
global.get $~lib/memory/__stack_pointer
local.get $1
i32.store offset=4
local.get $1
i32.const 0
call $assignment-chain/B#set:_setter_cnt
local.get $this
local.set $1
global.get $~lib/memory/__stack_pointer
local.get $1
i32.store offset=4
local.get $1
i32.const 0
call $assignment-chain/B#set:_getter_cnt
local.get $this
local.set $1
global.get $~lib/memory/__stack_pointer
local.get $1
i32.store offset=4
local.get $1
f64.const 0
call $assignment-chain/B#set:_y
local.get $this
local.set $1
global.get $~lib/memory/__stack_pointer
i32.const 8
i32.add
global.set $~lib/memory/__stack_pointer
Expand Down
34 changes: 2 additions & 32 deletions tests/compiler/assignment-chain.release.wat
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@
local.get $0
global.set $~lib/rt/itcms/iter
end
block $__inlined_func$~lib/rt/itcms/Object#unlink$129
block $__inlined_func$~lib/rt/itcms/Object#unlink$124
local.get $1
i32.load offset=4
i32.const -4
Expand All @@ -146,7 +146,7 @@
call $~lib/builtins/abort
unreachable
end
br $__inlined_func$~lib/rt/itcms/Object#unlink$129
br $__inlined_func$~lib/rt/itcms/Object#unlink$124
end
local.get $1
i32.load offset=8
Expand Down Expand Up @@ -1661,18 +1661,6 @@
local.tee $1
i32.store
global.get $~lib/memory/__stack_pointer
local.get $1
i32.store offset=4
local.get $1
i64.const 0
i64.store
global.get $~lib/memory/__stack_pointer
local.get $1
i32.store offset=4
local.get $1
i64.const 0
i64.store offset=8
global.get $~lib/memory/__stack_pointer
i32.const 8
i32.add
global.set $~lib/memory/__stack_pointer
Expand Down Expand Up @@ -1790,24 +1778,6 @@
local.tee $1
i32.store
global.get $~lib/memory/__stack_pointer
local.get $1
i32.store offset=4
local.get $1
i32.const 0
i32.store
global.get $~lib/memory/__stack_pointer
local.get $1
i32.store offset=4
local.get $1
i32.const 0
i32.store offset=4
global.get $~lib/memory/__stack_pointer
local.get $1
i32.store offset=4
local.get $1
f64.const 0
f64.store offset=8
global.get $~lib/memory/__stack_pointer
i32.const 8
i32.add
global.set $~lib/memory/__stack_pointer
Expand Down
Loading
Loading