diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 9660b0c98be9d..220561d726d36 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -890,8 +890,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 55db3ccabba1b..7c2d84e79278c 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 331887c52bdef..d0b14daea201d 100644 --- a/src/coreclr/jit/emit.cpp +++ b/src/coreclr/jit/emit.cpp @@ -2217,6 +2217,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) { @@ -2873,10 +2877,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 (prevBlock != NULL && 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()) @@ -3637,6 +3667,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 */ @@ -3716,6 +3747,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); @@ -8725,6 +8757,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 40a729dd70fee..6e1555a1a978b 100644 --- a/src/coreclr/jit/emit.h +++ b/src/coreclr/jit/emit.h @@ -762,10 +762,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 @@ -1565,7 +1565,7 @@ class emitter bool idIsLargeCns() const { - return _idLargeCns != 0; + return _idLargeCns != 0 && !idIsCall(); } void idSetIsLargeCns() { @@ -1585,13 +1585,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 @@ -2857,9 +2867,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 @@ -3190,6 +3202,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 262ab3d9c8973..ac471e29bb923 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 aec4b0b03de35..9bbf46a5e64d7 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 3e2d54748f4b8..419fa424077e5 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 e40be073ffa3f..9bb5a8a4f4dbe 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 dfd3b3e08281f..63fa8ffb47bdb 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; @@ -16546,9 +16568,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 3d2a9b2047ee4..6c62a0816113a 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.