From 96c46a871a812354f768a56d11cba9632dde0b1d Mon Sep 17 00:00:00 2001 From: Will Smith Date: Wed, 22 Jun 2022 16:33:43 -0700 Subject: [PATCH 1/9] Do not CSE constant operands for GT_SIMD --- src/coreclr/jit/morph.cpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 1758051daebfa..bb8f9b5eb1e1d 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -13807,6 +13807,21 @@ GenTree* Compiler::fgMorphMultiOp(GenTreeMultiOp* multiOp) } #endif +#if defined(FEATURE_SIMD) + if (multiOp->OperIs(GT_SIMD)) + { + dontCseConstArguments = true; + for (GenTree* arg : multiOp->Operands()) + { + if (!arg->OperIsConst()) + { + dontCseConstArguments = false; + break; + } + } + } +#endif + for (GenTree** use : multiOp->UseEdges()) { *use = fgMorphTree(*use); From 959cce9fdcb20b76519c5d86b9df057942875312 Mon Sep 17 00:00:00 2001 From: Will Smith Date: Wed, 22 Jun 2022 16:45:13 -0700 Subject: [PATCH 2/9] Re-arrange logic --- src/coreclr/jit/morph.cpp | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index bb8f9b5eb1e1d..f3017e35ffa6e 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -13807,21 +13807,6 @@ GenTree* Compiler::fgMorphMultiOp(GenTreeMultiOp* multiOp) } #endif -#if defined(FEATURE_SIMD) - if (multiOp->OperIs(GT_SIMD)) - { - dontCseConstArguments = true; - for (GenTree* arg : multiOp->Operands()) - { - if (!arg->OperIsConst()) - { - dontCseConstArguments = false; - break; - } - } - } -#endif - for (GenTree** use : multiOp->UseEdges()) { *use = fgMorphTree(*use); @@ -13927,6 +13912,29 @@ GenTree* Compiler::fgMorphMultiOp(GenTreeMultiOp* multiOp) } #endif +#if defined(FEATURE_SIMD) + if (opts.OptimizationEnabled() && multiOp->OperIs(GT_SIMD)) + { + bool allArgsAreConst = true; + for (GenTree* arg : multiOp->Operands()) + { + if (!arg->OperIsConst()) + { + allArgsAreConst = false; + break; + } + } + + if (allArgsAreConst) + { + for (GenTree* arg : multiOp->Operands()) + { + arg->SetDoNotCSE(); + } + } + } +#endif + return multiOp; } #endif // defined(FEATURE_SIMD) || defined(FEATURE_HW_INTRINSICS) From 415e964efe21d71f3a6187f4597c444454cd5973 Mon Sep 17 00:00:00 2001 From: Will Smith Date: Wed, 22 Jun 2022 17:33:14 -0700 Subject: [PATCH 3/9] Feedback --- src/coreclr/jit/gentree.h | 35 +++++++++++++++++++++++++++++ src/coreclr/jit/morph.cpp | 47 ++++++++------------------------------- 2 files changed, 44 insertions(+), 38 deletions(-) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 5b88e3219f6ed..9414672e3a7ff 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -1733,6 +1733,7 @@ struct GenTree inline bool IsIntegralConst(ssize_t constVal) const; inline bool IsFloatPositiveZero() const; inline bool IsVectorZero() const; + inline bool IsVectorCreate() const; inline bool IsVectorAllBitsSet() const; inline bool IsVectorConst(); @@ -8313,6 +8314,40 @@ inline bool GenTree::IsVectorZero() const return IsCnsVec() && AsVecCon()->IsZero(); } +//------------------------------------------------------------------- +// IsVectorCreate: returns true if this node is the creation of a vector. +// Does not include "Unsafe" method calls. +// +// Returns: +// True if this node is the creation of a vector +// +inline bool GenTree::IsVectorCreate() const +{ +#ifdef FEATURE_HW_INTRINSICS + if (OperIs(GT_HWINTRINSIC)) + { + switch (AsHWIntrinsic()->GetHWIntrinsicId()) + { + case NI_Vector128_Create: +#if defined(TARGET_XARCH) + case NI_Vector256_Create: +#elif defined(TARGET_ARMARCH) + case NI_Vector64_Create: + return true; + + default: + return false; + } + } +#endif // FEATURE_HW_INTRINSICS + +#ifdef FEATURE_SIMD + return OperIs(GT_SIMD); +#else + return false; +#endif // FEATURE_SIMD +} + //------------------------------------------------------------------- // IsVectorAllBitsSet: returns true if this node is a vector constant with all bits set. // diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index f3017e35ffa6e..27e1291e4b0ed 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -13870,50 +13870,13 @@ GenTree* Compiler::fgMorphMultiOp(GenTreeMultiOp* multiOp) } #endif - case NI_Vector128_Create: -#if defined(TARGET_XARCH) - case NI_Vector256_Create: -#elif defined(TARGET_ARMARCH) - case NI_Vector64_Create: -#endif - { - bool hwAllArgsAreConst = true; - for (GenTree** use : multiOp->UseEdges()) - { - if (!(*use)->OperIsConst()) - { - hwAllArgsAreConst = false; - break; - } - } - - // Avoid unexpected CSE for constant arguments for Vector_.Create - // but only if all arguments are constants. - if (hwAllArgsAreConst) - { - for (GenTree** use : multiOp->UseEdges()) - { - (*use)->SetDoNotCSE(); - } - } - } - break; - default: break; } } #endif // defined(FEATURE_HW_INTRINSICS) && defined(TARGET_XARCH) -#ifdef FEATURE_HW_INTRINSICS - if (multiOp->OperIsHWIntrinsic() && !optValnumCSE_phase) - { - return fgOptimizeHWIntrinsic(multiOp->AsHWIntrinsic()); - } -#endif - -#if defined(FEATURE_SIMD) - if (opts.OptimizationEnabled() && multiOp->OperIs(GT_SIMD)) + if (opts.OptimizationEnabled() && multiOp->IsVectorCreate()) { bool allArgsAreConst = true; for (GenTree* arg : multiOp->Operands()) @@ -13925,6 +13888,8 @@ GenTree* Compiler::fgMorphMultiOp(GenTreeMultiOp* multiOp) } } + // Avoid unexpected CSE for constant arguments for Vector_.Create + // but only if all arguments are constants. if (allArgsAreConst) { for (GenTree* arg : multiOp->Operands()) @@ -13933,6 +13898,12 @@ GenTree* Compiler::fgMorphMultiOp(GenTreeMultiOp* multiOp) } } } + +#ifdef FEATURE_HW_INTRINSICS + if (multiOp->OperIsHWIntrinsic() && !optValnumCSE_phase) + { + return fgOptimizeHWIntrinsic(multiOp->AsHWIntrinsic()); + } #endif return multiOp; From 12653fd802067d05efd96e229f0fd2c408c4787d Mon Sep 17 00:00:00 2001 From: Will Smith Date: Wed, 22 Jun 2022 17:39:43 -0700 Subject: [PATCH 4/9] Fixing build --- src/coreclr/jit/gentree.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 9414672e3a7ff..0368083d42fff 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -8333,6 +8333,7 @@ inline bool GenTree::IsVectorCreate() const case NI_Vector256_Create: #elif defined(TARGET_ARMARCH) case NI_Vector64_Create: +#endif return true; default: From 5279dd9c296356d3d10df8eedb8761640aeecc54 Mon Sep 17 00:00:00 2001 From: Will Smith Date: Wed, 22 Jun 2022 18:05:19 -0700 Subject: [PATCH 5/9] Feedback --- src/coreclr/jit/gentree.h | 15 +++++++++++++++ src/coreclr/jit/morph.cpp | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 0368083d42fff..fa74917f8688a 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -8343,6 +8343,21 @@ inline bool GenTree::IsVectorCreate() const #endif // FEATURE_HW_INTRINSICS #ifdef FEATURE_SIMD + if (OperIs(GT_SIMD)) + { + switch (AsSIMD()->GetSIMDIntrinsicId()) + { + case SIMDIntrinsicInit: + case SIMDIntrinsicInitFixed: + case SIMDIntrinsicInitArray: + case SIMDIntrinsicInitArrayX: + case SIMDIntrinsicInitN: + return true; + + default: + return false; + } + } return OperIs(GT_SIMD); #else return false; diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 27e1291e4b0ed..b01afdb85fe2a 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -13881,7 +13881,7 @@ GenTree* Compiler::fgMorphMultiOp(GenTreeMultiOp* multiOp) bool allArgsAreConst = true; for (GenTree* arg : multiOp->Operands()) { - if (!arg->OperIsConst()) + if (!arg->OperIsConst() || arg->OperIs(GT_CNS_VEC)) { allArgsAreConst = false; break; From 1c04881dc28418feff82d997a26b0d677d8d702f Mon Sep 17 00:00:00 2001 From: Will Smith Date: Wed, 22 Jun 2022 18:06:45 -0700 Subject: [PATCH 6/9] Feedback --- src/coreclr/jit/gentree.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index fa74917f8688a..4b4a04fafcb95 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -8348,9 +8348,6 @@ inline bool GenTree::IsVectorCreate() const switch (AsSIMD()->GetSIMDIntrinsicId()) { case SIMDIntrinsicInit: - case SIMDIntrinsicInitFixed: - case SIMDIntrinsicInitArray: - case SIMDIntrinsicInitArrayX: case SIMDIntrinsicInitN: return true; From 9d1857aba5f12a345516a8d9b174c2f475f74db4 Mon Sep 17 00:00:00 2001 From: Will Smith Date: Wed, 22 Jun 2022 18:07:04 -0700 Subject: [PATCH 7/9] Feedback --- 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 b01afdb85fe2a..27e1291e4b0ed 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -13881,7 +13881,7 @@ GenTree* Compiler::fgMorphMultiOp(GenTreeMultiOp* multiOp) bool allArgsAreConst = true; for (GenTree* arg : multiOp->Operands()) { - if (!arg->OperIsConst() || arg->OperIs(GT_CNS_VEC)) + if (!arg->OperIsConst()) { allArgsAreConst = false; break; From 0539c269797f373df0b319b1ebf8bce3cc76885b Mon Sep 17 00:00:00 2001 From: Will Smith Date: Wed, 22 Jun 2022 18:09:59 -0700 Subject: [PATCH 8/9] Feedback --- src/coreclr/jit/gentree.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 4b4a04fafcb95..780c8ecf5aab3 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -8355,7 +8355,6 @@ inline bool GenTree::IsVectorCreate() const return false; } } - return OperIs(GT_SIMD); #else return false; #endif // FEATURE_SIMD From 780ef329a7686fb960cc1d1fb24fbea566a7f20d Mon Sep 17 00:00:00 2001 From: Will Smith Date: Wed, 22 Jun 2022 18:11:18 -0700 Subject: [PATCH 9/9] Feedback --- src/coreclr/jit/gentree.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 780c8ecf5aab3..cc98b770d2b22 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -8355,9 +8355,9 @@ inline bool GenTree::IsVectorCreate() const return false; } } -#else - return false; #endif // FEATURE_SIMD + + return false; } //-------------------------------------------------------------------