From d1d7a46db4d75f9ef323fc4ae5b4a1efc661476b Mon Sep 17 00:00:00 2001 From: Bruce Forstall Date: Wed, 4 May 2016 09:41:57 -0700 Subject: [PATCH] RyuJIT/x86: convert SBCG to NYI for multi-level expression trees In general, we decompose longs by walking the tree bottom-up. So, e.g., we'll decompose 'add' as follows: + (gt_long(lo1, hi1), gt_long(lo2, hi2)) => gt_long (+ (lo1, lo2), +Hi (hi1, hi2)) But what if lo1 is '+' and hi1 is '+Hi'? So, + (gt_long( + (lo3, lo4), +Hi (hi3, hi4) ), gt_long(lo2, hi2)) => gt_long (+ ( + (lo3, lo4), lo2), +Hi ( +Hi (hi3, hi4), hi2)) But we can't put any instructions between '+ (lo3, lo4)' and '+Hi (hi3, hi4)' or we'll lose the carry bit. We need to model this differently, or introduce temps. Until this is done, convert the silent bad codegen into NYI fallback. --- src/jit/gentree.h | 23 +++++++++++++++++++++++ src/jit/lower.cpp | 21 +++++++++++++++++++++ tests/issues.targets | 12 ------------ 3 files changed, 44 insertions(+), 12 deletions(-) diff --git a/src/jit/gentree.h b/src/jit/gentree.h index 550a579..2f7435b 100644 --- a/src/jit/gentree.h +++ b/src/jit/gentree.h @@ -1153,6 +1153,29 @@ public: || OperIsShiftOrRotate(op); } +#if !defined(LEGACY_BACKEND) && !defined(_TARGET_64BIT_) + static + bool OperIsHigh(genTreeOps gtOper) + { + switch (gtOper) + { + case GT_ADD_HI: + case GT_SUB_HI: + case GT_MUL_HI: + case GT_DIV_HI: + case GT_MOD_HI: + return true; + default: + return false; + } + } + + bool OperIsHigh() const + { + return OperIsHigh(OperGet()); + } +#endif // !defined(LEGACY_BACKEND) && !defined(_TARGET_64BIT_) + static int OperIsUnary(genTreeOps gtOper) { diff --git a/src/jit/lower.cpp b/src/jit/lower.cpp index ff665fd..a5dd1ab 100644 --- a/src/jit/lower.cpp +++ b/src/jit/lower.cpp @@ -428,6 +428,27 @@ void Lowering::DecomposeNode(GenTreePtr* pTree, Compiler::fgWalkData* data) GenTree* loOp2 = op2->gtGetOp1(); GenTree* hiOp2 = op2->gtGetOp2(); + // We don't have support to decompose a TYP_LONG node that already has a child that has + // been decomposed into parts, where the high part depends on the value generated by the + // low part (via the flags register). For example, if we have: + // +(gt_long(+(lo3, lo4), +Hi(hi3, hi4)), gt_long(lo2, hi2)) + // We would decompose it here to: + // gt_long(+(+(lo3, lo4), lo2), +Hi(+Hi(hi3, hi4), hi2)) + // But this would generate incorrect code, because the "+Hi(hi3, hi4)" code generation + // needs to immediately follow the "+(lo3, lo4)" part. Also, if this node is one that + // requires a unique high operator, and the child nodes are not simple locals (e.g., + // they are decomposed nodes), then we also can't decompose the node, as we aren't + // guaranteed the high and low parts will be executed immediately after each other. + + NYI_IF(hiOp1->OperIsHigh() || + hiOp2->OperIsHigh() || + (GenTree::OperIsHigh(getHiOper(oper)) && + (!loOp1->OperIsLeaf() || + !hiOp1->OperIsLeaf() || + !loOp1->OperIsLeaf() || + !hiOp2->OperIsLeaf())), + "Can't decompose expression tree TYP_LONG node"); + // Now, remove op1 and op2 from the node list. comp->fgSnipNode(curStmt, op1); comp->fgSnipNode(curStmt, op2); diff --git a/tests/issues.targets b/tests/issues.targets index 0e6d050..4c05a0c 100644 --- a/tests/issues.targets +++ b/tests/issues.targets @@ -401,18 +401,6 @@ - - 4659 - - - 4659 - - - 4659 - - - 4659 - 3554 -- 2.7.4