Skip to content

Commit

Permalink
Make compCurBB available for fgMorphBlockReturn. (#34184)
Browse files Browse the repository at this point in the history 
* Extract `fgMergeBlockReturn`.

Morph is already a very vague verb in the Jit, try to use a more precise one.

* Add a function header.

* Make `compCurBB` available for `fgMorphBlockReturn`.

When we generate an assignment we could need to create a new assertion, that requires `compCurBB` to be available.

* Delete `INVALID_POINTER_VALUE`.

I would like to remove it because:
1) it was debug only;
2) there were no null checks for `compHndBBtab`, because it is a dependent variable
so there was no need to distinguish valid null pointer from a bad invalid pointer;
3) that is the only place where this mechanism was used.

* Allow to CSE the merge return ASG.

I can't see a reason why it should not, there are no diffs.
The issue with the previous version was that we did not actually know what we were marking: GT_ASG, GT_COMMA, something else? Was the idea to mark individual ASG under COMMA?
  • Loading branch information
Sergey Andreenko authored Mar 31, 2020
1 parent 162595e commit c7a1ef6
Show file tree
Hide file tree
Showing 4 changed files with 120 additions and 120 deletions.
2 changes: 2 additions & 0 deletions src/coreclr/src/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4440,6 +4440,8 @@ class Compiler
void fgMorphStmts(BasicBlock* block, bool* lnot, bool* loadw);
void fgMorphBlocks();

void fgMergeBlockReturn(BasicBlock* block);

bool fgMorphBlockStmt(BasicBlock* block, Statement* stmt DEBUGARG(const char* msg));

void fgSetOptions();
Expand Down
6 changes: 0 additions & 6 deletions src/coreclr/src/jit/jit.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -762,12 +762,6 @@ extern int jitNativeCode(CORINFO_METHOD_HANDLE methodHnd,
JitFlags* compileFlags,
void* inlineInfoPtr);

#ifdef HOST_64BIT
const size_t INVALID_POINTER_VALUE = 0xFEEDFACEABADF00D;
#else
const size_t INVALID_POINTER_VALUE = 0xFEEDFACE;
#endif

// Constants for making sure size_t fit into smaller types.
const size_t MAX_USHORT_SIZE_T = static_cast<size_t>(static_cast<unsigned short>(-1));
const size_t MAX_UNSIGNED_SIZE_T = static_cast<size_t>(static_cast<unsigned>(-1));
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/src/jit/jiteh.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1411,7 +1411,7 @@ void Compiler::fgRemoveEHTableEntry(unsigned XTnum)
if (compHndBBtabCount == 0)
{
// No more entries remaining.
INDEBUG(compHndBBtab = (EHblkDsc*)INVALID_POINTER_VALUE;)
compHndBBtab = nullptr;
}
else
{
Expand Down
230 changes: 117 additions & 113 deletions src/coreclr/src/jit/morph.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15467,10 +15467,6 @@ void Compiler::fgMorphStmts(BasicBlock* block, bool* lnot, bool* loadw)
{
fgRemoveRestOfBlock = false;

/* Make the current basic block address available globally */

compCurBB = block;

*lnot = *loadw = false;

fgCurrentlyInUseArgTemps = hashBv::Create(this);
Expand Down Expand Up @@ -15668,10 +15664,6 @@ void Compiler::fgMorphStmts(BasicBlock* block, bool* lnot, bool* loadw)
}
#endif

#ifdef DEBUG
compCurBB = (BasicBlock*)INVALID_POINTER_VALUE;
#endif

// Reset this back so that it doesn't leak out impacting other blocks
fgRemoveRestOfBlock = false;
}
Expand Down Expand Up @@ -15754,132 +15746,144 @@ void Compiler::fgMorphBlocks()
optAssertionReset(0);
}
#endif
// Make the current basic block address available globally.
compCurBB = block;

/* Process all statement trees in the basic block */

// Process all statement trees in the basic block.
fgMorphStmts(block, &lnot, &loadw);

/* Are we using a single return block? */

