dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.27k stars 4.73k forks source link

JIT: Forward substitution does not handle promoted structs #94994

Open jakobbotsch opened 11 months ago

jakobbotsch commented 11 months ago

For example:

***** BB01
STMT00016 ( 0x000[E-] ... ??? )
               [000046] DA---------                         ▌  STORE_LCL_VAR struct<FieldsVsPropsTests+MyVec3Props, 12>(P) V04 tmp1         
                                                            ▌    float  field V04.<X>k__BackingField (fldOffset=0x0) -> V19 tmp16        
                                                            ▌    float  field V04.<Y>k__BackingField (fldOffset=0x4) -> V20 tmp17        
                                                            ▌    float  field V04.<Z>k__BackingField (fldOffset=0x8) -> V21 tmp18        
               [000000] -----------                         └──▌  LCL_VAR   struct<FieldsVsPropsTests+MyVec3Props, 12>(P) V01 arg0         
                                                               ▌    float  field V01.<X>k__BackingField (fldOffset=0x0) -> V13 tmp10         (last use)
                                                               ▌    float  field V01.<Y>k__BackingField (fldOffset=0x4) -> V14 tmp11         (last use)
                                                               ▌    float  field V01.<Z>k__BackingField (fldOffset=0x8) -> V15 tmp12         (last use)

***** BB01
STMT00017 ( 0x000[E-] ... ??? )
               [000047] DA---------                         ▌  STORE_LCL_VAR struct<FieldsVsPropsTests+MyVec3Props, 12>(P) V05 tmp2         
                                                            ▌    float  field V05.<X>k__BackingField (fldOffset=0x0) -> V22 tmp19        
                                                            ▌    float  field V05.<Y>k__BackingField (fldOffset=0x4) -> V23 tmp20        
                                                            ▌    float  field V05.<Z>k__BackingField (fldOffset=0x8) -> V24 tmp21        
               [000001] -----------                         └──▌  LCL_VAR   struct<FieldsVsPropsTests+MyVec3Props, 12>(P) V02 arg1         
                                                               ▌    float  field V02.<X>k__BackingField (fldOffset=0x0) -> V16 tmp13         (last use)
                                                               ▌    float  field V02.<Y>k__BackingField (fldOffset=0x4) -> V17 tmp14         (last use)
                                                               ▌    float  field V02.<Z>k__BackingField (fldOffset=0x8) -> V18 tmp15         (last use)

***** BB01
STMT00007 ( INL01 @ ??? ... ??? ) <- INLRT @ 0x000[E-]
               [000019] DA---------                         ▌  STORE_LCL_VAR float  V07 tmp4         
               [000016] -----------                         └──▌  ADD       float 
               [000051] -----------                            ├──▌  LCL_VAR   float  V19 tmp16         (last use)
               [000055] -----------                            └──▌  LCL_VAR   float  V22 tmp19         (last use)

***** BB01
STMT00011 ( INL01 @ ??? ... ??? ) <- INLRT @ 0x000[E-]
               [000030] DA---------                         ▌  STORE_LCL_VAR float  V09 tmp6         
               [000027] -----------                         └──▌  ADD       float 
               [000059] -----------                            ├──▌  LCL_VAR   float  V20 tmp17         (last use)
               [000063] -----------                            └──▌  LCL_VAR   float  V23 tmp20         (last use)

***** BB01
STMT00021 ( INL01 @ ??? ... ??? ) <- INLRT @ 0x000[E-]
               [000083] DA---------                         ▌  STORE_LCL_VAR float  V12 tmp9         
               [000038] -----------                         └──▌  ADD       float 
               [000067] -----------                            ├──▌  LCL_VAR   float  V21 tmp18         (last use)
               [000071] -----------                            └──▌  LCL_VAR   float  V24 tmp21         (last use)

It would be possible to substitute the field uses and get rid of both of the struct copies... but it does not look like a straightforward extension to forward sub. It might be easier with physical promotion which will have decomposed the struct copies already, but even so we would need to handle forward sub across multiple statements.

Code example from @BladeWise We get containment in the field case, but not in the property case that ends up with IR like the above. ```csharp using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using static System.Runtime.CompilerServices.MethodImplOptions; public static class FieldsVsPropsTests { public static MyVec3Fields TestAdd(MyVec3Fields l, MyVec3Fields r) => MyVec3Fields.AddScalar(l, r); public static MyVec3Props TestAdd(MyVec3Props l, MyVec3Props r) => MyVec3Props.AddScalar(l, r); [StructLayout(LayoutKind.Sequential)] public struct MyVec3Fields(float x, float y, float z) { [MethodImpl(AggressiveInlining)] public static MyVec3Fields AddScalar(MyVec3Fields l, MyVec3Fields r) => new(l.X + r.X, l.Y + r.Y, l.Z + r.Z); public float X = x, Y = y, Z = z; } [StructLayout(LayoutKind.Sequential)] public struct MyVec3Props(float x, float y, float z) { [MethodImpl(AggressiveInlining)] public static MyVec3Props AddScalar(MyVec3Props l, MyVec3Props r) => new(l.X + r.X, l.Y + r.Y, l.Z + r.Z); public float X { [MethodImpl(AggressiveInlining)] get; [MethodImpl(AggressiveInlining)] set; } = x; public float Y { [MethodImpl(AggressiveInlining)] get; [MethodImpl(AggressiveInlining)] set; } = y; public float Z { [MethodImpl(AggressiveInlining)] get; [MethodImpl(AggressiveInlining)] set; } = z; } } ```
ghost commented 11 months ago

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

