Skip to content

Commit b100226

Browse files
kunalspathakkunalspathak
committed
review feedback
1 parent 74620ed commit b100226

File tree

8 files changed

+46
-25
lines changed

8 files changed

+46
-25
lines changed

‎src/coreclr/jit/codegencommon.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2258,7 +2258,7 @@ void CodeGen::genGenerateMachineCode()
22582258

22592259
GetEmitter()->emitJumpDistBind();
22602260

2261-
#ifdef FEATURE_LOOP_ALIGN
2261+
#if FEATURE_LOOP_ALIGN
22622262
/* Perform alignment adjustments */
22632263

22642264
GetEmitter()->emitLoopAlignAdjustments();

‎src/coreclr/jit/codegenlinear.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -349,11 +349,13 @@ void CodeGen::genCodeForBBlist()
349349
needLabel = true;
350350
}
351351

352-
if (GetEmitter()->emitCurIG->isLoopAlign())
352+
#if FEATURE_LOOP_ALIGN
353+
if (GetEmitter()->emitEndsWithAlignInstr())
353354
{
354355
// we had better be planning on starting a new IG
355356
assert(needLabel);
356357
}
358+
#endif
357359

358360
if (needLabel)
359361
{
@@ -745,7 +747,7 @@ void CodeGen::genCodeForBBlist()
745747

746748
case BBJ_COND:
747749

748-
#ifdef FEATURE_LOOP_ALIGN
750+
#if FEATURE_LOOP_ALIGN
749751
// This is the last place where we operate on blocks and after this, we operate
750752
// on IG. Hence, if we know that the destination of "block" is the first block
751753
// of a loop and needs alignment (it has BBF_LOOP_ALIGN), then "block" represents
@@ -766,7 +768,7 @@ void CodeGen::genCodeForBBlist()
766768
break;
767769
}
768770

769-
#ifdef FEATURE_LOOP_ALIGN
771+
#if FEATURE_LOOP_ALIGN
770772

771773
// If next block is the first block of a loop (identified by BBF_LOOP_ALIGN),
772774
// then need to add align instruction in current "block". Also mark the

‎src/coreclr/jit/emit.cpp

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -641,7 +641,7 @@ void emitter::emitGenIG(insGroup* ig)
641641

642642
assert(emitCurIGjmpList == nullptr);
643643

644-
#ifdef FEATURE_LOOP_ALIGN
644+
#if FEATURE_LOOP_ALIGN
645645
assert(emitCurIGAlignList == nullptr);
646646
#endif
647647

@@ -831,7 +831,7 @@ insGroup* emitter::emitSavIG(bool emitAdd)
831831
}
832832
#endif
833833

834-
#ifdef FEATURE_LOOP_ALIGN
834+
#if FEATURE_LOOP_ALIGN
835835
// Did we have any align instructions in this group?
836836
if (emitCurIGAlignList)
837837
{
@@ -996,7 +996,7 @@ void emitter::emitBegFN(bool hasFramePtr
996996
emitCurIGfreeBase = nullptr;
997997
emitIGbuffSize = 0;
998998

999-
#ifdef FEATURE_LOOP_ALIGN
999+
#if FEATURE_LOOP_ALIGN
10001000
emitLastAlignedIgNum = 0;
10011001
emitLastInnerLoopStartIgNum = 0;
10021002
emitLastInnerLoopEndIgNum = 0;
@@ -1037,7 +1037,7 @@ void emitter::emitBegFN(bool hasFramePtr
10371037
emitNoGCIG = false;
10381038
emitForceNewIG = false;
10391039

1040-
#ifdef FEATURE_LOOP_ALIGN
1040+
#if FEATURE_LOOP_ALIGN
10411041
/* We don't have any align instructions */
10421042

10431043
emitAlignList = emitAlignLast = nullptr;
@@ -3735,7 +3735,7 @@ size_t emitter::emitIssue1Instr(insGroup* ig, instrDesc* id, BYTE** dp)
37353735
// It is fatal to under-estimate the instruction size, except for alignment instructions
37363736
noway_assert(estimatedSize >= actualSize);
37373737

3738-
#ifdef FEATURE_LOOP_ALIGN
3738+
#if FEATURE_LOOP_ALIGN
37393739
// Should never over-estimate align instruction or any instruction before the last align instruction of a method
37403740
assert(id->idIns() != INS_align && emitCurIG->igNum > emitLastAlignedIgNum);
37413741
#endif
@@ -4610,7 +4610,7 @@ void emitter::emitJumpDistBind()
46104610
#endif // DEBUG
46114611
}
46124612

4613-
#ifdef FEATURE_LOOP_ALIGN
4613+
#if FEATURE_LOOP_ALIGN
46144614

46154615
//-----------------------------------------------------------------------------
46164616
// emitLoopAlignment: Insert an align instruction at the end of emitCurIG and
@@ -4636,6 +4636,16 @@ void emitter::emitLoopAlignment()
46364636
emitComp->compMethodID, emitCurIG->igNum);
46374637
}
46384638

