Use Unsafe.BitCast in System.Array
#116404
Conversation
Doesn't depend upon implementation specific behaviour, i.e. that data is properly aligned. Split from dotnet#113293.
github-actions
bot
added
the
needs-area-label
dotnet-policy-service
bot
added
the
community-contribution
| @@ -949,7 +949,7 @@ public static int BinarySearch(Array array, int index, int length, object? value | |||
| return (result >= 0) ? (index + result) : ~(index + ~result); | |||
|
|
|||
| static int GenericBinarySearch<T>(Array array, int adjustedIndex, int length, object value) where T : struct, IComparable<T> | |||
| => UnsafeArrayAsSpan<T>(array, adjustedIndex, length).BinarySearch(Unsafe.As<byte, T>(ref value.GetRawData())); | |||
| => UnsafeArrayAsSpan<T>(array, adjustedIndex, length).BinarySearch(Unsafe.ReadUnaligned<T>(ref value.GetRawData())); | |||
There was a problem hiding this comment.
aren't array's elements always aligned to T here?
There was a problem hiding this comment.
doesn't hurt to be obviously correct, also it's 5 bytes less IL
There was a problem hiding this comment.
I wasn't sure about the data alignment of e.g. double on all 32 bit platforms.
There was a problem hiding this comment.
I wasn't sure about the data alignment of e.g.
doubleon all 32 bit platforms.
Good point, I guess since #115985 it's not guaranteed
There was a problem hiding this comment.
doesn't hurt to be obviously correct, also it's 5 bytes less IL
The problem of using ReadUnaligned where it's not needed is a theoretical serious performance regression on platforms where ReadUnaligned is implemented using memcpy-like routine (due to slow/faulty unaligned access)
There was a problem hiding this comment.
There is no alignment problem here.
doesn't hurt to be obviously correct
I think it is confusing to use ReadUnaligned in place when there are no alignment issues.
I guess since #115985 it's not guaranteed
doubles are not 8-byte aligned on x86 and you do not need to worry about it. Regular double loads cannot use instructions that except 8-byte alignment on x86.
There was a problem hiding this comment.
Could you please revert these unnecessary ReadUnaligned? The BitCast changes are good.
xtqqczze
changed the title
Unsafe.As in System.ArrayUnsafe.BitCast in System.Array
|
/ba-g timeouts |
Doesn't depend upon implementation specific behaviour, i.e. that data is properly aligned.
Split from #113293.