Merge pull request #6326 from sivarv/phase2

Support for reg optional tree temps.

Author: sivarv

src/jit/codegencommon.cpp

@@ -1398,6 +1398,29 @@ regNumber           CodeGenInterface::genGetThisArgReg(GenTreePtr  call)
     return REG_ARG_0;
 }
 
+//----------------------------------------------------------------------
+// getSpillTempDsc: get the TempDsc corresponding to a spilled tree.
+//
+// Arguments:
+//   tree  -  spilled GenTree node
+//
+// Return Value:
+//   TempDsc corresponding to tree
+TempDsc*  CodeGenInterface::getSpillTempDsc(GenTree* tree)
+{
+    // tree must be in spilled state.
+    assert((tree->gtFlags & GTF_SPILLED) != 0);
+
+    // Get the tree's SpillDsc.
+    RegSet::SpillDsc* prevDsc;
+    RegSet::SpillDsc* spillDsc = regSet.rsGetSpillInfo(tree, tree->gtRegNum, &prevDsc);
+    assert(spillDsc != nullptr);
+
+    // Get the temp desc.
+    TempDsc* temp = regSet.rsGetSpillTempWord(tree->gtRegNum, spillDsc, prevDsc);
+    return temp;
+}
+
 #ifdef _TARGET_XARCH_
 
 #ifdef _TARGET_AMD64_

src/jit/codegeninterface.h

@@ -313,6 +313,9 @@ class CodeGenInterface
     void                SpillFloat              (regNumber reg, bool bIsCall = false);
 #endif // LEGACY_BACKEND
 
+    // The following method is used by xarch emitter for handling contained tree temps.
+    TempDsc*            getSpillTempDsc(GenTree* tree);
+
 public:
     emitter*            getEmitter()                { return m_cgEmitter; }
 protected:

src/jit/codegenxarch.cpp

@@ -1341,7 +1341,7 @@ void CodeGen::genCodeForDivMod(GenTreeOp* treeNode)
         {
             emit->emitInsBinary(genGetInsForOper(treeNode->gtOper, targetType), size, treeNode, divisor);
         }
