Skip to content

Commit 993f5ec

Browse files
github-actions[bot]AndyAyersMSCopilot
authored
[release/10.0] JIT: bail from optOptimizeBoolsCondBlock on GTF_UNSIGNED inputs (#128960)
Backport of #128928 to release/10.0 /cc @AndyAyersMS ## Customer Impact - [x] Customer reported - [ ] Found internally Reported in #128895. Impacts a shipping app. ## Regression - [x] Yes - [ ] No Regression from .NET 8/9 (though the introducing change #78786 was also in .NET 8, but apparently not easily reachable). ## Testing Created independent repro based on customer's report. Developed fix and shared with customer, who validated it on their app. Local testing (SPMI, etc) revealed no diffs. ## Risk Low. Disables an optimization. Co-authored-by: Andy Ayers <andya@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent ba2d2e2 commit 993f5ec

3 files changed

Lines changed: 139 additions & 0 deletions

File tree

‎src/coreclr/jit/optimizebools.cpp‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,13 @@ bool OptBoolsDsc::optOptimizeBoolsCondBlock()
209209
if (m_c1->OperIs(GT_LCL_VAR) && m_c2->OperIs(GT_LCL_VAR) &&
210210
m_c1->AsLclVarCommon()->GetLclNum() == m_c2->AsLclVarCommon()->GetLclNum())
211211
{
212+
// The folded comparisons below are signed (e.g. "c1 <= 0", "c1 >= 0"), so
213+
// bail if either input has GTF_UNSIGNED.
214+
if (m_testInfo1.GetTestOp()->IsUnsigned() || m_testInfo2.GetTestOp()->IsUnsigned())
215+
{
216+
return false;
217+
}
218+
212219
if ((m_testInfo1.compTree->OperIs(GT_LT) && m_testInfo2.compTree->OperIs(GT_EQ)) ||
213220
(m_testInfo1.compTree->OperIs(GT_EQ) && m_testInfo2.compTree->OperIs(GT_LT)))
214221
{
@@ -271,6 +278,13 @@ bool OptBoolsDsc::optOptimizeBoolsCondBlock()
271278
if (m_c1->OperIs(GT_LCL_VAR) && m_c2->OperIs(GT_LCL_VAR) &&
272279
m_c1->AsLclVarCommon()->GetLclNum() == m_c2->AsLclVarCommon()->GetLclNum())
273280
{
281+
// The folded comparisons below are signed (e.g. "c1 > 0", "c1 < 0"), so
282+
// bail if either input has GTF_UNSIGNED.
283+
if (m_testInfo1.GetTestOp()->IsUnsigned() || m_testInfo2.GetTestOp()->IsUnsigned())
284+
{
285+
return false;
286+
}
287+
274288
if ((m_testInfo1.compTree->OperIs(GT_LT) && m_testInfo2.compTree->OperIs(GT_NE)) ||
275289
(m_testInfo1.compTree->OperIs(GT_EQ) && m_testInfo2.compTree->OperIs(GT_GE)))
276290
{

‎src/tests/JIT/Regression/JitBlue/Runtime_128895/Runtime_128895.cs‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
// optOptimizeBoolsCondBlock folded a pair of BBJ_COND blocks of the form
5+
// "(x == 0) || (x relop 0)" into a single directional comparison (e.g. LT/LE/GE)
6+
// via SetOper, which preserved gtFlags. When the EQ side was imported from
7+
// "beq.un"/"bne.un" (the typical C# pattern for "long == 0") it carried a
8+
// meaningless GTF_UNSIGNED that became semantically active on the rewritten
9+
// directional relop, producing "x <_U 0" (always false) or "x >=_U 0"
10+
// (always true) instead of the intended signed comparison.
11+
12+
namespace Runtime_128895;
13+
14+
using System.Runtime.CompilerServices;
15+
using Xunit;
16+
17+
public abstract class Base
18+
{
19+
public long fValue;
20+
public abstract int Compare(long other);
21+
}
22+
23+
public sealed class Cell : Base
24+
{
25+
public override int Compare(long other)
26+
{
27+
if (fValue == other) return 0;
28+
if (fValue > other) return 1;
29+
return -1;
30+
}
31+
}
32+
33+
public static class CellPool
34+
{
35+
private static readonly Cell[] s_pool = CreatePool();
36+
private static int s_slot;
37+
38+
private static Cell[] CreatePool()
39+
{
40+
Cell[] pool = new Cell[16];
41+
for (int i = 0; i < pool.Length; i++)
42+
{
43+
pool[i] = new Cell();
44+
}
45+
46+
return pool;
47+
}
48+
49+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
50+
public static Cell Make(int idx)
51+
{
52+
Cell c = s_pool[s_slot++ & 15];
53+
c.fValue = idx >= 0 ? idx + 1 : -1;
54+
return c;
55+
}
56+
}
57+
58+
public class Runtime_128895
59+
{
60+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
61+
private static int FindCharLocal(string s, string needle) =>
62+
s.IndexOfAny(needle.ToCharArray(), 0, s.Length);
63+
64+
[MethodImpl(MethodImplOptions.NoInlining)]
65+
private static bool ValidateMarks(string s)
66+
{
67+
bool r = false;
68+
Cell mark1 = CellPool.Make(FindCharLocal(s, "\u00ab"));
69+
Cell mark2 = CellPool.Make(FindCharLocal(s, "\u00bb"));
70+
Cell mark3 = CellPool.Make(FindCharLocal(s, "\u201c"));
71+
if ((mark1.Compare(0) == 0 || mark1.Compare(0) > 0)
72+
|| (mark2.Compare(0) == 0 || mark2.Compare(0) > 0)
73+
|| (mark3.Compare(0) == 0 || mark3.Compare(0) > 0))
74+
{
75+
r = true;
76+
}
77+
78+
return r;
79+
}
80+
81+
private static readonly string[] s_inputs = new[]
82+
{
83+
"abc", "def", "ghi", "jkl", "mno", "pqr", "abc\u00abdef", "xyz"
84+
};
85+
86+
[Fact]
87+
public static int TestEntryPoint()
88+
{
89+
// Drive ValidateMarks through Tier0 -> Instrumented Tier0 -> Tier1+PGO.
90+
for (int i = 0; i < 1000; i++)
91+
{
92+
ValidateMarks(s_inputs[i & 7]);
93+
System.Threading.Thread.Sleep(1);
94+
}
95+
96+
System.Threading.Thread.Sleep(100);
97+
98+
int failures = 0;
99+
for (int k = 0; k < s_inputs.Length; k++)
100+
{
101+
string s = s_inputs[k];
102+
bool expected = s.Contains('\u00ab') || s.Contains('\u00bb') || s.Contains('\u201c');
103+
bool actual = ValidateMarks(s);
104+
if (expected != actual)
105+
{
106+
failures++;
107+
}
108+
}
109+
110+
return failures == 0 ? 100 : 1;
111+
}
112+
}

‎src/tests/JIT/Regression/JitBlue/Runtime_128895/Runtime_128895.csproj‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
<PropertyGroup>
3+
<DebugType>None</DebugType>
4+
<Optimize>True</Optimize>
5+
<!-- Needed for CLRTestEnvironmentVariable -->
6+
<RequiresProcessIsolation>true</RequiresProcessIsolation>
7+
</PropertyGroup>
8+
<ItemGroup>
9+
<Compile Include="$(MSBuildProjectName).cs" />
10+
<CLRTestEnvironmentVariable Include="DOTNET_TieredCompilation" Value="1" />
11+
<CLRTestEnvironmentVariable Include="DOTNET_TieredPGO" Value="1" />
12+
</ItemGroup>
13+
</Project>

0 commit comments

Comments
 (0)