Skip to content

Commit

Permalink
JIT: Remove duplicated stores (#93084)
Browse files Browse the repository at this point in the history
  • Loading branch information
EgorBo authored Oct 6, 2023
1 parent 7d429e4 commit 52806bc
Showing 1 changed file with 23 additions and 8 deletions.
31 changes: 23 additions & 8 deletions src/coreclr/jit/lower.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7907,6 +7907,9 @@ static bool GetStoreCoalescingData(Compiler* comp, GenTreeStoreInd* ind, StoreCo
// | \--* LCL_VAR byref V00
// \--* CNS_INT long 0x200000001
//
// NOTE: Our memory model allows us to do this optimization, see Memory-model.md:
// * Adjacent non-volatile writes to the same location can be coalesced. (see Memory-model.md)
//
// Arguments:
// ind - the current STOREIND node
//
Expand All @@ -7920,13 +7923,6 @@ void Lowering::LowerStoreIndirCoalescing(GenTreeStoreInd* ind)
return;
}

// For now, we require the current STOREIND to have LEA (previous store may not have it)
// So we can easily adjust the offset, consider making it more flexible in future.
if (!ind->Addr()->OperIs(GT_LEA))
{
return;
}

// TODO-ARM64-CQ: enable TYP_REF if we find a case where it's beneficial.
// The algorithm does support TYP_REF (with null value), but it seems to be not worth
// it on ARM64 where it's pretty efficient to do "stp xzr, xzr, [addr]" to clear two
Expand Down Expand Up @@ -8007,12 +8003,31 @@ void Lowering::LowerStoreIndirCoalescing(GenTreeStoreInd* ind)
return;
}

// Offset has to match the size of the type. We don't support the same or overlapping offsets.
// At this point we know that we have two consecutive STOREINDs with the same base address,
// index and scale, the only variable thing is the offset (constant)

// The same offset means that we're storing to the same location of the same width.
// Just remove the previous store then.
if (prevData.offset == currData.offset)
{
BlockRange().Remove(std::move(prevIndRange));
continue;
}

// Otherwise, the difference between two offsets has to match the size of the type.
// We don't support overlapping stores.
if (abs(prevData.offset - currData.offset) != (int)genTypeSize(prevData.targetType))
{
return;
}

// For now, we require the current STOREIND to have LEA (previous store may not have it)
// So we can easily adjust the offset, consider making it more flexible in future.
if (!ind->Addr()->OperIs(GT_LEA))
{
return;
}

// Now the hardest part: decide whether it's safe to use an unaligned write.
//
// IND<byte> is always fine (and all IND<X> created here from such)
Expand Down

0 comments on commit 52806bc

Please sign in to comment.