Skip to content

Commit 472a33b

Browse files
Fix side effect handling for certain TernaryLogic cases (dotnet#114848)
There were a few cases of TernaryLogic with NOT/AND and NOT/OR that swapped operands without considering side effects. Add some new logic to hoist those side effects if necessary. Fixes dotnet#114324
1 parent 337ffbd commit 472a33b

File tree

3 files changed

+120
-4
lines changed

3 files changed

+120
-4
lines changed

src/coreclr/jit/hwintrinsicxarch.cpp

Lines changed: 61 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4405,6 +4405,65 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,
44054405
}
44064406
}
44074407

4408+
// Some normalization cases require us to swap the operands, which might require
4409+
// spilling side effects. Check that here.
4410+
//
4411+
// cast in switch clause is needed for old gcc
4412+
switch ((TernaryLogicOperKind)info.oper1)
4413+
{
4414+
case TernaryLogicOperKind::Not:
4415+
{
4416+
assert(info.oper1Use != TernaryLogicUseFlags::None);
4417+
4418+
bool needSideEffectSpill = false;
4419+
4420+
if (info.oper2 == TernaryLogicOperKind::And)
4421+
{
4422+
assert(info.oper2Use != TernaryLogicUseFlags::None);
4423+
4424+
if ((control == static_cast<uint8_t>(~0xCC & 0xF0)) || // ~B & A
4425+
(control == static_cast<uint8_t>(~0xAA & 0xF0)) || // ~C & A
4426+
(control == static_cast<uint8_t>(~0xAA & 0xCC))) // ~C & B
4427+
{
4428+
// We're normalizing to ~B & C, so we need another swap
4429+
std::swap(val2, val3);
4430+
needSideEffectSpill = (control == static_cast<uint8_t>(~0xAA & 0xCC)); // ~C & B
4431+
}
4432+
}
4433+
else if (info.oper2 == TernaryLogicOperKind::Or)
4434+
{
4435+
assert(info.oper2Use != TernaryLogicUseFlags::None);
4436+
4437+
if ((control == static_cast<uint8_t>(~0xCC | 0xF0)) || // ~B | A
4438+
(control == static_cast<uint8_t>(~0xAA | 0xF0)) || // ~C | A
4439+
(control == static_cast<uint8_t>(~0xAA | 0xCC))) // ~C | B
4440+
{
4441+
// We're normalizing to ~B | C, so we need another swap
4442+
std::swap(val2, val3);
4443+
needSideEffectSpill = (control == static_cast<uint8_t>(~0xAA | 0xCC)); // ~C | B
4444+
}
4445+
}
4446+
4447+
if (needSideEffectSpill)
4448+
{
4449+
// Side-effect cases:
4450+
// ~B op A ; order before swap C A B
4451+
// op1 & op2 already set to be spilled; no further spilling necessary
4452+
// ~C op A ; order before swap B A C
4453+
// op1 already set to be spilled; no further spilling necessary
4454+
// ~C op B ; order before swap A B C
4455+
// nothing already set to be spilled; op1 & op2 need to be spilled
4456+
4457+
spillOp1 = true;
4458+
spillOp2 = true;
4459+
}
4460+
break;
4461+
}
4462+
4463+
default:
4464+
break;
4465+
}
4466+
44084467
if (spillOp1)
44094468
{
44104469
impSpillSideEffect(true, stackState.esStackDepth -
@@ -4529,8 +4588,7 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,
45294588
(control == static_cast<uint8_t>(~0xAA & 0xF0)) || // ~C & A
45304589
(control == static_cast<uint8_t>(~0xAA & 0xCC))) // ~C & B
45314590
{
4532-
// We're normalizing to ~B & C, so we need another swap
4533-
std::swap(*val2, *val3);
4591+
// We already normalized to ~B & C above.
45344592
}
45354593
else
45364594
{
@@ -4556,8 +4614,7 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,
45564614
(control == static_cast<uint8_t>(~0xAA | 0xF0)) || // ~C | A
45574615
(control == static_cast<uint8_t>(~0xAA | 0xCC))) // ~C | B
45584616
{
4559-
// We're normalizing to ~B & C, so we need another swap
4560-
std::swap(*val2, *val3);
4617+
// We already normalized to ~B | C above.
45614618
}
45624619
else
45634620
{
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
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+
// Generated by Fuzzlyn v2.5 on 2025-04-06 16:30:45
5+
// Run on X64 Windows
6+
// Seed: 13621799076737484256-vectort,vector128,vector256,x86aes,x86avx,x86avx2,x86avx512bw,x86avx512bwvl,x86avx512cd,x86avx512cdvl,x86avx512dq,x86avx512dqvl,x86avx512f,x86avx512fvl,x86avx512fx64,x86bmi1,x86bmi1x64,x86bmi2,x86bmi2x64,x86fma,x86lzcnt,x86lzcntx64,x86pclmulqdq,x86popcnt,x86popcntx64,x86sse,x86ssex64,x86sse2,x86sse2x64,x86sse3,x86sse41,x86sse41x64,x86sse42,x86sse42x64,x86ssse3,x86x86base
7+
// Reduced from 20.1 KiB to 0.7 KiB in 00:01:13
8+
// Debug: Runs successfully
9+
// Release: Throws 'System.NullReferenceException'
10+
using System;
11+
using System.Numerics;
12+
using System.Runtime.Intrinsics;
13+
using System.Runtime.Intrinsics.X86;
14+
using Xunit;
15+
16+
public class Runtime_114324
17+
{
18+
public static ulong[] s_3;
19+
20+
[Fact]
21+
public static void TestEntryPoint()
22+
{
23+
if (Avx512F.VL.IsSupported)
24+
{
25+
var vr1 = Vector128.Create<int>(1);
26+
Vector128<int> vr2 = Avx512F.VL.TernaryLogic(
27+
vr1,
28+
Sse2.ShiftLeftLogical128BitLane(
29+
Vector128.Create<int>((int)Sse42.Crc32(M2(), 0)),
30+
0),
31+
Avx512F.VL.ConvertToVector128Int32(Vector256.Create<ulong>(s_3[0])),
32+
221);
33+
System.Console.WriteLine(vr2);
34+
Assert.Equal(Vector128.Create(-1,-1,-1,-1), vr2);
35+
}
36+
}
37+
38+
public static uint M2()
39+
{
40+
s_3 = new ulong[]
41+
{
42+
0
43+
};
44+
return M3();
45+
}
46+
47+
public static byte M3()
48+
{
49+
return default(byte);
50+
}
51+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
<PropertyGroup>
3+
<Optimize>True</Optimize>
4+
</PropertyGroup>
5+
<ItemGroup>
6+
<Compile Include="$(MSBuildProjectName).cs" />
7+
</ItemGroup>
8+
</Project>

0 commit comments

Comments
 (0)