Re: middle-end/9997: Coelesce stack slots for disjoint scopes.

[Daniel Jacobowitz <drow@mvista.com>]

The following reply was made to PR middle-end/9997; it has been noted by GNATS.

From: Daniel Jacobowitz <drow@mvista.com>
To: Jim Wilson <wilson@tuliptree.org>
Cc: gcc-gnats@gcc.gnu.org
Subject: Re: middle-end/9997: Coelesce stack slots for disjoint scopes.
Date: Sun, 9 Mar 2003 15:45:47 -0500

 On Sun, Mar 09, 2003 at 11:32:47AM -0500, Jim Wilson wrote:
 > This patch seems more complicated than necessary.  expand_end_bindings 
 > already calls pop_temp_slots.  So all you have to do is arrange for 
 > locals to have the right temp_slot_level.
 > 
 > I tried looking at this.  We are calling preserve_stack_temps, and that 
 > preserves all temp slots whose address was taken.  This means a local of 
 > array type won't be freed when its scope terminates.  The question then 
 > is whether this is really necessary to preserve values whose address was 
 >      taken.
 > 
 > This code is used to preserve values that might be needed by an 
 > expression statement result.  In this case, the result is const0_rtx. 
 > This code has a test for !MEM.  This is apparently to handle the case 
 > where the result is in a REG that might contain an address.  However, 
 > !MEM also includes constants, and constants can't possibly contain the 
 > address of a stack local.  I think this problem would go away if we 
 > fixed this code to do nothing for constant results.  The problematic 
 > code is here:
 >   /* If X is not in memory or is at a constant address, it cannot be in
 >      a temporary slot, but it can contain something whose address was
 >      taken.  */
 >   if (p == 0 && (GET_CODE (x) != MEM || CONSTANT_P (XEXP (x, 0))))
 > We should do nothing here if x is a constant, but there is no code for that.
 
 I think I was looking at free_temp_slots instead of pop_temp_slots, so
 I just assumed p->keep was causing the problem; I didn't realize that
 pop_temp_slots overrode keep.  Thanks.  Your suggestion does fix
 Richard's test, so I bootstrapped and tested it on i686-pc-linux-gnu. 
 The patch is below; however, it causes one new failure:
 
 FAIL: g++.dg/opt/alias2.C execution test
 
 The interesting segment of the diff is:
 -       movl    %eax, -56(%ebp)
 -       movl    -24(%ebp), %eax
 -       movl    %eax, -44(%ebp)
 +       movl    %eax, -24(%ebp)
 +       movl    %eax, -12(%ebp)
         call    _ZN11_Deque_baseD2Ev
 
 I'm not quite sure what's going on.  0x123 ends up in eax instead of, I
 think, the "this" pointer.
 
 -- 
 Daniel Jacobowitz
 MontaVista Software                         Debian GNU/Linux Developer
 
 2003-03-09  Daniel Jacobowitz  <drow@mvista.com>
 
 	* function.c (preserve_temp_slots): If X is constant, don't
 	preserve temp slots.  Suggested by Jim Wilson.
 
 Index: function.c
 ===================================================================
 RCS file: /cvs/gcc/gcc/gcc/function.c,v
 retrieving revision 1.405
 diff -u -p -r1.405 function.c
 --- function.c	4 Mar 2003 06:20:12 -0000	1.405
 +++ function.c	9 Mar 2003 16:52:29 -0000
 @@ -1132,6 +1132,10 @@ preserve_temp_slots (x)
        return;
      }
  
 +  /* If X is a constant, then we don't need to preserve stack slots.  */
 +  if (CONSTANT_P (x))
 +    return;
 +
    /* If X is a register that is being used as a pointer, see if we have
       a temporary slot we know it points to.  To be consistent with
       the code below, we really should preserve all non-kept slots