4639+
//-----------------------------------------------------------------------------
4640+
// emitEndsWithAlignInstr: Checks if current IG ends with loop align instruction.
4641+
//
4642+
// Returns: true if current IG ends with align instruciton.
4643+
//
4644+
bool emitter::emitEndsWithAlignInstr()
4645+
{
4646+
return emitCurIG->isLoopAlign();
4647+
}
4648+
46394649
//-----------------------------------------------------------------------------
46404650
// getLoopSize: Starting from loopHeaderIg, find the size of the smallest possible loop
46414651
// such that it doesn't exceed the maxLoopSize.
@@ -7762,10 +7772,13 @@ void emitter::emitInitIG(insGroup* ig)
77627772
sure we act the same in non-DEBUG builds.
77637773
*/
77647774

7765-
ig->igSize = 0;
7766-
ig->igGCregs = RBM_NONE;
7767-
ig->igInsCnt = 0;
7775+
ig->igSize = 0;
7776+
ig->igGCregs = RBM_NONE;
7777+
ig->igInsCnt = 0;
7778+
7779+
#if FEATURE_LOOP_ALIGN
77687780
ig->igLoopBackEdge = nullptr;
7781+
#endif
77697782
}
77707783

77717784
/*****************************************************************************

‎src/coreclr/jit/emit.h

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,10 @@ struct insGroup
250250
unsigned int igFuncIdx; // Which function/funclet does this belong to? (Index into Compiler::compFuncInfos array.)
251251
unsigned short igFlags; // see IGF_xxx below
252252
unsigned short igSize; // # of bytes of code in this group
253-
insGroup* igLoopBackEdge; // "last" back-edge that branches back to an aligned loop head.
253+
254+
#if FEATURE_LOOP_ALIGN
255+
insGroup* igLoopBackEdge; // "last" back-edge that branches back to an aligned loop head.
256+
#endif
254257

255258
#define IGF_GC_VARS 0x0001 // new set of live GC ref variables
256259
#define IGF_BYREF_REGS 0x0002 // new set of live by-ref registers
@@ -1370,7 +1373,7 @@ class emitter
13701373
// hot to cold and cold to hot jumps)
13711374
};
13721375

1373-
#ifdef FEATURE_LOOP_ALIGN
1376+
#if FEATURE_LOOP_ALIGN
13741377
struct instrDescAlign : instrDesc
13751378
{
13761379
instrDescAlign* idaNext; // next align in the group/method
@@ -1755,7 +1758,7 @@ class emitter
17551758
instrDescJmp* emitJumpLast; // last of local jumps in method
17561759
void emitJumpDistBind(); // Bind all the local jumps in method
17571760

1758-
#ifdef FEATURE_LOOP_ALIGN
1761+
#if FEATURE_LOOP_ALIGN
17591762
instrDescAlign* emitCurIGAlignList; // list of align instructions in current IG
17601763
unsigned emitLastInnerLoopStartIgNum; // Start IG of last inner loop
17611764
unsigned emitLastInnerLoopEndIgNum; // End IG of last inner loop
@@ -1764,6 +1767,7 @@ class emitter
17641767
instrDescAlign* emitAlignLast; // last align instruction in method
17651768
unsigned getLoopSize(insGroup* igLoopHeader, unsigned maxLoopSize); // Get the smallest loop size
17661769
void emitLoopAlignment();
1770+
bool emitEndsWithAlignInstr(); // Validate if newLabel is appropriate
17671771
void emitSetLoopBackEdge(BasicBlock* loopTopBlock);
17681772
void emitLoopAlignAdjustments(); // Predict if loop alignment is needed and make appropriate adjustments
17691773
unsigned emitCalculatePaddingForLoopAlignment(insGroup* ig, size_t offset DEBUG_ARG(bool displayAlignmentDetails));
@@ -2009,7 +2013,7 @@ class emitter
20092013
return (instrDescCGCA*)emitAllocAnyInstr(sizeof(instrDescCGCA), attr);
20102014
}
20112015

2012-
#ifdef FEATURE_LOOP_ALIGN
2016+
#if FEATURE_LOOP_ALIGN
20132017
instrDescAlign* emitAllocInstrAlign()
20142018
{
20152019
#if EMITTER_STATS
@@ -2544,7 +2548,7 @@ inline emitter::instrDescJmp* emitter::emitNewInstrJmp()
25442548
return emitAllocInstrJmp();
25452549
}
25462550

2547-
#ifdef FEATURE_LOOP_ALIGN
2551+
#if FEATURE_LOOP_ALIGN
25482552
inline emitter::instrDescAlign* emitter::emitNewInstrAlign()
25492553
{
25502554
instrDescAlign* newInstr = emitAllocInstrAlign();

‎src/coreclr/jit/emitxarch.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7388,7 +7388,7 @@ size_t emitter::emitSizeOfInsDsc(instrDesc* id)
73887388
switch (idOp)
73897389
{
73907390
case ID_OP_NONE:
7391-
#ifdef FEATURE_LOOP_ALIGN
7391+
#if FEATURE_LOOP_ALIGN
73927392
if (id->idIns() == INS_align)
73937393
{
73947394
return sizeof(instrDescAlign);
@@ -13814,8 +13814,9 @@ size_t emitter::emitOutputInstr(insGroup* ig, instrDesc* id, BYTE** dp)
1381413814
{
1381513815
emitDispIns(id, false, dspOffs, true, emitCurCodeOffs(*dp), *dp, (dst - *dp));
1381613816
}
13817+
#endif
1381713818

13818-
#ifdef FEATURE_LOOP_ALIGN
13819+
#if FEATURE_LOOP_ALIGN
1381913820
// Only compensate over-estimated instructions if emitCurIG is before
1382013821
// the last IG that needs alignment.
1382113822
if (emitCurIG->igNum <= emitLastAlignedIgNum)
@@ -13852,6 +13853,7 @@ size_t emitter::emitOutputInstr(insGroup* ig, instrDesc* id, BYTE** dp)
1385213853
}
1385313854
#endif
1385413855

13856+
#ifdef DEBUG
1385513857
if (emitComp->compDebugBreak)
1385613858
{
1385713859
// set JitEmitPrintRefRegs=1 will print out emitThisGCrefRegs and emitThisByrefRegs

‎src/coreclr/jit/jitconfigvalues.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ CONFIG_INTEGER(EnableIncompleteISAClass, W("EnableIncompleteISAClass"), 0) // En
223223
// intrinsic classes
224224
#endif // defined(DEBUG)
225225

226-
#ifdef FEATURE_LOOP_ALIGN
226+
#if FEATURE_LOOP_ALIGN
227227
CONFIG_INTEGER(JitAlignLoops, W("JitAlignLoops"), 1) // If set, align inner loops
228228
#else
229229
CONFIG_INTEGER(JitAlignLoops, W("JitAlignLoops"), 0)

‎src/coreclr/jit/morph.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16317,7 +16317,7 @@ bool Compiler::fgFoldConditional(BasicBlock* block)
1631716317
* Remove the loop from the table */
1631816318

