Avoid calling GC write barrier for byrefs

[markples]

Avoid calling CORINFO_HELP_ASSIGN_BYREF for byref locations. This is Jakob's original fix except that it uses the new Layout helpers. It's possible that there originally was confusion with the helper because it accepts a byref to a ref (not a byref to a byref).

This ends up converting some helper calls to movs for large structs (small structs use other instructions after #86820). Often these then combine into a larger rep movs, leading to decent code space wins.

Fixes #80086

[ghost]

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

---

null

Author:	markples
Assignees:	-
Labels:	area-CodeGen-coreclr
Milestone:	-

[markples]

@jakobbotsch It would be nice to get a compete fix for #80086 in. I have some questions though. (1) Are we willing to take the possible movs time regression on x64 now that it is limited to large structs? New asm diffs are running that are independent of the changes in my last PR. (2) Should I apply the same "byref means stack" logic here and avoid the GC barrier calls for large structs too? Again this would add more movs but they would also likely be folded into rep movs. (3) Should the size limit for the previous PR be removed?

My initial thoughts are (1) yes, but (2) I should try it in the next hour, and (3) no, at least not now - the expanded vs rep expansion can/should be reexamined separately if required.

[Some background to all of this is that we could rewrite the movs/rep movs logic, but I'm not going to have time to do that.]

cc @dotnet/jit-contrib
cc @shushanhf @clamp03 @t-mustafin @alpencolt

[jakobbotsch]

@jakobbotsch It would be nice to get a compete fix for #80086 in. I have some questions though. (1) Are we willing to take the possible movs time regression on x64 now that it is limited to large structs? New asm diffs are running that are independent of the changes in my last PR. (2) Should I apply the same "byref means stack" logic here and avoid the GC barrier calls for large structs too? Again this would add more movs but they would also likely be folded into rep movs. (3) Should the size limit for the previous PR be removed?

My initial thoughts are (1) yes, but (2) I should try it in the next hour, and (3) no, at least not now - the expanded vs rep expansion can/should be reexamined separately if required.

I agree with all of your thoughts -- we still have time to see impact on the micro benchmarks from these changes. You could also try to benchmark some of the cases and see if there is any noticeable difference.

[markples]

(2) experiment is in #89116. CI is broken for the moment so we won't get automatic results back right away.

[markples]

/azp run runtime

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[markples]

/azp run runtime

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[ghost]

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.