Unmasking EGPRs in register allocator (dotnet#114867)

* unmask HWIntrinsic node.

* IMUL, GT_INDEX_ADDR

* GT_CKFINITE
GT_RETURNTRAP
GT_SWITCH_TABLE
GT_JMPTABLE
genFnProlog

* Complete BuildHWIntrinsics

* Let genFnProlog have EGPR when APX and EVEX are both available.

* Indir

* clean-up in lsrabuild

* Enable egprs in cast nodes

* unmask BuildIntrinsic

* unmask BuildShiftRotate

* Unmask BlockStore

* run formatting

* Use regMask when looking for srcReg candidate in BlkStore

* Use EGPR for NI_System_Math_Abs only and remove EGPR usage from dstReg

* Enable asserts for additional 8 reg

* Resolve error

* Apply suggestions from code review

Co-authored-by: Kunal Pathak <[email protected]>

* Unmask remaining instructions and cache evexIsSupported and canUseApxRegs

* revert changes in SetContainsAVXFlags

* Add APX bypass Knob

* Add Instruction Count reporting in superpmi

* remove debug comment

* Revert "revert changes in SetContainsAVXFlags"

This reverts commit 34c693f.

* Revert "Add APX bypass Knob"

This reverts commit 4be2048.

* revert changes in SetContainsAVXFlags

* Revert "Add Instruction Count reporting in superpmi"

This reverts commit c0793c3.

* Narrow down the cases where EGPRs cannot be alloted

* Run formatting and remove stale comments

* Move Apx Checks to Evex checks for egpr mask

* Mask Shift Rotate nodes when evex is not available

* Resolve code style comments

---------

Co-authored-by: Ruihan-Yin <[email protected]>
Co-authored-by: Kunal Pathak <[email protected]>

Author: 3 people

src/coreclr/jit/codegencommon.cpp

@@ -5044,8 +5044,11 @@ void CodeGen::genFnProlog()
     bool      initRegZeroed = false;
     regMaskTP excludeMask   = intRegState.rsCalleeRegArgMaskLiveIn;
 #if defined(TARGET_AMD64)
-    // TODO-Xarch-apx : Revert. Excluding eGPR so that it's not used for non REX2 supported movs.
-    excludeMask = excludeMask | RBM_HIGHINT;
+    // we'd require eEVEX present to enable EGPRs in HWIntrinsics.
+    if (!compiler->canUseEvexEncoding())
+    {
+        excludeMask = excludeMask | RBM_HIGHINT;
+    }
 #endif // !defined(TARGET_AMD64)
 
 #ifdef TARGET_ARM

src/coreclr/jit/codegenxarch.cpp

@@ -756,7 +756,30 @@ void CodeGen::genCodeForBswap(GenTree* tree)
     }
     else
     {
-        GetEmitter()->emitInsBinary(INS_movbe, emitTypeSize(operand), tree, operand);
+        instruction ins = INS_movbe;
+#ifdef TARGET_AMD64
+        bool needsEvex = false;
+
+        if (GetEmitter()->IsExtendedGPReg(tree->GetRegNum()))
+        {
+            needsEvex = true;
+        }
+        else if (operand->isIndir())
+        {
+            GenTreeIndir* indir = operand->AsIndir();
+            if (indir->HasBase() && GetEmitter()->IsExtendedGPReg(indir->Base()->GetRegNum()))
+            {
+                needsEvex = true;
+            }
+            else if (indir->HasIndex() && GetEmitter()->IsExtendedGPReg(indir->Index()->GetRegNum()))
+            {
+                needsEvex = true;
+            }
+        }
+
+        ins = needsEvex ? INS_movbe_apx : INS_movbe;
+#endif
+        GetEmitter()->emitInsBinary(ins, emitTypeSize(operand), tree, operand);
     }
 
     if (tree->OperIs(GT_BSWAP16) && !genCanOmitNormalizationForBswap16(tree))
