Skip to content

Reduce monomophic versions of bswap in DataView #1717

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.

Sign up for GitHub

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 2 commits into from
Mar 8, 2021
Merged

Reduce monomophic versions of bswap in DataView #1717

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
78f09dd
reduce monomophic versions of bswap
MaxGraey Mar 6, 2021
6a6cc92
use unsigned shifts for bswap
MaxGraey Mar 6, 2021
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
12 changes: 6 additions & 6 deletions std/assembly/dataview.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,15 @@ export class DataView {
(byteOffset >>> 31) | i32(byteOffset + 2 > this.byteLength)
) throw new RangeError(E_INDEXOUTOFRANGE);
var result: i16 = load<i16>(this.dataStart + <usize>byteOffset);
return littleEndian ? result : bswap<i16>(result);
return littleEndian ? result : bswap<u16>(result);
}

getInt32(byteOffset: i32, littleEndian: bool = false): i32 {
if (
(byteOffset >>> 31) | i32(byteOffset + 4 > this.byteLength)
) throw new RangeError(E_INDEXOUTOFRANGE);
var result: i32 = load<i32>(this.dataStart + <usize>byteOffset);
return littleEndian ? result : bswap<i32>(result);
return littleEndian ? result : bswap<u32>(result);
}

getUint8(byteOffset: i32): u8 {
Expand Down Expand Up @@ -114,14 +114,14 @@ export class DataView {
if (
(byteOffset >>> 31) | i32(byteOffset + 2 > this.byteLength)
) throw new RangeError(E_INDEXOUTOFRANGE);
store<i16>(this.dataStart + <usize>byteOffset, littleEndian ? value : bswap<i16>(value));
store<i16>(this.dataStart + <usize>byteOffset, littleEndian ? value : bswap<u16>(value));
}

setInt32(byteOffset: i32, value: i32, littleEndian: bool = false): void {
if (
(byteOffset >>> 31) | i32(byteOffset + 4 > this.byteLength)
) throw new RangeError(E_INDEXOUTOFRANGE);
store<i32>(this.dataStart + <usize>byteOffset, littleEndian ? value : bswap<i32>(value));
store<i32>(this.dataStart + <usize>byteOffset, littleEndian ? value : bswap<u32>(value));
}

setUint8(byteOffset: i32, value: u8): void {
Expand Down Expand Up @@ -150,7 +150,7 @@ export class DataView {
(byteOffset >>> 31) | i32(byteOffset + 8 > this.byteLength)
) throw new RangeError(E_INDEXOUTOFRANGE);
var result: i64 = load<i64>(this.dataStart + <usize>byteOffset);
return littleEndian ? result : bswap<i64>(result);
return littleEndian ? result : bswap<u64>(result);
}

getUint64(byteOffset: i32, littleEndian: bool = false): u64 {
Expand All @@ -165,7 +165,7 @@ export class DataView {
if (
(byteOffset >>> 31) | i32(byteOffset + 8 > this.byteLength)
) throw new RangeError(E_INDEXOUTOFRANGE);
store<i64>(this.dataStart + <usize>byteOffset, littleEndian ? value : bswap<i64>(value));
store<i64>(this.dataStart + <usize>byteOffset, littleEndian ? value : bswap<u64>(value));
}

setUint64(byteOffset: i32, value: u64, littleEndian: bool = false): void {
Expand Down
10 changes: 5 additions & 5 deletions std/assembly/polyfills.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
export function bswap<T extends number>(value: T): T {
if (isInteger<T>()) {
if (sizeof<T>() == 2) {
return <T>((value << 8) | ((value >> 8) & <T>0x00FF));
return <T>((value << 8) | ((value >>> 8) & <T>0x00FF));
}
if (sizeof<T>() == 4) {
return <T>(
rotl<u32>(<u32>value & 0xFF00FF00, 8) |
rotr<u32>(<u32>value & 0x00FF00FF, 8)
rotl<u32>(value & 0xFF00FF00, 8) |
rotr<u32>(value & 0x00FF00FF, 8)
);
}
if (sizeof<T>() == 8) {
Expand All @@ -28,9 +28,9 @@ export function bswap<T extends number>(value: T): T {
export function bswap16<T extends number>(value: T): T {
if (isInteger<T>() && sizeof<T>() <= 4) {
if (sizeof<T>() == 2) {
return <T>((value << 8) | ((value >> 8) & <T>0x00FF));
return <T>((value << 8) | ((value >>> 8) & <T>0x00FF));
} else if (sizeof<T>() == 4) {
return <T>(((value << 8) & <T>0xFF00) | ((value >> 8) & <T>0x00FF) | (value & <T>0xFFFF0000));
return <T>(((value << 8) & <T>0xFF00) | ((value >>> 8) & <T>0x00FF) | (value & <T>0xFFFF0000));
Copy link
Member

Choose a reason for hiding this comment

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

Wondering a bit about the usefulness of bswap16, since we are not using it anywhere. What are typical use cases for it?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know but post-MVP suggest add it: https://github.com/WebAssembly/design/blob/master/FutureFeatures.md#additional-integer-operators

Copy link
Member

Choose a reason for hiding this comment

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

I see, hmm. If I'm not mistaken, semantics there seem a little different tho in that high bits are discarded.

Copy link
Member Author

Choose a reason for hiding this comment

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

yes. bswap16 preserve high part of value while bswap<u16> don't

Copy link
Member

@dcodeIO dcodeIO Mar 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Do you think it would be possible to integrate it into bswap, as bswap<u16>, given the assumption that discarding high bits is fine? For i16, we'd then also sign extend after swapping I guess. (Doesn't need to be done in this PR)

Copy link
Member Author

@MaxGraey MaxGraey Mar 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I think we could remove swap16 at least for now. I don't remind where it needed. Anyway this easily implement in user space if it really required in some project. wdyt?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I guess we could do this similar to other generic builtins. For instance, load and load16_s also become just a generic load<T>, and I'd expect bswap and bswap16 to become a single bswap<T> as well. On bswap<i16>, the input is already known to have garbage in the high bits, so doing either just a bswap16 on unsigned or a bswap16 + sign-extend on signed seems sufficient.

}
return value;
}
Expand Down
29 changes: 8 additions & 21 deletions tests/compiler/std/dataview.optimized.wat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
(type $none_=>_none (func))
(type $i32_i32_=>_i32 (func (param i32 i32) (result i32)))
(type $i32_i32_i32_=>_i32 (func (param i32 i32 i32) (result i32)))
(type $i32_=>_i32 (func (param i32) (result i32)))
(type $i32_=>_none (func (param i32)))
(type $i32_=>_i32 (func (param i32) (result i32)))
(type $i32_i64_i32_=>_none (func (param i32 i64 i32)))
(type $i32_i32_=>_i64 (func (param i32 i32) (result i64)))
(type $i32_i32_i32_i32_=>_none (func (param i32 i32 i32 i32)))
Expand Down Expand Up @@ -1939,17 +1939,15 @@
i32.add
i32.load8_s
)
(func $~lib/polyfills/bswap<i16> (param $0 i32) (result i32)
(func $~lib/polyfills/bswap<u16> (param $0 i32) (result i32)
local.get $0
i32.const 16
i32.const 8
i32.shl
i32.const 24
i32.shr_s
i32.const 255
i32.and
local.get $0
i32.const 65535
i32.and
i32.const 8
i32.shl
i32.shr_u
i32.or
)
(func $~lib/dataview/DataView#getInt16 (param $0 i32) (param $1 i32) (param $2 i32) (result i32)
Expand Down Expand Up @@ -1982,7 +1980,7 @@
local.get $0
else
local.get $0
call $~lib/polyfills/bswap<i16>
call $~lib/polyfills/bswap<u16>
end
)
(func $~lib/dataview/DataView#getInt32 (param $0 i32) (param $1 i32) (param $2 i32) (result i32)
Expand Down Expand Up @@ -2063,17 +2061,6 @@
i32.add
i32.load8_u
)
(func $~lib/polyfills/bswap<u16> (param $0 i32) (result i32)
local.get $0
i32.const 8
i32.shl
local.get $0
i32.const 65535
i32.and
i32.const 8
i32.shr_u
i32.or
)
(func $~lib/dataview/DataView#getUint16 (param $0 i32) (param $1 i32) (param $2 i32) (result i32)
local.get $1
i32.const 31
Expand Down Expand Up @@ -2242,7 +2229,7 @@
local.get $1
else
local.get $1
call $~lib/polyfills/bswap<i16>
call $~lib/polyfills/bswap<u16>
end
i32.store16
)
Expand Down
125 changes: 11 additions & 114 deletions tests/compiler/std/dataview.untouched.wat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@
(type $i32_i32_i32_i32_=>_none (func (param i32 i32 i32 i32)))
(type $i32_i32_i64_i32_=>_none (func (param i32 i32 i64 i32)))
(type $i32_i32_i32_i32_=>_i32 (func (param i32 i32 i32 i32) (result i32)))
(type $i64_=>_i64 (func (param i64) (result i64)))
(type $i32_i32_i32_=>_i64 (func (param i32 i32 i32) (result i64)))
(type $i32_i32_f32_i32_=>_none (func (param i32 i32 f32 i32)))
(type $i32_i32_f64_i32_=>_none (func (param i32 i32 f64 i32)))
(type $none_=>_i32 (func (result i32)))
(type $i64_=>_i64 (func (param i64) (result i64)))
(type $i32_i32_i32_=>_f32 (func (param i32 i32 i32) (result f32)))
(type $i32_i32_i32_=>_f64 (func (param i32 i32 i32) (result f64)))
(import "env" "abort" (func $~lib/builtins/abort (param i32 i32 i32 i32)))
Expand Down Expand Up @@ -2672,7 +2672,7 @@
i32.add
i32.load8_s
)
(func $~lib/polyfills/bswap<i16> (param $0 i32) (result i32)
(func $~lib/polyfills/bswap<u16> (param $0 i32) (result i32)
i32.const 1
drop
i32.const 2
Expand All @@ -2685,14 +2685,12 @@
i32.and
i32.shl
local.get $0
i32.const 16
i32.shl
i32.const 16
i32.shr_s
i32.const 65535
i32.and
i32.const 8
i32.const 15
i32.and
i32.shr_s
i32.shr_u
i32.const 255
i32.and
i32.or
Expand Down Expand Up @@ -2729,33 +2727,9 @@
local.get $3
else
local.get $3
call $~lib/polyfills/bswap<i16>
call $~lib/polyfills/bswap<u16>
end
)
(func $~lib/polyfills/bswap<i32> (param $0 i32) (result i32)
i32.const 1
drop
i32.const 4
i32.const 2
i32.eq
drop
i32.const 4
i32.const 4
i32.eq
drop
local.get $0
i32.const -16711936
i32.and
i32.const 8
i32.rotl
local.get $0
i32.const 16711935
i32.and
i32.const 8
i32.rotr
i32.or
return
)
(func $~lib/dataview/DataView#getInt32 (param $0 i32) (param $1 i32) (param $2 i32) (result i32)
(local $3 i32)
local.get $1
Expand Down Expand Up @@ -2787,62 +2761,9 @@
local.get $3
else
local.get $3
call $~lib/polyfills/bswap<i32>
call $~lib/polyfills/bswap<u32>
end
)
(func $~lib/polyfills/bswap<i64> (param $0 i64) (result i64)
(local $1 i64)
(local $2 i64)
(local $3 i64)
i32.const 1
drop
i32.const 8
i32.const 2
i32.eq
drop
i32.const 8
i32.const 4
i32.eq
drop
i32.const 8
i32.const 8
i32.eq
drop
local.get $0
i64.const 8
i64.shr_u
i64.const 71777214294589695
i64.and
local.set $1
local.get $0
i64.const 71777214294589695
i64.and
i64.const 8
i64.shl
local.set $2
local.get $1
local.get $2
i64.or
local.set $3
local.get $3
i64.const 16
i64.shr_u
i64.const 281470681808895
i64.and
local.set $1
local.get $3
i64.const 281470681808895
i64.and
i64.const 16
i64.shl
local.set $2
local.get $1
local.get $2
i64.or
i64.const 32
i64.rotr
return
)
(func $~lib/dataview/DataView#getInt64 (param $0 i32) (param $1 i32) (param $2 i32) (result i64)
(local $3 i64)
local.get $1
Expand Down Expand Up @@ -2874,7 +2795,7 @@
local.get $3
else
local.get $3
call $~lib/polyfills/bswap<i64>
call $~lib/polyfills/bswap<u64>
end
)
(func $~lib/dataview/DataView#getUint8 (param $0 i32) (param $1 i32) (result i32)
Expand All @@ -2896,30 +2817,6 @@
i32.add
i32.load8_u
)
(func $~lib/polyfills/bswap<u16> (param $0 i32) (result i32)
i32.const 1
drop
i32.const 2
i32.const 2
i32.eq
drop
local.get $0
i32.const 8
i32.const 15
i32.and
i32.shl
local.get $0
i32.const 65535
i32.and
i32.const 8
i32.const 15
i32.and
i32.shr_u
i32.const 255
i32.and
i32.or
return
)
(func $~lib/dataview/DataView#getUint16 (param $0 i32) (param $1 i32) (param $2 i32) (result i32)
(local $3 i32)
local.get $1
Expand Down Expand Up @@ -3146,7 +3043,7 @@
local.get $2
else
local.get $2
call $~lib/polyfills/bswap<i16>
call $~lib/polyfills/bswap<u16>
end
i32.store16
)
Expand Down Expand Up @@ -3178,7 +3075,7 @@
local.get $2
else
local.get $2
call $~lib/polyfills/bswap<i32>
call $~lib/polyfills/bswap<u32>
end
i32.store
)
Expand Down Expand Up @@ -3210,7 +3107,7 @@
local.get $2
else
local.get $2
call $~lib/polyfills/bswap<i64>
call $~lib/polyfills/bswap<u64>
end
i64.store
)
Expand Down
6 changes: 3 additions & 3 deletions tests/compiler/std/polyfills.untouched.wat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@
i32.const 8
i32.const 15
i32.and
i32.shr_s
i32.shr_u
i32.const 255
i32.and
i32.or
Expand Down Expand Up @@ -400,7 +400,7 @@
i32.const 8
i32.const 15
i32.and
i32.shr_s
i32.shr_u
i32.const 255
i32.and
i32.or
Expand Down Expand Up @@ -466,7 +466,7 @@
i32.and
local.get $0
i32.const 8
i32.shr_s
i32.shr_u
i32.const 255
i32.and
i32.or
Expand Down