[PATCH] Fix PR67443

[PATCH] Fix PR67443

[Richard Biener]

The following patch fixes PR67443, ao_ref_from_mem's special handling
of some negative MEM_OFFSET cases being wrong in general.  The patch
removes the special-casing and places an additional safety net
for MEM_EXPR handlings that rely on component bounds being honored.

The patch might pessimize the mentioned "promoted subreg parameter"
handling as for negative MEM_OFFSET we now run into

  /* If MEM_OFFSET and MEM_SIZE get us outside of the base object of
     the MEM_EXPR punt.  This happens for STRICT_ALIGNMENT targets a lot.  
*/
  if (MEM_EXPR (mem) != get_spill_slot_decl (false)
      && (ref->offset < 0
          || (DECL_P (ref->base)
              && (DECL_SIZE (ref->base) == NULL_TREE
                  || TREE_CODE (DECL_SIZE (ref->base)) != INTEGER_CST
                  || wi::ltu_p (wi::to_offset (DECL_SIZE (ref->base)),
                                ref->offset + ref->size)))))
    return false;

but I can't see how the previous handling was correct with regard
to overlaps in the parameter area.

Bootstrapped and tested on x86_64-unknown-linux-gnu and 
powerpc64-linux-gnu by me and on s390[x]-linux by Dominik.

Eventually we'll get another testcase so I'll leave this for
comments a while and will commit somewhen later this week.

Thanks,
Richard.

2015-10-21  Richard Biener  <rguenther@suse.de>
	Dominik Vogt  <vogt@linux.vnet.ibm.com>

	PR middle-end/67443
	* alias.c (ao_ref_from_mem): Remove promoted subreg handling.
	Properly prune ref->ref for accesses outside of ref.

Index: gcc/alias.c
===================================================================
*** gcc/alias.c	(revision 229119)
--- gcc/alias.c	(working copy)
*************** ao_ref_from_mem (ao_ref *ref, const_rtx
*** 339,353 ****
        || !MEM_SIZE_KNOWN_P (mem))
      return true;
  
!   /* If the base decl is a parameter we can have negative MEM_OFFSET in
!      case of promoted subregs on bigendian targets.  Trust the MEM_EXPR
!      here.  */
    if (MEM_OFFSET (mem) < 0
!       && (MEM_SIZE (mem) + MEM_OFFSET (mem)) * BITS_PER_UNIT == ref->size)
!     return true;
  
!   /* Otherwise continue and refine size and offset we got from analyzing
!      MEM_EXPR by using MEM_SIZE and MEM_OFFSET.  */
  
    ref->offset += MEM_OFFSET (mem) * BITS_PER_UNIT;
    ref->size = MEM_SIZE (mem) * BITS_PER_UNIT;
--- 339,354 ----
        || !MEM_SIZE_KNOWN_P (mem))
      return true;
  
!   /* If MEM_OFFSET/MEM_SIZE get us outside of ref->offset/ref->max_size
!      drop ref->ref.  */
    if (MEM_OFFSET (mem) < 0
!       || (ref->max_size != -1
! 	  && ((MEM_OFFSET (mem) + MEM_SIZE (mem)) * BITS_PER_UNIT
! 	      > ref->max_size)))
!     ref->ref = NULL_TREE;
  
!   /* Refine size and offset we got from analyzing MEM_EXPR by using
!      MEM_SIZE and MEM_OFFSET.  */
  
    ref->offset += MEM_OFFSET (mem) * BITS_PER_UNIT;
    ref->size = MEM_SIZE (mem) * BITS_PER_UNIT;
Index: gcc/testsuite/gcc.target/s390/pr67443.c
===================================================================
*** gcc/testsuite/gcc.target/s390/pr67443.c	(revision 0)
--- gcc/testsuite/gcc.target/s390/pr67443.c	(working copy)
***************
*** 0 ****
--- 1,49 ----
+ /* Test case for PR/67443.  */
+ 
+ /* { dg-do run { target s390*-*-* } } */
+ /* { dg-prune-output "call-clobbered register used for global register variable" } */
+ /* { dg-options "-march=z900 -fPIC -fomit-frame-pointer -O3" } */
+ 
+ #include <assert.h>
+ 
+ /* Block all registers except the first three argument registers.  */
+ register long r0 asm ("r0");
+ register long r1 asm ("r1");
+ register long r5 asm ("r5");
+ register long r6 asm ("r6");
+ register long r7 asm ("r7");
+ register long r8 asm ("r8");
+ register long r9 asm ("r9");
+ register long r10 asm ("r10");
+ register long r11 asm ("r11");
+ 
+ struct s_t
+ {
+   unsigned f1 : 8;
+   unsigned f2 : 24;
+ };
+ 
+ __attribute__ ((noinline))
+ void foo (struct s_t *ps, int c, int i)
+ {
+   /* Uses r2 as address register.  */
+   ps->f1 = c;
+   /* The calculation of the value is so expensive that it's cheaper to spill ps
+      to the stack and reload it later (into a different register).
+      ==> Uses r4 as address register.*/
+   ps->f2 = i + i % 3;
+   /* If dead store elimination fails to detect that the address in r2 during
+      the first assignment is an alias of the address in r4 during the second
+      assignment, it eliminates the first assignment and the f1 field is not
+      written (bug).  */
+ }
+ 
+ int main (void)
+ {
+   struct s_t s = { 0x01u, 0x020304u };
+ 
+   foo (&s, 0, 0);
+   assert (s.f1 == 0&& s.f2 == 0);
+ 
+   return 0;
+ }

Re: [PATCH] Fix PR67443

[Dominik Vogt]

> Eventually we'll get another testcase so I'll leave this for
> comments a while and will commit somewhen later this week.

In case you're referring to my attempt to port the test case to
x86:  All the efforts to reproduce the bug on x86 have failed so
far.  It seems that Gcc is much better in handling bit filed
accesses on little endian targets (x86) than on big endian
(s390[x], power).  In other words, on x86 earlier optimisations
prevent the dse2 pass from having to deal with the problematic
situation.  So, don't wait for me coming up with more test cases;
I'll keep trying, but I'm pessimistic.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

Re: [PATCH] Fix PR67443

[Richard Biener]

On Thu, Oct 22, 2015 at 11:58 AM, Dominik Vogt <vogt@linux.vnet.ibm.com> wrote:
>> Eventually we'll get another testcase so I'll leave this for
>> comments a while and will commit somewhen later this week.
>
> In case you're referring to my attempt to port the test case to
> x86:  All the efforts to reproduce the bug on x86 have failed so
> far.  It seems that Gcc is much better in handling bit filed
> accesses on little endian targets (x86) than on big endian
> (s390[x], power).  In other words, on x86 earlier optimisations
> prevent the dse2 pass from having to deal with the problematic
> situation.  So, don't wait for me coming up with more test cases;
> I'll keep trying, but I'm pessimistic.

Now applied to trunk.

Richard.

> Ciao
>
> Dominik ^_^  ^_^
>
> --
>
> Dominik Vogt
> IBM Germany
>