On 9/27/19 12:23 PM, Aaron Sawdey wrote:
> This is the third piece of my effort to improve inline expansion of memmove.
> The
> first two parts I posted back in June fixed the names of the optab entries
> involved so that optab cpymem is used for memcpy() and optab movmem is used
> for
> memmove(). This piece adds support for actually attempting to invoke the
> movmem
> optab to do inline expansion of __builtin_memmove().
>
> Because what needs to be done for memmove() is very similar to memcpy(), I
> have
> just added a bool parm "might_overlap" to several of the functions involved so
> the same functions can handle both. The name might_overlap comes from the fact
> that if we still have a memmove() call at expand, this means
> gimple_fold_builtin_memory_op() was not able to prove that the source and
> destination do not overlap.
>
> There are a few places where might_overlap gets used to keep us from trying to
> use the by-pieces infrastructure or generate a copy loop, as neither of those
> things will work correctly if source and destination overlap.
>
> I've restructured things slightly in emit_block_move_hints() so that we can
> try the pattern first if we already know that by-pieces won't work. This way
> we can bail out immediately in the might_overlap case.
>
> Bootstrap/regtest passed on ppc64le, in progress on x86_64. If everything
> passes,
> is this ok for trunk?
>
>
> 2019-09-27 Aaron Sawdey <acsaw...@linux.ibm.com>
>
> * builtins.c (expand_builtin_memory_copy_args): Add might_overlap parm.
> (expand_builtin_memcpy): Use might_overlap parm.
> (expand_builtin_mempcpy_args): Use might_overlap parm.
> (expand_builtin_memmove): Call expand_builtin_memory_copy_args.
> (expand_builtin_memory_copy_args): Add might_overlap parm.
> * expr.c (emit_block_move_via_cpymem): Rename to
> emit_block_move_via_pattern, add might_overlap parm, use cpymem
> or movmem optab as appropriate.
> (emit_block_move_hints): Add might_overlap parm, do the right
> thing for might_overlap==true.
> * expr.h (emit_block_move_hints): Update prototype.
>
>
>
>
> Index: gcc/builtins.c
> ===================================================================
> --- gcc/builtins.c (revision 276131)
> +++ gcc/builtins.c (working copy)
> @@ -3894,10 +3897,11 @@
> &probable_max_size);
> src_str = c_getstr (src);
>
> - /* If SRC is a string constant and block move would be done
> - by pieces, we can avoid loading the string from memory
> - and only stored the computed constants. */
> - if (src_str
> + /* If SRC is a string constant and block move would be done by
> + pieces, we can avoid loading the string from memory and only
> + stored the computed constants. I'm not sure if the by pieces
> + method works if src/dest are overlapping, so avoid that case. */
> + if (src_str && !might_overlap
I don't think you need the check here. c_getstr, when it returns
somethign useful is going to be returning a string constant. Think read
only literals here. I'm pretty sure overlap isn't going to be possible.
> && CONST_INT_P (len_rtx)
> && (unsigned HOST_WIDE_INT) INTVAL (len_rtx) <= strlen (src_str) + 1
> && can_store_by_pieces (INTVAL (len_rtx), builtin_memcpy_read_str,
> @@ -3922,7 +3926,7 @@
> && (retmode == RETURN_BEGIN || target == const0_rtx))
> method = BLOCK_OP_TAILCALL;
> bool use_mempcpy_call = (targetm.libc_has_fast_function (BUILT_IN_MEMPCPY)
> - && retmode == RETURN_END
> + && retmode == RETURN_END && !might_overlap
Put the && !might_overlap on its own line for readability.
> Index: gcc/expr.c
> ===================================================================
> --- gcc/expr.c (revision 276131)
> +++ gcc/expr.c (working copy)
> @@ -1622,13 +1624,29 @@
> set_mem_size (y, const_size);
> }
>
> - if (CONST_INT_P (size) && can_move_by_pieces (INTVAL (size), align))
> + bool pieces_ok = can_move_by_pieces (INTVAL (size), align);
> + bool pattern_ok = false;
> +
> + if (!CONST_INT_P (size) || !pieces_ok || might_overlap)
> + {
> + pattern_ok =
> + emit_block_move_via_pattern (x, y, size, align,
> + expected_align, expected_size,
> + min_size, max_size, probable_max_size,
> + might_overlap);
> + if (!pattern_ok && might_overlap)
> + {
> + /* Do not try any of the other methods below as they are not safe
> + for overlapping moves. */
> + *is_move_done = false;
> + return retval;
> + }
> + }
> +
> + if (pattern_ok) ;
Drop the semi-colon down to its own line like
if (whatever)
;
else if (...)
something
else if (...)
something else
With the changes noted above, this is fine for th trunk.
jeff
