Skip to content

Commit d9307e4

Browse files
authored
JIT: Allow forward sub to reorder trees throwing the same single exception (#85647)
Allow forward sub to move a tree past another tree that throws an exception provided that they both throw the same exception.
1 parent 4b104fb commit d9307e4

File tree

1 file changed

+91
-30
lines changed

1 file changed

+91
-30
lines changed

src/coreclr/jit/forwardsub.cpp

Lines changed: 91 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -190,16 +190,7 @@ class ForwardSubVisitor final : public GenTreeVisitor<ForwardSubVisitor>
190190
};
191191

192192
ForwardSubVisitor(Compiler* compiler, unsigned lclNum, bool livenessBased)
193-
: GenTreeVisitor(compiler)
194-
, m_use(nullptr)
195-
, m_node(nullptr)
196-
, m_parentNode(nullptr)
197-
, m_lclNum(lclNum)
198-
, m_parentLclNum(BAD_VAR_NUM)
199-
, m_useFlags(GTF_EMPTY)
200-
, m_accumulatedFlags(GTF_EMPTY)
201-
, m_treeSize(0)
202-
, m_livenessBased(livenessBased)
193+
: GenTreeVisitor(compiler), m_lclNum(lclNum), m_livenessBased(livenessBased)
203194
{
204195
LclVarDsc* dsc = compiler->lvaGetDesc(m_lclNum);
205196
if (dsc->lvIsStructField)
@@ -238,10 +229,11 @@ class ForwardSubVisitor final : public GenTreeVisitor<ForwardSubVisitor>
238229

239230
if (!isDef && !isCallTarget && IsLastUse(node->AsLclVar()))
240231
{
241-
m_node = node;
242-
m_use = use;
243-
m_useFlags = m_accumulatedFlags;
244-
m_parentNode = parent;
232+
m_node = node;
233+
m_use = use;
234+
m_useFlags = m_accumulatedFlags;
235+
m_useExceptions = m_accumulatedExceptions;
236+
m_parentNode = parent;
245237
}
246238
}
247239
}
@@ -274,6 +266,20 @@ class ForwardSubVisitor final : public GenTreeVisitor<ForwardSubVisitor>
274266
}
275267

276268
m_accumulatedFlags |= (node->gtFlags & GTF_GLOB_EFFECT);
269+
if ((node->gtFlags & GTF_CALL) != 0)
270+
{
271+
m_accumulatedExceptions = ExceptionSetFlags::All;
272+
}
273+
else if ((node->gtFlags & GTF_EXCEPT) != 0)
274+
{
275+
// We can never reorder in the face of different exception types,
276+
// so stop calling 'OperExceptions' once we've seen more than one
277+
// different exception type.
278+
if (genCountBits(static_cast<uint32_t>(m_accumulatedExceptions)) <= 1)
279+
{
280+
m_accumulatedExceptions |= node->OperExceptions(m_compiler);
281+
}
282+
}
277283

278284
return fgWalkResult::WALK_CONTINUE;
279285
}
@@ -305,6 +311,23 @@ class ForwardSubVisitor final : public GenTreeVisitor<ForwardSubVisitor>
305311
return m_useFlags;
306312
}
307313

314+
//------------------------------------------------------------------------
315+
// GetExceptions: Get precise exceptions thrown by the trees executed
316+
// before the use.
317+
//
318+
// Returns:
319+
// Exception set.
320+
//
321+
// Remarks:
322+
// The visitor stops tracking precise exceptions once it finds that 2 or
323+
// more different exceptions can be thrown, so this set cannot be used
324+
// for determining the precise different exceptions thrown in that case.
325+
//
326+
ExceptionSetFlags GetExceptions() const
327+
{
328+
return m_useExceptions;
329+
}
330+
308331
bool IsCallArg() const
309332
{
310333
return m_parentNode->IsCall();
@@ -366,18 +389,23 @@ class ForwardSubVisitor final : public GenTreeVisitor<ForwardSubVisitor>
366389
}
367390

368391
private:
369-
GenTree** m_use;
370-
GenTree* m_node;
371-
GenTree* m_parentNode;
392+
GenTree** m_use = nullptr;
393+
GenTree* m_node = nullptr;
394+
GenTree* m_parentNode = nullptr;
372395
unsigned m_lclNum;
373-
unsigned m_parentLclNum;
396+
unsigned m_parentLclNum = BAD_VAR_NUM;
374397
#ifdef DEBUG
375398
unsigned m_useCount = 0;
376399
#endif
377-
GenTreeFlags m_useFlags;
378-
GenTreeFlags m_accumulatedFlags;
379-
unsigned m_treeSize;
380-
bool m_livenessBased;
400+
GenTreeFlags m_useFlags = GTF_EMPTY;
401+
GenTreeFlags m_accumulatedFlags = GTF_EMPTY;
402+
// Precise exceptions thrown by the nodes that were visited so far. Note
403+
// that we stop updating this field once we find that two or more separate
404+
// exceptions.
405+
ExceptionSetFlags m_accumulatedExceptions = ExceptionSetFlags::None;
406+
ExceptionSetFlags m_useExceptions = ExceptionSetFlags::None;
407+
unsigned m_treeSize = 0;
408+
bool m_livenessBased;
381409
};
382410

