Skip to content

Commit c63988c

Browse files
Fix the same problem in local propagation
Just one diff, missing null check elimination: we propagated a field sequence for an array address (PtrToArrElem) that later wasn't recognized as implying non-nullness of another PtrToArrElem to the same array.
1 parent fd76f2c commit c63988c

File tree

2 files changed

+61
-19
lines changed

2 files changed

+61
-19
lines changed

src/coreclr/jit/assertionprop.cpp

Lines changed: 52 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1062,6 +1062,11 @@ void Compiler::optPrintAssertion(AssertionDsc* curAssertion, AssertionIndex asse
10621062
{
10631063
printf(".%02u", curAssertion->op1.lcl.ssaNum);
10641064
}
1065+
if (curAssertion->op2.zeroOffsetFieldSeq != nullptr)
1066+
{
1067+
printf(" Zero");
1068+
gtDispFieldSeq(curAssertion->op2.zeroOffsetFieldSeq);
1069+
}
10651070
break;
10661071

10671072
case O2K_CONST_INT:
@@ -1582,10 +1587,14 @@ AssertionIndex Compiler::optCreateAssertion(GenTree* op1,
15821587
goto DONE_ASSERTION; // Don't make an assertion
15831588
}
15841589

1585-
assertion.op2.kind = O2K_LCLVAR_COPY;
1586-
assertion.op2.lcl.lclNum = lclNum2;
1587-
assertion.op2.vn = vnStore->VNConservativeNormalValue(op2->gtVNPair);
1588-
assertion.op2.lcl.ssaNum = op2->AsLclVarCommon()->GetSsaNum();
1590+
FieldSeqNode* zeroOffsetFieldSeq = nullptr;
1591+
GetZeroOffsetFieldMap()->Lookup(op2, &zeroOffsetFieldSeq);
1592+
1593+
assertion.op2.kind = O2K_LCLVAR_COPY;
1594+
assertion.op2.vn = vnStore->VNConservativeNormalValue(op2->gtVNPair);
1595+
assertion.op2.lcl.lclNum = lclNum2;
1596+
assertion.op2.lcl.ssaNum = op2->AsLclVarCommon()->GetSsaNum();
1597+
assertion.op2.zeroOffsetFieldSeq = zeroOffsetFieldSeq;
15891598

15901599
// Ok everything has been set and the assertion looks good
15911600
assertion.assertionKind = assertionKind;
@@ -3316,14 +3325,30 @@ GenTree* Compiler::optCopyAssertionProp(AssertionDsc* curAssertion,
33163325
return nullptr;
33173326
}
33183327

3319-
// Extract the matching lclNum and ssaNum.
3320-
const unsigned copyLclNum = (op1.lcl.lclNum == lclNum) ? op2.lcl.lclNum : op1.lcl.lclNum;
3321-
unsigned copySsaNum = SsaConfig::RESERVED_SSA_NUM;
3328+
// Extract the matching lclNum and ssaNum, as well as the field sequence.
3329+
unsigned copyLclNum;
3330+
unsigned copySsaNum;
3331+
FieldSeqNode* zeroOffsetFieldSeq;
3332+
if (op1.lcl.lclNum == lclNum)
3333+
{
3334+
copyLclNum = op2.lcl.lclNum;
3335+
copySsaNum = op2.lcl.ssaNum;
3336+
zeroOffsetFieldSeq = op2.zeroOffsetFieldSeq;
3337+
}
3338+
else
3339+
{
3340+
copyLclNum = op1.lcl.lclNum;
3341+
copySsaNum = op1.lcl.ssaNum;
3342+
zeroOffsetFieldSeq = nullptr; // Only the RHS of an assignment can have a FldSeq.
3343+
assert(optLocalAssertionProp); // Were we to perform replacements in global propagation, that makes copy
3344+
// assertions for control flow ("if (a == b) { ... }"), where both operands
3345+
// could have a FldSeq, we'd need to save it for "op1" too.
3346+
}
3347+
33223348
if (!optLocalAssertionProp)
33233349
{
33243350
// Extract the ssaNum of the matching lclNum.
33253351
unsigned ssaNum = (op1.lcl.lclNum == lclNum) ? op1.lcl.ssaNum : op2.lcl.ssaNum;
3326-
copySsaNum = (op1.lcl.lclNum == lclNum) ? op2.lcl.ssaNum : op1.lcl.ssaNum;
33273352

33283353
if (ssaNum != tree->GetSsaNum())
33293354
{
@@ -3349,12 +3374,25 @@ GenTree* Compiler::optCopyAssertionProp(AssertionDsc* curAssertion,
33493374
tree->SetLclNum(copyLclNum);
33503375
tree->SetSsaNum(copySsaNum);
33513376

3377+
// The sequence we are propagating (if any) represents the inner fields.
3378+
if (zeroOffsetFieldSeq != nullptr)
3379+
{
3380+
FieldSeqNode* outerZeroOffsetFieldSeq = nullptr;
3381+
if (GetZeroOffsetFieldMap()->Lookup(tree, &outerZeroOffsetFieldSeq))
3382+
{
3383+
zeroOffsetFieldSeq = GetFieldSeqStore()->Append(zeroOffsetFieldSeq, outerZeroOffsetFieldSeq);
3384+
GetZeroOffsetFieldMap()->Remove(tree);
3385+
}
3386+
3387+
fgAddFieldSeqForZeroOffset(tree, zeroOffsetFieldSeq);
3388+
}
3389+
33523390
#ifdef DEBUG
33533391
if (verbose)
33543392
{
33553393
printf("\nAssertion prop in " FMT_BB ":\n", compCurBB->bbNum);
33563394
optPrintAssertion(curAssertion, index);
3357-
gtDispTree(tree, nullptr, nullptr, true);
3395+
DISPNODE(tree);
33583396
}
33593397
#endif
33603398

@@ -4665,15 +4703,15 @@ GenTree* Compiler::optAssertionProp(ASSERT_VALARG_TP assertions, GenTree* tree,
46654703
}
46664704

46674705
//------------------------------------------------------------------------
4668-
// optImpliedAssertions: Given a tree node that makes an assertion this
4669-
// method computes the set of implied assertions
4670-
// that are also true. The updated assertions are
4671-
// maintained on the Compiler object.
4706+
// optImpliedAssertions: Given an assertion this method computes the set
4707+
// of implied assertions that are also true.
46724708
//
46734709
// Arguments:
46744710
// assertionIndex : The id of the assertion.
46754711
// activeAssertions : The assertions that are already true at this point.
4676-
4712+
// This method will add the discovered implied assertions
4713+
// to this set.
4714+
//
46774715
void Compiler::optImpliedAssertions(AssertionIndex assertionIndex, ASSERT_TP& activeAssertions)
46784716
{
46794717
noway_assert(!optLocalAssertionProp);
@@ -4822,7 +4860,6 @@ void Compiler::optImpliedByTypeOfAssertions(ASSERT_TP& activeAssertions)
48224860
// Return Value:
48234861
// The assertions we have about the value number.
48244862
//
4825-
48264863
ASSERT_VALRET_TP Compiler::optGetVnMappedAssertions(ValueNum vn)
48274864
{
48284865
ASSERT_TP set = BitVecOps::UninitVal();

src/coreclr/jit/compiler.h

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7626,7 +7626,6 @@ class Compiler
76267626
O2K_CONST_INT,
76277627
O2K_CONST_LONG,
76287628
O2K_CONST_DOUBLE,
7629-
O2K_ARR_LEN,
76307629
O2K_SUBRANGE,
76317630
O2K_COUNT
76327631
};
@@ -7666,7 +7665,11 @@ class Compiler
76667665
GenTreeFlags iconFlags; // gtFlags
76677666
};
76687667
union {
7669-
SsaVar lcl;
7668+
struct
7669+
{
7670+
SsaVar lcl;
7671+
FieldSeqNode* zeroOffsetFieldSeq;
7672+
};
76707673
IntVal u1;
76717674
__int64 lconVal;
76727675
double dconVal;
@@ -7745,6 +7748,7 @@ class Compiler
77457748
{
77467749
return false;
77477750
}
7751+
77487752
switch (op2.kind)
77497753
{
77507754
case O2K_IND_CNS_INT:
@@ -7759,9 +7763,9 @@ class Compiler
77597763
return (memcmp(&op2.dconVal, &that->op2.dconVal, sizeof(double)) == 0);
77607764

77617765
case O2K_LCLVAR_COPY:
7762-
case O2K_ARR_LEN:
77637766
return (op2.lcl.lclNum == that->op2.lcl.lclNum) &&
7764-
(!vnBased || op2.lcl.ssaNum == that->op2.lcl.ssaNum);
7767+
(!vnBased || op2.lcl.ssaNum == that->op2.lcl.ssaNum) &&
7768+
(op2.zeroOffsetFieldSeq == that->op2.zeroOffsetFieldSeq);
77657769

77667770
case O2K_SUBRANGE:
77677771
return op2.u2.Equals(that->op2.u2);
@@ -7774,6 +7778,7 @@ class Compiler
77747778
assert(!"Unexpected value for op2.kind in AssertionDsc.");
77757779
break;
77767780
}
7781+
77777782
return false;
77787783
}
77797784

0 commit comments

Comments
 (0)