Skip to content

[OptimizeForJS] Avoid 64-bit integer storage if it's possible #4083

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

MaxGraey
Copy link
Contributor

@MaxGraey MaxGraey commented Aug 14, 2021

i64.store(x, C)   =>   f64.store(x, C'),   
     where C' <- reinterpret<f64>(C),
           C' != NaN && !is_subnormal(C')

it will lower to one HEAP64[ptr] = C' instead two writes to HEAP32

Fix: emscripten-core/emscripten#13365

@MaxGraey MaxGraey marked this pull request as ready for review August 14, 2021 16:48
@MaxGraey
Copy link
Contributor Author

MaxGraey commented Aug 16, 2021

Fuzzer found this edge case:

!
Wasm2JS (before/after) comparison error, expected to have '[fuzz-exec] calling func_48
[fuzz-exec] note result: func_48 => -6996475.0
[fuzz-exec] calling func_48_invoker
[LoggingExternalInterface logging 1719783932]
[fuzz-exec] calling func_49_invoker
[LoggingExternalInterface logging 1719783932]' == '[fuzz-exec] calling func_48
[fuzz-exec] note result: func_48 => -6996475.0
[fuzz-exec] calling func_48_invoker
[LoggingExternalInterface logging 1359052027]
[fuzz-exec] calling func_49_invoker
[LoggingExternalInterface logging 1359052027]', diff:

--- expected

+++ actual

@@ -1,6 +1,6 @@

 [fuzz-exec] calling func_48
 [fuzz-exec] note result: func_48 => -6996475.0
 [fuzz-exec] calling func_48_invoker
-[LoggingExternalInterface logging 1719783932]
+[LoggingExternalInterface logging 1359052027]
 [fuzz-exec] calling func_49_invoker
-[LoggingExternalInterface logging 1719783932]
+[LoggingExternalInterface logging 1359052027]

================================================================================

seed: 1387706149137627325

And reduction example looks like this:

(module
 (type $i32_=>_none (func (param i32)))
 (type $none_=>_none (func))
 (type $none_=>_i32 (func (result i32)))
 (import "fuzzing-support" "log-i32" (func $fimport$0 (param i32)))
 (memory $0 16 17)
 (export "func_46" (func $291))
 (export "func_49_invoker" (func $74))
 (func $74
  (if
   (i64.eqz
    (i64.load16_u align=1
     (i32.const 4)
    )
   )
   (call $fimport$0
    (i32.const -59)
   )
  )
 )
 (func $291 (result i32)
  (i64.store
   (i32.const 4)
   (i64.const 126)
  )
  (i32.const 0)
 )
)

However 126 should be valid 6.23e-322. But fuzzer can't interpret this right. Hmm... May be we should avoid subnormals

@MaxGraey
Copy link
Contributor Author

MaxGraey commented Aug 16, 2021

it seems we should skip subnormals as well sue to non-determinism on some platforms

@MaxGraey
Copy link
Contributor Author

Fuzzed: ITERATION: 50171

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.

Wasm2JS double heap assignments get split into two i32 assignments.
1 participant