Skip to content

Commit 90ddd5e

Browse files
tannergoodingtannergooding
authored
Ensure Max/Min for floating-point on x86/x64 are not handled as commutative (#75683)
* Adding a regression test for SixLabors/ImageSharp#2117 * Ensure Max/Min for floating-point on x86/x64 are not handled as commutative * Applying formatting patch
1 parent e574380 commit 90ddd5e

File tree

5 files changed

+111
-9
lines changed

5 files changed

+111
-9
lines changed

‎src/coreclr/jit/gentree.cpp

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18659,10 +18659,45 @@ bool GenTree::isCommutativeHWIntrinsic() const
1865918659
assert(gtOper == GT_HWINTRINSIC);
1866018660

1866118661
#ifdef TARGET_XARCH
18662-
return HWIntrinsicInfo::IsCommutative(AsHWIntrinsic()->GetHWIntrinsicId());
18663-
#else
18664-
return false;
18662+
const GenTreeHWIntrinsic* node = AsHWIntrinsic();
18663+
NamedIntrinsic id = node->GetHWIntrinsicId();
18664+
18665+
if (HWIntrinsicInfo::IsCommutative(id))
18666+
{
18667+
return true;
18668+
}
18669+
18670+
if (HWIntrinsicInfo::IsMaybeCommutative(id))
18671+
{
18672+
switch (id)
18673+
{
18674+
case NI_SSE_Max:
18675+
case NI_SSE_Min:
18676+
{
18677+
return false;
18678+
}
18679+
18680+
case NI_SSE2_Max:
18681+
case NI_SSE2_Min:
18682+
{
18683+
return !varTypeIsFloating(node->GetSimdBaseType());
18684+
}
18685+
18686+
case NI_AVX_Max:
18687+
case NI_AVX_Min:
18688+
{
18689+
return false;
18690+
}
18691+
18692+
default:
18693+
{
18694+
unreached();
18695+
}
18696+
}
18697+
}
1866518698
#endif // TARGET_XARCH
18699+
18700+
return false;
1866618701
}
1866718702

1866818703
bool GenTree::isContainableHWIntrinsic() const

‎src/coreclr/jit/hwintrinsic.h

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,11 @@ enum HWIntrinsicFlag : unsigned int
153153
// the intrinsic can be used on hardware with AVX but not AVX2 support
154154
HW_Flag_AvxOnlyCompatible = 0x40000,
155155

156+
// MaybeCommutative
157+
// - if a binary-op intrinsic is maybe commutative (e.g., Max or Min for float/double), its op1 can possibly be
158+
// contained
159+
HW_Flag_MaybeCommutative = 0x80000,
160+
156161
#elif defined(TARGET_ARM64)
157162
// The intrinsic has an immediate operand
158163
// - the value can be (and should be) encoded in a corresponding instruction when the operand value is constant
@@ -626,6 +631,18 @@ struct HWIntrinsicInfo
626631
return (flags & HW_Flag_Commutative) != 0;
627632
}
628633

