From b69a784ad8d701200f47891f4d5a40428916a8c5 Mon Sep 17 00:00:00 2001 From: Miha Zupan Date: Fri, 17 Jan 2025 12:58:41 +0100 Subject: [PATCH 01/11] JIT: Use faster mod for uin16 values --- src/coreclr/jit/assertionprop.cpp | 14 +++++++-- src/coreclr/jit/gentree.h | 2 ++ src/coreclr/jit/lower.cpp | 50 ++++++++++++++++++++++++++++++- 3 files changed, 63 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 288891b0daab44..0cd012572fd524 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4116,6 +4116,7 @@ void Compiler::optAssertionProp_RangeProperties(ASSERT_VALARG_TP assertions, // 1) Convert DIV/MOD to UDIV/UMOD if both operands are proven to be never negative // 2) Marks DIV/UDIV/MOD/UMOD with GTF_DIV_MOD_NO_BY_ZERO if divisor is proven to be never zero // 3) Marks DIV/UDIV/MOD/UMOD with GTF_DIV_MOD_NO_OVERFLOW if both operands are proven to be never negative +// 4) UMOD with GTF_UMOD_UINT16_OPERANDS if both operands are proven to be in uint16 range // // Arguments: // assertions - set of live assertions @@ -4145,20 +4146,29 @@ GenTree* Compiler::optAssertionProp_ModDiv(ASSERT_VALARG_TP assertions, GenTreeO changed = true; } - if (op2IsNotZero) + if (((tree->gtFlags & GTF_DIV_MOD_NO_BY_ZERO) == 0) && op2IsNotZero) { JITDUMP("Divisor for DIV/MOD is proven to be never negative...\n") tree->gtFlags |= GTF_DIV_MOD_NO_BY_ZERO; changed = true; } - if (op1IsNotNegative || op2IsNotNegative) + if (((tree->gtFlags & GTF_DIV_MOD_NO_OVERFLOW) == 0) && (op1IsNotNegative || op2IsNotNegative)) { JITDUMP("DIV/MOD is proven to never overflow...\n") tree->gtFlags |= GTF_DIV_MOD_NO_OVERFLOW; changed = true; } + if (((tree->gtFlags & GTF_UMOD_UINT16_OPERANDS) == 0) && tree->OperIs(GT_UMOD) && op2->IsCnsIntOrI() && + op2->AsIntCon()->IconValue() <= UINT16_MAX && + IntegralRange::ForNode(op1, this).GetUpperBound() <= SymbolicIntegerValue::UShortMax) + { + JITDUMP("Both operands for UMOD are in uint16 range...\n") + tree->gtFlags |= GTF_UMOD_UINT16_OPERANDS; + changed = true; + } + return changed ? optAssertionProp_Update(tree, tree, stmt) : nullptr; } diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index e7c42dc83c146b..d7e4c32f889b33 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -540,6 +540,8 @@ enum GenTreeFlags : unsigned int GTF_DIV_MOD_NO_OVERFLOW = 0x40000000, // GT_DIV, GT_MOD -- Div or mod definitely does not overflow. + GTF_UMOD_UINT16_OPERANDS = 0x80000000, // UMOD -- Both operands to a mod are in uint16 range. + GTF_CHK_INDEX_INBND = 0x80000000, // GT_BOUNDS_CHECK -- have proven this check is always in-bounds GTF_ARRLEN_NONFAULTING = 0x20000000, // GT_ARR_LENGTH -- An array length operation that cannot fault. Same as GT_IND_NONFAULTING. diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 724dd17082a4e3..9dcef01123b9c0 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -7182,9 +7182,57 @@ bool Lowering::LowerUnsignedDivOrMod(GenTreeOp* divMod) } } + assert(divisorValue >= 3); + + if (comp->opts.MinOpts()) + { + return false; + } + + // Replace (uint16 % uint16) with a cheaper variant of FastMod, specialized for 16-bit operands. + if ((divMod->gtFlags & GTF_UMOD_UINT16_OPERANDS) != 0) + { + assert(!isDiv); + assert(divisorValue > 0 && divisorValue <= UINT16_MAX); + + // uint multiplier = uint.MaxValue / divisor + 1; + // ulong result = ((ulong)(dividend * multiplier) * divisor) >> 32; + // return (int)result; + + // multiplier = uint.MaxValue / divisor + 1 + GenTree* multiplier = comp->gtNewIconNode((UINT32_MAX / divisorValue) + 1, TYP_INT); + + // (dividend * multiplier) + GenTree* mul1 = comp->gtNewOperNode(GT_MUL, TYP_INT, dividend, multiplier); + mul1->SetUnsigned(); + + // (ulong)(dividend * multiplier) + GenTree* cast = comp->gtNewCastNode(TYP_LONG, mul1, true, TYP_LONG); + + // ((ulong)(dividend * multiplier) * divisor) + GenTree* mul2 = comp->gtNewOperNode(GT_MUL, TYP_LONG, cast, divisor); + mul2->SetUnsigned(); + + // ((ulong)(dividend * multiplier) * divisor) >> 32 + GenTree* shiftAmount = comp->gtNewIconNode(32, TYP_INT); + GenTree* shift = comp->gtNewOperNode(GT_RSZ, TYP_LONG, mul2, shiftAmount); + + BlockRange().InsertBefore(divMod, multiplier, mul1, cast, shiftAmount); + BlockRange().InsertBefore(divMod, mul2, shift); + + // (int)result + divMod->ChangeOper(GT_CAST); + divMod->AsCast()->gtCastType = TYP_INT; + divMod->gtOp1 = shift; + divMod->gtOp2 = nullptr; + divMod->SetUnsigned(); + ContainCheckRange(multiplier, divMod); + return true; + } + // TODO-ARM-CQ: Currently there's no GT_MULHI for ARM32 #if defined(TARGET_XARCH) || defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64) - if (!comp->opts.MinOpts() && (divisorValue >= 3)) + if (divisorValue >= 3) { size_t magic; bool increment; From f536ec9434ff8c9165840a70823b10ac6539be40 Mon Sep 17 00:00:00 2001 From: Miha Zupan Date: Fri, 17 Jan 2025 15:00:49 +0100 Subject: [PATCH 02/11] Skip 32bit --- src/coreclr/jit/lower.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 9dcef01123b9c0..266914406bc619 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -7189,6 +7189,7 @@ bool Lowering::LowerUnsignedDivOrMod(GenTreeOp* divMod) return false; } +#if defined(TARGET_64BIT) // Replace (uint16 % uint16) with a cheaper variant of FastMod, specialized for 16-bit operands. if ((divMod->gtFlags & GTF_UMOD_UINT16_OPERANDS) != 0) { @@ -7229,6 +7230,7 @@ bool Lowering::LowerUnsignedDivOrMod(GenTreeOp* divMod) ContainCheckRange(multiplier, divMod); return true; } +#endif // TODO-ARM-CQ: Currently there's no GT_MULHI for ARM32 #if defined(TARGET_XARCH) || defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64) From 1ab5eb4e0aff3a66974bd3e5e23868b79eb6e758 Mon Sep 17 00:00:00 2001 From: Miha Zupan Date: Fri, 17 Jan 2025 15:01:01 +0100 Subject: [PATCH 03/11] Also compare the new flag --- src/coreclr/jit/gentree.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 06854fa563b59c..feb5c2512b90e5 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -2678,8 +2678,8 @@ bool GenTree::Compare(GenTree* op1, GenTree* op2, bool swapOK) } if (op1->OperIs(GT_MOD, GT_UMOD, GT_DIV, GT_UDIV)) { - if ((op1->gtFlags & (GTF_DIV_MOD_NO_BY_ZERO | GTF_DIV_MOD_NO_OVERFLOW)) != - (op2->gtFlags & (GTF_DIV_MOD_NO_BY_ZERO | GTF_DIV_MOD_NO_OVERFLOW))) + if ((op1->gtFlags & (GTF_DIV_MOD_NO_BY_ZERO | GTF_DIV_MOD_NO_OVERFLOW | GTF_UMOD_UINT16_OPERANDS)) != + (op2->gtFlags & (GTF_DIV_MOD_NO_BY_ZERO | GTF_DIV_MOD_NO_OVERFLOW | GTF_UMOD_UINT16_OPERANDS))) { return false; } From 7a9c2a2c389462c6a0c345415b10558b97b93ac8 Mon Sep 17 00:00:00 2001 From: Miha Zupan Date: Fri, 17 Jan 2025 15:19:41 +0100 Subject: [PATCH 04/11] Use FitsIn helper --- src/coreclr/jit/assertionprop.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 0cd012572fd524..4c875b12c69ba4 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4161,7 +4161,7 @@ GenTree* Compiler::optAssertionProp_ModDiv(ASSERT_VALARG_TP assertions, GenTreeO } if (((tree->gtFlags & GTF_UMOD_UINT16_OPERANDS) == 0) && tree->OperIs(GT_UMOD) && op2->IsCnsIntOrI() && - op2->AsIntCon()->IconValue() <= UINT16_MAX && + FitsIn(op2->AsIntCon()->IconValue()) && IntegralRange::ForNode(op1, this).GetUpperBound() <= SymbolicIntegerValue::UShortMax) { JITDUMP("Both operands for UMOD are in uint16 range...\n") From f0687aa0d8cad183993d786e64f962f5619a6708 Mon Sep 17 00:00:00 2001 From: Miha Zupan Date: Fri, 17 Jan 2025 15:19:50 +0100 Subject: [PATCH 05/11] Assert that lower bound >= 0 --- src/coreclr/jit/assertionprop.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 4c875b12c69ba4..73d226a16654c2 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4164,6 +4164,7 @@ GenTree* Compiler::optAssertionProp_ModDiv(ASSERT_VALARG_TP assertions, GenTreeO FitsIn(op2->AsIntCon()->IconValue()) && IntegralRange::ForNode(op1, this).GetUpperBound() <= SymbolicIntegerValue::UShortMax) { + assert(IntegralRange::ForNode(op1, this).GetLowerBound() >= SymbolicIntegerValue::Zero); JITDUMP("Both operands for UMOD are in uint16 range...\n") tree->gtFlags |= GTF_UMOD_UINT16_OPERANDS; changed = true; From 2d582cbf6bbcc92e3aea0ebbe826c0dd22c4528c Mon Sep 17 00:00:00 2001 From: Miha Zupan Date: Fri, 17 Jan 2025 16:56:07 +0100 Subject: [PATCH 06/11] Fix the assert --- src/coreclr/jit/assertionprop.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 73d226a16654c2..7a79d7d3eefc9f 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4161,10 +4161,9 @@ GenTree* Compiler::optAssertionProp_ModDiv(ASSERT_VALARG_TP assertions, GenTreeO } if (((tree->gtFlags & GTF_UMOD_UINT16_OPERANDS) == 0) && tree->OperIs(GT_UMOD) && op2->IsCnsIntOrI() && - FitsIn(op2->AsIntCon()->IconValue()) && + FitsIn(op2->AsIntCon()->IconValue()) && op1IsNotNegative && IntegralRange::ForNode(op1, this).GetUpperBound() <= SymbolicIntegerValue::UShortMax) { - assert(IntegralRange::ForNode(op1, this).GetLowerBound() >= SymbolicIntegerValue::Zero); JITDUMP("Both operands for UMOD are in uint16 range...\n") tree->gtFlags |= GTF_UMOD_UINT16_OPERANDS; changed = true; From 8a66e6ecde8f6381e382c0f11e661b8db178875f Mon Sep 17 00:00:00 2001 From: Miha Zupan Date: Tue, 21 Jan 2025 16:59:18 +0100 Subject: [PATCH 07/11] Fix case where dividend is long that's known in range --- src/coreclr/jit/assertionprop.cpp | 2 +- src/coreclr/jit/lower.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 7a79d7d3eefc9f..e88e0734cf0db6 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4116,7 +4116,7 @@ void Compiler::optAssertionProp_RangeProperties(ASSERT_VALARG_TP assertions, // 1) Convert DIV/MOD to UDIV/UMOD if both operands are proven to be never negative // 2) Marks DIV/UDIV/MOD/UMOD with GTF_DIV_MOD_NO_BY_ZERO if divisor is proven to be never zero // 3) Marks DIV/UDIV/MOD/UMOD with GTF_DIV_MOD_NO_OVERFLOW if both operands are proven to be never negative -// 4) UMOD with GTF_UMOD_UINT16_OPERANDS if both operands are proven to be in uint16 range +// 4) Marks UMOD with GTF_UMOD_UINT16_OPERANDS if both operands are proven to be in uint16 range // // Arguments: // assertions - set of live assertions diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 266914406bc619..eb59ba2842ab25 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -7221,9 +7221,9 @@ bool Lowering::LowerUnsignedDivOrMod(GenTreeOp* divMod) BlockRange().InsertBefore(divMod, multiplier, mul1, cast, shiftAmount); BlockRange().InsertBefore(divMod, mul2, shift); - // (int)result + // (int)result or (long)result divMod->ChangeOper(GT_CAST); - divMod->AsCast()->gtCastType = TYP_INT; + divMod->AsCast()->gtCastType = type; divMod->gtOp1 = shift; divMod->gtOp2 = nullptr; divMod->SetUnsigned(); From 3a2075e7cabbed28611b838fb70ff1c32d123574 Mon Sep 17 00:00:00 2001 From: Miha Zupan Date: Sat, 25 Jan 2025 14:41:33 +0100 Subject: [PATCH 08/11] Improve detection --- src/coreclr/jit/assertionprop.cpp | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index e88e0734cf0db6..b9067805503717 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -213,6 +213,16 @@ bool IntegralRange::Contains(int64_t value) const break; } + case GT_STORE_LCL_VAR: + { + if (node->gtGetOp1()->OperIs(GT_CAST)) + { + return ForCastOutput(node->gtGetOp1()->AsCast(), compiler); + } + + break; + } + case GT_CNS_INT: if (node->IsIntegralConst(0) || node->IsIntegralConst(1)) { @@ -227,6 +237,16 @@ bool IntegralRange::Contains(int64_t value) const case GT_CAST: return ForCastOutput(node->AsCast(), compiler); + case GT_COMMA: + { + if (varTypeIsIntegral(node->gtGetOp1())) + { + return ForNode(node->gtGetOp1(), compiler); + } + + break; + } + #if defined(FEATURE_HW_INTRINSICS) case GT_HWINTRINSIC: switch (node->AsHWIntrinsic()->GetHWIntrinsicId()) @@ -4161,8 +4181,8 @@ GenTree* Compiler::optAssertionProp_ModDiv(ASSERT_VALARG_TP assertions, GenTreeO } if (((tree->gtFlags & GTF_UMOD_UINT16_OPERANDS) == 0) && tree->OperIs(GT_UMOD) && op2->IsCnsIntOrI() && - FitsIn(op2->AsIntCon()->IconValue()) && op1IsNotNegative && - IntegralRange::ForNode(op1, this).GetUpperBound() <= SymbolicIntegerValue::UShortMax) + FitsIn(op2->AsIntCon()->IconValue()) && + IntegralRange::ForType(TYP_USHORT).Contains(IntegralRange::ForNode(op1, this))) { JITDUMP("Both operands for UMOD are in uint16 range...\n") tree->gtFlags |= GTF_UMOD_UINT16_OPERANDS; From 5ada191440cecbfa0011f62528bafb01134df7c5 Mon Sep 17 00:00:00 2001 From: Miha Zupan Date: Sat, 25 Jan 2025 20:26:35 +0100 Subject: [PATCH 09/11] Add some tests --- .../Divide/Regressions/Regression4_Divide.cs | 169 ++++++++++++++++++ .../Regressions/Regression4_Divide.csproj | 9 + 2 files changed, 178 insertions(+) create mode 100644 src/tests/JIT/opt/Divide/Regressions/Regression4_Divide.cs create mode 100644 src/tests/JIT/opt/Divide/Regressions/Regression4_Divide.csproj diff --git a/src/tests/JIT/opt/Divide/Regressions/Regression4_Divide.cs b/src/tests/JIT/opt/Divide/Regressions/Regression4_Divide.cs new file mode 100644 index 00000000000000..eec5af4a8dd5bc --- /dev/null +++ b/src/tests/JIT/opt/Divide/Regressions/Regression4_Divide.cs @@ -0,0 +1,169 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Numerics; +using System.Runtime.CompilerServices; +using System.Runtime.Intrinsics.X86; +using Xunit; + +public class Program +{ + private static ushort s_field1; + private static ulong s_field2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static uint Umod_U4_CharByZero(char c) + { + return (uint)c % 0; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static ushort Umod_U2_CharByConst(char c) + { + return (ushort)(c % 42); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static int Umod_I4_CharByConst(char c) + { + return c % 42; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static uint Umod_U4_CharByConst(char c) + { + return (uint)c % 42; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static long Umod_I8_CharByConst(char c) + { + return (long)c % 42; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static ulong Umod_U8_CharByConst(char c) + { + return (ulong)c % 42; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool TestIsWhiteSpace(char c) + { + ReadOnlySpan HashEntries = [' ', ' ', '\u00A0', ' ', ' ', ' ', ' ', ' ', ' ', '\t', '\n', '\v', '\f', '\r', ' ', ' ', '\u2028', '\u2029', ' ', ' ', ' ', ' ', ' ', '\u202F', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', '\u3000', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', '\u0085', '\u2000', '\u2001', '\u2002', '\u2003', '\u2004', '\u2005', '\u2006', '\u2007', '\u2008', '\u2009', '\u200A', ' ', ' ', ' ', ' ', ' ', '\u205F', '\u1680', ' ', ' ', ' ', ' ', ' ', ' ']; + return HashEntries[c % HashEntries.Length] == c; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static ushort Umod_TZC(ulong value) + { + return (ushort)(BitOperations.TrailingZeroCount(value) % 42); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static ushort Umod_TZC_Intrinsic(ulong value) + { + return (ushort)(Bmi1.X64.TrailingZeroCount(value) % 42); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static int Umod_UInt16Range_ByConst(int value) + { + if (value is > 0 and < 1234) + { + return value % 123; + } + + return -1; + } + + [MethodImpl(MethodImplOptions.AggressiveOptimization)] + private static void Test1() + { + for (int i = 0; i < 2; i++) + { + Core(); + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + static void Core() + { + s_field1 = (ushort)(Bmi1.X64.TrailingZeroCount(s_field2) % 42); + } + } + + [Fact] + public static int TestEntryPoint() + { + try + { + Umod_U4_CharByZero('a'); + return 0; + } + catch (DivideByZeroException) { } + + if (Umod_U2_CharByConst('a') != 13) + return 0; + + if (Umod_I4_CharByConst('a') != 13) + return 0; + + if (Umod_U4_CharByConst('a') != 13) + return 0; + + if (Umod_I8_CharByConst('a') != 13) + return 0; + + if (Umod_U8_CharByConst('a') != 13) + return 0; + + if (!TestIsWhiteSpace(' ')) + return 0; + + if (!TestIsWhiteSpace('\u2029')) + return 0; + + if (TestIsWhiteSpace('\0')) + return 0; + + if (TestIsWhiteSpace('a')) + return 0; + + if (Umod_TZC(1L << 40) != 40) + return 0; + + if (Umod_TZC(1L << 50) != 8) + return 0; + + if (Bmi1.X64.IsSupported) + { + if (Umod_TZC_Intrinsic(1L << 40) != 40) + return 0; + + if (Umod_TZC_Intrinsic(1L << 50) != 8) + return 0; + } + + if (Umod_UInt16Range_ByConst(0) != -1) + return 0; + + if (Umod_UInt16Range_ByConst(42) != 42) + return 0; + + if (Umod_UInt16Range_ByConst(123) != 0) + return 0; + + if (Bmi1.X64.IsSupported) + { + s_field1 = 1; + s_field2 = 1L << 50; + Test1(); + + if (s_field1 != 8) + return 0; + } + + return 100; + } +} diff --git a/src/tests/JIT/opt/Divide/Regressions/Regression4_Divide.csproj b/src/tests/JIT/opt/Divide/Regressions/Regression4_Divide.csproj new file mode 100644 index 00000000000000..501217e4d86892 --- /dev/null +++ b/src/tests/JIT/opt/Divide/Regressions/Regression4_Divide.csproj @@ -0,0 +1,9 @@ + + + None + True + + + + + From be5e488830fe9a983e1d7a9c1e8a38b2aa4deca3 Mon Sep 17 00:00:00 2001 From: Miha Zupan Date: Sun, 26 Jan 2025 19:58:05 +0100 Subject: [PATCH 10/11] Skip the opt on Arm for now --- src/coreclr/jit/assertionprop.cpp | 4 +++- src/coreclr/jit/gentree.h | 2 +- src/coreclr/jit/lower.cpp | 12 ++++-------- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index b9067805503717..053ad8030b24ea 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4180,14 +4180,16 @@ GenTree* Compiler::optAssertionProp_ModDiv(ASSERT_VALARG_TP assertions, GenTreeO changed = true; } +#ifdef TARGET_AMD64 if (((tree->gtFlags & GTF_UMOD_UINT16_OPERANDS) == 0) && tree->OperIs(GT_UMOD) && op2->IsCnsIntOrI() && - FitsIn(op2->AsIntCon()->IconValue()) && + FitsIn(op2->AsIntCon()->IconValue()) && op2->AsIntCon()->IconValue() > 0 && IntegralRange::ForType(TYP_USHORT).Contains(IntegralRange::ForNode(op1, this))) { JITDUMP("Both operands for UMOD are in uint16 range...\n") tree->gtFlags |= GTF_UMOD_UINT16_OPERANDS; changed = true; } +#endif return changed ? optAssertionProp_Update(tree, tree, stmt) : nullptr; } diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index d7e4c32f889b33..87d2a6d082e8f1 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -540,7 +540,7 @@ enum GenTreeFlags : unsigned int GTF_DIV_MOD_NO_OVERFLOW = 0x40000000, // GT_DIV, GT_MOD -- Div or mod definitely does not overflow. - GTF_UMOD_UINT16_OPERANDS = 0x80000000, // UMOD -- Both operands to a mod are in uint16 range. + GTF_UMOD_UINT16_OPERANDS = 0x80000000, // UMOD -- Both operands to a mod are in uint16 range. The divisor is non-zero constant. GTF_CHK_INDEX_INBND = 0x80000000, // GT_BOUNDS_CHECK -- have proven this check is always in-bounds diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index eb59ba2842ab25..02be3f248884c1 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -7184,17 +7184,13 @@ bool Lowering::LowerUnsignedDivOrMod(GenTreeOp* divMod) assert(divisorValue >= 3); - if (comp->opts.MinOpts()) - { - return false; - } - -#if defined(TARGET_64BIT) +#ifdef TARGET_AMD64 // Replace (uint16 % uint16) with a cheaper variant of FastMod, specialized for 16-bit operands. if ((divMod->gtFlags & GTF_UMOD_UINT16_OPERANDS) != 0) { assert(!isDiv); assert(divisorValue > 0 && divisorValue <= UINT16_MAX); + assert(!comp->opts.MinOpts()); // uint multiplier = uint.MaxValue / divisor + 1; // ulong result = ((ulong)(dividend * multiplier) * divisor) >> 32; @@ -7230,11 +7226,11 @@ bool Lowering::LowerUnsignedDivOrMod(GenTreeOp* divMod) ContainCheckRange(multiplier, divMod); return true; } -#endif +#endif // TARGET_AMD64 // TODO-ARM-CQ: Currently there's no GT_MULHI for ARM32 #if defined(TARGET_XARCH) || defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64) - if (divisorValue >= 3) + if (!comp->opts.MinOpts() && (divisorValue >= 3)) { size_t magic; bool increment; From 53db0689f4ad79f2e0312c50f4a5cb325cb5382d Mon Sep 17 00:00:00 2001 From: Miha Zupan Date: Tue, 28 Jan 2025 12:30:07 +0100 Subject: [PATCH 11/11] Test reverting some changes --- src/coreclr/jit/assertionprop.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 053ad8030b24ea..c589fd67377edc 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4166,14 +4166,14 @@ GenTree* Compiler::optAssertionProp_ModDiv(ASSERT_VALARG_TP assertions, GenTreeO changed = true; } - if (((tree->gtFlags & GTF_DIV_MOD_NO_BY_ZERO) == 0) && op2IsNotZero) + if (op2IsNotZero) { JITDUMP("Divisor for DIV/MOD is proven to be never negative...\n") tree->gtFlags |= GTF_DIV_MOD_NO_BY_ZERO; changed = true; } - if (((tree->gtFlags & GTF_DIV_MOD_NO_OVERFLOW) == 0) && (op1IsNotNegative || op2IsNotNegative)) + if (op1IsNotNegative || op2IsNotNegative) { JITDUMP("DIV/MOD is proven to never overflow...\n") tree->gtFlags |= GTF_DIV_MOD_NO_OVERFLOW;