Skip to content

Commit daf6cdc

Browse files
tannergoodingtannergooding
authored
Allow gtFoldExprSpecial to handle side effects (#103382)
* Allow gtFoldExprSpecial to handle side effects * Ensure that return expressions are properly treated as having side effects
1 parent 0afabfb commit daf6cdc

File tree

1 file changed

+65
-40
lines changed

1 file changed

+65
-40
lines changed

‎src/coreclr/jit/gentree.cpp

+65-40
Original file line numberDiff line numberDiff line change
@@ -14327,15 +14327,12 @@ GenTree* Compiler::gtFoldExprSpecial(GenTree* tree)
1432714327

1432814328
val = cons->AsIntConCommon()->IconValue();
1432914329

14330-
// Transforms that would drop op cannot be performed if op has side effects
14331-
bool opHasSideEffects = (op->gtFlags & GTF_SIDE_EFFECT) != 0;
14332-
1433314330
// Helper function that creates a new IntCon node and morphs it, if required
1433414331
auto NewMorphedIntConNode = [&](int value) -> GenTreeIntCon* {
1433514332
GenTreeIntCon* icon = gtNewIconNode(value);
1433614333
if (fgGlobalMorph)
1433714334
{
14338-
fgMorphTreeDone(icon);
14335+
INDEBUG(icon->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED);
1433914336
}
1434014337
return icon;
1434114338
};
@@ -14370,43 +14367,52 @@ GenTree* Compiler::gtFoldExprSpecial(GenTree* tree)
1437014367
switch (oper)
1437114368
{
1437214369
case GT_LE:
14373-
if (tree->IsUnsigned() && (val == 0) && (op1 == cons) && !opHasSideEffects)
14370+
{
14371+
if (tree->IsUnsigned() && (val == 0) && (op1 == cons))
1437414372
{
1437514373
// unsigned (0 <= x) is always true
14376-
op = NewMorphedIntConNode(1);
14374+
op = gtWrapWithSideEffects(NewMorphedIntConNode(1), op, GTF_ALL_EFFECT);
1437714375
goto DONE_FOLD;
1437814376
}
1437914377
break;
14378+
}
1438014379

1438114380
case GT_GE:
14382-
if (tree->IsUnsigned() && (val == 0) && (op2 == cons) && !opHasSideEffects)
14381+
{
14382+
if (tree->IsUnsigned() && (val == 0) && (op2 == cons))
1438314383
{
1438414384
// unsigned (x >= 0) is always true
14385-
op = NewMorphedIntConNode(1);
14385+
op = gtWrapWithSideEffects(NewMorphedIntConNode(1), op, GTF_ALL_EFFECT);
1438614386
goto DONE_FOLD;
1438714387
}
1438814388
break;
14389+
}
1438914390

1439014391
case GT_LT:
14391-
if (tree->IsUnsigned() && (val == 0) && (op2 == cons) && !opHasSideEffects)
14392+
{
14393+
if (tree->IsUnsigned() && (val == 0) && (op2 == cons))
1439214394
{
1439314395
// unsigned (x < 0) is always false
14394-
op = NewMorphedIntConNode(0);
14396+
op = gtWrapWithSideEffects(NewMorphedIntConNode(0), op, GTF_ALL_EFFECT);
1439514397
goto DONE_FOLD;
1439614398
}
1439714399
break;
14400+
}
1439814401

1439914402
case GT_GT:
14400-
if (tree->IsUnsigned() && (val == 0) && (op1 == cons) && !opHasSideEffects)
14403+
{
14404+
if (tree->IsUnsigned() && (val == 0) && (op1 == cons))
1440114405
{
1440214406
// unsigned (0 > x) is always false
14403-
op = NewMorphedIntConNode(0);
14407+
op = gtWrapWithSideEffects(NewMorphedIntConNode(0), op, GTF_ALL_EFFECT);
1440414408
goto DONE_FOLD;
1440514409
}
14410+
}
1440614411
FALLTHROUGH;
14412+
1440714413
case GT_EQ:
1440814414
case GT_NE:
14409-
14415+
{
1441014416
// Optimize boxed value classes; these are always false. This IL is
1441114417
// generated when a generic value is tested against null:
1441214418
// <T> ... foo(T x) { ... if ((object)x == null) ...
@@ -14468,57 +14474,59 @@ GenTree* Compiler::gtFoldExprSpecial(GenTree* tree)
1446814474
{
1446914475
return gtFoldBoxNullable(tree);
1447014476
}
14471-
1447214477
break;
14478+
}
1447314479

1447414480
case GT_ADD:
14481+
{
1447514482
if (val == 0)
1447614483
{
1447714484
goto DONE_FOLD;
1447814485
}
1447914486
break;
14487+
}
1448014488

1448114489
case GT_MUL:
14490+
{
1448214491
if (val == 1)
1448314492
{
1448414493
goto DONE_FOLD;
1448514494
}
1448614495
else if (val == 0)
1448714496
{
14488-
/* Multiply by zero - return the 'zero' node, but not if side effects */
14489-
if (!opHasSideEffects)
14490-
{
14491-
op = cons;
14492-
goto DONE_FOLD;
14493-
}
14497+
// Multiply by zero - return the 'zero' node
14498+
op = gtWrapWithSideEffects(cons, op, GTF_ALL_EFFECT);
14499+
goto DONE_FOLD;
1449414500
}
1449514501
break;
14502+
}
1449614503

1449714504
case GT_DIV:
1449814505
case GT_UDIV:
14506+
{
1449914507
if ((op2 == cons) && (val == 1) && !op1->OperIsConst())
1450014508
{
1450114509
goto DONE_FOLD;
1450214510
}
1450314511
break;
14512+
}
1450414513

1450514514
case GT_SUB:
14515+
{
1450614516
if ((op2 == cons) && (val == 0) && !op1->OperIsConst())
1450714517
{
1450814518
goto DONE_FOLD;
1450914519
}
1451014520
break;
14521+
}
1451114522

1451214523
case GT_AND:
14524+
{
1451314525
if (val == 0)
1451414526
{
14515-
/* AND with zero - return the 'zero' node, but not if side effects */
14516-
14517-
if (!opHasSideEffects)
14518-
{
14519-
op = cons;
14520-
goto DONE_FOLD;
14521-
}
14527+
// AND with zero - return the 'zero' node
14528+
op = gtWrapWithSideEffects(cons, op, GTF_ALL_EFFECT);
14529+
goto DONE_FOLD;
1452214530
}
1452314531
else if (val == 0xFF)
1452414532
{
@@ -14551,8 +14559,10 @@ GenTree* Compiler::gtFoldExprSpecial(GenTree* tree)
1455114559
}
1455214560
}
1455314561
break;
14562+
}
1455414563

1455514564
case GT_OR:
14565+
{
1455614566
if (val == 0)
1455714567
{
1455814568
goto DONE_FOLD;
@@ -14563,34 +14573,33 @@ GenTree* Compiler::gtFoldExprSpecial(GenTree* tree)
1456314573

1456414574
assert(val == 1);
1456514575

14566-
/* OR with one - return the 'one' node, but not if side effects */
14567-
14568-
if (!opHasSideEffects)
14569-
{
14570-
op = cons;
14571-
goto DONE_FOLD;
14572-
}
14576+
// OR with one - return the 'one' node
14577+
op = gtWrapWithSideEffects(cons, op, GTF_ALL_EFFECT);
14578+
goto DONE_FOLD;
1457314579
}
1457414580
break;
14581+
}
1457514582

1457614583
case GT_LSH:
1457714584
case GT_RSH:
1457814585
case GT_RSZ:
1457914586
case GT_ROL:
1458014587
case GT_ROR:
14588+
{
1458114589
if (val == 0)
1458214590
{
1458314591
if (op2 == cons)
1458414592
{
1458514593
goto DONE_FOLD;
1458614594
}
14587-
else if (!opHasSideEffects)
14595+
else
1458814596
{
14589-
op = cons;
14597+
op = gtWrapWithSideEffects(cons, op, GTF_ALL_EFFECT);
1459014598
goto DONE_FOLD;
1459114599
}
1459214600
}
1459314601
break;
14602+
}
1459414603

1459514604
case GT_QMARK:
1459614605
{
@@ -14613,9 +14622,8 @@ GenTree* Compiler::gtFoldExprSpecial(GenTree* tree)
1461314622
{
1461414623
fgWalkTreePre(&op, gtClearColonCond);
1461514624
}
14616-
}
14617-
1461814625
goto DONE_FOLD;
14626+
}
1461914627

