Bug 43808 - [4.6 Regression] -fipa-reference -fschedule-insns -fstrict-aliasing causes two gfortran check failures
Summary: [4.6 Regression] -fipa-reference -fschedule-insns -fstrict-aliasing causes tw...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.6.0
: P1 normal
Target Milestone: 4.6.0
Assignee: Jakub Jelinek
URL:
Keywords: alias, wrong-code
Depends on:
Blocks:
 
Reported: 2010-04-19 22:18 UTC by Zdenek Sojka
Modified: 2010-11-09 19:32 UTC (History)
2 users (show)

See Also:
Host: x86_64-pc-linux-gnu
Target: x86_64-pc-linux-gnu
Build:
Known to work:
Known to fail:
Last reconfirmed: 2010-09-02 10:41:30


Attachments
gcc46-pr43808-1.patch (759 bytes, patch)
2010-11-08 16:38 UTC, Jakub Jelinek
Details | Diff
gcc46-pr43808-2.patch (555 bytes, patch)
2010-11-08 16:39 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zdenek Sojka 2010-04-19 22:18:26 UTC
Tested revisions:
r158486 - fail
r158150 - fail
4.5 r158486 - OK

$ binary-158486-lto-fortran/bin/gfortran -fipa-reference -fschedule-insns -fstrict-aliasing gcc/testsuite/gfortran.dg/alloc_comp_assign_3.f90 && ./a.out
Segmentation fault
$ binary-158486-lto-fortran/bin/gfortran -fipa-reference -fschedule-insns -fstrict-aliasing gcc/testsuite/gfortran.dg/alloc_comp_assign_4.f90 && ./a.out
Segmentation fault
Comment 1 Richard Biener 2010-07-23 11:47:07 UTC
Confirmed.
Comment 2 Richard Biener 2010-09-02 10:41:30 UTC
This looks like a genuine RTL alias problem, ipa-reference just causes x, y
and z to be marked as not aliased.
Comment 3 Jakub Jelinek 2010-11-08 11:11:56 UTC
Smaller testcase:

b.f90:

module pr43808
  type :: a
    integer, allocatable :: i(:)
  end type a
  type :: b
    type (a), allocatable :: at(:)
  end type b
contains
  subroutine foo(y)
    type(b) :: y(2)
    forall (j=1:2,k=1:4, y(1)%at(j)%i(k) .ne. y(2)%at(j)%i(k)) &
      y(1)%at(j)%i(k) = 999
  end subroutine foo
end module pr43808

c.f90:

  use pr43808
  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)/))
  call foo(y)
end

./f951 -quiet -g b.f90; ./f951 -quiet -g -fipa-reference -fschedule-insns -fno-schedule-insns2 -fstrict-aliasing -fverbose-asm c.f90; gfortran -o b{,.s} c.s; ./b; echo $?
Segmentation fault (core dumped)
139
Comment 4 Richard Biener 2010-11-08 11:27:33 UTC
As usual this might be as well a type-merging problem in the Frontend.
Comment 5 Jakub Jelinek 2010-11-08 14:39:51 UTC
The problem seems to be that on the c.f90:

...
  D.1548.i.data = 0B;
  atmp.7.dtype = 3113;
  atmp.7.dim[0].stride = 1;
  atmp.7.dim[0].lbound = 0;
  atmp.7.dim[0].ubound = 1;
  D.1691_62 = (void * restrict) &A.8[0];
  atmp.7.data = D.1691_62;
  atmp.7.offset = 0;
  D.1692_63 = atmp.7.data;
  MEM[(struct a[2] *)D.1692_63][0] = x[0];
  D.1693_64 = atmp.7.data;
  MEM[(struct a[2] *)D.1693_64][1] = x[1];

Partition 10: size 96 align 16
        A.8, offset 0
        A.20, offset 0
        a.0, offset 0
        D.1548, offset 0

(thus D.1548 and A.8 share stack slot) and we have in *.asmcons:

// This is D.1548.i.data = 0B;
(insn 250 249 251 18 (set (mem/s/f/c:DI (plus:DI (reg/f:DI 20 frame)
                (const_int -384 [0xfffffffffffffe80])) [3 D.1548.i.data+0 S8 A64])
        (const_int 0 [0])) c.f90:5 62 {*movdi_internal_rex64}
     (nil))
...
// This is D.1691_62 = (void * restrict) &A.8[0];
(insn 255 254 256 18 (parallel [
            (set (reg/f:DI 116 [ D.1691 ])
                (plus:DI (reg/f:DI 20 frame)
                    (const_int -384 [0xfffffffffffffe80])))
            (clobber (reg:CC 17 flags))
        ]) c.f90:6 253 {*adddi_1}
     (nil))

