From e060ee604eaa1624e700887c898dbe3007e91073 Mon Sep 17 00:00:00 2001 From: Hanjoung Lee Date: Wed, 7 Sep 2016 01:42:03 +0900 Subject: [PATCH] [ARM] Remove ARM_HAZARD_AVOIDANCE (dotnet/coreclr#7019) remove `ARM_HAZARD_AVOIDANCE` as it is a rare case. For dotnet/coreclr#7003 Commit migrated from https://github.com/dotnet/coreclr/commit/b9402adedcf9cf5204fd9ece52183ac61cef7c83 --- src/coreclr/src/jit/codegencommon.cpp | 11 -- src/coreclr/src/jit/emit.cpp | 25 ----- src/coreclr/src/jit/emit.h | 58 ----------- src/coreclr/src/jit/emitarm.cpp | 111 -------------------- src/coreclr/src/jit/emitarm.h | 3 - src/coreclr/src/jit/target.h | 13 --- src/coreclr/src/zap/zapcode.cpp | 188 +--------------------------------- src/coreclr/src/zap/zapcode.h | 7 +- 8 files changed, 2 insertions(+), 414 deletions(-) diff --git a/src/coreclr/src/jit/codegencommon.cpp b/src/coreclr/src/jit/codegencommon.cpp index 8047230..2710447 100755 --- a/src/coreclr/src/jit/codegencommon.cpp +++ b/src/coreclr/src/jit/codegencommon.cpp @@ -6343,17 +6343,6 @@ bool CodeGen::genCanUsePopToReturn(regMaskTP maskPopRegsInt, bool jmpEpilog) { assert(compiler->compGeneratingEpilog); -#ifdef ARM_HAZARD_AVOIDANCE - // Only need to handle the Krait Hazard when we are Jitting - // - if ((compiler->opts.eeFlags & CORJIT_FLG_PREJIT) == 0) - { - // We will never generate the T2 encoding of pop when we have a Krait Errata - if ((maskPopRegsInt & RBM_HIGH_REGS) != 0) - return false; - } -#endif - if (!jmpEpilog && regSet.rsMaskPreSpillRegs(true) == RBM_NONE) return true; else diff --git a/src/coreclr/src/jit/emit.cpp b/src/coreclr/src/jit/emit.cpp index b255ea7..5c991dd 100644 --- a/src/coreclr/src/jit/emit.cpp +++ b/src/coreclr/src/jit/emit.cpp @@ -1111,11 +1111,6 @@ void emitter::emitBegFN(bool hasFramePtr emitPrologIG = emitIGlist = emitIGlast = emitCurIG = ig = emitAllocIG(); -#ifdef ARM_HAZARD_AVOIDANCE - // This first IG is actually preceeded by the method prolog which may be composed of many T1 instructions - emitCurInstrCntT1 = MAX_INSTR_COUNT_T1; -#endif - emitLastIns = nullptr; ig->igNext = nullptr; @@ -1183,26 +1178,6 @@ void emitter::dispIns(instrDesc* id) void emitter::appendToCurIG(instrDesc* id) { emitCurIGsize += id->idCodeSize(); - -#ifdef ARM_HAZARD_AVOIDANCE - // - // Do we have a T1 instruction or an unbound jump instruction? - // (it could be bound to a T1 instruction) - if (id->idInstrIsT1() || - (((id->idInsFmt() == IF_T2_J2) || (id->idInsFmt() == IF_T2_J1) || (id->idInsFmt() == IF_LARGEJMP)) && - (id->idIsBound() == false))) - { - if (emitCurInstrCntT1 < MAX_INSTR_COUNT_T1) - { - emitCurInstrCntT1++; - } - } - else - { - emitCurInstrCntT1 = 0; - } - -#endif } /***************************************************************************** diff --git a/src/coreclr/src/jit/emit.h b/src/coreclr/src/jit/emit.h index db65e26..8fb24bc 100644 --- a/src/coreclr/src/jit/emit.h +++ b/src/coreclr/src/jit/emit.h @@ -711,9 +711,6 @@ protected: unsigned _idLclVar : 1; // access a local on stack unsigned _idLclFPBase : 1; // access a local on stack - SP based offset insOpts _idInsOpt : 3; // options for Load/Store instructions -#ifdef ARM_HAZARD_AVOIDANCE -#define _idKraitNop _idLclFPBase // Repurpose the _idLclFPBase for Krait Hazard -#endif // For arm we have used 16 bits #define ID_EXTRA_BITFIELD_BITS (16) @@ -1041,10 +1038,6 @@ protected: unsigned idCodeSize() const { unsigned result = (_idInsSize == ISZ_16BIT) ? 2 : (_idInsSize == ISZ_32BIT) ? 4 : 6; -#ifdef ARM_HAZARD_AVOIDANCE - if (idKraitNop()) - result += 4; -#endif return result; } insSize idInsSize() const @@ -1055,40 +1048,7 @@ protected: { _idInsSize = isz; assert(isz == _idInsSize); -#ifdef ARM_HAZARD_AVOIDANCE - if (idIsKraitBranch() && idInstrIsT1()) - idKraitNop(false); -#endif - } -#ifdef ARM_HAZARD_AVOIDANCE - // This function returns true if the current instruction represents a non T1 - // unconditional branch instruction that is subject to the Krait errata - // Note: The T2 pop encoding is handled separately as it only occurs in epilogs - // - bool idIsKraitBranch() const - { - if (idInstrIsT1()) - return false; - if ((idIns() == INS_b) || (idIns() == INS_bl) || ((idIns() == INS_ldr) && (idReg1() == REG_PC))) - { - return true; - } - return false; - } - bool idKraitNop() const - { - if (!idIsKraitBranch()) - return false; - else - return (_idKraitNop != 0); - } - void idKraitNop(bool val) - { - if (idIsKraitBranch()) - _idKraitNop = val; - assert(val == idKraitNop()); } -#endif insFlags idInsFlags() const { return _idInsFlags; @@ -1310,29 +1270,15 @@ protected: #endif // _TARGET_ARMARCH_ #if defined(_TARGET_ARM_) -#ifdef ARM_HAZARD_AVOIDANCE bool idIsLclFPBase() const { - assert(!idIsKraitBranch()); return !idIsTiny() && _idLclFPBase != 0; } void idSetIsLclFPBase() { - assert(!idIsKraitBranch()); assert(!idIsTiny()); _idLclFPBase = 1; } -#else - bool idIsLclFPBase() const - { - return !idIsTiny() && _idLclFPBase != 0; - } - void idSetIsLclFPBase() - { - assert(!idIsTiny()); - _idLclFPBase = 1; - } -#endif #endif // defined(_TARGET_ARM_) #ifdef RELOC_SUPPORT @@ -1798,10 +1744,6 @@ private: unsigned emitCurIGinsCnt; // # of collected instr's in buffer unsigned emitCurIGsize; // estimated code size of current group in bytes -#ifdef ARM_HAZARD_AVOIDANCE -#define MAX_INSTR_COUNT_T1 3 - unsigned emitCurInstrCntT1; // The count of consecutive T1 instructions issued by the JIT -#endif UNATIVE_OFFSET emitCurCodeOffset; // current code offset within group UNATIVE_OFFSET emitTotalCodeSize; // bytes of code in entire method diff --git a/src/coreclr/src/jit/emitarm.cpp b/src/coreclr/src/jit/emitarm.cpp index 893d380..1f57048 100644 --- a/src/coreclr/src/jit/emitarm.cpp +++ b/src/coreclr/src/jit/emitarm.cpp @@ -1406,34 +1406,6 @@ DONE: return false; } -#ifdef ARM_HAZARD_AVOIDANCE -// This function is called whenever we about to emit an unconditional branch instruction -// that could be encoded using a T2 instruction -// It returns true if we need to mark the instruction via idKraitNop(true) -// -bool emitter::emitKraitHazardActive(instrDesc* id) -{ - // Does the current instruction represent an - // unconditional branch instruction that is subject to the Krait errata - // - if (id->idIsKraitBranch()) - { - // Only need to handle the Krait Hazard when we are Jitting - // - if ((emitComp->opts.eeFlags & CORJIT_FLG_PREJIT) == 0) - { - // Have we seen the necessary number of T1 instructions? - if (emitCurInstrCntT1 >= MAX_INSTR_COUNT_T1) - { - return true; /* Assume that we need to add a nopw as well */ - } - } - } - return false; -} - -#endif - /***************************************************************************** * * Add an instruction with no operands. @@ -3304,10 +3276,6 @@ void emitter::emitIns_R_R_R_I(instruction ins, id->idReg2(reg2); id->idReg3(reg3); -#ifdef ARM_HAZARD_AVOIDANCE - id->idKraitNop(emitKraitHazardActive(id)); -#endif - dispIns(id); appendToCurIG(id); } @@ -4234,10 +4202,6 @@ void emitter::emitIns_J(instruction ins, BasicBlock* dst, int instrCount /* = 0 } } -#ifdef ARM_HAZARD_AVOIDANCE - id->idKraitNop(emitKraitHazardActive(id)); -#endif - dispIns(id); appendToCurIG(id); } @@ -4626,11 +4590,6 @@ void emitter::emitIns_Call(EmitCallType callType, id->idSetIsDspReloc(); } #endif - -#ifdef ARM_HAZARD_AVOIDANCE - // Unconditional calls/branches may need nop.w for Krait errata - id->idKraitNop(emitKraitHazardActive(id)); -#endif } #ifdef DEBUG @@ -5229,21 +5188,6 @@ BYTE* emitter::emitOutputLJ(insGroup* ig, BYTE* dst, instrDesc* i) /* For forward jumps, record the address of the distance value */ id->idjTemp.idjAddr = (dstOffs > srcOffs) ? dst : NULL; -#ifdef ARM_HAZARD_AVOIDANCE - if (id->idKraitNop()) - { - // This is a pseudo-format representing a 32-bit nop followed by unconditional branch. - // First emit the nop - - dst = emitOutputNOP(dst, INS_nopw, IF_T2_A); - - // The distVal was computed based on the beginning of the pseudo-instruction, which is - // the 32-bit nop. So subtract the size of the nop from the offset, so it is relative to the - // unconditional branch. - distVal -= 4; - } -#endif - if (fmt == IF_LARGEJMP) { // This is a pseudo-instruction format representing a large conditional branch, to allow @@ -5820,15 +5764,6 @@ size_t emitter::emitOutputInstr(insGroup* ig, instrDesc* id, BYTE** dp) case IF_T2_E0: // T2_E0 ............nnnn tttt......shmmmm R1 R2 R3 imm2 case IF_T2_E1: // T2_E1 ............nnnn tttt............ R1 R2 case IF_T2_E2: // T2_E2 ................ tttt............ R1 -#ifdef ARM_HAZARD_AVOIDANCE - if (id->idKraitNop()) - { - // This is a pseudo-format representing a 32-bit nop followed by ldr pc - // First emit the nop - - dst = emitOutputNOP(dst, INS_nopw, IF_T2_A); - } -#endif code = emitInsCode(ins, fmt); code |= insEncodeRegT2_T(id->idReg1()); if (fmt == IF_T2_E0) @@ -6255,16 +6190,6 @@ size_t emitter::emitOutputInstr(insGroup* ig, instrDesc* id, BYTE** dp) case IF_T2_J3: // T2_J3 .....Siiiiiiiiii ..j.jiiiiiiiiii. Call imm24 -#ifdef ARM_HAZARD_AVOIDANCE - if (id->idKraitNop()) - { - // This is a pseudo-format representing a 32-bit nop followed by unconditional call. - // First emit the nop - - dst = emitOutputNOP(dst, INS_nopw, IF_T2_A); - } -#endif - /* Is this a "fat" call descriptor? */ if (id->idIsLargeCall()) @@ -7480,42 +7405,6 @@ void emitter::emitDispIns( { insFormat fmt = id->idInsFmt(); -#ifdef ARM_HAZARD_AVOIDANCE - if (id->idKraitNop()) - { - assert(id->idIsKraitBranch()); - - // We will display a nop.w unless we have an unbound INS_b instruction - // - // Most unbound INS_b instructions will be resolve to short jumps and thus - // we don't display the nop.w while they are unbound. If they are bound and - // are still marked with idKraitNop they will display the nop.w - // - if ((id->idIns() != INS_b) || id->idIsBound()) - { - // First, display INS_nopw. Construct a temporary instrDesc to represent it, since - // there doesn't exist an actual one in the stream. - - instrDesc idNOP; - memset(&idNOP, 0, sizeof(idNOP)); - - instrDesc* pidNOP = &idNOP; - - pidNOP->idIns(INS_nopw); - pidNOP->idInsFmt(IF_T2_A); - pidNOP->idInsSize(emitInsSize(IF_T2_A)); - pidNOP->idDebugOnlyInfo(id->idDebugOnlyInfo()); // share the idDebugOnlyInfo() field - - size_t nopSizeOrZero = (code == NULL) ? 0 : 4; // NOPW is 4 bytes - emitDispInsHelp(pidNOP, false, doffs, asmfm, offset, code, nopSizeOrZero, ig); - - code += nopSizeOrZero; - sz -= nopSizeOrZero; - offset += 4; - } - } -#endif - /* Special-case IF_LARGEJMP */ if ((fmt == IF_LARGEJMP) && id->idIsBound()) diff --git a/src/coreclr/src/jit/emitarm.h b/src/coreclr/src/jit/emitarm.h index 69cae81..1440148 100644 --- a/src/coreclr/src/jit/emitarm.h +++ b/src/coreclr/src/jit/emitarm.h @@ -240,9 +240,6 @@ static bool emitIns_valid_imm_for_mov(int imm); static bool emitIns_valid_imm_for_small_mov(regNumber reg, int imm, insFlags flags); static bool emitIns_valid_imm_for_add(int imm, insFlags flags); static bool emitIns_valid_imm_for_add_sp(int imm); -#ifdef ARM_HAZARD_AVOIDANCE -bool emitKraitHazardActive(instrDesc* id); -#endif void emitIns(instruction ins); diff --git a/src/coreclr/src/jit/target.h b/src/coreclr/src/jit/target.h index 54c6674..fa0b18a 100644 --- a/src/coreclr/src/jit/target.h +++ b/src/coreclr/src/jit/target.h @@ -1145,19 +1145,6 @@ typedef unsigned short regPairNoSmall; // arm: need 12 bits #elif defined(_TARGET_ARM_) - #define ARM_HAZARD_AVOIDANCE // Avoid ARM hazard due to QualComm Krait processor bug. -/* - Krait Errata definition: - - The problem occurs if following code pattern occurs starting at an address ending in FB8: - Address -0x*****FB8 T16 instruction -0x*****FBA T16 instruction -0x*****FBC T16 instruction -0x*****FBE T32 unconditional pc relative branch (spans 2 cache lines in sets 62 and 63) - -*/ - // TODO-ARM-CQ: Use shift for division by power of 2 // TODO-ARM-CQ: Check for sdiv/udiv at runtime and generate it if available #define USE_HELPERS_FOR_INT_DIV 1 // BeagleBoard (ARMv7A) doesn't support SDIV/UDIV diff --git a/src/coreclr/src/zap/zapcode.cpp b/src/coreclr/src/zap/zapcode.cpp index fcb1459..adad361 100644 --- a/src/coreclr/src/zap/zapcode.cpp +++ b/src/coreclr/src/zap/zapcode.cpp @@ -805,144 +805,7 @@ void ZapImage::AddRelocsForEHClauses(ZapExceptionInfo * pExceptionInfo) // ZapMethodHeader // -#ifdef _TARGET_ARM_ -// Avoid ARM hazard due to QualComm Krait processor bug. -#define ARM_HAZARD_AVOIDANCE -#endif - -#ifdef ARM_HAZARD_AVOIDANCE - -// -// This code was stolen from the C++ linker (vctools\Link\src\arm.cpp) -// - -bool F32BitInstr(PBYTE pbInstr) -{ - const WORD wInstr = *((const WORD UNALIGNED *) pbInstr); - return (wInstr >> 11) >= 0x1D; -} - -bool FHazardCandidate(PBYTE pbInstr) - -/*++ - -Routine Description: - - If the following 4-instruction/10-byte pattern begins at the 0xFB8 offset - from a page, then it would cause the hazard. - - Address Contents - ------------------------------ - 0x*****FB8 T16 instruction - 0x*****FBA T16 instruction - 0x*****FBC T16 instruction - 0x*****FBE T32 instruction that can branch - - Finding all such instruction sequences could be very difficult, because - the function could have literal's which are mixed with instructions and - for now we don't have an easy way to find where literal is. - - So instead of detecting such instruction sequence, we assume an instr - starts at 0x*****FB8 and check whether such sequence exists. This may - introduce false positive's. - -Arguments: - - pbCon - pointer to the instruction at 0x*****FB8 - -Return Value: - - false no hazard true otherwise - ---*/ - -{ - // Check whether there are three 16-bit instructions. - - for (int i = 0; i < 3; i++) { - if (F32BitInstr(pbInstr)) { - return false; - } - - pbInstr += sizeof(WORD); - } - - // Check whether next is a 32-bit unconditional PC relative branch. - - if (!F32BitInstr(pbInstr)) { - return false; - } - - // Check 32-bit branch. - - const DWORD dwInstr = *((const DWORD UNALIGNED *) pbInstr); - - return - // B (A8.6.16, encoding T3) - - ((dwInstr & 0xD000F800) == 0x8000F000 && (dwInstr & 0x3C0) != 0x380) || - - // B (A8.6.16, encoding T4) - - ((dwInstr & 0xD000F800) == 0x9000F000) || - - // BL (A8.6.23, encoding T1) - - ((dwInstr & 0xD000F800) == 0xD000F000) || - - // BLX (A8.6.23, encoding T2) - - ((dwInstr & 0xD001F800) == 0xC000F000) || - - // BXJ (A8.6.26, encoding T1) - - ((dwInstr & 0xFFFFFFF0) == 0x8F00F3C0) || - - // LDM/LDMIA/LDMFD, with PC in target reg list (A8.6.53, encoding T2) - - ((dwInstr & 0xA000FFD0) == 0x8000E890 && (dwInstr & 0x02F) != 0x02D) || - - // LDMDB/LDMEA, with PC in target reg list (A.8.6.55, encoding T1) - - ((dwInstr & 0xA000FFD0) == 0x8000E910) || - - // LDR immediate, with PC as target reg (A8.6.57, encoding T3) - - ((dwInstr & 0xF000FFF0) == 0xF000F8D0 && (dwInstr & 0x0F) != 0x0F) || - - // LDR immediate, with PC as target reg (A8.6.57, encoding T4) - - ((dwInstr & 0xF800FFF0) == 0xF800F850 && (dwInstr & 0x0F) != 0x0F) || - - // LDR literal, with PC as target reg (A8.6.59, encoding T2) - - ((dwInstr & 0xF000FF7F) == 0xF000F85F) || - - // LDR register, with PC as target reg (A8.6.60, encoding T2) - - ((dwInstr & 0xFFC0FFF0) == 0xF000F850) || - - // POP, with PC in target reg list (A8.6.122, encoding T2) - - ((dwInstr & 0xA000FFFF) == 0x8000E8BD) || - - // POP, with PC as target reg (A8.6.122, encoding T3) - - (dwInstr == 0xFB04F85D) || - - // TBB/TBH (A8.6.226, encoding T1) - - ((dwInstr & 0xFFE0FFF0) == 0xF000E8D0); -} - -// -// End of code stolen from the C++ linker -// - -#endif // ARM_HAZARD_AVOIDANCE - - -#if defined(_TARGET_X86_) || defined(ARM_HAZARD_AVOIDANCE) +#if defined(_TARGET_X86_) DWORD ZapCodeBlob::ComputeRVA(ZapWriter * pZapWriter, DWORD dwPos) { @@ -982,55 +845,6 @@ DWORD ZapCodeBlob::ComputeRVA(ZapWriter * pZapWriter, DWORD dwPos) return dwPaddedPos + size; #endif // _TARGET_X86_ - -#ifdef ARM_HAZARD_AVOIDANCE - for (DWORD dwPadding = 0; dwPadding < 0x1000; dwPadding += dwAlignment) - { - DWORD dwPaddedPos = dwPos + dwPadding; - - BOOL fHasHazard = FALSE; - - // - // Go through all possible places where the hazard may occur within the data block - // - - // The possible occurences of the hazard are always at offset 0xFB8 within the 4k page. - // Start with the first page containing the block. - DWORD dwFirstHazardPos = AlignDown(dwPaddedPos, 0x1000) + 0xFB8; - if (dwFirstHazardPos < dwPaddedPos) - dwFirstHazardPos += 0x1000; - - // The last possible occurence of the hazard is 10 bytes before the end of the block - DWORD dwLastHazardPos = (dwPaddedPos + size) - 10; - - for (DWORD dwHazardPos = dwFirstHazardPos; dwHazardPos <= dwLastHazardPos; dwHazardPos += 0x1000) - { - int offset = dwHazardPos - dwPaddedPos; - - if (FHazardCandidate((BYTE * )pData + offset)) - { - fHasHazard = TRUE; - break; - } - } - - if (!fHasHazard) - { - SetRVA(dwPaddedPos); - - return dwPaddedPos + size; - } - } - - // There is a theoretical chance that we may not be able to find a suitable padding - // to workaround the bug. In this case don't attempt to workaround the bug, - // and simply place the code at the next natural RVM. It should happen - // very rarely for very large methods only, and there should be no retail devices - // with this processor bug. - SetRVA(dwPos); - - return dwPos + size; -#endif } template diff --git a/src/coreclr/src/zap/zapcode.h b/src/coreclr/src/zap/zapcode.h index ed8cc2d..2fe949e 100644 --- a/src/coreclr/src/zap/zapcode.h +++ b/src/coreclr/src/zap/zapcode.h @@ -187,12 +187,7 @@ public: }; -#ifdef _TARGET_ARM_ -// Avoid ARM hazard due to QualComm Krait processor bug. -#define ARM_HAZARD_AVOIDANCE -#endif - -#if defined(_TARGET_X86_) || defined(ARM_HAZARD_AVOIDANCE) +#if defined(_TARGET_X86_) class ZapCodeBlob : public ZapBlobWithRelocs { protected: -- 2.7.4