383411
//------------------------------------------------------------------------
@@ -689,23 +717,56 @@ bool Compiler::fgForwardSubStatement(Statement* stmt)
689717
//
690718
// if the next tree can't change the value of fwdSubNode or be impacted by fwdSubNode effects
691719
//
692-
const bool fwdSubNodeInvariant = ((fwdSubNode->gtFlags & GTF_ALL_EFFECT) == 0);
693-
const bool nextTreeIsPureUpToUse = ((fsv.GetFlags() & (GTF_EXCEPT | GTF_GLOB_REF | GTF_CALL)) == 0);
694-
if (!fwdSubNodeInvariant && !nextTreeIsPureUpToUse)
720+
if (((fwdSubNode->gtFlags & GTF_CALL) != 0) && ((fsv.GetFlags() & GTF_ALL_EFFECT) != 0))
695721
{
696-
// Fwd sub may impact global values and or reorder exceptions...
697-
//
698-
JITDUMP(" potentially interacting effects\n");
722+
JITDUMP(" cannot reorder call with any side effect\n");
723+
return false;
724+
}
725+
if (((fwdSubNode->gtFlags & GTF_GLOB_REF) != 0) && ((fsv.GetFlags() & GTF_PERSISTENT_SIDE_EFFECTS) != 0))
726+
{
727+
JITDUMP(" cannot reorder global reference with persistent side effects\n");
699728
return false;
700729
}
730+
if (((fwdSubNode->gtFlags & GTF_ORDER_SIDEEFF) != 0) &&
731+
((fsv.GetFlags() & (GTF_GLOB_REF | GTF_ORDER_SIDEEFF)) != 0))
732+
{
733+
JITDUMP(" cannot reorder ordering side effect with global reference/ordering side effect\n");
734+
return false;
735+
}
736+
if ((fwdSubNode->gtFlags & GTF_EXCEPT) != 0)
737+
{
738+
if ((fsv.GetFlags() & GTF_PERSISTENT_SIDE_EFFECTS) != 0)
739+
{
740+
JITDUMP(" cannot reorder exception with persistent side effect\n");
741+
return false;
742+
}
743+
744+
if ((fsv.GetFlags() & GTF_EXCEPT) != 0)
745+
{
746+
assert(fsv.GetExceptions() != ExceptionSetFlags::None);
747+
if (genCountBits(static_cast<uint32_t>(fsv.GetExceptions())) > 1)
748+
{
749+
JITDUMP(" cannot reorder different thrown exceptions\n");
750+
return false;
751+
}
752+
753+
ExceptionSetFlags fwdSubNodeExceptions = gtCollectExceptions(fwdSubNode);
754+
assert(fwdSubNodeExceptions != ExceptionSetFlags::None);
755+
if (fwdSubNodeExceptions != fsv.GetExceptions())
756+
{
757+
JITDUMP(" cannot reorder different thrown exceptions\n");
758+
return false;
759+
}
760+
}
761+
}
701762

702763
// If we're relying on purity of fwdSubNode for legality of forward sub,
703764
// do some extra checks for global uses that might not be reflected in the flags.
704765
//
705766
// TODO: remove this once we can trust upstream phases and/or gtUpdateStmtSideEffects
706767
// to set GTF_GLOB_REF properly.
707768
//
708-
if (fwdSubNodeInvariant && ((fsv.GetFlags() & (GTF_CALL | GTF_ASG)) != 0))
769+
if ((fsv.GetFlags() & GTF_PERSISTENT_SIDE_EFFECTS) != 0)
709770
{
710771
EffectsVisitor ev(this);
711772
ev.WalkTree(&fwdSubNode, nullptr);
@@ -874,7 +935,7 @@ bool Compiler::fgForwardSubStatement(Statement* stmt)
874935
fgForwardSubUpdateLiveness(firstLcl, lhsNode->gtPrev);
875936
}
876937

877-
if (!fwdSubNodeInvariant)
938+
if ((fwdSubNode->gtFlags & GTF_ALL_EFFECT) != 0)
878939
{
879940
gtUpdateStmtSideEffects(nextStmt);
880941
}

0 commit comments

Comments
 (0)