634+
static bool IsMaybeCommutative(NamedIntrinsic id)
635+
{
636+
HWIntrinsicFlag flags = lookupFlags(id);
637+
#if defined(TARGET_XARCH)
638+
return (flags & HW_Flag_MaybeCommutative) != 0;
639+
#elif defined(TARGET_ARM64)
640+
return false;
641+
#else
642+
#error Unsupported platform
643+
#endif
644+
}
645+
629646
static bool RequiresCodegen(NamedIntrinsic id)
630647
{
631648
HWIntrinsicFlag flags = lookupFlags(id);

‎src/coreclr/jit/hwintrinsiclistxarch.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -290,9 +290,9 @@ HARDWARE_INTRINSIC(SSE, LoadHigh,
290290
HARDWARE_INTRINSIC(SSE, LoadLow, 16, 2, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_movlps, INS_invalid}, HW_Category_MemoryLoad, HW_Flag_NoRMWSemantics)
291291
HARDWARE_INTRINSIC(SSE, LoadScalarVector128, 16, 1, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_movss, INS_invalid}, HW_Category_MemoryLoad, HW_Flag_NoRMWSemantics)
292292
HARDWARE_INTRINSIC(SSE, LoadVector128, 16, 1, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_movups, INS_invalid}, HW_Category_MemoryLoad, HW_Flag_NoRMWSemantics)
293-
HARDWARE_INTRINSIC(SSE, Max, 16, 2, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_maxps, INS_invalid}, HW_Category_SimpleSIMD, HW_Flag_Commutative)
293+
HARDWARE_INTRINSIC(SSE, Max, 16, 2, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_maxps, INS_invalid}, HW_Category_SimpleSIMD, HW_Flag_MaybeCommutative)
294294
HARDWARE_INTRINSIC(SSE, MaxScalar, 16, 2, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_maxss, INS_invalid}, HW_Category_SIMDScalar, HW_Flag_CopyUpperBits)
295-
HARDWARE_INTRINSIC(SSE, Min, 16, 2, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_minps, INS_invalid}, HW_Category_SimpleSIMD, HW_Flag_Commutative)
295+
HARDWARE_INTRINSIC(SSE, Min, 16, 2, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_minps, INS_invalid}, HW_Category_SimpleSIMD, HW_Flag_MaybeCommutative)
296296
HARDWARE_INTRINSIC(SSE, MinScalar, 16, 2, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_minss, INS_invalid}, HW_Category_SIMDScalar, HW_Flag_CopyUpperBits)
297297
HARDWARE_INTRINSIC(SSE, MoveHighToLow, 16, 2, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_movhlps, INS_invalid}, HW_Category_SimpleSIMD, HW_Flag_NoContainment)
298298
HARDWARE_INTRINSIC(SSE, MoveLowToHigh, 16, 2, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_movlhps, INS_invalid}, HW_Category_SimpleSIMD, HW_Flag_NoContainment)
@@ -403,10 +403,10 @@ HARDWARE_INTRINSIC(SSE2, LoadLow,
403403
HARDWARE_INTRINSIC(SSE2, LoadScalarVector128, 16, 1, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_movd, INS_movd, INS_movq, INS_movq, INS_invalid, INS_movsdsse2}, HW_Category_MemoryLoad, HW_Flag_NoRMWSemantics)
404404
HARDWARE_INTRINSIC(SSE2, LoadVector128, 16, 1, {INS_movdqu, INS_movdqu, INS_movdqu, INS_movdqu, INS_movdqu, INS_movdqu, INS_movdqu, INS_movdqu, INS_invalid, INS_movupd}, HW_Category_MemoryLoad, HW_Flag_NoRMWSemantics)
405405
HARDWARE_INTRINSIC(SSE2, MaskMove, 16, 3, {INS_maskmovdqu, INS_maskmovdqu, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid}, HW_Category_MemoryStore, HW_Flag_NoContainment|HW_Flag_NoRMWSemantics|HW_Flag_BaseTypeFromSecondArg)
406-
HARDWARE_INTRINSIC(SSE2, Max, 16, 2, {INS_invalid, INS_pmaxub, INS_pmaxsw, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_maxpd}, HW_Category_SimpleSIMD, HW_Flag_Commutative)
406+
HARDWARE_INTRINSIC(SSE2, Max, 16, 2, {INS_invalid, INS_pmaxub, INS_pmaxsw, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_maxpd}, HW_Category_SimpleSIMD, HW_Flag_MaybeCommutative)
407407
HARDWARE_INTRINSIC(SSE2, MemoryFence, 0, 0, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid}, HW_Category_Special, HW_Flag_NoContainment|HW_Flag_NoRMWSemantics)
408408
HARDWARE_INTRINSIC(SSE2, MaxScalar, 16, 2, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_maxsd}, HW_Category_SIMDScalar, HW_Flag_CopyUpperBits)
409-
HARDWARE_INTRINSIC(SSE2, Min, 16, 2, {INS_invalid, INS_pminub, INS_pminsw, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_minpd}, HW_Category_SimpleSIMD, HW_Flag_Commutative)
409+
HARDWARE_INTRINSIC(SSE2, Min, 16, 2, {INS_invalid, INS_pminub, INS_pminsw, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_minpd}, HW_Category_SimpleSIMD, HW_Flag_MaybeCommutative)
410410
HARDWARE_INTRINSIC(SSE2, MinScalar, 16, 2, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_minsd}, HW_Category_SIMDScalar, HW_Flag_CopyUpperBits)
411411
HARDWARE_INTRINSIC(SSE2, MoveMask, 16, 1, {INS_pmovmskb, INS_pmovmskb, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_movmskpd}, HW_Category_SimpleSIMD, HW_Flag_NoContainment|HW_Flag_NoRMWSemantics|HW_Flag_BaseTypeFromFirstArg)
412412
HARDWARE_INTRINSIC(SSE2, MoveScalar, 16, -1, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_movq, INS_movq, INS_invalid, INS_movsdsse2}, HW_Category_SIMDScalar, HW_Flag_NoContainment)
@@ -598,8 +598,8 @@ HARDWARE_INTRINSIC(AVX, InsertVector128,
598598
HARDWARE_INTRINSIC(AVX, LoadAlignedVector256, 32, 1, {INS_movdqa, INS_movdqa, INS_movdqa, INS_movdqa, INS_movdqa, INS_movdqa, INS_movdqa, INS_movdqa, INS_movaps, INS_movapd}, HW_Category_MemoryLoad, HW_Flag_NoRMWSemantics)
599599
HARDWARE_INTRINSIC(AVX, LoadDquVector256, 32, 1, {INS_lddqu, INS_lddqu, INS_lddqu, INS_lddqu, INS_lddqu, INS_lddqu, INS_lddqu, INS_lddqu, INS_invalid, INS_invalid}, HW_Category_MemoryLoad, HW_Flag_NoRMWSemantics)
600600
HARDWARE_INTRINSIC(AVX, LoadVector256, 32, 1, {INS_movdqu, INS_movdqu, INS_movdqu, INS_movdqu, INS_movdqu, INS_movdqu, INS_movdqu, INS_movdqu, INS_movups, INS_movupd}, HW_Category_MemoryLoad, HW_Flag_NoRMWSemantics)
601-
HARDWARE_INTRINSIC(AVX, Max, 32, 2, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_maxps, INS_maxpd}, HW_Category_SimpleSIMD, HW_Flag_Commutative)
602-
HARDWARE_INTRINSIC(AVX, Min, 32, 2, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_minps, INS_minpd}, HW_Category_SimpleSIMD, HW_Flag_Commutative)
601+
HARDWARE_INTRINSIC(AVX, Max, 32, 2, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_maxps, INS_maxpd}, HW_Category_SimpleSIMD, HW_Flag_MaybeCommutative)
602+
HARDWARE_INTRINSIC(AVX, Min, 32, 2, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_minps, INS_minpd}, HW_Category_SimpleSIMD, HW_Flag_MaybeCommutative)
603603
HARDWARE_INTRINSIC(AVX, MaskLoad, -1, 2, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_vmaskmovps, INS_vmaskmovpd}, HW_Category_MemoryLoad, HW_Flag_NoFlag)
604604
HARDWARE_INTRINSIC(AVX, MaskStore, -1, 3, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_vmaskmovps, INS_vmaskmovpd}, HW_Category_MemoryStore, HW_Flag_NoContainment|HW_Flag_BaseTypeFromSecondArg)
605605
HARDWARE_INTRINSIC(AVX, MoveMask, 32, 1, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_movmskps, INS_movmskpd}, HW_Category_SimpleSIMD, HW_Flag_NoContainment|HW_Flag_BaseTypeFromFirstArg)

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
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+
using System.Runtime.Intrinsics;
5+
using System.Runtime.Intrinsics.X86;
6+
7+
public unsafe class ImageSharp_2117
8+
{
9+
public static int Main(string[] args)
10+
{
11+
if (Sse.IsSupported)
12+
{
13+
Vector128<float> fnan = Vector128.Create(float.NaN);
14+
Vector128<float> res1 = Sse.Max(Sse.LoadVector128((float*)(&fnan)), Vector128<float>.Zero);
15+
Vector128<float> res2 = Sse.Min(Sse.LoadVector128((float*)(&fnan)), Vector128<float>.Zero);
16+
17+
if (float.IsNaN(res1[0]) || float.IsNaN(res2[0]))
18+
{
19+
return 0;
20+
}
21+
}
22+
23+
if (Sse2.IsSupported)
24+
{
25+
Vector128<double> dnan = Vector128.Create(double.NaN);
26+
Vector128<double> res3 = Sse2.Max(Sse2.LoadVector128((double*)(&dnan)), Vector128<double>.Zero);
27+
Vector128<double> res4 = Sse2.Min(Sse2.LoadVector128((double*)(&dnan)), Vector128<double>.Zero);
28+
29+
if (double.IsNaN(res3[0]) || double.IsNaN(res4[0]))
30+
{
31+
return 0;
32+
}
33+
}
34+
35+
return 100;
36+
}
37+
}

‎src/tests/JIT/Regression/JitBlue/ImageSharp_2117/ImageSharp_2117.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+
<OutputType>Exe</OutputType>
4+
<AllowUnsafeBlocks>True</AllowUnsafeBlocks>
5+
</PropertyGroup>
6+
<PropertyGroup>
7+
<DebugType>None</DebugType>
8+
<Optimize>True</Optimize>
9+
</PropertyGroup>
10+
<ItemGroup>
11+
<Compile Include="$(MSBuildProjectName).cs" />
12+
</ItemGroup>
13+
</Project>

0 commit comments

Comments
 (0)