Skip to content

Cast to bool by comparing to zero #343

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 23, 2018
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
5 changes: 3 additions & 2 deletions src/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7516,9 +7516,10 @@ export class Compiler extends DiagnosticEmitter {
}
case TypeKind.BOOL: {
if (flow.canOverflow(expr, type)) {
expr = module.createBinary(BinaryOp.AndI32,
// bool is special in that it compares to 0 instead of masking with 0x1
expr = module.createBinary(BinaryOp.NeI32,
expr,
module.createI32(0x1)
module.createI32(0)
);
}
break;
Expand Down
4 changes: 4 additions & 0 deletions src/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1169,6 +1169,10 @@ export function isTeeLocal(expr: ExpressionRef): bool {
return _BinaryenSetLocalIsTee(expr);
}

export function getGetGlobalName(expr: ExpressionRef): string | null {
return readString(_BinaryenGetGlobalGetName(expr));
}

export function getBinaryOp(expr: ExpressionRef): BinaryOp {
return _BinaryenBinaryGetOp(expr);
}
Expand Down
20 changes: 12 additions & 8 deletions src/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ import {
getBlockName,
getConstValueF32,
getConstValueF64,
getConstValueI64Low
getConstValueI64Low,
getGetGlobalName
} from "./module";

import {
Expand Down Expand Up @@ -3314,7 +3315,7 @@ export class Flow {
/**
* Tests if an expression can possibly overflow in the context of this flow. Assumes that the
* expression might already have overflown and returns `false` only if the operation neglects
* any possibly combination of garbage bits being present.
* any possible combination of garbage bits being present.
*/
canOverflow(expr: ExpressionRef, type: Type): bool {
// TODO: the following catches most common and a few uncommon cases, but there are additional
Expand All @@ -3336,13 +3337,18 @@ export class Flow {
}

// overflows if the value does
case ExpressionId.SetLocal: {
case ExpressionId.SetLocal: { // tee
assert(isTeeLocal(expr));
return this.canOverflow(getSetLocalValue(expr), type);
}

// never overflows because globals are wrapped on set
case ExpressionId.GetGlobal: return false;
// overflows if the conversion does (globals are wrapped on set)
case ExpressionId.GetGlobal: {
// TODO: this is inefficient because it has to read a string
let global = assert(this.currentFunction.program.elementsLookup.get(assert(getGetGlobalName(expr))));
assert(global.kind == ElementKind.GLOBAL);
return canConversionOverflow(assert((<Global>global).type), type);
Copy link
Member Author

@dcodeIO dcodeIO Nov 23, 2018

Choose a reason for hiding this comment

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

Not related to the casting itself, more an unrelated fix for reading from an i32 global before converting to an integer smaller than the global's type. Surfaced in the bool test.

}

case ExpressionId.Binary: {
switch (getBinaryOp(expr)) {
Expand Down Expand Up @@ -3567,9 +3573,7 @@ export class Flow {

/** Tests if a conversion from one type to another can technically overflow. */
function canConversionOverflow(fromType: Type, toType: Type): bool {
var fromSize = fromType.byteSize;
var toSize = toType.byteSize;
return !fromType.is(TypeFlags.INTEGER) // non-i32 locals or returns
|| fromSize > toSize
|| fromType.size > toType.size
|| fromType.is(TypeFlags.SIGNED) != toType.is(TypeFlags.SIGNED);
}
8 changes: 4 additions & 4 deletions tests/compiler/abi.untouched.wat
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,8 @@
i32.ctz
set_local $0
get_local $0
i32.const 1
i32.and
i32.const 0
i32.ne
i32.eqz
if
i32.const 0
Expand All @@ -172,8 +172,8 @@
i32.clz
set_local $0
get_local $0
i32.const 1
i32.and
i32.const 0
i32.ne
i32.eqz
if
i32.const 0
Expand Down
82 changes: 38 additions & 44 deletions tests/compiler/binary.optimized.wat
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,6 @@
set_local $3
end
get_local $3
i32.const 1
i32.and
if
get_local $0
get_local $0
Expand Down Expand Up @@ -655,27 +653,25 @@
get_global $binary/f
call $~lib/math/NativeMathf.mod
set_global $binary/f
block $__inlined_func$~lib/math/NativeMathf.pow0
get_global $binary/f
tee_local $0
i32.reinterpret/f32
i32.const 2147483647
i32.and
i32.const 2139095040
i32.gt_s
tee_local $1
i32.eqz
if
i32.const 0
set_local $1
end
get_local $1
if
get_local $0
f32.const 1
f32.add
set_local $0
end
get_global $binary/f
tee_local $0
i32.reinterpret/f32
i32.const 2147483647
i32.and
i32.const 2139095040
i32.gt_s
tee_local $1
i32.eqz
if
i32.const 0
set_local $1
end
get_local $1
if
get_local $0
f32.const 1
f32.add
set_local $0
end
get_local $0
set_global $binary/f
Expand All @@ -690,27 +686,25 @@
get_global $binary/f
call $~lib/math/NativeMathf.mod
set_global $binary/f
block $__inlined_func$~lib/math/NativeMathf.pow2
get_global $binary/f
tee_local $0
i32.reinterpret/f32
i32.const 2147483647
i32.and
i32.const 2139095040
i32.gt_s
tee_local $1
i32.eqz
if
i32.const 0
set_local $1
end
get_local $1
if
get_local $0
f32.const 1
f32.add
set_local $0
end
get_global $binary/f
tee_local $0
i32.reinterpret/f32
i32.const 2147483647
i32.and
i32.const 2139095040
i32.gt_s
tee_local $1
i32.eqz
if
i32.const 0
set_local $1
end
get_local $1
if
get_local $0
f32.const 1
f32.add
set_local $0
end
get_local $0
set_global $binary/f
Expand Down
8 changes: 4 additions & 4 deletions tests/compiler/binary.untouched.wat
Original file line number Diff line number Diff line change
Expand Up @@ -1250,8 +1250,8 @@
get_local $1
f32.ne
end
i32.const 1
i32.and
i32.const 0
i32.ne
if
get_local $0
get_local $1
Expand Down Expand Up @@ -2531,8 +2531,8 @@
get_local $1
f64.ne
end
i32.const 1
i32.and
i32.const 0
i32.ne
if
get_local $0
get_local $1
Expand Down
119 changes: 119 additions & 0 deletions tests/compiler/bool.optimized.wat
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
(module
(type $iiiiv (func (param i32 i32 i32 i32)))
(type $v (func))
(import "env" "abort" (func $~lib/env/abort (param i32 i32 i32 i32)))
(memory $0 1)
(data (i32.const 8) "\07\00\00\00b\00o\00o\00l\00.\00t\00s")
(table $0 1 anyfunc)
(elem (i32.const 0) $null)
(global $bool/i (mut i32) (i32.const 2))
(global $bool/I (mut i64) (i64.const 2))
(global $bool/u (mut i32) (i32.const 2))
(global $bool/U (mut i64) (i64.const 2))
(global $bool/f (mut f32) (f32.const 2))
(global $bool/F (mut f64) (f64.const 2))
(global $bool/uu (mut i32) (i32.const 2))
(export "memory" (memory $0))
(export "table" (table $0))
(start $start)
(func $start (; 1 ;) (type $v)
get_global $bool/i
i32.const 0
i32.ne
i32.const 1
i32.ne
if
i32.const 0
i32.const 8
i32.const 2
i32.const 0
call $~lib/env/abort
unreachable
end
get_global $bool/I
i32.wrap/i64
i32.const 0
i32.ne
i32.const 1
i32.ne
if
i32.const 0
i32.const 8
i32.const 4
i32.const 0
call $~lib/env/abort
unreachable
end
get_global $bool/u
i32.const 0
i32.ne
i32.const 1
i32.ne
if
i32.const 0
i32.const 8
i32.const 6
i32.const 0
call $~lib/env/abort
unreachable
end
get_global $bool/U
i32.wrap/i64
i32.const 0
i32.ne
i32.const 1
i32.ne
if
i32.const 0
i32.const 8
i32.const 8
i32.const 0
call $~lib/env/abort
unreachable
end
get_global $bool/f
i32.trunc_u/f32
i32.const 0
i32.ne
i32.const 1
i32.ne
if
i32.const 0
i32.const 8
i32.const 10
i32.const 0
call $~lib/env/abort
unreachable
end
get_global $bool/F
i32.trunc_u/f64
i32.const 0
i32.ne
i32.const 1
i32.ne
if
i32.const 0
i32.const 8
i32.const 12
i32.const 0
call $~lib/env/abort
unreachable
end
get_global $bool/uu
i32.const 0
i32.ne
i32.const 1
i32.ne
if
i32.const 0
i32.const 8
i32.const 14
i32.const 0
call $~lib/env/abort
unreachable
end
)
(func $null (; 2 ;) (type $v)
nop
)
)
14 changes: 14 additions & 0 deletions tests/compiler/bool.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
var i = <i32>2;
assert(<bool>i == true);
var I = <i64>2;
assert(<bool>I == true);
var u = <u32>2;
assert(<bool>u == true);
var U = <u64>2;
assert(<bool>U == true);
var f = <f32>2;
assert(<bool>f == true);
var F = <f64>2;
assert(<bool>F == true);
var uu = <u8>2;
assert(<bool>uu == true);
Loading