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.

Sign up for GitHub

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: Produce less convoluted IR for boolean isinst checks #103391

Merged
merged 6 commits into from
Jun 18, 2024
Merged

JIT: Produce less convoluted IR for boolean isinst checks #103391

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
14ac08d
JIT: Produce less convoluted IR for boolean `isinst` checks
jakobbotsch Jun 13, 2024
1ca8a25
Use two QMARKs like other case
jakobbotsch Jun 13, 2024
b313ec5
Run jit-format, add function headers
jakobbotsch Jun 13, 2024
0438a0b
Oops
jakobbotsch Jun 13, 2024
c69f319
Feedback
jakobbotsch Jun 17, 2024
cb72437
Dedup code a bit
jakobbotsch Jun 18, 2024
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: 3 additions & 1 deletion src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4774,8 +4774,10 @@ class Compiler
bool impIsCastHelperEligibleForClassProbe(GenTree* tree);
bool impIsCastHelperMayHaveProfileData(CorInfoHelpFunc helper);

bool impMatchIsInstBooleanConversion(const BYTE* codeAddr, const BYTE* codeEndp, int* consumed);

GenTree* impCastClassOrIsInstToTree(
GenTree* op1, GenTree* op2, CORINFO_RESOLVED_TOKEN* pResolvedToken, bool isCastClass, IL_OFFSET ilOffset);
GenTree* op1, GenTree* op2, CORINFO_RESOLVED_TOKEN* pResolvedToken, bool isCastClass, bool* booleanCheck, IL_OFFSET ilOffset);

GenTree* impOptimizeCastClassOrIsInst(GenTree* op1, CORINFO_RESOLVED_TOKEN* pResolvedToken, bool isCastClass);

Expand Down
86 changes: 71 additions & 15 deletions src/coreclr/jit/importer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5464,6 +5464,32 @@ GenTree* Compiler::impOptimizeCastClassOrIsInst(GenTree* op1, CORINFO_RESOLVED_T
return nullptr;
}

bool Compiler::impMatchIsInstBooleanConversion(const BYTE* codeAddr, const BYTE* codeEndp, int* consumed)
{
OPCODE nextOpcode = impGetNonPrefixOpcode(codeAddr, codeEndp);
switch (nextOpcode)
{
case CEE_BRFALSE:
case CEE_BRFALSE_S:
*consumed = 0;
return true;
case CEE_BRTRUE:
case CEE_BRTRUE_S:
*consumed = 0;
return true;
case CEE_LDNULL:
nextOpcode = impGetNonPrefixOpcode(codeAddr + 1, codeEndp);
if (nextOpcode == CEE_CGT_UN)
{
*consumed = 3;
return true;
}
return false;
default:
return false;
}
}

