From: Chen Gang <cheng...@emindsoft.com.cn>

On Sat, Dec 10, 2016 at 03:38:10PM +0000, Bernd Edlinger wrote:
> On 12/10/16 12:23, cheng...@emindsoft.com.cn wrote:
> > From: Chen Gang <cheng...@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.5...@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 cheng...@emindsoft.com.cn Mon Dec 12 19:23:58 2016
Date: Mon, 12 Dec 2016 19:23:58 +0800
From: Chen Gang <cheng...@emindsoft.com.cn>
To: Bernd Edlinger <bernd.edlin...@hotmail.de>
Cc: "cheng...@emindsoft.com.cn" <cheng...@emindsoft.com.cn>,
        "l...@redhat.com" <l...@redhat.com>,
        "r...@redhat.com" <r...@redhat.com>,
        "mikest...@comcast.net" <mikest...@comcast.net>,
        "w...@tilera.com" <w...@tilera.com>,
        "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
        "peter.mayd...@linaro.com" <peter.mayd...@linaro.com>,
        "cmetc...@mellanox.com" <cmetc...@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-cheng...@emindsoft.com.cn>
 
<am4pr0701mb2162e91f39f574bc29e3167de4...@am4pr0701mb2162.eurprd07.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
<am4pr0701mb2162e91f39f574bc29e3167de4...@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, cheng...@emindsoft.com.cn wrote:
> > From: Chen Gang <cheng...@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.5...@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 cheng...@emindsoft.com.cn Mon Dec 12 20:05:15 2016
Date: Mon, 12 Dec 2016 20:05:15 +0800
From: Chen Gang <cheng...@emindsoft.com.cn>
To: Bernd Edlinger <bernd.edlin...@hotmail.de>
Cc: "cheng...@emindsoft.com.cn" <cheng...@emindsoft.com.cn>,
        "l...@redhat.com" <l...@redhat.com>,
        "r...@redhat.com" <r...@redhat.com>,
        "mikest...@comcast.net" <mikest...@comcast.net>,
        "w...@tilera.com" <w...@tilera.com>,
        "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
        "peter.mayd...@linaro.com" <peter.mayd...@linaro.com>,
        "cmetc...@mellanox.com" <cmetc...@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-cheng...@emindsoft.com.cn>
 
<am4pr0701mb2162e91f39f574bc29e3167de4...@am4pr0701mb2162.eurprd07.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
<am4pr0701mb2162e91f39f574bc29e3167de4...@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, cheng...@emindsoft.com.cn wrote:
> > From: Chen Gang <cheng...@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.5...@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.

Reply via email to