Comment at end of stmt:expand_decl

Jim Wilson wilson@cygnus.com
Mon Feb 7 16:00:00 GMT 2000


In article < 10002062338.AA27015@vlsi1.ultra.nyu.edu > you write:
>Does that comment make sense to anybody?

Yes.  My original justification for it follows.  I see that loop no longer
uses RTX_UNCHANGING_P, and function inlining is different now, and will get
even more different when tree level inlining propagates from the C++ front
end to the C front end, so it seems reasonable that this change may not be
needed anymore.

>From wilson  Thu May 13 17:41:27 1993
Received: by cygnus.com (4.1/SMI-4.1)
	id AA14228; Thu, 13 May 93 17:36:40 PDT
Return-Path: <wilson@cygnus.com>
Received: from sphagnum.cygnus.com by cygnus.com (4.1/SMI-4.1)
	id AA14221; Thu, 13 May 93 17:36:39 PDT
Received: by sphagnum.cygnus.com (4.1/SMI-4.1)
	id AA09246; Thu, 13 May 93 17:36:38 PDT
Date: Thu, 13 May 93 17:36:38 PDT
From: wilson@cygnus.com
Message-Id: <9305140036.AA09246@sphagnum.cygnus.com>
To: gcc2
Subject: RTX_UNCHANGING_P and const variables


I have been working on a problem where incorrect code is generated by
the instruction scheduler because of a MEM that has RTX_UNCHANGING_P set.
The RTX_UNCHANGING_P bit came from a const parameter of a function
that was inlined into the current function.  I believe that it is wrong
for this mem to have the RTX_UNCHANGING_P bit set.

In trying to prove that it was a function inlining bug, and not a scheduler
bug, I managed to come up with an example that involves neither, but still
fails.

This program fails when compiled with -O on the sparc.

sub3 (i)
     const int *i;
{
}

main()
{
  int i, j;

  for (i = 0; i < 4; i++)
    {
      const int j = i;
      int k;
      sub3 (&j);
      k = j;
      printf ("%d %d\n", k, k);
    }
}

J is on the stack because it has its address taken, the store and load from
it have RTX_UNCHANGING_P set because the variable has const type.  The
loop optimizer then decides that the load from J is invariant because
it is RTX_UNCHANGING_P, and it has an invariant address, so it moves the
load before the loop.  This is wrong, J is not loop invariant.

Here is an example that fails because of a const parm and function inlining,
but is otherwise the same as the above example.

inline int
sub (g)
     const int g;
{
  sub3 (&g);
  return g;
}

sub3 (i)
     const int *i;
{
}

main()
{
  int i, j;

  for (i = 0; i < 4; i++)
    {
      j = sub (i);
      printf ("%d %d\n", j, j);
    }
}

I think the problem here is not loop/sched/etc, but the RTX_UNCHANGING_P bit.

I think that it is only safe to set RTX_UNCHANGING_P for static const
variables, and for const parameters.  And also that function inlining
needs to clear the RTX_UNCHANGING_P (except for constant pool references)
because the const parm is not necessarily const in the function that we
are inlining it into.

Patches to implement this follow.

Anybody see a different (and better) solution?

Thu May 13 17:15:26 1993  Jim Wilson  (wilson@sphagnum.cygnus.com)

	* integrate.c (expand_inline_function): Set map->integrating.
	(copy_rtx_and_substitute, case MEM): Don't copy RTX_UNCHANGING_P
	when doing function inlining.
	* integrate.h (struct inline_remap): Add integrating field.
	* stmt.c (expand_decl): Set RTX_UNCHANGING_P only for static const
	variables.
	* varasm.c (make_decl_rtl): Likewise.
	* unroll.c (unroll_loop): Clear map->integrating.

===================================================================
RCS file: /rel/cvsfiles/devo/gcc/integrate.c,v
retrieving revision 1.49
diff -p -r1.49 integrate.c
*** integrate.c	1993/04/22 00:40:44	1.49
--- integrate.c	1993/05/13 19:10:50
*************** expand_inline_function (fndecl, parms, t
*** 1284,1289 ****
--- 1284,1291 ----
    map->min_insnno = 0;
    map->max_insnno = INSN_UID (header);
  
+   map->integrating = 1;
+ 
    /* const_equiv_map maps pseudos in our routine to constants, so it needs to
       be large enough for all our pseudos.  This is the number we are currently
       using plus the number in the called routine, plus 15 for each arg,
*************** copy_rtx_and_substitute (orig, map)
*** 2148,2154 ****
        XEXP (copy, 0) = copy_rtx_and_substitute (XEXP (orig, 0), map);
        MEM_IN_STRUCT_P (copy) = MEM_IN_STRUCT_P (orig);
        MEM_VOLATILE_P (copy) = MEM_VOLATILE_P (orig);
!       RTX_UNCHANGING_P (copy) = RTX_UNCHANGING_P (orig);
        return copy;
      }
  
--- 2150,2164 ----
        XEXP (copy, 0) = copy_rtx_and_substitute (XEXP (orig, 0), map);
        MEM_IN_STRUCT_P (copy) = MEM_IN_STRUCT_P (orig);
        MEM_VOLATILE_P (copy) = MEM_VOLATILE_P (orig);
! 
!       /* If doing function inlining, this MEM might not be const in the
! 	 function that it is being inlined into, and thus may not be
! 	 unchanging after function inlining.  Constant pool references are
! 	 handled elsewhere, so this doesn't loose RTX_UNCHANGING_P bits
! 	 for them.  */
!       if (! map->integrating)
! 	RTX_UNCHANGING_P (copy) = RTX_UNCHANGING_P (orig);
! 
        return copy;
      }
  
