From 65e6b59208632a32503d7670e3eb1c4d7983b9f0 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Tue, 21 Feb 2023 10:17:31 -0800 Subject: [PATCH 1/3] remove FEATURE_MULTIREG_RET check --- src/coreclr/jit/gentree.cpp | 8 +++++--- src/coreclr/jit/gentree.h | 37 +++++++++++++++++++------------------ 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 16fe9b2f19cc05..89889fe6e4198e 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -975,12 +975,12 @@ bool GenTree::IsMultiRegNode() const return AsMultiRegOp()->GetRegCount() > 1; } #endif +#endif // FEATURE_MULTIREG_RET if (OperIs(GT_COPY, GT_RELOAD)) { return true; } -#endif // FEATURE_MULTIREG_RET #ifdef FEATURE_HW_INTRINSICS if (OperIsHWIntrinsic()) @@ -1026,12 +1026,13 @@ unsigned GenTree::GetMultiRegCount(Compiler* comp) const return AsMultiRegOp()->GetRegCount(); } #endif +#endif // FEATURE_MULTIREG_RET if (OperIs(GT_COPY, GT_RELOAD)) { return AsCopyOrReload()->GetRegCount(); } -#endif // FEATURE_MULTIREG_RET + #ifdef FEATURE_HW_INTRINSICS if (OperIsHWIntrinsic()) @@ -11696,7 +11697,8 @@ void Compiler::gtDispLeaf(GenTree* tree, IndentStack* indentStack) fieldVarDsc->PrintVarReg(); } - if (fieldVarDsc->lvTracked && fgLocalVarLivenessDone && tree->AsLclVar()->IsLastUse(index)) + if (fieldVarDsc->lvTracked && fgLocalVarLivenessDone && + tree->AsLclVarCommon()->IsLastUse(index)) { printf(" (last use)"); } diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 243474c42eb92f..8088f6cf961c57 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -8044,12 +8044,12 @@ struct GenTreePutArgSplit : public GenTreePutArgStk // struct GenTreeCopyOrReload : public GenTreeUnOp { -#if FEATURE_MULTIREG_RET +//#if FEATURE_MULTIREG_RET // State required to support copy/reload of a multi-reg call node. // The first register is always given by GetRegNum(). // regNumberSmall gtOtherRegs[MAX_MULTIREG_COUNT - 1]; -#endif +//#endif //---------------------------------------------------------- // ClearOtherRegs: set gtOtherRegs to REG_NA. @@ -8062,12 +8062,12 @@ struct GenTreeCopyOrReload : public GenTreeUnOp // void ClearOtherRegs() { -#if FEATURE_MULTIREG_RET +//#if FEATURE_MULTIREG_RET for (unsigned i = 0; i < MAX_MULTIREG_COUNT - 1; ++i) { gtOtherRegs[i] = REG_NA; } -#endif +//#endif } //----------------------------------------------------------- @@ -8088,11 +8088,11 @@ struct GenTreeCopyOrReload : public GenTreeUnOp return GetRegNum(); } -#if FEATURE_MULTIREG_RET +//#if FEATURE_MULTIREG_RET return (regNumber)gtOtherRegs[idx - 1]; -#else - return REG_NA; -#endif +//#else + //return REG_NA; +//#endif } //----------------------------------------------------------- @@ -8113,18 +8113,18 @@ struct GenTreeCopyOrReload : public GenTreeUnOp { SetRegNum(reg); } -#if FEATURE_MULTIREG_RET +//#if FEATURE_MULTIREG_RET else { gtOtherRegs[idx - 1] = (regNumberSmall)reg; assert(gtOtherRegs[idx - 1] == reg); } -#else - else - { - unreached(); - } -#endif +//#else +// else +// { +// unreached(); +// } +//#endif } //---------------------------------------------------------------------------- @@ -8153,7 +8153,7 @@ struct GenTreeCopyOrReload : public GenTreeUnOp unsigned GetRegCount() const { -#if FEATURE_MULTIREG_RET +//#if FEATURE_MULTIREG_RET // We need to return the highest index for which we have a valid register. // Note that the gtOtherRegs array is off by one (the 0th register is GetRegNum()). // If there's no valid register in gtOtherRegs, GetRegNum() must be valid. @@ -8168,7 +8168,7 @@ struct GenTreeCopyOrReload : public GenTreeUnOp return i; } } -#endif +//#endif // We should never have a COPY or RELOAD with no valid registers. assert(GetRegNum() != REG_NA); return 1; @@ -9097,12 +9097,13 @@ inline regNumber GenTree::GetRegByIndex(int regIndex) const return AsMultiRegOp()->GetRegNumByIdx(regIndex); } #endif +#endif // FEATURE_MULTIREG_RET if (OperIs(GT_COPY, GT_RELOAD)) { return AsCopyOrReload()->GetRegNumByIdx(regIndex); } -#endif // FEATURE_MULTIREG_RET + #ifdef FEATURE_HW_INTRINSICS if (OperIs(GT_HWINTRINSIC)) From ab7ffa3a8e88850a8928643af1fb147f716c3eaf Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Tue, 21 Feb 2023 14:35:44 -0800 Subject: [PATCH 2/3] Remove comments --- src/coreclr/jit/gentree.cpp | 1 - src/coreclr/jit/gentree.h | 22 ++-------------------- 2 files changed, 2 insertions(+), 21 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 89889fe6e4198e..2b0dac9eec1fc4 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -1033,7 +1033,6 @@ unsigned GenTree::GetMultiRegCount(Compiler* comp) const return AsCopyOrReload()->GetRegCount(); } - #ifdef FEATURE_HW_INTRINSICS if (OperIsHWIntrinsic()) { diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 8088f6cf961c57..bfeaca33ddf7fe 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -8039,17 +8039,14 @@ struct GenTreePutArgSplit : public GenTreePutArgStk // Represents GT_COPY or GT_RELOAD node // // As it turns out, these are only needed on targets that happen to have multi-reg returns. -// However, they are actually needed on any target that has any multi-reg ops. It is just -// coincidence that those are the same (and there isn't a FEATURE_MULTIREG_OPS). +// However, they are actually needed on any target that has any multi-reg ops. // struct GenTreeCopyOrReload : public GenTreeUnOp { -//#if FEATURE_MULTIREG_RET // State required to support copy/reload of a multi-reg call node. // The first register is always given by GetRegNum(). // regNumberSmall gtOtherRegs[MAX_MULTIREG_COUNT - 1]; -//#endif //---------------------------------------------------------- // ClearOtherRegs: set gtOtherRegs to REG_NA. @@ -8062,12 +8059,10 @@ struct GenTreeCopyOrReload : public GenTreeUnOp // void ClearOtherRegs() { -//#if FEATURE_MULTIREG_RET for (unsigned i = 0; i < MAX_MULTIREG_COUNT - 1; ++i) { gtOtherRegs[i] = REG_NA; } -//#endif } //----------------------------------------------------------- @@ -8088,11 +8083,7 @@ struct GenTreeCopyOrReload : public GenTreeUnOp return GetRegNum(); } -//#if FEATURE_MULTIREG_RET return (regNumber)gtOtherRegs[idx - 1]; -//#else - //return REG_NA; -//#endif } //----------------------------------------------------------- @@ -8113,18 +8104,11 @@ struct GenTreeCopyOrReload : public GenTreeUnOp { SetRegNum(reg); } -//#if FEATURE_MULTIREG_RET else { gtOtherRegs[idx - 1] = (regNumberSmall)reg; assert(gtOtherRegs[idx - 1] == reg); } -//#else -// else -// { -// unreached(); -// } -//#endif } //---------------------------------------------------------------------------- @@ -8153,7 +8137,6 @@ struct GenTreeCopyOrReload : public GenTreeUnOp unsigned GetRegCount() const { -//#if FEATURE_MULTIREG_RET // We need to return the highest index for which we have a valid register. // Note that the gtOtherRegs array is off by one (the 0th register is GetRegNum()). // If there's no valid register in gtOtherRegs, GetRegNum() must be valid. @@ -8168,7 +8151,7 @@ struct GenTreeCopyOrReload : public GenTreeUnOp return i; } } -//#endif + // We should never have a COPY or RELOAD with no valid registers. assert(GetRegNum() != REG_NA); return 1; @@ -9104,7 +9087,6 @@ inline regNumber GenTree::GetRegByIndex(int regIndex) const return AsCopyOrReload()->GetRegNumByIdx(regIndex); } - #ifdef FEATURE_HW_INTRINSICS if (OperIs(GT_HWINTRINSIC)) { From d032b8eaa8f979a7dfa812b32db67a01b250c2c6 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Wed, 22 Feb 2023 09:31:32 -0800 Subject: [PATCH 3/3] updated comment --- src/coreclr/jit/gentree.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index bfeaca33ddf7fe..514223546f6864 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -8038,8 +8038,7 @@ struct GenTreePutArgSplit : public GenTreePutArgStk // Represents GT_COPY or GT_RELOAD node // -// As it turns out, these are only needed on targets that happen to have multi-reg returns. -// However, they are actually needed on any target that has any multi-reg ops. +// Needed to support multi-reg ops. // struct GenTreeCopyOrReload : public GenTreeUnOp {