Skip to content

py/obj: Verify floating point type is correct for OBJ_REPR_C. #9649

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

Closed
wants to merge 2 commits into from

Conversation

jimmo
Copy link
Member

@jimmo jimmo commented Oct 17, 2022

Prevents double-precision floats being enabled on 32-bit architectures where they will not fit into the mp_obj_t encoding.

Also reverts the Portenta board to the default representation as it uses double-precision floats.
@iabdalkader -- I'm not sure why the Portenta was set to OBJ_REPR_C, assuming to avoid heap-allocation of floats. If you want to keep that I guess we could switch the Portenta to use single-precision float instead?

Prevents double-precision floats being enabled on 32-bit architectures
where they will not fit into the mp_obj_t encoding.

Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
@iabdalkader
Copy link
Contributor

I'm not sure why the Portenta was set to OBJ_REPR_C, assuming to avoid heap-allocation of floats

Yes, there's a longer discussion here: #8547 (comment)

We don't need double precision, but we do need REPR_C. Can you please instead set it to single precision ?

@iabdalkader
Copy link
Contributor

@jimmo

Can you please instead set it to single precision ?

Actually I'm sending a PR to update the board config, will set it to single.

Using repr C is incompatible with double-precision floats on 32-bit arch.

Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
@jimmo jimmo force-pushed the repr-c-float-impl branch from 86fc52e to 83f3517 Compare October 17, 2022 13:42
@jimmo
Copy link
Member Author

jimmo commented Oct 17, 2022

OK sure thing. I've updated the branch, but can drop the commit later in the rebase if necessary.

@iabdalkader
Copy link
Contributor

It's fixed here too along with misc small fixes #9657

@iabdalkader
Copy link
Contributor

@dpgeorge Could you please merge this, because it includes a critical fix for Portenta board.

@dpgeorge dpgeorge added port-stm32 py-core Relates to py/ directory in source labels Oct 25, 2022
@dpgeorge
Copy link
Member

Thanks, these patches look good. Merged in b161abc and 5ee1cb2

@dpgeorge dpgeorge closed this Oct 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
port-stm32 py-core Relates to py/ directory in source
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants