Skip to content

Commit

Permalink
Test commit to kick off diffs
Browse files Browse the repository at this point in the history
  • Loading branch information
@jakobbotsch
jakobbotsch committed Sep 29, 2023
1 parent a74949b commit 55e4bab
Show file tree
Hide file tree
Showing 4 changed files with 245 additions and 5 deletions.
222 changes: 219 additions & 3 deletions src/coreclr/jit/lower.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -409,8 +409,11 @@ GenTree* Lowering::LowerNode(GenTree* node)
{
case GT_NULLCHECK:
case GT_IND:
LowerIndir(node->AsIndir());
break;
{
GenTree* next = nullptr;
LowerIndir(node->AsIndir(), &next);
return next;
}

case GT_STOREIND:
LowerStoreIndirCommon(node->AsStoreInd());
Expand Down Expand Up @@ -7334,6 +7337,7 @@ void Lowering::LowerBlock(BasicBlock* block)
assert(block->isEmpty() || block->IsLIR());

m_block = block;
m_blockIndirs.Reset();

// NOTE: some of the lowering methods insert calls before the node being
// lowered (See e.g. InsertPInvoke{Method,Call}{Prolog,Epilog}). In
Expand Down Expand Up @@ -7823,6 +7827,10 @@ void Lowering::LowerStoreIndirCommon(GenTreeStoreInd* ind)

LowerStoreIndir(ind);
}

#ifdef TARGET_ARM64
m_blockIndirs.Push(ind);
#endif
}

//------------------------------------------------------------------------
Expand All @@ -7831,8 +7839,13 @@ void Lowering::LowerStoreIndirCommon(GenTreeStoreInd* ind)
// Arguments:
// ind - the ind node we are lowering.
//
void Lowering::LowerIndir(GenTreeIndir* ind)
void Lowering::LowerIndir(GenTreeIndir* ind, GenTree** next)
{
if (next != nullptr)
{
*next = ind->gtNext;
}

assert(ind->OperIs(GT_IND, GT_NULLCHECK));
// Process struct typed indirs separately unless they are unused;
// they only appear as the source of a block copy operation or a return node.
Expand Down Expand Up @@ -7879,6 +7892,209 @@ void Lowering::LowerIndir(GenTreeIndir* ind)
const bool isContainable = false;
TryCreateAddrMode(ind->Addr(), isContainable, ind);
}

#ifdef TARGET_ARM64
if (ind->OperIs(GT_IND) && comp->opts.OptimizationEnabled() && (next != nullptr))
{
target_ssize_t offs1 = 0;
GenTree* addr = ind->Addr();
if (addr->OperIs(GT_LEA) && !addr->AsAddrMode()->HasIndex())
{
offs1 = (target_ssize_t)addr->AsAddrMode()->Offset();
addr = addr->AsAddrMode()->Base();
}
else if (addr->OperIs(GT_ADD) && addr->gtGetOp2()->IsCnsIntOrI())
{
offs1 = (target_ssize_t)addr->gtGetOp2()->AsIntConCommon()->IconValue();
addr = addr->gtGetOp1();
}

if (ind->TypeIs(TYP_INT, TYP_LONG, TYP_DOUBLE, TYP_SIMD16))
{
for (int i = 0; i < m_blockIndirs.Height(); i++)
{
GenTreeIndir* prevIndir = m_blockIndirs.Top(i);
if (!prevIndir->OperIs(GT_IND))
{
continue;
}

target_ssize_t offs2 = 0;
GenTree* prevAddr = prevIndir->Addr();
if (prevAddr->OperIs(GT_LEA) && !prevAddr->AsAddrMode()->HasIndex())
{
offs2 = (target_ssize_t)prevAddr->AsAddrMode()->Offset();
prevAddr = prevAddr->AsAddrMode()->Base();
}
else if (prevAddr->OperIs(GT_ADD) && prevAddr->gtGetOp2()->IsCnsIntOrI())
{
offs2 = (target_ssize_t)prevAddr->gtGetOp2()->AsIntConCommon()->IconValue();
prevAddr = prevAddr->gtGetOp1();
}

if (addr->OperIsLocal() && GenTree::Compare(addr, prevAddr))
{
JITDUMP("[%06u] and [%06u] are indirs off the same base with offsets +%03u and +%03u\n",
Compiler::dspTreeID(ind), Compiler::dspTreeID(prevIndir), offs1, offs2);
if (abs(offs1 - offs2) == genTypeSize(ind))
{
JITDUMP(" ..and they are amenable to ldp optimization\n");
TryOptimizeForLdp(prevIndir, ind);
break;
}
else
{
JITDUMP(" ..but at wrong offset\n");
}
}
}
}

m_blockIndirs.Push(ind);
}
#endif
}

static void MarkTree(GenTree* node)
{
node->gtLIRFlags |= LIR::Flags::Mark;
node->VisitOperands([](GenTree* op) {
MarkTree(op);
return GenTree::VisitResult::Continue;
});
}

static void UnmarkTree(GenTree* node)
{
node->gtLIRFlags &= ~LIR::Flags::Mark;
node->VisitOperands([](GenTree* op) {
UnmarkTree(op);
return GenTree::VisitResult::Continue;
});
}

bool Lowering::TryOptimizeForLdp(GenTreeIndir* prevIndir, GenTreeIndir* indir)
{
if ((indir->gtFlags & GTF_IND_VOLATILE) != 0)
{
JITDUMP(" ..but second indir is volatile\n");
return false;
}

GenTree* cur = prevIndir;
for (int i = 0; i < 16; i++)
{
cur = cur->gtNext;
if (cur == indir)
break;
}

if (cur != indir)
{
JITDUMP(" ..but it is too far away\n");
return false;
}

JITDUMP(
" ..and it is close by. Trying to move the following range (where * are nodes part of the data flow):\n\n");
#ifdef DEBUG
bool isClosed;
GenTree* startDumpNode = BlockRange().GetTreeRange(prevIndir, &isClosed).FirstNode();
GenTree* endDumpNode = indir->gtNext;

auto dumpWithMarks = [=]() {
if (!comp->verbose)
{
return;
}

for (GenTree* node = startDumpNode; node != endDumpNode; node = node->gtNext)
{
const char* prefix;
if (node == prevIndir)
prefix = "1. ";
else if (node == indir)
prefix = "2. ";
else if ((node->gtLIRFlags & LIR::Flags::Mark) != 0)
prefix = "* ";
else
prefix = " ";

comp->gtDispLIRNode(node, prefix);
}
};

#endif

MarkTree(indir);

INDEBUG(dumpWithMarks());
JITDUMP("\n");

m_scratchSideEffects.Clear();

for (GenTree* cur = prevIndir->gtNext; cur != indir; cur = cur->gtNext)
{
if ((cur->gtLIRFlags & LIR::Flags::Mark) != 0)
{
// Part of data flow of 'indir', so we will be moving past this node.
if (m_scratchSideEffects.InterferesWith(comp, cur, true))
{
JITDUMP("Giving up due to interference with [%06u]\n", Compiler::dspTreeID(cur));
UnmarkTree(indir);
return false;
}
}
else
{
// Part of dataflow; add its effects.
m_scratchSideEffects.AddNode(comp, cur);
}
}

// Can we move it past the 'indir'? We can ignore effect flags when
// checking this, as we know the previous indir will make it non-faulting
// and keep the ordering correct. This makes use of the fact that
// non-volatile indirs have ordering side effects only for the "suppressed
// NRE" case.
// We still need some interference cehcks to ensure that the indir reads
// the same value even after reordering.
if (m_scratchSideEffects.InterferesWith(comp, indir, GTF_EMPTY, true))
{
JITDUMP("Giving up due to interference with [%06u]\n", Compiler::dspTreeID(indir));
UnmarkTree(indir);
return false;
}

JITDUMP("Interference checks passed. Moving nodes that are not part of data flow tree\n\n",
Compiler::dspTreeID(indir));

GenTree* previous = prevIndir;
for (GenTree* node = prevIndir->gtNext;;)
{
GenTree* next = node->gtNext;

if ((node->gtLIRFlags & LIR::Flags::Mark) != 0)
{
// Part of data flow. Move it to happen right after 'previous'.
BlockRange().Remove(node);
BlockRange().InsertAfter(previous, node);
previous = node;
}

if (node == indir)
{
break;
}

node = next;
}

JITDUMP("Result:\n\n");
INDEBUG(dumpWithMarks());
JITDUMP("\n");
UnmarkTree(indir);
return true;
}

