Widen index to a temporary register

mikedn · Feb 15, 2019 · 091bc56 · 091bc56
1 parent 9b78e13
commit 091bc56
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 38 deletions.
diff --git a/src/jit/codegenxarch.cpp b/src/jit/codegenxarch.cpp
@@ -4608,7 +4608,7 @@ void CodeGen::genCodeForIndexAddr(GenTreeIndexAddr* node)
     GenTree* const index = node->Index();
 
     const regNumber baseReg  = genConsumeReg(base);
-    const regNumber indexReg = genConsumeReg(index);
+    regNumber       indexReg = genConsumeReg(index);
     const regNumber dstReg   = node->gtRegNum;
 
     // NOTE: `genConsumeReg` marks the consumed register as not a GC pointer, as it assumes that the input registers
@@ -4620,6 +4620,9 @@ void CodeGen::genCodeForIndexAddr(GenTreeIndexAddr* node)
     assert(varTypeIsIntegral(index->TypeGet()));
 
     regNumber tmpReg = REG_NA;
+#ifdef _TARGET_64BIT_
+    tmpReg = node->GetSingleTempReg();
+#endif
 
     // Generate the bounds check if necessary.
     if ((node->gtFlags & GTF_INX_RNGCHK) != 0)
@@ -4629,7 +4632,6 @@ void CodeGen::genCodeForIndexAddr(GenTreeIndexAddr* node)
         // is a native int on a 64-bit platform, we will need to widen the array length and then compare.
         if (index->TypeGet() == TYP_I_IMPL)
         {
-            tmpReg = node->GetSingleTempReg();
             getEmitter()->emitIns_R_AR(INS_mov, EA_4BYTE, tmpReg, baseReg, static_cast<int>(node->gtLenOffset));
             getEmitter()->emitIns_R_R(INS_cmp, EA_8BYTE, indexReg, tmpReg);
         }
@@ -4646,9 +4648,8 @@ void CodeGen::genCodeForIndexAddr(GenTreeIndexAddr* node)
     if (index->TypeGet() != TYP_I_IMPL)
     {
         // LEA needs 64-bit operands so we need to widen the index if it's TYP_INT.
-        // Since it's TYP_INT the upper 32 bits aren't used so we should be able
-        // to widen in place, without needing a temporary register.
-        getEmitter()->emitIns_R_R(INS_mov, EA_4BYTE, indexReg, indexReg);
+        getEmitter()->emitIns_R_R(INS_mov, EA_4BYTE, tmpReg, indexReg);
+        indexReg = tmpReg;
     }
 #endif // _TARGET_64BIT_
 
@@ -4665,22 +4666,16 @@ void CodeGen::genCodeForIndexAddr(GenTreeIndexAddr* node)
             break;
 
         default:
-            if (tmpReg == REG_NA)
-            {
-                tmpReg = node->GetSingleTempReg();
-            }
 #ifdef _TARGET_64BIT_
-            if (scale > INT32_MAX)
-            {
-                genSetRegToIcon(tmpReg, static_cast<ssize_t>(scale), TYP_INT);
-                getEmitter()->emitIns_R_R(INS_imul, EA_PTRSIZE, tmpReg, indexReg);
-            }
-            else
-#endif // _TARGET_64BIT_
-            {
-                getEmitter()->emitIns_R_I(emitter::inst3opImulForReg(tmpReg), EA_PTRSIZE, indexReg,
-                                          static_cast<ssize_t>(scale));
-            }
+            // IMUL treats its immediate operand as signed so scale can't be larger than INT32_MAX.
+            // The VM doesn't allow such large array elements but let's be sure.
+            noway_assert(scale <= INT32_MAX);
+#else  // !_TARGET_64BIT_
+            tmpReg = node->GetSingleTempReg();
+#endif // !_TARGET_64BIT_
+
+            getEmitter()->emitIns_R_I(emitter::inst3opImulForReg(tmpReg), EA_PTRSIZE, indexReg,
+                                      static_cast<ssize_t>(scale));
             scale = 1;
             break;
     }

diff --git a/src/jit/lsraxarch.cpp b/src/jit/lsraxarch.cpp
@@ -659,25 +659,28 @@ int LinearScan::BuildNode(GenTree* tree)
         {
             assert(dstCount == 1);
             RefPosition* internalDef = nullptr;
-            if (tree->AsIndexAddr()->Index()->TypeGet() == TYP_I_IMPL)
-            {
-                internalDef = buildInternalIntRegisterDefForNode(tree);
-            }
-            else
-            {
-                switch (tree->AsIndexAddr()->gtElemSize)
-                {
-                    case 1:
-                    case 2:
-                    case 4:
-                    case 8:
-                        break;
-
-                    default:
-                        internalDef = buildInternalIntRegisterDefForNode(tree);
-                        break;
-                }
+#ifdef _TARGET_64BIT_
+            // On 64-bit we always need a temporary register:
+            //   - if the index is `native int` then we need to load the array
+            //     length into a register to widen it to `native int`
+            //   - if the index is `int` (or smaller) then we need to widen
+            //     it to `long` to peform the address calculation
+            internalDef = buildInternalIntRegisterDefForNode(tree);
+#else  // !_TARGET_64BIT_
+            assert(!varTypeIsLong(tree->AsIndexAddr()->Index()->TypeGet()));
+            switch (tree->AsIndexAddr()->gtElemSize)
+            {
+                case 1:
+                case 2:
+                case 4:
+                case 8:
+                    break;
+
+                default:
+                    internalDef = buildInternalIntRegisterDefForNode(tree);
+                    break;
             }
+#endif // !_TARGET_64BIT_
             srcCount = BuildBinaryUses(tree->AsOp());
             if (internalDef != nullptr)
             {