From fe3972265a0577d8cda5d1483b9dfb0bfeef0f40 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Sun, 12 May 2024 16:34:31 +0200 Subject: [PATCH 1/7] JIT: Reorder stores to make them amenable to stp optimization This generalizes the indir reordering optimization (that currently only triggers for loads) to kick in for GT_STOREIND nodes. The main complication with doing this is the fact that the data node of the second indirection needs its own reordering with the previous indirection. The existing logic works by reordering all nodes between the first and second indirection that are unrelated to the second indirection's computation to happen after it. Once that is done we know that there are no uses of the first indirection's result between it and the second indirection, so after doing the necessary interference checks we can safely move the previous indirection to happen after the data node of the second indirection. --- src/coreclr/jit/lower.cpp | 135 +++++++++++++++++++++------ src/coreclr/jit/lower.h | 6 +- src/coreclr/jit/lowerarmarch.cpp | 14 ++- src/coreclr/jit/lowerloongarch64.cpp | 5 +- src/coreclr/jit/lowerriscv64.cpp | 5 +- src/coreclr/jit/lowerxarch.cpp | 14 +-- 6 files changed, 138 insertions(+), 41 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 28538fc2b9053f..12861dbb8c186b 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -414,8 +414,7 @@ GenTree* Lowering::LowerNode(GenTree* node) } case GT_STOREIND: - LowerStoreIndirCommon(node->AsStoreInd()); - break; + return LowerStoreIndirCommon(node->AsStoreInd()); case GT_ADD: { @@ -8917,7 +8916,10 @@ void Lowering::LowerStoreIndirCoalescing(GenTreeIndir* ind) // Arguments: // ind - the store indirection node we are lowering. // -void Lowering::LowerStoreIndirCommon(GenTreeStoreInd* ind) +// Return Value: +// Next node to lower. +// +GenTree* Lowering::LowerStoreIndirCommon(GenTreeStoreInd* ind) { assert(ind->TypeGet() != TYP_STRUCT); @@ -8932,28 +8934,30 @@ void Lowering::LowerStoreIndirCommon(GenTreeStoreInd* ind) #endif TryCreateAddrMode(ind->Addr(), isContainable, ind); - if (!comp->codeGen->gcInfo.gcIsWriteBarrierStoreIndNode(ind)) + if (comp->codeGen->gcInfo.gcIsWriteBarrierStoreIndNode(ind)) { + return ind->gtNext; + } + #ifndef TARGET_XARCH - if (ind->Data()->IsIconHandle(GTF_ICON_OBJ_HDL)) + if (ind->Data()->IsIconHandle(GTF_ICON_OBJ_HDL)) + { + const ssize_t handle = ind->Data()->AsIntCon()->IconValue(); + if (!comp->info.compCompHnd->isObjectImmutable(reinterpret_cast(handle))) { - const ssize_t handle = ind->Data()->AsIntCon()->IconValue(); - if (!comp->info.compCompHnd->isObjectImmutable(reinterpret_cast(handle))) - { - // On platforms with weaker memory model we need to make sure we use a store with the release semantic - // when we publish a potentially mutable object - // See relevant discussions https://github.com/dotnet/runtime/pull/76135#issuecomment-1257258310 and - // https://github.com/dotnet/runtime/pull/76112#discussion_r980639782 + // On platforms with weaker memory model we need to make sure we use a store with the release semantic + // when we publish a potentially mutable object + // See relevant discussions https://github.com/dotnet/runtime/pull/76135#issuecomment-1257258310 and + // https://github.com/dotnet/runtime/pull/76112#discussion_r980639782 - // This can be relaxed to "just make sure to use stlr/memory barrier" if needed - ind->gtFlags |= GTF_IND_VOLATILE; - } + // This can be relaxed to "just make sure to use stlr/memory barrier" if needed + ind->gtFlags |= GTF_IND_VOLATILE; } + } #endif - LowerStoreIndirCoalescing(ind); - LowerStoreIndir(ind); - } + LowerStoreIndirCoalescing(ind); + return LowerStoreIndir(ind); } //------------------------------------------------------------------------ @@ -9036,7 +9040,7 @@ GenTree* Lowering::LowerIndir(GenTreeIndir* ind) #ifdef TARGET_ARM64 if (comp->opts.OptimizationEnabled() && ind->OperIs(GT_IND)) { - OptimizeForLdp(ind); + OptimizeForLdpStp(ind); } #endif @@ -9051,7 +9055,7 @@ GenTree* Lowering::LowerIndir(GenTreeIndir* ind) // cases passing the distance check, but 82 out of these 112 extra cases were // then rejected due to interference. So 16 seems like a good number to balance // the throughput costs. -const int LDP_REORDERING_MAX_DISTANCE = 16; +const int LDP_STP_REORDERING_MAX_DISTANCE = 16; //------------------------------------------------------------------------ // OptimizeForLdp: Record information about an indirection, and try to optimize @@ -9064,7 +9068,7 @@ const int LDP_REORDERING_MAX_DISTANCE = 16; // Returns: // True if the optimization was successful. // -bool Lowering::OptimizeForLdp(GenTreeIndir* ind) +bool Lowering::OptimizeForLdpStp(GenTreeIndir* ind) { if (!ind->TypeIs(TYP_INT, TYP_LONG, TYP_FLOAT, TYP_DOUBLE, TYP_SIMD8, TYP_SIMD16) || ind->IsVolatile()) { @@ -9082,7 +9086,7 @@ bool Lowering::OptimizeForLdp(GenTreeIndir* ind) // Every indirection takes an expected 2+ nodes, so we only expect at most // half the reordering distance to be candidates for the optimization. - int maxCount = min(m_blockIndirs.Height(), LDP_REORDERING_MAX_DISTANCE / 2); + int maxCount = min(m_blockIndirs.Height(), LDP_STP_REORDERING_MAX_DISTANCE / 2); for (int i = 0; i < maxCount; i++) { SavedIndir& prev = m_blockIndirs.TopRef(i); @@ -9097,11 +9101,22 @@ bool Lowering::OptimizeForLdp(GenTreeIndir* ind) continue; } + if (prevIndir->gtNext == nullptr) + { + // Deleted by other optimization + continue; + } + + if (prevIndir->OperIsStore() != ind->OperIsStore()) + { + continue; + } + JITDUMP("[%06u] and [%06u] are indirs off the same base with offsets +%03u and +%03u\n", Compiler::dspTreeID(ind), Compiler::dspTreeID(prevIndir), (unsigned)offs, (unsigned)prev.Offset); if (std::abs(offs - prev.Offset) == genTypeSize(ind)) { - JITDUMP(" ..and they are amenable to ldp optimization\n"); + JITDUMP(" ..and they are amenable to ldp/stp optimization\n"); if (TryMakeIndirsAdjacent(prevIndir, ind)) { // Do not let the previous one participate in @@ -9137,7 +9152,7 @@ bool Lowering::OptimizeForLdp(GenTreeIndir* ind) bool Lowering::TryMakeIndirsAdjacent(GenTreeIndir* prevIndir, GenTreeIndir* indir) { GenTree* cur = prevIndir; - for (int i = 0; i < LDP_REORDERING_MAX_DISTANCE; i++) + for (int i = 0; i < LDP_STP_REORDERING_MAX_DISTANCE; i++) { cur = cur->gtNext; if (cur == indir) @@ -9194,8 +9209,15 @@ bool Lowering::TryMakeIndirsAdjacent(GenTreeIndir* prevIndir, GenTreeIndir* indi INDEBUG(dumpWithMarks()); JITDUMP("\n"); + if ((prevIndir->gtLIRFlags & LIR::Flags::Mark) != 0) + { + JITDUMP("Previous indir is part of the data flow of current indir\n"); + return false; + } + m_scratchSideEffects.Clear(); + bool sawData = false; for (GenTree* cur = prevIndir->gtNext; cur != indir; cur = cur->gtNext) { if ((cur->gtLIRFlags & LIR::Flags::Mark) != 0) @@ -9208,6 +9230,11 @@ bool Lowering::TryMakeIndirsAdjacent(GenTreeIndir* prevIndir, GenTreeIndir* indi UnmarkTree(indir); return false; } + + if (indir->OperIsStore()) + { + sawData |= cur == indir->Data(); + } } else { @@ -9219,6 +9246,12 @@ bool Lowering::TryMakeIndirsAdjacent(GenTreeIndir* prevIndir, GenTreeIndir* indi if (m_scratchSideEffects.InterferesWith(comp, indir, true)) { + if (!indir->OperIsLoad()) + { + JITDUMP("Have conservative interference with last store. Giving up.\n"); + return false; + } + // Try a bit harder, making use of the following facts: // // 1. We know the indir is non-faulting, so we do not need to worry @@ -9315,8 +9348,42 @@ bool Lowering::TryMakeIndirsAdjacent(GenTreeIndir* prevIndir, GenTreeIndir* indi } } - JITDUMP("Interference checks passed. Moving nodes that are not part of data flow of [%06u]\n\n", - Compiler::dspTreeID(indir)); + JITDUMP("Interference checks passed: can to move unrelated nodes past second indir.\n"); + + if (sawData) + { + // If the data node of 'indir' is between 'prevIndir' and 'indir' then + // try to move the previous indir up happen after the data computation. + // We will be moving all nodes unrelated to the data flow past 'indir', + // so we only need to check interference between 'prevIndir' and all + // nodes that are part of 'indir's dataflow. + m_scratchSideEffects.Clear(); + + for (GenTree* cur = prevIndir->gtNext;; cur = cur->gtNext) + { + // * Previous indir should be moved past data + // * All nodes past of the data flow of current indir should be moved past data + // Other nodes will be moved past the indir below. + + if ((cur->gtLIRFlags & LIR::Flags::Mark) != 0) + { + m_scratchSideEffects.AddNode(comp, cur); + } + + if (cur == indir->Data()) + { + break; + } + } + + if (m_scratchSideEffects.InterferesWith(comp, prevIndir, true)) + { + JITDUMP("Cannot move prev indir up after data computation\n"); + return false; + } + } + + JITDUMP("Moving nodes that are not part of data flow of [%06u]\n\n", Compiler::dspTreeID(indir)); GenTree* previous = prevIndir; for (GenTree* node = prevIndir->gtNext;;) @@ -9339,6 +9406,22 @@ bool Lowering::TryMakeIndirsAdjacent(GenTreeIndir* prevIndir, GenTreeIndir* indi node = next; } + if (sawData) + { + // For some reason LSRA is not able to reuse a constant if both LIR + // temps are live simultaneously, so skip moving in those cases and + // expect LSRA to reuse the constant instead. + if (!indir->Data()->IsCnsIntOrI() || !GenTree::Compare(indir->Data(), prevIndir->Data())) + { + BlockRange().Remove(prevIndir); + BlockRange().InsertAfter(indir->Data(), prevIndir); + } + else + { + JITDUMP("Not moving previous indir since we are expecting constant reuse for the data\n"); + } + } + JITDUMP("Result:\n\n"); INDEBUG(dumpWithMarks()); JITDUMP("\n"); diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h index dc32f7941f53dc..898bc8fef53897 100644 --- a/src/coreclr/jit/lower.h +++ b/src/coreclr/jit/lower.h @@ -345,13 +345,13 @@ class Lowering final : public Phase bool GetLoadStoreCoalescingData(GenTreeIndir* ind, LoadStoreCoalescingData* data) const; // Per tree node member functions - void LowerStoreIndirCommon(GenTreeStoreInd* ind); + GenTree* LowerStoreIndirCommon(GenTreeStoreInd* ind); GenTree* LowerIndir(GenTreeIndir* ind); - bool OptimizeForLdp(GenTreeIndir* ind); + bool OptimizeForLdpStp(GenTreeIndir* ind); bool TryMakeIndirsAdjacent(GenTreeIndir* prevIndir, GenTreeIndir* indir); void MarkTree(GenTree* root); void UnmarkTree(GenTree* root); - void LowerStoreIndir(GenTreeStoreInd* node); + GenTree* LowerStoreIndir(GenTreeStoreInd* node); void LowerStoreIndirCoalescing(GenTreeIndir* node); GenTree* LowerAdd(GenTreeOp* node); GenTree* LowerMul(GenTreeOp* mul); diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index 6e3b8e3202991a..34ed6da7e84b9b 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -491,11 +491,21 @@ void Lowering::LowerStoreLoc(GenTreeLclVarCommon* storeLoc) // node - The indirect store node (GT_STORE_IND) of interest // // Return Value: -// None. +// Next node to lower. // -void Lowering::LowerStoreIndir(GenTreeStoreInd* node) +GenTree* Lowering::LowerStoreIndir(GenTreeStoreInd* node) { + GenTree* next = node->gtNext; ContainCheckStoreIndir(node); + +#ifdef TARGET_ARM64 + if (comp->opts.OptimizationEnabled()) + { + OptimizeForLdpStp(node); + } +#endif + + return next; } //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/lowerloongarch64.cpp b/src/coreclr/jit/lowerloongarch64.cpp index d3552e478fdfef..1bed44ca9e16e0 100644 --- a/src/coreclr/jit/lowerloongarch64.cpp +++ b/src/coreclr/jit/lowerloongarch64.cpp @@ -266,11 +266,12 @@ void Lowering::LowerStoreLoc(GenTreeLclVarCommon* storeLoc) // node - The indirect store node (GT_STORE_IND) of interest // // Return Value: -// None. +// Next node to lower. // -void Lowering::LowerStoreIndir(GenTreeStoreInd* node) +GenTree* Lowering::LowerStoreIndir(GenTreeStoreInd* node) { ContainCheckStoreIndir(node); + return node->gtNext; } //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/lowerriscv64.cpp b/src/coreclr/jit/lowerriscv64.cpp index 5b0bd99df484f3..e621b1d4f0e391 100644 --- a/src/coreclr/jit/lowerriscv64.cpp +++ b/src/coreclr/jit/lowerriscv64.cpp @@ -215,11 +215,12 @@ void Lowering::LowerStoreLoc(GenTreeLclVarCommon* storeLoc) // node - The indirect store node (GT_STORE_IND) of interest // // Return Value: -// None. +// Next node to lower. // -void Lowering::LowerStoreIndir(GenTreeStoreInd* node) +GenTree* Lowering::LowerStoreIndir(GenTreeStoreInd* node) { ContainCheckStoreIndir(node); + return node->gtNext; } //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/lowerxarch.cpp b/src/coreclr/jit/lowerxarch.cpp index dd2bc51ffcfa24..e02fb9a6c2ef46 100644 --- a/src/coreclr/jit/lowerxarch.cpp +++ b/src/coreclr/jit/lowerxarch.cpp @@ -73,9 +73,9 @@ void Lowering::LowerStoreLoc(GenTreeLclVarCommon* storeLoc) // node - The indirect store node (GT_STORE_IND) of interest // // Return Value: -// None. +// Next node to lower. // -void Lowering::LowerStoreIndir(GenTreeStoreInd* node) +GenTree* Lowering::LowerStoreIndir(GenTreeStoreInd* node) { // Mark all GT_STOREIND nodes to indicate that it is not known // whether it represents a RMW memory op. @@ -92,7 +92,7 @@ void Lowering::LowerStoreIndir(GenTreeStoreInd* node) // SSE2 doesn't support RMW form of instructions. if (LowerRMWMemOp(node)) { - return; + return node->gtNext; } } @@ -109,23 +109,25 @@ void Lowering::LowerStoreIndir(GenTreeStoreInd* node) { if (!node->Data()->IsCnsVec()) { - return; + return node->gtNext; } if (!node->Data()->AsVecCon()->TypeIs(TYP_SIMD32, TYP_SIMD64)) { - return; + return node->gtNext; } if (node->Data()->IsVectorAllBitsSet() || node->Data()->IsVectorZero()) { // To avoid some unexpected regression, this optimization only applies to non-all 1/0 constant vectors. - return; + return node->gtNext; } TryCompressConstVecData(node); } #endif + + return node->gtNext; } //---------------------------------------------------------------------------------------------- From 669681862a76a3f321f52b74de10ec41e98dd3a3 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Sun, 12 May 2024 16:43:24 +0200 Subject: [PATCH 2/7] Unmark tree; fix JITDUMP --- src/coreclr/jit/lower.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 12861dbb8c186b..9d9d626936b6e6 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -9212,6 +9212,7 @@ bool Lowering::TryMakeIndirsAdjacent(GenTreeIndir* prevIndir, GenTreeIndir* indi if ((prevIndir->gtLIRFlags & LIR::Flags::Mark) != 0) { JITDUMP("Previous indir is part of the data flow of current indir\n"); + UnmarkTree(indir); return false; } @@ -9249,6 +9250,7 @@ bool Lowering::TryMakeIndirsAdjacent(GenTreeIndir* prevIndir, GenTreeIndir* indi if (!indir->OperIsLoad()) { JITDUMP("Have conservative interference with last store. Giving up.\n"); + UnmarkTree(indir); return false; } @@ -9348,7 +9350,7 @@ bool Lowering::TryMakeIndirsAdjacent(GenTreeIndir* prevIndir, GenTreeIndir* indi } } - JITDUMP("Interference checks passed: can to move unrelated nodes past second indir.\n"); + JITDUMP("Interference checks passed: can move unrelated nodes past second indir.\n"); if (sawData) { @@ -9379,6 +9381,7 @@ bool Lowering::TryMakeIndirsAdjacent(GenTreeIndir* prevIndir, GenTreeIndir* indi if (m_scratchSideEffects.InterferesWith(comp, prevIndir, true)) { JITDUMP("Cannot move prev indir up after data computation\n"); + UnmarkTree(indir); return false; } } From ef40b649016b0b85ec85a38324eb7965146a1a58 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Sun, 12 May 2024 16:56:03 +0200 Subject: [PATCH 3/7] Remove outdated comment --- src/coreclr/jit/lower.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 9d9d626936b6e6..c4ce61b72c6be0 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -9363,10 +9363,6 @@ bool Lowering::TryMakeIndirsAdjacent(GenTreeIndir* prevIndir, GenTreeIndir* indi for (GenTree* cur = prevIndir->gtNext;; cur = cur->gtNext) { - // * Previous indir should be moved past data - // * All nodes past of the data flow of current indir should be moved past data - // Other nodes will be moved past the indir below. - if ((cur->gtLIRFlags & LIR::Flags::Mark) != 0) { m_scratchSideEffects.AddNode(comp, cur); From da4fbb1254d95bf354d3f533f3a7d7c1ed46f2be Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Sun, 12 May 2024 17:00:02 +0200 Subject: [PATCH 4/7] Rephrase interference check --- src/coreclr/jit/lower.cpp | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index c4ce61b72c6be0..3e215da6961ad4 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -9355,17 +9355,24 @@ bool Lowering::TryMakeIndirsAdjacent(GenTreeIndir* prevIndir, GenTreeIndir* indi if (sawData) { // If the data node of 'indir' is between 'prevIndir' and 'indir' then - // try to move the previous indir up happen after the data computation. - // We will be moving all nodes unrelated to the data flow past 'indir', - // so we only need to check interference between 'prevIndir' and all - // nodes that are part of 'indir's dataflow. + // try to move the previous indir up to happen after the data + // computation. We will be moving all nodes unrelated to the data flow + // past 'indir', so we only need to check interference between + // 'prevIndir' and all nodes that are part of 'indir's dataflow. m_scratchSideEffects.Clear(); + m_scratchSideEffects.AddNode(comp, prevIndir); for (GenTree* cur = prevIndir->gtNext;; cur = cur->gtNext) { if ((cur->gtLIRFlags & LIR::Flags::Mark) != 0) { - m_scratchSideEffects.AddNode(comp, cur); + if (m_scratchSideEffects.InterferesWith(comp, cur, true)) + { + JITDUMP("Cannot move prev indir [%06u] up past [%06u] to get it past the data computation\n", + Compiler::dspTreeID(prevIndir), Compiler::dspTreeID(cur)); + UnmarkTree(indir); + return false; + } } if (cur == indir->Data()) @@ -9373,13 +9380,6 @@ bool Lowering::TryMakeIndirsAdjacent(GenTreeIndir* prevIndir, GenTreeIndir* indi break; } } - - if (m_scratchSideEffects.InterferesWith(comp, prevIndir, true)) - { - JITDUMP("Cannot move prev indir up after data computation\n"); - UnmarkTree(indir); - return false; - } } JITDUMP("Moving nodes that are not part of data flow of [%06u]\n\n", Compiler::dspTreeID(indir)); From fb718e5a5111613be58567dce81cebfac18df3f6 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Sun, 12 May 2024 17:20:08 +0200 Subject: [PATCH 5/7] Run jit-format --- 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 3e215da6961ad4..ba5c3390aaceef 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -9369,7 +9369,7 @@ bool Lowering::TryMakeIndirsAdjacent(GenTreeIndir* prevIndir, GenTreeIndir* indi if (m_scratchSideEffects.InterferesWith(comp, cur, true)) { JITDUMP("Cannot move prev indir [%06u] up past [%06u] to get it past the data computation\n", - Compiler::dspTreeID(prevIndir), Compiler::dspTreeID(cur)); + Compiler::dspTreeID(prevIndir), Compiler::dspTreeID(cur)); UnmarkTree(indir); return false; } From 233554abe9abc486f8547cd69cb394ae105e9c5a Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Mon, 13 May 2024 11:03:19 +0200 Subject: [PATCH 6/7] Handle more constants --- src/coreclr/jit/lower.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index ba5c3390aaceef..e820c39b2fb69c 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -9410,14 +9410,14 @@ bool Lowering::TryMakeIndirsAdjacent(GenTreeIndir* prevIndir, GenTreeIndir* indi // For some reason LSRA is not able to reuse a constant if both LIR // temps are live simultaneously, so skip moving in those cases and // expect LSRA to reuse the constant instead. - if (!indir->Data()->IsCnsIntOrI() || !GenTree::Compare(indir->Data(), prevIndir->Data())) + if (indir->Data()->OperIs(GT_CNS_INT, GT_CNS_DBL, GT_CNS_VEC) && GenTree::Compare(indir->Data(), prevIndir->Data())) { - BlockRange().Remove(prevIndir); - BlockRange().InsertAfter(indir->Data(), prevIndir); + JITDUMP("Not moving previous indir since we are expecting constant reuse for the data\n"); } else { - JITDUMP("Not moving previous indir since we are expecting constant reuse for the data\n"); + BlockRange().Remove(prevIndir); + BlockRange().InsertAfter(indir->Data(), prevIndir); } } From 5a78b5ec8fbcaa6799971eb1b5c54cccf80a1a46 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Mon, 13 May 2024 11:51:48 +0200 Subject: [PATCH 7/7] Handle GT_CNS_DBL reuse as well --- 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 4d7dac36822163..afea141be4c220 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -9392,7 +9392,7 @@ bool Lowering::TryMakeIndirsAdjacent(GenTreeIndir* prevIndir, GenTreeIndir* indi // For some reason LSRA is not able to reuse a constant if both LIR // temps are live simultaneously, so skip moving in those cases and // expect LSRA to reuse the constant instead. - if (indir->Data()->OperIs(GT_CNS_INT, GT_CNS_DBL, GT_CNS_VEC) && GenTree::Compare(indir->Data(), prevIndir->Data())) + if (indir->Data()->OperIs(GT_CNS_INT, GT_CNS_DBL) && GenTree::Compare(indir->Data(), prevIndir->Data())) { JITDUMP("Not moving previous indir since we are expecting constant reuse for the data\n"); }