Patchwork [PR,debug/46258] tree cfgcleanup BB marking vs debug stmts

login
register
mail settings
Submitter Alexandre Oliva
Date Nov. 24, 2010, 1:24 a.m.
Message ID <orsjyrqzry.fsf@livre.localdomain>
Download mbox | patch
Permalink /patch/72784/
State New
Headers show

Comments

Alexandre Oliva - Nov. 24, 2010, 1:24 a.m.
When cleaning up the tree^Wtuples CFG, we will sometimes replace uses of
SSA names with other values.

Problem is, if the only tuples modified in a dominated block are debug
stmts, there may be divergence between -g and -g0 compilations, as there
was with the given testcase in the PR (from the GCC testsuite, so not
included in the patch): a block visited containing a modified debug stmt
was successfully simplified in the cfgcleanup loop that visits only
blocks marked before.

I suppose this may be exposing a weakness in our block marking strategy,
for the block could have been cleaned up in both cases, and after this
change, it isn't before a subsequence full cfgcleanup run.  However, in
this patch, I'm only addressing the -g/-g0 divergence.

Regstrapped on x86_64-linux-gnu and i686-pc-linux-gnu.  Ok to install?
Richard Guenther - Nov. 24, 2010, 11:50 a.m.
On Wed, Nov 24, 2010 at 2:24 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
> When cleaning up the tree^Wtuples CFG, we will sometimes replace uses of
> SSA names with other values.
>
> Problem is, if the only tuples modified in a dominated block are debug
> stmts, there may be divergence between -g and -g0 compilations, as there
> was with the given testcase in the PR (from the GCC testsuite, so not
> included in the patch): a block visited containing a modified debug stmt
> was successfully simplified in the cfgcleanup loop that visits only
> blocks marked before.
>
> I suppose this may be exposing a weakness in our block marking strategy,
> for the block could have been cleaned up in both cases, and after this
> change, it isn't before a subsequence full cfgcleanup run.  However, in
> this patch, I'm only addressing the -g/-g0 divergence.
>
> Regstrapped on x86_64-linux-gnu and i686-pc-linux-gnu.  Ok to install?

Ok.

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
>
>

Patch

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

	PR debug/46258
	* tree-cfg.c (replace_uses_by): Don't mark BBs as altered on
	debug stmts.

Index: gcc/tree-cfg.c
===================================================================
--- gcc/tree-cfg.c.orig	2010-11-18 10:37:23.716685868 -0200
+++ gcc/tree-cfg.c	2010-11-18 10:37:31.701412567 -0200
@@ -1570,7 +1570,7 @@  replace_uses_by (tree name, tree val)
 	  size_t i;
 
 	  fold_stmt_inplace (stmt);
-	  if (cfgcleanup_altered_bbs)
+	  if (cfgcleanup_altered_bbs && !is_gimple_debug (stmt))
 	    bitmap_set_bit (cfgcleanup_altered_bbs, gimple_bb (stmt)->index);
 
 	  /* FIXME.  This should go in update_stmt.  */