===================================================================
RCS file: /rel/cvsfiles/devo/gcc/integrate.h,v
retrieving revision 1.9
diff -p -r1.9 integrate.h
*** integrate.h	1993/04/22 00:40:47	1.9
--- integrate.h	1993/05/13 19:05:58
*************** the Free Software Foundation, 675 Mass A
*** 31,36 ****
--- 31,40 ----
  
  struct inline_remap
  {
+   /* True if we are doing function integration, false otherwise.
+      Used to control whether RTX_UNCHANGING bits are copied by
+      copy_rtx_and_substitute. */
+   int integrating;
    /* Definition of function be inlined.  */
    union tree_node *fndecl;
    /* Place to put insns needed at start of function.  */
===================================================================
RCS file: /rel/cvsfiles/devo/gcc/stmt.c,v
retrieving revision 1.51
diff -p -r1.51 stmt.c
*** stmt.c	1993/04/22 00:43:12	1.51
--- stmt.c	1993/05/13 19:35:31
*************** expand_decl (decl)
*** 3054,3061 ****
  
    if (TREE_THIS_VOLATILE (decl))
      MEM_VOLATILE_P (DECL_RTL (decl)) = 1;
!   if (TREE_READONLY (decl))
      RTX_UNCHANGING_P (DECL_RTL (decl)) = 1;
  
    /* If doing stupid register allocation, make sure life of any
       register variable starts here, at the start of its scope.  */
--- 3054,3066 ----
  
    if (TREE_THIS_VOLATILE (decl))
      MEM_VOLATILE_P (DECL_RTL (decl)) = 1;
! #if 0
!   /* Only static const variables are unchanging across the entire
!      function.  Anything else might be aliased to a non-const variable,
!      or may only be const within a single block.  */
!   if (TREE_READONLY (decl) && TREE_STATIC (decl))
      RTX_UNCHANGING_P (DECL_RTL (decl)) = 1;
+ #endif
  
    /* If doing stupid register allocation, make sure life of any
       register variable starts here, at the start of its scope.  */
===================================================================
RCS file: /rel/cvsfiles/devo/gcc/unroll.c,v
retrieving revision 1.43
diff -p -r1.43 unroll.c
*** unroll.c	1993/04/05 20:52:39	1.43
--- unroll.c	1993/05/13 19:07:59
*************** unroll_loop (loop_end, insn_count, loop_
*** 620,625 ****
--- 620,627 ----
  
    map = (struct inline_remap *) alloca (sizeof (struct inline_remap));
  
+   map->integrating = 0;
+ 
    /* Allocate the label map.  */
  
    if (max_labelno > 0)
===================================================================
RCS file: /rel/cvsfiles/devo/gcc/varasm.c,v
retrieving revision 1.72
diff -p -r1.72 varasm.c
*** varasm.c	1993/05/06 02:11:35	1.72
--- varasm.c	1993/05/13 19:21:02
*************** make_decl_rtl (decl, asmspec, top_level)
*** 450,456 ****
  	    || (flag_volatile_global && TREE_CODE (decl) == VAR_DECL
  		&& TREE_PUBLIC (decl)))
  	    MEM_VOLATILE_P (DECL_RTL (decl)) = 1;
! 	  if (TREE_READONLY (decl))
  	    RTX_UNCHANGING_P (DECL_RTL (decl)) = 1;
  	  MEM_IN_STRUCT_P (DECL_RTL (decl))
  	    = (TREE_CODE (TREE_TYPE (decl)) == ARRAY_TYPE
--- 450,459 ----
  	    || (flag_volatile_global && TREE_CODE (decl) == VAR_DECL
  		&& TREE_PUBLIC (decl)))
  	    MEM_VOLATILE_P (DECL_RTL (decl)) = 1;
! 	  /* Only static const variables are unchanging across the entire
! 	     function.  Anything else might be aliased to a non-const variable,
! 	     or may only be const within a single block.  */
! 	  if (TREE_READONLY (decl) && TREE_STATIC (decl))
  	    RTX_UNCHANGING_P (DECL_RTL (decl)) = 1;
  	  MEM_IN_STRUCT_P (DECL_RTL (decl))
  	    = (TREE_CODE (TREE_TYPE (decl)) == ARRAY_TYPE





More information about the Gcc mailing list