Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Finally cloning #8551

Merged
merged 1 commit into from Jan 11, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion Documentation/botr/clr-abi.md
Expand Up @@ -149,7 +149,9 @@ For x86, all handlers are generated within the method body, typically in lexical

JIT64 attempts to speed the normal control flow by 'inlining' a called finally along the 'normal' control flow (i.e., leaving a try body in a non-exceptional manner via C# fall-through). Because the VM semantics for non-rude Thread.Abort dictate that handlers will not be aborted, the JIT must mark these 'inlined' finally bodies. These show up as special entries at the end of the EH tables and are marked with `COR_ILEXCEPTION_CLAUSE_FINALLY | COR_ILEXCEPTION_CLAUSE_DUPLICATED`, and the try_start, try_end, and handler_start are all the same: the start of the cloned finally.

JIT32 and RyuJIT currently do not implement finally cloning.
RyuJit also implements finally cloning, for all supported architectures. However, the implementation does not yet handle the thread abort case; cloned finally bodies are not guaranteed to remain intact and are not reported to the runtime. Because of this, finally cloning is disabled for VMs that support thread abort (desktop clr).

Choose a reason for hiding this comment

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

Is there a way to override this and have the desktop CLR do finally cloning?
It would be useful for measurement purposes to have this available.

Copy link
Member Author

Choose a reason for hiding this comment

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

I can add a jit option to override and enable, sure.

FWIW the regular desktop tests were passing with finally cloning enabled. I have not yet tried running anything that stresses thread abort.

Choose a reason for hiding this comment

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

Yes, The new Config variable is what I had in mind. Thanks
COMPLUS_JitEnableFinallyCloning=1

Looks Good To Me

JIT32 does not implement finally cloning.

## Invoking Finallys/Non-local exits

Expand Down
451 changes: 451 additions & 0 deletions Documentation/design-docs/finally-optimizations.md

Large diffs are not rendered by default.

12 changes: 10 additions & 2 deletions src/jit/block.cpp
Expand Up @@ -365,6 +365,14 @@ void BasicBlock::dspFlags()
{
printf("KEEP ");
}
if (bbFlags & BBF_CLONED_FINALLY_BEGIN)
{
printf("cfb ");
}
if (bbFlags & BBF_CLONED_FINALLY_END)
{
printf("cfe ");
}
}

