[PATCH][Tree-optimization/PR89772]fold memchr builtins for character not in constant nul-padded string

[JunMa]

在 2019/6/18 上午6:46, Jeff Law 写道:
> On 5/9/19 2:01 AM, JunMa wrote:
>> 在 2019/5/9 上午10:22, JunMa 写道:
>>> 在 2019/5/9 上午3:02, Bernd Edlinger 写道:
>>>> On 5/8/19 4:31 PM, Richard Biener wrote:
>>>>> On Tue, May 7, 2019 at 4:34 AM JunMa <JunMa@linux.alibaba.com> wrote:
>>>>>> 在 2019/5/6 下午7:58, JunMa 写道:
>>>>>>> 在 2019/5/6 下午6:02, Richard Biener 写道:
>>>>>>>> On Thu, Mar 21, 2019 at 5:57 AM JunMa <JunMa@linux.alibaba.com>
>>>>>>>> wrote:
>>>>>>>>> Hi
>>>>>>>>> For now, gcc can not fold code like:
>>>>>>>>>
>>>>>>>>> const char a[5] = "123"
>>>>>>>>> __builtin_memchr (a, '7', sizeof a)
>>>>>>>>>
>>>>>>>>> It tries to avoid folding out of string length although length of a
>>>>>>>>> is 5.
>>>>>>>>> This is a bit conservative, it's safe to folding memchr/bcmp/memcmp
>>>>>>>>> builtins when constant string stores in array with some trailing
>>>>>>>>> nuls.
>>>>>>>>>
>>>>>>>>> This patch folds these cases by exposing additional length of
>>>>>>>>> trailing nuls in c_getstr().
>>>>>>>>> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>>>>>>>> It's probably better if gimple_fold_builtin_memchr uses
>>>>>>>> string_constant
>>>>>>>> directly instead?
>>>>>>> We can use string_constant in gimple_fold_builtin_memchr, however
>>>>>>> it is a
>>>>>>> bit complex to use it in memchr/memcmp constant folding.
>>>>>>>> You are changing memcmp constant folding but only have a testcase
>>>>>>>> for memchr.
>>>>>>> I'll add later.
>>>>>>>> If we decide to amend c_getstr I would rather make it return the
>>>>>>>> total length instead of the number of trailing zeros.
>>>>>>> I think return the total length is safe in other place as well.
>>>>>>> I used the argument in patch since I do not want any impact on
>>>>>>> other part at all.
>>>>>>>
>>>>>> Although it is safe to use total length, I found that function
>>>>>> inline_expand_builtin_string_cmp() which used c_getstr() may emit
>>>>>> redundant rtls for trailing null chars when total length is returned.
>>>>>>
>>>>>> Also it is trivial to handle constant string with trailing null chars.
>>>>>>
>>>>>> So this updated patch follow richard's suggestion: using
>>>>>> string_constant
>>>>>> directly.
>>>>>>
>>>>>> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>>>>> Doesn't this fold to NULL for the case where seaching for '0' and it
>>>>> doesn't occur in the string constant but only the zero-padding?
> So I'm not sure if JunMa addressed this question specifically.  What
> happens is we'll get back a NULL terminated string from c_getstr, but
> the returned length will include the NUL terminator.  Then we call
> memchr on the result with a length that would include that NUL
> terminator.  So I'm pretty sure the current patch will DTRT in that case.
>
> It'd be better to have a stronger test which verified not only that the
> call was folded away, but what the resultant value was and whether or
> not it was the right value.
>
> That would include testing for NUL in the string as well as testing for
> NUL in the tail padding.
>
> I'll ack the change to gimple-fold, but please improve the testcase a
> bit and commit the change to gimple-fold and the improved testcase together.
>
> Thanks, and sorry for the delays.
Thanks, I'll update the testcase and check in after pass the regtest.

Regards
Jun
> jeff