On Wed, Mar 14, 2018 at 01:54:39PM +0100, Martin Liška wrote:
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -3651,13 +3651,24 @@ expand_builtin_memory_copy_args (tree dest, tree src, tree len,
src_mem = get_memory_rtx (src, len);
set_mem_align (src_mem, src_align);
+ bool is_move_done;
+
/* Copy word part most expediently. */
This comment supposedly belongs right above the emit_block_move_hints call.
+ bool bail_out_libcall = endp == 1
+ && targetm.libc_func_speed ((int)BUILT_IN_MEMPCPY) == FAST_SPEED;
Formatting. && belongs below endp. So either:
bool bail_out_libcall
= (endp == 1
&& ...);
or
bool bail_out_libcall = false;
if (endp == 1
&& ...)
bail_out_libcall = true;
?
The variable is not named very well, shouldn't it be avoid_libcall or
something similar? Perhaps the variable should have a comment describing
what it is. Do you need separate argument for that bool and
is_move_done, rather than just the flag being that some pointer to bool is
non-NULL?
dest_addr = emit_block_move_hints (dest_mem, src_mem, len_rtx,
CALL_EXPR_TAILCALL (exp)
&& (endp == 0 || target == const0_rtx)
? BLOCK_OP_TAILCALL : BLOCK_OP_NORMAL,
expected_align, expected_size,
- min_size, max_size, probable_max_size);
+ min_size, max_size, probable_max_size,
+ bail_out_libcall, &is_move_done);
+
+ /* Bail out when a mempcpy call would be expanded as libcall and when
+ we have a target that provides a fast implementation
+ of mempcpy routine. */
+ if (!is_move_done)
+ return NULL_RTX;
if (dest_addr == 0)
{
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -2733,6 +2733,23 @@ ix86_using_red_zone (void)
&& (!cfun->machine->has_local_indirect_jump
|| cfun->machine->indirect_branch_type == indirect_branch_keep));
}
+
Missing function comment here. For target hooks, usually there is a copy of
the target.def comment, perhaps with further details on why it is overridden
and what it does.
--- a/gcc/coretypes.h
+++ b/gcc/coretypes.h
@@ -384,6 +384,13 @@ enum excess_precision_type
EXCESS_PRECISION_TYPE_FAST
};
Missing comment describing what it is, plus it the enumerators are too
generic, if it is libc_speed enum, perhaps LIBC_FAST_SPEED etc.?
--- a/gcc/targhooks.c
+++ b/gcc/targhooks.c
@@ -1642,6 +1642,12 @@ no_c99_libc_has_function (enum function_class fn_class ATTRIBUTE_UNUSED)
return false;
}
Again, missing function comment.
+enum libc_speed
+default_libc_func_speed (int)
+{
+ return UNKNOWN_SPEED;
+}
+
--- a/gcc/testsuite/gcc.dg/string-opt-1.c
+++ b/gcc/testsuite/gcc.dg/string-opt-1.c
@@ -48,5 +48,5 @@ main (void)
return 0;
}
-/* { dg-final { scan-assembler-not "\<mempcpy\>" } } */
-/* { dg-final { scan-assembler "memcpy" } } */
+/* { dg-final { scan-assembler-not "\<mempcpy\>" { target { i?86-*-* x86_64-*-* } } } } */
+/* { dg-final { scan-assembler "memcpy" { target { ! { i?86-*-* x86_64-*-* } } } } } */
First of all, I don't really understand this, I'd expect both the
two old dg-final lines to be used as is for non-x86 targets and another two
for x86_64, and more importantly, the target hook is only for glibc, not for
musl/bionic etc., nor for non-linux, so you probably need some effective
target for it for whether it is glibc rather than musl/bionic, and use
...-*-linux* and ...-*-gnu* etc. rather than ...-*-*. Or tweak the dg-fianl
patterns so that it succeeds on both variants of the target hook, but then
does the test test anything at all?