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

Make ScLang's integer 64bits #6242

Conversation

JordanHendersonMusic
Copy link
Contributor

@JordanHendersonMusic JordanHendersonMusic commented Mar 20, 2024

Purpose and Motivation

More numbers good.

There should be no breaking changes.

This is currently more of a test to see what the CI makes of it.

Types of changes

The slot already had a int64 in, this just updates everything else (mostly function signatures) and tries to keep the old behaviour where possible.

There are many many places where integers are just cast around willy nilly, no attempt is made to fix this here, but should happen in the future — I have tried to make some of these casts explicit.

To-do list

  • Code is tested
  • All tests are passing
  • Updated documentation
  • This PR is ready for review

@JordanHendersonMusic JordanHendersonMusic marked this pull request as draft March 20, 2024 18:39
@JordanHendersonMusic
Copy link
Contributor Author

Does anyone know why the CI failing to pull in the git submodules?

@JordanHendersonMusic
Copy link
Contributor Author

Just closing this because I no longer have time to play with it.

@telephon
Copy link
Member

telephon commented Apr 5, 2024

It is not good to make integer 64 bit because it prevents a possible future optimisation. We should just use integer floats if we need higher numbers.

@smoge
Copy link
Contributor

smoge commented Apr 5, 2024

It is not good to make integer 64 bit because it prevents a possible future optimisation. We should just use integer floats if we need higher numbers.

At least on x86_64, there is no performance difference. It won't get faster nor worse. Although there are 32bit computers around. This change would have to allow different compilations. I think it hasn't been done because it's a massive and repetitive work.

@JordanHendersonMusic
Copy link
Contributor Author

JordanHendersonMusic commented Apr 5, 2024

Just for reference, here are some odd things I found...

  1. Int already is 64bit on 64bit machines

  2. So is char?

    int64 c; /* char */

  3. 32bit is enforced by casting through the getters and setter for the slot, but only in some cases on some architectures
    32bit setter

    inline void SetRaw(PyrSlot* slot, int val) {

    This setter seems problematic because long is 64 on unix and 32 on windows (https://en.cppreference.com/w/cpp/language/types)
    inline void SetRaw(PyrSlot* slot, long val) {

  4. Bit shifting is weird

1 << 30 == 1073741824 // correct
1 << 31 == -2147483648 // correct but (I think) undefined behaviour according to the C++ spec
1 << 31 - 1 == 2147483647 // correct
1 << 36 == 0 // okay just sets higher bits to zero, standard C rules
1 << 60 == 0 
1 << 64  == 1 /// hmmm....
1 << 65  == 2

I think this is because of the long issue or some odd implicit casting, and might not occur on windows?
Left shift uses long

long ia = slotRawInt(a);

@smoge
Copy link
Contributor

smoge commented Apr 5, 2024

1 << 30 == 1073741824 // correct
1 << 31 == -2147483648 // correct but (I think) undefined behaviour according to the C++ spec
1 << 31 - 1 == 2147483647 // correct
1 << 36 == 0 // okay just sets higher bits to zero, standard C rules
1 << 60 == 0

This is the expected behavior. Tthe leftmost bit in a signed integer represents its sign

EDIT: I wonder what further bitwise shifting is actually doing. Is it specific to sclang or also cpp?

(Maybe there was some work to transition to int64? Would be informative to see the git dates)

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.

None yet

3 participants