Skip to content
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

Tiered JIT changes behavior of code in the absence of explicit conv.r4 #98745

Closed
VictorNicollet opened this issue Feb 21, 2024 · 6 comments
Closed
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone

Comments

@VictorNicollet
Copy link

Description

A method is created with System.Reflection.Emit to be nearly equivalent to the C# code:

float Method(Func<double> f) => Math.Max(0.0f, (float) f());

The only difference being that the created method does not contain the explicit conv.r4 corresponding to the (float) coercion.

With option <TieredCompilation>true</TieredCompilation> the call Method(() => 2.0) returns 2.0f.

With option <TieredCompilation>false</TieredCompilation> the same call returns 0.0f.

We also observed a return value of 0.0f with <TieredCompilation>true</TieredCompilation> in a long-running program, so this may be dependent on the compilation tier, but I could not reduce it to a minimal example.

Reproduction Steps

using System;
using System.Reflection;
using System.Reflection.Emit;

namespace NoConvR4;

public static class Program
{
    private static Type CreateType(bool dumpIL)
    {
        var am = new AssemblyName { Name = "MaxSqrt" };
        var ab = AssemblyBuilder.DefineDynamicAssembly(am, AssemblyBuilderAccess.Run);
        var mb = ab.DefineDynamicModule("TheModule");
        var tb = mb.DefineType("TheType", TypeAttributes.Public);

        var tfun = typeof(Func<double>);
        var invoke = tfun.GetMethod("Invoke")!;
        var max = typeof(Math).GetMethod(
            "Max", 
            BindingFlags.Public | BindingFlags.Static, 
            [typeof(float), typeof(float)])!;

        var metb = tb.DefineMethod(
            "Run",
            MethodAttributes.Public | MethodAttributes.Static, 
            typeof(float),
            [tfun]);

        ILGenerator il = metb.GetILGenerator();

        il.Emit(OpCodes.Ldc_R4, 0.0f);
        il.Emit(OpCodes.Ldarg_0);
        il.Emit(OpCodes.Callvirt, invoke);
        il.Emit(OpCodes.Call, max);
        il.Emit(OpCodes.Ret);

        var t = tb.CreateType();

        if (dumpIL)
        {
            new Lokad.ILPack.AssemblyGenerator().GenerateAssembly(ab, "MaxSqrt.dll");
        }

        return t;
    }

    public static void Main(string[] args)
    {
        var tt = CreateType(dumpIL: false);

        var method = tt.GetMethod("Run", BindingFlags.Public | BindingFlags.Static)!;

        if (method.Invoke(null, new object[] { () => 2.0 }) is float f)
        {
            // prints 2 with tiered compilation, 0 without
            Console.WriteLine($"{f}");
        }
    }
}
<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net8.0</TargetFramework>
    <ImplicitUsings>disable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <TieredCompilation>true</TieredCompilation>
  </PropertyGroup>

  <ItemGroup>
    <!-- Used to dump the assembly, removing it does not change observed behavior -->
    <PackageReference Include="Lokad.ILPack" Version="0.2.0" />
  </ItemGroup>

</Project>

Expected behavior

My interpretation of ECMA-335 is that this should return 2.0f in all cases. More specifically, the implicit starg to pass the float64 value to Math.Max(float, float) should coerce the value to float32.

In terms of generated machine code, I would expect a vcvtsd2ss to be produced before the vmaxss instruction.

Actual behavior

When tiered compilation is disabled, the function returns 0.0f. The disassembly of the function shows that no vcvtsd2ss was emitted, and so the vmaxss interprets xmm0 to contain a float32 when it actually contains a float64.

