Skip to content

Update Binaryen and other dependencies#1224

Merged
dcodeIO merged 4 commits into
masterfrom
binaryen202004
Apr 15, 2020
Merged

Update Binaryen and other dependencies#1224
dcodeIO merged 4 commits into
masterfrom
binaryen202004

Conversation

@dcodeIO

@dcodeIO dcodeIO commented Apr 14, 2020

Copy link
Copy Markdown
Member

As usual, this updates Binaryen and other dependencies to the latest and greatest.

Includes

  • v128.abs<T> for 8/16/32 bit integers incl. i8x16.abs, i16x8.abs, i32x4.abs
  • v128.bitmask<T> including i8x16.bitmask, i16x8.bitmask, i32x4.bitmask
  • tuple.create / tuple.extract pseudo instructions for multi value types
  • BinaryenType now being usize, unfortunately triggering lots of 32/64 conversion warnings in switches when compiling to Wasm

global.set $abi/condition
i32.const 0
global.set $abi/y
)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Interesting :)

@MaxGraey MaxGraey Apr 15, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

it's ok. I told you earlier that binaryen improved some passes)

Comment on lines -315 to -338
i32.lt_s
global.set $binary/b
global.get $binary/i
i32.const 1
i32.gt_s
global.set $binary/b
global.get $binary/i
i32.const 1
i32.le_s
global.set $binary/b
global.get $binary/i
i32.const 1
i32.ge_s
global.set $binary/b
global.get $binary/i
i32.const 1
i32.eq
global.set $binary/b
global.get $binary/i
i32.const 1
i32.eq
global.set $binary/b
global.get $binary/i
i32.const 1

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Even more interesting

Comment on lines -7 to -16
(global $constructor/emptyCtor (mut i32) (i32.const 0))
(global $constructor/emptyCtorWithFieldInit (mut i32) (i32.const 0))
(global $constructor/emptyCtorWithFieldNoInit (mut i32) (i32.const 0))
(global $constructor/none (mut i32) (i32.const 0))
(global $constructor/justFieldInit (mut i32) (i32.const 0))
(global $constructor/justFieldNoInit (mut i32) (i32.const 0))
(global $constructor/ctorReturns (mut i32) (i32.const 0))
(global $constructor/ctorConditionallyReturns (mut i32) (i32.const 0))
(global $constructor/ctorAllocates (mut i32) (i32.const 0))
(global $constructor/ctorConditionallyAllocates (mut i32) (i32.const 0))

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Nice

global.set $~argumentsLength
i32.const 2
global.set $~argumentsLength
)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This fixture apparently evaporated entirely

i32.const 3
i32.const 1
call $function-types/doAddWithFn<i32>
call_indirect (type $i32_i32_=>_i32)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is unfortunate, perhaps a regression?

@MaxGraey MaxGraey Apr 15, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm, perhaps need some extra tune of passes pipeline. Will try it after this pr

@MaxGraey MaxGraey Apr 15, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I get the point!
$function-types/doAddWithFn<i32> was:

(func $function-types/doAddWithFn<i32> (; 5 ;) (param $0 i32) (param $1 i32) (param $2 i32) (result i32)
  i32.const 2
  global.set $~argumentsLength
  local.get $0
  local.get $1
  local.get $2
  call_indirect (type $i32_i32_=>_i32)
 )

after current optimizations it became:

(func $function-types/doAddWithFn<i32> (; 5 ;) (param $0 i32) (param $1 i32) (param $2 i32) (result i32)
  call_indirect (type $i32_i32_=>_i32)
)

and inlined so everything alright!

PS Perhaps make sense add one extra directize pass for further optimization. Will investigate later.

@jtenner jtenner left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I have so much to learn. Just had a question.

(func $~start (; 1 ;)
i32.const 0
global.set $~argumentsLength
(func $~setArgumentsLength (param $0 i32)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isn't this a bug? I suppose if the binary never uses the global that's fine, but it seems like anyone inspecting the global would expect this function to modify it.

Just curious what this means.

@dcodeIO dcodeIO Apr 14, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Looks like the global is never read, so Binaryen figured it can remove it entirely. If there'd be inspection code, it would be read (somewhere, somewhen), but otherwise it's a blackbox anyway and should be fine to remove.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants