Patchwork [PR44181] skip debug insns when inserting IV updates

login
register
mail settings
Submitter Alexandre Oliva
Date June 11, 2010, 7:18 p.m.
Message ID <oriq5pml7v.fsf@livre.localdomain>
Download mbox | patch
Permalink /patch/55350/
State New
Headers show

Comments

Alexandre Oliva - June 11, 2010, 7:18 p.m.
A number of graphite -fcompare-debug bug reports were fixed with this
patch.  The problem was that we inserted induction variable increment
stmts before the last stmt in the block, but if the block ended with a
fall-through, the insert would be before a non-debug stmt in the non-VTA
case, whereas in the VTA case it could be before a debug stmt, after the
non-debug stmt.  Skipping trailing debug stmts so that we insert before
the last non-debug stmt removed the codegen differences.

Bootstrapped the compiler, along with the upcoming patch for PR43656, on
x86_64-linux-gnu, with the following options:

-O2 -g -fschedule-insns -fsched-pressure -funroll-loops -fgraphite-identity

for stage2 and stage3 host, and for stage3 libs (also -fcompare-debug)

I got build failures on zlib and libada, presumably because of this
unusual combination of flags, but no -fcompare-debug errors.

Ok to install?
Richard Guenther - June 11, 2010, 11:16 p.m.
On Fri, Jun 11, 2010 at 9:18 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
> A number of graphite -fcompare-debug bug reports were fixed with this
> patch.  The problem was that we inserted induction variable increment
> stmts before the last stmt in the block, but if the block ended with a
> fall-through, the insert would be before a non-debug stmt in the non-VTA
> case, whereas in the VTA case it could be before a debug stmt, after the
> non-debug stmt.  Skipping trailing debug stmts so that we insert before
> the last non-debug stmt removed the codegen differences.
>
> Bootstrapped the compiler, along with the upcoming patch for PR43656, on
> x86_64-linux-gnu, with the following options:
>
> -O2 -g -fschedule-insns -fsched-pressure -funroll-loops -fgraphite-identity
>
> for stage2 and stage3 host, and for stage3 libs (also -fcompare-debug)
>
> I got build failures on zlib and libada, presumably because of this
> unusual combination of flags, but no -fcompare-debug errors.
>
> Ok to install?

Ok (is there a testcase that can be added to the testsuite?)

Thanks,
Richard.

>
>
> --
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist      Red Hat Brazil Compiler Engineer
>
>
Alexandre Oliva - June 14, 2010, 7:36 p.m.
On Jun 11, 2010, Richard Guenther <richard.guenther@gmail.com> wrote:

> On Fri, Jun 11, 2010 at 9:18 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
>> A number of graphite -fcompare-debug bug reports were fixed with this
>> patch.

> Ok (is there a testcase that can be added to the testsuite?)

Nothing really useful, I'm afraid.  I actually had to go back to earlier
versions of the compiler to as much as duplicate the reported problems.
They're quite sensitive to other changes in the compiler.
H.J. Lu - Dec. 10, 2010, 4:57 p.m.
On Fri, Jun 11, 2010 at 12:18 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
> A number of graphite -fcompare-debug bug reports were fixed with this
> patch.  The problem was that we inserted induction variable increment
> stmts before the last stmt in the block, but if the block ended with a
> fall-through, the insert would be before a non-debug stmt in the non-VTA
> case, whereas in the VTA case it could be before a debug stmt, after the
> non-debug stmt.  Skipping trailing debug stmts so that we insert before
> the last non-debug stmt removed the codegen differences.
>
> Bootstrapped the compiler, along with the upcoming patch for PR43656, on
> x86_64-linux-gnu, with the following options:
>
> -O2 -g -fschedule-insns -fsched-pressure -funroll-loops -fgraphite-identity
>
> for stage2 and stage3 host, and for stage3 libs (also -fcompare-debug)
>
> I got build failures on zlib and libada, presumably because of this
> unusual combination of flags, but no -fcompare-debug errors.
>

This caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46885

Patch

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR debug/43650
	PR debug/44181
	PR debug/44247
	* tree-ssa-loop-manip.c (tree_transform_and_unroll_loop): Skip
	debug stmts.
	(canonicalize_loop_ivs): Likewise.

Index: gcc/tree-ssa-loop-manip.c
===================================================================
--- gcc/tree-ssa-loop-manip.c.orig	2010-06-11 09:16:28.000000000 -0300
+++ gcc/tree-ssa-loop-manip.c	2010-06-11 09:17:38.000000000 -0300
@@ -1081,7 +1081,7 @@  tree_transform_and_unroll_loop (struct l
 
   /* Finally create the new counter for number of iterations and add the new
      exit instruction.  */
-  bsi = gsi_last_bb (exit_bb);
+  bsi = gsi_last_nondebug_bb (exit_bb);
   exit_if = gsi_stmt (bsi);
   create_iv (exit_base, exit_step, NULL_TREE, loop,
 	     &bsi, false, &ctr_before, &ctr_after);
@@ -1217,7 +1217,7 @@  canonicalize_loop_ivs (struct loop *loop
 	gsi_insert_seq_on_edge_immediate (loop_preheader_edge (loop), stmts);
     }
 
-  gsi = gsi_last_bb (bump_in_latch ? loop->latch : loop->header);
+  gsi = gsi_last_nondebug_bb (bump_in_latch ? loop->latch : loop->header);
   create_iv (build_int_cst_type (type, 0), build_int_cst (type, 1), NULL_TREE,
 	     loop, &gsi, bump_in_latch, &var_before, NULL);