if (block->bbJumpKind == BBJ_RETURN)
// Do we need to merge the result of this block into a single return block?
if ((block->bbJumpKind == BBJ_RETURN) && ((block->bbFlags & BBF_HAS_JMP) == 0))
{
if ((genReturnBB != nullptr) && (genReturnBB != block) && ((block->bbFlags & BBF_HAS_JMP) == 0))
if ((genReturnBB != nullptr) && (genReturnBB != block))
{
fgMergeBlockReturn(block);
}
}

// Note 1: A block is not guaranteed to have a last stmt if its jump kind is BBJ_RETURN.
// For example a method returning void could have an empty block with jump kind BBJ_RETURN.
// Such blocks do materialize as part of in-lining.
//
// Note 2: A block with jump kind BBJ_RETURN does not necessarily need to end with GT_RETURN.
// It could end with a tail call or rejected tail call or monitor.exit or a GT_INTRINSIC.
// For now it is safe to explicitly check whether last stmt is GT_RETURN if genReturnLocal
// is BAD_VAR_NUM.
//
// TODO: Need to characterize the last top level stmt of a block ending with BBJ_RETURN.
block = block->bbNext;
} while (block != nullptr);

Statement* lastStmt = block->lastStmt();
GenTree* ret = (lastStmt != nullptr) ? lastStmt->GetRootNode() : nullptr;
// We are done with the global morphing phase
fgGlobalMorph = false;
compCurBB = nullptr;

if ((ret != nullptr) && (ret->OperGet() == GT_RETURN) && ((ret->gtFlags & GTF_RET_MERGED) != 0))
{
// This return was generated during epilog merging, so leave it alone
}
else
{
/* We'll jump to the genReturnBB */
CLANG_FORMAT_COMMENT_ANCHOR;
#ifdef DEBUG
if (verboseTrees)
{
fgDispBasicBlocks(true);
}
#endif
}

