From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18984 invoked by alias); 7 Sep 2015 08:48:36 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 18964 invoked by uid 89); 7 Sep 2015 08:48:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mailapp01.imgtec.com Received: from mailapp01.imgtec.com (HELO mailapp01.imgtec.com) (195.59.15.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 07 Sep 2015 08:48:33 +0000 Received: from KLMAIL01.kl.imgtec.org (unknown [192.168.5.35]) by Websense Email Security Gateway with ESMTPS id 21A4DEE7B1541; Mon, 7 Sep 2015 09:48:28 +0100 (IST) Received: from LEMAIL01.le.imgtec.org (192.168.152.62) by KLMAIL01.kl.imgtec.org (192.168.5.35) with Microsoft SMTP Server (TLS) id 14.3.195.1; Mon, 7 Sep 2015 09:48:30 +0100 Received: from LEMAIL01.le.imgtec.org ([fe80::5ae:ee16:f4b9:cda9]) by LEMAIL01.le.imgtec.org ([fe80::5ae:ee16:f4b9:cda9%17]) with mapi id 14.03.0210.002; Mon, 7 Sep 2015 09:48:29 +0100 From: Matthew Fortune To: Robert Suchanek , "Catherine_Moore@mentor.com" CC: "gcc-patches@gcc.gnu.org" Subject: RE: [PATCH] [MIPS] Fix wrong instruction in the delay slot Date: Mon, 07 Sep 2015 09:14:00 -0000 Message-ID: <6D39441BF12EF246A7ABCE6654B0235321267077@LEMAIL01.le.imgtec.org> References: In-Reply-To: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-IsSubscribed: yes X-SW-Source: 2015-09/txt/msg00411.txt.bz2 Robert Suchanek > IMO, the fix is to recognize the empty basic block that has a code_label > followed by a barrier (ignoring notes and debug_insns), forbid going > beyond the barrier if the empty block is found in > skip_consecutive_labels () and first_active_target_insn (). I can't approve this but I agree that the delay slot filler should not be allowed to look past barriers to fill delay slots (especially if moving a memory operation). The redundant/no-op branch here is also annoying but the delay slot issue does seem to be an independent problem. I've seen lots of pointless branches being generated by GCC that get all the way though to emitting code. Perhaps this is just one of several reasons for generating either always or never taken conditional branches. They end up as things like BEQ $4, $4 or BNE $4, $4 which MIPS R6 converts to 'B' or 'nothing' respectively as I have not had time to track their origin. Thanks, Matthew >=20 > The patch was cross tested on mips-img-linux-gnu and sparc-linux-gnu. >=20 >=20 > Regards, > Robert >=20 > gcc/ > * reorg.c (label_with_barrier_p): New function. > (skip_consecutive_labels): Use it. Don't skip the label if an > empty > block is found. > (first_active_target_insn): Likewise. Don't ignore the empty > block when searching for the next active instruction. >=20 > gcc/testsuite > * gcc.target/mips/builtin-unreachable-bug-1.c: New test. > --- > gcc/reorg.c | 28 +++++++ > .../gcc.target/mips/builtin-unreachable-bug-1.c | 90 > ++++++++++++++++++++++ > 2 files changed, 118 insertions(+) > create mode 100644 gcc/testsuite/gcc.target/mips/builtin-unreachable- > bug-1.c >=20 > diff --git a/gcc/reorg.c b/gcc/reorg.c > index cdaa60c..269f666 100644 > --- a/gcc/reorg.c > +++ b/gcc/reorg.c > @@ -145,6 +145,30 @@ along with GCC; see the file COPYING3. If not see > These functions are now only used here in reorg.c, and have > therefore > been moved here to avoid inadvertent misuse elsewhere in the > compiler. */ >=20 > +/* Return true iff a LABEL is followed by a BARRIER. Ignore notes and > debug > + instructions. */ > + > +static bool > +label_with_barrier_p (rtx_insn *label) > +{ > + bool empty_bb =3D true; > + > + if (GET_CODE (label) !=3D CODE_LABEL) > + empty_bb =3D false; > + else > + label =3D NEXT_INSN (label); > + > + while (!BARRIER_P (label) && empty_bb) > + { > + if (!(DEBUG_INSN_P (label) > + || NOTE_P (label))) > + empty_bb =3D false; > + label =3D NEXT_INSN (label); > + } > + > + return empty_bb; > +} > + > /* Return the last label to mark the same position as LABEL. Return > LABEL > itself if it is null or any return rtx. */ >=20 > @@ -159,6 +183,8 @@ skip_consecutive_labels (rtx label_or_return) > rtx_insn *label =3D as_a (label_or_return); >=20 > for (insn =3D label; insn !=3D 0 && !INSN_P (insn); insn =3D NEXT_INSN > (insn)) > + if (LABEL_P (insn) && label_with_barrier_p (insn)) > + break; > if (LABEL_P (insn)) > label =3D insn; >=20 > @@ -267,6 +293,8 @@ first_active_target_insn (rtx insn) { > if (ANY_RETURN_P (insn)) > return insn; > + if (LABEL_P (insn) && label_with_barrier_p (as_a > (insn))) > + return NULL_RTX; > return next_active_insn (as_a (insn)); } >=20 >=20 > diff --git a/gcc/testsuite/gcc.target/mips/builtin-unreachable-bug-1.c > b/gcc/testsuite/gcc.target/mips/builtin-unreachable-bug-1.c > new file mode 100644 > index 0000000..65eb9a3 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/mips/builtin-unreachable-bug-1.c > @@ -0,0 +1,90 @@ > +/* { dg-options "-mcompact-branches=3Dnever -G0 -mno-abicalls" } */ > +/* { dg-final { scan-assembler-not > +"beq\t\\\$4,\\\$0,\\\$L\[0-9\]+\n\tlw\t\\\$4,%lo\\(mips_cm_is64\\)\\(\\ > +\$4\\)" } } */ > + > +enum > +{ > + CPU_R4700, > + CPU_R5000, > + CPU_R5500, > + CPU_NEVADA, > + CPU_RM7000, > + CPU_SR71000, > + CPU_4KC, > + CPU_4KEC, > + CPU_4KSC, > + CPU_24K, > + CPU_34K, > + CPU_1004K, > + CPU_74K, > + CPU_ALCHEMY, > + CPU_PR4450, > + CPU_BMIPS32, > + CPU_BMIPS3300, > + CPU_BMIPS5000, > + CPU_JZRISC, > + CPU_M14KC, > + CPU_M14KEC, > + CPU_INTERAPTIV, > + CPU_P5600, > + CPU_PROAPTIV, > + CPU_1074K, > + CPU_M5150, > + CPU_I6400, > + CPU_R3000, > + CPU_5KC, > + CPU_5KE, > + CPU_20KC, > + CPU_25KF, > + CPU_SB1, > + CPU_SB1A, > + CPU_XLP, > + CPU_QEMU_GENERIC > +}; > + > +struct cpuinfo_mips > +{ > + long options; > + int isa_level; > +} cpu_data[1]; > + > +struct thread_info > +{ > + int cpu; > +}; > + > +int a, b, c, d, e, mips_cm_is64; > + > +static int __attribute__ ((__cold__)) > +mips_sc_probe_cm3 () > +{ > + struct thread_info *info; > + struct cpuinfo_mips *c =3D &cpu_data[info->cpu]; > + if (mips_cm_is64) > + c->options =3D 0; > + return 1; > +} > + > +int > +mips_sc_probe () > +{ > + struct cpuinfo_mips ci =3D cpu_data[c]; > + if (mips_cm_is64) > + __asm__("" ::: "memory"); > + if (d) > + return mips_sc_probe_cm3 (); > + if (ci.isa_level) > + switch (a) > + { > + case CPU_QEMU_GENERIC: > + case CPU_I6400: > + case CPU_R3000: > + case CPU_NEVADA: > + case CPU_BMIPS3300: > + break; > + default: > + __builtin_unreachable (); > + } > + switch (a) > + case CPU_R3000: > + e =3D b; > +} > -- > 2.4.5