Re: [PATCH] Fix an ICE caused by assuming all builtin stringops have 3 arguments

[Richard Guenther <richard dot guenther at gmail dot com>]

On Fri, Sep 18, 2009 at 12:32 AM, Neil Vachharajani <nvachhar@google.com> wrote:
> Fixed a bug where the compiler was assuming that all builtin stringops
> have 3 arguments (where the third argument is the length of the
> stringop). ?This is not true for bzero and it was causing a compiler
> segfault. ?This bug only manifests in FDO compilation (in value
> profiling optimizations).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux. ?Added test
> case for this problem and verified it fails before the patch and
> passes after. ?Ok for trunk?

Ok.

Thanks,
Richard.

> 2009-09-17 ?Neil Vachharajani ?<nvachhar@gmail.com>
>
> ? ? ? ?* value-prof.c (interesting_stringop_to_profile_p): Added output
> ? ? ? ?argument to indicate which parameter is the size parameter.
> ? ? ? ?* value-prof.c (gimple_stringop_fixed_value): Use
> ? ? ? ?INTERESTING_STRINGOP_TO_PROFILE_P to find size argument.
> ? ? ? ?* value-prof.c (gimple_stringops_transform): Update call sites to
> ? ? ? ?INTERESTING_STRINGOP_TO_PROFILE_P to reflect parameter change.
> ? ? ? ?* /gcc.dg/tree-prof/val-prof-7.c: Added test case.
>
> --- gcc/value-prof.c.orig
> +++ gcc/value-prof.c
> @@ -1239,9 +1239,12 @@
> ? return true;
> ?}
>
> -/* Return true if the stringop CALL with FNDECL shall be profiled. ?*/
> +/* Return true if the stringop CALL with FNDECL shall be profiled. ?If
> + ? SIZE_ARG is not null, it will be the argument index for the size of
> + ? the string operation.
> +*/
> ?static bool
> -interesting_stringop_to_profile_p (tree fndecl, gimple call)
> +interesting_stringop_to_profile_p (tree fndecl, gimple call, int *size_arg)
> ?{
> ? enum built_in_function fcode = DECL_FUNCTION_CODE (fndecl);
>
> @@ -1253,12 +1256,18 @@
> ? ? {
> ? ? ?case BUILT_IN_MEMCPY:
> ? ? ?case BUILT_IN_MEMPCPY:
> + ? ? ? if (size_arg)
> + ? ? ? ? *size_arg = 2;
> ? ? ? ?return validate_gimple_arglist (call, POINTER_TYPE, POINTER_TYPE,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? INTEGER_TYPE, VOID_TYPE);
> ? ? ?case BUILT_IN_MEMSET:
> + ? ? ? if (size_arg)
> + ? ? ? ? *size_arg = 2;
> ? ? ? ?return validate_gimple_arglist (call, POINTER_TYPE, INTEGER_TYPE,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?INTEGER_TYPE, VOID_TYPE);
> ? ? ?case BUILT_IN_BZERO:
> + ? ? ? if (size_arg)
> + ? ? ? ? *size_arg = 1;
> ? ? ? ?return validate_gimple_arglist (call, POINTER_TYPE, INTEGER_TYPE,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? VOID_TYPE);
> ? ? ?default:
> @@ -1276,7 +1285,7 @@
> ?*/
> ?static void
> ?gimple_stringop_fixed_value (gimple stmt, tree value, int prob,
> gcov_type count,
> - ? ? ? ? ? ? ? ? ? ? ? ? ?gcov_type all)
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ?gcov_type all)
> ?{
> ? gimple stmt1, stmt2, stmt3;
> ? tree tmp1, tmpv;
> @@ -1284,10 +1293,18 @@
> ? basic_block bb, bb2, bb3, bb4;
> ? edge e12, e13, e23, e24, e34;
> ? gimple_stmt_iterator gsi;
> - ?tree blck_size = gimple_call_arg (stmt, 2);
> - ?tree optype = TREE_TYPE (blck_size);
> + ?tree fndecl;
> + ?tree blck_size;
> + ?tree optype;
> ? int region;
> + ?int size_arg;
>
> + ?fndecl = gimple_call_fndecl (stmt);
> + ?if (!interesting_stringop_to_profile_p (fndecl, stmt, &size_arg))
> + ? ?gcc_unreachable();
> + ?blck_size = gimple_call_arg (stmt, size_arg);
> + ?optype = TREE_TYPE (blck_size);
> +
> ? bb = gimple_bb (stmt);
> ? gsi = gsi_for_stmt (stmt);
>
> @@ -1316,7 +1333,7 @@
> ? bb1end = stmt3;
>
> ? stmt1 = gimple_copy (stmt);
> - ?gimple_call_set_arg (stmt1, 2, value);
> + ?gimple_call_set_arg (stmt1, size_arg, value);
> ? gsi_insert_before (&gsi, stmt1, GSI_SAME_STMT);
> ? region = lookup_stmt_eh_region (stmt);
> ? if (region >= 0)
> @@ -1367,6 +1384,7 @@
> ? unsigned int dest_align, src_align;
> ? gcov_type prob;
> ? tree tree_val;
> + ?int size_arg;
>
> ? if (gimple_code (stmt) != GIMPLE_CALL)
> ? ? return false;
> @@ -1374,13 +1392,10 @@
> ? if (!fndecl)
> ? ? return false;
> ? fcode = DECL_FUNCTION_CODE (fndecl);
> - ?if (!interesting_stringop_to_profile_p (fndecl, stmt))
> + ?if (!interesting_stringop_to_profile_p (fndecl, stmt, &size_arg))
> ? ? return false;
>
> - ?if (fcode == BUILT_IN_BZERO)
> - ? ?blck_size = gimple_call_arg (stmt, 1);
> - ?else
> - ? ?blck_size = gimple_call_arg (stmt, 2);
> + ?blck_size = gimple_call_arg (stmt, size_arg);
> ? if (TREE_CODE (blck_size) == INTEGER_CST)
> ? ? return false;
>
> @@ -1591,6 +1606,7 @@
> ? tree blck_size;
> ? tree dest;
> ? enum built_in_function fcode;
> + ?int size_arg;
>
> ? if (gimple_code (stmt) != GIMPLE_CALL)
> ? ? return;
> @@ -1599,14 +1615,11 @@
> ? ? return;
> ? fcode = DECL_FUNCTION_CODE (fndecl);
>
> - ?if (!interesting_stringop_to_profile_p (fndecl, stmt))
> + ?if (!interesting_stringop_to_profile_p (fndecl, stmt, &size_arg))
> ? ? return;
>
> ? dest = gimple_call_arg (stmt, 0);
> - ?if (fcode == BUILT_IN_BZERO)
> - ? ?blck_size = gimple_call_arg (stmt, 1);
> - ?else
> - ? ?blck_size = gimple_call_arg (stmt, 2);
> + ?blck_size = gimple_call_arg (stmt, size_arg);
>
> ? if (TREE_CODE (blck_size) != INTEGER_CST)
> ? ? {
>
> --- gcc/testsuite/gcc.dg/tree-prof/val-prof-7.c.orig
> +++ gcc/testsuite/gcc.dg/tree-prof/val-prof-7.c
> @@ -0,0 +1,26 @@
> +/* { dg-options "-O2 -fdump-tree-tree_profile -mtune=core2" } */
> +/* { dg-skip-if "" { ! { i?86-*-* x86_64-*-* } } { "*" } { "" } } */
> +
> +#include <strings.h>
> +
> +int foo(int len)
> +{
> + ?char array[1000];
> + ?bzero(array, len);
> + ?return 0;
> +}
> +
> +int main() {
> + ?int i;
> + ?for (i = 0; i < 1000; i++)
> + ? ?{
> + ? ? ?if (i > 990)
> + ? ? ? foo(16);
> + ? ? ?else
> + ? ? ? foo(8);
> + ? ?}
> + ?return 0;
> +}
> +
> +/* { dg-final-use { scan-tree-dump "Single value 8 stringop
> transformation on bzero" "tree_profile"} } */
> +/* { dg-final-use { cleanup-tree-dump "tree_profile" } } */
>
>
> --
> Neil Vachharajani
> Google
> 650-214-1804
>