From b2d407f30af0cc56c64e04204b472504309b6c2c Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 28 Mar 2023 02:37:14 +0200 Subject: [PATCH 1/4] Improve IsKnownConstant --- src/coreclr/jit/gentree.cpp | 5 +++++ src/coreclr/jit/lower.cpp | 29 ++++++++++++++++++++++++++--- src/coreclr/jit/lower.h | 1 + src/coreclr/jit/morph.cpp | 22 ++++------------------ src/coreclr/jit/valuenum.cpp | 6 ++++++ 5 files changed, 42 insertions(+), 21 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 027bc375cf076c..ac418fac0b11fa 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -5145,6 +5145,11 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree) costSz = 12; break; + case NI_System_Runtime_CompilerServices_RuntimeHelpers_IsKnownConstant: + costEx = 1; + costSz = 1; + break; + case NI_System_Math_Abs: costEx = 5; costSz = 15; diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index decaed31eeb836..2a86e313f65ea1 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -583,11 +583,9 @@ GenTree* Lowering::LowerNode(GenTree* node) ContainCheckLclHeap(node->AsOp()); break; -#ifdef TARGET_XARCH case GT_INTRINSIC: - ContainCheckIntrinsic(node->AsOp()); + LowerIntrinsic(node->AsIntrinsic()); break; -#endif // TARGET_XARCH #ifdef FEATURE_HW_INTRINSICS case GT_HWINTRINSIC: @@ -7786,6 +7784,31 @@ void Lowering::TransformUnusedIndirection(GenTreeIndir* ind, Compiler* comp, Bas } } +//------------------------------------------------------------------------ +// LowerIntrinsic: a common logic to lower INTRINSIC. +// +// Arguments: +// node - the INTRINSIC node we are lowering. +// +void Lowering::LowerIntrinsic(GenTreeIntrinsic* node) +{ + if (node->AsIntrinsic()->gtIntrinsicName == NI_System_Runtime_CompilerServices_RuntimeHelpers_IsKnownConstant) + { + // IsKnownConstant is expected to be folded in Importer/Morph/VN+ConstantProp + // This path is just in case if VN+ConstantProp didn't fold them for some reason (or e.g. they were disabled) + node->gtGetOp1()->SetUnusedValue(); + LIR::Use use; + if (BlockRange().TryGetUse(node, &use)) + { + use.ReplaceWith(comp->gtNewFalse()); + } + BlockRange().Remove(node); + } +#ifdef TARGET_XARCH + ContainCheckIntrinsic(node->AsOp()); +#endif +} + //------------------------------------------------------------------------ // LowerBlockStoreCommon: a common logic to lower STORE_OBJ/BLK/DYN_BLK. // diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h index 6937d2e3c0430f..cd7efe3319d3bf 100644 --- a/src/coreclr/jit/lower.h +++ b/src/coreclr/jit/lower.h @@ -312,6 +312,7 @@ class Lowering final : public Phase bool LowerUnsignedDivOrMod(GenTreeOp* divMod); GenTree* LowerConstIntDivOrMod(GenTree* node); GenTree* LowerSignedDivOrMod(GenTree* node); + void LowerIntrinsic(GenTreeIntrinsic* node); void LowerBlockStore(GenTreeBlk* blkNode); void LowerBlockStoreCommon(GenTreeBlk* blkNode); void ContainBlockStoreAddress(GenTreeBlk* blkNode, unsigned size, GenTree* addr, GenTree* addrParent); diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 64006e890d8595..a585338d8bc392 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -10010,8 +10010,8 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA if (tree->AsIntrinsic()->gtIntrinsicName == NI_System_Runtime_CompilerServices_RuntimeHelpers_IsKnownConstant) { - // Should be expanded by the time it reaches CSE phase - assert(!optValnumCSE_phase); + // Should be expanded in importer for MinOpts + assert(opts.OptimizationEnabled()); JITDUMP("\nExpanding RuntimeHelpers.IsKnownConstant to "); if (op1->OperIsConst() || gtIsTypeof(op1)) @@ -10023,22 +10023,8 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA } else { - GenTree* op1SideEffects = nullptr; - gtExtractSideEffList(op1, &op1SideEffects, GTF_ALL_EFFECT); - if (op1SideEffects != nullptr) - { - DEBUG_DESTROY_NODE(tree); - // Keep side-effects of op1 - tree = gtNewOperNode(GT_COMMA, TYP_INT, op1SideEffects, gtNewIconNode(0)); - JITDUMP("false with side effects:\n") - DISPTREE(tree); - } - else - { - JITDUMP("false\n"); - DEBUG_DESTROY_NODE(tree, op1); - tree = gtNewIconNode(0); - } + // Leave it for VN/ConstantProp (or Lowering) + return tree; } INDEBUG(tree->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); return tree; diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 1c9e7460285ed8..63fb079a4cf23e 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -11047,6 +11047,12 @@ void Compiler::fgValueNumberIntrinsic(GenTree* tree) intrinsic->gtVNPair = vnStore->VNPWithExc(newVNP, excSet); } } + else if (intrinsic->gtIntrinsicName == NI_System_Runtime_CompilerServices_RuntimeHelpers_IsKnownConstant) + { + bool isKnownConstant = arg0VNP.BothEqual() && vnStore->IsVNConstant(arg0VNP.GetLiberal()); + ValueNum isCnsVN = vnStore->VNForIntCon(isKnownConstant ? 1 : 0); + intrinsic->gtVNPair = vnStore->VNPWithExc(ValueNumPair(isCnsVN, isCnsVN), arg0VNPx); + } else { assert(intrinsic->gtIntrinsicName == NI_System_Object_GetType); From f4708cd2424bdf561d454e6cfce4870fa79f9e62 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Tue, 28 Mar 2023 19:01:46 +0200 Subject: [PATCH 2/4] Update lower.cpp --- src/coreclr/jit/lower.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 2a86e313f65ea1..a710a136cd99de 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -7792,7 +7792,7 @@ void Lowering::TransformUnusedIndirection(GenTreeIndir* ind, Compiler* comp, Bas // void Lowering::LowerIntrinsic(GenTreeIntrinsic* node) { - if (node->AsIntrinsic()->gtIntrinsicName == NI_System_Runtime_CompilerServices_RuntimeHelpers_IsKnownConstant) + if (node->gtIntrinsicName == NI_System_Runtime_CompilerServices_RuntimeHelpers_IsKnownConstant) { // IsKnownConstant is expected to be folded in Importer/Morph/VN+ConstantProp // This path is just in case if VN+ConstantProp didn't fold them for some reason (or e.g. they were disabled) From d4bb2b3e599d1ae159333557e83e3fa4d985e1f1 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Tue, 28 Mar 2023 20:00:52 +0200 Subject: [PATCH 3/4] Update src/coreclr/jit/lower.cpp Co-authored-by: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com> --- src/coreclr/jit/lower.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index a710a136cd99de..b67f7fc28bc109 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -7797,12 +7797,8 @@ void Lowering::LowerIntrinsic(GenTreeIntrinsic* node) // IsKnownConstant is expected to be folded in Importer/Morph/VN+ConstantProp // This path is just in case if VN+ConstantProp didn't fold them for some reason (or e.g. they were disabled) node->gtGetOp1()->SetUnusedValue(); - LIR::Use use; - if (BlockRange().TryGetUse(node, &use)) - { - use.ReplaceWith(comp->gtNewFalse()); - } - BlockRange().Remove(node); + node->BashToConst(0); + return; } #ifdef TARGET_XARCH ContainCheckIntrinsic(node->AsOp()); From e473e6d0c96ebd5e9ee91be33df7ecf49f9b8afb Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 28 Mar 2023 20:06:40 +0200 Subject: [PATCH 4/4] less conservative VN impl --- src/coreclr/jit/valuenum.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 63fb079a4cf23e..7c7be1e7c85162 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -11049,9 +11049,9 @@ void Compiler::fgValueNumberIntrinsic(GenTree* tree) } else if (intrinsic->gtIntrinsicName == NI_System_Runtime_CompilerServices_RuntimeHelpers_IsKnownConstant) { - bool isKnownConstant = arg0VNP.BothEqual() && vnStore->IsVNConstant(arg0VNP.GetLiberal()); - ValueNum isCnsVN = vnStore->VNForIntCon(isKnownConstant ? 1 : 0); - intrinsic->gtVNPair = vnStore->VNPWithExc(ValueNumPair(isCnsVN, isCnsVN), arg0VNPx); + ValueNum isCnsVNL = vnStore->VNForIntCon(vnStore->IsVNConstant(arg0VNP.GetLiberal()) ? 1 : 0); + ValueNum isCnsVNC = vnStore->VNForIntCon(vnStore->IsVNConstant(arg0VNP.GetConservative()) ? 1 : 0); + intrinsic->gtVNPair = vnStore->VNPWithExc(ValueNumPair(isCnsVNL, isCnsVNC), arg0VNPx); } else {