Skip to content

Conversation

@MaxGraey
Copy link
Contributor

@MaxGraey MaxGraey commented Nov 5, 2021

i64.extend_i32_u(i32.load8_u(x))   ->  i64.load8_u(x)
i64.extend_i32_u(i32.load16_u(x))  ->  i64.load16_u(x)

i64.extend_i32_s(i32.load8_u(x))   ->  i64.load8_u(x)
i64.extend_i32_s(i32.load16_u(x))  ->  i64.load16_u(x)
i64.extend_i32_s(i32.load8_s(x))   ->  i64.load8_s(x)
i64.extend_i32_s(i32.load16_s(x))  ->  i64.load16_s(x)

i64.extend_i32_u(i32.load(x)))  ->  i64.load32_u(x)
i64.extend_i32_s(i32.load(x)))  ->  i64.load32_s(x)

don't apply to

i64.extend_i32_u(i32.load8_s(x))   ->  skip
i64.extend_i32_u(i32.load16_s(x))  ->  skip
i64.extend_i32_s(i32.atomic.load(x))  ->  skip

@lexaknyazev
Copy link

Are you sure about these?

i64.extend_i32_u(i32.load8_s(x))   ->  i64.load8_s(x)
i64.extend_i32_u(i32.load16_s(x))  ->  i64.load16_s(x)

For example:

  • 0xFF is loaded by i32.load8_s as 0xFFFFFFFF and i64.extend_i32_u produces 0x00000000FFFFFFFF.
  • On the other hand, loading 0xFF with i64.load8_s returns 0xFFFFFFFFFFFFFFFF.

The same applies to the 16-bit case.

@MaxGraey
Copy link
Contributor Author

MaxGraey commented Nov 5, 2021

@lexaknyazev Yes, such pairs of operations we should avoid

@MaxGraey
Copy link
Contributor Author

MaxGraey commented Nov 5, 2021

Fuzzed ITERATION: 27789

@MaxGraey
Copy link
Contributor Author

MaxGraey commented Nov 5, 2021

@tlively @kripken could take a look please?

@MaxGraey MaxGraey requested a review from kripken November 11, 2021 13:25
Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

Nice, looks good!

Please fuzz the last version, unless you already have?

@MaxGraey
Copy link
Contributor Author

Ok, I'll refuzz this tomorrow

@MaxGraey
Copy link
Contributor Author

MaxGraey commented Nov 12, 2021

Hmm, I got this:

[wasm-validator error in function 57] unexpected true: atomic loads must be unsigned, on 
(i64.atomic.load32_s offset=22
 (i32.and
  (i32.const -66)
  (i32.const 15)
 )
)

for seed 16896761022808288108

Upd: Oh atomics doesn't have sign extended versions for loads!
Upd Upd: Fixed!

@MaxGraey
Copy link
Contributor Author

Refuzzed ITERATION: 17513

@MaxGraey MaxGraey requested a review from kripken November 12, 2021 15:52
@kripken kripken merged commit 5597d1c into WebAssembly:main Nov 12, 2021
@MaxGraey MaxGraey deleted the combine-load32-store32-zero-ext-64 branch November 12, 2021 22:06
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