More optimizations for pow of two and pos/neg one const on the right

[MaxGraey]

• introduce optimizePowerOf2UDiv which complete optimizePowerOf2URem / optimizePowerOf2Mul set.
• optimizePowerOf2UDiv and friends now define as template functions for handling i32 and i64 types.
• more simplifications for binary ops with negative one constant for rhs:
  • (integer)x * -1 ==> 0 - x
  • (signed)x % -1 ==> 0
  • (signed)x % 1 ==> 0
  • (uint32_t)x / -1 ==> x != -1
  • (unsigned)x > -1 ==> 0
  • (unsigned)x < -1 ==> x != -1
  • (unsigned)x <= -1 ==> 1
  • (signed)x <= -1 ==> (unsigned)x >> sizeof(bits) - 1

[kripken]

Could you give me advise how better to fix this on CI?

I think it's an unupdated test output? Building binaryen_js locally and running ./auto_update_tests.py binaryenjs (or without the last param, for all the suites) should do it.

[kripken]

Please add a summary in the PR description of what this does (we'll need it for the landed commit's description, and also for review it helps to read it before reading the code) - thanks!

[MaxGraey]

Done! Added description for current PR

[MaxGraey]

I think it's an unupdated test output? Building binaryen_js locally and running ./auto_update_tests.py binaryenjs (or without the last param, for all the suites) should do it.

Thanks! Done!

[kripken]

Nice work!

It might be better to have smaller PRs than this one, in general. But if it's hard to split up now, this one is ok.

In addition to the comments, please run the fuzzer for a while as I mentioned in another PR.

[kripken]

lgtm with minor comments fixed, and if you've fuzzed it for a while.

[kripken]

Running the fuzzer+reducer I found this:

(module
 (type $i32_f64_i32_f64_i32_i32_f64_f64_=>_none (func (param i32 f64 i32 f64 i32 i32 f64 f64)))
 (func $0 (param $0 i32) (param $1 f64) (param $2 i32) (param $3 f64) (param $4 i32) (param $5 i32) (param $6 f64) (param $7 f64)
  (if
   (i32.const 0)
   (block
    (drop
     (i64.extend_i32_s
      (i64.gt_u
       (i64.const 0)
       (i64.const -1)
      )
     )
    )
    (unreachable)
   )
   (nop)
  )
 )
)

That fails with -Oz with

wasm-opt: /home/azakai/Dev/2-binaryen/src/wasm/literal.cpp:407: wasm::Literal wasm::Literal::extendToSI64() const: Assertion `type == Type::i32' failed.

[MaxGraey]

Yeah, It's bug related to Asyncify and basically present on master
@kripken @aheejin can you confirm?

[MaxGraey]

So tested during 2.5 hours (last ITERATION: 25037) and seems everything ok. Only issue related to Asyncify which already existed before.

[kripken]

lgtm (and I did some fuzzing locally).

Does this have tests for everything? For example I don't think I saw a test for x * -1 ==> 0 - x on floats.

[MaxGraey]

Added more tests

[kripken]

As discussed in the other issue, it's very hard to review these complex PRs in parallel. Let's focus on one at a time, which I think is first your canonicalization PR. Once that is landed please ping me in the next PR after it.

[MaxGraey]

Got it!

[kripken]

(Btw, if it's not clear, that was for me personally - if other devs in this project have time to review other PRs in parallel that would be great of course.)

[tlively]

(Btw, if it's not clear, that was for me personally - if other devs in this project have time to review other PRs in parallel that would be great of course.)

I agree that focusing on one thing at a time is helpful. @kripken Do you want to review request me into some particular PR you haven't looked at yet so I can help balance the load?

[kripken]

Do you want to review request me into some particular PR you haven't looked at yet so I can help balance the load?

I don't have a particular one in mind (I haven't looked at them in detail) - feel free to pick any of them.

[MaxGraey]

Will refuzz tomorrow

[MaxGraey]

Ok, refuzzed until ITERATION: 29819 without issues.

Could somebody review / look at this PR again?

[tlively]

LGTM besides that one comment 👍

[MaxGraey]

Thanks!