//------------------------------------------------------------------------
Expand Down
8 changes: 6 additions & 2 deletions src/coreclr/jit/lower.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ class Lowering final : public Phase
{
public:
inline Lowering(Compiler* compiler, LinearScanInterface* lsra)
: Phase(compiler, PHASE_LOWERING), vtableCallTemp(BAD_VAR_NUM)
: Phase(compiler, PHASE_LOWERING)
, vtableCallTemp(BAD_VAR_NUM)
, m_blockIndirs(compiler->getAllocator(CMK_ArrayStack))
{
m_lsra = (LinearScan*)lsra;
assert(m_lsra);
Expand Down Expand Up @@ -310,7 +312,8 @@ class Lowering final : public Phase

// Per tree node member functions
void LowerStoreIndirCommon(GenTreeStoreInd* ind);
void LowerIndir(GenTreeIndir* ind);
void LowerIndir(GenTreeIndir* ind, GenTree** next = nullptr);
bool TryOptimizeForLdp(GenTreeIndir* prevIndir, GenTreeIndir* indir);
void LowerStoreIndir(GenTreeStoreInd* node);
GenTree* LowerAdd(GenTreeOp* node);
GenTree* LowerMul(GenTreeOp* mul);
Expand Down Expand Up @@ -559,6 +562,7 @@ class Lowering final : public Phase
#ifdef FEATURE_FIXED_OUT_ARGS
unsigned m_outgoingArgSpaceSize = 0;
#endif
ArrayStack<GenTreeIndir*> m_blockIndirs;
};

#endif // _LOWER_H_
19 changes: 19 additions & 0 deletions src/coreclr/jit/sideeffects.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,25 @@ bool SideEffectSet::InterferesWith(Compiler* compiler, GenTree* node, bool stric
return InterferesWith(node->OperEffects(compiler), AliasSet::NodeInfo(compiler, node), strict);
}

//------------------------------------------------------------------------
// SideEffectSet::InterferesWith:
// Returns true if the side effects in this set interfere with the side
// effects for the given node.
//
// A side effect set interferes with a given node iff it interferes
// with the side effect set of the node.
//
// Arguments:
// compiler - The compiler context.
// node - The node in question.
// overriddenSideEffects - Side effect flags to use in place of the ones from the node
// strict - True if the analysis should be strict as described above.
//
bool SideEffectSet::InterferesWith(Compiler* compiler, GenTree* node, unsigned overriddenSideEffects, bool strict) const
{
return InterferesWith(overriddenSideEffects, AliasSet::NodeInfo(compiler, node), strict);
}

//------------------------------------------------------------------------
// SideEffectSet::Clear:
// Clears the current side effect set.
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/sideeffects.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ class SideEffectSet final
void AddNode(Compiler* compiler, GenTree* node);
bool InterferesWith(const SideEffectSet& other, bool strict) const;
bool InterferesWith(Compiler* compiler, GenTree* node, bool strict) const;
bool InterferesWith(Compiler* compiler, GenTree* node, unsigned overriddenSideEffects, bool strict) const;
void Clear();
};

Expand Down

0 comments on commit 55e4bab

Please sign in to comment.