// This is atmp.7.data = D.1691_62;
(insn 256 255 257 18 (set (mem/s/f/c:DI (plus:DI (reg/f:DI 20 frame)
                (const_int -192 [0xffffffffffffff40])) [3 atmp.7.data+0 S8 A64])
        (reg/f:DI 116 [ D.1691 ])) c.f90:6 62 {*movdi_internal_rex64}
     (nil))
...
// This is D.1692_63 = atmp.7.data;
(insn 258 257 260 18 (set (reg/f:DI 117 [ D.1692 ])
        (mem/s/f/c:DI (plus:DI (reg/f:DI 20 frame)
                (const_int -192 [0xffffffffffffff40])) [3 atmp.7.data+0 S8 A64])) c.f90:6 62 {*movdi_internal_rex64}
     (nil))

// These two are start of MEM[(struct a[2] *)D.1692_63][0] = x[0];
(insn 260 258 261 18 (set (reg:DI 332)
        (mem/s:DI (symbol_ref:DI ("x.1518") [flags 0x2]  <var_decl 0x7ff967060140 x>) [6 x+0 S8 A256])) c.f90:6 62 {*movdi_internal_rex64}
     (nil))

(insn 261 260 262 18 (set (mem/s:DI (reg/f:DI 117 [ D.1692 ]) [6 MEM[(struct a[2] *)D.1692_63]+0 S8 A64])
        (reg:DI 332)) c.f90:6 62 {*movdi_internal_rex64}
     (nil))

Now sched1 unfortunately schedules insn 250 after insn 261 (and whole bunch of other insns) and as D.1548 and A.8 share stack space, this means overwriting first 8 bytes in A.8 with NULL.

This all is with current trunk, ./f951 -quiet -fdump-tree-optimized -g -fipa-reference -fschedule-insns -fno-schedule-insns2 -fstrict-aliasing -fverbose-asm c.f90 on x86_64-linux.

D.1548 is:
 <var_decl 0x7ffff1ac9140 D.1548
    type <record_type 0x7ffff1cd69d8 a asm_written BLK
        size <integer_cst 0x7ffff1cccb40 constant 384>
        unit size <integer_cst 0x7ffff1cc7dc0 constant 48>
        align 64 symtab -238246416 alias set 6 canonical type 0x7ffff1cd69d8
        fields <field_decl 0x7ffff1cd85f0 i type <record_type 0x7ffff1cd6dc8 array1_integer(kind=4)>
            BLK file c.f90 line 1 col 0 size <integer_cst 0x7ffff1cccb40 384> unit size <integer_cst 0x7ffff1cc7dc0 48>
            align 64 offset_align 128
            offset <integer_cst 0x7ffff1bdb438 constant 0>
            bit offset <integer_cst 0x7ffff1bdbb40 constant 0> context <record_type 0x7ffff1cd69d8 a>>
        pointer_to_this <pointer_type 0x7ffff1abf690> chain <type_decl 0x7ffff1abd170 D.1516>>
    used ignored BLK file c.f90 line 5 col 0 size <integer_cst 0x7ffff1cccb40 384> unit size <integer_cst 0x7ffff1cc7dc0 48>
    align 64 context <function_decl 0x7ffff1cd7100 MAIN__>
    (mem/s/c:BLK (plus:DI (reg/f:DI 54 virtual-stack-vars)
        (const_int -384 [0xfffffffffffffe80])) [6 D.1548+0 S48 A64]) chain <var_decl 0x7ffff1ac1aa0 a.3>>
.

In the MEM[(struct a[2] *)D.1692_63][0] = x[0]; stmt LHS is:
 <array_ref 0x7ffff1c14630
    type <record_type 0x7ffff1cd69d8 a asm_written BLK
        size <integer_cst 0x7ffff1cccb40 constant 384>
        unit size <integer_cst 0x7ffff1cc7dc0 constant 48>
        align 64 symtab -238246416 alias set 6 canonical type 0x7ffff1cd69d8
        fields <field_decl 0x7ffff1cd85f0 i type <record_type 0x7ffff1cd6dc8 array1_integer(kind=4)>
            BLK file c.f90 line 1 col 0 size <integer_cst 0x7ffff1cccb40 384> unit size <integer_cst 0x7ffff1cc7dc0 48>
            align 64 offset_align 128
            offset <integer_cst 0x7ffff1bdb438 constant 0>
            bit offset <integer_cst 0x7ffff1bdbb40 constant 0> context <record_type 0x7ffff1cd69d8 a>>
        pointer_to_this <pointer_type 0x7ffff1abf690> chain <type_decl 0x7ffff1abd170 D.1516>>
   
    arg 0 <mem_ref 0x7ffff1adca80
        type <array_type 0x7ffff1ac63f0 type <record_type 0x7ffff1cd69d8 a>
            BLK
            size <integer_cst 0x7ffff1cccbb8 constant 768>
            unit size <integer_cst 0x7ffff1cc7e38 constant 96>
            align 64 symtab 0 alias set 6 canonical type 0x7ffff1ac63f0 domain <integer_type 0x7ffff1abf888>
            pointer_to_this <pointer_type 0x7ffff1ac6498>>
       
        arg 0 <ssa_name 0x7ffff1b135d8 type <pointer_type 0x7ffff1bfce70>
            visited var <var_decl 0x7ffff1aea6e0 D.1692>def_stmt D.1692_63 = atmp.7.data;

            version 63>
        arg 1 <integer_cst 0x7ffff1ac82d0 constant 0>
        c.f90:6:0>
    arg 1 <integer_cst 0x7ffff1bfe528 type <integer_type 0x7ffff1beb5e8 integer(kind=8)> constant 0>
    c.f90:6:0>
and the type of MEM_REF's arg1 is:
 <pointer_type 0x7ffff1ac6540
    type <array_type 0x7ffff1ac63f0
        type <record_type 0x7ffff1cd69d8 a asm_written BLK
            size <integer_cst 0x7ffff1cccb40 constant 384>
            unit size <integer_cst 0x7ffff1cc7dc0 constant 48>
            align 64 symtab -238246416 alias set 6 canonical type 0x7ffff1cd69d8 fields <field_decl 0x7ffff1cd85f0 i>
            pointer_to_this <pointer_type 0x7ffff1abf690> chain <type_decl 0x7ffff1abd170 D.1516>>
        BLK
        size <integer_cst 0x7ffff1cccbb8 constant 768>
        unit size <integer_cst 0x7ffff1cc7e38 constant 96>
        align 64 symtab 0 alias set 6 canonical type 0x7ffff1ac63f0
        domain <integer_type 0x7ffff1abf888 type <integer_type 0x7ffff1beb5e8 integer(kind=8)>
            public DI
            size <integer_cst 0x7ffff1bdb7d0 constant 64>
            unit size <integer_cst 0x7ffff1bdb7f8 constant 8>
            align 64 symtab 0 alias set -1 canonical type 0x7ffff1abf888 precision 64 min <integer_cst 0x7ffff1bfe528 0> max <integer_cst 0x7ffff1cd5640 1>>
        pointer_to_this <pointer_type 0x7ffff1ac6498>>
    public unsigned restrict DI size <integer_cst 0x7ffff1bdb7d0 64> unit size <integer_cst 0x7ffff1bdb7f8 8>
    align 64 symtab 0 alias set -1 canonical type 0x7ffff1ac6540>

So alias set 6 for the aggregate copy is fine and so is 3 for the void * type (which is part of array_descriptor1 aggregate, which is part of struct a).
So it must be the alias oracle saying these two can't alias, perhaps related somehow to the restrict or something.  Richard, can you have a look at this?
Comment 6 Jakub Jelinek 2010-11-08 15:21:41 UTC
Indeed, for
x:
(mem/s:DI (reg/f:DI 117 [ D.1692 ]) [6 MEM[(struct a[2] *)D.1692_63]+0 S8 A64])
mem:
(mem/s/f/c:DI (plus:DI (reg/f:DI 20 frame)
        (const_int -384 [0xfffffffffffffe80])) [3 D.1548.i.data+0 S8 A64])
rtx_refs_may_alias_p (x, mem, 0)
returns 0.
indirect_ref_may_alias_decl_p is called with base1 the MEM_REF and base2 VAR_DECL D.1548.

And the problem is that may_be_aliased is false for D.1548.  Sure, it doesn't have address taken.  But as expansion decided to share its slot together with
other variables and those are may_be_aliased, this of course breaks.

I wonder whether expansion shouldn't somehow make sure that if at least one var in the partition is may_be_aliased, all of them are.  Be it by not merging vars that have different may_be_aliased, or by say marking the non-addressable vars TREE_ADDRESSABLE if anything in the partition is addressable.

