Skip to content

Revert bswap PRs (480 and 482)#501

Merged
mshinwell merged 2 commits intooxcaml:mainfrom
mshinwell:revert-bswap-prs
Feb 2, 2022
Merged

Revert bswap PRs (480 and 482)#501
mshinwell merged 2 commits intooxcaml:mainfrom
mshinwell:revert-bswap-prs

Conversation

@mshinwell
Copy link
Copy Markdown
Collaborator

These are producing the wrong answer (testcase to be provided shortly), possibly only with Flambda 2 -- being looked into now. There must be some disagreement about the semantics of these, so let's revert for the moment.

@mshinwell mshinwell added bug Something isn't working backend labels Feb 2, 2022
@mshinwell
Copy link
Copy Markdown
Collaborator Author

Testcase:

external ( = ) : int -> int -> bool = "%equal"
external int32_of_int : int -> int32 = "%int32_of_int"
external int32_to_int : int32 -> int = "%int32_to_int"
external int64_to_int : int64 -> int = "%int64_to_int"
external bswap32 : int32 -> int32 = "%bswap_int32"
external unsafe_get32 : bytes -> int -> int32 = "%caml_bytes_get32u"
external unsafe_set32 : bytes -> int -> int32 -> unit = "%caml_bytes_set32u"
let buf = Bytes.create 32
let bin_read_network32_int buf ~pos = unsafe_get32 buf pos |> bswap32 |> int32_to_int
let bin_write_network32_int buf ~pos n =
  unsafe_set32 buf pos (n |> int32_of_int |> bswap32)
;;
let () =
  let n = -2147483648L |> int64_to_int in
  bin_write_network32_int buf ~pos:0 n;
  let n' = bin_read_network32_int buf ~pos:0 in
  assert (n = n')
;;

@mshinwell mshinwell merged commit 52cd50e into oxcaml:main Feb 2, 2022
mshinwell added a commit that referenced this pull request Feb 2, 2022
@lukemaurer
Copy link
Copy Markdown
Contributor

Tested with closure and flambda1 (before this was merged); both succeeded.

gretay-js added a commit that referenced this pull request Feb 2, 2022
gretay-js added a commit that referenced this pull request Feb 16, 2022
* Revert "Revert bswap PRs (480 and 482) (#501)"

This reverts commit 52cd50e.

* Test for bswap32

* Add more test inputs

* Refactor test

* Add more tests
Gbury pushed a commit to Gbury/flambda-backend that referenced this pull request Feb 17, 2022
* Revert "Revert bswap PRs (480 and 482) (oxcaml#501)"

This reverts commit 52cd50e.

* Test for bswap32

* Add more test inputs

* Refactor test

* Add more tests
mshinwell pushed a commit that referenced this pull request Mar 3, 2022
* Revert "Revert bswap PRs (480 and 482) (#501)"

This reverts commit 52cd50e.

* Test for bswap32

* Add more test inputs

* Refactor test

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

Labels

backend bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants