From patchwork Thu Sep 25 17:44:30 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Uros Bizjak X-Patchwork-Id: 393417 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 38FB614008B for ; Fri, 26 Sep 2014 03:44:44 +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=SpseIc/LbzLjVc7Qrp TZSNIghswPKIV8Ds9ncyJdC90sxgPYNiWWd5Xnkl6dRWL75LBhdKQQKSHZxFrFIz HuebgfejezaBC0TpaLsyE7j8d/P3x7auh2q4x1OruWoITXnMJoAEM5rFmoUmVvG2 OAVsTUD6pv5+lZ3uCu583m2oA= 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=pYlwu5DnaJiZZppWxSecFv3S cHw=; b=yGAPIWJlhhB/8TwLmHQQg60tvbQMVCpSTbzzyptEQgBh8u/wmzthiIKz lk9CCtb33KKXpWP6+U0XKvEPXETYqEYX+a8fGrqtzTW8G1ZgcaIZ5P59jQY7lakv rCtTWFUFZ73aicTzW9wCV3g25xt6k+XB7Otciq5oytksdgM4EKc= Received: (qmail 4329 invoked by alias); 25 Sep 2014 17:44:37 -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 4316 invoked by uid 89); 25 Sep 2014 17:44:36 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-la0-f43.google.com Received: from mail-la0-f43.google.com (HELO mail-la0-f43.google.com) (209.85.215.43) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Thu, 25 Sep 2014 17:44:34 +0000 Received: by mail-la0-f43.google.com with SMTP id gb8so3502142lab.2 for ; Thu, 25 Sep 2014 10:44:30 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.112.55.7 with SMTP id n7mr14435038lbp.16.1411667070596; Thu, 25 Sep 2014 10:44:30 -0700 (PDT) Received: by 10.152.1.193 with HTTP; Thu, 25 Sep 2014 10:44:30 -0700 (PDT) In-Reply-To: <54245058.5020709@redhat.com> References: <54245058.5020709@redhat.com> Date: Thu, 25 Sep 2014 19:44:30 +0200 Message-ID: Subject: Re: [RFC PATCH, RTL]: Fix PR63348, gcc.dg/pr43670.c fail -fcompare-debug on MIPS From: Uros Bizjak To: Jeff Law Cc: "gcc-patches@gcc.gnu.org" On Thu, Sep 25, 2014 at 7:26 PM, Jeff Law wrote: > On 09/24/14 13:39, Uros Bizjak wrote: >> >> Hello! >> >> The failure was caused by barrier detection code, which failed to >> detect barrier after call insn was to be split when >> NOTE_CALL_ARG_LOCATION was present. This problem caused >> -fcompare-debug failure. >> >> Digging a bit deeped, and as hinted in the PR, the handling of >> barriers in try_split seems to be broken. The code is emitting extra >> barrier for non-debug compiles, but it "forgots" to remove the >> existing one, leading to duplicated barriers. The barrier is not >> detected at all for debug build. >> >> I have removed special handling of barriers here (also, the comment in >> removed code was not helpful at all), and this solved -fcompare-debug >> failure. >> >> The patch was also bootstrapped and regression tested on >> x86_64-linux-gnu {,-m32} which in -m32 mode splits x87 FP jump insns, >> and there were no regressions. However, I am not too familiar with >> rtl-optimization part and I am not confident that this code surgery is >> fully correct, so this is the reason for RFC status of the patch. >> >> 2014-09-24 Uros Bizjak >> >> PR rtl-optimization/63348 >> * emit-rtl.c (try_split): Do not emit extra barrier. > > Good grief, the code you're removing pre-dates any version control we have. > ie, it's in the first revision of emit-rtl.c from 1992. Egad. > > It's going to be a hell of a time figuring out why that code exists in the > first place. I don't like removing code if we don't know why the code > exists... Any reason you picked that route rather than looking forward > through the NOTEs to see if they're followed by a suitable BARRIER? I have tried with alternative patch that just skipped the NOTE: --cut here-- --cut here-- and resulted in: (call_insn 184 190 185 (parallel [ (call (mem:SI (reg:SI 25 $25 [217]) [0 S4 A32]) (const_int 16 [0x10])) (clobber (reg:SI 31 $31)) (clobber (reg:SI 28 $28)) ]) pr43670.c:29 595 {call_split} (expr_list:REG_NORETURN (const_int 0 [0]) (nil)) (expr_list (use (reg:SI 79 $fakec)) (expr_list (use (reg:SI 28 $28)) (nil)))) (barrier 185 184 175) (note 175 185 130 (expr_list:REG_DEP_TRUE (concat:SI (pc) (unspec:SI [ (reg:SI 28 $28) (const:SI (unspec:SI [ (symbol_ref:SI ("abort") [flags 0x41] ) ] 227)) (reg:SI 79 $fakec) ] UNSPEC_LOAD_CALL)) (nil)) NOTE_INSN_CALL_ARG_LOCATION) (barrier 130 175 174) I have noticed that the barrier is always there, since without -g, we have: (call_insn 76 82 77 (parallel [ (call (mem:SI (reg:SI 25 $25 [217]) [0 S4 A32]) (const_int 16 [0x10])) (clobber (reg:SI 31 $31)) (clobber (reg:SI 28 $28)) ]) pr43670.c:29 595 {call_split} (expr_list:REG_NORETURN (const_int 0 [0]) (nil)) (expr_list (use (reg:SI 79 $fakec)) (expr_list (use (reg:SI 28 $28)) (nil)))) (barrier 77 76 37) (barrier 37 77 40) and considering the fact that the code didn't process barriers correctly with -g, I simply removed the emission. The - probably stalled - comment was not helpful at all. Uros. Index: emit-rtl.c =================================================================== --- emit-rtl.c (revision 215606) +++ emit-rtl.c (working copy) @@ -3622,6 +3622,10 @@ try_split (rtx pat, rtx uncast_trial, int last) int njumps = 0; rtx call_insn = NULL_RTX; + if (after && NOTE_P (after) + && NOTE_KIND (after) == NOTE_INSN_CALL_ARG_LOCATION) + after = NEXT_INSN (after); + /* We're not good at redistributing frame information. */ if (RTX_FRAME_RELATED_P (trial)) return trial;