Re: Patch for 3.4.4. Infinite memory allocation on -O3 (PR #18081).

[Ulrich Weigand <uweigand at de dot ibm dot com>]

Mark Mitchell wrote:

> Ulrich Weigand wrote:
> > The testcases crash at run time due to apparent wrong code generation.
> > The generated assembler with and without the patch differs significantly;
> > I haven't yet managed to pin down the particular bug.  I'll continue
> > analysing this tomorrow ...
> 
> Did you update to get the follow-on check-in as well?
Yes, this doesn't make any difference.

> Thank you for keeping me informed!
I've still not fully analysed the specific failure case on s390x, sorry.

However, I've found a definite bug in 3.4 caused by this particular 
backport, which is also very likely the cause of the performance
regression reported here:
http://gcc.gnu.org/ml/gcc/2005-05/msg00523.html

The problem is that after your backported patch, set_mem_attributes
now assumes the MEM_VOLATILE_P flag of the incoming MEM ref to be
valid.  However, there are (at least) two locations in function.c
where a DECL is moved from register onto the stack by doing
  PUT_CODE (..., MEM)
on a pre-existing REG rtx.  Now, the very same flag bit that stands 
for MEM_VOLATILE_P in MEMs stands for REG_USERVAR_P in REGs.  The
latter bit will frequently be *set* in such DECL REG rtxs.

This didn't matter until now as the code calls set_mem_attribute on
the MEM generated by PUT_CODE, which used to recompute the value of
the MEM_VOLATILE_P flag.  However, after your patch, such MEMs will
remain marked as volatile ...

(Note that these bits of code appear to be gone in 4.0, which is
presumably why the original patch didn't trigger this problem there.)

The following patch fixes those two locations by manually resetting
the MEM_VOLATILE_P flag after performing PUT_CODE, just like reload
does (where it use PUT_CODE for the same effect).  With this patch,
the testsuite crashes on s390x are gone, and -from looking at the
generated code- the performance regressions appears to be gone as
well.  (I haven't yet completed bootstrap and regression testing
with this patch, however.)

I'd suggest to either revert the MEM_VOLATILE backport(s) for 3.4.4
or else fix the regressions by something like the below patch.

Bye,
Ulrich


Index: gcc/function.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/function.c,v
retrieving revision 1.483.4.24
diff -c -p -r1.483.4.24 function.c
*** gcc/function.c	10 May 2005 21:53:05 -0000	1.483.4.24
--- gcc/function.c	11 May 2005 16:30:34 -0000
*************** put_var_into_stack (tree decl, int resca
*** 1397,1402 ****
--- 1397,1403 ----
  
        /* Change the CONCAT into a combined MEM for both parts.  */
        PUT_CODE (reg, MEM);
+       MEM_VOLATILE_P (reg) = 0;
        MEM_ATTRS (reg) = 0;
  
        /* set_mem_attributes uses DECL_RTL to avoid re-generating of
*************** gen_mem_addressof (rtx reg, tree decl, i
*** 2859,2864 ****
--- 2860,2866 ----
    RTX_UNCHANGING_P (XEXP (r, 0)) = RTX_UNCHANGING_P (reg);
  
    PUT_CODE (reg, MEM);
+   MEM_VOLATILE_P (reg) = 0;
    MEM_ATTRS (reg) = 0;
    XEXP (reg, 0) = r;
  

-- 
  Dr. Ulrich Weigand
  Linux on zSeries Development
  Ulrich.Weigand@de.ibm.com