On Mon, 16 Jul 2018, Jakub Jelinek wrote:
> Hi!
>
> builtin_memcpy_read_str is a function meant to be called just as a callback
> and verifies that we don't cross a '\0' boundary in the string. For
> inline_string_cmp, we've checked that the length returned from c_getstr
> is fine, so we can cross as many embedded NULs as there are within the
> TREE_STRING_LENGTH.
>
> The rest of the patch is just a temporary to avoid using as_a twice in
> each loop iteration, and lots of formatting fixes, mostly to avoid trailing
> whitespace.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Richard.
> 2018-07-16 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/86526
> * builtins.c (expand_builtin_memcmp): Formatting fixes.
> (inline_expand_builtin_string_cmp): Likewise.
> (inline_string_cmp): Likewise. Use c_readstr instead of
> builtin_memcpy_read_str. Add unit_mode temporary.
>
> * gcc.c-torture/compile/pr86526.c: New test.
>
> --- gcc/builtins.c.jj 2018-07-16 09:42:25.743985945 +0200
> +++ gcc/builtins.c 2018-07-16 11:47:46.535014889 +0200
> @@ -4454,19 +4454,19 @@ expand_builtin_memcmp (tree exp, rtx tar
> no_overflow = check_access (exp, /*dst=*/NULL_TREE, /*src=*/NULL_TREE,
> len, /*maxread=*/NULL_TREE, size,
> /*objsize=*/NULL_TREE);
> - if (no_overflow)
> + if (no_overflow)
> {
> size = compute_objsize (arg2, 0);
> no_overflow = check_access (exp, /*dst=*/NULL_TREE, /*src=*/NULL_TREE,
> len, /*maxread=*/NULL_TREE, size,
> /*objsize=*/NULL_TREE);
> - }
> + }
>
> - /* Due to the performance benefit, always inline the calls first
> + /* Due to the performance benefit, always inline the calls first
> when result_eq is false. */
> rtx result = NULL_RTX;
> -
> - if (!result_eq && fcode != BUILT_IN_BCMP && no_overflow)
> +
> + if (!result_eq && fcode != BUILT_IN_BCMP && no_overflow)
> {
> result = inline_expand_builtin_string_cmp (exp, target, true);
> if (result)
> @@ -6748,7 +6748,7 @@ expand_builtin_goacc_parlevel_id_size (t
> return target;
> }
>
> -/* Expand a string compare operation using a sequence of char comparison
> +/* Expand a string compare operation using a sequence of char comparison
> to get rid of the calling overhead, with result going to TARGET if
> that's convenient.
>
> @@ -6757,7 +6757,7 @@ expand_builtin_goacc_parlevel_id_size (t
> LENGTH is the number of chars to compare;
> CONST_STR_N indicates which source string is the constant string;
> IS_MEMCMP indicates whether it's a memcmp or strcmp.
> -
> +
> to: (assume const_str_n is 2, i.e., arg2 is a constant string)
>
> target = var_str[0] - const_str[0];
> @@ -6772,41 +6772,38 @@ expand_builtin_goacc_parlevel_id_size (t
> */
>
> static rtx
> -inline_string_cmp (rtx target, tree var_str, const char* const_str,
> +inline_string_cmp (rtx target, tree var_str, const char *const_str,
> unsigned HOST_WIDE_INT length,
> int const_str_n, machine_mode mode,
> - bool is_memcmp)
> + bool is_memcmp)
> {
> HOST_WIDE_INT offset = 0;
> - rtx var_rtx_array
> + rtx var_rtx_array
> = get_memory_rtx (var_str, build_int_cst (unsigned_type_node,length));
> rtx var_rtx = NULL_RTX;
> - rtx const_rtx = NULL_RTX;
> - rtx result = target ? target : gen_reg_rtx (mode);
> - rtx_code_label *ne_label = gen_label_rtx ();
> + rtx const_rtx = NULL_RTX;
> + rtx result = target ? target : gen_reg_rtx (mode);
> + rtx_code_label *ne_label = gen_label_rtx ();
> tree unit_type_node = is_memcmp ? unsigned_char_type_node : char_type_node;
> + scalar_int_mode unit_mode
> + = as_a <scalar_int_mode> TYPE_MODE (unit_type_node);
>
> start_sequence ();
>
> for (unsigned HOST_WIDE_INT i = 0; i < length; i++)
> {
> - var_rtx
> + var_rtx
> = adjust_address (var_rtx_array, TYPE_MODE (unit_type_node), offset);
> - const_rtx
> - = builtin_memcpy_read_str (CONST_CAST (char *, const_str),
> - offset,
> - as_a <scalar_int_mode>
> - TYPE_MODE (unit_type_node));
> + const_rtx = c_readstr (const_str + offset, unit_mode);
> rtx op0 = (const_str_n == 1) ? const_rtx : var_rtx;
> rtx op1 = (const_str_n == 1) ? var_rtx : const_rtx;
> -
> - result = expand_simple_binop (mode, MINUS, op0, op1,
> - result, is_memcmp ? 1 : 0, OPTAB_WIDEN);
> - if (i < length - 1)
> - emit_cmp_and_jump_insns (result, CONST0_RTX (mode), NE, NULL_RTX,
> - mode, true, ne_label);
> - offset
> - += GET_MODE_SIZE (as_a <scalar_int_mode> TYPE_MODE (unit_type_node));
> +
> + result = expand_simple_binop (mode, MINUS, op0, op1,
> + result, is_memcmp ? 1 : 0, OPTAB_WIDEN);
> + if (i < length - 1)
> + emit_cmp_and_jump_insns (result, CONST0_RTX (mode), NE, NULL_RTX,
> + mode, true, ne_label);
> + offset += GET_MODE_SIZE (unit_mode);
> }
>
> emit_label (ne_label);
> @@ -6817,7 +6814,7 @@ inline_string_cmp (rtx target, tree var_
> return result;
> }
>
> -/* Inline expansion a call to str(n)cmp, with result going to
> +/* Inline expansion a call to str(n)cmp, with result going to
> TARGET if that's convenient.
> If the call is not been inlined, return NULL_RTX. */
> static rtx
> @@ -6829,7 +6826,7 @@ inline_expand_builtin_string_cmp (tree e
> bool is_ncmp = (fcode == BUILT_IN_STRNCMP || fcode == BUILT_IN_MEMCMP);
>
> gcc_checking_assert (fcode == BUILT_IN_STRCMP
> - || fcode == BUILT_IN_STRNCMP
> + || fcode == BUILT_IN_STRNCMP
> || fcode == BUILT_IN_MEMCMP);
>
> tree arg1 = CALL_EXPR_ARG (exp, 0);
> @@ -6842,7 +6839,7 @@ inline_expand_builtin_string_cmp (tree e
>
> const char *src_str1 = c_getstr (arg1, &len1);
> const char *src_str2 = c_getstr (arg2, &len2);
> -
> +
> /* If neither strings is constant string, the call is not qualify. */
> if (!src_str1 && !src_str2)
> return NULL_RTX;
> @@ -6867,16 +6864,16 @@ inline_expand_builtin_string_cmp (tree e
> if (is_ncmp && (len3 = tree_to_uhwi (len3_tree)) < length)
> length = len3;
>
> - /* If the length of the comparision is larger than the threshold,
> + /* If the length of the comparision is larger than the threshold,
> do nothing. */
> - if (length > (unsigned HOST_WIDE_INT)
> + if (length > (unsigned HOST_WIDE_INT)
> PARAM_VALUE (BUILTIN_STRING_CMP_INLINE_LENGTH))
> return NULL_RTX;
>
> machine_mode mode = TYPE_MODE (TREE_TYPE (exp));
>
> /* Now, start inline expansion the call. */
> - return inline_string_cmp (target, (const_str_n == 1) ? arg2 : arg1,
> + return inline_string_cmp (target, (const_str_n == 1) ? arg2 : arg1,
> (const_str_n == 1) ? src_str1 : src_str2, length,
> const_str_n, mode, is_memcmp);
> }
> @@ -7282,7 +7279,7 @@ expand_builtin (tree exp, rtx target, rt
> return target;
> break;
>
> - /* Expand it as BUILT_IN_MEMCMP_EQ first. If not successful, change it
> + /* Expand it as BUILT_IN_MEMCMP_EQ first. If not successful, change it
> back to a BUILT_IN_STRCMP. Remember to delete the 3rd paramater
> when changing it to a strcmp call. */
> case BUILT_IN_STRCMP_EQ:
> @@ -7291,7 +7288,7 @@ expand_builtin (tree exp, rtx target, rt
> return target;
>
> /* Change this call back to a BUILT_IN_STRCMP. */
> - TREE_OPERAND (exp, 1)
> + TREE_OPERAND (exp, 1)
> = build_fold_addr_expr (builtin_decl_explicit (BUILT_IN_STRCMP));
>
> /* Delete the last parameter. */
> @@ -7317,7 +7314,7 @@ expand_builtin (tree exp, rtx target, rt
> return target;
>
> /* Change it back to a BUILT_IN_STRNCMP. */
> - TREE_OPERAND (exp, 1)
> + TREE_OPERAND (exp, 1)
> = build_fold_addr_expr (builtin_decl_explicit (BUILT_IN_STRNCMP));
> /* FALLTHROUGH */
>
> --- gcc/testsuite/gcc.c-torture/compile/pr86526.c.jj 2018-07-16
> 11:50:30.756223270 +0200
> +++ gcc/testsuite/gcc.c-torture/compile/pr86526.c 2018-07-16
> 11:50:16.205203998 +0200
> @@ -0,0 +1,8 @@
> +/* PR tree-optimization/86526 */
> +
> +void
> +foo (char *x)
> +{
> + if (__builtin_memcmp (x, "\0a", 3))
> + __builtin_abort ();
> +}
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