@@ -5679,6 +5702,22 @@ void CodeGen::genCodeForStoreInd(GenTreeStoreInd* tree)
                 if (data->OperIs(GT_BSWAP, GT_BSWAP16))
                 {
                     ins = INS_movbe;
+#ifdef TARGET_AMD64
+                    bool needsEvex = false;
+                    if (GetEmitter()->IsExtendedGPReg(data->gtGetOp1()->GetRegNum()))
+                    {
+                        needsEvex = true;
+                    }
+                    else if (tree->HasBase() && GetEmitter()->IsExtendedGPReg(tree->Base()->GetRegNum()))
+                    {
+                        needsEvex = true;
+                    }
+                    else if (tree->HasIndex() && GetEmitter()->IsExtendedGPReg(tree->Index()->GetRegNum()))
+                    {
+                        needsEvex = true;
+                    }
+                    ins = needsEvex ? INS_movbe_apx : INS_movbe;
+#endif // TARGET_AMD64
                 }
 #if defined(FEATURE_HW_INTRINSICS)
                 else if (data->OperIsHWIntrinsic())
@@ -7331,9 +7370,11 @@ void CodeGen::genIntToFloatCast(GenTree* treeNode)
         //   addsd    xmm0, xmm0
         //.LABEL
         //
-        regNumber argReg  = treeNode->gtGetOp1()->GetRegNum();
-        regNumber tmpReg1 = internalRegisters.Extract(treeNode);
+        regNumber argReg = treeNode->gtGetOp1()->GetRegNum();
+        // Get the APXIncompatible register first
         regNumber tmpReg2 = internalRegisters.Extract(treeNode);
+        // tmpReg1 can be EGPR
+        regNumber tmpReg1 = internalRegisters.Extract(treeNode);
 
         inst_Mov(TYP_LONG, tmpReg1, argReg, /* canSkip */ false, EA_8BYTE);
         inst_RV_SH(INS_shr, EA_8BYTE, tmpReg1, 1);

src/coreclr/jit/emitinl.h

@@ -13,7 +13,7 @@ inline bool emitter::instrIs3opImul(instruction ins)
 #ifdef TARGET_X86
     return ((ins >= INS_imul_AX) && (ins <= INS_imul_DI));
 #else // TARGET_AMD64
-    return ((ins >= INS_imul_AX) && (ins <= INS_imul_15));
+    return ((ins >= INS_imul_AX) && (ins <= INS_imul_31));
 #endif
 }
 
@@ -23,7 +23,7 @@ inline bool emitter::instrIsExtendedReg3opImul(instruction ins)
 #ifdef TARGET_X86
     return false;
 #else // TARGET_AMD64
-    return ((ins >= INS_imul_08) && (ins <= INS_imul_15));
+    return ((ins >= INS_imul_08) && (ins <= INS_imul_31));
 #endif
 }
 
@@ -55,6 +55,22 @@ inline void emitter::check3opImulValues()
     assert(INS_imul_13 - INS_imul_AX == REG_R13);
     assert(INS_imul_14 - INS_imul_AX == REG_R14);
     assert(INS_imul_15 - INS_imul_AX == REG_R15);
+    assert(INS_imul_16 - INS_imul_AX == REG_R16);
+    assert(INS_imul_17 - INS_imul_AX == REG_R17);
+    assert(INS_imul_18 - INS_imul_AX == REG_R18);
+    assert(INS_imul_19 - INS_imul_AX == REG_R19);
+    assert(INS_imul_20 - INS_imul_AX == REG_R20);
+    assert(INS_imul_21 - INS_imul_AX == REG_R21);
+    assert(INS_imul_22 - INS_imul_AX == REG_R22);
+    assert(INS_imul_23 - INS_imul_AX == REG_R23);
+    assert(INS_imul_24 - INS_imul_AX == REG_R24);
+    assert(INS_imul_25 - INS_imul_AX == REG_R25);
+    assert(INS_imul_26 - INS_imul_AX == REG_R26);
+    assert(INS_imul_27 - INS_imul_AX == REG_R27);
+    assert(INS_imul_28 - INS_imul_AX == REG_R28);
+    assert(INS_imul_29 - INS_imul_AX == REG_R29);
+    assert(INS_imul_30 - INS_imul_AX == REG_R30);
+    assert(INS_imul_31 - INS_imul_AX == REG_R31);
 #endif
 }
 

src/coreclr/jit/emitxarch.cpp

@@ -402,7 +402,7 @@ bool emitter::IsApxExtendedEvexInstruction(instruction ins) const
         return true;
     }
 
