From 4973c823cc2ff285d37411e32654622a29bd6e40 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Fri, 17 Jun 2022 15:13:11 +0200 Subject: [PATCH 01/13] JIT: Stop reordering arguments throwing exceptions Implement a precise scan for arguments that may throw exceptions which collects the exceptions they may throw. When we see two such interfering sets, make sure that the first argument is evaluated to a temp so that they do not get reordered by sort. Fix #63905 --- src/coreclr/jit/compiler.h | 9 +- src/coreclr/jit/flowgraph.cpp | 41 --------- src/coreclr/jit/gentree.cpp | 168 ++++++++++++++++++++++++++++------ src/coreclr/jit/gentree.h | 39 +++++++- src/coreclr/jit/morph.cpp | 76 +++++++++++---- 5 files changed, 238 insertions(+), 95 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index a7ece7e80e9cd3..96bbb455fe1dc7 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -5354,13 +5354,6 @@ class Compiler void* pCallBackData = nullptr, bool computeStack = false); - // An fgWalkPreFn that looks for expressions that have inline throws in - // minopts mode. Basically it looks for tress with gtOverflowEx() or - // GTF_IND_RNGCHK. It returns WALK_ABORT if one is found. It - // returns WALK_SKIP_SUBTREES if GTF_EXCEPT is not set (assumes flags - // properly propagated to parent trees). It returns WALK_CONTINUE - // otherwise. - static fgWalkResult fgChkThrowCB(GenTree** pTree, Compiler::fgWalkData* data); static fgWalkResult fgChkLocAllocCB(GenTree** pTree, Compiler::fgWalkData* data); static fgWalkResult fgChkQmarkCB(GenTree** pTree, Compiler::fgWalkData* data); @@ -5880,6 +5873,8 @@ class Compiler bool gtIsTypeHandleToRuntimeTypeHandleHelper(GenTreeCall* call, CorInfoHelpFunc* pHelper = nullptr); bool gtIsActiveCSE_Candidate(GenTree* tree); + ExceptionSetFlags gtCollectPreciseExceptions(GenTree* tree); + bool fgIsBigOffset(size_t offset); bool fgNeedReturnSpillTemp(); diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 52399aa6c16929..570f5ef4957329 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -4129,47 +4129,6 @@ void Compiler::fgSetBlockOrder(BasicBlock* block) return firstNode; } -/*static*/ Compiler::fgWalkResult Compiler::fgChkThrowCB(GenTree** pTree, fgWalkData* data) -{ - GenTree* tree = *pTree; - - // If this tree doesn't have the EXCEPT flag set, then there is no - // way any of the child nodes could throw, so we can stop recursing. - if (!(tree->gtFlags & GTF_EXCEPT)) - { - return Compiler::WALK_SKIP_SUBTREES; - } - - switch (tree->gtOper) - { - case GT_MUL: - case GT_ADD: - case GT_SUB: - case GT_CAST: - if (tree->gtOverflow()) - { - return Compiler::WALK_ABORT; - } - break; - - case GT_INDEX_ADDR: - // This calls CORINFO_HELP_RNGCHKFAIL for Debug code. - if (tree->AsIndexAddr()->IsBoundsChecked()) - { - return Compiler::WALK_ABORT; - } - break; - - case GT_BOUNDS_CHECK: - return Compiler::WALK_ABORT; - - default: - break; - } - - return Compiler::WALK_CONTINUE; -} - /*static*/ Compiler::fgWalkResult Compiler::fgChkLocAllocCB(GenTree** pTree, fgWalkData* data) { GenTree* tree = *pTree; diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 9e56b90be61a43..f9ebb8cf03f12b 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -6351,16 +6351,20 @@ bool GenTree::OperIsImplicitIndir() const } //------------------------------------------------------------------------------ -// OperMayThrow : Check whether the operation may throw. +// OperPreciseExceptions : Get exception set this tree may throw. // // // Arguments: // comp - Compiler instance // // Return Value: -// True if the given operator may cause an exception - -bool GenTree::OperMayThrow(Compiler* comp) +// A bit set of exceptions this tree may throw. +// +// Remarks: +// Should not be used on calls given that we can say nothing precise about +// those. +// +ExceptionSetFlags GenTree::OperPreciseExceptions(Compiler* comp) { GenTree* op; @@ -6377,26 +6381,40 @@ bool GenTree::OperMayThrow(Compiler* comp) if (varTypeIsFloating(op->TypeGet())) { - return false; // Floating point division does not throw. + return ExceptionSetFlags::None; } // For integers only division by 0 or by -1 can throw - if (op->IsIntegralConst() && !op->IsIntegralConst(0) && !op->IsIntegralConst(-1)) + if (op->IsIntegralConst()) { - return false; + if (op->IsIntegralConst(0)) + { + return ExceptionSetFlags::DivideByZeroException; + } + if (op->IsIntegralConst(-1)) + { + return ExceptionSetFlags::ArithmeticException; + } + + return ExceptionSetFlags::None; } - return true; + + return ExceptionSetFlags::DivideByZeroException | ExceptionSetFlags::ArithmeticException; case GT_INTRINSIC: // If this is an intrinsic that represents the object.GetType(), it can throw an NullReferenceException. // Currently, this is the only intrinsic that can throw an exception. - return AsIntrinsic()->gtIntrinsicName == NI_System_Object_GetType; + if (AsIntrinsic()->gtIntrinsicName == NI_System_Object_GetType) + { + return ExceptionSetFlags::NullReferenceException; + } + + return ExceptionSetFlags::None; case GT_CALL: + assert(!"Unexpected GT_CALL in OperPreciseExceptions"); - CorInfoHelpFunc helper; - helper = comp->eeGetHelperNum(this->AsCall()->gtCallMethHnd); - return ((helper == CORINFO_HELP_UNDEF) || !comp->s_helperCallProperties.NoThrow(helper)); + return static_cast(0xffffffff); case GT_IND: case GT_BLK: @@ -6404,14 +6422,28 @@ bool GenTree::OperMayThrow(Compiler* comp) case GT_NULLCHECK: case GT_STORE_BLK: case GT_STORE_DYN_BLK: - return (((this->gtFlags & GTF_IND_NONFAULTING) == 0) && comp->fgAddrCouldBeNull(this->AsIndir()->Addr())); + if (((this->gtFlags & GTF_IND_NONFAULTING) == 0) && comp->fgAddrCouldBeNull(this->AsIndir()->Addr())) + { + return ExceptionSetFlags::NullReferenceException; + } + + return ExceptionSetFlags::None; case GT_ARR_LENGTH: - return (((this->gtFlags & GTF_IND_NONFAULTING) == 0) && - comp->fgAddrCouldBeNull(this->AsArrLen()->ArrRef())); + if (((this->gtFlags & GTF_IND_NONFAULTING) == 0) && comp->fgAddrCouldBeNull(this->AsArrLen()->ArrRef())) + { + return ExceptionSetFlags::NullReferenceException; + } + + return ExceptionSetFlags::None; case GT_ARR_ELEM: - return comp->fgAddrCouldBeNull(this->AsArrElem()->gtArrObj); + if (comp->fgAddrCouldBeNull(this->AsArrElem()->gtArrObj)) + { + return ExceptionSetFlags::NullReferenceException; + } + + return ExceptionSetFlags::None; case GT_FIELD: { @@ -6419,19 +6451,28 @@ bool GenTree::OperMayThrow(Compiler* comp) if (fldObj != nullptr) { - return comp->fgAddrCouldBeNull(fldObj); + if (comp->fgAddrCouldBeNull(fldObj)) + { + return ExceptionSetFlags::NullReferenceException; + } } - return false; + return ExceptionSetFlags::None; } case GT_BOUNDS_CHECK: + case GT_INDEX_ADDR: + return ExceptionSetFlags::IndexOutOfRangeException; + case GT_ARR_INDEX: case GT_ARR_OFFSET: - case GT_LCLHEAP: + return ExceptionSetFlags::NullReferenceException; + case GT_CKFINITE: - case GT_INDEX_ADDR: - return true; + return ExceptionSetFlags::ArithmeticException; + + case GT_LCLHEAP: + return ExceptionSetFlags::StackOverflowException; #ifdef FEATURE_HW_INTRINSICS case GT_HWINTRINSIC: @@ -6443,23 +6484,42 @@ bool GenTree::OperMayThrow(Compiler* comp) // This operation contains an implicit indirection // it could throw a null reference exception. // - return true; + return ExceptionSetFlags::NullReferenceException; } - break; + + return ExceptionSetFlags::None; } #endif // FEATURE_HW_INTRINSICS default: - break; - } + if (gtOverflowEx()) + { + return ExceptionSetFlags::OverflowException; + } - /* Overflow arithmetic operations also throw exceptions */ + return ExceptionSetFlags::None; + } +} - if (gtOverflowEx()) +//------------------------------------------------------------------------------ +// OperMayThrow : Check whether the operation may throw. +// +// +// Arguments: +// comp - Compiler instance +// +// Return Value: +// True if the given operator may cause an exception +// +bool GenTree::OperMayThrow(Compiler* comp) +{ + if (OperIs(GT_CALL)) { - return true; + CorInfoHelpFunc helper; + helper = comp->eeGetHelperNum(this->AsCall()->gtCallMethHnd); + return ((helper == CORINFO_HELP_UNDEF) || !comp->s_helperCallProperties.NoThrow(helper)); } - return false; + return OperPreciseExceptions(comp) != ExceptionSetFlags::None; } //----------------------------------------------------------------------------------- @@ -16135,6 +16195,56 @@ bool Compiler::gtIsActiveCSE_Candidate(GenTree* tree) return (optValnumCSE_phase && IS_CSE_INDEX(tree->gtCSEnum)); } +ExceptionSetFlags Compiler::gtCollectPreciseExceptions(GenTree* tree) +{ + class PreciseExceptionsWalker final : public GenTreeVisitor + { + ExceptionSetFlags m_preciseExceptions = ExceptionSetFlags::None; + + public: + PreciseExceptionsWalker(Compiler* comp) : GenTreeVisitor(comp) + { + } + + enum + { + DoPreOrder = true, + }; + + ExceptionSetFlags GetFlags() + { + return m_preciseExceptions; + } + + fgWalkResult PreOrderVisit(GenTree** use, GenTree* user) + { + GenTree* tree = *use; + if ((tree->gtFlags & GTF_EXCEPT) == 0) + { + return WALK_SKIP_SUBTREES; + } + + m_preciseExceptions |= tree->OperPreciseExceptions(m_compiler); + return WALK_CONTINUE; + } + }; + + // We only expect the caller to ask for precise exceptions for cases where + // it may help with disambiguating between exceptions. If the tree contains + // a call it can always throw arbitrary exceptions. + assert((tree->gtFlags & GTF_CALL) == 0); + + if ((tree->gtFlags & GTF_EXCEPT) == 0) + { + return ExceptionSetFlags::None; + } + + PreciseExceptionsWalker walker(this); + walker.WalkTree(&tree, nullptr); + assert(walker.GetFlags() != ExceptionSetFlags::None); + return walker.GetFlags(); +} + /*****************************************************************************/ struct ComplexityStruct diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index ff72fae3378d16..b4b829110ebe79 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -130,6 +130,42 @@ enum gtCallTypes : BYTE CT_COUNT // fake entry (must be last) }; +enum class ExceptionSetFlags +{ + None = 0x0, + OverflowException = 0x1, + DivideByZeroException = 0x2, + ArithmeticException = 0x4, + NullReferenceException = 0x8, + IndexOutOfRangeException = 0x10, + StackOverflowException = 0x20, +}; + +inline constexpr ExceptionSetFlags operator~(ExceptionSetFlags a) +{ + return (ExceptionSetFlags)(~(unsigned short)a); +} + +inline constexpr ExceptionSetFlags operator|(ExceptionSetFlags a, ExceptionSetFlags b) +{ + return (ExceptionSetFlags)((unsigned short)a | (unsigned short)b); +} + +inline constexpr ExceptionSetFlags operator&(ExceptionSetFlags a, ExceptionSetFlags b) +{ + return (ExceptionSetFlags)((unsigned short)a & (unsigned short)b); +} + +inline ExceptionSetFlags& operator|=(ExceptionSetFlags& a, ExceptionSetFlags b) +{ + return a = (ExceptionSetFlags)((unsigned short)a | (unsigned short)b); +} + +inline ExceptionSetFlags& operator&=(ExceptionSetFlags& a, ExceptionSetFlags b) +{ + return a = (ExceptionSetFlags)((unsigned short)a & (unsigned short)b); +} + #ifdef DEBUG /***************************************************************************** * @@ -1810,6 +1846,7 @@ struct GenTree bool OperRequiresCallFlag(Compiler* comp); bool OperMayThrow(Compiler* comp); + ExceptionSetFlags OperPreciseExceptions(Compiler* comp); unsigned GetScaleIndexMul(); unsigned GetScaleIndexShf(); @@ -4578,6 +4615,7 @@ class CallArgs void RemovedWellKnownArg(WellKnownArg arg); regNumber GetCustomRegister(Compiler* comp, CorInfoCallConvExtension cc, WellKnownArg arg); void SplitArg(CallArg* arg, unsigned numRegs, unsigned numSlots); + void SortArgs(Compiler* comp, GenTreeCall* call, CallArg** sortedArgs); public: CallArgs(); @@ -4622,7 +4660,6 @@ class CallArgs void AddFinalArgsAndDetermineABIInfo(Compiler* comp, GenTreeCall* call); void ArgsComplete(Compiler* comp, GenTreeCall* call); - void SortArgs(Compiler* comp, GenTreeCall* call, CallArg** sortedArgs); void EvalArgsToTemps(Compiler* comp, GenTreeCall* call); void SetNeedsTemp(CallArg* arg); bool IsNonStandard(Compiler* comp, GenTreeCall* call, CallArg* arg); diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index ac07cb20ddc18e..ff8e0ed0b89253 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -735,6 +735,12 @@ void CallArgs::ArgsComplete(Compiler* comp, GenTreeCall* call) unsigned argCount = CountArgs(); + // Overapproximation of exception flags for previous args that may not + // already be evaluated into temps. + ExceptionSetFlags nonTempExFlags = ExceptionSetFlags::None; + + CallArg* prevNonTempExceptionThrowingArg = nullptr; + for (CallArg& arg : Args()) { GenTree* argx = arg.GetEarlyNode(); @@ -816,6 +822,11 @@ void CallArgs::ArgsComplete(Compiler* comp, GenTreeCall* call) } bool treatLikeCall = ((argx->gtFlags & GTF_CALL) != 0); + + // TODO: Keep previous GTF_EXCEPT arg and lazily collect this set instead. + ExceptionSetFlags preciseExceptions = + treatLikeCall ? ExceptionSetFlags::None : comp->gtCollectPreciseExceptions(argx); + #if FEATURE_FIXED_OUT_ARGS // Like calls, if this argument has a tree that will do an inline throw, // a call to a jit helper, then we need to treat it like a call (but only @@ -824,32 +835,34 @@ void CallArgs::ArgsComplete(Compiler* comp, GenTreeCall* call) // conservative, but I want to avoid as much special-case debug-only code // as possible, so leveraging the GTF_CALL flag is the easiest. // - if (!treatLikeCall && (argx->gtFlags & GTF_EXCEPT) && (argCount > 1) && comp->opts.compDbgCode && - (comp->fgWalkTreePre(&argx, Compiler::fgChkThrowCB) == Compiler::WALK_ABORT)) + if (!treatLikeCall && (argCount > 1) && comp->opts.compDbgCode) { - for (CallArg& otherArg : Args()) + if ((preciseExceptions & (ExceptionSetFlags::IndexOutOfRangeException | + ExceptionSetFlags::OverflowException)) != ExceptionSetFlags::None) { - if (&otherArg == &arg) + for (CallArg& otherArg : Args()) { - continue; - } + if (&otherArg == &arg) + { + continue; + } - if (otherArg.AbiInfo.GetRegNum() == REG_STK) - { - treatLikeCall = true; - break; + if (otherArg.AbiInfo.GetRegNum() == REG_STK) + { + treatLikeCall = true; + break; + } } } } #endif // FEATURE_FIXED_OUT_ARGS - /* If it contains a call (GTF_CALL) then itself and everything before the call - with a GLOB_EFFECT must eval to temp (this is because everything with SIDE_EFFECT - has to be kept in the right order since we will move the call to the first position) + // If it contains a call (GTF_CALL) then itself and everything before the call + // with a GLOB_EFFECT must eval to temp (this is because everything with SIDE_EFFECT + // has to be kept in the right order since we will move the call to the first position) - For calls we don't have to be quite as conservative as we are with an assignment - since the call won't be modifying any non-address taken LclVars. - */ + // For calls we don't have to be quite as conservative as we are with an assignment + // since the call won't be modifying any non-address taken LclVars. if (treatLikeCall) { @@ -894,6 +907,35 @@ void CallArgs::ArgsComplete(Compiler* comp, GenTreeCall* call) #endif } } + else + { + // If a previous arg may throw a different exception than this arg, + // then we evaluate all previous arguments with GTF_EXCEPT to temps + // to avoid reordering them in our sort late. The only case we can + // avoid this is if all previous args throw the same single + // exception as this arg. + unsigned numExceptions = genCountBits(static_cast(preciseExceptions)); + bool distinctExceptions = + (nonTempExFlags != ExceptionSetFlags::None) && + ((numExceptions > 1) || ((numExceptions == 1) && (preciseExceptions != nonTempExFlags))); + if (distinctExceptions) + { + for (CallArg& prevArg : Args()) + { + if (&prevArg == &arg) + { + break; + } + + if ((prevArg.GetEarlyNode() != nullptr) && ((prevArg.GetEarlyNode()->gtFlags & GTF_EXCEPT) != 0)) + { + SetNeedsTemp(&prevArg); + } + } + } + + nonTempExFlags |= preciseExceptions; + } #if FEATURE_MULTIREG_ARGS // For RyuJIT backend we will expand a Multireg arg into a GT_FIELD_LIST @@ -1786,7 +1828,7 @@ void CallArgs::EvalArgsToTemps(Compiler* comp, GenTreeCall* call) #ifdef DEBUG if (comp->verbose) { - printf("\nShuffled argument table: "); + printf("\nRegister placement order: "); for (CallArg& arg : LateArgs()) { if (arg.AbiInfo.GetRegNum() != REG_STK) From 15231fbde7b3f41c495741cb0d5dad9037517542 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Fri, 17 Jun 2022 15:20:24 +0200 Subject: [PATCH 02/13] Add test --- .../JitBlue/Runtime_63905/Runtime_63905.cs | 46 +++++++++++++++++++ .../Runtime_63905/Runtime_63905.csproj | 9 ++++ 2 files changed, 55 insertions(+) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_63905/Runtime_63905.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_63905/Runtime_63905.csproj diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_63905/Runtime_63905.cs b/src/tests/JIT/Regression/JitBlue/Runtime_63905/Runtime_63905.cs new file mode 100644 index 00000000000000..5749133e9ee559 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_63905/Runtime_63905.cs @@ -0,0 +1,46 @@ +// 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.Runtime.CompilerServices; + +public class Runtime_63905 +{ + public static int Main() + { + C c = GetNull(); + int i = GetOne(); + try + { + Foo(c.Field, i / 0); + } + catch (NullReferenceException) + { + Console.WriteLine("PASS: Caught NullReferenceException in first argument"); + return 100; + } + catch (DivideByZeroException) + { + Console.WriteLine("FAIL: Arguments were reordered incorrectly"); + return -1; + } + + return -1; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static C GetNull() => null; + + [MethodImpl(MethodImplOptions.NoInlining)] + private static int GetOne() => 1; + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void Foo(object o, int val) + { + } + + private class C + { + public object Field; + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_63905/Runtime_63905.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_63905/Runtime_63905.csproj new file mode 100644 index 00000000000000..f492aeac9d056b --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_63905/Runtime_63905.csproj @@ -0,0 +1,9 @@ + + + Exe + True + + + + + \ No newline at end of file From 1536b67b3a5746e1174c0760b16db85f882c2085 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Fri, 17 Jun 2022 15:20:42 +0200 Subject: [PATCH 03/13] Allow more permissive forward subbing into call arguments --- src/coreclr/jit/forwardsub.cpp | 41 +--------------------------------- 1 file changed, 1 insertion(+), 40 deletions(-) diff --git a/src/coreclr/jit/forwardsub.cpp b/src/coreclr/jit/forwardsub.cpp index 4c3fae5f1d9f9e..a05ef79fd572fc 100644 --- a/src/coreclr/jit/forwardsub.cpp +++ b/src/coreclr/jit/forwardsub.cpp @@ -187,7 +187,6 @@ class ForwardSubVisitor final : public GenTreeVisitor public: enum { - ComputeStack = true, DoPostOrder = true, UseExecutionOrder = true }; @@ -197,7 +196,6 @@ class ForwardSubVisitor final : public GenTreeVisitor , m_use(nullptr) , m_node(nullptr) , m_parentNode(nullptr) - , m_callAncestor(nullptr) , m_lclNum(lclNum) , m_useCount(0) , m_useFlags(GTF_EMPTY) @@ -221,7 +219,7 @@ class ForwardSubVisitor final : public GenTreeVisitor // Screen out contextual "uses" // - GenTree* const parent = m_ancestors.Top(1); + GenTree* const parent = user; bool const isDef = parent->OperIs(GT_ASG) && (parent->gtGetOp1() == node); bool const isAddr = parent->OperIs(GT_ADDR); @@ -243,19 +241,6 @@ class ForwardSubVisitor final : public GenTreeVisitor m_use = use; m_useFlags = m_accumulatedFlags; m_parentNode = parent; - - // If this use contributes to a call arg we need to - // remember the call and handle it specially when we - // see it later in the postorder walk. - // - for (int i = 1; i < m_ancestors.Height(); i++) - { - if (m_ancestors.Top(i)->IsCall()) - { - m_callAncestor = m_ancestors.Top(i)->AsCall(); - break; - } - } } } } @@ -274,29 +259,6 @@ class ForwardSubVisitor final : public GenTreeVisitor } } - // Is this the use's call ancestor? - // - if ((m_callAncestor != nullptr) && (node == m_callAncestor)) - { - // To be conservative and avoid issues with morph - // reordering call args, we merge in effects of all args - // to this call. - // - // Remove this if/when morph's arg sorting is fixed. - // - GenTreeFlags oldUseFlags = m_useFlags; - - for (CallArg& arg : m_callAncestor->gtArgs.Args()) - { - m_useFlags |= (arg.GetNode()->gtFlags & GTF_GLOB_EFFECT); - } - - if (oldUseFlags != m_useFlags) - { - JITDUMP(" [added other call arg use flags: 0x%x]", m_useFlags & ~oldUseFlags); - } - } - m_accumulatedFlags |= (node->gtFlags & GTF_GLOB_EFFECT); return fgWalkResult::WALK_CONTINUE; @@ -341,7 +303,6 @@ class ForwardSubVisitor final : public GenTreeVisitor GenTree** m_use; GenTree* m_node; GenTree* m_parentNode; - GenTreeCall* m_callAncestor; unsigned m_lclNum; unsigned m_useCount; GenTreeFlags m_useFlags; From fdf15200b084a6e8872fd942d8eb2eec70034503 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 21 Jun 2022 16:47:16 +0200 Subject: [PATCH 04/13] Couple minor fixes --- src/coreclr/jit/gentree.h | 12 ++++++------ src/coreclr/jit/morph.cpp | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index b4b829110ebe79..5d7ec569194444 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -130,7 +130,7 @@ enum gtCallTypes : BYTE CT_COUNT // fake entry (must be last) }; -enum class ExceptionSetFlags +enum class ExceptionSetFlags : uint32_t { None = 0x0, OverflowException = 0x1, @@ -143,27 +143,27 @@ enum class ExceptionSetFlags inline constexpr ExceptionSetFlags operator~(ExceptionSetFlags a) { - return (ExceptionSetFlags)(~(unsigned short)a); + return (ExceptionSetFlags)(~(uint32_t)a); } inline constexpr ExceptionSetFlags operator|(ExceptionSetFlags a, ExceptionSetFlags b) { - return (ExceptionSetFlags)((unsigned short)a | (unsigned short)b); + return (ExceptionSetFlags)((uint32_t)a | (uint32_t)b); } inline constexpr ExceptionSetFlags operator&(ExceptionSetFlags a, ExceptionSetFlags b) { - return (ExceptionSetFlags)((unsigned short)a & (unsigned short)b); + return (ExceptionSetFlags)((uint32_t)a & (uint32_t)b); } inline ExceptionSetFlags& operator|=(ExceptionSetFlags& a, ExceptionSetFlags b) { - return a = (ExceptionSetFlags)((unsigned short)a | (unsigned short)b); + return a = (ExceptionSetFlags)((uint32_t)a | (uint32_t)b); } inline ExceptionSetFlags& operator&=(ExceptionSetFlags& a, ExceptionSetFlags b) { - return a = (ExceptionSetFlags)((unsigned short)a & (unsigned short)b); + return a = (ExceptionSetFlags)((uint32_t)a & (uint32_t)b); } #ifdef DEBUG diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index ff8e0ed0b89253..3448fa365c0ebb 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -911,7 +911,7 @@ void CallArgs::ArgsComplete(Compiler* comp, GenTreeCall* call) { // If a previous arg may throw a different exception than this arg, // then we evaluate all previous arguments with GTF_EXCEPT to temps - // to avoid reordering them in our sort late. The only case we can + // to avoid reordering them in our sort later. The only case we can // avoid this is if all previous args throw the same single // exception as this arg. unsigned numExceptions = genCountBits(static_cast(preciseExceptions)); From bf10e8db00c3d9a3252a67c1c215662d5939ba08 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 21 Jun 2022 18:07:27 +0200 Subject: [PATCH 05/13] Revert "Allow more permissive forward subbing into call arguments" This reverts commit 1536b67b3a5746e1174c0760b16db85f882c2085. Will do this separately in a follow-up change. --- src/coreclr/jit/forwardsub.cpp | 41 +++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/forwardsub.cpp b/src/coreclr/jit/forwardsub.cpp index a05ef79fd572fc..4c3fae5f1d9f9e 100644 --- a/src/coreclr/jit/forwardsub.cpp +++ b/src/coreclr/jit/forwardsub.cpp @@ -187,6 +187,7 @@ class ForwardSubVisitor final : public GenTreeVisitor public: enum { + ComputeStack = true, DoPostOrder = true, UseExecutionOrder = true }; @@ -196,6 +197,7 @@ class ForwardSubVisitor final : public GenTreeVisitor , m_use(nullptr) , m_node(nullptr) , m_parentNode(nullptr) + , m_callAncestor(nullptr) , m_lclNum(lclNum) , m_useCount(0) , m_useFlags(GTF_EMPTY) @@ -219,7 +221,7 @@ class ForwardSubVisitor final : public GenTreeVisitor // Screen out contextual "uses" // - GenTree* const parent = user; + GenTree* const parent = m_ancestors.Top(1); bool const isDef = parent->OperIs(GT_ASG) && (parent->gtGetOp1() == node); bool const isAddr = parent->OperIs(GT_ADDR); @@ -241,6 +243,19 @@ class ForwardSubVisitor final : public GenTreeVisitor m_use = use; m_useFlags = m_accumulatedFlags; m_parentNode = parent; + + // If this use contributes to a call arg we need to + // remember the call and handle it specially when we + // see it later in the postorder walk. + // + for (int i = 1; i < m_ancestors.Height(); i++) + { + if (m_ancestors.Top(i)->IsCall()) + { + m_callAncestor = m_ancestors.Top(i)->AsCall(); + break; + } + } } } } @@ -259,6 +274,29 @@ class ForwardSubVisitor final : public GenTreeVisitor } } + // Is this the use's call ancestor? + // + if ((m_callAncestor != nullptr) && (node == m_callAncestor)) + { + // To be conservative and avoid issues with morph + // reordering call args, we merge in effects of all args + // to this call. + // + // Remove this if/when morph's arg sorting is fixed. + // + GenTreeFlags oldUseFlags = m_useFlags; + + for (CallArg& arg : m_callAncestor->gtArgs.Args()) + { + m_useFlags |= (arg.GetNode()->gtFlags & GTF_GLOB_EFFECT); + } + + if (oldUseFlags != m_useFlags) + { + JITDUMP(" [added other call arg use flags: 0x%x]", m_useFlags & ~oldUseFlags); + } + } + m_accumulatedFlags |= (node->gtFlags & GTF_GLOB_EFFECT); return fgWalkResult::WALK_CONTINUE; @@ -303,6 +341,7 @@ class ForwardSubVisitor final : public GenTreeVisitor GenTree** m_use; GenTree* m_node; GenTree* m_parentNode; + GenTreeCall* m_callAncestor; unsigned m_lclNum; unsigned m_useCount; GenTreeFlags m_useFlags; From 55411b2d2beaa059ee8ed5dffc26a9a45ecad77b Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 21 Jun 2022 18:40:29 +0200 Subject: [PATCH 06/13] Some renames --- src/coreclr/jit/compiler.h | 2 +- src/coreclr/jit/gentree.cpp | 39 +++++++++++++++++++++---------------- src/coreclr/jit/gentree.h | 5 ++++- src/coreclr/jit/morph.cpp | 7 +------ 4 files changed, 28 insertions(+), 25 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 96bbb455fe1dc7..d90a3d85ed1ea3 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -5873,7 +5873,7 @@ class Compiler bool gtIsTypeHandleToRuntimeTypeHandleHelper(GenTreeCall* call, CorInfoHelpFunc* pHelper = nullptr); bool gtIsActiveCSE_Candidate(GenTree* tree); - ExceptionSetFlags gtCollectPreciseExceptions(GenTree* tree); + ExceptionSetFlags gtCollectExceptions(GenTree* tree); bool fgIsBigOffset(size_t offset); diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index f9ebb8cf03f12b..51b3fe48d0514e 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -6351,7 +6351,7 @@ bool GenTree::OperIsImplicitIndir() const } //------------------------------------------------------------------------------ -// OperPreciseExceptions : Get exception set this tree may throw. +// OperExceptions : Get exception set this tree may throw. // // // Arguments: @@ -6364,7 +6364,7 @@ bool GenTree::OperIsImplicitIndir() const // Should not be used on calls given that we can say nothing precise about // those. // -ExceptionSetFlags GenTree::OperPreciseExceptions(Compiler* comp) +ExceptionSetFlags GenTree::OperExceptions(Compiler* comp) { GenTree* op; @@ -6412,9 +6412,9 @@ ExceptionSetFlags GenTree::OperPreciseExceptions(Compiler* comp) return ExceptionSetFlags::None; case GT_CALL: - assert(!"Unexpected GT_CALL in OperPreciseExceptions"); + assert(!"Unexpected GT_CALL in OperExceptions"); - return static_cast(0xffffffff); + return ExceptionSetFlags::All; case GT_IND: case GT_BLK: @@ -6519,7 +6519,7 @@ bool GenTree::OperMayThrow(Compiler* comp) return ((helper == CORINFO_HELP_UNDEF) || !comp->s_helperCallProperties.NoThrow(helper)); } - return OperPreciseExceptions(comp) != ExceptionSetFlags::None; + return OperExceptions(comp) != ExceptionSetFlags::None; } //----------------------------------------------------------------------------------- @@ -16168,7 +16168,7 @@ bool Compiler::gtIsTypeHandleToRuntimeTypeHelper(GenTreeCall* call) // // Return Value: // True if so - +// bool Compiler::gtIsTypeHandleToRuntimeTypeHandleHelper(GenTreeCall* call, CorInfoHelpFunc* pHelper) { CorInfoHelpFunc helper = CORINFO_HELP_UNDEF; @@ -16195,14 +16195,24 @@ bool Compiler::gtIsActiveCSE_Candidate(GenTree* tree) return (optValnumCSE_phase && IS_CSE_INDEX(tree->gtCSEnum)); } -ExceptionSetFlags Compiler::gtCollectPreciseExceptions(GenTree* tree) +//------------------------------------------------------------------------ +// gtCollectExceptions: walk a tree collecting a bit set of exceptions the tree +// may throw. +// +// Arguments: +// tree - tree to examine +// +// Return Value: +// Bit set of exceptions the tree may throw. +// +ExceptionSetFlags Compiler::gtCollectExceptions(GenTree* tree) { - class PreciseExceptionsWalker final : public GenTreeVisitor + class ExceptionsWalker final : public GenTreeVisitor { ExceptionSetFlags m_preciseExceptions = ExceptionSetFlags::None; public: - PreciseExceptionsWalker(Compiler* comp) : GenTreeVisitor(comp) + ExceptionsWalker(Compiler* comp) : GenTreeVisitor(comp) { } @@ -16224,7 +16234,7 @@ ExceptionSetFlags Compiler::gtCollectPreciseExceptions(GenTree* tree) return WALK_SKIP_SUBTREES; } - m_preciseExceptions |= tree->OperPreciseExceptions(m_compiler); + m_preciseExceptions |= tree->OperExceptions(m_compiler); return WALK_CONTINUE; } }; @@ -16234,14 +16244,9 @@ ExceptionSetFlags Compiler::gtCollectPreciseExceptions(GenTree* tree) // a call it can always throw arbitrary exceptions. assert((tree->gtFlags & GTF_CALL) == 0); - if ((tree->gtFlags & GTF_EXCEPT) == 0) - { - return ExceptionSetFlags::None; - } - - PreciseExceptionsWalker walker(this); + ExceptionsWalker walker(this); walker.WalkTree(&tree, nullptr); - assert(walker.GetFlags() != ExceptionSetFlags::None); + assert(((tree->gtFlags & GTF_EXCEPT) == 0) || (walker.GetFlags() != ExceptionSetFlags::None)); return walker.GetFlags(); } diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 5d7ec569194444..e7c6e202f9a038 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -139,6 +139,9 @@ enum class ExceptionSetFlags : uint32_t NullReferenceException = 0x8, IndexOutOfRangeException = 0x10, StackOverflowException = 0x20, + + All = OverflowException | DivideByZeroException | ArithmeticException | NullReferenceException | + IndexOutOfRangeException | StackOverflowException, }; inline constexpr ExceptionSetFlags operator~(ExceptionSetFlags a) @@ -1846,7 +1849,7 @@ struct GenTree bool OperRequiresCallFlag(Compiler* comp); bool OperMayThrow(Compiler* comp); - ExceptionSetFlags OperPreciseExceptions(Compiler* comp); + ExceptionSetFlags OperExceptions(Compiler* comp); unsigned GetScaleIndexMul(); unsigned GetScaleIndexShf(); diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 3448fa365c0ebb..25ec652e159b4d 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -1155,12 +1155,7 @@ void CallArgs::SortArgs(Compiler* comp, GenTreeCall* call, CallArg** sortedArgs) { assert(m_argsComplete); -#ifdef DEBUG - if (comp->verbose) - { - printf("\nSorting the arguments:\n"); - } -#endif + JITDUMP("\nSorting the arguments:\n"); // Shuffle the arguments around before we build the late args list. The // idea is to move all "simple" arguments like constants and local vars to From 21b3ae3c78e88be1010c56df2f9786050d064086 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 21 Jun 2022 18:40:38 +0200 Subject: [PATCH 07/13] Implement alternative more lazy method --- src/coreclr/jit/morph.cpp | 76 ++++++++++++++++++++++++--------------- 1 file changed, 48 insertions(+), 28 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 25ec652e159b4d..f603bcc32c3148 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -735,9 +735,11 @@ void CallArgs::ArgsComplete(Compiler* comp, GenTreeCall* call) unsigned argCount = CountArgs(); - // Overapproximation of exception flags for previous args that may not - // already be evaluated into temps. - ExceptionSetFlags nonTempExFlags = ExceptionSetFlags::None; + // Previous argument with GTF_EXCEPT + GenTree* prevExceptionTree = nullptr; + // Exceptions previous tree with GTF_EXCEPT may throw (computed lazily, may + // be empty) + ExceptionSetFlags prevExceptionFlags = ExceptionSetFlags::None; CallArg* prevNonTempExceptionThrowingArg = nullptr; @@ -823,10 +825,7 @@ void CallArgs::ArgsComplete(Compiler* comp, GenTreeCall* call) bool treatLikeCall = ((argx->gtFlags & GTF_CALL) != 0); - // TODO: Keep previous GTF_EXCEPT arg and lazily collect this set instead. - ExceptionSetFlags preciseExceptions = - treatLikeCall ? ExceptionSetFlags::None : comp->gtCollectPreciseExceptions(argx); - + ExceptionSetFlags exceptionFlags = ExceptionSetFlags::None; #if FEATURE_FIXED_OUT_ARGS // Like calls, if this argument has a tree that will do an inline throw, // a call to a jit helper, then we need to treat it like a call (but only @@ -837,8 +836,9 @@ void CallArgs::ArgsComplete(Compiler* comp, GenTreeCall* call) // if (!treatLikeCall && (argCount > 1) && comp->opts.compDbgCode) { - if ((preciseExceptions & (ExceptionSetFlags::IndexOutOfRangeException | - ExceptionSetFlags::OverflowException)) != ExceptionSetFlags::None) + exceptionFlags = comp->gtCollectExceptions(argx); + if ((exceptionFlags & (ExceptionSetFlags::IndexOutOfRangeException | + ExceptionSetFlags::OverflowException)) != ExceptionSetFlags::None) { for (CallArg& otherArg : Args()) { @@ -907,34 +907,54 @@ void CallArgs::ArgsComplete(Compiler* comp, GenTreeCall* call) #endif } } - else + else if ((argx->gtFlags & GTF_EXCEPT) != 0) { - // If a previous arg may throw a different exception than this arg, + // If a previous arg may throw a different exception than this arg // then we evaluate all previous arguments with GTF_EXCEPT to temps - // to avoid reordering them in our sort later. The only case we can - // avoid this is if all previous args throw the same single - // exception as this arg. - unsigned numExceptions = genCountBits(static_cast(preciseExceptions)); - bool distinctExceptions = - (nonTempExFlags != ExceptionSetFlags::None) && - ((numExceptions > 1) || ((numExceptions == 1) && (preciseExceptions != nonTempExFlags))); - if (distinctExceptions) - { - for (CallArg& prevArg : Args()) + // to avoid reordering them in our sort later. + if (prevExceptionTree != nullptr) + { + if (prevExceptionFlags == ExceptionSetFlags::None) { - if (&prevArg == &arg) - { - break; - } + prevExceptionFlags = comp->gtCollectExceptions(prevExceptionTree); + } + + if (exceptionFlags == ExceptionSetFlags::None) + { + exceptionFlags = comp->gtCollectExceptions(argx); + } + + if ((genCountBits(static_cast(exceptionFlags)) >= 2) || + (exceptionFlags != prevExceptionFlags)) + { + JITDUMP("Exception set for arg [%06u] interferes with previous tree [%06u]; must evaluate previous " + "trees with exceptions to temps", + Compiler::dspTreeID(argx), Compiler::dspTreeID(prevExceptionTree)); - if ((prevArg.GetEarlyNode() != nullptr) && ((prevArg.GetEarlyNode()->gtFlags & GTF_EXCEPT) != 0)) + for (CallArg& prevArg : Args()) { - SetNeedsTemp(&prevArg); + if (&prevArg == &arg) + { + break; + } + + // Invariant here is that all nodes that were not + // already evaluated into temps and that throw + // GTF_EXCEPT can only be throwing the same single + // exception as the previous tree, so all of them + // interfere in the same way with the current arg and + // must be evaluated early. + if ((prevArg.GetEarlyNode() != nullptr) && + ((prevArg.GetEarlyNode()->gtFlags & GTF_EXCEPT) != 0)) + { + SetNeedsTemp(&prevArg); + } } } } - nonTempExFlags |= preciseExceptions; + prevExceptionTree = argx; + prevExceptionFlags = exceptionFlags; } #if FEATURE_MULTIREG_ARGS From 5a247152fc32a0669263422e08356aca20b3f118 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 21 Jun 2022 18:42:23 +0200 Subject: [PATCH 08/13] Remove unused local --- src/coreclr/jit/morph.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index f603bcc32c3148..3ff1324272e3ae 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -741,8 +741,6 @@ void CallArgs::ArgsComplete(Compiler* comp, GenTreeCall* call) // be empty) ExceptionSetFlags prevExceptionFlags = ExceptionSetFlags::None; - CallArg* prevNonTempExceptionThrowingArg = nullptr; - for (CallArg& arg : Args()) { GenTree* argx = arg.GetEarlyNode(); From 97392cda0bdfce8c22d55dc85fe7e86657d4d082 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 21 Jun 2022 18:43:18 +0200 Subject: [PATCH 09/13] Readd --- src/coreclr/jit/morph.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 3ff1324272e3ae..4867008b1c2b5f 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -832,7 +832,7 @@ void CallArgs::ArgsComplete(Compiler* comp, GenTreeCall* call) // conservative, but I want to avoid as much special-case debug-only code // as possible, so leveraging the GTF_CALL flag is the easiest. // - if (!treatLikeCall && (argCount > 1) && comp->opts.compDbgCode) + if (!treatLikeCall && (argx->gtFlags & GTF_EXCEPT) && (argCount > 1) && comp->opts.compDbgCode) { exceptionFlags = comp->gtCollectExceptions(argx); if ((exceptionFlags & (ExceptionSetFlags::IndexOutOfRangeException | From 2ff27a0bc37cc9379f145ae93bff80d78bbc7fef Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 21 Jun 2022 18:45:31 +0200 Subject: [PATCH 10/13] Add newline --- src/coreclr/jit/morph.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 4867008b1c2b5f..f6f3603fe75916 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -926,7 +926,7 @@ void CallArgs::ArgsComplete(Compiler* comp, GenTreeCall* call) (exceptionFlags != prevExceptionFlags)) { JITDUMP("Exception set for arg [%06u] interferes with previous tree [%06u]; must evaluate previous " - "trees with exceptions to temps", + "trees with exceptions to temps\n", Compiler::dspTreeID(argx), Compiler::dspTreeID(prevExceptionTree)); for (CallArg& prevArg : Args()) From 88d64dcc388f192f03e1911f9a26554279cde130 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 21 Jun 2022 18:46:16 +0200 Subject: [PATCH 11/13] Rephrase --- src/coreclr/jit/morph.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index f6f3603fe75916..4b480595af6efa 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -937,11 +937,11 @@ void CallArgs::ArgsComplete(Compiler* comp, GenTreeCall* call) } // Invariant here is that all nodes that were not - // already evaluated into temps and that throw - // GTF_EXCEPT can only be throwing the same single - // exception as the previous tree, so all of them - // interfere in the same way with the current arg and - // must be evaluated early. + // already evaluated into temps and that throw can only + // be throwing the same single exception as the + // previous tree, so all of them interfere in the same + // way with the current arg and must be evaluated + // early. if ((prevArg.GetEarlyNode() != nullptr) && ((prevArg.GetEarlyNode()->gtFlags & GTF_EXCEPT) != 0)) { From 3d4e4974a47e6fadee0123cbaad21ee096dbea71 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 21 Jun 2022 23:04:21 +0200 Subject: [PATCH 12/13] Add early break in new loop Ensure we do not set stack args as needing a temp. --- src/coreclr/jit/morph.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 02e7070e65605a..0a4c772aaade40 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -959,6 +959,14 @@ void CallArgs::ArgsComplete(Compiler* comp, GenTreeCall* call) break; } +#if !FEATURE_FIXED_OUT_ARGS + if (prevArg.AbiInfo.GetRegNum() == REG_STK) + { + // All stack args are already evaluated and placed in order + // in this case. + break; + } +#endif // Invariant here is that all nodes that were not // already evaluated into temps and that throw can only // be throwing the same single exception as the From ebdfa775e3025cf064379c96203021418c36843d Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 21 Jun 2022 23:07:47 +0200 Subject: [PATCH 13/13] Flip a condition; use isPow2 --- src/coreclr/jit/morph.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 0a4c772aaade40..276d0063505881 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -945,8 +945,9 @@ void CallArgs::ArgsComplete(Compiler* comp, GenTreeCall* call) exceptionFlags = comp->gtCollectExceptions(argx); } - if ((genCountBits(static_cast(exceptionFlags)) >= 2) || - (exceptionFlags != prevExceptionFlags)) + bool exactlyOne = isPow2(static_cast(exceptionFlags)); + bool throwsSameAsPrev = exactlyOne && (exceptionFlags == prevExceptionFlags); + if (!throwsSameAsPrev) { JITDUMP("Exception set for arg [%06u] interferes with previous tree [%06u]; must evaluate previous " "trees with exceptions to temps\n",