From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x32c.google.com (mail-ot1-x32c.google.com [IPv6:2607:f8b0:4864:20::32c]) by sourceware.org (Postfix) with ESMTPS id 619D9386102E for ; Thu, 22 Jul 2021 18:03:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 619D9386102E Received: by mail-ot1-x32c.google.com with SMTP id 59-20020a9d0ac10000b0290462f0ab0800so6087122otq.11 for ; Thu, 22 Jul 2021 11:03:16 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=W+tlANkBGQOfu5KK+ZxCo5lbUolpk26MiR4f0BvX9FE=; b=ZsakvxmcLExvKSDfxdR/WpPWp35ZiVExOCmfKz/ti0xEox2aCyEQZbYY5bd/pKWt/t Bvppa/Ydd1XBrDkzECJqLyhSFa1m4YDfPbzZgmgIAdo0V9Pr9GO/1WgkkoKysK0i79JC dHnWoyfx0PvwXXx9RPXtl21goo/Lz92JWLoMNinXoXP0OFQWTewPdAEIeoLkpCzxTE7W YMZFp6UbLcMNj347iAIGkSQoGKsQXUSTSg4+CeNJV2f2nDpujh046e1gFihsx2qXHtRK 8mlAyq67cZAfESea61N9HCNE6TVxyiTjiIP7yWStVWSuKrcfxDqStQ1IM3qyZepeHZkw Wx+g== X-Gm-Message-State: AOAM530kFPerrG5OjDdjlkGWHFyBeJWPqYE0RIgyhhkgg4+5UOxMkCfp 5NF5iCzvaInr2meiWH1QHg8= X-Google-Smtp-Source: ABdhPJwV6FqJS+5qpO8/PUBRhRprH9Pn4DbunGMk1FcxNQhwka5uW7Tfcq+nCf9C19Ms/NTEU6FN3w== X-Received: by 2002:a05:6830:544:: with SMTP id l4mr671137otb.164.1626976995694; Thu, 22 Jul 2021 11:03:15 -0700 (PDT) Received: from [192.168.0.41] (75-166-102-22.hlrn.qwest.net. [75.166.102.22]) by smtp.gmail.com with ESMTPSA id 33sm5267257ots.19.2021.07.22.11.03.15 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 22 Jul 2021 11:03:15 -0700 (PDT) Subject: Re: [PATCH] tree-optimization/101573 - improve uninit warning at -O0 To: Richard Biener , gcc-patches@gcc.gnu.org Cc: msebor@redhat.com References: <3on3509q-309q-8562-r54n-qr20o3ns1p5q@fhfr.qr> From: Martin Sebor Message-ID: Date: Thu, 22 Jul 2021 12:03:14 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 In-Reply-To: <3on3509q-309q-8562-r54n-qr20o3ns1p5q@fhfr.qr> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_NUMSUBJECT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Jul 2021 18:03:18 -0000 On 7/22/21 6:34 AM, Richard Biener wrote: > We can improve uninit warnings from the early pass by looking > at PHI arguments on fallthru edges that are uninitialized and > have uses that are before a possible loop exit. This catches > some cases earlier that we'd only warn in a more confusing > way after early inlining as seen by testcase adjustments. > > It introduces > > FAIL: gcc.dg/uninit-23.c (test for excess errors) > > where we additionally warn > > gcc.dg/uninit-23.c:21:13: warning: 't4' is used uninitialized [-Wuninitialized] > > which I think is OK even if it's not obvious that the new > warning is an improvement when you look at the obvious source. > > Somehow for all cases I never get the `'foo' was declared here` > notes, I didn't dig why that happens but it's odd. I happen to be working in that area right now so let me look into that. > > Bootstrapped and tested on x86_64-unknown-linux-gnu. > > Any comments? I'm (of course) favor of improving the warning -- thank you! I've been meaning to do change the early pass to look at PHIs as well. I would just suggest to put the new code in a new function for better readability. Martin > > Thanks, > Richard. > > 2021-07-22 Richard Biener > > PR tree-optimization/101573 > * tree-ssa-uninit.c (warn_uninitialized_vars): Look at > uninitialized PHI arg defs in some constrained cases. > (execute_early_warn_uninitialized): Calculate dominators. > > * gcc.dg/uninit-pr101573.c: New testcase. > * gcc.dg/uninit-15-O0.c: Adjust. > * gcc.dg/uninit-15.c: Likewise. > * gcc.dg/uninit-23.c: Likewise. > * c-c++-common/uninit-17.c: Likewise. > --- > gcc/testsuite/c-c++-common/uninit-17.c | 6 +-- > gcc/testsuite/gcc.dg/uninit-15-O0.c | 2 +- > gcc/testsuite/gcc.dg/uninit-15.c | 10 ++-- > gcc/testsuite/gcc.dg/uninit-23.c | 2 +- > gcc/testsuite/gcc.dg/uninit-pr101573.c | 10 ++++ > gcc/tree-ssa-uninit.c | 66 ++++++++++++++++++++++++++ > 6 files changed, 86 insertions(+), 10 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/uninit-pr101573.c > > diff --git a/gcc/testsuite/c-c++-common/uninit-17.c b/gcc/testsuite/c-c++-common/uninit-17.c > index fd773da78ad..b5495366c5b 100644 > --- a/gcc/testsuite/c-c++-common/uninit-17.c > +++ b/gcc/testsuite/c-c++-common/uninit-17.c > @@ -9,11 +9,11 @@ static void bar(int a, int *ptr) > { > do > { > - int b; /* { dg-message "declared" } */ > + int b; > if (b < 40) { > - ptr[0] = b; /* { dg-warning "may be used uninitialized" } */ > + ptr[0] = b; > } > - b += 1; > + b += 1; /* { dg-warning "is used uninitialized" } */ > ptr++; > } > while (--a != 0); > diff --git a/gcc/testsuite/gcc.dg/uninit-15-O0.c b/gcc/testsuite/gcc.dg/uninit-15-O0.c > index a3fd2b63ba7..36d96348617 100644 > --- a/gcc/testsuite/gcc.dg/uninit-15-O0.c > +++ b/gcc/testsuite/gcc.dg/uninit-15-O0.c > @@ -15,6 +15,6 @@ void baz(); > void bar() > { > int j; /* { dg-message "was declared here" {} { xfail *-*-* } } */ > - for (; foo(j); ++j) > + for (; foo(j); ++j) /* { dg-warning "is used uninitialized" } */ > baz(); > } > diff --git a/gcc/testsuite/gcc.dg/uninit-15.c b/gcc/testsuite/gcc.dg/uninit-15.c > index 8ee10c27aba..85cb116f349 100644 > --- a/gcc/testsuite/gcc.dg/uninit-15.c > +++ b/gcc/testsuite/gcc.dg/uninit-15.c > @@ -1,7 +1,7 @@ > /* PR tree-optimization/17506 > - We issue an uninitialized variable warning at a wrong location at > + We used to issue an uninitialized variable warning at a wrong location at > line 11, which is very confusing. Make sure we print out a note to > - make it less confusing. (not xfailed alternative) > + make it less confusing. (xfailed alternative) > But it is of course ok if we warn in bar about uninitialized use > of j. (not xfailed alternative) */ > /* { dg-do compile } */ > @@ -10,7 +10,7 @@ > inline int > foo (int i) > { > - if (i) /* { dg-warning "used uninitialized" } */ > + if (i) /* { dg-warning "used uninitialized" "" { xfail *-*-* } } */ > return 1; > return 0; > } > @@ -20,7 +20,7 @@ void baz (void); > void > bar (void) > { > - int j; /* { dg-message "note: 'j' was declared here" "" } */ > - for (; foo (j); ++j) /* { dg-warning "'j' is used uninitialized" "" { xfail *-*-* } } */ > + int j; /* { dg-message "note: 'j' was declared here" "" { xfail *-*-* } } */ > + for (; foo (j); ++j) /* { dg-warning "'j' is used uninitialized" } */ > baz (); > } > diff --git a/gcc/testsuite/gcc.dg/uninit-23.c b/gcc/testsuite/gcc.dg/uninit-23.c > index d64eb7d2ee9..87b4e989b53 100644 > --- a/gcc/testsuite/gcc.dg/uninit-23.c > +++ b/gcc/testsuite/gcc.dg/uninit-23.c > @@ -18,7 +18,7 @@ ql (void) > int *t4 = go; /* { dg-warning "is used uninitialized" } */ > > l1: > - *t4 = (*t4 != 0) ? 0 : 2; > + *t4 = (*t4 != 0) ? 0 : 2; /* { dg-warning "is used uninitialized" } */ > } > > if (ij != 0) > diff --git a/gcc/testsuite/gcc.dg/uninit-pr101573.c b/gcc/testsuite/gcc.dg/uninit-pr101573.c > new file mode 100644 > index 00000000000..a574844b791 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/uninit-pr101573.c > @@ -0,0 +1,10 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O0 -Wuninitialized" } */ > + > +int main(int argc, char **argv) > +{ > + int a; > + for(; a < 5; ++a) /* { dg-warning "is used uninitialized" } */ > + ; > + return 0; > +} > diff --git a/gcc/tree-ssa-uninit.c b/gcc/tree-ssa-uninit.c > index 148f3c2b31d..2ee4edf353f 100644 > --- a/gcc/tree-ssa-uninit.c > +++ b/gcc/tree-ssa-uninit.c > @@ -652,6 +652,71 @@ warn_uninitialized_vars (bool wmaybe_uninit) > { > basic_block succ = single_succ (ENTRY_BLOCK_PTR_FOR_FN (cfun)); > wlims.always_executed = dominated_by_p (CDI_POST_DOMINATORS, succ, bb); > + > + if (wlims.always_executed) > + { > + edge_iterator ei; > + edge e, found = NULL, found_back = NULL; > + /* Look for a fallthru and possibly a single backedge. */ > + FOR_EACH_EDGE (e, ei, bb->preds) > + { > + /* Ignore backedges. */ > + if (dominated_by_p (CDI_DOMINATORS, e->src, bb)) > + { > + if (found_back) > + { > + found = NULL; > + break; > + } > + found_back = e; > + continue; > + } > + if (found) > + { > + found = NULL; > + break; > + } > + found = e; > + } > + if (found) > + for (gphi_iterator si = gsi_start_phis (bb); !gsi_end_p (si); > + gsi_next (&si)) > + { > + gphi *phi = si.phi (); > + tree def = PHI_ARG_DEF_FROM_EDGE (phi, found); > + if (TREE_CODE (def) != SSA_NAME > + || !SSA_NAME_IS_DEFAULT_DEF (def) > + || virtual_operand_p (def)) > + continue; > + /* If there's a default def on the fallthru edge PHI > + value and there's a use that post-dominates entry > + then that use is uninitialized and we can warn. */ > + imm_use_iterator iter; > + use_operand_p use_p; > + gimple *use_stmt = NULL; > + FOR_EACH_IMM_USE_FAST (use_p, iter, gimple_phi_result (phi)) > + { > + use_stmt = USE_STMT (use_p); > + if (gimple_location (use_stmt) != UNKNOWN_LOCATION > + && dominated_by_p (CDI_POST_DOMINATORS, succ, > + gimple_bb (use_stmt)) > + /* If we found a non-fallthru edge make sure the > + use is inside the loop, otherwise the backedge > + can serve as initialization. */ > + && (!found_back > + || dominated_by_p (CDI_DOMINATORS, found_back->src, > + gimple_bb (use_stmt)))) > + break; > + use_stmt = NULL; > + } > + if (use_stmt) > + warn_uninit (OPT_Wuninitialized, def, SSA_NAME_VAR (def), > + SSA_NAME_VAR (def), > + "%qD is used uninitialized", use_stmt, > + UNKNOWN_LOCATION); > + } > + } > + > for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi)) > { > gimple *stmt = gsi_stmt (gsi); > @@ -3135,6 +3200,7 @@ execute_early_warn_uninitialized (void) > optimization we want to warn about possible uninitialized as late > as possible, thus don't do it here. However, without > optimization we need to warn here about "may be uninitialized". */ > + calculate_dominance_info (CDI_DOMINATORS); > calculate_dominance_info (CDI_POST_DOMINATORS); > > warn_uninitialized_vars (/*warn_maybe_uninitialized=*/!optimize); >