diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index eed9a96a981724..1f9a2e4a646227 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -893,8 +893,8 @@ void CodeGen::genLogLabel(BasicBlock* bb) void CodeGen::genDefineTempLabel(BasicBlock* label) { genLogLabel(label); - label->bbEmitCookie = GetEmitter()->emitAddLabel(gcInfo.gcVarPtrSetCur, gcInfo.gcRegGCrefSetCur, - gcInfo.gcRegByrefSetCur DEBUG_ARG(label)); + label->bbEmitCookie = + GetEmitter()->emitAddLabel(gcInfo.gcVarPtrSetCur, gcInfo.gcRegGCrefSetCur, gcInfo.gcRegByrefSetCur); } // genDefineInlineTempLabel: Define an inline label that does not affect the GC diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index 0011b0add280a2..60d825c928c8e0 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -348,7 +348,7 @@ void CodeGen::genCodeForBBlist() // Mark a label and update the current set of live GC refs block->bbEmitCookie = GetEmitter()->emitAddLabel(gcInfo.gcVarPtrSetCur, gcInfo.gcRegGCrefSetCur, - gcInfo.gcRegByrefSetCur DEBUG_ARG(block)); + gcInfo.gcRegByrefSetCur, block->Prev()); } if (block->IsFirstColdBlock(compiler)) diff --git a/src/coreclr/jit/emit.cpp b/src/coreclr/jit/emit.cpp index 838cf5677aa154..f3634e056edc8d 100644 --- a/src/coreclr/jit/emit.cpp +++ b/src/coreclr/jit/emit.cpp @@ -2232,6 +2232,10 @@ void emitter::emitCreatePlaceholderIG(insGroupPlaceholderType igType, emitCurIG->igFlags &= ~IGF_PROPAGATE_MASK; } + // since we have emitted a placeholder, the last ins is not longer the last. + emitLastIns = nullptr; + emitLastInsIG = nullptr; + #ifdef DEBUG if (emitComp->verbose) { @@ -2895,10 +2899,36 @@ bool emitter::emitNoGChelper(CORINFO_METHOD_HANDLE methHnd) * Mark the current spot as having a label. */ -void* emitter::emitAddLabel(VARSET_VALARG_TP GCvars, - regMaskTP gcrefRegs, - regMaskTP byrefRegs DEBUG_ARG(BasicBlock* block)) +void* emitter::emitAddLabel(VARSET_VALARG_TP GCvars, regMaskTP gcrefRegs, regMaskTP byrefRegs, BasicBlock* prevBlock) { + if (emitLastInsIsCallWithGC()) + { + // We have just emitted a call that can do GC and conservatively recorded what is alive after the call. + // Now we see that the next instruction may be reachable by a branch with a different liveness. + // We want to maintain the invariant that the GC info at IP after a GC-capable call is the same + // regardless how it is reached. + // One way to fix this is to fish out the call instruction and patch its GC info, but we must be + // certain that the current IP is indeed reachable after the call. + // Another way it to add an instruction (NOP or BRK) after the call. + if (emitThisGCrefRegs != gcrefRegs || emitThisByrefRegs != byrefRegs || + !VarSetOps::Equal(emitComp, emitThisGCrefVars, GCvars)) + { + if (prevBlock->KindIs(BBJ_THROW)) + { + emitIns(INS_BREAKPOINT); + } + else + { + // other block kinds should emit something at the end that is not a call. + assert(prevBlock->KindIs(BBJ_ALWAYS)); + // CONSIDER: We could patch up the previous call instruction with new GC info instead. + // But that will need to be coordinated with how the GC info vor variables is used. + // We currently apply that info to the instruction before the call. It may need to change. + emitIns(INS_nop); + } + } + } + /* Create a new IG if the current one is non-empty */ if (emitCurIGnonEmpty()) @@ -3663,6 +3693,7 @@ emitter::instrDesc* emitter::emitNewInstrCallInd(int argCnt, /* Make sure we didn't waste space unexpectedly */ assert(!id->idIsLargeCns()); + id->idSetIsCall(); #ifdef TARGET_XARCH /* Store the displacement and make sure the value fit */ @@ -3742,6 +3773,7 @@ emitter::instrDesc* emitter::emitNewInstrCallDir(int argCnt, /* Make sure we didn't waste space unexpectedly */ assert(!id->idIsLargeCns()); + id->idSetIsCall(); /* Save the live GC registers in the unused register fields */ assert((gcrefRegs & RBM_CALLEE_TRASH) == 0); @@ -8754,6 +8786,16 @@ void emitter::emitUpdateLiveGCvars(VARSET_VALARG_TP vars, BYTE* addr) emitThisGCrefVset = true; } +/***************************************************************************** + * + * Last emitted instruction is a call and it is not a NoGC call. + */ + +bool emitter::emitLastInsIsCallWithGC() +{ + return emitLastIns != nullptr && emitLastIns->idIsCall() && !emitLastIns->idIsNoGC(); +} + /***************************************************************************** * * Record a call location for GC purposes (we know that this is a method that diff --git a/src/coreclr/jit/emit.h b/src/coreclr/jit/emit.h index 4e37226e2b5816..ab8f686fd482a2 100644 --- a/src/coreclr/jit/emit.h +++ b/src/coreclr/jit/emit.h @@ -772,10 +772,10 @@ class emitter // loongarch64: 28 bits // risc-v: 28 bits - unsigned _idSmallDsc : 1; // is this a "small" descriptor? - unsigned _idLargeCns : 1; // does a large constant follow? - unsigned _idLargeDsp : 1; // does a large displacement follow? - unsigned _idLargeCall : 1; // large call descriptor used + unsigned _idSmallDsc : 1; // is this a "small" descriptor? + unsigned _idLargeCns : 1; // does a large constant follow? (or if large call descriptor used) + unsigned _idLargeDsp : 1; // does a large displacement follow? + unsigned _idCall : 1; // this is a call // We have several pieces of information we need to encode but which are only applicable // to a subset of instrDescs. To accommodate that, we define a several _idCustom# bitfields @@ -1575,7 +1575,7 @@ class emitter bool idIsLargeCns() const { - return _idLargeCns != 0; + return _idLargeCns != 0 && !idIsCall(); } void idSetIsLargeCns() { @@ -1595,13 +1595,23 @@ class emitter _idLargeDsp = 0; } + bool idIsCall() const + { + return _idCall != 0; + } + void idSetIsCall() + { + _idCall = 1; + } + bool idIsLargeCall() const { - return _idLargeCall != 0; + return idIsCall() && _idLargeCns == 1; } void idSetIsLargeCall() { - _idLargeCall = 1; + idSetIsCall(); + _idLargeCns = 1; } bool idIsBound() const @@ -2871,9 +2881,11 @@ class emitter // Mark this instruction group as having a label; return the new instruction group. // Sets the emitter's record of the currently live GC variables // and registers. - void* emitAddLabel(VARSET_VALARG_TP GCvars, - regMaskTP gcrefRegs, - regMaskTP byrefRegs DEBUG_ARG(BasicBlock* block = nullptr)); + // prevBlock is passed when starting a new block. + void* emitAddLabel(VARSET_VALARG_TP GCvars, + regMaskTP gcrefRegs, + regMaskTP byrefRegs, + BasicBlock* prevBlock = nullptr); // Same as above, except the label is added and is conceptually "inline" in // the current block. Thus it extends the previous block and the emitter @@ -3204,6 +3216,8 @@ class emitter void emitRecordGCcall(BYTE* codePos, unsigned char callInstrSize); + bool emitLastInsIsCallWithGC(); + // Helpers for the above void emitStackPushLargeStk(BYTE* addr, GCtype gcType, unsigned count = 1); diff --git a/src/coreclr/jit/emitarm.cpp b/src/coreclr/jit/emitarm.cpp index 8e1dc878b8073c..fc6d3fd3247daf 100644 --- a/src/coreclr/jit/emitarm.cpp +++ b/src/coreclr/jit/emitarm.cpp @@ -4754,6 +4754,16 @@ void emitter::emitIns_Call(EmitCallType callType, /* Update the emitter's live GC ref sets */ + // If the method returns a GC ref, mark R0 appropriately + if (retSize == EA_GCREF) + { + gcrefRegs |= RBM_R0; + } + else if (retSize == EA_BYREF) + { + byrefRegs |= RBM_R0; + } + VarSetOps::Assign(emitComp, emitThisGCrefVars, ptrVars); emitThisGCrefRegs = gcrefRegs; emitThisByrefRegs = byrefRegs; diff --git a/src/coreclr/jit/emitarm64.cpp b/src/coreclr/jit/emitarm64.cpp index aec4b0b03de35a..9bbf46a5e64d77 100644 --- a/src/coreclr/jit/emitarm64.cpp +++ b/src/coreclr/jit/emitarm64.cpp @@ -9020,6 +9020,26 @@ void emitter::emitIns_Call(EmitCallType callType, /* Update the emitter's live GC ref sets */ + // If the method returns a GC ref, mark RBM_INTRET appropriately + if (retSize == EA_GCREF) + { + gcrefRegs |= RBM_INTRET; + } + else if (retSize == EA_BYREF) + { + byrefRegs |= RBM_INTRET; + } + + // If is a multi-register return method is called, mark RBM_INTRET_1 appropriately + if (secondRetSize == EA_GCREF) + { + gcrefRegs |= RBM_INTRET_1; + } + else if (secondRetSize == EA_BYREF) + { + byrefRegs |= RBM_INTRET_1; + } + VarSetOps::Assign(emitComp, emitThisGCrefVars, ptrVars); emitThisGCrefRegs = gcrefRegs; emitThisByrefRegs = byrefRegs; diff --git a/src/coreclr/jit/emitloongarch64.cpp b/src/coreclr/jit/emitloongarch64.cpp index 3e2d54748f4b84..419fa424077e5b 100644 --- a/src/coreclr/jit/emitloongarch64.cpp +++ b/src/coreclr/jit/emitloongarch64.cpp @@ -2464,6 +2464,26 @@ void emitter::emitIns_Call(EmitCallType callType, /* Update the emitter's live GC ref sets */ + // If the method returns a GC ref, mark RBM_INTRET appropriately + if (retSize == EA_GCREF) + { + gcrefRegs |= RBM_INTRET; + } + else if (retSize == EA_BYREF) + { + byrefRegs |= RBM_INTRET; + } + + // If is a multi-register return method is called, mark RBM_INTRET_1 appropriately + if (secondRetSize == EA_GCREF) + { + gcrefRegs |= RBM_INTRET_1; + } + else if (secondRetSize == EA_BYREF) + { + byrefRegs |= RBM_INTRET_1; + } + VarSetOps::Assign(emitComp, emitThisGCrefVars, ptrVars); emitThisGCrefRegs = gcrefRegs; emitThisByrefRegs = byrefRegs; diff --git a/src/coreclr/jit/emitriscv64.cpp b/src/coreclr/jit/emitriscv64.cpp index 8f7618023db55e..08de8a43e7041c 100644 --- a/src/coreclr/jit/emitriscv64.cpp +++ b/src/coreclr/jit/emitriscv64.cpp @@ -1373,6 +1373,26 @@ void emitter::emitIns_Call(EmitCallType callType, /* Update the emitter's live GC ref sets */ + // If the method returns a GC ref, mark RBM_INTRET appropriately + if (retSize == EA_GCREF) + { + gcrefRegs |= RBM_INTRET; + } + else if (retSize == EA_BYREF) + { + byrefRegs |= RBM_INTRET; + } + + // If is a multi-register return method is called, mark RBM_INTRET_1 appropriately + if (secondRetSize == EA_GCREF) + { + gcrefRegs |= RBM_INTRET_1; + } + else if (secondRetSize == EA_BYREF) + { + byrefRegs |= RBM_INTRET_1; + } + VarSetOps::Assign(emitComp, emitThisGCrefVars, ptrVars); emitThisGCrefRegs = gcrefRegs; emitThisByrefRegs = byrefRegs; diff --git a/src/coreclr/jit/emitxarch.cpp b/src/coreclr/jit/emitxarch.cpp index 65ed4dd04004ed..c35ee7c7086612 100644 --- a/src/coreclr/jit/emitxarch.cpp +++ b/src/coreclr/jit/emitxarch.cpp @@ -9578,6 +9578,28 @@ void emitter::emitIns_Call(EmitCallType callType, /* Update the emitter's live GC ref sets */ + // If the method returns a GC ref, mark EAX appropriately + if (retSize == EA_GCREF) + { + gcrefRegs |= RBM_EAX; + } + else if (retSize == EA_BYREF) + { + byrefRegs |= RBM_EAX; + } + +#ifdef UNIX_AMD64_ABI + // If is a multi-register return method is called, mark RDX appropriately (for System V AMD64). + if (secondRetSize == EA_GCREF) + { + gcrefRegs |= RBM_RDX; + } + else if (secondRetSize == EA_BYREF) + { + byrefRegs |= RBM_RDX; + } +#endif // UNIX_AMD64_ABI + VarSetOps::Assign(emitComp, emitThisGCrefVars, ptrVars); emitThisGCrefRegs = gcrefRegs; emitThisByrefRegs = byrefRegs; @@ -16548,9 +16570,9 @@ size_t emitter::emitOutputInstr(insGroup* ig, instrDesc* id, BYTE** dp) #ifdef TARGET_X86 dst += emitOutputWord(dst, code | 0x0500); #else // TARGET_AMD64 - // Amd64: addr fits within 32-bits and can be encoded as a displacement relative to zero. - // This addr mode should never be used while generating relocatable ngen code nor if - // the addr can be encoded as pc-relative address. + // Amd64: addr fits within 32-bits and can be encoded as a displacement relative to zero. + // This addr mode should never be used while generating relocatable ngen code nor if + // the addr can be encoded as pc-relative address. noway_assert(!emitComp->opts.compReloc); noway_assert(codeGen->genAddrRelocTypeHint((size_t)addr) != IMAGE_REL_BASED_REL32); noway_assert(static_cast(reinterpret_cast(addr)) == (ssize_t)addr); diff --git a/src/coreclr/jit/jitgcinfo.h b/src/coreclr/jit/jitgcinfo.h index 9f9c06d9265817..e953f0e9e124d6 100644 --- a/src/coreclr/jit/jitgcinfo.h +++ b/src/coreclr/jit/jitgcinfo.h @@ -183,8 +183,8 @@ class GCInfo } unsigned short rpdIsThis : 1; // is it the 'this' pointer - unsigned short rpdCall : 1; // is this a true call site? - unsigned short : 1; // Padding bit, so next two start on a byte boundary + unsigned short rpdCall : 1; // is this a true call site? + unsigned short : 1; // Padding bit, so next two start on a byte boundary unsigned short rpdCallGCrefRegs : CNT_CALL_GC_REGS; // Callee-saved and return registers containing GC pointers. unsigned short rpdCallByrefRegs : CNT_CALL_GC_REGS; // Callee-saved and return registers containing byrefs.