Skip to content

Conversation

@lateralusX
Copy link
Member

ep_rt_atomic_compare_exchange_size_t was always truncated to gint32 on Mono 64-bit platforms. This commit use 32 or 64-bit cas version depending on SIZEOF_SIZE_T. NOTE, CoreCLR used a template function for this and already picks 32 or 64 bit version depending on size of size_t.

ep_rt_atomic_compare_exchange_size_t is currently used to reflect size of EventPipe buffer manager buffers and will most likely never need more than 2GB of data, but better to fix it and inline with CoreCLR shim implementation.

ep_rt_atomic_compare_exchange_size_t was always truncated to gint32 on
Mono 64-bit platforms. This commit use 32 or 64-bit cas version
depending on SIZEOF_SIZE_T. NOTE, CoreCLR used a template function for
this and already picks 32 or 64 bit version depending on size of size_t.

ep_rt_atomic_compare_exchange_size_t is currently used to reflect size
of EventPipe buffer manager buffers and will most likely never need
more than 2GB of data, but better to fix it and inline with CoreCLR shim
implementation.
@ghost
Copy link

ghost commented Aug 26, 2021

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@alexrp
Copy link
Contributor

alexrp commented Aug 26, 2021

I don't know how relevant this is today, but note that we had some issues with 64-bit CAS on some platforms in the past:

@lateralusX
Copy link
Member Author

I don't know how relevant this is today, but note that we had some issues with 64-bit CAS on some platforms in the past:

@alexrp thank you for feedback. Note that it will only be used on 64-bit platforms and just looking at the fallbacks in atomic.h/atomic.c it seems that the fallbacks won't be applied in that case.

@marek-safar marek-safar requested a review from lambdageek August 30, 2021 15:40
@lambdageek
Copy link
Member

/backport to release/6.0

@github-actions
Copy link
Contributor

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1182809965

@lambdageek lambdageek merged commit dbe32b5 into dotnet:main Aug 30, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Sep 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants