Re: PR79697: Delete calls to strdup, strndup, realloc if there is no lhs

[Prathamesh Kulkarni <prathamesh dot kulkarni at linaro dot org>]

On 27 February 2017 at 00:10, Prathamesh Kulkarni
<prathamesh.kulkarni@linaro.org> wrote:
> On 26 February 2017 at 05:16, Martin Sebor <msebor@gmail.com> wrote:
>> On 02/25/2017 11:54 AM, Prathamesh Kulkarni wrote:
>>>
>>> On 25 February 2017 at 14:43, Marc Glisse <marc.glisse@inria.fr> wrote:
>>>>
>>>> On Sat, 25 Feb 2017, Prathamesh Kulkarni wrote:
>>>>
>>>>> Hi,
>>>>> The attached patch deletes calls to strdup, strndup if it's
>>>>> return-value is unused,
>>>>> and same for realloc if the first arg is NULL.
>>>>> Bootstrap+tested on x86_64-unknown-linux-gnu.
>>>>> OK for GCC 8 ?
>>>>
>>>>
>>>>
>>>> Instead of specializing realloc(0,*) wherever we can perform the same
>>>> optimization as with malloc, wouldn't it be better to optimize:
>>>> realloc(0,n) -> malloc(n)
>>>> and let the malloc optimizations happen?
>>>
>>> Thanks for the suggestions. In the attached patch, realloc (0, n) is
>>> folded to malloc (n).
>>> Bootstrap+test in progress on x86_64-unknown-linux-gnu.
>>> Does the patch look OK ?
>>
>>
>> Although it's not part of the bug report, I wonder if a complete
>> patch should also extend the malloc/free DCE to str{,n}dup/free
>> calls and eliminate pairs like these:
>>
>>   void f (const char *s)
>>   {
>>     char *p = strdup (src);
>>     free (p);
>>   }
>>
>> (That seems to be just a matter of adding a couple of conditionals
>> for BUILT_IN_STR{,N}DUP in propagate_necessity).
> Hi Martin,
> Thanks for the suggestions, I have updated the patch accordingly.
> Does it look OK ?
Hi,
The attached patch for PR79697 passes bootstrap+test on x86_64 and
cross-tested on arm*-*-* and aarch64*-*-*.
As per the previous suggestions, the patch folds realloc (0, n) -> malloc (n).
Is it OK for trunk ?

Thanks,
Prathamesh
>>
>> Martin
>>
>> PS Another optimization, though one that's most likely outside
>> the scope of this patch, is to eliminate all of the following:
>>
>>   void f (const char *s, char *s2)
>>   {
>>     char *p = strdup (s);
>>     strcpy (p, s2);
>>     free (p);
>>   }
>>
>> as is done in:
>>
>>   void f (unsigned n, const char *s)
>>   {
>>     char *p = malloc (n);
>>     memcpy (p, s, n);
>>     free (p);
>>   }
>>
> Hmm, dse1 detects that the store to p in memcpy() is dead and deletes the call.
> cddce1 then removes calls to  malloc and free thus making the function empty.
> It doesn't work if memcpy is replaced by strcpy:
>
> void f (unsigned n, char *s2)
> {
>   char *p = __builtin_malloc (n);
>   __builtin_strcpy (p, s2);
>   __builtin_free (p);
> }
>
> I suppose strcpy should be special-cased too in dse similar to memcpy ?
>
> Thanks,
> Prathamesh

Attachment: pr79697-3.txt
Description: Text document