[cft] fix big-endian bitfield problems

Richard Henderson rth@redhat.com
Sat Nov 13 00:33:00 GMT 2004


On Fri, Nov 12, 2004 at 05:03:31PM -0500, Jakub Jelinek wrote:
> /usr/src/gcc/gcc/testsuite/gcc.dg/compat/struct-by-value-11_x.c:17: error:
> unable to find a register to spill in class 'CREG'
> /usr/src/gcc/gcc/testsuite/gcc.dg/compat/struct-by-value-11_x.c:17: error:
> this is the insn:
> (insn 254 253 255 0 (parallel [
>             (set (reg:DI 41 r12 [202])
>                 (lshiftrt:DI (subreg:DI (reg:TI 39 r10 [198]) 8)
>                     (subreg:QI (reg:DI 0 ax [200]) 0)))
>             (clobber (reg:CC 17 flags))
>         ]) 443 {*lshrdi3_1_rex64} (nil)

Indeed.

This is a latent bug.  We are mixing computation and loading values
into hard registers for a function call.  Which is verboten for 
exactly the reason shown above.  Previously the computation consisted
only of shifts with constant bit counts, whose arguments are not tied
to particular register classes, so we happened to get away with it on
this particular target.

By inspection, we have a similar problem copying values out of hard
registers on function entry.  This one is sigificantly more difficult
to fix.  I believe that I've covered the most common cases, but there
is still the matter of the emit_group_store call in
assign_parm_adjust_entry_rtl; I don't know how to handle that one.

I'm testing the following fix.


r~


Index: calls.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/calls.c,v
retrieving revision 1.368
diff -c -p -d -r1.368 calls.c
*** calls.c	8 Nov 2004 19:03:17 -0000	1.368
--- calls.c	13 Nov 2004 00:21:28 -0000
*************** call_expr_flags (tree t)
*** 644,650 ****
     Set REG_PARM_SEEN if we encounter a register parameter.  */
  
  static void
! precompute_register_parameters (int num_actuals, struct arg_data *args, int *reg_parm_seen)
  {
    int i;
  
--- 644,651 ----
     Set REG_PARM_SEEN if we encounter a register parameter.  */
  
  static void
! precompute_register_parameters (int num_actuals, struct arg_data *args,
! 				int *reg_parm_seen)
  {
    int i;
  
*************** precompute_register_parameters (int num_
*** 679,684 ****
--- 680,696 ----
  			     TYPE_MODE (TREE_TYPE (args[i].tree_value)),
  			     args[i].value, args[i].unsignedp);
  
+ 	/* If we're going to have to load the value by parts, pull the
+ 	   parts into pseudos.  The part extraction process can involve
+ 	   non-trivial computation.  */
+ 	if (GET_CODE (args[i].reg) == PARALLEL)
+ 	  {
+ 	    tree type = TREE_TYPE (args[i].tree_value);
+ 	    args[i].value
+ 	      = emit_group_load_into_temps (args[i].reg, args[i].value,
+ 					    type, int_size_in_bytes (type));
+ 	  }
+ 
  	/* If the value is expensive, and we are inside an appropriately
  	   short loop, put the value into a pseudo and then put the pseudo
  	   into the hard reg.
*************** precompute_register_parameters (int num_
*** 687,699 ****
  	   register parameters.  This is to avoid reload conflicts while
  	   loading the parameters registers.  */
  
! 	if ((! (REG_P (args[i].value)
! 		|| (GET_CODE (args[i].value) == SUBREG
! 		    && REG_P (SUBREG_REG (args[i].value)))))
! 	    && args[i].mode != BLKmode
! 	    && rtx_cost (args[i].value, SET) > COSTS_N_INSNS (1)
! 	    && ((SMALL_REGISTER_CLASSES && *reg_parm_seen)
! 		|| optimize))
  	  args[i].value = copy_to_mode_reg (args[i].mode, args[i].value);
        }
  }
--- 699,711 ----
  	   register parameters.  This is to avoid reload conflicts while
  	   loading the parameters registers.  */
  
! 	else if ((! (REG_P (args[i].value)
! 		     || (GET_CODE (args[i].value) == SUBREG
! 			 && REG_P (SUBREG_REG (args[i].value)))))
! 		 && args[i].mode != BLKmode
! 		 && rtx_cost (args[i].value, SET) > COSTS_N_INSNS (1)
! 		 && ((SMALL_REGISTER_CLASSES && *reg_parm_seen)
! 		     || optimize))
  	  args[i].value = copy_to_mode_reg (args[i].mode, args[i].value);
        }
  }
