Skip to content

Commit

Permalink
JIT: remove early prop's type propagation (#45655)
Browse files Browse the repository at this point in the history
This doesn't do any actual propagation, so remove it and the associated
tracking flags.
  • Loading branch information
AndyAyersMS authored Dec 9, 2020
1 parent d5a9b9d commit e516cf3
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 76 deletions.
41 changes: 20 additions & 21 deletions src/coreclr/jit/block.h
Original file line number Diff line number Diff line change
Expand Up @@ -412,14 +412,13 @@ struct BasicBlock : private LIR::Range
// cases, because the BB occurs in a loop, and we've determined that all
// paths in the loop body leading to BB include a call.

#define BBF_HAS_VTABREF MAKE_BBFLAG(20) // BB contains reference of vtable
#define BBF_HAS_IDX_LEN MAKE_BBFLAG(21) // BB contains simple index or length expressions on an array local var.
#define BBF_HAS_NEWARRAY MAKE_BBFLAG(22) // BB contains 'new' of an array
#define BBF_HAS_NEWOBJ MAKE_BBFLAG(23) // BB contains 'new' of an object type.
#define BBF_HAS_IDX_LEN MAKE_BBFLAG(20) // BB contains simple index or length expressions on an array local var.
#define BBF_HAS_NEWARRAY MAKE_BBFLAG(21) // BB contains 'new' of an array
#define BBF_HAS_NEWOBJ MAKE_BBFLAG(22) // BB contains 'new' of an object type.

#if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM)

#define BBF_FINALLY_TARGET MAKE_BBFLAG(24) // BB is the target of a finally return: where a finally will return during
#define BBF_FINALLY_TARGET MAKE_BBFLAG(23) // BB is the target of a finally return: where a finally will return during
// non-exceptional flow. Because the ARM calling sequence for calling a
// finally explicitly sets the return address to the finally target and jumps
// to the finally, instead of using a call instruction, ARM needs this to
Expand All @@ -428,27 +427,27 @@ struct BasicBlock : private LIR::Range

#endif // defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM)

#define BBF_BACKWARD_JUMP MAKE_BBFLAG(25) // BB is surrounded by a backward jump/switch arc
#define BBF_RETLESS_CALL MAKE_BBFLAG(26) // BBJ_CALLFINALLY that will never return (and therefore, won't need a paired
#define BBF_BACKWARD_JUMP MAKE_BBFLAG(24) // BB is surrounded by a backward jump/switch arc
#define BBF_RETLESS_CALL MAKE_BBFLAG(25) // BBJ_CALLFINALLY that will never return (and therefore, won't need a paired
// BBJ_ALWAYS); see isBBCallAlwaysPair().
#define BBF_LOOP_PREHEADER MAKE_BBFLAG(27) // BB is a loop preheader block
#define BBF_LOOP_PREHEADER MAKE_BBFLAG(26) // BB is a loop preheader block
#define BBF_COLD MAKE_BBFLAG(27) // BB is cold

#define BBF_COLD MAKE_BBFLAG(28) // BB is cold
#define BBF_PROF_WEIGHT MAKE_BBFLAG(29) // BB weight is computed from profile data
#define BBF_IS_LIR MAKE_BBFLAG(30) // Set if the basic block contains LIR (as opposed to HIR)
#define BBF_KEEP_BBJ_ALWAYS MAKE_BBFLAG(31) // A special BBJ_ALWAYS block, used by EH code generation. Keep the jump kind
#define BBF_PROF_WEIGHT MAKE_BBFLAG(28) // BB weight is computed from profile data
#define BBF_IS_LIR MAKE_BBFLAG(29) // Set if the basic block contains LIR (as opposed to HIR)
#define BBF_KEEP_BBJ_ALWAYS MAKE_BBFLAG(30) // A special BBJ_ALWAYS block, used by EH code generation. Keep the jump kind
// as BBJ_ALWAYS. Used for the paired BBJ_ALWAYS block following the
// BBJ_CALLFINALLY block, as well as, on x86, the final step block out of a
// finally.
#define BBF_CLONED_FINALLY_BEGIN MAKE_BBFLAG(31) // First block of a cloned finally region