1631916319
optLoopTable[loopNum].lpFlags |= LPFLG_REMOVED;
16320-
#ifdef FEATURE_LOOP_ALIGN
16320+
#if FEATURE_LOOP_ALIGN
1632116321
optLoopTable[loopNum].lpFirst->bbFlags &= ~BBF_LOOP_ALIGN;
1632216322
JITDUMP("Removing LOOP_ALIGN flag from bogus loop in " FMT_BB "\n",
1632316323
optLoopTable[loopNum].lpFirst->bbNum);

‎src/coreclr/jit/optimizer.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2586,7 +2586,7 @@ void Compiler::optFindNaturalLoops()
25862586

25872587
void Compiler::optIdentifyLoopsForAlignment()
25882588
{
2589-
#ifdef FEATURE_LOOP_ALIGN
2589+
#if FEATURE_LOOP_ALIGN
25902590
if (codeGen->ShouldAlignLoops())
25912591
{
25922592
for (unsigned char loopInd = 0; loopInd < optLoopCount; loopInd++)
@@ -3792,7 +3792,7 @@ void Compiler::optUnrollLoops()
37923792
#endif
37933793
}
37943794

3795-
#ifdef FEATURE_LOOP_ALIGN
3795+
#if FEATURE_LOOP_ALIGN
37963796
for (block = head->bbNext;; block = block->bbNext)
37973797
{
37983798
if (block->isLoopAlign())
@@ -8032,7 +8032,7 @@ bool Compiler::optComputeLoopSideEffectsOfBlock(BasicBlock* blk)
80328032
void Compiler::AddContainsCallAllContainingLoops(unsigned lnum)
80338033
{
80348034

8035-
#ifdef FEATURE_LOOP_ALIGN
8035+
#if FEATURE_LOOP_ALIGN
80368036
// If this is the inner most loop, reset the LOOP_ALIGN flag
80378037
// because a loop having call will not likely to benefit from
80388038
// alignment

0 commit comments

Comments
 (0)