00007FFA67EA8628  mov         eax,ecx  
00007FFA67EA862A  mov         rcx,qword ptr [rax+8]  
00007FFA67EA862E  call        qword ptr [rax+18h]  
00007FFA67EA8631  vxorps      xmm1,xmm1,xmm1  
00007FFA67EA8635  vmaxss      xmm1,xmm1,xmm0  
00007FFA67EA8639  vfixupimmss xmm1,xmm0,dword ptr [TheType.Run(System.Func`1<Double>)+030h (07FFA67EA8650h)],0  
00007FFA67EA8644  vmovaps     xmm0,xmm1  
00007FFA67EA8648  add         rsp,28h  
00007FFA67EA864C  ret  

Regression?

This behavior was not present on .NET 6, and was observed as part of our migration from .NET 6 to .NET 8.

Known Workarounds

We can generate the additional conv.r4 to force the coercion.

Configuration

Observed on Windows 10 with runtime Microsoft.NETCore.App 8.0.0, processor Intel i7-11850H

Observed on Ubuntu 20.04.6 LTS with runtime Microsoft.NETCore.App 8.0.2, processor AMD EPYC 7551

Other information

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Feb 21, 2024
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Feb 21, 2024
@huoyaoyuan huoyaoyuan added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Feb 21, 2024
@ghost
Copy link

ghost commented Feb 21, 2024

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

Issue Details

Description

A method is created with System.Reflection.Emit to be nearly equivalent to the C# code:

float Method(Func<double> f) => Math.Max(0.0f, (float) f());

The only difference being that the created method does not contain the explicit conv.r4 corresponding to the (float) coercion.

With option <TieredCompilation>true</TieredCompilation> the call Method(() => 2.0) returns 2.0f.

With option <TieredCompilation>false</TieredCompilation> the same call returns 0.0f.

We also observed a return value of 0.0f with <TieredCompilation>true</TieredCompilation> in a long-running program, so this may be dependent on the compilation tier, but I could not reduce it to a minimal example.

Reproduction Steps

using System;
using System.Reflection;
using System.Reflection.Emit;

namespace NoConvR4;

public static class Program
{
    private static Type CreateType(bool dumpIL)
    {
        var am = new AssemblyName { Name = "MaxSqrt" };
        var ab = AssemblyBuilder.DefineDynamicAssembly(am, AssemblyBuilderAccess.Run);
        var mb = ab.DefineDynamicModule("TheModule");
        var tb = mb.DefineType("TheType", TypeAttributes.Public);

        var tfun = typeof(Func<double>);
        var invoke = tfun.GetMethod("Invoke")!;
        var max = typeof(Math).GetMethod(
            "Max", 
            BindingFlags.Public | BindingFlags.Static, 
            [typeof(float), typeof(float)])!;

        var metb = tb.DefineMethod(
            "Run",
            MethodAttributes.Public | MethodAttributes.Static, 
            typeof(float),
            [tfun]);

        ILGenerator il = metb.GetILGenerator();

        il.Emit(OpCodes.Ldc_R4, 0.0f);
        il.Emit(OpCodes.Ldarg_0);
        il.Emit(OpCodes.Callvirt, invoke);
        il.Emit(OpCodes.Call, max);
        il.Emit(OpCodes.Ret);

        var t = tb.CreateType();

        if (dumpIL)
        {
            new Lokad.ILPack.AssemblyGenerator().GenerateAssembly(ab, "MaxSqrt.dll");
        }

        return t;
    }

    public static void Main(string[] args)
    {
        var tt = CreateType(dumpIL: false);

        var method = tt.GetMethod("Run", BindingFlags.Public | BindingFlags.Static)!;

        if (method.Invoke(null, new object[] { () => 2.0 }) is float f)
        {
            // prints 2 with tiered compilation, 0 without
            Console.WriteLine($"{f}");
        }
    }
}
<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net8.0</TargetFramework>
    <ImplicitUsings>disable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <TieredCompilation>true</TieredCompilation>
  </PropertyGroup>

  <ItemGroup>
    <!-- Used to dump the assembly, removing it does not change observed behavior -->
    <PackageReference Include="Lokad.ILPack" Version="0.2.0" />
  </ItemGroup>

</Project>

Expected behavior

My interpretation of ECMA-335 is that this should return 2.0f in all cases. More specifically, the implicit starg to pass the float64 value to Math.Max(float, float) should coerce the value to float32.

In terms of generated machine code, I would expect a vcvtsd2ss to be produced before the vmaxss instruction.

Actual behavior

When tiered compilation is disabled, the function returns 0.0f. The disassembly of the function shows that no vcvtsd2ss was emitted, and so the vmaxss interprets xmm0 to contain a float32 when it actually contains a float64.

00007FFA67EA8628  mov         eax,ecx  
00007FFA67EA862A  mov         rcx,qword ptr [rax+8]  
00007FFA67EA862E  call        qword ptr [rax+18h]  
00007FFA67EA8631  vxorps      xmm1,xmm1,xmm1  
00007FFA67EA8635  vmaxss      xmm1,xmm1,xmm0  
00007FFA67EA8639  vfixupimmss xmm1,xmm0,dword ptr [TheType.Run(System.Func`1<Double>)+030h (07FFA67EA8650h)],0  
00007FFA67EA8644  vmovaps     xmm0,xmm1  
00007FFA67EA8648  add         rsp,28h  
00007FFA67EA864C  ret  

Regression?

This behavior was not present on .NET 6, and was observed as part of our migration from .NET 6 to .NET 8.

Known Workarounds

We can generate the additional conv.r4 to force the coercion.

Configuration

Observed on Windows 10 with runtime Microsoft.NETCore.App 8.0.0, processor Intel i7-11850H

Observed on Ubuntu 20.04.6 LTS with runtime Microsoft.NETCore.App 8.0.2, processor AMD EPYC 7551

Other information

No response

Author: VictorNicollet
Assignees: -
Labels:

area-CodeGen-coreclr, untriaged

Milestone: -

@huoyaoyuan
Copy link
Member

It looks like invalid IL when the conversion is missing. In .NET Core the behavior of invalid IL is undefined.

What about the behavior of .NET Framework, both 32bit and 64bit? Is there any exception thrown?

@SingleAccretion
Copy link
Contributor

It looks like invalid IL when the conversion is missing.

The IL is valid - you can omit conversions between FP values quite freely in IL.

@VictorNicollet
Copy link
Author

I believe the IL to be valid. According to ECMA-335 §III.3.19 (describing instruction call):

The arguments are passed as though by implicit starg (§III.3.61) instructions, see Implicit argument coercion §III.1.6

And §III.3.61 (describing instruction starg):

Floating-point values are rounded from their native size (type F) to the size associated with the argument. (See §III.1.1.1, Numeric data types.)

@BruceForstall BruceForstall added this to the 9.0.0 milestone Feb 26, 2024
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Feb 26, 2024
@BruceForstall
Copy link
Member

@EgorBo PTAL

@EgorBo
Copy link
Member

EgorBo commented May 21, 2024

Looks like this no longer reproduces on Main. This specific one was fixed in #102098 by adding impImplicitR4orR8Cast for args

@EgorBo EgorBo closed this as completed May 21, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jun 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

No branches or pull requests

5 participants