#define BBF_CLONED_FINALLY_BEGIN MAKE_BBFLAG(32) // First block of a cloned finally region
#define BBF_CLONED_FINALLY_END MAKE_BBFLAG(33) // Last block of a cloned finally region
#define BBF_HAS_CALL MAKE_BBFLAG(34) // BB contains a call
#define BBF_DOMINATED_BY_EXCEPTIONAL_ENTRY MAKE_BBFLAG(35) // Block is dominated by exceptional entry.
#define BBF_CLONED_FINALLY_END MAKE_BBFLAG(32) // Last block of a cloned finally region
#define BBF_HAS_CALL MAKE_BBFLAG(33) // BB contains a call
#define BBF_DOMINATED_BY_EXCEPTIONAL_ENTRY MAKE_BBFLAG(34) // Block is dominated by exceptional entry.
#define BBF_BACKWARD_JUMP_TARGET MAKE_BBFLAG(35) // Block is a target of a backward jump

#define BBF_BACKWARD_JUMP_TARGET MAKE_BBFLAG(36) // Block is a target of a backward jump
#define BBF_PATCHPOINT MAKE_BBFLAG(37) // Block is a patchpoint
#define BBF_HAS_CLASS_PROFILE MAKE_BBFLAG(38) // BB contains a call needing a class profile
#define BBF_PATCHPOINT MAKE_BBFLAG(36) // Block is a patchpoint
#define BBF_HAS_CLASS_PROFILE MAKE_BBFLAG(37) // BB contains a call needing a class profile

// clang-format on

Expand All @@ -469,7 +468,7 @@ struct BasicBlock : private LIR::Range

#define BBF_COMPACT_UPD \
(BBF_CHANGED | BBF_GC_SAFE_POINT | BBF_HAS_JMP | BBF_HAS_IDX_LEN | BBF_BACKWARD_JUMP | BBF_HAS_NEWARRAY | \
BBF_HAS_NEWOBJ | BBF_HAS_NULLCHECK | BBF_HAS_VTABREF)
BBF_HAS_NEWOBJ | BBF_HAS_NULLCHECK)

// Flags a block should not have had before it is split.

Expand All @@ -493,7 +492,7 @@ struct BasicBlock : private LIR::Range
#define BBF_SPLIT_GAINED \
(BBF_DONT_REMOVE | BBF_HAS_LABEL | BBF_HAS_JMP | BBF_BACKWARD_JUMP | BBF_HAS_IDX_LEN | BBF_HAS_NEWARRAY | \
BBF_PROF_WEIGHT | BBF_HAS_NEWOBJ | BBF_KEEP_BBJ_ALWAYS | BBF_CLONED_FINALLY_END | BBF_HAS_NULLCHECK | \
BBF_HAS_VTABREF | BBF_HAS_CLASS_PROFILE)
BBF_HAS_CLASS_PROFILE)