//------------------------------------------------------------------------
// impCastClassOrIsInstToTree: build and import castclass/isinst
//
Expand All @@ -5479,8 +5505,12 @@ GenTree* Compiler::impOptimizeCastClassOrIsInst(GenTree* op1, CORINFO_RESOLVED_T
// Notes:
// May expand into a series of runtime checks or a helper call.
//
GenTree* Compiler::impCastClassOrIsInstToTree(
GenTree* op1, GenTree* op2, CORINFO_RESOLVED_TOKEN* pResolvedToken, bool isCastClass, IL_OFFSET ilOffset)
GenTree* Compiler::impCastClassOrIsInstToTree(GenTree* op1,
GenTree* op2,
CORINFO_RESOLVED_TOKEN* pResolvedToken,
bool isCastClass,
bool* booleanCheck,
IL_OFFSET ilOffset)
{
assert(op1->TypeGet() == TYP_REF);

Expand Down Expand Up @@ -5553,6 +5583,8 @@ GenTree* Compiler::impCastClassOrIsInstToTree(
call->gtCallMoreFlags |= GTF_CALL_M_CAST_CAN_BE_EXPANDED;
call->gtCastHelperILOffset = ilOffset;
}

*booleanCheck = false;
return call;
}

Expand All @@ -5578,19 +5610,34 @@ GenTree* Compiler::impCastClassOrIsInstToTree(
GenTree* op1Clone;
op1 = impCloneExpr(op1, &op1Clone, CHECK_SPILL_ALL, nullptr DEBUGARG("ISINST eval op1"));

GenTreeOp* condMT = gtNewOperNode(GT_NE, TYP_INT, gtNewMethodTableLookup(op1Clone), op2);
GenTreeOp* condNull = gtNewOperNode(GT_EQ, TYP_INT, gtClone(op1), gtNewNull());
GenTreeQmark* qmarkMT = gtNewQmarkNode(TYP_REF, condMT, gtNewColonNode(TYP_REF, gtNewNull(), gtClone(op1)));
GenTreeQmark* qmarkNull = gtNewQmarkNode(TYP_REF, condNull, gtNewColonNode(TYP_REF, gtNewNull(), qmarkMT));
if (*booleanCheck)
{
GenTreeOp* condMT = gtNewOperNode(GT_EQ, TYP_INT, gtNewMethodTableLookup(op1Clone), op2);
GenTreeOp* condNull = gtNewOperNode(GT_EQ, TYP_INT, gtClone(op1), gtNewNull());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: looks like these two branches have a lot of common code, e.g. condMT and condNull are the same.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deduplicated a bit of it

GenTreeQmark* qmarkNull =
gtNewQmarkNode(TYP_INT, condNull, gtNewColonNode(TYP_INT, gtNewZeroConNode(TYP_INT), condMT));

// Make QMark node a top level node by spilling it.
const unsigned result = lvaGrabTemp(true DEBUGARG("spilling qmarkNull"));
impStoreToTemp(result, qmarkNull, CHECK_SPILL_NONE);
// Make QMark node a top level node by spilling it.
const unsigned result = lvaGrabTemp(true DEBUGARG("spilling qmarkNull"));
impStoreToTemp(result, qmarkNull, CHECK_SPILL_NONE);
return gtNewLclvNode(result, TYP_INT);
}
else
{
GenTreeOp* condMT = gtNewOperNode(GT_NE, TYP_INT, gtNewMethodTableLookup(op1Clone), op2);
GenTreeOp* condNull = gtNewOperNode(GT_EQ, TYP_INT, gtClone(op1), gtNewNull());
GenTreeQmark* qmarkMT = gtNewQmarkNode(TYP_REF, condMT, gtNewColonNode(TYP_REF, gtNewNull(), gtClone(op1)));
GenTreeQmark* qmarkNull = gtNewQmarkNode(TYP_REF, condNull, gtNewColonNode(TYP_REF, gtNewNull(), qmarkMT));

// Make QMark node a top level node by spilling it.
const unsigned result = lvaGrabTemp(true DEBUGARG("spilling qmarkNull"));
impStoreToTemp(result, qmarkNull, CHECK_SPILL_NONE);

// See also gtGetHelperCallClassHandle where we make the same
// determination for the helper call variants.
lvaSetClass(result, pResolvedToken->hClass);
return gtNewLclvNode(result, TYP_REF);
// See also gtGetHelperCallClassHandle where we make the same
// determination for the helper call variants.
lvaSetClass(result, pResolvedToken->hClass);
return gtNewLclvNode(result, TYP_REF);
}
}

#ifndef DEBUG
Expand Down Expand Up @@ -9626,7 +9673,15 @@ void Compiler::impImportBlockCode(BasicBlock* block)
if (!usingReadyToRunHelper)
#endif
{
op1 = impCastClassOrIsInstToTree(op1, op2, &resolvedToken, false, opcodeOffs);
bool reverse = false;
int consumed = 0;
bool booleanCheck = impMatchIsInstBooleanConversion(codeAddr + sz, codeEndp, &consumed);
op1 = impCastClassOrIsInstToTree(op1, op2, &resolvedToken, false, &booleanCheck, opcodeOffs);

if (booleanCheck)
{
sz += consumed;
}
}
if (compDonotInline())
{
Expand Down Expand Up @@ -10148,7 +10203,8 @@ void Compiler::impImportBlockCode(BasicBlock* block)
if (!usingReadyToRunHelper)
#endif
{
op1 = impCastClassOrIsInstToTree(op1, op2, &resolvedToken, true, opcodeOffs);
bool booleanCheck = false;
op1 = impCastClassOrIsInstToTree(op1, op2, &resolvedToken, true, &booleanCheck, opcodeOffs);
}
if (compDonotInline())
{
Expand Down
Loading