-    if (ins == INS_crc32_apx)
+    if (ins == INS_crc32_apx || ins == INS_movbe_apx)
     {
         // With the new opcode, CRC32 is promoted to EVEX with APX.
         return true;
@@ -1102,6 +1102,22 @@ bool emitter::emitIsInstrWritingToReg(instrDesc* id, regNumber reg)
         case INS_imul_13:
         case INS_imul_14:
         case INS_imul_15:
+        case INS_imul_16:
+        case INS_imul_17:
+        case INS_imul_18:
+        case INS_imul_19:
+        case INS_imul_20:
+        case INS_imul_21:
+        case INS_imul_22:
+        case INS_imul_23:
+        case INS_imul_24:
+        case INS_imul_25:
+        case INS_imul_26:
+        case INS_imul_27:
+        case INS_imul_28:
+        case INS_imul_29:
+        case INS_imul_30:
+        case INS_imul_31:
 #endif // TARGET_AMD64
             if (reg == inst3opImulReg(ins))
             {
@@ -1879,6 +1895,12 @@ bool emitter::TakesRex2Prefix(const instrDesc* id) const
         return true;
     }
 
+    if (ins >= INS_imul_16 && ins <= INS_imul_31)
+    {
+        // The instructions have implicit use of EGPRs.
+        return true;
+    }
+
 #if defined(DEBUG)
     if (emitComp->DoJitStressRex2Encoding())
     {
@@ -1931,7 +1953,7 @@ bool emitter::TakesApxExtendedEvexPrefix(const instrDesc* id) const
         return true;
     }
 
-    if (ins == INS_crc32_apx)
+    if (ins == INS_crc32_apx || ins == INS_movbe_apx)
     {
         return true;
     }
@@ -6085,7 +6107,11 @@ void emitter::emitInsStoreInd(instruction ins, emitAttr attr, GenTreeStoreInd* m
 
     if (data->OperIs(GT_BSWAP, GT_BSWAP16) && data->isContained())
     {
+#ifdef TARGET_AMD64
+        assert(ins == INS_movbe || ins == INS_movbe_apx);
+#else
         assert(ins == INS_movbe);
+#endif
         data = data->gtGetOp1();
     }
 
@@ -14701,7 +14727,7 @@ BYTE* emitter::emitOutputAM(BYTE* dst, instrDesc* id, code_t code, CnsVal* addc)
                 break;
         }
 #ifdef TARGET_AMD64
-        if (ins == INS_crc32_apx)
+        if (ins == INS_crc32_apx || ins == INS_movbe_apx)
         {
             code |= (insEncodeReg345(id, id->idReg1(), size, &code) << 8);
         }
@@ -15586,7 +15612,7 @@ BYTE* emitter::emitOutputSV(BYTE* dst, instrDesc* id, code_t code, CnsVal* addc)
                 break;
         }
 #ifdef TARGET_AMD64
-        if (ins == INS_crc32_apx)
+        if (ins == INS_crc32_apx || ins == INS_movbe_apx)
         {
             // The promoted CRC32 is in 1-byte opcode, unlike other instructions on this path, the register encoding for
             // CRC32 need to be done here.
@@ -20427,6 +20453,22 @@ emitter::insExecutionCharacteristics emitter::getInsExecutionCharacteristics(ins
         case INS_imul_13:
         case INS_imul_14:
         case INS_imul_15:
+        case INS_imul_16:
+        case INS_imul_17:
+        case INS_imul_18:
+        case INS_imul_19:
+        case INS_imul_20:
+        case INS_imul_21:
+        case INS_imul_22:
+        case INS_imul_23:
+        case INS_imul_24:
+        case INS_imul_25:
+        case INS_imul_26:
+        case INS_imul_27:
+        case INS_imul_28:
+        case INS_imul_29:
+        case INS_imul_30:
+        case INS_imul_31:
 #endif // TARGET_AMD64
         case INS_imul:
             result.insThroughput = PERFSCORE_THROUGHPUT_1C;
@@ -21941,6 +21983,9 @@ emitter::insExecutionCharacteristics emitter::getInsExecutionCharacteristics(ins
         }
 
         case INS_movbe:
+#ifdef TARGET_AMD64
+        case INS_movbe_apx:
+#endif
             if (memAccessKind == PERFSCORE_MEMORY_READ)
             {
                 result.insThroughput = PERFSCORE_THROUGHPUT_2X;