#ifndef __GNUC__ // GCC doesn't like C_ASSERT at global scope
static_assert_no_msg((BBF_SPLIT_NONEXIST & BBF_SPLIT_LOST) == 0);
Expand Down
18 changes: 8 additions & 10 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -6705,15 +6705,14 @@ class Compiler
#define OMF_HAS_NEWARRAY 0x00000001 // Method contains 'new' of an array
#define OMF_HAS_NEWOBJ 0x00000002 // Method contains 'new' of an object type.
#define OMF_HAS_ARRAYREF 0x00000004 // Method contains array element loads or stores.
#define OMF_HAS_VTABLEREF 0x00000008 // Method contains method table reference.
#define OMF_HAS_NULLCHECK 0x00000010 // Method contains null check.
#define OMF_HAS_FATPOINTER 0x00000020 // Method contains call, that needs fat pointer transformation.
#define OMF_HAS_OBJSTACKALLOC 0x00000040 // Method contains an object allocated on the stack.
#define OMF_HAS_GUARDEDDEVIRT 0x00000080 // Method contains guarded devirtualization candidate
#define OMF_HAS_EXPRUNTIMELOOKUP 0x00000100 // Method contains a runtime lookup to an expandable dictionary.
#define OMF_HAS_PATCHPOINT 0x00000200 // Method contains patchpoints
#define OMF_NEEDS_GCPOLLS 0x00000400 // Method needs GC polls
#define OMF_HAS_FROZEN_STRING 0x00000800 // Method has a frozen string (REF constant int), currently only on CoreRT.
#define OMF_HAS_NULLCHECK 0x00000008 // Method contains null check.
#define OMF_HAS_FATPOINTER 0x00000010 // Method contains call, that needs fat pointer transformation.
#define OMF_HAS_OBJSTACKALLOC 0x00000020 // Method contains an object allocated on the stack.
#define OMF_HAS_GUARDEDDEVIRT 0x00000040 // Method contains guarded devirtualization candidate
#define OMF_HAS_EXPRUNTIMELOOKUP 0x00000080 // Method contains a runtime lookup to an expandable dictionary.
#define OMF_HAS_PATCHPOINT 0x00000100 // Method contains patchpoints
#define OMF_NEEDS_GCPOLLS 0x00000200 // Method needs GC polls
#define OMF_HAS_FROZEN_STRING 0x00000400 // Method has a frozen string (REF constant int), currently only on CoreRT.

bool doesMethodHaveFatPointer()
{
Expand Down Expand Up @@ -6813,7 +6812,6 @@ class Compiler
{
OPK_INVALID,
OPK_ARRAYLEN,
OPK_OBJ_GETTYPE,
OPK_NULLCHECK
};

Expand Down
44 changes: 5 additions & 39 deletions src/coreclr/jit/earlyprop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
// Early Value Propagation
//
// This phase performs an SSA-based value propagation optimization that currently only applies to array
// lengths, runtime type handles, and explicit null checks. An SSA-based backwards tracking of local variables
// lengths and explicit null checks. An SSA-based backwards tracking of local variables
// is performed at each point of interest, e.g., an array length reference site, a method table reference site, or
// an indirection.
// The tracking continues until an interesting value is encountered. The value is then used to rewrite
Expand All @@ -18,17 +18,15 @@
bool Compiler::optDoEarlyPropForFunc()
{
bool propArrayLen = (optMethodFlags & OMF_HAS_NEWARRAY) && (optMethodFlags & OMF_HAS_ARRAYREF);
bool propGetType = (optMethodFlags & (OMF_HAS_NEWOBJ | OMF_HAS_NEWARRAY)) && (optMethodFlags & OMF_HAS_VTABLEREF);
bool propNullCheck = (optMethodFlags & OMF_HAS_NULLCHECK) != 0;
return propArrayLen || propGetType || propNullCheck;
return propArrayLen || propNullCheck;
}

bool Compiler::optDoEarlyPropForBlock(BasicBlock* block)
{
bool bbHasArrayRef = (block->bbFlags & BBF_HAS_IDX_LEN) != 0;
bool bbHasVtableRef = (block->bbFlags & BBF_HAS_VTABREF) != 0;
bool bbHasNullCheck = (block->bbFlags & BBF_HAS_NULLCHECK) != 0;
return bbHasArrayRef || bbHasVtableRef || bbHasNullCheck;
return bbHasArrayRef || bbHasNullCheck;
}

//--------------------------------------------------------------------
Expand Down Expand Up @@ -321,26 +319,7 @@ GenTree* Compiler::optEarlyPropRewriteTree(GenTree* tree, LocalNumberToNullCheck
}
else
{
assert(tree->OperIsIndir());
if (gtIsVtableRef(tree))
{
// Don't propagate type handles that are used as null checks, which are usually in
// form of
// * stmtExpr void (top level)
// \--* indir int
// \--* lclVar ref V02 loc0
if (compCurStmt->GetRootNode() == tree)
{
return nullptr;
}

objectRefPtr = tree->AsIndir()->Addr();
propKind = optPropKind::OPK_OBJ_GETTYPE;
}
else
{
return nullptr;
}
return nullptr;
}

if (!objectRefPtr->OperIsScalarLocal() || !lvaInSsa(objectRefPtr->AsLclVarCommon()->GetLclNum()))
Expand All @@ -355,11 +334,6 @@ GenTree* Compiler::optEarlyPropRewriteTree(GenTree* tree, LocalNumberToNullCheck
optCheckFlagsAreSet(OMF_HAS_ARRAYREF, "OMF_HAS_ARRAYREF", BBF_HAS_IDX_LEN, "BBF_HAS_IDX_LEN", tree,
compCurBB);
}
else
{
optCheckFlagsAreSet(OMF_HAS_VTABLEREF, "OMF_HAS_VTABLEREF", BBF_HAS_VTABREF, "BBF_HAS_VTABREF", tree,
compCurBB);
}
}
#endif