*************** load_register_parameters (struct arg_dat
*** 1454,1464 ****
  	     locations.  The Irix 6 ABI has examples of this.  */
  
  	  if (GET_CODE (reg) == PARALLEL)
! 	    {
! 	      tree type = TREE_TYPE (args[i].tree_value);
! 	      emit_group_load (reg, args[i].value, type,
! 			       int_size_in_bytes (type));
! 	    }
  
  	  /* If simple case, just do move.  If normal partial, store_one_arg
  	     has already loaded the register for us.  In all other cases,
--- 1466,1472 ----
  	     locations.  The Irix 6 ABI has examples of this.  */
  
  	  if (GET_CODE (reg) == PARALLEL)
! 	    emit_group_move (reg, args[i].value);
  
  	  /* If simple case, just do move.  If normal partial, store_one_arg
  	     has already loaded the register for us.  In all other cases,
Index: expr.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/expr.c,v
retrieving revision 1.739
diff -c -p -d -r1.739 expr.c
*** expr.c	12 Nov 2004 06:59:47 -0000	1.739
--- expr.c	13 Nov 2004 00:21:30 -0000
*************** gen_group_rtx (rtx orig)
*** 1557,1571 ****
    return gen_rtx_PARALLEL (GET_MODE (orig), gen_rtvec_v (length, tmps));
  }
  
! /* Emit code to move a block ORIG_SRC of type TYPE to a block DST,
!    where DST is non-consecutive registers represented by a PARALLEL.
!    SSIZE represents the total size of block ORIG_SRC in bytes, or -1
!    if not known.  */
  
! void
! emit_group_load (rtx dst, rtx orig_src, tree type ATTRIBUTE_UNUSED, int ssize)
  {
!   rtx *tmps, src;
    int start, i;
    enum machine_mode m = GET_MODE (orig_src);
  
--- 1557,1570 ----
    return gen_rtx_PARALLEL (GET_MODE (orig), gen_rtvec_v (length, tmps));
  }
  
! /* A subroutine of emit_group_load.  Arguments as for emit_group_load,
!    except that values are placed in TMPS[i], and must later be moved
!    into corrosponding XEXP (XVECEXP (DST, 0, i), 0) element.  */
  
! static void
! emit_group_load_1 (rtx *tmps, rtx dst, rtx orig_src, tree type, int ssize)
  {
!   rtx src;
    int start, i;
    enum machine_mode m = GET_MODE (orig_src);
  
*************** emit_group_load (rtx dst, rtx orig_src, 
*** 1585,1591 ****
        /* ...and back again.  */
        if (imode != BLKmode)
  	src = gen_lowpart (imode, src);
!       emit_group_load (dst, src, type, ssize);
        return;
      }
  
--- 1584,1590 ----
        /* ...and back again.  */
        if (imode != BLKmode)
  	src = gen_lowpart (imode, src);
!       emit_group_load_1 (tmps, dst, src, type, ssize);
        return;
      }
  
*************** emit_group_load (rtx dst, rtx orig_src, 
*** 1596,1603 ****
    else
      start = 1;
  
-   tmps = alloca (sizeof (rtx) * XVECLEN (dst, 0));
- 
    /* Process the pieces.  */
    for (i = start; i < XVECLEN (dst, 0); i++)
      {
--- 1595,1600 ----
*************** emit_group_load (rtx dst, rtx orig_src, 
*** 1709,1718 ****
  	tmps[i] = expand_shift (LSHIFT_EXPR, mode, tmps[i],
  				build_int_cst (NULL_TREE, shift), tmps[i], 0);
      }
  
    /* Copy the extracted pieces into the proper (probable) hard regs.  */
!   for (i = start; i < XVECLEN (dst, 0); i++)
!     emit_move_insn (XEXP (XVECEXP (dst, 0, i), 0), tmps[i]);
  }
  
  /* Emit code to move a block SRC to block DST, where SRC and DST are
--- 1706,1766 ----
  	tmps[i] = expand_shift (LSHIFT_EXPR, mode, tmps[i],
  				build_int_cst (NULL_TREE, shift), tmps[i], 0);
      }
+ }
+ 
+ /* Emit code to move a block SRC of type TYPE to a block DST,
+    where DST is non-consecutive registers represented by a PARALLEL.
+    SSIZE represents the total size of block ORIG_SRC in bytes, or -1
+    if not known.  */
+ 
+ void
+ emit_group_load (rtx dst, rtx src, tree type, int ssize)
+ {
+   rtx *tmps;
+   int i;
+ 
+   tmps = alloca (sizeof (rtx) * XVECLEN (dst, 0));
+   emit_group_load_1 (tmps, dst, src, type, ssize);
  
    /* Copy the extracted pieces into the proper (probable) hard regs.  */
!   for (i = 0; i < XVECLEN (dst, 0); i++)
!     {
!       rtx d = XEXP (XVECEXP (dst, 0, i), 0);
!       if (d == NULL)
! 	continue;
!       emit_move_insn (d, tmps[i]);
!     }
! }
! 
! /* Similar, but load SRC into new pseudos in a format that looks like
!    PARALLEL.  This can later be fed to emit_group_move to get things
!    in the right place.  */
! 
! rtx
! emit_group_load_into_temps (rtx parallel, rtx src, tree type, int ssize)
! {
!   rtvec vec;
!   int i;
! 
!   vec = rtvec_alloc (XVECLEN (parallel, 0));
!   emit_group_load_1 (&RTVEC_ELT (vec, 0), parallel, src, type, ssize);
! 
!   /* Convert the vector to look just like the original PARALLEL, except
!      with the computed values.  */
!   for (i = 0; i < XVECLEN (parallel, 0); i++)
!     {
!       rtx e = XVECEXP (parallel, 0, i);
!       rtx d = XEXP (e, 0);
! 
!       if (d)
! 	{
! 	  d = force_reg (GET_MODE (d), RTVEC_ELT (vec, i));
! 	  e = alloc_EXPR_LIST (REG_NOTE_KIND (e), d, XEXP (e, 1));
! 	}
!       RTVEC_ELT (vec, i) = e;
!     }
! 
!   return gen_rtx_PARALLEL (GET_MODE (parallel), vec);
  }
  
  /* Emit code to move a block SRC to block DST, where SRC and DST are
*************** emit_group_move (rtx dst, rtx src)
*** 1733,1738 ****
--- 1781,1807 ----
  		    XEXP (XVECEXP (src, 0, i), 0));
  }
  
+ /* Move a group of registers represented by a PARALLEL into pseudos.  */
+ 
+ rtx
+ emit_group_move_into_temps (rtx src)
+ {
+   rtvec vec = rtvec_alloc (XVECLEN (src, 0));
+   int i;
+ 
+   for (i = 0; i < XVECLEN (src, 0); i++)
+     {
+       rtx e = XVECEXP (src, 0, i);
+       rtx d = XEXP (e, 0);
+ 
+       if (d)
+ 	e = alloc_EXPR_LIST (REG_NOTE_KIND (e), copy_to_reg (d), XEXP (e, 1));
+       RTVEC_ELT (vec, i) = e;
+     }
+ 
+   return gen_rtx_PARALLEL (GET_MODE (src), vec);
+ }
+ 
  /* Emit code to move a block SRC to a block ORIG_DST of type TYPE,
     where SRC is non-consecutive registers represented by a PARALLEL.
     SSIZE represents the total size of block ORIG_DST, or -1 if not
Index: expr.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/expr.h,v
retrieving revision 1.178
diff -c -p -d -r1.178 expr.h
*** expr.h	11 Nov 2004 23:14:21 -0000	1.178
--- expr.h	13 Nov 2004 00:21:31 -0000
*************** extern rtx gen_group_rtx (rtx);
*** 387,396 ****
--- 387,402 ----
     PARALLEL.  */
  extern void emit_group_load (rtx, rtx, tree, int);
  
+ /* Similarly, but load into new temporaries.  */
+ extern rtx emit_group_load_into_temps (rtx, rtx, tree, int);
+ 
  /* Move a non-consecutive group of registers represented by a PARALLEL into
     a non-consecutive group of registers represented by a PARALLEL.  */
  extern void emit_group_move (rtx, rtx);
  
+ /* Move a group of registers represented by a PARALLEL into pseudos.  */
+ extern rtx emit_group_move_into_temps (rtx);
+ 
  /* Store a BLKmode value from non-consecutive registers represented by a
     PARALLEL.  */
  extern void emit_group_store (rtx, rtx, tree, int);
Index: function.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/function.c,v
retrieving revision 1.587
diff -c -p -d -r1.587 function.c
*** function.c	11 Nov 2004 22:02:46 -0000	1.587
--- function.c	13 Nov 2004 00:21:32 -0000
*************** assign_parm_setup_block_p (struct assign
*** 2536,2546 ****
     present and valid in DATA->STACK_RTL.  */
  
  static void
! assign_parm_setup_block (tree parm, struct assign_parm_data_one *data)
  {
    rtx entry_parm = data->entry_parm;
    rtx stack_parm = data->stack_parm;
  
    /* If we've a non-block object that's nevertheless passed in parts,
       reconstitute it in register operations rather than on the stack.  */
    if (GET_CODE (entry_parm) == PARALLEL
--- 2536,2550 ----
     present and valid in DATA->STACK_RTL.  */
  
  static void
! assign_parm_setup_block (struct assign_parm_data_all *all,
! 			 tree parm, struct assign_parm_data_one *data)
  {
    rtx entry_parm = data->entry_parm;
    rtx stack_parm = data->stack_parm;
  
+   if (GET_CODE (entry_parm) == PARALLEL)
+     entry_parm = emit_group_move_into_temps (entry_parm);
+ 
    /* If we've a non-block object that's nevertheless passed in parts,
       reconstitute it in register operations rather than on the stack.  */
    if (GET_CODE (entry_parm) == PARALLEL
*************** assign_parm_setup_block (tree parm, stru
*** 2550,2555 ****
--- 2554,2561 ----
      {
        rtx parmreg = gen_reg_rtx (data->nominal_mode);
  
+       push_to_sequence (all->conversion_insns);
+ 
        /* For values returned in multiple registers, handle possible
  	 incompatible calls to emit_group_store.
  
*************** assign_parm_setup_block (tree parm, stru
*** 2572,2577 ****
--- 2578,2587 ----
        else
  	emit_group_store (parmreg, entry_parm, data->nominal_type,
  			  int_size_in_bytes (data->nominal_type));
+ 
+       all->conversion_insns = get_insns ();
+       end_sequence ();
+ 
        SET_DECL_RTL (parm, parmreg);
        return;
      }
*************** assign_parm_setup_block (tree parm, stru
*** 2609,2615 ****
  
        /* Handle values in multiple non-contiguous locations.  */
        if (GET_CODE (entry_parm) == PARALLEL)
! 	emit_group_store (mem, entry_parm, data->passed_type, size);
  
        else if (size == 0)
  	;
--- 2619,2630 ----
  
        /* Handle values in multiple non-contiguous locations.  */
        if (GET_CODE (entry_parm) == PARALLEL)
! 	{
! 	  push_to_sequence (all->conversion_insns);
! 	  emit_group_store (mem, entry_parm, data->passed_type, size);
! 	  all->conversion_insns = get_insns ();
! 	  end_sequence ();
! 	}
  
        else if (size == 0)
  	;
*************** assign_parm_setup_block (tree parm, stru
*** 2648,2654 ****
  	    {
  	      rtx tem, x;
  	      int by = (UNITS_PER_WORD - size) * BITS_PER_UNIT;
! 	      rtx reg = gen_rtx_REG (word_mode, REGNO (data->entry_parm));
  
  	      x = expand_shift (LSHIFT_EXPR, word_mode, reg,
  				build_int_cst (NULL_TREE, by),
--- 2663,2669 ----
  	    {
  	      rtx tem, x;
  	      int by = (UNITS_PER_WORD - size) * BITS_PER_UNIT;
! 	      rtx reg = gen_lowpart (word_mode, entry_parm);
  
  	      x = expand_shift (LSHIFT_EXPR, word_mode, reg,
  				build_int_cst (NULL_TREE, by),
*************** assign_parm_setup_block (tree parm, stru
*** 2657,2667 ****
  	      emit_move_insn (tem, x);
  	    }
  	  else
! 	    move_block_from_reg (REGNO (data->entry_parm), mem,
  				 size_stored / UNITS_PER_WORD);
  	}
        else
! 	move_block_from_reg (REGNO (data->entry_parm), mem,
  			     size_stored / UNITS_PER_WORD);
      }
  
--- 2672,2682 ----
  	      emit_move_insn (tem, x);
  	    }
  	  else
! 	    move_block_from_reg (REGNO (entry_parm), mem,
  				 size_stored / UNITS_PER_WORD);
  	}
        else
! 	move_block_from_reg (REGNO (entry_parm), mem,
  			     size_stored / UNITS_PER_WORD);
      }
  
*************** assign_parm_setup_reg (struct assign_par
*** 2782,2788 ****
  	  emit_move_insn (tempreg, DECL_RTL (parm));
  	  tempreg = convert_to_mode (GET_MODE (parmreg), tempreg, unsigned_p);
  	  emit_move_insn (parmreg, tempreg);
! 	  all->conversion_insns = get_insns();
  	  end_sequence ();
  
  	  did_conversion = true;
--- 2797,2803 ----
  	  emit_move_insn (tempreg, DECL_RTL (parm));
  	  tempreg = convert_to_mode (GET_MODE (parmreg), tempreg, unsigned_p);
  	  emit_move_insn (parmreg, tempreg);
! 	  all->conversion_insns = get_insns ();
  	  end_sequence ();
  
  	  did_conversion = true;
*************** assign_parms (tree fndecl)
*** 3083,3089 ****
        assign_parm_adjust_stack_rtl (&data);
  
        if (assign_parm_setup_block_p (&data))
! 	assign_parm_setup_block (parm, &data);
        else if (data.passed_pointer || use_register_for_decl (parm))
  	assign_parm_setup_reg (&all, parm, &data);
        else
--- 3098,3104 ----
        assign_parm_adjust_stack_rtl (&data);
  
        if (assign_parm_setup_block_p (&data))
! 	assign_parm_setup_block (&all, parm, &data);
        else if (data.passed_pointer || use_register_for_decl (parm))
  	assign_parm_setup_reg (&all, parm, &data);
        else



More information about the Gcc-patches mailing list