Patchwork [committed] Fix stack slot coalescing at -O0 (PR target/43808)

login
register
mail settings
Submitter Jakub Jelinek
Date Nov. 9, 2010, 7:11 p.m.
Message ID <20101109191112.GV29412@tyan-ft48-01.lab.bos.redhat.com>
Download mbox | patch
Permalink /patch/70570/
State New
Headers show

Comments

Jakub Jelinek - Nov. 9, 2010, 7:11 p.m.
Hi!

If -O0 is used together with passes that use alias into in the RTL
(e.g. scheduling), we may miscompile code, as
update_alias_info_with_stack_vars wasn't being called and thus
a !may_be_alias var which shared a stack slot with a may_be_alias
var could result in wrong scheduling.  Fixed thusly, bootstrapped/regtested
on x86_64-linux and i686-linux, acked by Richard on IRC, committed to trunk.

2010-11-09  Jakub Jelinek  <jakub@redhat.com>

	PR target/43808
	* cfgexpand.c (partition_stack_vars): Call
	update_alias_info_with_stack_vars unconditionally.
	(update_alias_info_with_stack_vars): Allow unused
	unreferenced vars when not optimizing.

	* gfortran.dg/pr43808.f90: New test.


	Jakub
Paolo Bonzini - Nov. 9, 2010, 11:50 p.m.
On 11/09/2010 08:11 PM, Jakub Jelinek wrote:
> Hi!
>
> If -O0 is used together with passes that use alias into in the RTL
> (e.g. scheduling), we may miscompile code

Why not disable most optimization passes at -O0 independent of flags 
(maybe even give a hard error) instead?  It would be a bigger patch, but 
this set of flags makes no sense.

Paolo
Richard Guenther - Nov. 10, 2010, 11:15 a.m.
On Wed, Nov 10, 2010 at 12:50 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
> On 11/09/2010 08:11 PM, Jakub Jelinek wrote:
>>
>> Hi!
>>
>> If -O0 is used together with passes that use alias into in the RTL
>> (e.g. scheduling), we may miscompile code
>
> Why not disable most optimization passes at -O0 independent of flags (maybe
> even give a hard error) instead?  It would be a bigger patch, but this set
> of flags makes no sense.

If we want unit-testing of some sort then we should make sure passes
work regardless of what earlier optimization passes did.  So instead of
adding more optimize checks I'd rather remove more of them.

Richard.

> Paolo
>

Patch

--- gcc/cfgexpand.c.jj	2010-11-03 16:58:31.000000000 +0100
+++ gcc/cfgexpand.c	2010-11-08 17:34:22.000000000 +0100
@@ -516,9 +516,11 @@  update_alias_info_with_stack_vars (void)
 	  unsigned int uid = DECL_PT_UID (decl);
 	  /* We should never end up partitioning SSA names (though they
 	     may end up on the stack).  Neither should we allocate stack
-	     space to something that is unused and thus unreferenced.  */
+	     space to something that is unused and thus unreferenced, except
+	     for -O0 where we are preserving even unreferenced variables.  */
 	  gcc_assert (DECL_P (decl)
-		      && referenced_var_lookup (DECL_UID (decl)));
+		      && (!optimize
+			  || referenced_var_lookup (DECL_UID (decl))));
 	  bitmap_set_bit (part, uid);
 	  *((bitmap *) pointer_map_insert (decls_to_partitions,
 					   (void *)(size_t) uid)) = part;
@@ -684,8 +686,7 @@  partition_stack_vars (void)
 	}
     }
 
-  if (optimize)
-    update_alias_info_with_stack_vars ();
+  update_alias_info_with_stack_vars ();
 }
 
 /* A debugging aid for expand_used_vars.  Dump the generated partitions.  */
--- gcc/testsuite/gfortran.dg/pr43808.f90.jj	2010-11-08 17:32:16.000000000 +0100
+++ gcc/testsuite/gfortran.dg/pr43808.f90	2010-11-08 17:30:41.000000000 +0100
@@ -0,0 +1,18 @@ 
+! PR target/43808
+! { dg-do run }
+! { dg-options "-O0 -fipa-reference -fschedule-insns -fstrict-aliasing" }
+
+  type :: a
+    integer, allocatable :: i(:)
+  end type a
+  type :: b
+    type (a), allocatable :: j(:)
+  end type b
+  type(a) :: x(2)
+  type(b) :: y(2)
+  x(1) = a((/1,2,3,4/))
+  x(2) = a((/1,2,3,4/)+10)
+  y(1) = b((/x(1),x(2)/))
+  y(2) = b((/x(1),x(2)/))
+  if (y(1)%j(1)%i(1) .ne. 1) call abort
+end