-
Notifications
You must be signed in to change notification settings - Fork 5k
Align inner loops #44370
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
Align inner loops #44370
Changes from all commits
32236ac
a2ec5d1
89b5812
a84b8be
a51759a
2f57c78
b4848f8
fe92c62
51c3171
0b227a8
a9c9764
7f1f787
aac18a4
5c3c40e
fb91d41
0b8eb78
072a113
7e1328a
d49e84d
f6b5135
256aae5
26c3c84
6bb1a75
03f6ba6
8036061
809fc85
5b9b7a0
5584d71
f66ee24
f2f935d
99ea31f
8f64963
1c85c3c
ef0b149
599ad43
97fd373
37b0cdb
c0cc8af
26f7e61
305b812
f8bdfec
a205cc0
d576e9c
28480d1
bf03842
dae9749
a153742
ef02fbb
e7e0d68
4b0e64d
6fddce8
bad5685
8c30a96
f32f560
23177b0
9f3cb2d
74620ed
b100226
dbeb7d6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -311,13 +311,6 @@ void CodeGen::genCodeForBBlist() | |
|
||
genUpdateCurrentFunclet(block); | ||
|
||
#ifdef TARGET_XARCH | ||
if (ShouldAlignLoops() && block->bbFlags & BBF_LOOP_HEAD) | ||
{ | ||
GetEmitter()->emitLoopAlign(); | ||
} | ||
#endif | ||
|
||
genLogLabel(block); | ||
|
||
// Tell everyone which basic block we're working on | ||
|
@@ -356,6 +349,14 @@ void CodeGen::genCodeForBBlist() | |
needLabel = true; | ||
} | ||
|
||
#if FEATURE_LOOP_ALIGN | ||
if (GetEmitter()->emitEndsWithAlignInstr()) | ||
{ | ||
// we had better be planning on starting a new IG | ||
assert(needLabel); | ||
} | ||
#endif | ||
|
||
if (needLabel) | ||
{ | ||
// Mark a label and update the current set of live GC refs | ||
|
@@ -667,10 +668,6 @@ void CodeGen::genCodeForBBlist() | |
|
||
switch (block->bbJumpKind) | ||
{ | ||
case BBJ_ALWAYS: | ||
inst_JMP(EJ_jmp, block->bbJumpDest); | ||
break; | ||
|
||
case BBJ_RETURN: | ||
genExitCode(block); | ||
break; | ||
|
@@ -741,15 +738,55 @@ void CodeGen::genCodeForBBlist() | |
#endif // !FEATURE_EH_FUNCLETS | ||
|
||
case BBJ_NONE: | ||
case BBJ_COND: | ||
case BBJ_SWITCH: | ||
break; | ||
|
||
case BBJ_ALWAYS: | ||
inst_JMP(EJ_jmp, block->bbJumpDest); | ||
FALLTHROUGH; | ||
|
||
case BBJ_COND: | ||
|
||
#if FEATURE_LOOP_ALIGN | ||
// This is the last place where we operate on blocks and after this, we operate | ||
// on IG. Hence, if we know that the destination of "block" is the first block | ||
// of a loop and needs alignment (it has BBF_LOOP_ALIGN), then "block" represents | ||
// end of the loop. Propagate that information on the IG through "igLoopBackEdge". | ||
// | ||
// During emitter, this information will be used to calculate the loop size. | ||
// Depending on the loop size, decision of whether to align a loop or not will be taken. | ||
|
||
if (block->bbJumpDest->isLoopAlign()) | ||
{ | ||
GetEmitter()->emitSetLoopBackEdge(block->bbJumpDest); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This condition seems odd to me: Presumably, you've already determined that an "isLoopAlign" block is the top of a loop. But now, any block that jumps to that is considered a back edge. What if the lexically first block of the loop is the target of branches that aren't loop branches: forward branches jumping to the loop top, or non-loop backward branches? (The non-loop backward branches might not matter since presumably your algorithm will hit an in-loop backward branch first, but possibly marking them could confuse other code? E.g., what if the first block in a non-first loop branched back to the top of the first loop? I'm not sure if all these cases can occur, due to loop canonicalization, etc., but this case seems not too specific) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} | ||
#endif | ||
break; | ||
|
||
default: | ||
noway_assert(!"Unexpected bbJumpKind"); | ||
break; | ||
} | ||
|
||
#if FEATURE_LOOP_ALIGN | ||
|
||
// If next block is the first block of a loop (identified by BBF_LOOP_ALIGN), | ||
// then need to add align instruction in current "block". Also mark the | ||
// corresponding IG with IGF_LOOP_ALIGN to know that there will be align | ||
// instructions at the end of that IG. | ||
// | ||
// For non-adaptive alignment, add alignment instruction of size depending on the | ||
// compJitAlignLoopBoundary. | ||
// For adaptive alignment, alignment instruction will always be of 15 bytes. | ||
|
||
if ((block->bbNext != nullptr) && (block->bbNext->isLoopAlign())) | ||
{ | ||
assert(ShouldAlignLoops()); | ||
|
||
GetEmitter()->emitLoopAlignment(); | ||
} | ||
#endif | ||
|
||
#if defined(DEBUG) && defined(USING_VARIABLE_LIVE_RANGE) | ||
if (compiler->verbose) | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2308,7 +2308,7 @@ void Compiler::compSetProcessor() | |
opts.compUseCMOV = jitFlags.IsSet(JitFlags::JIT_FLAG_USE_CMOV); | ||
#ifdef DEBUG | ||
if (opts.compUseCMOV) | ||
opts.compUseCMOV = !compStressCompile(STRESS_USE_CMOV, 50); | ||
opts.compUseCMOV = !compStressCompile(STRESS_USE_CMOV, 50); | ||
#endif // DEBUG | ||
|
||
#endif // TARGET_X86 | ||
|
@@ -2615,6 +2615,29 @@ void Compiler::compInitOptions(JitFlags* jitFlags) | |
opts.compDbgInfo = jitFlags->IsSet(JitFlags::JIT_FLAG_DEBUG_INFO); | ||
opts.compDbgEnC = jitFlags->IsSet(JitFlags::JIT_FLAG_DEBUG_EnC); | ||
|
||
#ifdef DEBUG | ||
opts.compJitAlignLoopAdaptive = JitConfig.JitAlignLoopAdaptive() == 1; | ||
opts.compJitAlignLoopBoundary = (unsigned short)JitConfig.JitAlignLoopBoundary(); | ||
opts.compJitAlignLoopMinBlockWeight = (unsigned short)JitConfig.JitAlignLoopMinBlockWeight(); | ||
|
||
opts.compJitAlignLoopForJcc = JitConfig.JitAlignLoopForJcc() == 1; | ||
opts.compJitAlignLoopMaxCodeSize = (unsigned short)JitConfig.JitAlignLoopMaxCodeSize(); | ||
#else | ||
opts.compJitAlignLoopAdaptive = true; | ||
opts.compJitAlignLoopBoundary = DEFAULT_ALIGN_LOOP_BOUNDARY; | ||
opts.compJitAlignLoopMinBlockWeight = DEFAULT_ALIGN_LOOP_MIN_BLOCK_WEIGHT; | ||
#endif | ||
if (opts.compJitAlignLoopAdaptive) | ||
{ | ||
opts.compJitAlignPaddingLimit = (opts.compJitAlignLoopBoundary >> 1) - 1; | ||
} | ||
else | ||
{ | ||
opts.compJitAlignPaddingLimit = opts.compJitAlignLoopBoundary - 1; | ||
} | ||
|
||
assert(isPow2(opts.compJitAlignLoopBoundary)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like And whatever value it does get set to should be overridable in debug, in case you want to experiment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I missed setting There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, it can wait. |
||
|
||
#if REGEN_SHORTCUTS || REGEN_CALLPAT | ||
// We never want to have debugging enabled when regenerating GC encoding patterns | ||
opts.compDbgCode = false; | ||
|
@@ -3913,19 +3936,17 @@ void Compiler::compSetOptimizationLevel() | |
codeGen->setFrameRequired(true); | ||
#endif | ||
|
||
if (opts.jitFlags->IsSet(JitFlags::JIT_FLAG_RELOC)) | ||
if (opts.jitFlags->IsSet(JitFlags::JIT_FLAG_PREJIT)) | ||
{ | ||
codeGen->SetAlignLoops(false); // loop alignment not supported for prejitted code | ||
|
||
// The zapper doesn't set JitFlags::JIT_FLAG_ALIGN_LOOPS, and there is | ||
// no reason for it to set it as the JIT doesn't currently support loop alignment | ||
// for prejitted images. (The JIT doesn't know the final address of the code, hence | ||
// The JIT doesn't currently support loop alignment for prejitted images. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I realize this is preexisting code, but this seems like a roundabout way to check if we are prejitting; why not just check the prejit flag? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You mean use |
||
// (The JIT doesn't know the final address of the code, hence | ||
// it can't align code based on unknown addresses.) | ||
assert(!opts.jitFlags->IsSet(JitFlags::JIT_FLAG_ALIGN_LOOPS)); | ||
|
||
codeGen->SetAlignLoops(false); // loop alignment not supported for prejitted code | ||
kunalspathak marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
else | ||
{ | ||
codeGen->SetAlignLoops(opts.jitFlags->IsSet(JitFlags::JIT_FLAG_ALIGN_LOOPS)); | ||
codeGen->SetAlignLoops(JitConfig.JitAlignLoops() == 1); | ||
} | ||
} | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.