Richard?
Comment 7 Richard Biener 2010-11-08 15:32:03 UTC
(In reply to comment #6)
> Indeed, for
> x:
> (mem/s:DI (reg/f:DI 117 [ D.1692 ]) [6 MEM[(struct a[2] *)D.1692_63]+0 S8 A64])
> mem:
> (mem/s/f/c:DI (plus:DI (reg/f:DI 20 frame)
>         (const_int -384 [0xfffffffffffffe80])) [3 D.1548.i.data+0 S8 A64])
> rtx_refs_may_alias_p (x, mem, 0)
> returns 0.
> indirect_ref_may_alias_decl_p is called with base1 the MEM_REF and base2
> VAR_DECL D.1548.
> 
> And the problem is that may_be_aliased is false for D.1548.  Sure, it doesn't
> have address taken.  But as expansion decided to share its slot together with
> other variables and those are may_be_aliased, this of course breaks.
> 
> I wonder whether expansion shouldn't somehow make sure that if at least one var
> in the partition is may_be_aliased, all of them are.  Be it by not merging vars
> that have different may_be_aliased, or by say marking the non-addressable vars
> TREE_ADDRESSABLE if anything in the partition is addressable.
> 
> Richard?

We do this already, see cfgexpand.c:add_partitioned_vars_to_ptset and
update_alias_info_with_stack_vars and alias.c:ao_ref_from_mem:

  /* If this is a reference based on a partitioned decl replace the
     base with an INDIRECT_REF of the pointer representative we
     created during stack slot partitioning.  */
  if (TREE_CODE (base) == VAR_DECL
      && ! TREE_STATIC (base)
      && cfun->gimple_df->decls_to_pointers != NULL)
    {
      void *namep;
      namep = pointer_map_contains (cfun->gimple_df->decls_to_pointers, base);
      if (namep)
        ref->base = build_simple_mem_ref (*(tree *)namep);
    }

and we try to handle this particular case in update_alias_info_with_stack_vars:

  /* Make all points-to sets that contain one member of a partition
     contain all members of the partition.  */
  if (decls_to_partitions)
    {
      unsigned i;
      struct pointer_set_t *visited = pointer_set_create ();
      bitmap temp = BITMAP_ALLOC (NULL);

      for (i = 1; i < num_ssa_names; i++)
        {
...


So you say that for some reason this doesn't work.
Comment 8 Richard Biener 2010-11-08 15:35:38 UTC
I will still have a look.
Comment 9 Jakub Jelinek 2010-11-08 15:38:21 UTC
Ah,
  if (optimize)
    update_alias_info_with_stack_vars ();
explains that.  Guess it should be if (optimize || ...).
if (optimize || flag_strict_aliasing) would work for this testcase, but not sure
if things still wouldn't break for -O0 -fschedule-insns or similar.
Comment 10 Jakub Jelinek 2010-11-08 16:38:55 UTC
Created attachment 22328 [details]
gcc46-pr43808-1.patch

One possible fix (so far untested).
Comment 11 Jakub Jelinek 2010-11-08 16:39:39 UTC
Created attachment 22329 [details]
gcc46-pr43808-2.patch

Another possible fix (also untested).
Comment 12 Jakub Jelinek 2010-11-09 12:47:56 UTC
The first patch bootstrapped/regtested fine on both x86_64-linux and i686-linux, the second one caused a couple of regressions on x86_64-linux (and worked fine on i686-linux):
FAIL: gcc.target/x86_64/abi/test_3_element_struct_and_unions.c compilation,  -O0  (internal compiler error)
UNRESOLVED: gcc.target/x86_64/abi/test_3_element_struct_and_unions.c execution,  -O0 
FAIL: gcc.target/x86_64/abi/test_basic_array_size_and_align.c compilation,  -O0  (internal compiler error)
UNRESOLVED: gcc.target/x86_64/abi/test_basic_array_size_and_align.c execution,  -O0 
FAIL: gnat.dg/gen_disp.adb (test for excess errors)
FAIL: gnat.dg/specs/private_with.ads (test for excess errors)

All of these are ICEs in update_alias_info_with_stack_vars, short testcase on x86_64-linux is:
int
main (void)
{
  {
    struct S1 { long double t1; long double t2; float t3; } s1;
  }
  {
    struct S2 { long double t1; long double t2; double t3; } s2;
  }
}

The ICE is in:
          /* 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.  */
          gcc_assert (DECL_P (decl)
                      && referenced_var_lookup (DECL_UID (decl)));
decl is s1 (VAR_DECL), but referenced_var_lookup returns NULL on it.
Wonder if we just shouldn't add && optimize into the assert, or whether it is really a problem if a non-referenced var makes it in.
Comment 13 Jakub Jelinek 2010-11-09 19:31:48 UTC
Author: jakub
Date: Tue Nov  9 19:31:45 2010
New Revision: 166509

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=166509
Log:
	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.

Added:
    trunk/gcc/testsuite/gfortran.dg/pr43808.f90
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/cfgexpand.c
    trunk/gcc/testsuite/ChangeLog
Comment 14 Jakub Jelinek 2010-11-09 19:32:30 UTC
Fixed.