Merge pull request dotnet#17372 from BruceForstall/ArmWriteBarrierTig…

…hten3

Tighten arm32/arm64 write barrier kill reg sets

src/jit/codegenarm64.cpp

@@ -2757,7 +2757,7 @@ void CodeGen::genLockedInstructions(GenTreeOp* treeNode)
 }
 
 //------------------------------------------------------------------------
-// genCodeForSwap: Produce code for a GT_CMPXCHG node.
+// genCodeForCmpXchg: Produce code for a GT_CMPXCHG node.
 //
 // Arguments:
 //    tree - the GT_CMPXCHG node
@@ -3006,41 +3006,16 @@ void CodeGen::genCodeForStoreInd(GenTreeStoreInd* tree)
         // registers are taken care of.
         genConsumeOperands(tree);
 
-#if NOGC_WRITE_BARRIERS
         // At this point, we should not have any interference.
         // That is, 'data' must not be in REG_WRITE_BARRIER_DST_BYREF,
         //  as that is where 'addr' must go.
         noway_assert(data->gtRegNum != REG_WRITE_BARRIER_DST_BYREF);
 
-        // 'addr' goes into x14 (REG_WRITE_BARRIER_DST_BYREF)
-        if (addr->gtRegNum != REG_WRITE_BARRIER_DST_BYREF)
-        {
-            inst_RV_RV(INS_mov, REG_WRITE_BARRIER_DST_BYREF, addr->gtRegNum, addr->TypeGet());
-        }
-
-        // 'data'  goes into x15 (REG_WRITE_BARRIER)
-        if (data->gtRegNum != REG_WRITE_BARRIER)
-        {
-            inst_RV_RV(INS_mov, REG_WRITE_BARRIER, data->gtRegNum, data->TypeGet());
-        }
-#else
-        // At this point, we should not have any interference.
-        // That is, 'data' must not be in REG_ARG_0,
-        //  as that is where 'addr' must go.
-        noway_assert(data->gtRegNum != REG_ARG_0);
+        // 'addr' goes into x14 (REG_WRITE_BARRIER_DST)
+        genCopyRegIfNeeded(addr, REG_WRITE_BARRIER_DST);
 
-        // addr goes in REG_ARG_0
-        if (addr->gtRegNum != REG_ARG_0)
-        {
-            inst_RV_RV(INS_mov, REG_ARG_0, addr->gtRegNum, addr->TypeGet());
-        }
-
-        // data goes in REG_ARG_1
-        if (data->gtRegNum != REG_ARG_1)
-        {
-            inst_RV_RV(INS_mov, REG_ARG_1, data->gtRegNum, data->TypeGet());
-        }
-#endif // NOGC_WRITE_BARRIERS
+        // 'data' goes into x15 (REG_WRITE_BARRIER_SRC)
+        genCopyRegIfNeeded(data, REG_WRITE_BARRIER_SRC);
 
         genGCWriteBarrier(tree, writeBarrierForm);
     }
