[PATCH] gcc: config: tilegx: Reserve prev insn when delete useless insn

chengang@emindsoft.com.cn chengang@emindsoft.com.cn
Mon Dec 12 12:09:00 GMT 2016


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.



More information about the Gcc-patches mailing list