1462014628
default:
1462114629
break;
@@ -14632,6 +14640,13 @@ GenTree* Compiler::gtFoldExprSpecial(GenTree* tree)
1463214640
JITDUMP("Transformed into:\n");
1463314641
DISPTREE(op);
1463414642

14643+
if (fgGlobalMorph)
14644+
{
14645+
// We can sometimes produce a comma over the constant if the original op
14646+
// had a side effect, so just ensure we set the flag (which will be already
14647+
// set for the operands otherwise).
14648+
INDEBUG(op->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED);
14649+
}
1463514650
return op;
1463614651
}
1463714652

@@ -16903,9 +16918,19 @@ bool Compiler::gtNodeHasSideEffects(GenTree* tree, GenTreeFlags flags)
1690316918
// Are there only GTF_CALL side effects remaining? (and no other side effect kinds)
1690416919
if (flags & GTF_CALL)
1690516920
{
16906-
if (tree->OperGet() == GT_CALL)
16921+
GenTree* potentialCall = tree;
16922+
16923+
if (potentialCall->OperIs(GT_RET_EXPR))
16924+
{
16925+
// We need to preserve return expressions where the underlying call
16926+
// has side effects. Otherwise early folding can result in us dropping
16927+
// the call.
16928+
potentialCall = potentialCall->AsRetExpr()->gtInlineCandidate;
16929+
}
16930+
16931+
if (potentialCall->OperIs(GT_CALL))
1690716932
{
16908-
GenTreeCall* const call = tree->AsCall();
16933+
GenTreeCall* const call = potentialCall->AsCall();
1690916934
const bool ignoreExceptions = (flags & GTF_EXCEPT) == 0;
1691016935
const bool ignoreCctors = (flags & GTF_IS_IN_CSE) != 0; // We can CSE helpers that run cctors.
1691116936
if (!call->HasSideEffects(this, ignoreExceptions, ignoreCctors))

0 commit comments

Comments
 (0)