@@ -3116,6 +3091,8 @@ void CodeGen::genCodeForStoreInd(GenTreeStoreInd* tree)
 //
 void CodeGen::genCodeForSwap(GenTreeOp* tree)
 {
+    assert(tree->OperIs(GT_SWAP));
+
     // Swap is only supported for lclVar operands that are enregistered
     // We do not consume or produce any registers.  Both operands remain enregistered.
     // However, the gc-ness may change.

src/jit/codegencommon.cpp

@@ -635,7 +635,7 @@ void CodeGenInterface::genUpdateRegLife(const LclVarDsc* varDsc, bool isBorn, bo
 //   helper - The helper being inquired about
 //
 // Return Value:
-//   Mask of register kills -- registers whose value is no longer guaranteed to be the same.
+//   Mask of register kills -- registers whose values are no longer guaranteed to be the same.
 //
 regMaskTP Compiler::compHelperCallKillSet(CorInfoHelpFunc helper)
 {
@@ -645,14 +645,20 @@ regMaskTP Compiler::compHelperCallKillSet(CorInfoHelpFunc helper)
 #if defined(_TARGET_AMD64_)
             return RBM_RSI | RBM_RDI | RBM_CALLEE_TRASH_NOGC;
 #elif defined(_TARGET_ARMARCH_)
-            return RBM_WRITE_BARRIER_SRC_BYREF | RBM_WRITE_BARRIER_DST_BYREF | RBM_CALLEE_TRASH_NOGC;
+            return RBM_CALLEE_TRASH_WRITEBARRIER_BYREF;
 #elif defined(_TARGET_X86_)
             return RBM_ESI | RBM_EDI | RBM_ECX;
 #else
             NYI("Model kill set for CORINFO_HELP_ASSIGN_BYREF on target arch");
             return RBM_CALLEE_TRASH;
 #endif
 
+#if defined(_TARGET_ARMARCH_)
+        case CORINFO_HELP_ASSIGN_REF:
+        case CORINFO_HELP_CHECKED_ASSIGN_REF:
+            return RBM_CALLEE_TRASH_WRITEBARRIER;
+#endif
+
         case CORINFO_HELP_PROF_FCN_ENTER:
 #ifdef RBM_PROFILER_ENTER_TRASH
             return RBM_PROFILER_ENTER_TRASH;
@@ -742,15 +748,20 @@ regMaskTP Compiler::compNoGCHelperCallKillSet(CorInfoHelpFunc helper)
             return RBM_PROFILER_TAILCALL_TRASH;
 #endif // defined(_TARGET_XARCH_)
 
+#if defined(_TARGET_X86_)
         case CORINFO_HELP_ASSIGN_BYREF:
-#if defined(_TARGET_AMD64_)
-            return RBM_CALLEE_TRASH_NOGC;
-#elif defined(_TARGET_X86_)
             // This helper only trashes ECX.
             return RBM_ECX;
-#elif defined(_TARGET_ARMARCH_)
-            return RBM_CALLEE_TRASH_NOGC;
-#endif // defined(_TARGET_AMD64_)
+#endif // defined(_TARGET_X86_)
+
+#if defined(_TARGET_ARMARCH_)
+        case CORINFO_HELP_ASSIGN_BYREF:
+            return RBM_CALLEE_GCTRASH_WRITEBARRIER_BYREF;
+
+        case CORINFO_HELP_ASSIGN_REF:
+        case CORINFO_HELP_CHECKED_ASSIGN_REF:
+            return RBM_CALLEE_GCTRASH_WRITEBARRIER;
+#endif
 
         default:
             return RBM_CALLEE_TRASH_NOGC;
@@ -3969,8 +3980,8 @@ void CodeGen::genReportEH()
 //
 // Return Value:
 //   true if an optimized write barrier helper should be used, false otherwise.
-//   Note: only x86 implements (register-specific source) optimized write
-//   barriers currently).
+//   Note: only x86 implements register-specific source optimized write
+//   barriers currently.
 //
 bool CodeGenInterface::genUseOptimizedWriteBarriers(GCInfo::WriteBarrierForm wbf)
 {
@@ -3999,8 +4010,8 @@ bool CodeGenInterface::genUseOptimizedWriteBarriers(GCInfo::WriteBarrierForm wbf
 //
 // Return Value:
 //   true if an optimized write barrier helper should be used, false otherwise.
-//   Note: only x86 implements (register-specific source) optimized write
-//   barriers currently).
+//   Note: only x86 implements register-specific source optimized write
+//   barriers currently.
 //
 bool CodeGenInterface::genUseOptimizedWriteBarriers(GenTree* tgt, GenTree* assignVal)
 {

src/jit/codegenlegacy.cpp

@@ -2102,7 +2102,7 @@ regMaskTP CodeGen::genMakeAddrArrElem(GenTree* arrElem, GenTree* tree, regMaskTP
     regMaskTP addrReg  = RBM_NONE;
     regMaskTP regNeed  = RBM_ALLINT;
 
-#if FEATURE_WRITE_BARRIER && !NOGC_WRITE_BARRIERS
+#if !NOGC_WRITE_BARRIERS
     // In CodeGen::WriteBarrier we set up ARG_1 followed by ARG_0
     // since the arrObj participates in the lea/add instruction
     // that computes ARG_0 we should avoid putting it in ARG_1
@@ -3108,8 +3108,6 @@ regMaskTP CodeGen::WriteBarrier(GenTree* tgt, GenTree* assignVal, regMaskTP tgtA
 
     regMaskTP resultRegMask = RBM_NONE;
 
-#if FEATURE_WRITE_BARRIER
-
     regNumber reg = assignVal->gtRegNum;
 
 #if defined(_TARGET_X86_) && NOGC_WRITE_BARRIERS
@@ -3340,13 +3338,6 @@ regMaskTP CodeGen::WriteBarrier(GenTree* tgt, GenTree* assignVal, regMaskTP tgtA
         return resultRegMask;
     }
 #endif // defined(DEBUG) || !(defined(_TARGET_X86_) && NOGC_WRITE_BARRIERS)
-
-#else // !FEATURE_WRITE_BARRIER
-
-    NYI("FEATURE_WRITE_BARRIER unimplemented");
-    return resultRegMask;
-
-#endif // !FEATURE_WRITE_BARRIER
 }
 
 #ifdef _TARGET_X86_
@@ -8877,8 +8868,7 @@ void CodeGen::genCodeForCopyObj(GenTree* tree, regMaskTP destReg)
         CorInfoGCType gcTypeNext = TYPE_GC_NONE;
         var_types     type       = TYP_I_IMPL;
 
-#if FEATURE_WRITE_BARRIER
-        gcType                   = (CorInfoGCType)(*gcPtrs++);
+        gcType = (CorInfoGCType)(*gcPtrs++);
         if (blkSize > TARGET_POINTER_SIZE)
             gcTypeNext = (CorInfoGCType)(*gcPtrs);
 
@@ -8893,9 +8883,6 @@ void CodeGen::genCodeForCopyObj(GenTree* tree, regMaskTP destReg)
             assert(regSrc == REG_ARG_1);
             assert(regTemp == REG_R2);
         }
-#else
-        gcType = TYPE_GC_NONE;
-#endif // FEATURE_WRITE_BARRIER
 
         blkSize -= TARGET_POINTER_SIZE;
 
@@ -10427,14 +10414,7 @@ void CodeGen::genCodeForTreeSmpOp(GenTree* tree, regMaskTP destReg, regMaskTP be
         }
             return;
 
-#else // !_TARGET_XARCH_
-
-        case GT_LOCKADD:
-        case GT_XADD:
-        case GT_XCHG:
-
-            NYI_ARM("LOCK instructions");
-#endif
+#endif // _TARGET_XARCH_
 
         case GT_ARR_LENGTH:
         {

src/jit/emit.cpp

@@ -2335,15 +2335,12 @@ bool emitter::emitNoGChelper(unsigned IHX)
 #endif
 
         case CORINFO_HELP_ASSIGN_REF:
-
         case CORINFO_HELP_CHECKED_ASSIGN_REF:
+        case CORINFO_HELP_ASSIGN_BYREF:
 
         case CORINFO_HELP_GETSHARED_GCSTATIC_BASE_NOCTOR:
-
         case CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE_NOCTOR:
 
-        case CORINFO_HELP_ASSIGN_BYREF:
-
         case CORINFO_HELP_INIT_PINVOKE_FRAME:
 
             return true;

src/jit/emitarm.cpp

@@ -4467,12 +4467,8 @@ void emitter::emitIns_Call(EmitCallType          callType,
     {
         assert(emitNoGChelper(Compiler::eeGetHelperNum(methHnd)));
 
-        // This call will preserve the liveness of most registers
-        //
-        // - On the ARM the NOGC helpers will preserve all registers,
-        //   except for those listed in the RBM_CALLEE_TRASH_NOGC mask
-
-        savedSet = RBM_ALLINT & ~RBM_CALLEE_TRASH_NOGC;
+        // Get the set of registers that this call kills and remove it from the saved set.
+        savedSet = RBM_ALLINT & ~emitComp->compNoGCHelperCallKillSet(Compiler::eeGetHelperNum(methHnd));
 
         // In case of Leave profiler callback, we need to preserve liveness of REG_PROFILER_RET_SCRATCH
         if (isProfLeaveCB)

src/jit/emitarm64.cpp

@@ -7356,6 +7356,16 @@ void emitter::emitIns_Call(EmitCallType          callType,
         {
             savedSet |= RBM_PROFILER_RET_SCRATCH;
         }
+
+#ifdef DEBUG
+        if (emitComp->verbose)
+        {
+            printf("NOGC Call: savedSet=");
+            printRegMaskInt(savedSet);
+            emitDispRegSet(savedSet);
+            printf("\n");
+        }
+#endif
     }
     else
     {

src/jit/gcinfo.cpp

@@ -231,8 +231,6 @@ void GCInfo::gcMarkRegPtrVal(regNumber reg, var_types type)
 
 GCInfo::WriteBarrierForm GCInfo::gcIsWriteBarrierCandidate(GenTree* tgt, GenTree* assignVal)
 {
-#if FEATURE_WRITE_BARRIER
-
     /* Are we storing a GC ptr? */
 
     if (!varTypeIsGC(tgt->TypeGet()))
@@ -292,7 +290,6 @@ GCInfo::WriteBarrierForm GCInfo::gcIsWriteBarrierCandidate(GenTree* tgt, GenTree
     }
 
     assert(!"Missing case in gcIsWriteBarrierCandidate");
-#endif
 
     return WBF_NoBarrier;
 }

src/jit/gentree.cpp

@@ -7456,7 +7456,7 @@ GenTree* Compiler::gtNewPutArgReg(var_types type, GenTree* arg, regNumber argReg
 //    Returns the newly created BitCast node.
 //
 // Notes:
-//    The node is generated as GenTreeMultiRegOp on RyuJIT/armel, as GenTreeOp on all the other archs.
+//    The node is generated as GenTreeMultiRegOp on RyuJIT/arm, as GenTreeOp on all the other archs.
 //
 GenTree* Compiler::gtNewBitCastNode(var_types type, GenTree* arg)
 {

src/jit/importer.cpp

@@ -19221,9 +19221,9 @@ bool Compiler::IsTargetIntrinsic(CorInfoIntrinsics intrinsicId)
 
 bool Compiler::IsIntrinsicImplementedByUserCall(CorInfoIntrinsics intrinsicId)
 {
-    // Currently, if an math intrisic is not implemented by target-specific
-    // intructions, it will be implemented by a System.Math call. In the
-    // future, if we turn to implementing some of them with helper callers,
+    // Currently, if a math intrinsic is not implemented by target-specific
+    // instructions, it will be implemented by a System.Math call. In the
+    // future, if we turn to implementing some of them with helper calls,
     // this predicate needs to be revisited.
     return !IsTargetIntrinsic(intrinsicId);
 }

src/jit/lsra.cpp

@@ -6377,14 +6377,14 @@ void LinearScan::insertCopyOrReload(BasicBlock* block, GenTree* tree, unsigned m
     }
 
     // If the parent is a reload/copy node, then tree must be a multi-reg call node
-    // that has already had one of its registers spilled. This is Because multi-reg
+    // that has already had one of its registers spilled. This is because multi-reg
     // call node is the only node whose RefTypeDef positions get independently
     // spilled or reloaded.  It is possible that one of its RefTypeDef position got
     // spilled and the next use of it requires it to be in a different register.
     //
-    // In this case set the ith position reg of reload/copy node to the reg allocated
+    // In this case set the i'th position reg of reload/copy node to the reg allocated
     // for copy/reload refPosition.  Essentially a copy/reload node will have a reg
-    // for each multi-reg position of its child. If there is a valid reg in ith
+    // for each multi-reg position of its child. If there is a valid reg in i'th
     // position of GT_COPY or GT_RELOAD node then the corresponding result of its
     // child needs to be copied or reloaded to that reg.
     if (parent->IsCopyOrReload())

src/jit/lsrabuild.cpp

@@ -664,7 +664,8 @@ regMaskTP LinearScan::getKillSetForStoreInd(GenTreeStoreInd* tree)
         {
             // We can't determine the exact helper to be used at this point, because it depends on
             // the allocated register for the `data` operand. However, all the (x86) optimized
-            // helpers have the same kill set: EDX.
+            // helpers have the same kill set: EDX. And note that currently, only x86 can return
+            // `true` for genUseOptimizedWriteBarriers().
             killMask = RBM_CALLEE_TRASH_NOGC;
         }
         else
@@ -3141,22 +3142,18 @@ void LinearScan::BuildGCWriteBarrier(GenTree* tree)
     assert(info->dstCount == 0);
     bool customSourceRegs = false;
 
-#if NOGC_WRITE_BARRIERS
-
 #if defined(_TARGET_ARM64_)
-    // For the NOGC JIT Helper calls
-    //
-    // the 'addr' goes into x14 (REG_WRITE_BARRIER_DST_BYREF)
-    // the 'src'  goes into x15 (REG_WRITE_BARRIER)
+
+    // the 'addr' goes into x14 (REG_WRITE_BARRIER_DST)
+    // the 'src'  goes into x15 (REG_WRITE_BARRIER_SRC)
     //
-    addrInfo->info.setSrcCandidates(this, RBM_WRITE_BARRIER_DST_BYREF);
-    srcInfo->info.setSrcCandidates(this, RBM_WRITE_BARRIER);
+    addrInfo->info.setSrcCandidates(this, RBM_WRITE_BARRIER_DST);
+    srcInfo->info.setSrcCandidates(this, RBM_WRITE_BARRIER_SRC);
     customSourceRegs = true;
 
-#elif defined(_TARGET_X86_)
+#elif defined(_TARGET_X86_) && NOGC_WRITE_BARRIERS
 
     bool useOptimizedWriteBarrierHelper = compiler->codeGen->genUseOptimizedWriteBarriers(tree, src);
-
     if (useOptimizedWriteBarrierHelper)
     {
         // Special write barrier:
@@ -3166,11 +3163,8 @@ void LinearScan::BuildGCWriteBarrier(GenTree* tree)
         srcInfo->info.setSrcCandidates(this, RBM_WRITE_BARRIER_SRC);
         customSourceRegs = true;
     }
-#else // !defined(_TARGET_X86_) && !defined(_TARGET_ARM64_)
-#error "NOGC_WRITE_BARRIERS is not supported"
-#endif // !defined(_TARGET_X86_)
 
-#endif // NOGC_WRITE_BARRIERS
+#endif // defined(_TARGET_X86_) && NOGC_WRITE_BARRIERS
 
     if (!customSourceRegs)
     {

src/jit/optimizer.cpp

@@ -5573,7 +5573,7 @@ void Compiler::optEnsureUniqueHead(unsigned loopInd, unsigned ambientWeight)
 
             return CALLINT_SCL_INDIRS;
 
-        case CORINFO_HELP_ASSIGN_STRUCT: // Not strictly needed as we don't use this in Jit32
+        case CORINFO_HELP_ASSIGN_STRUCT: // Not strictly needed as we don't use this
         case CORINFO_HELP_MEMSET:        // Not strictly needed as we don't make a GT_CALL with this
         case CORINFO_HELP_MEMCPY:        // Not strictly needed as we don't make a GT_CALL with this
         case CORINFO_HELP_SETFIELDSTRUCT:

src/jit/regset.cpp

@@ -1902,11 +1902,13 @@ void RegSet::rsSpillRegs(regMaskTP regMask)
  *  for internal tree temps to live in
  */
 
+#ifdef LEGACY_BACKEND
 extern const regNumber raRegTmpOrder[] = {REG_TMP_ORDER};
 extern const regNumber rpRegTmpOrder[] = {REG_PREDICT_ORDER};
 #if FEATURE_FP_REGALLOC
 extern const regNumber raRegFltTmpOrder[] = {REG_FLT_TMP_ORDER};
 #endif
+#endif // LEGACY_BACKEND
 
 /*****************************************************************************
  *