Expand All @@ -369,7 +343,7 @@ GenTree* Compiler::optEarlyPropRewriteTree(GenTree* tree, LocalNumberToNullCheck

if (actualVal != nullptr)
{
assert((propKind == optPropKind::OPK_ARRAYLEN) || (propKind == optPropKind::OPK_OBJ_GETTYPE));
assert(propKind == optPropKind::OPK_ARRAYLEN);
assert(actualVal->IsCnsIntOrI());
assert(actualVal->GetNodeSize() == TREE_NODE_SZ_SMALL);

Expand Down Expand Up @@ -550,14 +524,6 @@ GenTree* Compiler::optPropGetValueRec(unsigned lclNum, unsigned ssaNum, optPropK
}
}
}
else if (valueKind == optPropKind::OPK_OBJ_GETTYPE)
{
value = getObjectHandleNodeFromAllocation(treeRhs DEBUGARG(ssaVarDsc->GetBlock()));
if (value != nullptr)
{
assert(value->IsCnsIntOrI());
}
}
}
}

Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/jit/flowgraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15442,8 +15442,7 @@ bool Compiler::fgOptimizeBranch(BasicBlock* bJump)
gtReverseCond(condTree);

// We need to update the following flags of the bJump block if they were set in the bDest block
bJump->bbFlags |=
(bDest->bbFlags & (BBF_HAS_NEWOBJ | BBF_HAS_NEWARRAY | BBF_HAS_NULLCHECK | BBF_HAS_IDX_LEN | BBF_HAS_VTABREF));
bJump->bbFlags |= (bDest->bbFlags & (BBF_HAS_NEWOBJ | BBF_HAS_NEWARRAY | BBF_HAS_NULLCHECK | BBF_HAS_IDX_LEN));

bJump->bbJumpKind = BBJ_COND;
bJump->bbJumpDest = bDest->bbNext;
Expand Down
2 changes: 0 additions & 2 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12995,8 +12995,6 @@ GenTree* Compiler::gtFoldTypeCompare(GenTree* tree)

// Update various flags
objMT->gtFlags |= GTF_EXCEPT;
compCurBB->bbFlags |= BBF_HAS_VTABREF;
optMethodFlags |= OMF_HAS_VTABLEREF;

// Compare the two method tables
GenTree* const compare = gtCreateHandleCompare(oper, objMT, knownMT, typeCheckInliningResult);
Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/jit/optimizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4281,8 +4281,7 @@ void Compiler::fgOptWhileLoop(BasicBlock* block)
// Flag the block that received the copy as potentially having an array/vtable
// reference, nullcheck, object/array allocation if the block copied from did;
// this is a conservative guess.
if (auto copyFlags = bTest->bbFlags &
(BBF_HAS_VTABREF | BBF_HAS_IDX_LEN | BBF_HAS_NULLCHECK | BBF_HAS_NEWOBJ | BBF_HAS_NEWARRAY))
if (auto copyFlags = bTest->bbFlags & (BBF_HAS_IDX_LEN | BBF_HAS_NULLCHECK | BBF_HAS_NEWOBJ | BBF_HAS_NEWARRAY))
{
block->bbFlags |= copyFlags;
}
Expand Down

0 comments on commit e516cf3

Please sign in to comment.