Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JIT: Fold null checks against initialized static readonly fields #50000

Merged
merged 7 commits into from
Mar 23, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9079,6 +9079,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 0x00200000 // GT_IND -- the indirection never returns null (zero)
EgorBo marked this conversation as resolved.
Show resolved Hide resolved

#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
16 changes: 12 additions & 4 deletions src/coreclr/jit/valuenum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8139,11 +8139,19 @@ 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))
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
{
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()));
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
}
else
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
{
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) // indirect for a string literal
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
ValueNumFuncDef(Unbox, 2, false, true, false)

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