Run on .NET 8 Arm64 seems to remove some parts of a conditional statement, probably only after Tier 1 JIT compilation

[Louis-Antoine-Blais-Morin]

Description

We have a code running that runs correctly on Windows x64 and that was running correctly on Arm64 with .NET 7.

Starting from .NET 8, we observed that one if statement does not seem to behave as written in the code.

Extract of the code is like this:

public void Select(Entity entity)
{
   if (ShouldSelectEntity(entity)
   {
      SelectEntity(entity);
   }
}

private void ShouldSelectEntity(Entity entity)
{
   if (entity.TestObject != null ||
      entity.Score <= m_scoreThreshold) // THIS DOES NOT SEEM TO WORK!!!
      return false;

   ... // Other complex conditions, returning true or false according to entity content.
}

private void SelectEntity(Entity entity)
{
   if (entity.Score <= m_scoreThreshold)
   {
      // We should never get here because of the test in 
      // ShouldSelectEntity.
      throw new Exception($"Should not have selected this object because Score {entity.Score} is lower than threshold {m_scoreThreshold}");
   }

   ... // Other stuff.
}

We observed that the exception in SelectEntity is thrown, even if it should never happens.

The code above is not sufficient to reproduce the bug. For the bug to be present, the above code, it needs to be part of a larger program. The initial program has been stripped greatly and is published here:
https://github.com/Louis-Antoine-Blais-Morin/JitBugArm64
It is still relatively large, but it seems that removing parts of the program seems to make the bug disappear. Maybe more effort can lead to a smaller code.

Reproduction Steps

• Checkout code from
  https://github.com/Louis-Antoine-Blais-Morin/JitBugArm64
• Open JitBugArm64.sln
• Compile it in Release in VisualStudio 17.9.1.
• Copy the content of bin\Release\net8.0 on a Arm64 platform running Linux and where .NET Runtime 8.0.3 is installed
• dotnet run JitBugArm64.dll
• An exception is thrown after few seconds

Expected behavior

The if statement should behave as expected in the C# code.
Therefore, no exception should be thrown in the code linked to this bug.

Actual behavior

The if statement in the ShouldSelectEntity does not behave correctly.
The function seems to returns true even if entity.Score > m_scoreThreshold.

This seems to happen after a few seconds of correct operation.
I suspect that the behavior starts to be incorrect after Tier 1 JIT compilation.

Regression?

This is a regression since the program is working correctly with .NET 7.
The program is also working correctly under Windows x64.

Known Workarounds

The following workarounds seem to fix the behavior:

• Replace the if ( ... || ...) return false; by two individual if statements.
• Add the following attribute on the ShouldSelectEntity method: [MethodImpl(MethodImplOptions.NoInlining)]
• Remove the <DebugSymbols>true</DebugSymbols> in the .csproj

Configuration

$ dotnet --list-runtimes
Microsoft.AspNetCore.App 8.0.3 [/usr/share/dotnet/shared/Microsoft.AspNetCore.App]
Microsoft.NETCore.App 8.0.3 [/usr/share/dotnet/shared/Microsoft.NETCore.App]`

Processor Architecture is Arm64 Cortex A53 / Armv8-A.

Linux is "Yocto Kirkstone".

Other information

No response

[jakobbotsch]

cc @dotnet/jit-contrib

[JulieLeeMSFT]

@kunalspathak PTAL.

[AndyAyersMS]

@kunalspathak can you give an update on this? Or if you're tied up can we find somebody else to dig in?

[kunalspathak]

@kunalspathak can you give an update on this? Or if you're tied up can we find somebody else to dig in?

Sorry, I forgot about this. I was able to repro this and I can confirm that this doesn't repro on .NET 9 main. I will investigate.

[kunalspathak]

The problem is the reverse condition was wrongly calculated as seen in screenshot( left= net9/correct, right= net8/wrong)

We fixed it in #92316 and might need to backport.

[kunalspathak]

btw, this is easily reproducible with DOTNET_TieredCompilation=0 and marking Select as NoInlining and ShouldSelectEntity as AggresivelyInline. I also changed ScoreThreshold=10.0f and Score = (float)0.0f,//random.NextDouble(),

        [MethodImpl(MethodImplOptions.NoInlining)]
        private void Select(Entity trackingObject)
        {
            if (ShouldSelectEntity(trackingObject))
            {
                SelectEntity(trackingObject);
            }
        }

        [MethodImpl(MethodImplOptions.AggressiveInlining)]
        private bool ShouldSelectEntity(Entity entity)
        {
            ...
        }

[kunalspathak]

We fixed it in #92316 and might need to backport.

I verified this fix locally and it doesn't throw Exception anymore.

[kunalspathak]

This should be part of the upcoming .NET 8.0.5

[AndyAyersMS]

@Louis-Antoine-Blais-Morin .NET 8.0.5 is now available -- when you get a chance can you verify it fixes this issue for you?