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: Tidy store_bit_field_1 & co.


> 	* expmed.c (lowpart_bit_field_p): New function.
> 	(store_bit_field_1): Remove unit, offset, bitpos and byte_offset
> 	from the outermost scope.  Express conditions in terms of bitnum
> 	rather than offset, bitpos and byte_offset.  Split the plain move
> 	cases into two, one for memory accesses and one for register accesses.
> 	Allow simplify_gen_subreg to fail rather than calling validate_subreg.
> 	Move the handling of multiword OP0s after the code that coerces VALUE
> 	to an integer mode.  Use simplify_gen_subreg for this case and assert
> 	that it succeeds.  If the field still spans several words, pass it
> 	directly to store_split_bit_field.  Assume after that point that
> 	both sources and register targets fit within a word.  Replace
> 	x-prefixed variables with non-prefixed forms.  Compute the bitpos
> 	for insv register operands directly in the chosen unit size, rather
> 	than going through an intermediate BITS_PER_WORD unit size.
> 	Update the call to store_fixed_bit_field.
> 	(store_fixed_bit_field): Replace the bitpos and offset parameters
> 	with a single bitnum parameter, of the same form as store_bit_field.
> 	Assume that OP0 contains the full field.  Simplify the memory offset
> 	calculation.  Assert that the processed OP0 has an integral mode.
> 	(store_split_bit_field): Update the call to store_fixed_bit_field.

This looks good to me, modulo:

>    /* If the target is a register, overwriting the entire object, or storing
> -     a full-word or multi-word field can be done with just a SUBREG. +    
> a full-word or multi-word field can be done with just a SUBREG.  */ +  if
> (!MEM_P (op0)
> +      && bitnum % BITS_PER_WORD == 0
> +      && bitsize == GET_MODE_BITSIZE (fieldmode)
> +      && (bitsize == GET_MODE_BITSIZE (GET_MODE (op0))
> +	  || bitsize % BITS_PER_WORD == 0))
> +    {
> +      /* Use the subreg machinery either to narrow OP0 to the required
> +	 words or to cope with mode punning between equal-sized modes.  */
> +      rtx sub = simplify_gen_subreg (fieldmode, op0, GET_MODE (op0),
> +				     bitnum / BITS_PER_UNIT);
> +      if (sub)
> +	{
> +	  emit_move_insn (sub, value);
> +	  return true;
> +	}
> +    }

Are you sure that you don't need to keep the bitnum == 0 condition in the 
first arm that was present in the previous patch?  And, on second thoughts, 
the first formulation was more in keeping with the comment just above (sorry 
about that).  So I'd reinstate it and swap the arms:

  /* If the target is a register, overwriting the entire object, or storing
     a full-word or multi-word field can be done with just a SUBREG.  */
  if (!MEM_P (op0)
      && bitsize == GET_MODE_BITSIZE (fieldmode)
      && ((bitsize == GET_MODE_BITSIZE (GET_MODE (op0)) && bitnum == 0)
          || (bitsize % BITS_PER_WORD == 0 && bitnum % BITS_PER_WORD == 0)))
    {
      /* Use the subreg machinery either to narrow OP0 to the required
        words or to cope with mode punning between equal-sized modes.  */
      rtx sub = simplify_gen_subreg (fieldmode, op0, GET_MODE (op0),
                                    bitnum / BITS_PER_UNIT);
      if (sub)
       {
         emit_move_insn (sub, value);
         return true;
       }
    }

In any case, no need to retest, I'd apply it and wait for the apocalypse. :-)

-- 
Eric Botcazou


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