From patchwork Tue Sep 6 21:31:52 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Steven Bosscher X-Patchwork-Id: 113670 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]) by ozlabs.org (Postfix) with SMTP id B144AB6F77 for ; Wed, 7 Sep 2011 07:32:11 +1000 (EST) Received: (qmail 14934 invoked by alias); 6 Sep 2011 21:32:07 -0000 Received: (qmail 14926 invoked by uid 22791); 6 Sep 2011 21:32:06 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, TW_CF X-Spam-Check-By: sourceware.org Received: from mail-yx0-f175.google.com (HELO mail-yx0-f175.google.com) (209.85.213.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 06 Sep 2011 21:31:53 +0000 Received: by yxl11 with SMTP id 11so3290882yxl.20 for ; Tue, 06 Sep 2011 14:31:52 -0700 (PDT) MIME-Version: 1.0 Received: by 10.101.3.40 with SMTP id f40mr3989529ani.89.1315344712285; Tue, 06 Sep 2011 14:31:52 -0700 (PDT) Received: by 10.100.78.8 with HTTP; Tue, 6 Sep 2011 14:31:52 -0700 (PDT) In-Reply-To: References: <4E65E463.6070004@codesourcery.com> Date: Tue, 6 Sep 2011 23:31:52 +0200 Message-ID: Subject: Re: [PATCH] check_cfg assert fix From: Steven Bosscher To: Maxim Kuvyrkov Cc: Tom de Vries , Vladimir Makarov , "gcc-patches@gcc.gnu.org" X-IsSubscribed: yes 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 On Tue, Sep 6, 2011 at 8:00 PM, Maxim Kuvyrkov wrote: > > On 7/09/2011, at 3:13 AM, Steven Bosscher wrote: > >> On Tue, Sep 6, 2011 at 11:14 AM, Tom de Vries wrote: >>> Hi, >>> >>> During testing the approved-for-commit middle-end patch for bug 43864 on ARM, I >>> ran into a gcc.dg/torture/pr46068.c ICE. >>> >>> The following assert in haifa-sched.c:check_cfg triggered: >>> ... >>>                  else if (any_condjump_p (head)) >>>                    gcc_assert (/* Usual case.  */ >>>                                (EDGE_COUNT (bb->succs) > 1 >>>                                 && !BARRIER_P (NEXT_INSN (head))) >>>                                /* Or jump to the next instruction.  */ >>>                                || (EDGE_COUNT (bb->succs) == 1 >>>                                    && (BB_HEAD (EDGE_I (bb->succs, 0)->dest) >>>                                        == JUMP_LABEL (head)))); >>> > ... >>> Bootstrapped and reg-tested on x86_64 and build and reg-tested on arm. >>> >>> OK for trunk? >> >> No. If the conditional return is not taken, you should not fall into a >> barrier. It looks like the CFG somehow got corrupted, and if that's >> indeed the case then your patch just papers over the problem. That >> follows after the barrier? > > Initially, I thought so too, that is why I wrote the above assert in the first place.  However, __builtin_unreachable() adds a whole new spin on where a BARRIER can occur; after all, this built-in expands directly to BARRIER. > > Before Tom's optimization the fall-through path of conditional jump was followed by an unconditional jump to a label directly followed by a barrier.  Tom's patch removed the middle-man in the face of unconditional jump so that the barrier now follows a conditional jump.  This is very odd, but, given the initial test case, is a valid case. Well, shouldn't the assert be changed then to handle !onlyjump_p and returnjump_p cases separately? Tom's patch removes a valid check for all but these corner cases. Perhaps something like this: Ciao! Steven Index: haifa-sched.c =================================================================== --- haifa-sched.c (revision 178601) +++ haifa-sched.c (working copy) @@ -6071,7 +6071,10 @@ check_cfg (rtx head, rtx tail) /* Or jump to the next instruction. */ || (EDGE_COUNT (bb->succs) == 1 && (BB_HEAD (EDGE_I (bb->succs, 0)->dest) - == JUMP_LABEL (head)))); + == JUMP_LABEL (head))) + /* Or the jump is not just a jump. */ + || (!onlyjump_p (head) + || returnjump_p (head))); } if (BB_END (bb) == head) {