//------------------------------------------------------------------------
// fgMergeBlockReturn: assign the block return value (if any) into the single return temp
// and branch to the single return block.
//
// Arguments:
// block - the block to process.
//
// Notes:
// A block is not guaranteed to have a last stmt if its jump kind is BBJ_RETURN.
// For example a method returning void could have an empty block with jump kind BBJ_RETURN.
// Such blocks do materialize as part of in-lining.
//
// A block with jump kind BBJ_RETURN does not necessarily need to end with GT_RETURN.
// It could end with a tail call or rejected tail call or monitor.exit or a GT_INTRINSIC.
// For now it is safe to explicitly check whether last stmt is GT_RETURN if genReturnLocal
// is BAD_VAR_NUM.
//
void Compiler::fgMergeBlockReturn(BasicBlock* block)
{
assert((block->bbJumpKind == BBJ_RETURN) && ((block->bbFlags & BBF_HAS_JMP) == 0));
assert((genReturnBB != nullptr) && (genReturnBB != block));

// TODO: Need to characterize the last top level stmt of a block ending with BBJ_RETURN.

Statement* lastStmt = block->lastStmt();
GenTree* ret = (lastStmt != nullptr) ? lastStmt->GetRootNode() : nullptr;

if ((ret != nullptr) && (ret->OperGet() == GT_RETURN) && ((ret->gtFlags & GTF_RET_MERGED) != 0))
{
// This return was generated during epilog merging, so leave it alone
}
else
{
// We'll jump to the genReturnBB.
CLANG_FORMAT_COMMENT_ANCHOR;

#if !defined(TARGET_X86)
if (info.compFlags & CORINFO_FLG_SYNCH)
{
fgConvertSyncReturnToLeave(block);
}
else
if (info.compFlags & CORINFO_FLG_SYNCH)
{
fgConvertSyncReturnToLeave(block);
}
else
#endif // !TARGET_X86
{
block->bbJumpKind = BBJ_ALWAYS;
block->bbJumpDest = genReturnBB;
fgAddRefPred(genReturnBB, block);
fgReturnCount--;
}
if (genReturnLocal != BAD_VAR_NUM)
{
// replace the GT_RETURN node to be a GT_ASG that stores the return value into genReturnLocal.

// Method must be returning a value other than TYP_VOID.
noway_assert(compMethodHasRetVal());

// This block must be ending with a GT_RETURN
noway_assert(lastStmt != nullptr);
noway_assert(lastStmt->GetNextStmt() == nullptr);
noway_assert(ret != nullptr);

// GT_RETURN must have non-null operand as the method is returning the value assigned to
// genReturnLocal
noway_assert(ret->OperGet() == GT_RETURN);
noway_assert(ret->gtGetOp1() != nullptr);

Statement* pAfterStatement = lastStmt;
IL_OFFSETX offset = lastStmt->GetILOffsetX();
GenTree* tree =
gtNewTempAssign(genReturnLocal, ret->gtGetOp1(), &pAfterStatement, offset, block);
if (tree->OperIsCopyBlkOp())
{
tree = fgMorphCopyBlock(tree);
}
{
block->bbJumpKind = BBJ_ALWAYS;
block->bbJumpDest = genReturnBB;
fgAddRefPred(genReturnBB, block);
fgReturnCount--;
}
if (genReturnLocal != BAD_VAR_NUM)
{
// replace the GT_RETURN node to be a GT_ASG that stores the return value into genReturnLocal.

if (pAfterStatement == lastStmt)
{
lastStmt->SetRootNode(tree);
}
else
{
// gtNewTempAssign inserted additional statements after last
fgRemoveStmt(block, lastStmt);
Statement* newStmt = gtNewStmt(tree, offset);
fgInsertStmtAfter(block, pAfterStatement, newStmt);
lastStmt = newStmt;
}
// Method must be returning a value other than TYP_VOID.
noway_assert(compMethodHasRetVal());

// make sure that copy-prop ignores this assignment.
lastStmt->GetRootNode()->gtFlags |= GTF_DONT_CSE;
}
else if (ret != nullptr && ret->OperGet() == GT_RETURN)
{
// This block ends with a GT_RETURN
noway_assert(lastStmt != nullptr);
noway_assert(lastStmt->GetNextStmt() == nullptr);
// This block must be ending with a GT_RETURN
noway_assert(lastStmt != nullptr);
noway_assert(lastStmt->GetNextStmt() == nullptr);
noway_assert(ret != nullptr);

// Must be a void GT_RETURN with null operand; delete it as this block branches to oneReturn
// block
noway_assert(ret->TypeGet() == TYP_VOID);
noway_assert(ret->gtGetOp1() == nullptr);
// GT_RETURN must have non-null operand as the method is returning the value assigned to
// genReturnLocal
noway_assert(ret->OperGet() == GT_RETURN);
noway_assert(ret->gtGetOp1() != nullptr);

fgRemoveStmt(block, lastStmt);
}
#ifdef DEBUG
if (verbose)
{
printf("morph " FMT_BB " to point at onereturn. New block is\n", block->bbNum);
fgTableDispBasicBlock(block);
}
#endif
}
Statement* pAfterStatement = lastStmt;
IL_OFFSETX offset = lastStmt->GetILOffsetX();
GenTree* tree = gtNewTempAssign(genReturnLocal, ret->gtGetOp1(), &pAfterStatement, offset, block);
if (tree->OperIsCopyBlkOp())
{
tree = fgMorphCopyBlock(tree);
}
}
block = block->bbNext;
} while (block);

/* We are done with the global morphing phase */
if (pAfterStatement == lastStmt)
{
lastStmt->SetRootNode(tree);
}
else
{
// gtNewTempAssign inserted additional statements after last
fgRemoveStmt(block, lastStmt);
Statement* newStmt = gtNewStmt(tree, offset);
fgInsertStmtAfter(block, pAfterStatement, newStmt);
lastStmt = newStmt;
}
}
else if (ret != nullptr && ret->OperGet() == GT_RETURN)
{
// This block ends with a GT_RETURN
noway_assert(lastStmt != nullptr);
noway_assert(lastStmt->GetNextStmt() == nullptr);

fgGlobalMorph = false;
// Must be a void GT_RETURN with null operand; delete it as this block branches to oneReturn
// block
noway_assert(ret->TypeGet() == TYP_VOID);
noway_assert(ret->gtGetOp1() == nullptr);

fgRemoveStmt(block, lastStmt);
}
#ifdef DEBUG
if (verboseTrees)
{
fgDispBasicBlocks(true);
}
if (verbose)
{
printf("morph " FMT_BB " to point at onereturn. New block is\n", block->bbNum);
fgTableDispBasicBlock(block);
}
#endif
}
}

/*****************************************************************************
Expand Down

0 comments on commit c7a1ef6

Please sign in to comment.