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]

Re: store_bit_field, CONCATs and subregs


Richard Henderson <rth@redhat.com> writes:
> I think the proper solution is there in store_bit_field, noticing
> that we need to copy the concat into a temporary of the proper
> integer mode, rather than using gen_lowpart.

OK, patch below.  I wasn't sure whether it was worth treating CONCATs
as a special case or not.  Aside from the usual reason of not wanting
to make unnecessary distinctions, I think using a temporary might avoid
other problems too.

A couple of week ago, I noticed that tests like:

    struct s { _Complex short cs __attribute__((packed)); };
    void f (struct s *s) { s->cs = 1 + 2i; }

could fail due to alignment problems.  The value we're storing into
s->cs is a CHImode constant pool reference:

    (mem/i:CHI (lo_sum:SI (reg:SI 182)
                          (symbol_ref/f:SI ("*$LC0")
        [flags 0x2] <complex_cst 0x401aab70>)) [0 S4 A16])

and the lowpart generated by the current store_bit_field code is:

    (mem/i:SI (lo_sum:SI (reg:SI 182)
                         (symbol_ref/f:SI ("*$LC0")
        [flags 0x2] <complex_cst 0x401aab70>)) [0 S4 A16])

Both MEMs say that the reference only has 16-bit alignment, but it's
unusual (maybe even wrong?) to have unaligned SImode MEMs on a
strict-alignment target.  Leastways, the insv code that follows
just validates VALUE with:

      /* If this machine's insv insists on a register,
         get VALUE1 into a register.  */
      if (! ((*insn_data[(int) CODE_FOR_insv].operand[3].predicate)
             (value1, maxmode)))
        value1 = force_reg (maxmode, value1);

and nothing called by force_reg() considers unaligned MEMs.
We therefore end up with a normal word-sized load from $LC0.

Having a REG rather than a SUBREG value might also make it easier
to optimise the unaligned store instructions that follow.

FWIW, I tried some random-ish examples by hand to see it if the patch
generated an unnecessary move.  I didn't find any cases where it did.

Also, I wondered whether there was a specific reason why we had to
generate the word_mode lowpart of a CONST_INT, rather than just keep
the original value.  Judging from:

    http://gcc.gnu.org/ml/gcc-patches/2002-11/msg01942.html

the current handling seems to have been an afterthought, and IMO it
would be cleaner to treat CONST_INT values in the same way as other
integer values.

Boostrapped & regression tested on mips-sgi-irix6.5 in combination with
the two approved hunks (which I haven't committed because I didn't test
them separately).  Same test results as before.  OK to install?

Richard


	* expmed.c (store_bit_field): Create a temporary when changing the
	value to an integer mode.

Index: expmed.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/expmed.c,v
retrieving revision 1.211
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.211 expmed.c
*** expmed.c	1 Dec 2004 18:13:26 -0000	1.211
--- expmed.c	8 Dec 2004 18:58:37 -0000
*************** store_bit_field (rtx str_rtx, unsigned H
*** 588,603 ****
        offset = 0;
      }
  
!   /* 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);
  
    /* Now OFFSET is nonzero only if OP0 is memory
       and is therefore always measured in bytes.  */
--- 588,605 ----
        offset = 0;
      }
  
!   /* If VALUE has a floating-point or complex 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.  It can also
!      occur for unaligned float or complex fields.  */
    orig_value = value;
!   if (GET_MODE (value) != VOIDmode
!       && GET_MODE_CLASS (GET_MODE (value)) != MODE_INT
        && GET_MODE_CLASS (GET_MODE (value)) != MODE_PARTIAL_INT)
!     {
!       value = gen_reg_rtx (int_mode_for_mode (GET_MODE (value)));
!       emit_move_insn (gen_lowpart (GET_MODE (orig_value), value), orig_value);
!     }
  
    /* Now OFFSET is nonzero only if OP0 is memory
       and is therefore always measured in bytes.  */


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