diff mbox

combine: Handle aborts in is_parallel_of_n_reg_sets (PR68381)

Message ID 565464A8.4000400@arm.com
State New
Headers show

Commit Message

Kyrylo Tkachov Nov. 24, 2015, 1:22 p.m. UTC 
On 24/11/15 06:37, Segher Boessenkool wrote:
> Some users of is_parallel_of_n_reg_sets disregard the clobbers in a

> parallel after it has returned "yes, this is a parallel of N sets and

> maybe some clobbers".  But combine uses a clobber of const0_rtx to

> indicate substitution failure, so this leads to disaster.

>

> Fix this by checking for such special clobbers in is_parallel_of_n_reg_sets.

>

> Tested on powerpc64-linux.  Also tested with Kyrill's testcase, manually

> inspected the generated asm and the combine dump file (with some extra

> instrumentation).  This testcase needs -O1 btw.

>

> The "performance problem" in the PR (same testcase, but with -O3) is

> a missed jump optimization: a pseudo is set (to 0) in one BB (and

> nowhere else), and then tested against 0 in another BB.  Nothing after

> combine seems to handle this.

>

> Applying this patch to trunk.  Kyrill, could you handle the testcase?

> Together with whatever you decide should be done for the -O3 problem.

> Thank you for tracking down this nastiness!


Sure, here's the testcase with the "-O" in the dg-options I had left out.
Applying to trunk.

For the performance issue I'll investigate a bit further.

Thanks,
Kyrill

2015-11-24  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     PR rtl-optimization/68381
     * gcc.c-torture/execute/pr68381.c: New test.

>

> Segher

>

>

> 2015-11-24  Segher Boessenkool  <segher@kernel.crashing.org>

>

> 	PR rtl-optimization/68381

> 	* combine.c (is_parallel_of_n_reg_sets): Return false if the pattern

> 	is poisoned.

>

> ---

>   gcc/combine.c | 3 ++-

>   1 file changed, 2 insertions(+), 1 deletion(-)

>

> diff --git a/gcc/combine.c b/gcc/combine.c

> index 2a66fd5..4958d3b 100644

> --- a/gcc/combine.c

> +++ b/gcc/combine.c

> @@ -2512,7 +2512,8 @@ is_parallel_of_n_reg_sets (rtx pat, int n)

>   	|| !REG_P (SET_DEST (XVECEXP (pat, 0, i))))

>         return false;

>     for ( ; i < len; i++)

> -    if (GET_CODE (XVECEXP (pat, 0, i)) != CLOBBER)

> +    if (GET_CODE (XVECEXP (pat, 0, i)) != CLOBBER

> +	|| XEXP (XVECEXP (pat, 0, i), 0) == const0_rtx)

>         return false;

>   

>     return true;
diff mbox

Patch

commit e5ab9ee234cb7f4b25e5aa56e64cf3a8a0e56f58
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Tue Nov 17 15:33:58 2015 +0000

    [combine] PR rtl-optimization/68381: Only restrict pure simplification in mult-extend subst case, allow other substitutions

diff --git a/gcc/testsuite/gcc.c-torture/execute/pr68381.c b/gcc/testsuite/gcc.c-torture/execute/pr68381.c
new file mode 100644
index 0000000..cb6abcb
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr68381.c
@@ -0,0 +1,22 @@ 
+/* { dg-options "-O -fexpensive-optimizations -fno-tree-bit-ccp" } */
+
+__attribute__ ((noinline, noclone))
+int
+foo (unsigned short x, unsigned short y)
+{
+  int r;
+  if (__builtin_mul_overflow (x, y, &r))
+    __builtin_abort ();
+  return r;
+}
+
+int
+main (void)
+{
+  int x = 1;
+  int y = 2;
+  if (foo (x, y) != x * y)
+    __builtin_abort ();
+  return 0;
+}
+