Issue Details
For example: ```scala ***** BB01 STMT00016 ( 0x000[E-] ... ??? ) [000046] DA--------- ▌ STORE_LCL_VAR struct(P) V04 tmp1 ▌ float field V04.k__BackingField (fldOffset=0x0) -> V19 tmp16 ▌ float field V04.k__BackingField (fldOffset=0x4) -> V20 tmp17 ▌ float field V04.k__BackingField (fldOffset=0x8) -> V21 tmp18 [000000] ----------- └──▌ LCL_VAR struct(P) V01 arg0 ▌ float field V01.k__BackingField (fldOffset=0x0) -> V13 tmp10 (last use) ▌ float field V01.k__BackingField (fldOffset=0x4) -> V14 tmp11 (last use) ▌ float field V01.k__BackingField (fldOffset=0x8) -> V15 tmp12 (last use) ***** BB01 STMT00017 ( 0x000[E-] ... ??? ) [000047] DA--------- ▌ STORE_LCL_VAR struct(P) V05 tmp2 ▌ float field V05.k__BackingField (fldOffset=0x0) -> V22 tmp19 ▌ float field V05.k__BackingField (fldOffset=0x4) -> V23 tmp20 ▌ float field V05.k__BackingField (fldOffset=0x8) -> V24 tmp21 [000001] ----------- └──▌ LCL_VAR struct(P) V02 arg1 ▌ float field V02.k__BackingField (fldOffset=0x0) -> V16 tmp13 (last use) ▌ float field V02.k__BackingField (fldOffset=0x4) -> V17 tmp14 (last use) ▌ float field V02.k__BackingField (fldOffset=0x8) -> V18 tmp15 (last use) ***** BB01 STMT00007 ( INL01 @ ??? ... ??? ) <- INLRT @ 0x000[E-] [000019] DA--------- ▌ STORE_LCL_VAR float V07 tmp4 [000016] ----------- └──▌ ADD float [000051] ----------- ├──▌ LCL_VAR float V19 tmp16 (last use) [000055] ----------- └──▌ LCL_VAR float V22 tmp19 (last use) ***** BB01 STMT00011 ( INL01 @ ??? ... ??? ) <- INLRT @ 0x000[E-] [000030] DA--------- ▌ STORE_LCL_VAR float V09 tmp6 [000027] ----------- └──▌ ADD float [000059] ----------- ├──▌ LCL_VAR float V20 tmp17 (last use) [000063] ----------- └──▌ LCL_VAR float V23 tmp20 (last use) ***** BB01 STMT00021 ( INL01 @ ??? ... ??? ) <- INLRT @ 0x000[E-] [000083] DA--------- ▌ STORE_LCL_VAR float V12 tmp9 [000038] ----------- └──▌ ADD float [000067] ----------- ├──▌ LCL_VAR float V21 tmp18 (last use) [000071] ----------- └──▌ LCL_VAR float V24 tmp21 (last use) ``` It would be possible to substitute the field uses and get rid of both of the struct copies... but it does not look like a straightforward extension to forward sub. It might be easier with physical promotion which will have decomposed the struct copies already, but even so we would need to handle forward sub across multiple statements.
Code example from @BladeWise ```csharp using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using static System.Runtime.CompilerServices.MethodImplOptions; public static class FieldsVsPropsTests { public static MyVec3Fields TestAdd(MyVec3Fields l, MyVec3Fields r) => MyVec3Fields.AddScalar(l, r); public static MyVec3Props TestAdd(MyVec3Props l, MyVec3Props r) => MyVec3Props.AddScalar(l, r); [StructLayout(LayoutKind.Sequential)] public struct MyVec3Fields(float x, float y, float z) { [MethodImpl(AggressiveInlining)] public static MyVec3Fields AddScalar(MyVec3Fields l, MyVec3Fields r) => new(l.X + r.X, l.Y + r.Y, l.Z + r.Z); public float X = x, Y = y, Z = z; } [StructLayout(LayoutKind.Sequential)] public struct MyVec3Props(float x, float y, float z) { [MethodImpl(AggressiveInlining)] public static MyVec3Props AddScalar(MyVec3Props l, MyVec3Props r) => new(l.X + r.X, l.Y + r.Y, l.Z + r.Z); public float X { [MethodImpl(AggressiveInlining)] get; [MethodImpl(AggressiveInlining)] set; } = x; public float Y { [MethodImpl(AggressiveInlining)] get; [MethodImpl(AggressiveInlining)] set; } = y; public float Z { [MethodImpl(AggressiveInlining)] get; [MethodImpl(AggressiveInlining)] set; } = z; } } ```
Author: jakobbotsch
Assignees: -
Labels: `area-CodeGen-coreclr`
Milestone: -