Skip to content
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

[patch] allow propagation of Sys.big_endian in native code #5769

Closed
vicuna opened this Issue Oct 1, 2012 · 6 comments

Comments

Projects
None yet
2 participants
@vicuna
Copy link
Collaborator

commented Oct 1, 2012

Original bug ID: 5769
Reporter: @chambart
Assigned to: @chambart
Status: closed (set by @xavierleroy on 2015-07-24T08:38:57Z)
Resolution: fixed
Priority: normal
Severity: feature
Version: 4.00.0
Fixed in version: 4.01.0
Category: back end (clambda to assembly)
Tags: patch
Monitored by: @chambart @hcarty

Bug description

When writing machine specific code like

let read_int32_little_endian v n =
let v = raw_read_int32 v n in
if Sys.big_endian
then swap_bytes v
else v

It would be prettier to know that the compiler cleans the test rather than having to use a preprocessor for that.

The patch provided add a primitive %big_endian of type unit -> bool that calls a C stub in byte code and is simplified in native code

File attachments

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 1, 2012

Comment author: @chambart

The first patch had spurious trailing white space removal cleaned in the second patch

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 2, 2012

Comment author: @chambart

Added a patch allowing propagation of other compile time constants.

It makes things like

let int_of_uint16 i =
let n = Sys.word_size - 17 in
(i lsl n) asr n

more efficient.

What other constant could be added ?

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 2, 2012

Comment author: @johnwhitington

Presumably Nativeint.size:

let sign_extend l n =
let shift = Nativeint.size - 1 - l in
(n lsl shift) asr shift

Although I suppose the gain is smaller here because l is non-constant.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 8, 2012

Comment author: @chambart

It is not needed to change Nativeint.size because it is computed using Sys.bigendian.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 9, 2012

Comment author: @chambart

Add version updated for current trunk.
Separate patch in 2 parts: to apply before and after bootstraping

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 17, 2013

Comment author: @chambart

Commited to 4.01

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.