-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
[mono] Use InlineArrayAttribute for ArgumentData and StackAllocatedByRefs #88149
[mono] Use InlineArrayAttribute for ArgumentData and StackAllocatedByRefs #88149
Conversation
…Refs Mono supports InlineArray
FYI @kotlarmilos |
Tagging subscribers to this area: @dotnet/area-system-reflection Issue DetailsMono supports InlineArray
|
There is one more src\libraries\System.Private.CoreLib\src\System\ParamsArray.cs |
src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBase.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBase.cs
Show resolved
Hide resolved
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Hi @lambdageek, this seems to have caused a severe regression on s390x: all CI tests are currently failing with this error:
I haven't looked into the root cause yet - it looks like this is failing only on s390x, not ppc64le, so maybe an endian issue? |
I see the following suspicious code in has_inline_array_attribute_value_func:
and later
If this is actually a 4-byte value in memory, then the |
And indeed the following patch fixes the problem for me:
Does this look reasonable to you? |
Maybe we should use one of our READ32 macros (sorry, I'm on a phone, can't look up the exact name) |
Looks like READ32 is only available/used inside the interpreter (src/mono/mini/interp), not in the rest of Mono. |
I've now opened a PR including this fix: #88417 |
Mono supports InlineArray