Skip to content

Commit 558a6dc

Browse files
authored
Track local var ranges for "interfering writes" LIR check (#64804)
For LIR we verify that we can really consider locals to be used at their user by having a checker that looks for interfering stores to the same locals. However, in some cases we may have "interfering" GT_LCL_FLD/GT_STORE_LCL_FLD, in the sense that they work on the same local but on a disjoint range of bytes. Add support to validate this. This fixes #57919 which made the fuzzer jobs very noisy and made it easy to miss actual new examples (e.g. #63720 was just merged even though there were real examples found there). Fix #57919
1 parent a74d7d1 commit 558a6dc

File tree

3 files changed

+71
-16
lines changed

3 files changed

+71
-16
lines changed

src/coreclr/jit/lir.cpp

Lines changed: 56 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1338,7 +1338,10 @@ class CheckLclVarSemanticsHelper
13381338
CheckLclVarSemanticsHelper(Compiler* compiler,
13391339
const LIR::Range* range,
13401340
SmallHashTable<GenTree*, bool, 32U>& unusedDefs)
1341-
: compiler(compiler), range(range), unusedDefs(unusedDefs), unusedLclVarReads(compiler->getAllocator())
1341+
: compiler(compiler)
1342+
, range(range)
1343+
, unusedDefs(unusedDefs)
1344+
, unusedLclVarReads(compiler->getAllocator(CMK_DebugOnly))
13421345
{
13431346
}
13441347

@@ -1358,13 +1361,45 @@ class CheckLclVarSemanticsHelper
13581361
AliasSet::NodeInfo nodeInfo(compiler, node);
13591362
if (nodeInfo.IsLclVarRead() && !unusedDefs.Contains(node))
13601363
{
1361-
int count = 0;
1362-
unusedLclVarReads.TryGetValue(nodeInfo.LclNum(), &count);
1363-
unusedLclVarReads.AddOrUpdate(nodeInfo.LclNum(), count + 1);
1364+
jitstd::list<GenTree*>* reads;
1365+
if (!unusedLclVarReads.TryGetValue(nodeInfo.LclNum(), &reads))
1366+
{
1367+
reads = new (compiler, CMK_DebugOnly) jitstd::list<GenTree*>(compiler->getAllocator(CMK_DebugOnly));
1368+
unusedLclVarReads.AddOrUpdate(nodeInfo.LclNum(), reads);
1369+
}
1370+
1371+
reads->push_back(node);
13641372
}
13651373

1366-
// If this node is a lclVar write, it must be to a lclVar that does not have an outstanding read.
1367-
assert(!nodeInfo.IsLclVarWrite() || !unusedLclVarReads.Contains(nodeInfo.LclNum()));
1374+
if (nodeInfo.IsLclVarWrite())
1375+
{
1376+
// If this node is a lclVar write, it must be not alias a lclVar with an outstanding read
1377+
jitstd::list<GenTree*>* reads;
1378+
if (unusedLclVarReads.TryGetValue(nodeInfo.LclNum(), &reads))
1379+
{
1380+
for (GenTree* read : *reads)
1381+
{
1382+
AliasSet::NodeInfo readInfo(compiler, read);
1383+
assert(readInfo.IsLclVarRead() && readInfo.LclNum() == nodeInfo.LclNum());
1384+
unsigned readStart = readInfo.LclOffs();
1385+
unsigned readEnd = readStart + genTypeSize(read->TypeGet());
1386+
unsigned writeStart = nodeInfo.LclOffs();
1387+
unsigned writeEnd = writeStart + genTypeSize(node->TypeGet());
1388+
if ((readEnd > writeStart) && (writeEnd > readStart))
1389+
{
1390+
JITDUMP(
1391+
"Write to unaliased local overlaps outstanding read (write: %u..%u, read: %u..%u)\n",
1392+
writeStart, writeEnd, readStart, readEnd);
1393+
JITDUMP("Read:\n");
1394+
DISPTREERANGE(const_cast<LIR::Range&>(*range), read);
1395+
JITDUMP("Write:\n");
1396+
DISPTREERANGE(const_cast<LIR::Range&>(*range), node);
1397+
assert(!"Write to unaliased local overlaps outstanding read");
1398+
break;
1399+
}
1400+
}
1401+
}
1402+
}
13681403
}
13691404

