From 91d116db2b63fa81668fa3ebf7b4d3d64797ef0d Mon Sep 17 00:00:00 2001 From: Mikhail Skvortcov Date: Wed, 8 Feb 2017 13:26:22 +0300 Subject: [PATCH] Address the feedback. Commit migrated from https://github.com/dotnet/coreclr/commit/a38f3351a8c01931f3f9dd18a3e7ec8ff788d1e3 --- src/coreclr/src/jit/codegen.h | 10 ++++++++++ src/coreclr/src/jit/codegenarm.cpp | 11 +++-------- src/coreclr/src/jit/codegencommon.cpp | 31 +++++++++++++++++++++++++++++++ src/coreclr/src/jit/codegenlegacy.cpp | 25 +------------------------ src/coreclr/src/jit/codegenlinear.cpp | 4 ++++ src/coreclr/src/jit/instr.cpp | 8 ++------ src/coreclr/src/jit/jiteh.cpp | 2 +- 7 files changed, 52 insertions(+), 39 deletions(-) diff --git a/src/coreclr/src/jit/codegen.h b/src/coreclr/src/jit/codegen.h index 090283ee..507970e 100755 --- a/src/coreclr/src/jit/codegen.h +++ b/src/coreclr/src/jit/codegen.h @@ -470,6 +470,9 @@ protected: void genSetPSPSym(regNumber initReg, bool* pInitRegZeroed); void genUpdateCurrentFunclet(BasicBlock* block); +#if defined(_TARGET_ARM_) + void genInsertNopForUnwinder(BasicBlock* block); +#endif #else // FEATURE_EH_FUNCLETS @@ -479,6 +482,13 @@ protected: return; } +#if defined(_TARGET_ARM_) + void genInsertNopForUnwinder(BasicBlock* block) + { + return; + } +#endif + #endif // FEATURE_EH_FUNCLETS void genGeneratePrologsAndEpilogs(); diff --git a/src/coreclr/src/jit/codegenarm.cpp b/src/coreclr/src/jit/codegenarm.cpp index 2e0183b..580cad5 100644 --- a/src/coreclr/src/jit/codegenarm.cpp +++ b/src/coreclr/src/jit/codegenarm.cpp @@ -78,14 +78,9 @@ BasicBlock* CodeGen::genCallFinally(BasicBlock* block) // jump target using bbJumpDest - that is already used to point // to the finally block. So just skip past the BBJ_ALWAYS unless the // block is RETLESS. - if (!(block->bbFlags & BBF_RETLESS_CALL)) - { - assert(block->isBBCallAlwaysPair()); - - lblk = block; - block = block->bbNext; - } - return block; + assert(!(block->bbFlags & BBF_RETLESS_CALL)); + assert(block->isBBCallAlwaysPair()); + return block->bbNext; } //------------------------------------------------------------------------ diff --git a/src/coreclr/src/jit/codegencommon.cpp b/src/coreclr/src/jit/codegencommon.cpp index b1e474b..65cc6de 100644 --- a/src/coreclr/src/jit/codegencommon.cpp +++ b/src/coreclr/src/jit/codegencommon.cpp @@ -2814,6 +2814,37 @@ void CodeGen::genUpdateCurrentFunclet(BasicBlock* block) } } } + +#if defined(_TARGET_ARM_) +void CodeGen::genInsertNopForUnwinder(BasicBlock* block) +{ + // If this block is the target of a finally return, we need to add a preceding NOP, in the same EH region, + // so the unwinder doesn't get confused by our "movw lr, xxx; movt lr, xxx; b Lyyy" calling convention that + // calls the funclet during non-exceptional control flow. + if (block->bbFlags & BBF_FINALLY_TARGET) + { + assert(block->bbFlags & BBF_JMP_TARGET); + +#ifdef DEBUG + if (compiler->verbose) + { + printf("\nEmitting finally target NOP predecessor for BB%02u\n", block->bbNum); + } +#endif + // Create a label that we'll use for computing the start of an EH region, if this block is + // at the beginning of such a region. If we used the existing bbEmitCookie as is for + // determining the EH regions, then this NOP would end up outside of the region, if this + // block starts an EH region. If we pointed the existing bbEmitCookie here, then the NOP + // would be executed, which we would prefer not to do. + + block->bbUnwindNopEmitCookie = + getEmitter()->emitAddLabel(gcInfo.gcVarPtrSetCur, gcInfo.gcRegGCrefSetCur, gcInfo.gcRegByrefSetCur); + + instGen(INS_nop); + } +} +#endif + #endif // FEATURE_EH_FUNCLETS /***************************************************************************** diff --git a/src/coreclr/src/jit/codegenlegacy.cpp b/src/coreclr/src/jit/codegenlegacy.cpp index 0530863..c616546 100644 --- a/src/coreclr/src/jit/codegenlegacy.cpp +++ b/src/coreclr/src/jit/codegenlegacy.cpp @@ -12680,30 +12680,7 @@ void CodeGen::genCodeForBBlist() #if FEATURE_EH_FUNCLETS #if defined(_TARGET_ARM_) - // If this block is the target of a finally return, we need to add a preceding NOP, in the same EH region, - // so the unwinder doesn't get confused by our "movw lr, xxx; movt lr, xxx; b Lyyy" calling convention that - // calls the funclet during non-exceptional control flow. - if (block->bbFlags & BBF_FINALLY_TARGET) - { - assert(block->bbFlags & BBF_JMP_TARGET); - -#ifdef DEBUG - if (compiler->verbose) - { - printf("\nEmitting finally target NOP predecessor for BB%02u\n", block->bbNum); - } -#endif - // Create a label that we'll use for computing the start of an EH region, if this block is - // at the beginning of such a region. If we used the existing bbEmitCookie as is for - // determining the EH regions, then this NOP would end up outside of the region, if this - // block starts an EH region. If we pointed the existing bbEmitCookie here, then the NOP - // would be executed, which we would prefer not to do. - - block->bbUnwindNopEmitCookie = - getEmitter()->emitAddLabel(gcInfo.gcVarPtrSetCur, gcInfo.gcRegGCrefSetCur, gcInfo.gcRegByrefSetCur); - - instGen(INS_nop); - } + genInsertNopForUnwinder(block); #endif // defined(_TARGET_ARM_) genUpdateCurrentFunclet(block); diff --git a/src/coreclr/src/jit/codegenlinear.cpp b/src/coreclr/src/jit/codegenlinear.cpp index 329c4a7..001048e 100644 --- a/src/coreclr/src/jit/codegenlinear.cpp +++ b/src/coreclr/src/jit/codegenlinear.cpp @@ -246,6 +246,10 @@ void CodeGen::genCodeForBBlist() } } +#if FEATURE_EH_FUNCLETS && defined(_TARGET_ARM_) + genInsertNopForUnwinder(block); +#endif + /* Start a new code output block */ genUpdateCurrentFunclet(block); diff --git a/src/coreclr/src/jit/instr.cpp b/src/coreclr/src/jit/instr.cpp index 5cb2ccd..09cdcca 100644 --- a/src/coreclr/src/jit/instr.cpp +++ b/src/coreclr/src/jit/instr.cpp @@ -2577,7 +2577,6 @@ AGAIN: inst_RV_IV(ins, reg, tree->gtIntCon.gtIconVal, emitActualTypeSize(tree->TypeGet()), flags); break; -#if CPU_LONG_USES_REGPAIR case GT_CNS_LNG: assert(size == EA_4BYTE || size == EA_8BYTE); @@ -2598,8 +2597,7 @@ AGAIN: constVal = (ssize_t)(tree->gtLngCon.gtLconVal >> 32); size = EA_4BYTE; } -#ifndef LEGACY_BACKEND -#ifdef _TARGET_ARM_ +#if defined(_TARGET_ARM_) && CPU_LONG_USES_REGPAIR if ((ins != INS_mov) && !arm_Valid_Imm_For_Instr(ins, constVal, flags)) { regNumber constReg = (offs == 0) ? genRegPairLo(tree->gtRegPair) : genRegPairHi(tree->gtRegPair); @@ -2607,12 +2605,10 @@ AGAIN: getEmitter()->emitIns_R_R(ins, size, reg, constReg, flags); break; } -#endif // _TARGET_ARM_ -#endif // !LEGACY_BACKEND +#endif // _TARGET_ARM_ && CPU_LONG_USES_REGPAIR inst_RV_IV(ins, reg, constVal, size, flags); break; -#endif // CPU_LONG_USES_REGPAIR case GT_COMMA: tree = tree->gtOp.gtOp2; diff --git a/src/coreclr/src/jit/jiteh.cpp b/src/coreclr/src/jit/jiteh.cpp index 4eded3e..2d0eee3 100644 --- a/src/coreclr/src/jit/jiteh.cpp +++ b/src/coreclr/src/jit/jiteh.cpp @@ -1075,7 +1075,7 @@ void* Compiler::ehEmitCookie(BasicBlock* block) void* cookie; -#if FEATURE_EH_FUNCLETS && defined(_TARGET_ARM_) && defined(LEGACY_BACKEND) +#if FEATURE_EH_FUNCLETS && defined(_TARGET_ARM_) if (block->bbFlags & BBF_FINALLY_TARGET) { // Use the offset of the beginning of the NOP padding, not the main block. -- 2.7.4