From patchwork Mon Jul 28 06:08:48 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Wei Mi X-Patchwork-Id: 374093 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 14282140108 for ; Mon, 28 Jul 2014 16:09:03 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; q=dns; s=default; b=ZxdfqxuxsMB6gl/nbN j4P3MWbo6PeBTb/6/TjL1Vqdp4YHvbV2sX184q6CzGjtvsYuxcUp0buk3qNztILd /htMGK92NSWpD7O11/CA3iC3lj3BaAV9Mfm9dgtHvF/Tmki5TWIL+1QFha+iIx5+ 4cx+WKKPcscrj/LBoYp5Af+Uc= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; s=default; bh=ovj2McmrnBj4Ql92UN/ttp06 4G0=; b=lCgJO3tWwtVgkbsOK7Qh2wzxH4gtKJcb9j6el+U7bi2K3vxncNEjlfVt 9lzHHJjEFqVs++hKrbtS4BPFnX/0+PMSjUnb/clJZARvpPDkZhE7SAJA2g3BNfrR Z5nFrlyH7clVek5wIvgElrcX6jzgXvOC1SBQBVRvohGDbOZzTQE= Received: (qmail 16481 invoked by alias); 28 Jul 2014 06:08:55 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 16454 invoked by uid 89); 28 Jul 2014 06:08:53 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.0 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-oi0-f52.google.com Received: from mail-oi0-f52.google.com (HELO mail-oi0-f52.google.com) (209.85.218.52) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Mon, 28 Jul 2014 06:08:51 +0000 Received: by mail-oi0-f52.google.com with SMTP id h136so5599529oig.11 for ; Sun, 27 Jul 2014 23:08:49 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=AGb8V6MqVtJjqlmjhYOuTfEiWEPQc5UvvWDDZcs5Ajs=; b=Kjy8pXC2atV5xqEYVkWvMqzmsTERyr4TDmmhM8OHU49dQuU456dPc8Vii9uF/3W47g iH7sr9xNVAB34UOR+7IHdcycDoApo/cc2Tlx+sfzvWoqyfKe57UOuEnu2VFD9nueuqvq rDqoHYXwyVGQjL9ZNMMZNTRt0l0ps+lcQeF1Kj36GM1MdbqOQYpNG3zJN6MuWNZMBLPR RhSNeOidE34O7ao2MwoVCj3ZBEbLhC0+V3+jrBzehix1YcwUqnBGJZhygR9Rwq9LFnTM xe6ugXp/HltJnwYDCCaufyQpXEV72J07ZQAO/drnJHyIvhWSdSlwchnmFioR4Q1s59ss Z+Eg== X-Gm-Message-State: ALoCoQle/1ioErVPVOzK33czdsAfSrCABmEmYJ5E1ztaXXI2hhXo1GRj3f5oh6wFj+N6LLLZfrnR MIME-Version: 1.0 X-Received: by 10.60.77.68 with SMTP id q4mr45800924oew.1.1406527728944; Sun, 27 Jul 2014 23:08:48 -0700 (PDT) Received: by 10.76.91.138 with HTTP; Sun, 27 Jul 2014 23:08:48 -0700 (PDT) In-Reply-To: References: Date: Sun, 27 Jul 2014 23:08:48 -0700 Message-ID: Subject: Re: [PATCH, PR61776] verify_flow_info failed: control flow in the middle of basic block with -fprofile-generate From: Wei Mi To: Richard Biener Cc: GCC Patches , Jan Hubicka , David Li , Martin Jambor > But fact is that it is _not_ necessary to split the block because there > are no outgoing abnormal edges from it. > > The verifier failure is an artifact from using the same predicates during > CFG building and CFG verifying (usually ok, but for this particular > case it leads to this issue). > > So I don't think your patch is the proper way to address this issue > (while it certainly works). > > Instead whether a call can make abnormal gotos should be recorded > per-call and stored on the gimple-call. Martin - this is exactly > one of the cases your patch would address? > Thanks for the comment and thanks to Martin's patch. I try the patch. It works well to address both pr60449 and pr61776 after some extension. One extension is to replace GF_CALL_LEAF attribute using GF_CALL_NO_ABNORMAL_GOTO. That is because not only dropping "leaf" attribute in lto symbol merge could introduce the control flow verification problem in pr60449, dropping "const/pure" attributes could introduce the same problem too. It is unnecessary to introduce per-call attributes for all these three: ECF_LEAF/ECF_CONST/ECF_PURE, so GF_CALL_NO_ABNORMAL_GOTO is introduced to indicate that a call stmt has no abnormal goto. GF_CALL_NO_ABNORMAL_GOTO will be set according to gimple_call_flags() once gimple call stmt is created, then updated in execute_fixup_cfg and cleanup_tree_cfg. I posted the extended patch here. I didn't add the noreturn part in because it has no direct impact on pr60449 and pr61776. I can help Martin to test and post that part as an independent patch later. bootstrap and regression pass on x86_64-linux-gnu. Is it ok? Thanks, Wei. ChangeLog: 2014-07-27 Martin Jambor Wei Mi PR ipa/60449 PR middle-end/61776 * tree-cfgcleanup.c (update_no_abnormal_goto_attr): New function. (cleanup_tree_cfg_1): Use update_no_abnormal_goto_attr. * gimple.c (gimple_call_initialize_no_abnormal_goto): New function. (gimple_build_call_1): Use gimple_call_initialize_no_abnormal_goto. (gimple_build_call_internal_1): Ditto. * gimple.h (enum gf_mask): Added GF_NO_ABNORMAL_GOTO. (gimple_call_set_no_abnormal_goto): New function. (gimple_call_no_abnormal_goto_p): Ditto. * tree-cfg.c (call_can_make_abnormal_goto): Use gimple_call_no_abnormal_goto_p. (execute_fixup_cfg): Use gimple_call_set_no_abnormal_goto. 2014-07-27 Martin Jambor Wei Mi PR ipa/60449 PR middle-end/61776 * testsuite/gcc.dg/pr61776.c: New test. * testsuite/gcc.dg/lto/pr60449_1.c: New test. * testsuite/gcc.dg/lto/pr60449_0.c: New test. Index: tree-cfgcleanup.c =================================================================== --- tree-cfgcleanup.c (revision 212442) +++ tree-cfgcleanup.c (working copy) @@ -621,6 +621,28 @@ split_bbs_on_noreturn_calls (void) return changed; } +/* Update GF_NO_ABNORMAL_GOTO attribute for call stmts in BB according + to gimple_call_flags. */ + +static void +update_no_abnormal_goto_attr (basic_block bb) +{ + gimple_stmt_iterator gsi; + for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi)) + { + gimple stmt = gsi_stmt (gsi); + + if (!is_gimple_call (stmt)) + continue; + + int flags = gimple_call_flags (stmt); + if ((flags & (ECF_CONST | ECF_PURE) + && !(flags & ECF_LOOPING_CONST_OR_PURE)) + || (flags & ECF_LEAF)) + gimple_call_set_no_abnormal_goto (stmt, true); + } +} + /* Tries to cleanup cfg in basic block BB. Returns true if anything changes. */ @@ -672,7 +694,10 @@ cleanup_tree_cfg_1 (void) { bb = BASIC_BLOCK_FOR_FN (cfun, i); if (bb) - retval |= cleanup_tree_cfg_bb (bb); + { + update_no_abnormal_goto_attr (bb); + retval |= cleanup_tree_cfg_bb (bb); + } } /* Now process the altered blocks, as long as any are available. */ @@ -687,6 +712,7 @@ cleanup_tree_cfg_1 (void) if (!bb) continue; + update_no_abnormal_goto_attr (bb); retval |= cleanup_tree_cfg_bb (bb); /* Rerun split_bbs_on_noreturn_calls, in case we have altered any noreturn Index: gimple.c =================================================================== --- gimple.c (revision 212442) +++ gimple.c (working copy) @@ -186,6 +186,19 @@ gimple_build_return (tree retval) return s; } +/* Set GF_NO_ABNORMAL_GOTO attribute according to gimple_call_flags(STMT). */ + +void +gimple_call_initialize_no_abnormal_goto (gimple stmt) +{ + int flags = gimple_call_flags (stmt); + + if ((flags & (ECF_CONST | ECF_PURE) + && !(flags & ECF_LOOPING_CONST_OR_PURE)) + || (flags & ECF_LEAF)) + gimple_call_set_no_abnormal_goto (stmt, true); +} + /* Reset alias information on call S. */ void @@ -215,6 +228,7 @@ gimple_build_call_1 (tree fn, unsigned n gimple_set_op (s, 1, fn); gimple_call_set_fntype (s, TREE_TYPE (TREE_TYPE (fn))); gimple_call_reset_alias_info (s); + gimple_call_initialize_no_abnormal_goto (s); return s; } @@ -290,6 +304,7 @@ gimple_build_call_internal_1 (enum inter s->subcode |= GF_CALL_INTERNAL; gimple_call_set_internal_fn (s, fn); gimple_call_reset_alias_info (s); + gimple_call_initialize_no_abnormal_goto (s); return s; } Index: gimple.h =================================================================== --- gimple.h (revision 212442) +++ gimple.h (working copy) @@ -90,6 +90,7 @@ enum gf_mask { GF_CALL_NOTHROW = 1 << 4, GF_CALL_ALLOCA_FOR_VAR = 1 << 5, GF_CALL_INTERNAL = 1 << 6, + GF_CALL_NO_ABNORMAL_GOTO = 1 << 7, GF_OMP_PARALLEL_COMBINED = 1 << 0, GF_OMP_FOR_KIND_MASK = (1 << 2) - 1, GF_OMP_FOR_KIND_FOR = 0, @@ -2449,7 +2450,28 @@ gimple_call_internal_p (const_gimple gs) } -/* Return the target of internal call GS. */ +/* If NO_ABNORMAL_GOTO_P is true mark GIMPLE_CALL S as not having any + abnormal goto effect. It doesn't exclude EH. */ +static inline void +gimple_call_set_no_abnormal_goto (gimple s, bool no_abnormal_goto_p) +{ + GIMPLE_CHECK (s, GIMPLE_CALL); + if (no_abnormal_goto_p) + s->subcode |= GF_CALL_NO_ABNORMAL_GOTO; + else + s->subcode &= ~GF_CALL_NO_ABNORMAL_GOTO; +} + +/* Return true if call GS calls an func without abnormal goto. Such call + could be a stmt in the middle of a bb. */ + +static inline bool +gimple_call_no_abnormal_goto_p (const_gimple gs) +{ + GIMPLE_CHECK (gs, GIMPLE_CALL); + return (gs->subcode & GF_CALL_NO_ABNORMAL_GOTO) != 0; +} + static inline enum internal_fn gimple_call_internal_fn (const_gimple gs) Index: tree-cfg.c =================================================================== --- tree-cfg.c (revision 212442) +++ tree-cfg.c (working copy) @@ -2318,15 +2318,11 @@ call_can_make_abnormal_goto (gimple t) && !cfun->calls_setjmp) return false; - /* Likewise if the call has no side effects. */ - if (!gimple_has_side_effects (t)) + /* If the call has been marked as having no abnormal goto. */ + if (gimple_call_no_abnormal_goto_p (t)) return false; - - /* Likewise if the called function is leaf. */ - if (gimple_call_flags (t) & ECF_LEAF) - return false; - - return true; + else + return true; } @@ -8485,6 +8481,11 @@ execute_fixup_cfg (void) } } + if ((flags & (ECF_CONST | ECF_PURE) + && !(flags & ECF_LOOPING_CONST_OR_PURE)) + || (flags & ECF_LEAF)) + gimple_call_set_no_abnormal_goto (stmt, true); + if (flags & ECF_NORETURN && fixup_noreturn_call (stmt)) todo |= TODO_cleanup_cfg; Index: testsuite/gcc.dg/pr61776.c =================================================================== --- testsuite/gcc.dg/pr61776.c (revision 0) +++ testsuite/gcc.dg/pr61776.c (revision 0) @@ -0,0 +1,27 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fprofile-generate" } */ + +#include + +int cond1, cond2; + +int goo() __attribute__((noinline)); + +int goo() { + if (cond1) + return 1; + else + return 2; +} + +jmp_buf env; +int foo() { + int a; + + setjmp(env); + if (cond2) + a = goo(); + else + a = 3; + return a; +} Index: testsuite/gcc.dg/lto/pr60449_1.c =================================================================== --- testsuite/gcc.dg/lto/pr60449_1.c (revision 0) +++ testsuite/gcc.dg/lto/pr60449_1.c (revision 0) @@ -0,0 +1,76 @@ +extern int printf (const char *__restrict __format, ...); +typedef long int __time_t; +typedef long int __suseconds_t; +struct timeval + { + __time_t tv_sec; + __suseconds_t tv_usec; + }; +struct timezone + { + int tz_minuteswest; + int tz_dsttime; + }; +typedef struct timezone *__restrict __timezone_ptr_t; +extern int gettimeofday (struct timeval *__restrict __tv, + __timezone_ptr_t __tz) __attribute__ ((__nothrow__ , __leaf__)) __attribute__ ((__nonnull__ (1))); + +typedef long int __jmp_buf[8]; +typedef struct + { + unsigned long int __val[(1024 / (8 * sizeof (unsigned long int)))]; + } __sigset_t; +struct __jmp_buf_tag + { + __jmp_buf __jmpbuf; + int __mask_was_saved; + __sigset_t __saved_mask; + }; +typedef struct __jmp_buf_tag jmp_buf[1]; + +extern int setjmp (jmp_buf __env) __attribute__ ((__nothrow__)); +extern void longjmp (struct __jmp_buf_tag __env[1], int __val) + __attribute__ ((__nothrow__)) __attribute__ ((__noreturn__)); + +extern int bar (void); + +int __attribute__ ((noinline, noclone)) +get_input (void) +{ + return 0; +} + +static jmp_buf buf; + +int foo (void) +{ + if (get_input ()) + longjmp(buf, 1); + return 0; +} + +volatile int z; + + +int main (void) +{ + struct timeval tv; + struct timezone tz; + + bar(); + if (setjmp (buf)) + return 1; + + if (!get_input ()) + { + gettimeofday (&tv, &tz); + z = 0; + printf ("This is from main %i\n", tz.tz_dsttime); + } + + foo (); + bar (); + bar (); + + return 0; +} Index: testsuite/gcc.dg/lto/pr60449_0.c =================================================================== --- testsuite/gcc.dg/lto/pr60449_0.c (revision 0) +++ testsuite/gcc.dg/lto/pr60449_0.c (revision 0) @@ -0,0 +1,30 @@ +/* { dg-lto-do link } */ + +extern int printf (const char *__restrict __format, ...); +typedef long int __time_t; +typedef long int __suseconds_t; + +struct timeval + { + __time_t tv_sec; + __suseconds_t tv_usec; + }; + +struct timezone + { + int tz_minuteswest; + int tz_dsttime; + }; +typedef struct timezone *__restrict __timezone_ptr_t; + +extern int gettimeofday (struct timeval *__restrict __tv, __timezone_ptr_t __tz); + +int bar (void) +{ + struct timeval tv; + struct timezone tz; + + gettimeofday (&tv, &tz); + printf ("This is from bar %i\n", tz.tz_dsttime); + return 5; +}