-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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 SROA-friendly struct type declarations #59007
Conversation
5d0d344
to
1984c62
Compare
int chunk = size_align.align < TARGET_SIZEOF_VOID_P ? size_align.align : TARGET_SIZEOF_VOID_P; | ||
for (; chunk > 0; chunk = chunk >> 1) { | ||
for (; (bytes + chunk) <= size_align.size; bytes += chunk) { | ||
eltypes [size] = LLVMIntType (chunk * 8); |
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.
@imhameed, I'm curious, why do we use these element types that don't have anything to do with the actual elements of the original Mono type (and presumably we bitcast at the use sites?). Why not use the field layout of the Mono type to create the LLVM struct? (ie: for a mono class int x; double y;
represent it as int32 x; int32 padding; double y
instead of I think int32 el0; int32 el1; int32 el2; int32 el3
)?
Is it to limit the number of unique llvm types that we have to create?
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.
We were only doing it for simplicity i think.
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.
@imhameed, I'm curious, why do we use these element types that don't have anything to do with the actual elements of the original Mono type (and presumably we bitcast at the use sites?). Why not use the field layout of the Mono type to create the LLVM struct? (ie: for a mono class
int x; double y;
represent it asint32 x; int32 padding; double y
instead of I thinkint32 el0; int32 el1; int32 el2; int32 el3
)?
Simplicity, mostly. The diff is small. I'd like to more accurately represent IL types in our LLVM IR in the future.
Reads and writes of individual fields are done via normal loads/stores, pointer coercion, and getelementpointer.
@imhameed - once all validations are complete and we are confident of the change, please indicate so, and we can start backport to release\6.0 fyi @marek-safar |
50ec2c7
to
adf8918
Compare
LLVM's SROA can decompose loads and stores of aggregate type into a sequence of aggregate-element-typed loads and stores. Before this change, Mono translates .NET-level value types into LLVM IR-level structs containing nothing but `i8` elements. When a value type field has reference type, and a value of this value type is copied using a `memcpy` intrinsic or an LLVM IR load followed by a store, LLVM will emit code that loads managed references in multiple byte-sized fragments before reconstructing the original pointer using a sequence of ALU ops. This causes sgen to fail to pin the referent. This change works around this by translating value types to LLVM IR structs with pointer-sized fields. Packed value types with non-standard alignment will be translated into LLVM IR structs containing alignment-sized fields. Note that this does not completely guarantee that the code we generate will respect sgen's requirements. No specific guarantees are provided about the translation of non-atomic LLVM IR loads and stores to machine code. And we'll need some alternative means (perhaps a special `gc_copy_unaligned` runtime call or similar) to copy packed or misaligned value types that contain managed references. For stronger LLVM IR-level guarantees, we'll want to make use of unordered atomic loads and stores and unordered atomic memcpy, but that work is out of scope for this change. Fixes dotnet#58062, but see the previous paragraph for caveats. See: - https://github.com/dotnet/llvm-project/blob/release/11.x/llvm/lib/Transforms/Scalar/SROA.cpp#L3371-L3388 - https://github.com/dotnet/llvm-project/blob/release/11.x/llvm/lib/Transforms/Scalar/SROA.cpp#L3327-L3340
adf8918
to
0b9b0e7
Compare
/backport to release/6.0 |
Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1230643469 |
@imhameed backporting to release/6.0 failed, the patch most likely resulted in conflicts: $ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: [mono] Use SROA-friendly struct type declarations
Applying: Reenable CscBench test
Using index info to reconstruct a base tree...
M src/tests/issues.targets
Falling back to patching base and 3-way merge...
Auto-merging src/tests/issues.targets
CONFLICT (content): Merge conflict in src/tests/issues.targets
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0002 Reenable CscBench test
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128 Please backport manually! |
"runtime (Mono llvmfullaot Pri0 Runtime Tests Run Linux arm64 release)" is timing out right now because the backing Helix pool (ubuntu.1804.armarch.open) currently has over 15,000 jobs queued with a wait time of over 3 hours. |
LLVM's SROA can decompose loads and stores of aggregate type into a
sequence of aggregate-element-typed loads and stores. Before this
change, Mono translated .NET-level value types into LLVM IR-level
structs containing nothing but
i8
elements.When a value type field has reference type, and a value of this value
type is copied using a
memcpy
intrinsic or an LLVM IR load followed bya store, LLVM will emit code that loads managed references in multiple
byte-sized fragments before reconstructing the original pointer using a
sequence of ALU ops. This causes sgen to fail to pin the referent.
This change works around this by translating value types to LLVM IR
structs with pointer-sized fields. Packed value types with non-standard
alignment will be translated into LLVM IR structs containing
alignment-sized fields.
Note that this does not completely guarantee that the code we generate
will respect sgen's requirements. No specific guarantees are provided
about the translation of non-atomic LLVM IR loads and stores to machine
code. And we'll need some alternative means (perhaps a special
gc_copy_unaligned
runtime call or similar) to copy packed ormisaligned value types that contain managed references. For stronger
LLVM IR-level guarantees, we'll want to make use of unordered atomic
loads and stores and unordered atomic memcpy, but that work is out of
scope for this change.
Fixes #58062, but see the previous paragraph for caveats.
See: