From 5718b54e2b069a20e0a9c5f2d15ff2a6d1ea1e72 Mon Sep 17 00:00:00 2001 From: Gleb Balykov Date: Mon, 7 Aug 2017 13:33:58 +0300 Subject: [PATCH 28/32] Review fixes FIX: fix No.6, rebased --- src/ToolBox/superpmi/superpmi-shared/icorjitinfoimpl.h | 2 +- src/ToolBox/superpmi/superpmi-shared/methodcontext.cpp | 10 +++++----- src/ToolBox/superpmi/superpmi-shared/methodcontext.h | 6 +++--- .../superpmi/superpmi-shim-collector/icorjitinfo.cpp | 2 +- .../superpmi/superpmi-shim-counter/icorjitinfo.cpp | 2 +- src/ToolBox/superpmi/superpmi-shim-simple/icorjitinfo.cpp | 2 +- src/ToolBox/superpmi/superpmi/icorjitinfo.cpp | 2 +- src/inc/corinfo.h | 2 +- src/jit/ICorJitInfo_API_wrapper.hpp | 2 +- src/jit/codegenlegacy.cpp | 8 ++++---- src/jit/emitarm.cpp | 6 +++--- src/jit/lower.cpp | 15 +++++++++++++-- src/jit/morph.cpp | 2 +- src/vm/jitinterface.cpp | 2 +- src/vm/jitinterface.h | 2 +- src/zap/zapinfo.cpp | 2 +- src/zap/zapinfo.h | 2 +- 17 files changed, 40 insertions(+), 29 deletions(-) diff --git a/src/ToolBox/superpmi/superpmi-shared/icorjitinfoimpl.h b/src/ToolBox/superpmi/superpmi-shared/icorjitinfoimpl.h index 44b81aa..e38a66b 100644 --- a/src/ToolBox/superpmi/superpmi-shared/icorjitinfoimpl.h +++ b/src/ToolBox/superpmi/superpmi-shared/icorjitinfoimpl.h @@ -110,7 +110,7 @@ CORINFO_MODULE_HANDLE getMethodModule(CORINFO_METHOD_HANDLE method); void getMethodVTableOffset(CORINFO_METHOD_HANDLE method, /* IN */ unsigned* offsetOfIndirection, /* OUT */ unsigned* offsetAfterIndirection,/* OUT */ - unsigned* isRelative /* OUT */ + bool* isRelative /* OUT */ ); // Find the virtual method in implementingClass that overrides virtualMethod. diff --git a/src/ToolBox/superpmi/superpmi-shared/methodcontext.cpp b/src/ToolBox/superpmi/superpmi-shared/methodcontext.cpp index f4130e9..bac6004 100644 --- a/src/ToolBox/superpmi/superpmi-shared/methodcontext.cpp +++ b/src/ToolBox/superpmi/superpmi-shared/methodcontext.cpp @@ -3383,7 +3383,7 @@ void MethodContext::repGetEHinfo(CORINFO_METHOD_HANDLE ftn, unsigned EHnumber, C void MethodContext::recGetMethodVTableOffset(CORINFO_METHOD_HANDLE method, unsigned* offsetOfIndirection, unsigned* offsetAfterIndirection, - unsigned* isRelative) + bool* isRelative) { if (GetMethodVTableOffset == nullptr) GetMethodVTableOffset = new LightWeightMap(); @@ -3391,18 +3391,18 @@ void MethodContext::recGetMethodVTableOffset(CORINFO_METHOD_HANDLE method, DDD value; value.A = (DWORD)*offsetOfIndirection; value.B = (DWORD)*offsetAfterIndirection; - value.C = (DWORD)*isRelative; + value.C = *isRelative; GetMethodVTableOffset->Add((DWORDLONG)method, value); DEBUG_REC(dmpGetMethodVTableOffset((DWORDLONG)method, value)); } void MethodContext::dmpGetMethodVTableOffset(DWORDLONG key, DDD value) { - printf("GetMethodVTableOffset key ftn-%016llX, value offi-%u, offa-%u", key, value.A, value.B); + printf("GetMethodVTableOffset key ftn-%016llX, value offi-%u, offa-%u. offr-%d", key, value.A, value.B, value.C); } void MethodContext::repGetMethodVTableOffset(CORINFO_METHOD_HANDLE method, unsigned* offsetOfIndirection, unsigned* offsetAfterIndirection, - unsigned* isRelative) + bool* isRelative) { DDD value; @@ -3414,7 +3414,7 @@ void MethodContext::repGetMethodVTableOffset(CORINFO_METHOD_HANDLE method, *offsetOfIndirection = (unsigned)value.A; *offsetAfterIndirection = (unsigned)value.B; - *isRelative = (unsigned)value.C; + *isRelative = value.C; DEBUG_REP(dmpGetMethodVTableOffset((DWORDLONG)method, value)); } diff --git a/src/ToolBox/superpmi/superpmi-shared/methodcontext.h b/src/ToolBox/superpmi/superpmi-shared/methodcontext.h index a8612b5..70e04ea 100644 --- a/src/ToolBox/superpmi/superpmi-shared/methodcontext.h +++ b/src/ToolBox/superpmi/superpmi-shared/methodcontext.h @@ -210,7 +210,7 @@ public: { DWORD A; DWORD B; - DWORD C; + bool C; }; struct Agnostic_CanTailCall { @@ -781,12 +781,12 @@ public: void recGetMethodVTableOffset(CORINFO_METHOD_HANDLE method, unsigned* offsetOfIndirection, unsigned* offsetAfterIndirection, - unsigned* isRelative); + bool* isRelative); void dmpGetMethodVTableOffset(DWORDLONG key, DDD value); void repGetMethodVTableOffset(CORINFO_METHOD_HANDLE method, unsigned* offsetOfIndirection, unsigned* offsetAfterIndirection, - unsigned* isRelative); + bool* isRelative); void recResolveVirtualMethod(CORINFO_METHOD_HANDLE virtMethod, CORINFO_CLASS_HANDLE implClass, diff --git a/src/ToolBox/superpmi/superpmi-shim-collector/icorjitinfo.cpp b/src/ToolBox/superpmi/superpmi-shim-collector/icorjitinfo.cpp index 1f81883..a02c88d 100644 --- a/src/ToolBox/superpmi/superpmi-shim-collector/icorjitinfo.cpp +++ b/src/ToolBox/superpmi/superpmi-shim-collector/icorjitinfo.cpp @@ -215,7 +215,7 @@ CORINFO_MODULE_HANDLE interceptor_ICJI::getMethodModule(CORINFO_METHOD_HANDLE me void interceptor_ICJI::getMethodVTableOffset(CORINFO_METHOD_HANDLE method, /* IN */ unsigned* offsetOfIndirection, /* OUT */ unsigned* offsetAfterIndirection,/* OUT */ - unsigned* isRelative /* OUT */ + bool* isRelative /* OUT */ ) { mc->cr->AddCall("getMethodVTableOffset"); diff --git a/src/ToolBox/superpmi/superpmi-shim-counter/icorjitinfo.cpp b/src/ToolBox/superpmi/superpmi-shim-counter/icorjitinfo.cpp index 5c2e784..5f02638 100644 --- a/src/ToolBox/superpmi/superpmi-shim-counter/icorjitinfo.cpp +++ b/src/ToolBox/superpmi/superpmi-shim-counter/icorjitinfo.cpp @@ -146,7 +146,7 @@ CORINFO_MODULE_HANDLE interceptor_ICJI::getMethodModule(CORINFO_METHOD_HANDLE me void interceptor_ICJI::getMethodVTableOffset(CORINFO_METHOD_HANDLE method, /* IN */ unsigned* offsetOfIndirection, /* OUT */ unsigned* offsetAfterIndirection,/* OUT */ - unsigned* isRelative /* OUT */ + bool* isRelative /* OUT */ ) { mcs->AddCall("getMethodVTableOffset"); diff --git a/src/ToolBox/superpmi/superpmi-shim-simple/icorjitinfo.cpp b/src/ToolBox/superpmi/superpmi-shim-simple/icorjitinfo.cpp index df223f4..5f89a87 100644 --- a/src/ToolBox/superpmi/superpmi-shim-simple/icorjitinfo.cpp +++ b/src/ToolBox/superpmi/superpmi-shim-simple/icorjitinfo.cpp @@ -135,7 +135,7 @@ CORINFO_MODULE_HANDLE interceptor_ICJI::getMethodModule(CORINFO_METHOD_HANDLE me void interceptor_ICJI::getMethodVTableOffset(CORINFO_METHOD_HANDLE method, /* IN */ unsigned* offsetOfIndirection, /* OUT */ unsigned* offsetAfterIndirection,/* OUT */ - unsigned* isRelative /* OUT */ + bool* isRelative /* OUT */ ) { original_ICorJitInfo->getMethodVTableOffset(method, offsetOfIndirection, offsetAfterIndirection, isRelative); diff --git a/src/ToolBox/superpmi/superpmi/icorjitinfo.cpp b/src/ToolBox/superpmi/superpmi/icorjitinfo.cpp index dc73a75..dd974d1 100644 --- a/src/ToolBox/superpmi/superpmi/icorjitinfo.cpp +++ b/src/ToolBox/superpmi/superpmi/icorjitinfo.cpp @@ -166,7 +166,7 @@ CORINFO_MODULE_HANDLE MyICJI::getMethodModule(CORINFO_METHOD_HANDLE method) void MyICJI::getMethodVTableOffset(CORINFO_METHOD_HANDLE method, /* IN */ unsigned* offsetOfIndirection, /* OUT */ unsigned* offsetAfterIndirection,/* OUT */ - unsigned* isRelative /* OUT */ + bool* isRelative /* OUT */ ) { jitInstance->mc->cr->AddCall("getMethodVTableOffset"); diff --git a/src/inc/corinfo.h b/src/inc/corinfo.h index 1489a74..63ade7f 100644 --- a/src/inc/corinfo.h +++ b/src/inc/corinfo.h @@ -2068,7 +2068,7 @@ public: CORINFO_METHOD_HANDLE method, /* IN */ unsigned* offsetOfIndirection, /* OUT */ unsigned* offsetAfterIndirection, /* OUT */ - unsigned* isRelative /* OUT */ + bool* isRelative /* OUT */ ) = 0; // Find the virtual method in implementingClass that overrides virtualMethod, diff --git a/src/jit/ICorJitInfo_API_wrapper.hpp b/src/jit/ICorJitInfo_API_wrapper.hpp index 8e0d1df..0d3bf5e 100644 --- a/src/jit/ICorJitInfo_API_wrapper.hpp +++ b/src/jit/ICorJitInfo_API_wrapper.hpp @@ -123,7 +123,7 @@ void WrapICorJitInfo::getMethodVTableOffset( CORINFO_METHOD_HANDLE method, /* IN */ unsigned* offsetOfIndirection, /* OUT */ unsigned* offsetAfterIndirection, /* OUT */ - unsigned* isRelative /* OUT */) + bool* isRelative /* OUT */) { API_ENTER(getMethodVTableOffset); wrapHnd->getMethodVTableOffset(method, offsetOfIndirection, offsetAfterIndirection, isRelative); diff --git a/src/jit/codegenlegacy.cpp b/src/jit/codegenlegacy.cpp index a925c97..53c8f8d 100644 --- a/src/jit/codegenlegacy.cpp +++ b/src/jit/codegenlegacy.cpp @@ -18890,7 +18890,7 @@ regMaskTP CodeGen::genCodeForCall(GenTreeCall* call, bool valUsed) regMaskTP vptrMask1; unsigned vtabOffsOfIndirection; unsigned vtabOffsAfterIndirection; - unsigned isRelative; + bool isRelative; noway_assert(callType == CT_USER_FUNC); @@ -18944,7 +18944,7 @@ regMaskTP CodeGen::genCodeForCall(GenTreeCall* call, bool valUsed) // ADD vptrReg1, REG_CALL_IND_SCRATCH, vtabOffsOfIndirection + vtabOffsAfterIndirection getEmitter()->emitIns_R_R_I(INS_add, EA_PTRSIZE, vptrReg1, vptrReg, offset); #else - _ASSERTE(false); + unreached(); #endif } @@ -18963,7 +18963,7 @@ regMaskTP CodeGen::genCodeForCall(GenTreeCall* call, bool valUsed) getEmitter()->emitIns_R_ARR(ins_Load(TYP_I_IMPL), EA_PTRSIZE, REG_TAILCALL_ADDR, vptrReg1, vptrReg, 0); #else - _ASSERTE(false); + unreached(); #endif } else @@ -18993,7 +18993,7 @@ regMaskTP CodeGen::genCodeForCall(GenTreeCall* call, bool valUsed) gcInfo.gcRegByrefSetCur, ilOffset, vptrReg); // ireg #else - _ASSERTE(!isRelative); + assert(!isRelative); getEmitter()->emitIns_Call(emitter::EC_FUNC_VIRTUAL, call->gtCallMethHnd, INDEBUG_LDISASM_COMMA(sigInfo) NULL, // addr args, retSize, gcInfo.gcVarPtrSetCur, gcInfo.gcRegGCrefSetCur, diff --git a/src/jit/emitarm.cpp b/src/jit/emitarm.cpp index e765af7..ba89c9b 100644 --- a/src/jit/emitarm.cpp +++ b/src/jit/emitarm.cpp @@ -2446,13 +2446,13 @@ void emitter::emitIns_R_R_I(instruction ins, fmt = IF_T2_M0; sf = INS_FLAGS_NOT_SET; } - else if (insDoesNotSetFlags(flags) && reg1 != REG_SP && reg1 != REG_PC) + else if (insDoesNotSetFlags(flags) && (reg1 != REG_SP) && (reg1 != REG_PC)) { // movw,movt reg1, imm - codeGen->instGen_Set_Reg_To_Imm(attr, reg1, imm); + codeGen->instGen_Set_Reg_To_Imm(attr, reg1, (ins == INS_sub ? -1 : 1) * imm); // ins reg1, reg2 - emitIns_R_R(ins, attr, reg1, reg2); + emitIns_R_R(INS_add, attr, reg1, reg2); return; } diff --git a/src/jit/lower.cpp b/src/jit/lower.cpp index d154f68..c06dcb6 100644 --- a/src/jit/lower.cpp +++ b/src/jit/lower.cpp @@ -3404,7 +3404,7 @@ GenTree* Lowering::LowerVirtualVtableCall(GenTreeCall* call) // Get hold of the vtable offset (note: this might be expensive) unsigned vtabOffsOfIndirection; unsigned vtabOffsAfterIndirection; - unsigned isRelative; + bool isRelative; comp->info.compCompHnd->getMethodVTableOffset(call->gtCallMethHnd, &vtabOffsOfIndirection, &vtabOffsAfterIndirection, &isRelative); @@ -3426,13 +3426,24 @@ GenTree* Lowering::LowerVirtualVtableCall(GenTreeCall* call) // Get the appropriate vtable chunk if (isRelative) { + // MethodTable offset is a relative pointer. + // + // Additional temporary variable is used to store virtual table pointer. + // Address of method is obtained by the next computations: + // + // Save relative offset to tmp (vtab is virtual table pointer, vtabOffsOfIndirection is offset of + // vtable-1st-level-indirection): + // tmp = [vtab + vtabOffsOfIndirection] + // + // Save address of method to result (vtabOffsAfterIndirection is offset of vtable-2nd-level-indirection): + // result = [vtab + vtabOffsOfIndirection + vtabOffsAfterIndirection + tmp] unsigned lclNumTmp = comp->lvaGrabTemp(true DEBUGARG("lclNumTmp")); comp->lvaTable[lclNumTmp].incRefCnts(comp->compCurBB->getBBWeight(comp), comp); GenTree* lclvNodeStore = comp->gtNewTempAssign(lclNumTmp, result); LIR::Range range = LIR::SeqTree(comp, lclvNodeStore); - JITDUMP("results of lowering call interm:\n"); + JITDUMP("result of obtaining pointer to virtual table:\n"); DISPRANGE(range); BlockRange().InsertBefore(call, std::move(range)); diff --git a/src/jit/morph.cpp b/src/jit/morph.cpp index c5d1ff2..79b3fef 100644 --- a/src/jit/morph.cpp +++ b/src/jit/morph.cpp @@ -7116,7 +7116,7 @@ void Compiler::fgMorphTailCall(GenTreeCall* call) unsigned vtabOffsOfIndirection; unsigned vtabOffsAfterIndirection; - unsigned isRelative; + bool isRelative; info.compCompHnd->getMethodVTableOffset(call->gtCallMethHnd, &vtabOffsOfIndirection, &vtabOffsAfterIndirection, &isRelative); diff --git a/src/vm/jitinterface.cpp b/src/vm/jitinterface.cpp index 72f4131..9cefd10 100644 --- a/src/vm/jitinterface.cpp +++ b/src/vm/jitinterface.cpp @@ -8725,7 +8725,7 @@ CONTRACTL { void CEEInfo::getMethodVTableOffset (CORINFO_METHOD_HANDLE methodHnd, unsigned * pOffsetOfIndirection, unsigned * pOffsetAfterIndirection, - unsigned * isRelative) + bool * isRelative) { CONTRACTL { SO_TOLERANT; diff --git a/src/vm/jitinterface.h b/src/vm/jitinterface.h index cf1097c..a906a0f 100644 --- a/src/vm/jitinterface.h +++ b/src/vm/jitinterface.h @@ -728,7 +728,7 @@ public: CORINFO_METHOD_HANDLE methodHnd, unsigned * pOffsetOfIndirection, unsigned * pOffsetAfterIndirection, - unsigned * isRelative); + bool * isRelative); CORINFO_METHOD_HANDLE resolveVirtualMethod( CORINFO_METHOD_HANDLE virtualMethod, diff --git a/src/zap/zapinfo.cpp b/src/zap/zapinfo.cpp index 507cc25..19247dd 100644 --- a/src/zap/zapinfo.cpp +++ b/src/zap/zapinfo.cpp @@ -3710,7 +3710,7 @@ CORINFO_MODULE_HANDLE ZapInfo::getMethodModule(CORINFO_METHOD_HANDLE method) void ZapInfo::getMethodVTableOffset(CORINFO_METHOD_HANDLE method, unsigned * pOffsetOfIndirection, unsigned * pOffsetAfterIndirection, - unsigned * isRelative) + bool * isRelative) { m_pEEJitInfo->getMethodVTableOffset(method, pOffsetOfIndirection, pOffsetAfterIndirection, isRelative); } diff --git a/src/zap/zapinfo.h b/src/zap/zapinfo.h index 65c1ddd..afa50c7 100644 --- a/src/zap/zapinfo.h +++ b/src/zap/zapinfo.h @@ -664,7 +664,7 @@ public: void getMethodVTableOffset(CORINFO_METHOD_HANDLE method, unsigned * pOffsetOfIndirection, unsigned * pOffsetAfterIndirection, - unsigned * isRelative); + bool * isRelative); CORINFO_METHOD_HANDLE resolveVirtualMethod( CORINFO_METHOD_HANDLE virtualMethod, -- 2.7.4