From 01ab88cd6548eeb0adaeeff9f312645bd34c187e Mon Sep 17 00:00:00 2001 From: Michelle McDaniel Date: Mon, 2 May 2016 13:52:39 -0700 Subject: [PATCH] Set overflow flag for add/sub hi on x86 TYP_LONG When we create the hi operation for add and sub on TYP_LONG, we don't carry the overflow flag to the hi operation. This change sets the overflow flag on hiResult in lower if it was set on loResult, and adds GT_ADD_HI and GT_SUB_HI to the operations that can have overflow. We also need to pass the unsigned flag to the high part in the instance that we are dealing with an add or subtract with overflow. Fixes dotnet/coreclr#4596. Commit migrated from https://github.com/dotnet/coreclr/commit/230d693afc8b253df012fb4569e42ba731e6f5d2 --- src/coreclr/src/jit/codegenxarch.cpp | 5 +++++ src/coreclr/src/jit/compiler.hpp | 10 ++++++++++ src/coreclr/src/jit/lower.cpp | 12 ++++++++++++ src/coreclr/tests/issues.targets | 15 --------------- src/coreclr/tests/ryujit_x86_no_fallback_issues.targets | 17 ++++------------- 5 files changed, 31 insertions(+), 28 deletions(-) diff --git a/src/coreclr/src/jit/codegenxarch.cpp b/src/coreclr/src/jit/codegenxarch.cpp index b75dc9f..edd485b 100644 --- a/src/coreclr/src/jit/codegenxarch.cpp +++ b/src/coreclr/src/jit/codegenxarch.cpp @@ -1464,7 +1464,12 @@ void CodeGen::genCodeForBinary(GenTree* treeNode) if (treeNode->gtOverflowEx()) { +#if !defined(_TARGET_64BIT_) + assert(oper == GT_ADD || oper == GT_SUB || + oper == GT_ADD_HI || oper == GT_SUB_HI); +#else assert(oper == GT_ADD || oper == GT_SUB); +#endif genCheckOverflow(treeNode); } genProduceReg(treeNode); diff --git a/src/coreclr/src/jit/compiler.hpp b/src/coreclr/src/jit/compiler.hpp index ec9cba3..d8cb640 100644 --- a/src/coreclr/src/jit/compiler.hpp +++ b/src/coreclr/src/jit/compiler.hpp @@ -1525,9 +1525,16 @@ bool GenTree::IsVarAddr() const inline bool GenTree::gtOverflow() const { +#if !defined(_TARGET_64BIT_) && !defined(LEGACY_BACKEND) + assert(gtOper == GT_MUL || gtOper == GT_CAST || + gtOper == GT_ADD || gtOper == GT_SUB || + gtOper == GT_ASG_ADD || gtOper == GT_ASG_SUB || + gtOper == GT_ADD_HI || gtOper == GT_SUB_HI); +#else assert(gtOper == GT_MUL || gtOper == GT_CAST || gtOper == GT_ADD || gtOper == GT_SUB || gtOper == GT_ASG_ADD || gtOper == GT_ASG_SUB); +#endif if (gtFlags & GTF_OVERFLOW) { @@ -1546,6 +1553,9 @@ bool GenTree::gtOverflowEx() const { if ( gtOper == GT_MUL || gtOper == GT_CAST || gtOper == GT_ADD || gtOper == GT_SUB || +#if !defined(_TARGET_64BIT_) && !defined(LEGACY_BACKEND) + gtOper == GT_ADD_HI || gtOper == GT_SUB_HI || +#endif gtOper == GT_ASG_ADD || gtOper == GT_ASG_SUB) { return gtOverflow(); diff --git a/src/coreclr/src/jit/lower.cpp b/src/coreclr/src/jit/lower.cpp index b742f01..03c47e0 100644 --- a/src/coreclr/src/jit/lower.cpp +++ b/src/coreclr/src/jit/lower.cpp @@ -469,6 +469,18 @@ void Lowering::DecomposeNode(GenTreePtr* pTree, Compiler::fgWalkData* data) hiOp2->gtNext = hiResult; hiResult->gtPrev = hiOp2; + if (oper == GT_ADD || oper == GT_SUB) + { + if (loResult->gtOverflow()) + { + hiResult->gtFlags |= GTF_OVERFLOW; + loResult->gtFlags &= ~GTF_OVERFLOW; + } + if (loResult->gtFlags & GTF_UNSIGNED) + { + hiResult->gtFlags |= GTF_UNSIGNED; + } + } // Below, we'll put the loResult and hiResult trees together, using the more // general fgInsertTreeInListAfter() method. } diff --git a/src/coreclr/tests/issues.targets b/src/coreclr/tests/issues.targets index f72d0bf..edddd60 100644 --- a/src/coreclr/tests/issues.targets +++ b/src/coreclr/tests/issues.targets @@ -428,21 +428,6 @@ - - 4596 - - - 4596 - - - 4596 - - - 4596 - - - 4596 - 4659 diff --git a/src/coreclr/tests/ryujit_x86_no_fallback_issues.targets b/src/coreclr/tests/ryujit_x86_no_fallback_issues.targets index ce74aef..def4e2d 100644 --- a/src/coreclr/tests/ryujit_x86_no_fallback_issues.targets +++ b/src/coreclr/tests/ryujit_x86_no_fallback_issues.targets @@ -16515,20 +16515,11 @@ 4186 - - 4596 - - - 4596 - - - 4596 - - - 4596 - - 4596 + 4751 + + + needs triage -- 2.7.4