On Fri, Apr 01, 2011 at 12:52:04PM +0200, Eric Botcazou wrote:
> > --- gcc/expr.c.jj 2011-03-23 17:15:55.000000000 +0100
> > +++ gcc/expr.c 2011-03-30 11:38:15.000000000 +0200
> > @@ -4278,16 +4278,47 @@ expand_assignment (tree to, tree from, b
> > /* Handle expand_expr of a complex value returning a CONCAT. */
> > else if (GET_CODE (to_rtx) == CONCAT)
> > {
> > - if (COMPLEX_MODE_P (TYPE_MODE (TREE_TYPE (from))))
> > + unsigned short mode_bitsize = GET_MODE_BITSIZE (GET_MODE (to_rtx));
> > + if (COMPLEX_MODE_P (TYPE_MODE (TREE_TYPE (from)))
> > + && bitpos == 0
> > + && bitsize == mode_bitsize)
> > + result = store_expr (from, to_rtx, false, nontemporal);
> > + else if (bitsize == mode_bitsize / 2
> > + && (bitpos == 0 || bitpos == GET_MODE_BITSIZE (mode1)))
> > + result = store_expr (from, XEXP (to_rtx, bitpos != 0), false,
> > + nontemporal);
>
> Why GET_MODE_BITSIZE (mode1) and not mode_bitsize / 2 here?
It should be mode_bitsize / 2 yeah, GET_MODE_BITSIZE (mode1) just came
from the original code. Will change.
> > {
> > - gcc_assert (bitpos == 0 || bitpos == GET_MODE_BITSIZE (mode1));
> > - result = store_expr (from, XEXP (to_rtx, bitpos != 0), false,
> > - nontemporal);
> > + rtx temp = assign_stack_temp (GET_MODE (to_rtx),
> > + GET_MODE_SIZE (GET_MODE (to_rtx)),
> > + 0);
> > + write_complex_part (temp, XEXP (to_rtx, 0), false);
> > + write_complex_part (temp, XEXP (to_rtx, 1), true);
> > + result = store_field (temp, bitsize, bitpos, mode1, from,
> > + TREE_TYPE (tem), get_alias_set (to),
> > + nontemporal);
> > + emit_move_insn (XEXP (to_rtx, 0), read_complex_part (temp,
> > false));
> > + emit_move_insn (XEXP (to_rtx, 1), read_complex_part (temp, true));
> > }
>
> Can't you add result = NULL at the end of the block?
result is set there from store_field, and result is only used in
if (result)
preserve_temp_slots (result);
afterwards. store_field might want to preserve_temp_slots perhaps, so
clearing result afterwards might be wrong.
> > : (! SLOW_UNALIGNED_ACCESS (fieldmode, MEM_ALIGN (op0))
> > :
> > || (offset * BITS_PER_UNIT % bitsize == 0
> >
> > - && MEM_ALIGN (op0) % GET_MODE_BITSIZE (fieldmode) == 0))))
> > + && MEM_ALIGN (op0) % GET_MODE_BITSIZE (fieldmode) == 0)))
> > + && (MEM_P (op0)
> > + || GET_MODE (op0) == fieldmode
> > + || validate_subreg (fieldmode, GET_MODE (op0), op0, byte_offset)))
>
> This partially duplicates the existing test. Can't the new code be
> retrofitted
> into the existing test?
You mean just with the MEM_P test? So something like:
@@ -418,8 +418,11 @@ store_bit_field_1 (rtx str_rtx, unsigned
&& bitsize == GET_MODE_BITSIZE (fieldmode)
&& (!MEM_P (op0)
? ((GET_MODE_SIZE (fieldmode) >= UNITS_PER_WORD
- || GET_MODE_SIZE (GET_MODE (op0)) == GET_MODE_SIZE (fieldmode))
- && byte_offset % GET_MODE_SIZE (fieldmode) == 0)
+ || GET_MODE_SIZE (GET_MODE (op0)) == GET_MODE_SIZE (fieldmode))
+ && byte_offset % GET_MODE_SIZE (fieldmode) == 0
+ && (GET_MODE (op0) == fieldmode
+ || validate_subreg (fieldmode, GET_MODE (op0), op0,
+ byte_offset)))
: (! SLOW_UNALIGNED_ACCESS (fieldmode, MEM_ALIGN (op0))
|| (offset * BITS_PER_UNIT % bitsize == 0
&& MEM_ALIGN (op0) % GET_MODE_BITSIZE (fieldmode) == 0))))
instead?
> > /* OFFSET is in UNITs, and UNIT is in bits.
> > - store_fixed_bit_field wants offset in bytes. */
> > - store_fixed_bit_field (word, offset * unit / BITS_PER_UNIT,
> > thissize, - thispos, part);
> > + store_fixed_bit_field wants offset in bytes. If WORD is const0_rtx,
> > + it is jut an out of bounds access. Ignore it. */
> > + if (word != const0_rtx)
> > + store_fixed_bit_field (word, offset * unit / BITS_PER_UNIT, thissize,
> > + thispos, part);
> > bitsdone += thissize;
>
> "it is just an out-of-bounds access."
Ok, will change, thanks.
Jakub
