This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] gcc: config: tilegx: Reserve prev insn when delete useless insn
From: Chen Gang <chengang@emindsoft.com.cn>
On Sat, Dec 10, 2016 at 03:38:10PM +0000, Bernd Edlinger wrote:
> On 12/10/16 12:23, chengang@emindsoft.com.cn wrote:
> > From: Chen Gang <chengang@emindsoft.com.cn>
> >
> > When check bundle, gcc may still need modify the prev insn. Original
> > implementation will lose the prev insn.
> >
> > Also correct the coding styles of relate code.
> >
> > 2016-12-10 Chen Gang <gang.chen.5i5j@gmail.com>
> >
> > gcc/
> > PR target/78222
> > * tilegx.c (tilegx_gen_bundle): Reserve prev insn when delete
> > useless insn.
>
> I think this was already fixed by Walt:
>
> ------------------------------------------------------------------------
> r242617 | walt | 2016-11-19 03:34:17 +0100 (Sat, 19 Nov 2016) | 5 lines
> Changed paths:
> M /trunk/gcc/ChangeLog
> M /trunk/gcc/config/tilegx/tilegx.c
>
> TILE-Gx: Fix bundling when encountering consecutive barriers.
>
> * config/tilegx/tilegx.c (tilegx_gen_bundles): Preserve
> end-of-bundle marker for consecutive barriers.
>
Oh, yes, Good news to me.
>
> But the formatting here is still odd, and should be fixed:
> TAB usage, single statements in braces, { not in a line of its own.
>
Yes.
> I am however not sure about this statement:
>
> /* Never wrap {} around inline asm. */
> if (GET_CODE (PATTERN (insn)) != ASM_INPUT)
>
> ... because this does only exclude asm(""); that is basic asm with
> empty assembler string. To exclude all other forms of asm statements
> that are hidden in PARALLEL constructs we would need:
>
> /* Never wrap {} around inline asm. */
> if (GET_CODE (PATTERN (insn)) != ASM_INPUT
> && asm_noperands (PATTERN (insn)) < 0)
>
>
> I think this if-condition is probably unnecessary, because it does
> apparently not create any problems although it is completely broken.
>
For me, we need not touch it, since we are not quite sure about it. And
welcome any other related members' idea for it.
All together, I guess, I can leave this patch and continue to find and
send another patches about tilegx. If I should still do somthing about
this patch, please let me know.
Thanks.
>From chengang@emindsoft.com.cn Mon Dec 12 19:23:58 2016
Date: Mon, 12 Dec 2016 19:23:58 +0800
From: Chen Gang <chengang@emindsoft.com.cn>
To: Bernd Edlinger <bernd.edlinger@hotmail.de>
Cc: "chengang@emindsoft.com.cn" <chengang@emindsoft.com.cn>,
"law@redhat.com" <law@redhat.com>,
"rth@redhat.com" <rth@redhat.com>,
"mikestump@comcast.net" <mikestump@comcast.net>,
"walt@tilera.com" <walt@tilera.com>,
"gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
"peter.maydell@linaro.com" <peter.maydell@linaro.com>,
"cmetcalf@mellanox.com" <cmetcalf@mellanox.com>
Subject: Re: [PATCH] gcc: config: tilegx: Reserve prev insn when delete
useless insn
Message-ID: <20161212112320.GA22787@localhost.localdomain>
References: <1481369032-26571-1-git-send-email-chengang@emindsoft.com.cn>
<AM4PR0701MB2162E91F39F574BC29E3167DE4860@AM4PR0701MB2162.eurprd07.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <AM4PR0701MB2162E91F39F574BC29E3167DE4860@AM4PR0701MB2162.eurprd07.prod.outlook.com>
User-Agent: Mutt/1.7.1 (2016-10-04)
Status: RO
Content-Length: 2159
Lines: 66
On Sat, Dec 10, 2016 at 03:38:10PM +0000, Bernd Edlinger wrote:
> On 12/10/16 12:23, chengang@emindsoft.com.cn wrote:
> > From: Chen Gang <chengang@emindsoft.com.cn>
> >
> > When check bundle, gcc may still need modify the prev insn. Original
> > implementation will lose the prev insn.
> >
> > Also correct the coding styles of relate code.
> >
> > 2016-12-10 Chen Gang <gang.chen.5i5j@gmail.com>
> >
> > gcc/
> > PR target/78222
> > * tilegx.c (tilegx_gen_bundle): Reserve prev insn when delete
> > useless insn.
>
> I think this was already fixed by Walt:
>
> ------------------------------------------------------------------------
> r242617 | walt | 2016-11-19 03:34:17 +0100 (Sat, 19 Nov 2016) | 5 lines
> Changed paths:
> M /trunk/gcc/ChangeLog
> M /trunk/gcc/config/tilegx/tilegx.c
>
> TILE-Gx: Fix bundling when encountering consecutive barriers.
>
> * config/tilegx/tilegx.c (tilegx_gen_bundles): Preserve
> end-of-bundle marker for consecutive barriers.
>
Oh, yes, Good news to me.
>
> But the formatting here is still odd, and should be fixed:
> TAB usage, single statements in braces, { not in a line of its own.
>
Yes.
> I am however not sure about this statement:
>
> /* Never wrap {} around inline asm. */
> if (GET_CODE (PATTERN (insn)) != ASM_INPUT)
>
> ... because this does only exclude asm(""); that is basic asm with
> empty assembler string. To exclude all other forms of asm statements
> that are hidden in PARALLEL constructs we would need:
>
> /* Never wrap {} around inline asm. */
> if (GET_CODE (PATTERN (insn)) != ASM_INPUT
> && asm_noperands (PATTERN (insn)) < 0)
>
>
> I think this if-condition is probably unnecessary, because it does
> apparently not create any problems although it is completely broken.
>
For me, we need not touch it, since we are not quite sure about it. And
welcome any other related members' idea for it.
All together, I guess, I can leave this patch and continue to find and
send another patches about tilegx. If I should still do somthing about
this patch, please let me know.
Thanks.
>From chengang@emindsoft.com.cn Mon Dec 12 20:05:15 2016
Date: Mon, 12 Dec 2016 20:05:15 +0800
From: Chen Gang <chengang@emindsoft.com.cn>
To: Bernd Edlinger <bernd.edlinger@hotmail.de>
Cc: "chengang@emindsoft.com.cn" <chengang@emindsoft.com.cn>,
"law@redhat.com" <law@redhat.com>,
"rth@redhat.com" <rth@redhat.com>,
"mikestump@comcast.net" <mikestump@comcast.net>,
"walt@tilera.com" <walt@tilera.com>,
"gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
"peter.maydell@linaro.com" <peter.maydell@linaro.com>,
"cmetcalf@mellanox.com" <cmetcalf@mellanox.com>
Subject: Re: [PATCH] gcc: config: tilegx: Reserve prev insn when delete
useless insn
Message-ID: <20161212112320.GA22787@localhost.localdomain>
References: <1481369032-26571-1-git-send-email-chengang@emindsoft.com.cn>
<AM4PR0701MB2162E91F39F574BC29E3167DE4860@AM4PR0701MB2162.eurprd07.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <AM4PR0701MB2162E91F39F574BC29E3167DE4860@AM4PR0701MB2162.eurprd07.prod.outlook.com>
User-Agent: Mutt/1.7.1 (2016-10-04)
Status: RO
Content-Length: 2159
Lines: 66
On Sat, Dec 10, 2016 at 03:38:10PM +0000, Bernd Edlinger wrote:
> On 12/10/16 12:23, chengang@emindsoft.com.cn wrote:
> > From: Chen Gang <chengang@emindsoft.com.cn>
> >
> > When check bundle, gcc may still need modify the prev insn. Original
> > implementation will lose the prev insn.
> >
> > Also correct the coding styles of relate code.
> >
> > 2016-12-10 Chen Gang <gang.chen.5i5j@gmail.com>
> >
> > gcc/
> > PR target/78222
> > * tilegx.c (tilegx_gen_bundle): Reserve prev insn when delete
> > useless insn.
>
> I think this was already fixed by Walt:
>
> ------------------------------------------------------------------------
> r242617 | walt | 2016-11-19 03:34:17 +0100 (Sat, 19 Nov 2016) | 5 lines
> Changed paths:
> M /trunk/gcc/ChangeLog
> M /trunk/gcc/config/tilegx/tilegx.c
>
> TILE-Gx: Fix bundling when encountering consecutive barriers.
>
> * config/tilegx/tilegx.c (tilegx_gen_bundles): Preserve
> end-of-bundle marker for consecutive barriers.
>
Oh, yes, Good news to me.
>
> But the formatting here is still odd, and should be fixed:
> TAB usage, single statements in braces, { not in a line of its own.
>
Yes.
> I am however not sure about this statement:
>
> /* Never wrap {} around inline asm. */
> if (GET_CODE (PATTERN (insn)) != ASM_INPUT)
>
> ... because this does only exclude asm(""); that is basic asm with
> empty assembler string. To exclude all other forms of asm statements
> that are hidden in PARALLEL constructs we would need:
>
> /* Never wrap {} around inline asm. */
> if (GET_CODE (PATTERN (insn)) != ASM_INPUT
> && asm_noperands (PATTERN (insn)) < 0)
>
>
> I think this if-condition is probably unnecessary, because it does
> apparently not create any problems although it is completely broken.
>
For me, we need not touch it, since we are not quite sure about it. And
welcome any other related members' idea for it.
All together, I guess, I can leave this patch and continue to find and
send another patches about tilegx. If I should still do somthing about
this patch, please let me know.
Thanks.