This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Optimize strchr (s, 0) to strlen
- From: Wilco Dijkstra <Wilco dot Dijkstra at arm dot com>
- To: Richard Biener <richard dot guenther at gmail dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, nd <nd at arm dot com>
- Date: Wed, 20 Apr 2016 11:56:32 +0000
- Subject: Re: [PATCH] Optimize strchr (s, 0) to strlen
- Authentication-results: sourceware.org; auth=none
- Nodisclaimer: True
- References: <AM3PR08MB0088CA61259F65FAAB4D8196836B0 at AM3PR08MB0088 dot eurprd08 dot prod dot outlook dot com> <CAFiYyc1rGd2KWOaN4RTG45Y1uUp6O0A5qOm=i5ma0BZSK5CrXw at mail dot gmail dot com> <AM3PR08MB00881BE3867DF3FC5B5B7530836C0 at AM3PR08MB0088 dot eurprd08 dot prod dot outlook dot com>,<CAFiYyc0cUOs19FV-2PnYxfba4_N8Qwox2tkgXZJEw2obe20zgg at mail dot gmail dot com>
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:23
Richard Biener wrote:
> 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?
In what way would it get more info to decide on profitability? The transform is
profitable unless you messed up your strlen implementation badly.
For -Os one could do the reverse, but I don't think it is going to give a substantial
codesize gain compared to other simple improvements, so unlikely worth it.
>> + 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).
Does that use the often incorrect BB probabilities? I used the function variant on
purpose to avoid it making the wrong decision. A typical example I see is that GCC
inlines a return sequence into an if marked with __builtin_expect (c, 0) but not in the
hot code that follows...
> I think you want to build a gimple_assign directly here, otherwise ...
>... this may not reliably end up at the call stmt.
OK, I revisit that once we've agreed how to proceed with this patch - we now have
3 variants...
Wilco