-        else if (divisor->gtRegNum == targetReg)
+        else if (!divisor->isContained() && divisor->gtRegNum == targetReg)
         {
             // It is not possible to generate 2-operand divss or divsd where reg2 = reg1 / reg2
             // because divss/divsd reg1, reg2 will over-write reg1.  Therefore, in case of AMD64
@@ -1466,8 +1466,8 @@ void CodeGen::genCodeForBinary(GenTree* treeNode)
     // The arithmetic node must be sitting in a register (since it's not contained)
     noway_assert(targetReg != REG_NA);
 
-    regNumber op1reg = op1->gtRegNum;
-    regNumber op2reg = op2->gtRegNum;
+    regNumber op1reg = op1->isContained() ? REG_NA: op1->gtRegNum;
+    regNumber op2reg = op2->isContained() ? REG_NA: op2->gtRegNum;
 
     GenTreePtr dst;
     GenTreePtr src;
@@ -5148,7 +5148,11 @@ void CodeGen::genConsumeRegs(GenTree* tree)
 
     if (tree->isContained())
     {
-        if (tree->isIndir())
+        if (tree->isContainedSpillTemp())
+        {
+            // spill temps are un-tracked and hence no need to update life
+        }
+        else if (tree->isIndir())
         {
             genConsumeAddress(tree->AsIndir()->Addr());
         }

src/jit/emitxarch.cpp

@@ -2893,7 +2893,7 @@ regNumber emitter::emitInsBinary(instruction ins, emitAttr attr, GenTree* dst, G
     {
         dblConst = src->AsDblCon();
     }
-    
+
     // find local field if any
     GenTreeLclFld* lclField = nullptr;    
     if (src->isContainedLclField())
@@ -2918,9 +2918,25 @@ regNumber emitter::emitInsBinary(instruction ins, emitAttr attr, GenTree* dst, G
         lclVar = dst->AsLclVar();
     }
 
+    // find contained spill tmp if any
+    TempDsc* tmpDsc = nullptr;
+    if (src->isContainedSpillTemp())
+    {
+        assert(src->IsRegOptional());
+        tmpDsc = codeGen->getSpillTempDsc(src);
+    }
+    else if (dst->isContainedSpillTemp())
+    {
+        assert(dst->IsRegOptional());
+        tmpDsc = codeGen->getSpillTempDsc(dst);
+    }
+
     // First handle the simple non-memory cases
     //
-    if ((mem == nullptr) && (lclField == nullptr) && (lclVar == nullptr))
+    if ((mem == nullptr) && 
+        (lclField == nullptr) && 
+        (lclVar == nullptr) && 
+        (tmpDsc == nullptr))
     {
         if (intConst != nullptr)
         {
@@ -2959,7 +2975,7 @@ regNumber emitter::emitInsBinary(instruction ins, emitAttr attr, GenTree* dst, G
     // Next handle the cases where we have a stack based local memory operand.
     //
     unsigned varNum = BAD_VAR_NUM;
-    unsigned offset = (unsigned) -1;
+    unsigned offset = (unsigned)-1;
 
     if (lclField != nullptr)
     {
@@ -2971,12 +2987,22 @@ regNumber emitter::emitInsBinary(instruction ins, emitAttr attr, GenTree* dst, G
         varNum = lclVar->AsLclVarCommon()->GetLclNum();
         offset = 0;
     }
+    else if (tmpDsc != nullptr)
+    {
+        varNum = tmpDsc->tdTempNum();
+        offset = 0;
+    }
 
-    if (varNum != BAD_VAR_NUM)
+    // Spill temp numbers are negative and start with -1
+    // which also happens to be BAD_VAR_NUM. For this reason
+    // we also need to check 'tmpDsc != nullptr' here.
+    if (varNum != BAD_VAR_NUM ||
+        tmpDsc != nullptr)
     {
         // Is the memory op in the source position?
         if (src->isContainedLclField() ||
-            src->isContainedLclVar())
+            src->isContainedLclVar() ||
+            src->isContainedSpillTemp())
         {
             if (instrHasImplicitRegPairDest(ins))
             {
@@ -2993,7 +3019,7 @@ regNumber emitter::emitInsBinary(instruction ins, emitAttr attr, GenTree* dst, G
         }
         else  // The memory op is in the dest position.
         {
-            assert(dst->gtRegNum == REG_NA);
+            assert(dst->gtRegNum == REG_NA || dst->IsRegOptional());
 
             // src could be int or reg
             if (src->isContainedIntOrIImmed())
@@ -3011,6 +3037,11 @@ regNumber emitter::emitInsBinary(instruction ins, emitAttr attr, GenTree* dst, G
             }
         }
 
+        if (tmpDsc != nullptr)
+        {
+            emitComp->tmpRlsTemp(tmpDsc);
+        }
+
         return dst->gtRegNum;
     }
 

src/jit/gentree.cpp

@@ -13306,13 +13306,22 @@ GenTree::IsLclVarUpdateTree(GenTree** pOtherTree, genTreeOps *pOper)
 // until after the LSRA phase has allocated physical registers to the treenodes.
 bool GenTree::isContained() const
 {
+    if (isContainedSpillTemp())
+    {
+        return true;
+    }
+
     if (gtHasReg())
+    {
         return false;
+    }
 
     // these actually produce a register (the flags reg, we just don't model it)
     // and are a separate instruction from the branch that consumes the result
     if (OperKind() & GTK_RELOP)
+    {
         return false;
+    }
 
     // TODO-Cleanup : this is not clean, would be nice to have some way of marking this.
     switch (OperGet())

src/jit/gentree.h

@@ -495,15 +495,17 @@ struct GenTree
 
     bool isContainedLclField() const        { return isContained() && isLclField(); }
 
-    bool isContainedLclVar() const          {  return isContained() && (OperGet() == GT_LCL_VAR);  } 
+    bool isContainedLclVar() const          { return isContained() && (OperGet() == GT_LCL_VAR);  } 
+
+    bool isContainedSpillTemp() const;
 
     // Indicates whether it is a memory op.
     // Right now it includes Indir and LclField ops.
     bool isMemoryOp() const                 { return isIndir() || isLclField(); }
 
     bool isContainedMemoryOp() const        
     { 
-        return (isContained() && isMemoryOp()) || isContainedLclVar(); 
+        return (isContained() && isMemoryOp()) || isContainedLclVar() || isContainedSpillTemp(); 
     }
 
     regNumber GetRegNum() const
@@ -665,11 +667,13 @@ struct GenTree
     #define GTF_REVERSE_OPS     0x00000020  // operand op2 should be evaluated before op1 (normally, op1 is evaluated first and op2 is evaluated second)
     #define GTF_REG_VAL         0x00000040  // operand is sitting in a register (or part of a TYP_LONG operand is sitting in a register)
 
-    #define GTF_SPILLED         0x00000080  // the value   has been spilled
-    #define GTF_SPILLED_OPER    0x00000100  //   op1 has been spilled
+    #define GTF_SPILLED         0x00000080  // the value has been spilled
 
 #ifdef LEGACY_BACKEND
-    #define GTF_SPILLED_OP2     0x00000200  //   op2 has been spilled
+    #define GTF_SPILLED_OPER    0x00000100  // op1 has been spilled
+    #define GTF_SPILLED_OP2     0x00000200  // op2 has been spilled
+#else
+    #define GTF_NOREG_AT_USE    0x00000100  // tree node is in memory at the point of use
 #endif // LEGACY_BACKEND
 
     #define GTF_REDINDEX_CHECK  0x00000100  // Used for redundant range checks. Disjoint from GTF_SPILLED_OPER
@@ -4497,6 +4501,19 @@ GenTreeBlkOp::HasGCPtr()
     return false;
 }
 
+inline bool GenTree::isContainedSpillTemp() const
+{
+#if !defined(LEGACY_BACKEND)
+    // If spilled and no reg at use, then it is treated as contained.
+    if (((gtFlags & GTF_SPILLED) != 0) &&
+        ((gtFlags & GTF_NOREG_AT_USE) != 0))
+    {
+        return true;
+    }
+#endif //!LEGACY_BACKEND
+
+    return false;
+}
 
 /*****************************************************************************/
 

src/jit/lower.h

@@ -143,8 +143,62 @@ class Lowering : public Phase
     void TreeNodeInfoInit(GenTreePtr* tree, GenTree* parent);
 #if defined(_TARGET_XARCH_)
     void TreeNodeInfoInitSimple(GenTree* tree);
-    void SetRegOptionalForBinOp(GenTree* tree);
-    void TryToSetRegOptional(GenTree* operand);
+
+    //----------------------------------------------------------------------
+    // SetRegOptional - sets a bit to indicate to LSRA that register
+    // for a given tree node is optional for codegen purpose.  If no
+    // register is allocated to such a tree node, its parent node treats
+    // it as a contained memory operand during codegen.
+    //
+    // Arguments:
+    //    tree    -   GenTree node
+    //
+    // Returns
+    //    None
+    void SetRegOptional(GenTree* tree)
+    {
+        tree->gtLsraInfo.regOptional = true;
+    }
+    
+    GenTree* PreferredRegOptionalOperand(GenTree* tree);
+
+    // ------------------------------------------------------------------
+    // SetRegOptionalBinOp - Indicates which of the operands of a bin-op
+    // register requirement is optional. Xarch instruction set allows
+    // either of op1 or op2 of binary operation (e.g. add, mul etc) to be
+    // a memory operand.  This routine provides info to register allocator
+    // which of its operands optionally require a register.  Lsra might not
+    // allocate a register to RefTypeUse positions of such operands if it
+    // is beneficial. In such a case codegen will treat them as memory
+    // operands.
+    //
+    // Arguments:
+    //     tree  -  Gentree of a bininary operation.
+    //
+    // Returns 
+    //     None.
+    // 
+    // Note: On xarch at most only one of the operands will be marked as
+    // reg optional, even when both operands could be considered register
+    // optional.
+    void SetRegOptionalForBinOp(GenTree* tree)
+    {
+        assert(GenTree::OperIsBinary(tree->OperGet()));
+
+        GenTree* op1 = tree->gtGetOp1();
+        GenTree* op2 = tree->gtGetOp2();
+
+        if (tree->OperIsCommutative() &&
+            tree->TypeGet() == op1->TypeGet())
+        {
+            GenTree* preferredOp = PreferredRegOptionalOperand(tree);
+            SetRegOptional(preferredOp);
+        }
+        else if (tree->TypeGet() == op2->TypeGet())
+        {
+            SetRegOptional(op2);
+        }
+    }
 #endif // defined(_TARGET_XARCH_)
     void TreeNodeInfoInitReturn(GenTree* tree);
     void TreeNodeInfoInitShiftRotate(GenTree* tree);