Re: [RFC][PATCH] Extend DCE to remove unnecessary new/delete-pairs

[Jeff Law <law at redhat dot com>]

On 11/22/2017 03:31 AM, Martin Jambor wrote:
> On Tue, Nov 21 2017, Jeff Law wrote:
>> On 11/21/2017 04:14 AM, Dominik Inführ wrote:
>>> Hi,
>>>
>>> this patch tries to extend tree-ssa-dce.c to remove unnecessary new/delete-pairs (it already does that for malloc/free). Clang does it too and it seems to be allowed by http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3664.html. I’ve bootstrapped/regtested on aarch64-linux and x86_64-linux.
>> Just a note, we've transitioned into stage3 in preparation for the
>> upcoming gcc-8 release in the spring.  During stage3 we're addressing
>> bugfixes, not further enhancements (with the exception of enhancements
>> that were posted prior to stage1 close).
>>
>> So it's unlikely anyone will dig into this right now, unless there's an
>> existing bugzilla around this missed optimization.
> 
> Unless I am mistaken, I think it is PR 23383
I think 23383 touches on some of these issues, but I'm not sure it's the
same thing.  Marking new with the malloc attribute should in theory be
all that's necessary to address the original issue in 23383.

Marking *should* allow the alias analysis code to disambiguate "a" and
"b" and in turn allow us to determine that the return value is always
"1".  That's the crux of 23383.

The final comment notes we can omit the calls which is good as it gives
us freedom in c++14 mode.   I'm going to assume that the requirement is
that we have to provide suitable space either on the stack, in a
register or within another object and that removal of that additional
space is valid if we determine it's actually not needed.

Anyway, 23383 is likely a piece what's done with this patch.  Namely
that with this patch we'd likely wipe out the new/delete pair for "b" as
the value is never used and never escapes.  Then we'd probably do the
same for "a".  But ISTM this is beyond what the BZ was complaining about.


You could make the argument that the marking done by the patch should go
in under 23383, that seems pretty easy.  You might then argue that the
DCE extension is a trivial extension of that work, trivial enough that
it could be considered under the umbrella of 23383.

Jeff



> 
> Martin
>