This is the mail archive of the gcc-patches@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]
Other format: [Raw text]

store_bit_field, CONCATs and subregs


It's obviously the season for complex modes and subregs.  I wasn't sure
whether there were more patches pending in this area, but just in case
there aren't, this three-pronged patch fixes the remaining struct-layout-1
failures on mips-sgi-irix6.5.

In the testcases, store_bit_field is trying to store a CHImode CONCAT to
an unaligned address.  It first tries to mode-pun the value to SImode:

  /* If VALUE is a floating-point mode, access it as an integer of the
     corresponding size.  This can occur on a machine with 64 bit registers
     that uses SFmode for float.  This can also occur for unaligned float
     structure fields.  */
  orig_value = value;
  if (GET_MODE_CLASS (GET_MODE (value)) != MODE_INT
      && GET_MODE_CLASS (GET_MODE (value)) != MODE_PARTIAL_INT)
    value = gen_lowpart ((GET_MODE (value) == VOIDmode
			  ? word_mode : int_mode_for_mode (GET_MODE (value))),
			 value);

and the problem is with the rtx that gen_lowpart returns.

Things first go wrong because:

    simplify_subreg (SImode, (concat:CHImode X Y), CHImode, 0)

returns (subreg:SI X 0), i.e. a paradoxical SImode subreg of the
real part.  This is caused by the CONCAT case picking either the
real or imaginary part without checking whether the requested subreg
is entirely contained within it.  The simplify-rtx.c hunk below
(hopefully fairly obvious) adds an extra sanity check.

That's enough to make simplify_subreg return null, but the problem then
is that simplify_gen_subreg returns:

    (subreg:SI (concat:CHI X Y) 0)

My understanding is that this subreg isn't valid (the justification
being being that subregs normally select a consecutive bit sequence).
However, I wasn't sure in the new world order whether this should be
checked in validate_subreg or in simplify_subreg itself.  I went for
simplify_subreg in the patch below because:

   (a) validate_subreg seems to assume that the given object is valid
       for _some_ subregs.  All its current checks are mode-based and
       it doesn't reject something based on the object type alone.

   (b) simplify_subreg already filters out subregs of SUBREGs, so it
       seemed natural to extend the check to CONCATs too.

These two hunks stop us generating the invalid subreg, but gen_lowpart
has nothing to fall back on, so the call quoted above now fails.  The
right fix for that depends on where we're headed.  Is this something
that gen_lowpart_general should handle, or should store_bit_field
not be asking for this lowpart in the first place?

The third hunk adds a case to gen_lowpart_general.  It creates an integer
pseudo of the lowpart mode, takes a paradoxical complex subreg of it,
then moves the CONCAT into that subreg.  This works correctly thanks
to the new emit_move_insn handling.

But, like I say, I realise that might not be the preferred fix.  If it
isn't, please let me know what is, and I'll test that version instead.

Bootstrapped & regression tested on mips-sgi-irix6.5.  OK to install?

Richard


	* simplify-rtx.c (simplify_subreg): In the CONCAT case, check whether
	the request subreg is entirely contained in the requested component.
	(simplify_gen_subreg): Return null for CONCATs that are rejected
	by simplify_subreg.
	* rtlhooks.c (gen_lowpart_general): Handle CONCATs.

Index: simplify-rtx.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/simplify-rtx.c,v
retrieving revision 1.213
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.213 simplify-rtx.c
*** simplify-rtx.c	3 Dec 2004 03:27:34 -0000	1.213
--- simplify-rtx.c	6 Dec 2004 22:41:27 -0000
*************** simplify_subreg (enum machine_mode outer
*** 3722,3738 ****
       of real and imaginary part.  */
    if (GET_CODE (op) == CONCAT)
      {
!       int is_realpart = byte < (unsigned int) GET_MODE_UNIT_SIZE (innermode);
!       rtx part = is_realpart ? XEXP (op, 0) : XEXP (op, 1);
!       unsigned int final_offset;
!       rtx res;
  
-       final_offset = byte % (GET_MODE_UNIT_SIZE (innermode));
        res = simplify_subreg (outermode, part, GET_MODE (part), final_offset);
        if (res)
  	return res;
        if (validate_subreg (outermode, GET_MODE (part), part, final_offset))
!         return gen_rtx_SUBREG (outermode, part, final_offset);
        return NULL_RTX;
      }
  
--- 3722,3741 ----
       of real and imaginary part.  */
    if (GET_CODE (op) == CONCAT)
      {
!       unsigned int inner_size, final_offset;
!       rtx part, res;
! 
!       inner_size = GET_MODE_UNIT_SIZE (innermode);
!       part = byte < inner_size ? XEXP (op, 0) : XEXP (op, 1);
!       final_offset = byte % inner_size;
!       if (final_offset + GET_MODE_SIZE (outermode) > inner_size)
! 	return NULL_RTX;
  
        res = simplify_subreg (outermode, part, GET_MODE (part), final_offset);
        if (res)
  	return res;
        if (validate_subreg (outermode, GET_MODE (part), part, final_offset))
! 	return gen_rtx_SUBREG (outermode, part, final_offset);
        return NULL_RTX;
      }
  
*************** simplify_gen_subreg (enum machine_mode o
*** 3786,3792 ****
    if (newx)
      return newx;
  
!   if (GET_CODE (op) == SUBREG || GET_MODE (op) == VOIDmode)
      return NULL_RTX;
  
    if (validate_subreg (outermode, innermode, op, byte))
--- 3789,3797 ----
    if (newx)
      return newx;
  
!   if (GET_CODE (op) == SUBREG
!       || GET_CODE (op) == CONCAT
!       || GET_MODE (op) == VOIDmode)
      return NULL_RTX;
  
    if (validate_subreg (outermode, innermode, op, byte))
Index: rtlhooks.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/rtlhooks.c,v
retrieving revision 2.5
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r2.5 rtlhooks.c
*** rtlhooks.c	9 Sep 2004 17:19:12 -0000	2.5
--- rtlhooks.c	6 Dec 2004 22:41:26 -0000
*************** gen_lowpart_general (enum machine_mode m
*** 50,55 ****
--- 50,61 ----
        gcc_assert (result != 0);
        return result;
      }
+   else if (GET_CODE (x) == CONCAT)
+     {
+       rtx tmp = gen_reg_rtx (mode);
+       emit_move_insn (gen_lowpart_common (GET_MODE (x), tmp), x);
+       return tmp;
+     }
    else
      {
        int offset = 0;


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