On Fri, Nov 25, 2016 at 8:23 AM, Richard Biener <richard.guent...@gmail.com> wrote: > On Thu, Nov 24, 2016 at 4:22 PM, Bin Cheng <bin.ch...@arm.com> wrote: >> Hi, >> This patch fixes two issues in newly introduced pattern: (cond (cmp >> (convert1? @1) @3) (convert2? @1) @2). >> For PR78507, we need to check if from_type is INTEGRAL_TYPE_P explicitly, >> this patch adds check for that. >> Note we don't check c1_type/c2_type for that because it's guarded by >> INTEGER_CST@. >> For PR78510, the ICE is covered by revision 242831, but underlying issue is >> when the block of conditions >> is not satisfied, the last case simplification is applied anyway since code >> is initialized to cmp at the >> first place. This patch fixes that by setting code to proper value only >> when transformation is valid, it >> also incorporates Marc's suggestion by using cmp directly. Two tests are >> added too. >> Bootstrap and test on x86_64 and AArch64 ongoing, is it OK if no failures? > > Ok. > > Btw, > > - (cond (cmp@0 (convert1? @1) ... > ... > - enum tree_code code = TREE_CODE (@0)... > > shows another misconception (I didn't notice...) in that TREE_CODE of a > captured > expression results in the expression code. On GENERIC that works but on > GIMPLE > @0 will be a SSA_NAME and thus 'code' was SSA_NAME ... Yes, that was a mistake. Patch updated by including new test from duplicate PR78517. Is it OK?
Thanks, bin 2016-11-24 Bin Cheng <bin.ch...@arm.com> PR middle-end/78507 PR middle-end/78510 PR middle-end/78517 * match.pd ((cond (cmp (convert1? @1) @3) (convert2? @1) @2)): Use cmp directly, rather than cmp_code. Initialize code to ERROR_MARK and set it to result code if transformation is valid. Use code EQ directly in last simplification case. gcc/testsuite/ChangeLog 2016-11-24 Bin Cheng <bin.ch...@arm.com> PR middle-end/78507 PR middle-end/78510 PR middle-end/78517 * g++.dg/torture/pr78507.C: New test. * gcc.dg/torture/pr78510.c: New test. * gcc.dg/torture/pr78517.c: New test. > > Thanks, > Richard. > >> Thanks, >> bin >> >> 2016-11-24 Bin Cheng <bin.ch...@arm.com> >> >> PR middle-end/78507 >> PR middle-end/78510 >> * match.pd ((cond (cmp (convert1? @1) @3) (convert2? @1) @2)): Use >> cmp directly, rather than cmp_code. Initialize code to ERROR_MARK >> and set it to result code if transformation is valid. Use code EQ >> directly in last simplification case. >> >> gcc/testsuite/ChangeLog >> 2016-11-24 Bin Cheng <bin.ch...@arm.com> >> >> PR middle-end/78507 >> PR middle-end/78510 >> * g++.dg/torture/pr78507.C: New test. >> * gcc.dg/torture/pr78510.c: New test.
diff --git a/gcc/match.pd b/gcc/match.pd index 3aa27fa..2d4e019 100644 --- a/gcc/match.pd +++ b/gcc/match.pd @@ -1971,14 +1971,15 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) (cond (eq (convert1? x) c1) (convert2? x) c2) -> (cond (eq x c1) c1 c2). */ (for cmp (lt le gt ge eq) (simplify - (cond (cmp@0 (convert1? @1) INTEGER_CST@3) (convert2? @1) INTEGER_CST@2) + (cond (cmp (convert1? @1) INTEGER_CST@3) (convert2? @1) INTEGER_CST@2) (with { tree from_type = TREE_TYPE (@1); tree c1_type = TREE_TYPE (@3), c2_type = TREE_TYPE (@2); - enum tree_code code = TREE_CODE (@0), cmp_code = TREE_CODE (@0); + enum tree_code code = ERROR_MARK; - if (int_fits_type_p (@2, from_type) + if (INTEGRAL_TYPE_P (from_type) + && int_fits_type_p (@2, from_type) && (types_match (c1_type, from_type) || (TYPE_PRECISION (c1_type) > TYPE_PRECISION (from_type) && (TYPE_UNSIGNED (from_type) @@ -1988,37 +1989,38 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) && (TYPE_UNSIGNED (from_type) || TYPE_SIGN (c2_type) == TYPE_SIGN (from_type))))) { - if (code != EQ_EXPR) + if (cmp != EQ_EXPR) { if (wi::to_widest (@3) == (wi::to_widest (@2) - 1)) { /* X <= Y - 1 equals to X < Y. */ - if (cmp_code == LE_EXPR) + if (cmp == LE_EXPR) code = LT_EXPR; /* X > Y - 1 equals to X >= Y. */ - if (cmp_code == GT_EXPR) + if (cmp == GT_EXPR) code = GE_EXPR; } if (wi::to_widest (@3) == (wi::to_widest (@2) + 1)) { /* X < Y + 1 equals to X <= Y. */ - if (cmp_code == LT_EXPR) + if (cmp == LT_EXPR) code = LE_EXPR; /* X >= Y + 1 equals to X > Y. */ - if (cmp_code == GE_EXPR) + if (cmp == GE_EXPR) code = GT_EXPR; } - if (code != cmp_code || wi::to_widest (@2) == wi::to_widest (@3)) + if (code != ERROR_MARK + || wi::to_widest (@2) == wi::to_widest (@3)) { - if (cmp_code == LT_EXPR || cmp_code == LE_EXPR) + if (cmp == LT_EXPR || cmp == LE_EXPR) code = MIN_EXPR; - if (cmp_code == GT_EXPR || cmp_code == GE_EXPR) + if (cmp == GT_EXPR || cmp == GE_EXPR) code = MAX_EXPR; } } /* Can do A == C1 ? A : C2 -> A == C1 ? C1 : C2? */ - else if (!int_fits_type_p (@3, from_type)) - code = ERROR_MARK; + else if (int_fits_type_p (@3, from_type)) + code = EQ_EXPR; } } (if (code == MAX_EXPR) @@ -2026,7 +2028,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) (if (code == MIN_EXPR) (convert (min @1 (convert @2))) (if (code == EQ_EXPR) - (convert (cond (cmp @1 (convert @3)) + (convert (cond (eq @1 (convert @3)) (convert:from_type @3) (convert:from_type @2))))))))) (for cnd (cond vec_cond) diff --git a/gcc/testsuite/g++.dg/torture/pr78507.C b/gcc/testsuite/g++.dg/torture/pr78507.C new file mode 100644 index 0000000..9691cf9 --- /dev/null +++ b/gcc/testsuite/g++.dg/torture/pr78507.C @@ -0,0 +1,57 @@ +// PR middle-end/78507 +// { dg-do compile } +struct A { + template <typename _Iterator1, typename _Iterator2> + int operator()(_Iterator1, _Iterator2); +}; +struct B { + template <typename _BI1, typename _BI2> + static _BI2 __copy_move_b(_BI1 p1, _BI2 p2) { + _BI1 a; + long b = p1 - a; + for (; b > 0; --b) + *--p2 = *--p1; + } +}; +template <int, typename _BI1, typename _BI2> +void __copy_move_backward_a(_BI1 p1, _BI2 p2) { + B::__copy_move_b(p1, p2); +} +template <int, typename _BI1, typename _BI2> +void __copy_move_backward_a2(_BI1 p1, _BI2 p2) { + __copy_move_backward_a<0>(p1, p2); +} +template <typename _BI1, typename _BI2> void move_backward(_BI1 p1, _BI2 p2) { + __copy_move_backward_a2<0>(p1, p2); +} +template <typename _RandomAccessIterator, typename _Compare> +void __insertion_sort(_RandomAccessIterator, _Compare p2) { + for (_RandomAccessIterator c;; ++c) + if (p2(0, 0)) + move_backward(c, c + 1); +} +template <typename _RandomAccessIterator, typename _Compare> +void __final_insertion_sort(_RandomAccessIterator, _Compare p2) { + _RandomAccessIterator d; + __insertion_sort(d, p2); +} +template <typename _RandomAccessIterator, typename _Compare> +void __sort(_RandomAccessIterator p1, _Compare p2) { + __final_insertion_sort(p1, p2); +} +template <typename _RandomAccessIterator, typename _Compare> +void sort(_RandomAccessIterator, _RandomAccessIterator p2, _Compare) { + A e; + __sort(p2, e); +} +struct C { + struct D { + int DwarfRegNum; + }; + int parseRegisterLiveOutMask() const; +}; +int C::parseRegisterLiveOutMask() const { + D f, g; + sort(&f, &g, [] {}); +} + diff --git a/gcc/testsuite/gcc.dg/torture/pr78510.c b/gcc/testsuite/gcc.dg/torture/pr78510.c new file mode 100644 index 0000000..8d0ab5f --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr78510.c @@ -0,0 +1,19 @@ +/* PR middle-end/78510 */ +/* { dg-do compile } */ +int a, b, c, e, f; +char d; +short g, h; +char fn1(int p1) { + for (;;) { + h = p1 << 2; + int i = h; + g = i > 32767 >> 13 ? i : i << 3; + f = a ?: c; + if (e) + return d; + } +} + +static int fn2() { fn1(0 || b); } + +int main() { fn2(); return 0; } diff --git a/gcc/testsuite/gcc.dg/torture/pr78517.c b/gcc/testsuite/gcc.dg/torture/pr78517.c new file mode 100644 index 0000000..475bce7 --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr78517.c @@ -0,0 +1,4 @@ +/* PR middle-end/78517 */ +/* { dg-do compile } */ +char a; +int fn1() { return a == '[' ? a : 0; }