-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
arm64: Assertion failed 'loop->ContainsBlock(pred)' during 'Do value numbering' #110959
Comments
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
Bisected to #110273, cc @AndyAyersMS Presumably the new removal phases are not properly maintaining canonicalized loop shapes. Can the second run of these phases be changed to run before loop finding/canonicalization happens? |
Probably so... |
So far I can't repro. |
Still can't repro with the above... let me see if there are other instances to try. |
Maybe requires linux arm64... let me try a cross jit. |
Yes, you need |
No luck with that either. |
The example provided above has an empty |
Ugh. will fix it in Antigen. |
Yeah I have to do that fix frequently, but in the case above I've verified the method is jitted. |
Fixed - kunalspathak/Antigen@acc331f |
I cannot repro it either which makes me think that it got fixed along with other fixes that went in from last few days. I also saw latest Antigen run that you triggered and don't see this repro. Closing for now. |
Here's one from Fuzzlyn that repros it on main: // Generated by Fuzzlyn v2.4 on 2024-12-29 17:20:48
// Run on Arm64 MacOS
// Seed: 561497581619134845-vectort,vector64,vector128,armadvsimd,armadvsimdarm64,armaes,armarmbase,armarmbasearm64,armcrc32,armcrc32arm64,armdp,armrdm,armrdmarm64,armsha1,armsha256
// Reduced from 163.4 KiB to 0.9 KiB in 00:00:40
// Hits JIT assert in Release:
// Assertion failed 'loop->ContainsBlock(pred)' in 'Program:M0()' during 'Do value numbering' (IL size 92; hash 0xaf50ff37; FullOpts)
//
// File: /Users/runner/work/1/s/src/coreclr/jit/fgdiagnostic.cpp Line: 4713
//
using System;
using System.Numerics;
using System.Runtime.Intrinsics;
using System.Runtime.Intrinsics.Arm;
public struct S0
{
public Vector64<uint> F7;
}
public struct S1
{
public S0 F3;
}
public class Program
{
public static double s_1;
public static void Main()
{
M0();
}
public static void M0()
{
S1 var5 = default(S1);
Vector128<long>[][] var6 = default(Vector128<long>[][]);
for (int var2 = 0; var2 < 0; var2++)
{
try
{
if ((0 >= s_1))
{
}
else
{
System.Console.WriteLine(var6[0][0]);
return;
}
}
finally
{
var5.F3.F7 = var5.F3.F7;
}
}
}
} Repros on win-x64 as well. |
The original loop formation:
Why is BB03 in the loop? It's because of EH: an exception in BB03 can cause BB07 (which is in the loop) to be executed next. We canonicalize the exit here by introducing BB13 and update flow so we have BB03, BB04 -> BB13 and BB013 -> BB04. We later realize the finally is empty and remove the try/finally, so this EH flow possibility goes away. But that means the exit canonicalization (loop exit blocks have only loop blocks as preds) we did earlier is no longer valid, one exit now (effectively) moves to BB03 and so BB13 (the other exit) now has a non-loop block as pred. |
This shows up as an assert in VN because the EH opt invalidates DFS and hence loops. SSA rebuilds DFS and then VN rebuilds loops and so VN is where post-phase loop checks are first re-enabled. We could move these EH passes before loop finding, but thought was that cloning would suppress range checks which might make loops bodies inside trys be exception-free and so enable EH region removal before we run the optimization phases. I guess I can try moving the EH phases earlier and see what diffs we end up with. |
to avoid messing up loop exit canonicalization (which is sensitive to loops with internal EH flow). Fixed dotnet#110959. Also, remove some of the ceremony around `fgModified` -- seems like this is no longer useful/needed. Defer deciding if we need a PSPSym until we create funclets. This removes unused PSPSym allocations in some cases. Also, there is no benefit to tracking the PSPSym, so stop doing that.
to avoid messing up loop exit canonicalization (which is sensitive to loops with internal EH flow). Fixed #110959. Also, remove some of the ceremony around `fgModified` -- seems like this is no longer useful/needed. Defer deciding if we need a PSPSym until we create funclets. This removes unused PSPSym allocations in some cases. Also, there is no benefit to tracking the PSPSym, so stop doing that.
The text was updated successfully, but these errors were encountered: