[PATCH] Use movmem optab to attempt inline expansion of __builtin_memmove()

[Jeff Law]

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 <acsawdey@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