From e5b4c7b611414e189b967254914c84aad46fd1ad Mon Sep 17 00:00:00 2001 From: Hanjoung Lee Date: Tue, 25 Jul 2017 20:50:01 +0900 Subject: [PATCH 1/5] [RyuJIT/armel] Make RefPosition arg regs fixed Passing double arg regs via `r0:r1` or `r2:r3`, each RefPosition's RegMask should have only one exact register so the candidate register can be fixed. e.g.) ``` BB35 regmask=[r0-r1] last> BB35 regmask=[r0-r1] last> ``` to be: ``` BB35 regmask=[r0] last fixed> BB35 regmask=[r1] last fixed> ``` Fix #12994 --- src/jit/lsra.cpp | 9 +++++++++ src/jit/regset.cpp | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/jit/lsra.cpp b/src/jit/lsra.cpp index 9c4f7d888072..691865df9b01 100644 --- a/src/jit/lsra.cpp +++ b/src/jit/lsra.cpp @@ -3985,6 +3985,15 @@ void LinearScan::buildRefPositionsForNode(GenTree* tree, #endif // DEBUG regMaskTP candidates = getUseCandidates(useNode); +#ifdef ARM_SOFTFP + // If oper is GT_PUTARG_REG, set bits in useCandidates must be in sequential order. + if (useNode->OperGet() == GT_PUTARG_REG || useNode->OperGet() == GT_COPY) + { + regMaskTP candidate = genFindLowestReg(candidates); + useNode->gtLsraInfo.setSrcCandidates(this, candidates & ~candidate); + candidates = candidate; + } +#endif // ARM_SOFTFP assert((candidates & allRegs(i->registerType)) != 0); // For non-localVar uses we record nothing, as nothing needs to be written back to the tree. diff --git a/src/jit/regset.cpp b/src/jit/regset.cpp index 3f3acc0a8910..6c1a98d07718 100644 --- a/src/jit/regset.cpp +++ b/src/jit/regset.cpp @@ -1608,7 +1608,7 @@ void RegSet::rsSpillTree(regNumber reg, GenTreePtr tree, unsigned regIdx /* =0 * #endif // _TARGET_ARM_ else { - assert(!varTypeIsMultiReg(tree)); + assert(!varTypeIsMultiReg(tree) || (m_rsCompiler->opts.compUseSoftFP && treeType == TYP_LONG)); tree->gtFlags &= ~GTF_SPILL; } #endif // !LEGACY_BACKEND From 0e0e60d758ac330e873d901c53672411961ac5c5 Mon Sep 17 00:00:00 2001 From: Hanjoung Lee Date: Tue, 1 Aug 2017 11:45:17 +0900 Subject: [PATCH 2/5] [RyuJIT/armel] Each reg gets own SpillFlag for MultiRegOp --- src/jit/codegenlinear.cpp | 42 ++++++++++++ src/jit/gentree.h | 131 ++++++++++++++++++++++++++++++++++++++ src/jit/lsra.cpp | 7 +- src/jit/regset.cpp | 26 +++++++- 4 files changed, 204 insertions(+), 2 deletions(-) diff --git a/src/jit/codegenlinear.cpp b/src/jit/codegenlinear.cpp index 59d08de9cee8..dfe0458c512e 100644 --- a/src/jit/codegenlinear.cpp +++ b/src/jit/codegenlinear.cpp @@ -1032,6 +1032,32 @@ void CodeGen::genUnspillRegIfNeeded(GenTree* tree) } } + unspillTree->gtFlags &= ~GTF_SPILLED; + } + else if (unspillTree->OperIsMultiRegOp()) + { + GenTreeMultiRegOp* multiReg = unspillTree->AsMultiRegOp(); + unsigned regCount = multiReg->gtOtherReg == REG_NA ? 1 : 2; + + // In case of split struct argument node, GTF_SPILLED flag on it indicates that + // one or more of its result regs are spilled. Call node needs to be + // queried to know which specific result regs to be unspilled. + for (unsigned i = 0; i < regCount; ++i) + { + unsigned flags = multiReg->GetRegSpillFlagByIdx(i); + if ((flags & GTF_SPILLED) != 0) + { + var_types dstType = multiReg->GetRegType(i); + regNumber dstReg = multiReg->GetRegNumByIdx(i); + + TempDsc* t = regSet.rsUnspillInPlace(multiReg, dstReg, i); + getEmitter()->emitIns_R_S(ins_Load(dstType), emitActualTypeSize(dstType), dstReg, t->tdTempNum(), + 0); + compiler->tmpRlsTemp(t); + gcInfo.gcMarkRegPtrVal(dstReg, dstType); + } + } + unspillTree->gtFlags &= ~GTF_SPILLED; } #endif @@ -1656,6 +1682,22 @@ void CodeGen::genProduceReg(GenTree* tree) } } } + else if (tree->OperIsMultiRegOp()) + { + GenTreeMultiRegOp* multiReg = tree->AsMultiRegOp(); + unsigned regCount = multiReg->gtOtherReg == REG_NA ? 1 : 2; + + for (unsigned i = 0; i < regCount; ++i) + { + unsigned flags = multiReg->GetRegSpillFlagByIdx(i); + if ((flags & GTF_SPILL) != 0) + { + regNumber reg = multiReg->GetRegNumByIdx(i); + regSet.rsSpillTree(reg, multiReg, i); + gcInfo.gcMarkRegSetNpt(genRegMask(reg)); + } + } + } #endif // _TARGET_ARM_ else { diff --git a/src/jit/gentree.h b/src/jit/gentree.h index 365dca6b7b69..54545aa896de 100644 --- a/src/jit/gentree.h +++ b/src/jit/gentree.h @@ -1333,6 +1333,18 @@ struct GenTree return OperIsPutArgStk() || OperIsPutArgReg() || OperIsPutArgSplit(); } + bool OperIsMultiRegOp() const + { +#if !defined(LEGACY_BACKEND) && defined(_TARGET_ARM_) + if (gtOper == GT_MUL_LONG || gtOper == GT_PUTARG_REG || gtOper == GT_COPY) + { + return true; + } +#endif + + return false; + } + bool OperIsAddrMode() const { return OperIsAddrMode(OperGet()); @@ -3894,9 +3906,128 @@ struct GenTreeMultiRegOp : public GenTreeOp { regNumber gtOtherReg; + // GTF_SPILL or GTF_SPILLED flag on a multi-reg call node indicates that one or + // more of its result regs are in that state. The spill flag of each of the + // return register is stored here. We only need 2 bits per returned register, + // so this is treated as a 2-bit array. No architecture needs more than 8 bits. + + static const unsigned PACKED_GTF_SPILL = 1; + static const unsigned PACKED_GTF_SPILLED = 2; + unsigned char gtSpillFlags; + GenTreeMultiRegOp(genTreeOps oper, var_types type, GenTreePtr op1, GenTreePtr op2) : GenTreeOp(oper, type, op1, op2), gtOtherReg(REG_NA) { + ClearOtherRegFlags(); + } + + //--------------------------------------------------------------------------- + // GetRegNumByIdx: get ith register allocated to this struct argument. + // + // Arguments: + // idx - index of the register + // + // Return Value: + // Return regNumber of ith register of this register argument + // + regNumber GetRegNumByIdx(unsigned idx) const + { + assert(idx < 2); + + if (idx == 0) + { + return gtRegNum; + } + + return gtOtherReg; + } + + //---------------------------------------------------------------------- + // GetRegSpillFlagByIdx: get spill flag associated with the register + // specified by its index. + // + // Arguments: + // idx - Position or index of the register + // + // Return Value: + // Returns GTF_* flags associated with the register. Only GTF_SPILL and GTF_SPILLED are considered. + // + unsigned GetRegSpillFlagByIdx(unsigned idx) const + { + assert(idx < MAX_REG_ARG); + + unsigned bits = gtSpillFlags >> (idx * 2); // It doesn't matter that we possibly leave other high bits here. + unsigned spillFlags = 0; + if (bits & PACKED_GTF_SPILL) + { + spillFlags |= GTF_SPILL; + } + if (bits & PACKED_GTF_SPILLED) + { + spillFlags |= GTF_SPILLED; + } + + return spillFlags; + } + + //---------------------------------------------------------------------- + // SetRegSpillFlagByIdx: set spill flags for the register + // specified by its index. + // + // Arguments: + // flags - GTF_* flags. Only GTF_SPILL and GTF_SPILLED are allowed. + // idx - Position or index of the register + // + // Return Value: + // None + // + void SetRegSpillFlagByIdx(unsigned flags, unsigned idx) + { + assert(idx < MAX_REG_ARG); + + unsigned bits = 0; + if (flags & GTF_SPILL) + { + bits |= PACKED_GTF_SPILL; + } + if (flags & GTF_SPILLED) + { + bits |= PACKED_GTF_SPILLED; + } + + // Clear anything that was already there by masking out the bits before 'or'ing in what we want there. + gtSpillFlags = (unsigned char)((gtSpillFlags & ~(0xffU << (idx * 2))) | (bits << (idx * 2))); + } + + //-------------------------------------------------------------------------- + // GetRegType: Get var_type of the register specified by index. + // + // Arguments: + // index - Index of the register. + // First register will have an index 0 and so on. + // + // Return Value: + // var_type of the register specified by its index. + + var_types GetRegType(unsigned index) + { + assert(index < 2); + var_types result = TYP_INT; // XXX + return result; + } + + //------------------------------------------------------------------- + // clearOtherRegFlags: clear GTF_* flags associated with gtOtherRegs + // + // Arguments: + // None + // + // Return Value: + // None + // + void ClearOtherRegFlags() + { + gtSpillFlags = 0; } #if DEBUGGABLE_GENTREE diff --git a/src/jit/lsra.cpp b/src/jit/lsra.cpp index 691865df9b01..e5c62fd34487 100644 --- a/src/jit/lsra.cpp +++ b/src/jit/lsra.cpp @@ -3987,7 +3987,7 @@ void LinearScan::buildRefPositionsForNode(GenTree* tree, regMaskTP candidates = getUseCandidates(useNode); #ifdef ARM_SOFTFP // If oper is GT_PUTARG_REG, set bits in useCandidates must be in sequential order. - if (useNode->OperGet() == GT_PUTARG_REG || useNode->OperGet() == GT_COPY) + if (useNode->OperIsMultiRegOp()) { regMaskTP candidate = genFindLowestReg(candidates); useNode->gtLsraInfo.setSrcCandidates(this, candidates & ~candidate); @@ -9239,6 +9239,11 @@ void LinearScan::resolveRegisters() GenTreePutArgSplit* splitArg = treeNode->AsPutArgSplit(); splitArg->SetRegSpillFlagByIdx(GTF_SPILL, currentRefPosition->getMultiRegIdx()); } + else if (treeNode->OperIsMultiRegOp()) + { + GenTreeMultiRegOp* multiReg = treeNode->AsMultiRegOp(); + multiReg->SetRegSpillFlagByIdx(GTF_SPILL, currentRefPosition->getMultiRegIdx()); + } #endif } diff --git a/src/jit/regset.cpp b/src/jit/regset.cpp index 6c1a98d07718..e73b64598f40 100644 --- a/src/jit/regset.cpp +++ b/src/jit/regset.cpp @@ -1533,6 +1533,7 @@ void RegSet::rsSpillTree(regNumber reg, GenTreePtr tree, unsigned regIdx /* =0 * var_types treeType; #if !defined(LEGACY_BACKEND) && defined(_TARGET_ARM_) GenTreePutArgSplit* splitArg = nullptr; + GenTreeMultiRegOp* multiReg = nullptr; #endif #ifndef LEGACY_BACKEND @@ -1548,6 +1549,11 @@ void RegSet::rsSpillTree(regNumber reg, GenTreePtr tree, unsigned regIdx /* =0 * splitArg = tree->AsPutArgSplit(); treeType = splitArg->GetRegType(regIdx); } + else if (tree->OperIsMultiRegOp()) + { + multiReg = tree->AsMultiRegOp(); + treeType = multiReg->GetRegType(regIdx); // XXX check + } #endif // _TARGET_ARM_ else #endif // !LEGACY_BACKEND @@ -1605,10 +1611,16 @@ void RegSet::rsSpillTree(regNumber reg, GenTreePtr tree, unsigned regIdx /* =0 * assert((regFlags & GTF_SPILL) != 0); regFlags &= ~GTF_SPILL; } + else if (multiReg != nullptr) + { + regFlags = multiReg->GetRegSpillFlagByIdx(regIdx); + assert((regFlags & GTF_SPILL) != 0); + regFlags &= ~GTF_SPILL; + } #endif // _TARGET_ARM_ else { - assert(!varTypeIsMultiReg(tree) || (m_rsCompiler->opts.compUseSoftFP && treeType == TYP_LONG)); + assert(!varTypeIsMultiReg(tree)); tree->gtFlags &= ~GTF_SPILL; } #endif // !LEGACY_BACKEND @@ -1759,6 +1771,11 @@ void RegSet::rsSpillTree(regNumber reg, GenTreePtr tree, unsigned regIdx /* =0 * regFlags |= GTF_SPILLED; splitArg->SetRegSpillFlagByIdx(regFlags, regIdx); } + else if (multiReg != nullptr) + { + regFlags |= GTF_SPILLED; + multiReg->SetRegSpillFlagByIdx(regFlags, regIdx); + } #endif // _TARGET_ARM_ #endif //! LEGACY_BACKEND } @@ -2401,6 +2418,13 @@ TempDsc* RegSet::rsUnspillInPlace(GenTreePtr tree, regNumber oldReg, unsigned re flags &= ~GTF_SPILLED; splitArg->SetRegSpillFlagByIdx(flags, regIdx); } + else if (tree->OperIsMultiRegOp()) + { + GenTreeMultiRegOp* multiReg = tree->AsMultiRegOp(); + unsigned flags = multiReg->GetRegSpillFlagByIdx(regIdx); + flags &= ~GTF_SPILLED; + multiReg->SetRegSpillFlagByIdx(flags, regIdx); + } #endif // !LEGACY_BACKEND && _TARGET_ARM_ else { From 45a2404fc8109aec9e8620e1596d63a832f4f2b4 Mon Sep 17 00:00:00 2001 From: Hanjoung Lee Date: Wed, 2 Aug 2017 17:19:55 +0900 Subject: [PATCH 3/5] [RyuJIT/armel] Allocate one more temp for call address --- src/jit/lsra.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/jit/lsra.cpp b/src/jit/lsra.cpp index e5c62fd34487..d54a174e5f34 100644 --- a/src/jit/lsra.cpp +++ b/src/jit/lsra.cpp @@ -8839,6 +8839,13 @@ void LinearScan::recordMaxSpill() maxSpill[TYP_FLOAT] += 1; } #endif // _TARGET_X86_ + + if (compiler->opts.compUseSoftFP) + { + JITDUMP("Adding a spill temp for moving target address to a register.\n"); + maxSpill[TYP_INT] += 1; + } + for (int i = 0; i < TYP_COUNT; i++) { if (var_types(i) != compiler->tmpNormalizeType(var_types(i))) From e36d674d2ed4acbfb1c165a45b7af7518426d375 Mon Sep 17 00:00:00 2001 From: Hanjoung Lee Date: Fri, 4 Aug 2017 17:20:34 +0900 Subject: [PATCH 4/5] [RyuJIT/armel] arg regs are always TYP_INT --- src/jit/lsra.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/jit/lsra.cpp b/src/jit/lsra.cpp index d54a174e5f34..3c096064a810 100644 --- a/src/jit/lsra.cpp +++ b/src/jit/lsra.cpp @@ -8839,13 +8839,6 @@ void LinearScan::recordMaxSpill() maxSpill[TYP_FLOAT] += 1; } #endif // _TARGET_X86_ - - if (compiler->opts.compUseSoftFP) - { - JITDUMP("Adding a spill temp for moving target address to a register.\n"); - maxSpill[TYP_INT] += 1; - } - for (int i = 0; i < TYP_COUNT; i++) { if (var_types(i) != compiler->tmpNormalizeType(var_types(i))) @@ -8923,6 +8916,15 @@ void LinearScan::updateMaxSpill(RefPosition* refPosition) ReturnTypeDesc* retTypeDesc = treeNode->AsCall()->GetReturnTypeDesc(); typ = retTypeDesc->GetReturnRegType(refPosition->getMultiRegIdx()); } +#ifdef _TARGET_ARM_ + else if (treeNode->OperIsPutArgReg()) + { + // For double arg regs, the type is changed to long since they must be passed via `r0-r3`. + // However when they get spilled, they should be treated as separated int registers. + var_types typNode = treeNode->TypeGet(); + typ = (typNode == TYP_LONG) ? TYP_INT : typNode; + } +#endif // _TARGET_ARM_ else { typ = treeNode->TypeGet(); From 99e4b01180b7a52a419a3a92c9f7b436489f52b3 Mon Sep 17 00:00:00 2001 From: Hanjoung Lee Date: Mon, 7 Aug 2017 11:35:21 +0900 Subject: [PATCH 5/5] [RyuJIT/armel] Fix ifdef condition and Formatting Error --- src/jit/lsra.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/jit/lsra.cpp b/src/jit/lsra.cpp index 3c096064a810..20449604155b 100644 --- a/src/jit/lsra.cpp +++ b/src/jit/lsra.cpp @@ -8916,15 +8916,15 @@ void LinearScan::updateMaxSpill(RefPosition* refPosition) ReturnTypeDesc* retTypeDesc = treeNode->AsCall()->GetReturnTypeDesc(); typ = retTypeDesc->GetReturnRegType(refPosition->getMultiRegIdx()); } -#ifdef _TARGET_ARM_ +#ifdef ARM_SOFTFP else if (treeNode->OperIsPutArgReg()) { // For double arg regs, the type is changed to long since they must be passed via `r0-r3`. // However when they get spilled, they should be treated as separated int registers. var_types typNode = treeNode->TypeGet(); - typ = (typNode == TYP_LONG) ? TYP_INT : typNode; + typ = (typNode == TYP_LONG) ? TYP_INT : typNode; } -#endif // _TARGET_ARM_ +#endif // ARM_SOFTFP else { typ = treeNode->TypeGet();