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

Submitted by Jakub Jelinek on Nov. 9, 2010, 7:11 p.m.

Details

Message ID 20101109191112.GV29412@tyan-ft48-01.lab.bos.redhat.com
State New
Headers show

Commit Message

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

Comments

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 hide | download patch | download mbox

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