On Wed, Apr 20, 2016 at 10:45 AM, Richard Biener <richard.guent...@gmail.com> wrote: > On Tue, Apr 19, 2016 at 6:32 PM, Wilco Dijkstra <wilco.dijks...@arm.com> > wrote: >> Richard Biener wrote: >>> >>> This folding should be added to gimple-fold.c:gimple_fold_builtin instead, >>> the builtins.c foldings are purerly for folding to constants nowadays. >> >> So is this better? It's a lot more verbose for something so simple... >> Unfortunately match.pd doesn't support this kind of thing either. > > Better - comments below. Jakub objections to the usefulness of the transform > remain - we do have the strlen pass that uses some global knowledge to decide > on profitability. One could argue that for -Os doing the reverse transform is > profitable? > > Yes, match.pd doesn't support this (yet). It may be possible to teach it > simple > cases like this - I plan to revisit this at some point. The issue is one of > side-effects which are tricky to handle if you consider patterns that do not > only match a single call (as this one would). So a baby-step towards getting > this supported in match.pd is to handle matching toplevel calls specially.
Ok, just gave it a stab in a different way - see attached. This makes
(simplify
(BUILT_IN_STRCHR @0 integer_zerop)
(pointer_plus @0 (BUILT_IN_STRLEN:size_type_node @0)))
work. Note how the SSA following predicate needs to be aware of
side-effects to guard against bogus applies of complicated patterns
(none of those exist yet).
Richard.
> Richard.
>
>> Wilco
>>
>>
>> ChangeLog:
>> 2016-04-19 Wilco Dijkstra <wdijk...@arm.com>
>>
>> gcc/
>> * gcc/gimple-fold.c (gimple_fold_builtin_strchr):
>> New function to optimize strchr (s, 0) to strlen.
>> (gimple_fold_builtin): Add BUILT_IN_STRCHR case.
>>
>> testsuite/
>> * gcc/testsuite/gcc.dg/strlenopt-20.c: Update test.
>> * gcc/testsuite/gcc.dg/strlenopt-21.c: Likewise.
>> * gcc/testsuite/gcc.dg/strlenopt-22.c: Likewise.
>> * gcc/testsuite/gcc.dg/strlenopt-26.c: Likewise.
>> * gcc/testsuite/gcc.dg/strlenopt-5.c: Likewise.
>> * gcc/testsuite/gcc.dg/strlenopt-7.c: Likewise.
>> * gcc/testsuite/gcc.dg/strlenopt-9.c: Likewise.
>>
>> --
>>
>> diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
>> index
>> eb130d048469f0b8196e565fed9a40de74b098bd..11dcf69fc919f066362f4f713db392d14b39764e
>> 100644
>> --- a/gcc/gimple-fold.c
>> +++ b/gcc/gimple-fold.c
>> @@ -1380,6 +1380,59 @@ gimple_fold_builtin_strncpy (gimple_stmt_iterator
>> *gsi,
>> return true;
>> }
>>
>> +/* Simplify strchr (str, 0) into str + strlen (str).
>> + In general strlen is significantly faster than strchr
>> + due to being a simpler operation. */
>> +static bool
>> +gimple_fold_builtin_strchr (gimple_stmt_iterator *gsi)
>> +{
>> + gimple *stmt = gsi_stmt (*gsi);
>> + tree str = gimple_call_arg (stmt, 0);
>> + tree c = gimple_call_arg (stmt, 1);
>> + location_t loc = gimple_location (stmt);
>> +
>> + if (optimize_function_for_size_p (cfun))
>> + return false;
>
> Hmm, I think we'd want a optimize_stmt_for_size_p (stmt) which
> does the right thing for the case we have a CFG (look at the BB)
> or when not (look at the function).
>
>> + if (!integer_zerop (c) || !gimple_call_lhs (stmt))
>> + return false;
>> +
>> + tree newstr;
>> + tree strlen_fn = builtin_decl_implicit (BUILT_IN_STRLEN);
>> +
>> + if (!strlen_fn)
>> + return false;
>> +
>> + /* Create newstr = strlen (str). */
>> + gimple_seq stmts = NULL, stmts2;
>> + gimple *repl = gimple_build_call (strlen_fn, 1, str);
>> + gimple_set_location (repl, loc);
>> + if (gimple_in_ssa_p (cfun))
>> + newstr = make_ssa_name (size_type_node);
>> + else
>> + newstr = create_tmp_reg (size_type_node);
>> + gimple_call_set_lhs (repl, newstr);
>> + gimple_seq_add_stmt_without_update (&stmts, repl);
>> +
>> + /* Create (str p+ strlen (str)). */
>> + newstr = fold_build_pointer_plus_loc (loc, str, newstr);
>> + newstr = force_gimple_operand (newstr, &stmts2, true, NULL_TREE);
>
> I think you want to build a gimple_assign directly here, otherwise ...
>
>> + gimple_seq_add_seq_without_update (&stmts, stmts2);
>> +
>> + repl = gimple_build_assign (gimple_call_lhs (stmt), newstr);
>> + gimple_seq_add_stmt_without_update (&stmts, repl);
>> + gsi_replace_with_seq_vops (gsi, stmts);
>> + /* gsi now points at the assignment to the lhs, get a
>> + stmt iterator to the strlen.
>> + ??? We can't use gsi_for_stmt as that doesn't work when the
>> + CFG isn't built yet. */
>> + gimple_stmt_iterator gsi2 = *gsi;
>> + gsi_prev (&gsi2);
>> + gsi_prev (&gsi2);
>
> ... this may not reliably end up at the call stmt.
>
>> + fold_stmt (&gsi2);
>> + return true;
>> +}
>> +
>> /* Simplify a call to the strcat builtin. DST and SRC are the arguments
>> to the call.
>>
>> @@ -2821,6 +2874,8 @@ gimple_fold_builtin (gimple_stmt_iterator *gsi)
>> gimple_call_arg (stmt, 1));
>> case BUILT_IN_STRNCAT:
>> return gimple_fold_builtin_strncat (gsi);
>> + case BUILT_IN_STRCHR:
>> + return gimple_fold_builtin_strchr (gsi);
>> case BUILT_IN_FPUTS:
>> return gimple_fold_builtin_fputs (gsi, gimple_call_arg (stmt, 0),
>> gimple_call_arg (stmt, 1), false);
>> diff --git a/gcc/testsuite/gcc.dg/strlenopt-20.c
>> b/gcc/testsuite/gcc.dg/strlenopt-20.c
>> index
>> a83e845c26d88e5acdcabf142f7b319136663488..7b483eaeac1aa47278111a92148a16f00b2aaa2d
>> 100644
>> --- a/gcc/testsuite/gcc.dg/strlenopt-20.c
>> +++ b/gcc/testsuite/gcc.dg/strlenopt-20.c
>> @@ -86,9 +86,9 @@ main ()
>> return 0;
>> }
>>
>> -/* { dg-final { scan-tree-dump-times "strlen \\(" 1 "strlen" } } */
>> +/* { dg-final { scan-tree-dump-times "strlen \\(" 2 "strlen" } } */
>> /* { dg-final { scan-tree-dump-times "memcpy \\(" 4 "strlen" } } */
>> /* { dg-final { scan-tree-dump-times "strcpy \\(" 0 "strlen" } } */
>> /* { dg-final { scan-tree-dump-times "strcat \\(" 0 "strlen" } } */
>> -/* { dg-final { scan-tree-dump-times "strchr \\(" 1 "strlen" } } */
>> +/* { dg-final { scan-tree-dump-times "strchr \\(" 0 "strlen" } } */
>> /* { dg-final { scan-tree-dump-times "stpcpy \\(" 0 "strlen" } } */
>> diff --git a/gcc/testsuite/gcc.dg/strlenopt-21.c
>> b/gcc/testsuite/gcc.dg/strlenopt-21.c
>> index
>> e22fa9fca9ba14354db2cd5f602283b64bd8dcac..05b85a49dde0a7f5d269174fd4269e40be910dbd
>> 100644
>> --- a/gcc/testsuite/gcc.dg/strlenopt-21.c
>> +++ b/gcc/testsuite/gcc.dg/strlenopt-21.c
>> @@ -57,9 +57,9 @@ main ()
>> return 0;
>> }
>>
>> -/* { dg-final { scan-tree-dump-times "strlen \\(" 1 "strlen" } } */
>> +/* { dg-final { scan-tree-dump-times "strlen \\(" 2 "strlen" } } */
>> /* { dg-final { scan-tree-dump-times "memcpy \\(" 3 "strlen" } } */
>> /* { dg-final { scan-tree-dump-times "strcpy \\(" 0 "strlen" } } */
>> /* { dg-final { scan-tree-dump-times "strcat \\(" 0 "strlen" } } */
>> -/* { dg-final { scan-tree-dump-times "strchr \\(" 1 "strlen" } } */
>> +/* { dg-final { scan-tree-dump-times "strchr \\(" 0 "strlen" } } */
>> /* { dg-final { scan-tree-dump-times "stpcpy \\(" 0 "strlen" } } */
>> diff --git a/gcc/testsuite/gcc.dg/strlenopt-22.c
>> b/gcc/testsuite/gcc.dg/strlenopt-22.c
>> index
>> aa55f5ebd6a2d4803ee9a7fd60fc538d86f47124..b4ef772f0e59252f10a5419ede6837b3c8ca8265
>> 100644
>> --- a/gcc/testsuite/gcc.dg/strlenopt-22.c
>> +++ b/gcc/testsuite/gcc.dg/strlenopt-22.c
>> @@ -31,9 +31,9 @@ main ()
>> return 0;
>> }
>>
>> -/* { dg-final { scan-tree-dump-times "strlen \\(" 3 "strlen" } } */
>> +/* { dg-final { scan-tree-dump-times "strlen \\(" 4 "strlen" } } */
>> /* { dg-final { scan-tree-dump-times "memcpy \\(" 1 "strlen" } } */
>> /* { dg-final { scan-tree-dump-times "strcpy \\(" 1 "strlen" } } */
>> /* { dg-final { scan-tree-dump-times "strcat \\(" 0 "strlen" } } */
>> -/* { dg-final { scan-tree-dump-times "strchr \\(" 1 "strlen" } } */
>> +/* { dg-final { scan-tree-dump-times "strchr \\(" 0 "strlen" } } */
>> /* { dg-final { scan-tree-dump-times "stpcpy \\(" 0 "strlen" } } */
>> diff --git a/gcc/testsuite/gcc.dg/strlenopt-26.c
>> b/gcc/testsuite/gcc.dg/strlenopt-26.c
>> index
>> 4bd54bef540806ca90d2b9bcfc33eb531991c967..da2f465a5b5003fa5dca05f3a6ee00e97b98b5dd
>> 100644
>> --- a/gcc/testsuite/gcc.dg/strlenopt-26.c
>> +++ b/gcc/testsuite/gcc.dg/strlenopt-26.c
>> @@ -21,4 +21,5 @@ main (void)
>> return 0;
>> }
>>
>> -/* { dg-final { scan-tree-dump-times "strlen \\(" 1 "strlen" } } */
>> +/* { dg-final { scan-tree-dump-times "strlen \\(" 2 "strlen" } } */
>> +/* { dg-final { scan-tree-dump-times "strchr \\(" 0 "strlen" } } */
>> diff --git a/gcc/testsuite/gcc.dg/strlenopt-5.c
>> b/gcc/testsuite/gcc.dg/strlenopt-5.c
>> index
>> 1b006a93045599a75bdb10a39e86ffa59b475c83..a24aea44e8b00ff7b35a907aaa941b4c509642c4
>> 100644
>> --- a/gcc/testsuite/gcc.dg/strlenopt-5.c
>> +++ b/gcc/testsuite/gcc.dg/strlenopt-5.c
>> @@ -48,9 +48,9 @@ main ()
>> return 0;
>> }
>>
>> -/* { dg-final { scan-tree-dump-times "strlen \\(" 0 "strlen" } } */
>> +/* { dg-final { scan-tree-dump-times "strlen \\(" 2 "strlen" } } */
>> /* { dg-final { scan-tree-dump-times "memcpy \\(" 2 "strlen" } } */
>> /* { dg-final { scan-tree-dump-times "strcpy \\(" 1 "strlen" } } */
>> /* { dg-final { scan-tree-dump-times "strcat \\(" 0 "strlen" } } */
>> -/* { dg-final { scan-tree-dump-times "strchr \\(" 2 "strlen" } } */
>> +/* { dg-final { scan-tree-dump-times "strchr \\(" 0 "strlen" } } */
>> /* { dg-final { scan-tree-dump-times "stpcpy \\(" 0 "strlen" } } */
>> diff --git a/gcc/testsuite/gcc.dg/strlenopt-7.c
>> b/gcc/testsuite/gcc.dg/strlenopt-7.c
>> index
>> 3ae1e2cb3f0d08c8b05107c4e65b67bdb39cf7ab..aa53d7e75254dfe56c93172afc49f95e5b7901e6
>> 100644
>> --- a/gcc/testsuite/gcc.dg/strlenopt-7.c
>> +++ b/gcc/testsuite/gcc.dg/strlenopt-7.c
>> @@ -40,11 +40,11 @@ main ()
>> return 0;
>> }
>>
>> -/* { dg-final { scan-tree-dump-times "strlen \\(" 0 "strlen" } } */
>> +/* { dg-final { scan-tree-dump-times "strlen \\(" 1 "strlen" } } */
>> /* { dg-final { scan-tree-dump-times "memcpy \\(" 2 "strlen" } } */
>> /* { dg-final { scan-tree-dump-times "strcpy \\(" 0 "strlen" } } */
>> /* { dg-final { scan-tree-dump-times "strcat \\(" 0 "strlen" } } */
>> -/* { dg-final { scan-tree-dump-times "strchr \\(" 1 "strlen" } } */
>> +/* { dg-final { scan-tree-dump-times "strchr \\(" 0 "strlen" } } */
>> /* { dg-final { scan-tree-dump-times "stpcpy \\(" 0 "strlen" } } */
>> /* { dg-final { scan-tree-dump-times "\\*r_\[0-9\]* = 0;" 1 "strlen" } } */
>> /* { dg-final { scan-tree-dump-times "return 3;" 1 "optimized" } } */
>> diff --git a/gcc/testsuite/gcc.dg/strlenopt-9.c
>> b/gcc/testsuite/gcc.dg/strlenopt-9.c
>> index
>> b0406b162d48fca375883035043b0c50b9db61a1..e5e276210ba4b7d75867605f1ecf5c06eb970ef5
>> 100644
>> --- a/gcc/testsuite/gcc.dg/strlenopt-9.c
>> +++ b/gcc/testsuite/gcc.dg/strlenopt-9.c
>> @@ -98,10 +98,10 @@ main ()
>> return 0;
>> }
>>
>> -/* { dg-final { scan-tree-dump-times "strlen \\(" 3 "strlen" } } */
>> +/* { dg-final { scan-tree-dump-times "strlen \\(" 5 "strlen" } } */
>> /* { dg-final { scan-tree-dump-times "memcpy \\(" 6 "strlen" } } */
>> /* { dg-final { scan-tree-dump-times "strcpy \\(" 1 "strlen" } } */
>> /* { dg-final { scan-tree-dump-times "strcat \\(" 0 "strlen" } } */
>> -/* { dg-final { scan-tree-dump-times "strchr \\(" 3 "strlen" } } */
>> +/* { dg-final { scan-tree-dump-times "strchr \\(" 0 "strlen" } } */
>> /* { dg-final { scan-tree-dump-times "stpcpy \\(" 0 "strlen" } } */
>> /* { dg-final { scan-tree-dump-times "return 4;" 1 "optimized" } } */
>>
mas-allow-string-builtins
Description: Binary data
