PATCH: middle-end/58981: movmem/setmem use mode wider than Pmode for size

[H.J. Lu]

Y
On Mon, Nov 4, 2013 at 3:11 AM, Richard Sandiford
<rsandifo@linux.vnet.ibm.com> wrote:
> "H.J. Lu" <hongjiu.lu@intel.com> writes:
>> emit_block_move_via_movmem and set_storage_via_setmem have
>>
>>   for (mode = GET_CLASS_NARROWEST_MODE (MODE_INT); mode != VOIDmode;
>>        mode = GET_MODE_WIDER_MODE (mode))
>>     {
>>       enum insn_code code = direct_optab_handler (movmem_optab, mode);
>>
>>       if (code != CODE_FOR_nothing
>>           /* We don't need MODE to be narrower than BITS_PER_HOST_WIDE_INT
>>              here because if SIZE is less than the mode mask, as it is
>>              returned by the macro, it will definitely be less than the
>>              actual mode mask.  */
>>           && ((CONST_INT_P (size)
>>                && ((unsigned HOST_WIDE_INT) INTVAL (size)
>>                    <= (GET_MODE_MASK (mode) >> 1)))
>>               || GET_MODE_BITSIZE (mode) >= BITS_PER_WORD))
>>         {
>>
>> Backend may assume mode of size in movmem and setmem expanders is no
>> widder than Pmode since size is within the Pmode address space.  X86
>> backend expand_set_or_movmem_prologue_epilogue_by_misaligned has
>>
>>       rtx saveddest = *destptr;
>>
>>       gcc_assert (desired_align <= size);
>>       /* Align destptr up, place it to new register.  */
>>       *destptr = expand_simple_binop (GET_MODE (*destptr), PLUS, *destptr,
>>                                       GEN_INT (prolog_size),
>>                                       NULL_RTX, 1, OPTAB_DIRECT);
>>       *destptr = expand_simple_binop (GET_MODE (*destptr), AND, *destptr,
>>                                       GEN_INT (-desired_align),
>>                                       *destptr, 1, OPTAB_DIRECT);
>>       /* See how many bytes we skipped.  */
>>       saveddest = expand_simple_binop (GET_MODE (*destptr), MINUS, saveddest,
>>                                        *destptr,
>>                                        saveddest, 1, OPTAB_DIRECT);
>>       /* Adjust srcptr and count.  */
>>       if (!issetmem)
>>         *srcptr = expand_simple_binop (GET_MODE (*srcptr), MINUS, *srcptr, saveddest,
>>                                         *srcptr, 1, OPTAB_DIRECT);
>>       *count = expand_simple_binop (GET_MODE (*count), PLUS, *count,
>>                                     saveddest, *count, 1, OPTAB_DIRECT);
>>
>> saveddest is a negative number in Pmode and *count is in word_mode.  For
>> x32, when Pmode is SImode and word_mode is DImode, saveddest + *count
>> leads to overflow.  We could fix it by using mode of saveddest to compute
>> saveddest + *count.  But it leads to extra conversions and other backends
>> may run into the same problem.  A better fix is to limit mode of size in
>> movmem and setmem expanders to Pmode.  It generates better and correct
>> memcpy and memset for x32.
>>
>> There is also a typo in comments.  It should be BITS_PER_WORD, not
>> BITS_PER_HOST_WIDE_INT.
>
> I don't think it's a typo.  It's explaining why we don't have to worry about:
>
>     GET_MODE_BITSIZE (mode) > BITS_PER_HOST_WIDE_INT
>
> in the CONST_INT_P test (because in that case the GET_MODE_MASK macro
> will be an all-1 HOST_WIDE_INT, even though that's narrower than the
> real mask).

Thanks for explanation.

> I don't think the current comment covers the BITS_PER_WORD test at all.
> AIUI it's there because the pattern is defined as taking a length of
> at most word_mode, so we should stop once we reach it.

I see.

> FWIW, I agree Pmode makes more sense on face value.  But shouldn't
> we replace the BITS_PER_WORD test instead of adding to it?  Having both
> would only make a difference for Pmode > word_mode targets, which might
> be able to handle full Pmode lengths.

Do we ever have a target with Pmode is wider than
word_mode? If not, we can check Pmode instead.

> Either way, the md.texi documentation should be updated too.
>
> Thanks,
> Richard
>



-- 
H.J.