JIT: Stop representing local addresses as TYP_BYREF

[jakobbotsch]

The JIT today represents local addresses as TYP_BYREF, in particular for implicit byrefs. However, after speaking to @jkotas, we believe that implicit byrefs cannot actually point to heap today. Previously this may have been the case for remoting/reflection.
We should update the JIT to uniformly treat all local addresses as TYP_I_IMPL and to type implicit byrefs as TYP_I_IMPL, which should remove some GC reporting and write barriers we emit today.

We should also be able to get rid of impBashVarAddrsToI and various places that we retype local addresses as TYP_I_IMPL.

Return buffers should stay TYP_BYREF typed as the JIT itself will pass heap pointers as return buffers sometimes.

The following section in clr-abi should also be updated:
https://github.com/dotnet/runtime/blob/3a3692029a2a038581fdc8c1c6496e0506a7e0c7/docs/design/coreclr/botr/clr-abi.md#amd64-only-by-value-value-types

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

Issue Details

---

The JIT today represents local addresses as TYP_BYREF, in particular for implicit byrefs. However, after speaking to @jkotas, we believe that implicit byrefs cannot actually point to heap today. Previously this may have been the case for remoting/reflection.
We should update the JIT to uniformly treat all local addresses as TYP_I_IMPL and to type implicit byrefs as TYP_I_IMPL, which should remove some GC reporting and write barriers we emit today.

We should also be able to get rid of impBashVarAddrsToI and various places that we retype local addresses as TYP_I_IMPL.

Return buffers should stay TYP_BYREF typed as the JIT itself will pass heap pointers as return buffers sometimes.

The following section in clr-abi should also be updated:
https://github.com/dotnet/runtime/blob/3a3692029a2a038581fdc8c1c6496e0506a7e0c7/docs/design/coreclr/botr/clr-abi.md#amd64-only-by-value-value-types

Author:	jakobbotsch
Assignees:	-
Labels:	area-CodeGen-coreclr, untriaged
Milestone:	-

[jakobbotsch]

I have a branch where I started doing the work: https://github.com/dotnet/runtime/compare/main...jakobbotsch:runtime:type-addresses-of-locals-unmanaged?expand=1

Likely this would be easier when GT_ADDR is removed.

cc @dotnet/jit-contrib

[BruceForstall]

However, after speaking to @jkotas, we believe that implicit byrefs cannot actually point to heap today.

Is there a way to validate this believe? E.g., create an instrumented JIT that checks the addresses of all implicit byrefs and run our test suite?

[jakobbotsch]

Is there a way to validate this believe? E.g., create an instrumented JIT that checks the addresses of all implicit byrefs and run our test suite?

Yeah, seems like that wouldn't be too hard to do. It would probably be a good idea to validate it before committing too much to this.

If it isn't the case we are probably skating on thin ice, however, since Roslyn allows taking the address of parameters as unmanaged pointers. Not sure how this was reconciled in the past (maybe things were always pinned but write barriers were still needed for managed types, and TYP_BYREF is necessary for this?)

[jkotas]

I have looked at the history. We had number of issues around this in the original x64 port that were fixed later. I see the last set of fixes in 2009. If this text was written before 2009, it would explain where it came from.

[jakobbotsch]

Somewhat related is that can get some odd-typed trees today when inlining is involved:

STMT00051 ( 0x000[E-] ... ??? ) <- INL02 @ 0x000[E-] <- INLRT @ 0x0A1[E-]
               [000177] -A---------                         ▌  ASG       long  
               [000176] D------N---                         ├──▌  LCL_VAR   long   V10 tmp1         
               [000175] -----------                         └──▌  SUB       byref 
               [000173] -----------                            ├──▌  FIELD_ADDR byref  SequentialLayoutMinPacking`1[ushort]:_byte
               [000174] -----------                            │  └──▌  LCL_VAR_ADDR byref  V04 loc1         
               [000172] -----------                            └──▌  LCL_VAR_ADDR long   V04 loc1         

This is produced from

runtime/src/tests/Interop/StructPacking/StructPacking.cs

Lines 1665 to 1668 in e579ccb

	internal static int OffsetOf<T, U>(ref T origin, ref U target)
	{
	return Unsafe.ByteOffset(ref origin, ref Unsafe.As<U, T>(ref target)).ToInt32();
	}

as part of inlining. The fact that the result of the SUB is TYP_BYREF seems moderately dangerous. As part of retyping local addresses as TYP_I_IMPL we might need to investigate retyping inline arguments that are directly substituted in some cases.

[jakobbotsch]

I probably won't have time to do the full cleanup in 8.0, but we can at least get rid of write barriers for these cases.