13701405
return true;
@@ -1393,23 +1428,31 @@ class CheckLclVarSemanticsHelper
13931428
AliasSet::NodeInfo operandInfo(compiler, operand);
13941429
if (operandInfo.IsLclVarRead())
13951430
{
1396-
int count;
1397-
const bool removed = unusedLclVarReads.TryRemove(operandInfo.LclNum(), &count);
1398-
assert(removed);
1431+
jitstd::list<GenTree*>* reads;
1432+
const bool foundList = unusedLclVarReads.TryGetValue(operandInfo.LclNum(), &reads);
1433+
assert(foundList);
13991434

1400-
if (count > 1)
1435+
bool found = false;
1436+
for (jitstd::list<GenTree*>::iterator it = reads->begin(); it != reads->end(); ++it)
14011437
{
1402-
unusedLclVarReads.AddOrUpdate(operandInfo.LclNum(), count - 1);
1438+
if (*it == operand)
1439+
{
1440+
reads->erase(it);
1441+
found = true;
1442+
break;
1443+
}
14031444
}
1445+
1446+
assert(found || !"Could not find consumed local in unusedLclVarReads");
14041447
}
14051448
}
14061449
}
14071450

14081451
private:
14091452
Compiler* compiler;
14101453
const LIR::Range* range;
1411-
SmallHashTable<GenTree*, bool, 32U>& unusedDefs;
1412-
SmallHashTable<int, int, 32U> unusedLclVarReads;
1454+
SmallHashTable<GenTree*, bool, 32U>& unusedDefs;
1455+
SmallHashTable<int, jitstd::list<GenTree*>*, 16U> unusedLclVarReads;
14131456
};
14141457

14151458
//------------------------------------------------------------------------

src/coreclr/jit/sideeffects.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ AliasSet::AliasSet()
136136
// node - The node in question.
137137
//
138138
AliasSet::NodeInfo::NodeInfo(Compiler* compiler, GenTree* node)
139-
: m_compiler(compiler), m_node(node), m_flags(0), m_lclNum(0)
139+
: m_compiler(compiler), m_node(node), m_flags(0), m_lclNum(0), m_lclOffs(0)
140140
{
141141
if (node->IsCall())
142142
{
@@ -174,6 +174,7 @@ AliasSet::NodeInfo::NodeInfo(Compiler* compiler, GenTree* node)
174174
bool isMemoryAccess = false;
175175
bool isLclVarAccess = false;
176176
unsigned lclNum = 0;
177+
unsigned lclOffs = 0;
177178
if (node->OperIsIndir())
178179
{
179180
// If the indirection targets a lclVar, we can be more precise with regards to aliasing by treating the
@@ -183,6 +184,7 @@ AliasSet::NodeInfo::NodeInfo(Compiler* compiler, GenTree* node)
183184
{
184185
isLclVarAccess = true;
185186
lclNum = address->AsLclVarCommon()->GetLclNum();
187+
lclOffs = address->AsLclVarCommon()->GetLclOffs();
186188
}
187189
else
188190
{
@@ -197,6 +199,7 @@ AliasSet::NodeInfo::NodeInfo(Compiler* compiler, GenTree* node)
197199
{
198200
isLclVarAccess = true;
199201
lclNum = node->AsLclVarCommon()->GetLclNum();
202+
lclOffs = node->AsLclVarCommon()->GetLclOffs();
200203
}
201204
else
202205
{
@@ -221,7 +224,8 @@ AliasSet::NodeInfo::NodeInfo(Compiler* compiler, GenTree* node)
221224
if (isLclVarAccess)
222225
{
223226
m_flags |= ALIAS_READS_LCL_VAR;
224-
m_lclNum = lclNum;
227+
m_lclNum = lclNum;
228+
m_lclOffs = lclOffs;
225229
}
226230
}
227231
else
@@ -234,7 +238,8 @@ AliasSet::NodeInfo::NodeInfo(Compiler* compiler, GenTree* node)
234238
if (isLclVarAccess)
235239
{
236240
m_flags |= ALIAS_WRITES_LCL_VAR;
237-
m_lclNum = lclNum;
241+
m_lclNum = lclNum;
242+
m_lclOffs = lclOffs;
238243
}
239244
}
240245
}

src/coreclr/jit/sideeffects.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ class AliasSet final
7272
GenTree* m_node;
7373
unsigned m_flags;
7474
unsigned m_lclNum;
75+
unsigned m_lclOffs;
7576

7677
public:
7778
NodeInfo(Compiler* compiler, GenTree* node);
@@ -112,6 +113,12 @@ class AliasSet final
112113
return m_lclNum;
113114
}
114115

116+
inline unsigned LclOffs() const
117+
{
118+
assert(IsLclVarRead() || IsLclVarWrite());
119+
return m_lclOffs;
120+
}
121+
115122
inline bool WritesAnyLocation() const
116123
{
117124
return (m_flags & (ALIAS_WRITES_ADDRESSABLE_LOCATION | ALIAS_WRITES_LCL_VAR)) != 0;

0 commit comments

Comments
 (0)