This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

PR2076



PR2076 is an optimization bug on RS6000/AIX.  (It's a regression from
GCC 2.95.x, so it's a high-priority bug for the GCC 3.0 release.)

We have a function:

  void fdouble (double one, ...)

We generate RTL that looks like

  ;; Save R3 into the varargs area.

  (insn 4 2 6 (set (mem:SI (reg/f:SI 77 virtual-incoming-args) 2)
	  (reg:SI 3 r3)) -1 (nil)
      (nil))
  ...

  ;; Load `one' from the saved value.

  (insn 20 18 21 (set (reg/v:DF 82)
	  (mem/f:DF (reg/f:SI 77 virtual-incoming-args) 3)) -1 (nil)
      (nil))

This is bogus because the two MEMs are in different alias sets.  So,
the scheduler reorders the load and store.

The bug comes from this hunk in rs6000.c:setup_incoming_varargs:

  set = get_varargs_alias_set ();
  if (! no_rtl && first_reg_offset < GP_ARG_NUM_REG)
    {
      mem = gen_rtx_MEM (BLKmode,
		         plus_constant (save_area,
					first_reg_offset * reg_size)),
      MEM_ALIAS_SET (mem) = set;

      move_block_from_reg
	(GP_ARG_MIN_REG + first_reg_offset, mem,
	 GP_ARG_NUM_REG - first_reg_offset,
	 (GP_ARG_NUM_REG - first_reg_offset) * UNITS_PER_WORD);

The first thing we're storing is the last named parameter.

Certainly, one safe way to fix the bug is to not use the alias set
here.  If I don't hear a better solution, I'll do that on the release
branch at least.

I *think* that what we want to do is store only the last named
parameter in alias set zero.  If so, then something like the attached
patch should be right.  (It needs cleaning up; for one thing, it's
appalling that we don't have FN_TYPE_STDARGS_P and FN_TYPE_VARARGS_P
macros.)

This patch does seem to fix David's test-case.  

I would appreciate two things:

  - David, would you mind firing off an AIX bootstrap to test 
    the patch?

  - Richard, since you seem to have introduced the alias set stuff
    (in revision 1.88), would you care to comment?

Thanks in advance to both,

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Index: rs6000.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/rs6000/rs6000.c,v
retrieving revision 1.167
diff -c -p -r1.167 rs6000.c
*** rs6000.c	2001/02/09 03:15:56	1.167
--- rs6000.c	2001/04/25 18:45:17
*************** setup_incoming_varargs (cum, mode, type,
*** 2208,2230 ****
    int reg_size = TARGET_32BIT ? 4 : 8;
    rtx save_area, mem;
    int first_reg_offset, set;
  
    if (DEFAULT_ABI == ABI_V4 || DEFAULT_ABI == ABI_SOLARIS)
      {
-       tree fntype;
-       int stdarg_p;
- 
-       fntype = TREE_TYPE (current_function_decl);
-       stdarg_p = (TYPE_ARG_TYPES (fntype) != 0
- 		  && (TREE_VALUE (tree_last (TYPE_ARG_TYPES (fntype)))
- 		      != void_type_node));
- 
-       /* For varargs, we do not want to skip the dummy va_dcl argument.
-          For stdargs, we do want to skip the last named argument.  */
-       next_cum = *cum;
-       if (stdarg_p)
- 	function_arg_advance (&next_cum, mode, type, 1);
- 
        /* Indicate to allocate space on the stack for varargs save area.  */
        /* ??? Does this really have to be located at a magic spot on the
  	 stack, or can we allocate this with assign_stack_local instead.  */
--- 2208,2229 ----
    int reg_size = TARGET_32BIT ? 4 : 8;
    rtx save_area, mem;
    int first_reg_offset, set;
+   tree fntype;
+   int stdarg_p;
  
+   fntype = TREE_TYPE (current_function_decl);
+   stdarg_p = (TYPE_ARG_TYPES (fntype) != 0
+ 	      && (TREE_VALUE (tree_last (TYPE_ARG_TYPES (fntype)))
+ 		  != void_type_node));
+ 
+   /* For varargs, we do not want to skip the dummy va_dcl argument.
+      For stdargs, we do want to skip the last named argument.  */
+   next_cum = *cum;
+   if (stdarg_p)
+     function_arg_advance (&next_cum, mode, type, 1);
+ 
    if (DEFAULT_ABI == ABI_V4 || DEFAULT_ABI == ABI_SOLARIS)
      {
        /* Indicate to allocate space on the stack for varargs save area.  */
        /* ??? Does this really have to be located at a magic spot on the
  	 stack, or can we allocate this with assign_stack_local instead.  */
*************** setup_incoming_varargs (cum, mode, type,
*** 2239,2248 ****
      {
        save_area = virtual_incoming_args_rtx;
        cfun->machine->sysv_varargs_p = 0;
  
!       first_reg_offset = cum->words;
!       if (MUST_PASS_IN_STACK (mode, type))
! 	first_reg_offset += RS6000_ARG_SIZE (TYPE_MODE (type), type, 1);
      }
  
    set = get_varargs_alias_set ();
--- 2238,2256 ----
      {
        save_area = virtual_incoming_args_rtx;
        cfun->machine->sysv_varargs_p = 0;
+ 
+       first_reg_offset = next_cum.words;
  
!       if (next_cum.words != cum->words)
! 	{
! 	  mem = gen_rtx_MEM (BLKmode,
! 			     plus_constant (save_area,
! 					    cum->words * reg_size));
! 	  move_block_from_reg
! 	    (GP_ARG_MIN_REG + cum->words, mem,
! 	     first_reg_offset - cum->words,
! 	     (first_reg_offset - cum->words) * UNITS_PER_WORD);
! 	}
      }
  
    set = get_varargs_alias_set ();


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]