Skip to content

Commit

Permalink
JIT: Fold null checks against initialized static readonly fields (#50000
Browse files Browse the repository at this point in the history
)

Co-authored-by: Andy Ayers <andya@microsoft.com>
Co-authored-by: Sergey Andreenko <seandree@microsoft.com>
  • Loading branch information
3 people authored Mar 23, 2021
1 parent d068d95 commit 65814eb
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 7 deletions.
4 changes: 4 additions & 0 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9069,6 +9069,10 @@ void cTreeFlags(Compiler* comp, GenTree* tree)
{
chars += printf("[IND_INVARIANT]");
}
if (tree->gtFlags & GTF_IND_NONNULL)
{
chars += printf("[IND_NONNULL]");
}
break;

case GT_CLS_VAR:
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/fgdiagnostic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2608,6 +2608,7 @@ void Compiler::fgDebugCheckDispFlags(GenTree* tree, unsigned dispFlags, unsigned
{
printf("%c", (dispFlags & GTF_IND_INVARIANT) ? '#' : '-');
printf("%c", (dispFlags & GTF_IND_NONFAULTING) ? 'n' : '-');
printf("%c", (dispFlags & GTF_IND_NONNULL) ? '@' : '-');
}
GenTree::gtDispFlags(dispFlags, debugFlags);
}
Expand Down
12 changes: 12 additions & 0 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6107,6 +6107,12 @@ GenTree* Compiler::gtNewIndOfIconHandleNode(var_types indType, size_t addr, unsi

// This indirection also is invariant.
indNode->gtFlags |= GTF_IND_INVARIANT;

if (iconFlags == GTF_ICON_STR_HDL)
{
// String literals are never null
indNode->gtFlags |= GTF_IND_NONNULL;
}
}
return indNode;
}
Expand Down Expand Up @@ -10252,6 +10258,12 @@ void Compiler::gtDispNode(GenTree* tree, IndentStack* indentStack, __in __in_z _
--msgLength;
break;
}
if (tree->gtFlags & GTF_IND_NONNULL)
{
printf("@");
--msgLength;
break;
}
}
FALLTHROUGH;

Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -866,10 +866,11 @@ struct GenTree
// alignment of 1 byte)
#define GTF_IND_INVARIANT 0x01000000 // GT_IND -- the target is invariant (a prejit indirection)
#define GTF_IND_ARR_INDEX 0x00800000 // GT_IND -- the indirection represents an (SZ) array index
#define GTF_IND_NONNULL 0x00400000 // GT_IND -- the indirection never returns null (zero)

#define GTF_IND_FLAGS \
(GTF_IND_VOLATILE | GTF_IND_TGTANYWHERE | GTF_IND_NONFAULTING | GTF_IND_TLS_REF | \
GTF_IND_UNALIGNED | GTF_IND_INVARIANT | GTF_IND_ARR_INDEX | GTF_IND_TGT_NOT_HEAP)
GTF_IND_UNALIGNED | GTF_IND_INVARIANT | GTF_IND_NONNULL | GTF_IND_ARR_INDEX | GTF_IND_TGT_NOT_HEAP)

#define GTF_CLS_VAR_VOLATILE 0x40000000 // GT_FIELD/GT_CLS_VAR -- same as GTF_IND_VOLATILE
#define GTF_CLS_VAR_INITCLASS 0x20000000 // GT_FIELD/GT_CLS_VAR -- same as GTF_FLD_INITCLASS
Expand Down
4 changes: 3 additions & 1 deletion src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6520,7 +6520,9 @@ GenTree* Compiler::fgMorphField(GenTree* tree, MorphAddrContext* mac)
if (isStaticReadOnlyInited)
{
JITDUMP("Marking initialized static read-only field '%s' as invariant.\n", eeGetFieldName(symHnd));
tree->gtFlags |= GTF_IND_INVARIANT;

// Static readonly field is not null at this point (see getStaticFieldCurrentClass impl).
tree->gtFlags |= (GTF_IND_INVARIANT | GTF_IND_NONFAULTING | GTF_IND_NONNULL);
tree->gtFlags &= ~GTF_ICON_INITCLASS;
addr->gtFlags = GTF_ICON_CONST_PTR;
}
Expand Down
17 changes: 13 additions & 4 deletions src/coreclr/jit/valuenum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8139,11 +8139,20 @@ void Compiler::fgValueNumberTree(GenTree* tree)
{
assert(!isVolatile); // We don't expect both volatile and invariant

// Is it a string literal? (it's always non-null)
if (addr->IsCnsIntOrI() && addr->IsIconHandle(GTF_ICON_STR_HDL))
// Is this invariant indirect expected to always return a non-null value?
if ((tree->gtFlags & GTF_IND_NONNULL) != 0)
{
tree->gtVNPair = vnStore->VNPairForFunc(tree->TypeGet(), VNF_StrCns, addrNvnp);
assert(addrXvnp.BothEqual() && (addrXvnp.GetLiberal() == ValueNumStore::VNForEmptyExcSet()));
assert(tree->gtFlags & GTF_IND_NONFAULTING);
tree->gtVNPair = vnStore->VNPairForFunc(tree->TypeGet(), VNF_NonNullIndirect, addrNvnp);
if (addr->IsCnsIntOrI())
{
assert(addrXvnp.BothEqual() && (addrXvnp.GetLiberal() == ValueNumStore::VNForEmptyExcSet()));
}
else
{
assert(false && "it's not expected to be hit at the moment, but can be allowed.");
// tree->gtVNPair = vnStore->VNPWithExc(tree->gtVNPair, addrXvnp);
}
}
else
{
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/valuenumfuncs.h
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ ValueNumFuncDef(Box, 3, false, false, false)
ValueNumFuncDef(BoxNullable, 3, false, false, false)

ValueNumFuncDef(LazyStrCns, 2, false, true, false) // lazy-initialized string literal (helper)
ValueNumFuncDef(StrCns, 1, false, true, false) // indirect for a string literal
ValueNumFuncDef(NonNullIndirect, 1, false, true, false) // this indirect is expected to always return a non-null value
ValueNumFuncDef(Unbox, 2, false, true, false)

ValueNumFuncDef(LT_UN, 2, false, false, false) // unsigned or unordered comparisons
Expand Down

0 comments on commit 65814eb

Please sign in to comment.