[combine] PR46164: Don't combine the insns if a volatile register is contained.

On Mon, Jan 26, 2015 at 05:55:52PM +0800, Hale Wang wrote:
> diff --git a/gcc/combine.c b/gcc/combine.c
> index 5c763b4..cf48666 100644
> --- a/gcc/combine.c
> +++ b/gcc/combine.c
> @@ -2004,6 +2004,13 @@ can_combine_p (rtx_insn *insn, rtx_insn *i3, rtx_insn
> *pred ATTRIBUTE_UNUSED,
>  	      return 0;
>  	}
>  
> +  /* If src contains a volatile register, reject, because the register may
> +     possibly be used in a asm operand.  The combined insn may cause the
> asm
> +     operand to be generated unexpectly.  */
> +
> +  if (REG_P (src) && REG_USERVAR_P (src))
> +    return 0;
> +
>    /* If INSN contains anything volatile, or is an `asm' (whether volatile
>       or not), reject, unless nothing volatile comes between it and I3 */


> diff --git a/gcc/testsuite/gcc.target/arm/pr46164.c
> b/gcc/testsuite/gcc.target/arm/pr46164.c
> new file mode 100644
> index 0000000..ad3b7cb
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/pr46164.c
> @@ -0,0 +1,26 @@
> +/* { dg-do compile } */
> +/* { dg-options "-mcpu=cortex-m3 -mthumb -O1" } */

Just "-O1" reproduces the problem here, FWIW.


Could you try this patch please?


Segher

> -----Original Message-----
> From: Segher Boessenkool [mailto:segher@kernel.crashing.org]
> Sent: Tuesday, January 27, 2015 3:07 AM
> To: Hale Wang
> Cc: GCC Patches
> Subject: Re: [PATCH] [gcc, combine] PR46164: Don't combine the insns if a
> volatile register is contained.
> 
> On Mon, Jan 26, 2015 at 05:55:52PM +0800, Hale Wang wrote:
> > diff --git a/gcc/combine.c b/gcc/combine.c index 5c763b4..cf48666
> > 100644
> > --- a/gcc/combine.c
> > +++ b/gcc/combine.c
> > @@ -2004,6 +2004,13 @@ can_combine_p (rtx_insn *insn, rtx_insn *i3,
> > rtx_insn *pred ATTRIBUTE_UNUSED,
> >  	      return 0;
> >  	}
> >
> > +  /* If src contains a volatile register, reject, because the register
may
> > +     possibly be used in a asm operand.  The combined insn may cause
> > + the
> > asm
> > +     operand to be generated unexpectly.  */
> > +
> > +  if (REG_P (src) && REG_USERVAR_P (src))
> > +    return 0;
> > +
> >    /* If INSN contains anything volatile, or is an `asm' (whether
volatile
> >       or not), reject, unless nothing volatile comes between it and I3
> > */
> 
> 
> > diff --git a/gcc/testsuite/gcc.target/arm/pr46164.c
> > b/gcc/testsuite/gcc.target/arm/pr46164.c
> > new file mode 100644
> > index 0000000..ad3b7cb
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/arm/pr46164.c
> > @@ -0,0 +1,26 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-mcpu=cortex-m3 -mthumb -O1" } */
> 
> Just "-O1" reproduces the problem here, FWIW.
> 

You are correct. Just "-O1" reproduces this problem.
However it's a combine bug which is related to the combing user specified
register into inline-asm.

> 
> Could you try this patch please?
>
 
Your patch rejected the combine 98+43, that's correct. However, Jakub
pointed out that preventing that to be combined would be a serious
regression on code quality.

Andrew Pinski suggested: can_combine_p would reject combing into an
inline-asm to prevent this issue. And I have updated the patch. What do you
think about this change?

BR,
Hale


@@ -1983,6 +1983,10 @@ can_combine_p (rtx_insn *insn, rtx_insn *i3, rtx_insn
*pred ATTRIBUTE_UNUSED,
   else if (GET_CODE (dest) != CC0)
     return 0;
 
+  /* If i3 contains an inline-asm operand, reject, because the user
specified
+     registers in the inline-asm maybe removed by the combining.  */  
+ if ((asm_op = extract_asm_operands (PATTERN (i3))) != NULL)
+    return 0;
 
   if (GET_CODE (PATTERN (i3)) == PARALLEL)
     for (i = XVECLEN (PATTERN (i3), 0) - 1; i >= 0; i--)

> 
> Segher
> 
>