/*****************************************************************************
Expand Down Expand Up @@ -664,7 +672,7 @@ bool BasicBlock::IsLIR()
// Return Value:
// The first statement in the block's bbTreeList.
//
GenTreeStmt* BasicBlock::firstStmt()
GenTreeStmt* BasicBlock::firstStmt() const
{
if (bbTreeList == nullptr)
{
Expand All @@ -683,7 +691,7 @@ GenTreeStmt* BasicBlock::firstStmt()
// Return Value:
// The last statement in the block's bbTreeList.
//
GenTreeStmt* BasicBlock::lastStmt()
GenTreeStmt* BasicBlock::lastStmt() const
{
if (bbTreeList == nullptr)
{
Expand Down
15 changes: 9 additions & 6 deletions src/jit/block.h
Expand Up @@ -353,15 +353,18 @@ struct BasicBlock : private LIR::Range
// BBJ_CALLFINALLY block, as well as, on x86, the final step block out of a
// finally.

#define BBF_CLONED_FINALLY_BEGIN 0x100000000 // First block of a cloned finally region
#define BBF_CLONED_FINALLY_END 0x200000000 // Last block of a cloned finally region

// Flags that relate blocks to loop structure.

#define BBF_LOOP_FLAGS (BBF_LOOP_PREHEADER | BBF_LOOP_HEAD | BBF_LOOP_CALL0 | BBF_LOOP_CALL1)

bool isRunRarely()
bool isRunRarely() const
{
return ((bbFlags & BBF_RUN_RARELY) != 0);
}
bool isLoopHead()
bool isLoopHead() const
{
return ((bbFlags & BBF_LOOP_HEAD) != 0);
}
Expand All @@ -388,7 +391,7 @@ struct BasicBlock : private LIR::Range
// For example, the top block might or might not have BBF_GC_SAFE_POINT,
// but we assume it does not have BBF_GC_SAFE_POINT any more.

#define BBF_SPLIT_LOST (BBF_GC_SAFE_POINT | BBF_HAS_JMP | BBF_KEEP_BBJ_ALWAYS)
#define BBF_SPLIT_LOST (BBF_GC_SAFE_POINT | BBF_HAS_JMP | BBF_KEEP_BBJ_ALWAYS | BBF_CLONED_FINALLY_END)

// Flags gained by the bottom block when a block is split.
// Note, this is a conservative guess.
Expand All @@ -399,7 +402,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_PROF_WEIGHT | BBF_HAS_NEWOBJ | BBF_KEEP_BBJ_ALWAYS | BBF_CLONED_FINALLY_END)

#ifndef __GNUC__ // GCC doesn't like C_ASSERT at global scope
static_assert_no_msg((BBF_SPLIT_NONEXIST & BBF_SPLIT_LOST) == 0);
Expand Down Expand Up @@ -980,8 +983,8 @@ struct BasicBlock : private LIR::Range
return bbNum - 1;
}

GenTreeStmt* firstStmt();
GenTreeStmt* lastStmt();
GenTreeStmt* firstStmt() const;
GenTreeStmt* lastStmt() const;
GenTreeStmt* lastTopLevelStmt();

GenTree* firstNode();
Expand Down
5 changes: 5 additions & 0 deletions src/jit/compiler.h
Expand Up @@ -3500,6 +3500,10 @@ class Compiler

void fgInline();

void fgRemoveEmptyFinally();

void fgCloneFinally();

Copy link

@briansull briansull Jan 4, 2017

Choose a reason for hiding this comment

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

Has the source code for the bodies of these two methods already been checked in?
If so maybe a reference to the PR# for them could be mentioned.

Copy link
Member Author

Choose a reason for hiding this comment

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

The code is in this PR. It may be github is hiding flowgraph.cpp changes from you (since they are large).

GenTreePtr fgGetCritSectOfStaticMethod();

#if !defined(_TARGET_X86_)
Expand Down Expand Up @@ -4271,6 +4275,7 @@ class Compiler
void fgDebugCheckNodeLinks(BasicBlock* block, GenTreePtr stmt);
void fgDebugCheckFlags(GenTreePtr tree);
void fgDebugCheckFlagsHelper(GenTreePtr tree, unsigned treeFlags, unsigned chkFlags);
void fgDebugCheckTryFinallyExits();
#endif

#ifdef LEGACY_BACKEND
Expand Down
6 changes: 5 additions & 1 deletion src/jit/compphases.h
Expand Up @@ -11,9 +11,10 @@
// corresponding array of string names of those phases. This include file undefines CompPhaseNameMacro
// after the last use.
// The arguments are:
// CompPhaseNameMacro(enumName, stringName, hasChildren, parent)
// CompPhaseNameMacro(enumName, stringName, shortName, hasChildren, parent)
// "enumName" is an Enumeration-style all-caps name.
// "stringName" is a self-explanatory.
// "shortName" is an abbreviated form for stringName
// "hasChildren" is true if this phase is broken out into subphases.
// (We should never do EndPhase on a phase that has children, only on 'leaf phases.')
// "parent" is -1 for leaf phases, otherwise it is the "enumName" of the parent phase.
Expand Down Expand Up @@ -97,6 +98,9 @@ CompPhaseNameMacro(PHASE_EMIT_GCEH, "Emit GC+EH tables",
// for calls through ICorJitInfo across all "real" phases.
CompPhaseNameMacro(PHASE_CLR_API, "CLR API calls", "CLR-API", false, -1)
#endif

CompPhaseNameMacro(PHASE_EMPTY_FINALLY, "Remove empty finally", "EMPTYFIN", false, -1)
CompPhaseNameMacro(PHASE_CLONE_FINALLY, "Clone finally", "CLONEFIN", false, -1)
// clang-format on

#undef CompPhaseNameMacro