This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix memory leaks in tree-ssa-uninit.c
- From: Bernd Schmidt <bschmidt at redhat dot com>
- To: Martin LiÅka <mliska at suse dot cz>, gcc-patches at gcc dot gnu dot org, joseph at codesourcery dot com
- Date: Thu, 19 Nov 2015 14:58:22 +0100
- Subject: Re: [PATCH] Fix memory leaks in tree-ssa-uninit.c
- Authentication-results: sourceware.org; auth=none
- References: <564081EF dot 7030003 at suse dot cz> <5645DCB1 dot 6070309 at suse dot cz> <564610B4 dot 9080708 at redhat dot com> <564616BD dot 6070206 at suse dot cz> <564637C9 dot 4000009 at redhat dot com> <564C89D9 dot 7090303 at suse dot cz> <alpine dot DEB dot 2 dot 10 dot 1511190039250 dot 26547 at digraph dot polyomino dot org dot uk> <564DA193 dot 9030509 at suse dot cz>
On 11/19/2015 11:16 AM, Martin Liška wrote:
You are right, however as the original coding style was really broken,
it was much easier
to use the tool and clean-up fall-out.
Waiting for thoughts related to v2.
Better, but still some oddities. I hope you won't get mad at me if I
suggest doing this in stages? A first patch could just deal with
non-reformatting whitespace changes, such as removing trailing
whitespace, and converting leading spaces to tabs - that would be
mechanical, and reduce the size of the rest of the patch (it seems emacs
has an appropriate command, M-x whitespace-cleanup). Such a change is
preapproved.
A few things I noticed:
- flag_1 = phi <0, 1> // (1)
+ flag_1 = phi <0, 1> // (1)
Check whether the // (1) is still lined up with the // (2) and // (3)
parts. In general I'm not sure we should to what extent we should be
reformatting comments in this patch. Breaking long lines and ensuring
two spaces after a period seems fine.
+ Checking recursively into (1), the compiler can find out that only some_val
+ (which is defined) can flow into (3) which is OK.
*/
Could take the opportunity to move the */ onto the end of the line.
+ if (is_gimple_call (cond_stmt) && EDGE_COUNT (e->src->succs) >= 2)
+ {
+ /* Ignore EH edge. Can add assertion
+ on the other edge's flag. */
+ continue;
+ }
Could also take the opportunity to remove excess braces and parentheses.
Not a requirement and probably a distraction at this point.
- return (pred.cond_code == NE_EXPR && !pred.invert)
- || (pred.cond_code == EQ_EXPR && pred.invert);
+ return (pred.cond_code == NE_EXPR && !pred.invert)
+ || (pred.cond_code == EQ_EXPR && pred.invert);
This still isn't right.
Bernd