From patchwork Tue May 16 09:10:58 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Hua X-Patchwork-Id: 762846 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 3wRsBl5Rrwz9s5L for ; Tue, 16 May 2017 19:11:26 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="eBh6V5UJ"; dkim-atps=neutral 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:from:date:message-id :subject:to:cc:content-type; q=dns; s=default; b=ENAAkrx1F1hHoQ8 WeMwH/mGKwbJ2/g4CLWa2YBv/HnX3XNsCsKxBIOd8XnbHDIWcDqzaN4VZ9PAbFhR zVOV6hwno/MtMF9C6hziB1I682IrOABjxzvzMPDhR3lCjpZwOgbDO4wRq3UfPw2e HnFcLrh1sktc5yOBIO8AadQBOA/Y= 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:from:date:message-id :subject:to:cc:content-type; s=default; bh=f/m3K4ryyusAK3lw4vR4K j0gVh8=; b=eBh6V5UJzHtckFiQP5mx304CefvJGthNma/1587/6/t4JWZ5+39nk MM2HnqE3699ksuK0KUdfL3XY0anhRxgYILssADIMSyQrEZpH/6+6ULaemCOTSwKE vlqDZTEx2/BidfGBvUqM7HhLfaPKgEgzaekSe2nIoFQ8QZi3wV0WD8= Received: (qmail 45237 invoked by alias); 16 May 2017 09:11:05 -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 45227 invoked by uid 89); 16 May 2017 09:11:03 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-23.9 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-qk0-f175.google.com Received: from mail-qk0-f175.google.com (HELO mail-qk0-f175.google.com) (209.85.220.175) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 16 May 2017 09:10:57 +0000 Received: by mail-qk0-f175.google.com with SMTP id a72so121594702qkj.2 for ; Tue, 16 May 2017 02:11:00 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=LP1t7YDzhND4M3eBZUz6FeyQOXOz0Oa27NwnBdZCcrw=; b=CCVhBWlJ6jGjwy2HpFwOEaAbBQX7x6V58wfZO7GNKyrC17264Ho0YbHCjWScHojFuU K9G7ZAD1fNxT077JdDs69DoGMRFEPA9A4KuFdKaJ74+h++OL5X6MyGAkw1K17MBifaci CoKvszH1Uw5lA8zZGbDy7eFFl6RDrsmQaXi1jhxKK+xqAF7OQDAeUpCiD2qfWub2hkMr OpGp41F5zsdsCPFzu1Qcau+NkZAQAdhp5QSNa92t8Iqz8eXCWP2UhqUpQtPTuG8WG/t1 CeawDPzPLMYwb8qARYjHYCLbTo3lgDFG1fl2yPywkvK/hkLiM2CJEN4+L1feCjqRqxsV PplA== X-Gm-Message-State: AODbwcDNqRJk2Vexr5QsO/QlvFuipwDjMwYumNYwED1a0J03iRJGOsus MNDkXp9gP2dYvKC2+IiIFs3S4ZUjZg== X-Received: by 10.55.22.79 with SMTP id g76mr9390912qkh.280.1494925859036; Tue, 16 May 2017 02:10:59 -0700 (PDT) MIME-Version: 1.0 Received: by 10.55.160.194 with HTTP; Tue, 16 May 2017 02:10:58 -0700 (PDT) In-Reply-To: <699a90aa-ba04-62ce-9d40-13e0ddf838b3@redhat.com> References: <699a90aa-ba04-62ce-9d40-13e0ddf838b3@redhat.com> From: Paul Hua Date: Tue, 16 May 2017 17:10:58 +0800 Message-ID: Subject: Re: Fix minor reorg.c bug affecting MIPS targets To: Jeff Law Cc: gcc-patches X-IsSubscribed: yes Hi: There are new failures between r248067 and r248036 on mips64el-unknown-linux-gnu. ERROR: gcc.target/mips/reorgbug-1.c -O0 : Unrecognised option: -O2 for " dg-options 1 "-O2 -msoft-float -mips2" " UNRESOLVED: gcc.target/mips/reorgbug-1.c -O0 : Unrecognised option: -O2 for " dg-options 1 "-O2 -msoft-float -mips2" " ERROR: gcc.target/mips/reorgbug-1.c -O1 : Unrecognised option: -O2 for " dg-options 1 "-O2 -msoft-float -mips2" " UNRESOLVED: gcc.target/mips/reorgbug-1.c -O1 : Unrecognised option: -O2 for " dg-options 1 "-O2 -msoft-float -mips2" " ERROR: gcc.target/mips/reorgbug-1.c -O2 -flto -fno-use-linker-plugin -flto-partition=none : Unrecognised option: -O2 for " dg-options 1 "-O2 -msoft-float -mips2" " UNRESOLVED: gcc.target/mips/reorgbug-1.c -O2 -flto -fno-use-linker-plugin -flto-partition=none : Unrecognised option: -O2 for " dg-options 1 "-O2 -msoft-float -mips2" " ERROR: gcc.target/mips/reorgbug-1.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects : Unrecognised option: -O2 for " dg-options 1 "-O2 -msoft-float -mips2" " UNRESOLVED: gcc.target/mips/reorgbug-1.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects : Unrecognised option: -O2 for " dg-options 1 "-O2 -msoft-float -mips2" " ERROR: gcc.target/mips/reorgbug-1.c -O2 : Unrecognised option: -O2 for " dg-options 1 "-O2 -msoft-float -mips2" " UNRESOLVED: gcc.target/mips/reorgbug-1.c -O2 : Unrecognised option: -O2 for " dg-options 1 "-O2 -msoft-float -mips2" " ERROR: gcc.target/mips/reorgbug-1.c -O3 -g : Unrecognised option: -O2 for " dg-options 1 "-O2 -msoft-float -mips2" " UNRESOLVED: gcc.target/mips/reorgbug-1.c -O3 -g : Unrecognised option: -O2 for " dg-options 1 "-O2 -msoft-float -mips2" " ERROR: gcc.target/mips/reorgbug-1.c -Os : Unrecognised option: -O2 for " dg-options 1 "-O2 -msoft-float -mips2" " UNRESOLVED: gcc.target/mips/reorgbug-1.c -Os : Unrecognised option: -O2 for " dg-options 1 "-O2 -msoft-float -mips2" " I don't know why? just delete the "-O2" options in dg-options, then the test passed. config info:Compiler version: 8.0.0 20170515 (experimental) (gcc trunk r248067 mips64el o32 n32 n64) Platform: mips64el-unknown-linux-gnu configure flags: --prefix=/home/xuchenghua/toolchain/gcc-trunk-r248067 --enable-bootstrap --enable-shared --enable-threads=posix --enable-checking=release --enable-multilib--with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-linker-build-id --enable-languages=c,c++,fortran --enable-plugin --enable-initfini-array --disable-libgcj --with-arch=mips64r2 --with-abi=64 --with-multilib-list=32,n32,64 --enable-gnu-indirect-function --with-long-double-128 --build=mips64el-unknown-linux --target=mips64el-unknown-linux --with-pkgversion='gcc trunk r248067 mips64el o32 n32 n64' --disable-werror paul On Tue, May 16, 2017 at 1:22 AM, Jeff Law wrote: > > > There's a subtle bug in reorg.c's relax_delay_slots that my tester tripped > this weekend. Not sure what changed code generation wise as the affected > port built just fine last week. But it is what is is. > > > > Assume before this code we've set TARGET_LABEL to the code_label associated > with DELAY_JUMP_INSN (which is what we want)... > > > > /* If the first insn at TARGET_LABEL is redundant with a previous > insn, redirect the jump to the following insn and process again. > We use next_real_insn instead of next_active_insn so we > don't skip USE-markers, or we'll end up with incorrect > liveness info. */ > > [ ... ] > > /* Similarly, if it is an unconditional jump with one insn in its > delay list and that insn is redundant, thread the jump. */ > rtx_sequence *trial_seq = > trial ? dyn_cast (PATTERN (trial)) : NULL; > if (trial_seq > && trial_seq->len () == 2 > && JUMP_P (trial_seq->insn (0)) > && simplejump_or_return_p (trial_seq->insn (0)) > && redundant_insn (trial_seq->insn (1), insn, vNULL)) > { > target_label = JUMP_LABEL (trial_seq->insn (0)); > if (ANY_RETURN_P (target_label)) > target_label = find_end_label (target_label); > > if (target_label > && redirect_with_delay_slots_safe_p (delay_jump_insn, > target_label, insn)) > { > update_block (trial_seq->insn (1), insn); > reorg_redirect_jump (delay_jump_insn, target_label); > next = insn; > continue; > } > } > > /* See if we have a simple (conditional) jump that is useless. */ > if (! INSN_ANNULLED_BRANCH_P (delay_jump_insn) > && ! condjump_in_parallel_p (delay_jump_insn) > && prev_active_insn (as_a (target_label)) == insn > > Now assume that we get into the TRUE arm of that first conditional which > sets a new value for TARGET_LABEL. Normally when this happens in > relax_delay_slots we're going to unconditionally continue. But as we can > see there's an inner conditional and if we don't get into its true arm, then > we'll pop out a nesting level and execute the second outer IF. > > That second outer IF assumes that TARGET_LABEL still points to the code > label associated with DELAY_JUMP_INSN. Opps. In my particular case it was > NULL and caused an ICE. But I could probably construct a case where it > pointed to a real label and could result in incorrect code generation. > > The fix is pretty simple. Just creating a new variable (to avoid -Wshadow) > inside that first outer IF is sufficient. > > Tested by building the affected target (mipsisa64r2el-elf) through newlib. > > Installed on the trunk. > > jeff > > diff --git a/gcc/ChangeLog b/gcc/ChangeLog > index 8cceb247a85..18b6ed59c73 100644 > --- a/gcc/ChangeLog > +++ b/gcc/ChangeLog > @@ -1,5 +1,8 @@ > 2017-05-15 Jeff Law > > + * reorg.c (relax_delay_slots): Create a new variable to hold > + the temporary target rather than clobbering TARGET_LABEL. > + > * config/tilegx/tilegx.c (tilegx_expand_unaligned_load): Add > missing argument to extract_bit_field call. > * config/tilepro/tilepro.c (tilepro_expand_unaligned_load): > Likewise. > diff --git a/gcc/reorg.c b/gcc/reorg.c > index 85ef7d6880c..1a6fd86e286 100644 > --- a/gcc/reorg.c > +++ b/gcc/reorg.c > @@ -3351,16 +3351,16 @@ relax_delay_slots (rtx_insn *first) > && simplejump_or_return_p (trial_seq->insn (0)) > && redundant_insn (trial_seq->insn (1), insn, vNULL)) > { > - target_label = JUMP_LABEL (trial_seq->insn (0)); > - if (ANY_RETURN_P (target_label)) > - target_label = find_end_label (target_label); > + rtx temp_label = JUMP_LABEL (trial_seq->insn (0)); > + if (ANY_RETURN_P (temp_label)) > + temp_label = find_end_label (temp_label); > > - if (target_label > + if (temp_label > && redirect_with_delay_slots_safe_p (delay_jump_insn, > - target_label, insn)) > + temp_label, insn)) > { > update_block (trial_seq->insn (1), insn); > - reorg_redirect_jump (delay_jump_insn, target_label); > + reorg_redirect_jump (delay_jump_insn, temp_label); > next = insn; > continue; > } > diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog > index f198fc6b42b..9fb8c8598b8 100644 > --- a/gcc/testsuite/ChangeLog > +++ b/gcc/testsuite/ChangeLog > @@ -1,3 +1,7 @@ > +2017-05-15 Jeff Law > + > + * gcc.target/mips/reorgbug-1.c: New test. > + > 2017-05-15 Pierre-Marie de Rodat > > * gnat.dg/specs/pack13.ads: New test. > diff --git a/gcc/testsuite/gcc.target/mips/reorgbug-1.c > b/gcc/testsuite/gcc.target/mips/reorgbug-1.c > new file mode 100644 > index 00000000000..b820a2b5df1 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/mips/reorgbug-1.c > @@ -0,0 +1,39 @@ > +/* { dg-options "-O2 -msoft-float -mips2" } */ > + > +typedef long int __int32_t; > +typedef long unsigned int __uint32_t; > +typedef union > +{ > + double value; > + struct > + { > + __uint32_t msw; > + __uint32_t lsw; > + } > + parts; > +} > +ieee_double_shape_type; > +double > +__ieee754_fmod (double x, double y, int z, int xx) > +{ > + __int32_t n, hx, hy, hz, ix, iy, sx, i; > + __uint32_t lx, ly, lz; > + ieee_double_shape_type ew_u; > + ew_u.value = (x); > + (lx) = ew_u.parts.lsw; > + ew_u.value = (y); > + (hy) = ew_u.parts.msw; > + (ly) = ew_u.parts.lsw; > + if (hy == 0 || hx >= 0x7ff00000) > + return (x * y); > + if (z) > + { > + if ((hx < hy) || (lx < ly)) > + return x; > + lz = lx - ly; > + lx = lz + lz; > + } > + ieee_double_shape_type iw_u; > + iw_u.parts.lsw = (lx); > + return iw_u.value; > +} > diff --git a/gcc/testsuite/gcc.target/mips/reorgbug-1.c b/gcc/testsuite/gcc.target/mips/reorgbug-1.c index b820a2b..9537d21 100644 --- a/gcc/testsuite/gcc.target/mips/reorgbug-1.c +++ b/gcc/testsuite/gcc.target/mips/reorgbug-1.c @@ -1,4 +1,4 @@ -/* { dg-options "-O2 -msoft-float -mips2" } */ +/* { dg-options "-msoft-float -mips2" } */ typedef long int __int32_t; typedef long unsigned int __uint32_t;