From 56eff9f52534ebb226323ed0bf7ea36bb4a227d5 Mon Sep 17 00:00:00 2001 From: Will Smith Date: Tue, 2 May 2023 20:06:04 -0700 Subject: [PATCH] [JIT] ARM64 - Fix Assertion failed 'node->OperIs(GT_DIV, GT_UDIV, GT_MOD)' during 'Lowering nodeinfo' (#85664) * Rename LowerConstIntDivOrMode to TryLowerConstIntDivOrMod with an out parameter for the next node to lower * Add test case * Simplify test * Add extra assert --- src/coreclr/jit/lower.cpp | 45 +++-- src/coreclr/jit/lower.h | 2 +- .../JitBlue/Runtime_85630/Runtime_85630.cs | 181 ++++++++++++++++++ .../Runtime_85630/Runtime_85630.csproj | 8 + 4 files changed, 215 insertions(+), 21 deletions(-) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_85630/Runtime_85630.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_85630/Runtime_85630.csproj diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 33d31bb3db95c0..de2cd661d7f250 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -6596,14 +6596,16 @@ bool Lowering::LowerUnsignedDivOrMod(GenTreeOp* divMod) // // Arguments: // node - pointer to the DIV or MOD node +// nextNode - out parameter for the next node in the transformed node sequence that needs to be lowered // // Returns: -// nullptr if no transformation is done, or the next node in the transformed node sequence that -// needs to be lowered. +// false if no transformation is done, true if a transformation is done // -GenTree* Lowering::LowerConstIntDivOrMod(GenTree* node) +bool Lowering::TryLowerConstIntDivOrMod(GenTree* node, GenTree** nextNode) { assert((node->OperGet() == GT_DIV) || (node->OperGet() == GT_MOD)); + assert(nextNode != nullptr); + GenTree* divMod = node; GenTree* dividend = divMod->gtGetOp1(); GenTree* divisor = divMod->gtGetOp2(); @@ -6618,14 +6620,15 @@ GenTree* Lowering::LowerConstIntDivOrMod(GenTree* node) if (divMod->OperIs(GT_MOD) && divisor->IsIntegralConstPow2()) { LowerModPow2(node); - return node->gtNext; + *nextNode = node->gtNext; + return true; } assert(node->OperGet() != GT_MOD); #endif // TARGET_ARM64 if (!divisor->IsCnsIntOrI()) { - return nullptr; // no transformations to make + return false; // no transformations to make } if (dividend->IsCnsIntOrI()) @@ -6633,7 +6636,7 @@ GenTree* Lowering::LowerConstIntDivOrMod(GenTree* node) // We shouldn't see a divmod with constant operands here but if we do then it's likely // because optimizations are disabled or it's a case that's supposed to throw an exception. // Don't optimize this. - return nullptr; + return false; } ssize_t divisorValue = divisor->AsIntCon()->IconValue(); @@ -6649,7 +6652,7 @@ GenTree* Lowering::LowerConstIntDivOrMod(GenTree* node) // case so optimizing this case would break C# code. // A runtime check could be used to handle this case but it's probably too rare to matter. - return nullptr; + return false; } bool isDiv = divMod->OperGet() == GT_DIV; @@ -6661,7 +6664,8 @@ GenTree* Lowering::LowerConstIntDivOrMod(GenTree* node) // If the divisor is the minimum representable integer value then we can use a compare, // the result is 1 iff the dividend equals divisor. divMod->SetOper(GT_EQ); - return node; + *nextNode = node; + return true; } } @@ -6672,7 +6676,7 @@ GenTree* Lowering::LowerConstIntDivOrMod(GenTree* node) { if (comp->opts.MinOpts()) { - return nullptr; + return false; } #if defined(TARGET_XARCH) || defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64) @@ -6774,13 +6778,14 @@ GenTree* Lowering::LowerConstIntDivOrMod(GenTree* node) divMod->AsOp()->gtOp2 = mul; } - return mulhi; + *nextNode = mulhi; + return true; #elif defined(TARGET_ARM) // Currently there's no GT_MULHI for ARM32 - return nullptr; + return false; #elif defined(TARGET_RISCV64) NYI_RISCV64("-----unimplemented on RISCV64 yet----"); - return nullptr; + return false; #else #error Unsupported or unset target architecture #endif @@ -6790,7 +6795,7 @@ GenTree* Lowering::LowerConstIntDivOrMod(GenTree* node) LIR::Use use; if (!BlockRange().TryGetUse(node, &use)) { - return nullptr; + return false; } // We need to use the dividend node multiple times so its value needs to be @@ -6856,7 +6861,8 @@ GenTree* Lowering::LowerConstIntDivOrMod(GenTree* node) // replace the original divmod node with the new divmod tree use.ReplaceWith(newDivMod); - return newDivMod->gtNext; + *nextNode = newDivMod->gtNext; + return true; } //------------------------------------------------------------------------ // LowerSignedDivOrMod: transform integer GT_DIV/GT_MOD nodes with a power of 2 @@ -6871,20 +6877,19 @@ GenTree* Lowering::LowerConstIntDivOrMod(GenTree* node) GenTree* Lowering::LowerSignedDivOrMod(GenTree* node) { assert((node->OperGet() == GT_DIV) || (node->OperGet() == GT_MOD)); - GenTree* next = node->gtNext; if (varTypeIsIntegral(node->TypeGet())) { - // LowerConstIntDivOrMod will return nullptr if it doesn't transform the node. - GenTree* newNode = LowerConstIntDivOrMod(node); - if (newNode != nullptr) + GenTree* nextNode = nullptr; + if (TryLowerConstIntDivOrMod(node, &nextNode)) { - return newNode; + return nextNode; } + assert(nextNode == nullptr); } ContainCheckDivOrMod(node->AsOp()); - return next; + return node->gtNext; } //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h index 74ee828c2e9a2b..3e4bd7ddc7292d 100644 --- a/src/coreclr/jit/lower.h +++ b/src/coreclr/jit/lower.h @@ -312,7 +312,7 @@ class Lowering final : public Phase bool TryLowerAndNegativeOne(GenTreeOp* node, GenTree** nextNode); GenTree* LowerBinaryArithmetic(GenTreeOp* binOp); bool LowerUnsignedDivOrMod(GenTreeOp* divMod); - GenTree* LowerConstIntDivOrMod(GenTree* node); + bool TryLowerConstIntDivOrMod(GenTree* node, GenTree** nextNode); GenTree* LowerSignedDivOrMod(GenTree* node); void LowerBlockStore(GenTreeBlk* blkNode); void LowerBlockStoreCommon(GenTreeBlk* blkNode); diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_85630/Runtime_85630.cs b/src/tests/JIT/Regression/JitBlue/Runtime_85630/Runtime_85630.cs new file mode 100644 index 00000000000000..53a1442ac6d0e6 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_85630/Runtime_85630.cs @@ -0,0 +1,181 @@ +// 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.Collections; +using System.Collections.Generic; +using System.Runtime.CompilerServices; +using Xunit; + +public class Test +{ + public class TestClass + { + public struct S1 + { + public float float_0; + } + public struct S2 + { + public struct S2_D1_F1 + { + public uint uint_1; + } + } + static bool s_bool_2 = true; + static byte s_byte_3 = 1; + static char s_char_4 = 'M'; + static decimal s_decimal_5 = 2.0405405405405405405405405405m; + static double s_double_6 = -1.8846153846153846; + static short s_short_7 = -1; + static int s_int_8 = -5; + static long s_long_9 = -5; + static sbyte s_sbyte_10 = 1; + static float s_float_11 = -4.952381f; + static string s_string_12 = "JZDP"; + static ushort s_ushort_13 = 1; + static uint s_uint_14 = 5; + static ulong s_ulong_15 = 2; + static S1 s_s1_16 = new S1(); + static S2.S2_D1_F1 s_s2_s2_d1_f1_17 = new S2.S2_D1_F1(); + static S2 s_s2_18 = new S2(); + bool bool_19 = true; + byte byte_20 = 0; + char char_21 = 'W'; + decimal decimal_22 = 2.2307692307692307692307692308m; + double double_23 = -1; + short short_24 = 0; + int int_25 = 0; + long long_26 = 2; + sbyte sbyte_27 = -2; + float float_28 = 0.071428575f; + string string_29 = "MNK"; + ushort ushort_30 = 2; + uint uint_31 = 1; + ulong ulong_32 = 31; + S1 s1_33 = new S1(); + S2.S2_D1_F1 s2_s2_d1_f1_34 = new S2.S2_D1_F1(); + S2 s2_35 = new S2(); + static int s_loopInvariant = 3; + private static List toPrint = new List(); + [MethodImpl(MethodImplOptions.NoInlining)] + public bool LeafMethod0() + { + unchecked + { + return ((bool)(((ulong)(((ulong)(((int)(int_25 % ((int)((int_25) | 66)))) % ((int)((((int)(int_25 | s_int_8))) | 53)))) + ulong_32)) == ((ulong)(((ulong)(((ulong)(s_ulong_15 ^ s_ulong_15)) + ((ulong)(s_int_8 % ((int)((s_int_8) | 49)))))) * ((ulong)(((ulong)(ulong_32 * s_ulong_15)) | ulong_32)))))); + } + } + public byte LeafMethod1() + { + unchecked + { + return ((byte)(byte_20 - ((byte)(((int)(s_int_8 / ((int)((((int)(s_int_8 %= ((int)((-1) | 4))))) | 81)))) / ((int)((((int)(int_25 = ((int)(s_int_8 - 5))))) | 85)))))); + } + } + public char LeafMethod2() + { + unchecked + { + return s_char_4; + } + } + public decimal LeafMethod3() + { + unchecked + { + return ((decimal)(((int)(((int)(((int)(s_int_8 &= s_int_8)) >> ((int)(int_25 >> s_int_8)))) ^ ((int)(int_25 / ((int)((((int)(1 * int_25))) | 83)))))) / ((int)((((int)(((int)(int_25 *= ((int)(int_25 % ((int)((int_25) | 1)))))) >> ((int)(((int)(s_int_8 / ((int)((s_int_8) | 18)))) & ((int)(31 & int_25))))))) | 20)))); + } + } + public double LeafMethod4() + { + unchecked + { + return ((double)(((double)(s_double_6 *= ((double)(((double)(s_int_8 % ((int)((s_int_8) | 4)))) * ((double)(s_double_6 += s_double_6)))))) + ((double)(((int)(s_int_8 <<= ((int)(s_int_8 | int_25)))) % ((int)((((int)(s_int_8 = -5))) | 16)))))); + } + } + public short LeafMethod5() + { + unchecked + { + return ((short)(s_short_7 + ((short)(short_24 |= ((short)(((short)(s_int_8 / ((int)((int_25) | 30)))) << ((int)(int_25 + 5)))))))); + } + } + public int LeafMethod6() + { + unchecked + { + return ((int)(int_25 <<= ((int)(((int)(((int)(s_int_8 * s_int_8)) << ((int)(int_25 - 31)))) & s_int_8)))); + } + } + public long LeafMethod7() + { + unchecked + { + return ((long)(((long)(s_long_9 += ((long)(((long)(s_long_9 >>= LeafMethod6())) >> s_int_8)))) - ((long)(long_26 = long_26)))); + } + } + public S1 Method23(out S1 p_s1_653, out S1 p_s1_654, ref int p_int_655, S2.S2_D1_F1 p_s2_s2_d1_f1_656, out S2 p_s2_657, ref S2.S2_D1_F1 p_s2_s2_d1_f1_658, float p_float_659) + { + unchecked + { + bool bool_660 = true; + byte byte_661 = 2; + char char_662 = 'Y'; + decimal decimal_663 = 1.0476190476190476190476190476m; + double double_664 = -1.95; + short short_665 = 31; + int int_666 = 0; + long long_667 = 31; + sbyte sbyte_668 = 0; + float float_669 = 31f; + string string_670 = "3P5A58X"; + ushort ushort_671 = 5; + uint uint_672 = 2; + ulong ulong_673 = 2; + S1 s1_674 = new S1(); + S2.S2_D1_F1 s2_s2_d1_f1_675 = new S2.S2_D1_F1(); + S2 s2_676 = new S2(); + p_s1_653 = s1_674; + p_s1_654 = s_s1_16; + switch (((long)(((long)(s_long_9 *= ((long)(((int)(s_int_8 <<= int_25)) % ((int)((((int)(s_int_8 | s_int_8))) | 82)))))) ^ ((long)(((long)(int_25 /= ((int)((((int)(s_int_8 = LeafMethod6()))) | 38)))) + ((long)(long_26 + ((long)(-2 & LeafMethod7()))))))))) + { + case -2147483648: + { + int __loopvar0 = s_loopInvariant; + break; + } + case 31: + { + break; + } + default: + { + long_667 += ((long)(long_667 |= ((long)(long_667 & ((long)(((int)(LeafMethod6() % ((int)((p_int_655) | 24)))) % ((int)((int_666) | 2)))))))); + break; + } + } + return s_s1_16; + } + } + public void Method0() + { + unchecked + { + int int_2775 = -2147483648; + S2.S2_D1_F1 s2_s2_d1_f1_2784 = new S2.S2_D1_F1(); + int __loopvar2 = s_loopInvariant, __loopSecondaryVar2_0 = s_loopInvariant; + s_s1_16 = Method23(out s1_33, out s_s1_16, ref int_2775, s2_s2_d1_f1_2784, out s_s2_18, ref s_s2_s2_d1_f1_17, ((float)(s_int_8 /= ((int)((((int)(((int)(int_25 % ((int)((LeafMethod6()) | 91)))) * ((int)(int_25 >> s_int_8))))) | 29))))); + return; + } + } + } + + // This is trying to stress the JIT to ensure we do not encounter an assertion. + [Fact] + public static int TestEntryPoint() + { + new TestClass().Method0(); + return 100; + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_85630/Runtime_85630.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_85630/Runtime_85630.csproj new file mode 100644 index 00000000000000..15edd99711a1a4 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_85630/Runtime_85630.csproj @@ -0,0 +1,8 @@ + + + True + + + + + \ No newline at end of file