[MIPS] Fix ICE in bitmap routines with LRA and inline assembly language

Hi, 

Following up the problem reported
https://gcc.gnu.org/ml/gcc/2014-09/msg00094.html

I am attaching the patch fixing the internal compiler error.

The problem appeared to be that the current program point was not
incremented after changing liveliness of a pseudo. In the attached testcase,
an early clobber pseudo register became dead but the result was overwritten 
by subsequent calls to mark_regno_dead function returning false. As a result, 
the live range of the pseudo was 0..1 but the lra_live_max_point was not 
+1 higher leading to lack of initialization of internal structures and 
processing random data, hence, the ICE.

The patch was bootstrapped and regtested on x86_64-unknown-linux-gnu.

Regards,
Robert

2014-09-09  Robert Suchanek  <robert.suchanek@imgtec.com>

gcc/
	* lra-lives.c (process_bb_lives): Replace assignment with bitwise OR
	assignment.

gcc/testsuite/
	* gcc.target/mips/20140909.c: New test.
---
 gcc/lra-lives.c                          |    6 +++---
 gcc/testsuite/gcc.target/mips/20140909.c |   20 ++++++++++++++++++++
 2 files changed, 23 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/mips/20140909.c

On 2014-09-09 5:39 AM, Robert Suchanek wrote:
> Hi,
>
> Following up the problem reported
> https://gcc.gnu.org/ml/gcc/2014-09/msg00094.html
>
> I am attaching the patch fixing the internal compiler error.
>
> The problem appeared to be that the current program point was not
> incremented after changing liveliness of a pseudo. In the attached testcase,
> an early clobber pseudo register became dead but the result was overwritten
> by subsequent calls to mark_regno_dead function returning false. As a result,
> the live range of the pseudo was 0..1 but the lra_live_max_point was not
> +1 higher leading to lack of initialization of internal structures and
> processing random data, hence, the ICE.
>
> The patch was bootstrapped and regtested on x86_64-unknown-linux-gnu.
>

The patch is ok to commit.  I think I made a typo as other analogous 
code contains '|='.

Thanks for fixing this, Robert.


>
> 2014-09-09  Robert Suchanek  <robert.suchanek@imgtec.com>
>
> gcc/
> 	* lra-lives.c (process_bb_lives): Replace assignment with bitwise OR
> 	assignment.
>
> gcc/testsuite/
> 	* gcc.target/mips/20140909.c: New test.
> ---
>   gcc/lra-lives.c                          |    6 +++---
>   gcc/testsuite/gcc.target/mips/20140909.c |   20 ++++++++++++++++++++
>   2 files changed, 23 insertions(+), 3 deletions(-)
>   create mode 100644 gcc/testsuite/gcc.target/mips/20140909.c
>
> diff --git a/gcc/lra-lives.c b/gcc/lra-lives.c
> index f34517d..b72824e 100644
> --- a/gcc/lra-lives.c
> +++ b/gcc/lra-lives.c
> @@ -680,9 +680,9 @@ process_bb_lives (basic_block bb, int &curr_point)
>         /* Mark early clobber outputs dead.  */
>         for (reg = curr_id->regs; reg != NULL; reg = reg->next)
>   	if (reg->type == OP_OUT && reg->early_clobber && ! reg->subreg_p)
> -	  need_curr_point_incr = mark_regno_dead (reg->regno,
> -						  reg->biggest_mode,
> -						  curr_point);
> +	  need_curr_point_incr |= mark_regno_dead (reg->regno,
> +						   reg->biggest_mode,
> +						   curr_point);
>
>         for (reg = curr_static_id->hard_regs; reg != NULL; reg = reg->next)
>   	if (reg->type == OP_OUT && reg->early_clobber && ! reg->subreg_p)
> diff --git a/gcc/testsuite/gcc.target/mips/20140909.c b/gcc/testsuite/gcc.target/mips/20140909.c
> new file mode 100644
> index 0000000..dbdfb30
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/mips/20140909.c
> @@ -0,0 +1,20 @@
> +/* { dg-do compile } */
> +/* { dg-skip-if "code quality test" { *-*-* } { "-O0" } { "" } } */
> +
> +NOMIPS16 int NoBarrier_AtomicIncrement(volatile int* ptr, int increment) {
> +  int temp, temp2;
> +  __asm__ __volatile__(".set push\n"
> +                       ".set noreorder\n"
> +                       "1:\n"
> +                       "ll %0, 0(%3)\n"
> +                       "addu %1, %0, %2\n"
> +                       "sc %1, 0(%3)\n"
> +                       "beqz %1, 1b\n"
> +                       "addu %1, %0, %2\n"
> +                       ".set pop\n"
> +                       : "=&r" (temp), "=&r" (temp2)
> +                       : "Ir" (increment), "r" (ptr)
> +                       : "memory");
> +
> +  return temp2;
> +}
>

> The patch is ok to commit.  I think I made a typo as other analogous
> code contains '|='.
> 
> Thanks for fixing this, Robert.

The LRA part of this is committed as r215119.

Eric: Is the test OK to commit for MIPS? I suggest removing the
dg-skip-if as the test is for successful compilation, not inspecting
the generated code.

Thanks,
Matthew

> 
> >
> > 2014-09-09  Robert Suchanek  <robert.suchanek@imgtec.com>
> >
> > gcc/testsuite/
> > 	* gcc.target/mips/20140909.c: New test.

> > diff --git a/gcc/testsuite/gcc.target/mips/20140909.c
> b/gcc/testsuite/gcc.target/mips/20140909.c
> > new file mode 100644
> > index 0000000..dbdfb30
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/mips/20140909.c
> > @@ -0,0 +1,20 @@
> > +/* { dg-do compile } */
> > +/* { dg-skip-if "code quality test" { *-*-* } { "-O0" } { "" } } */
> > +
> > +NOMIPS16 int NoBarrier_AtomicIncrement(volatile int* ptr, int
> increment) {
> > +  int temp, temp2;
> > +  __asm__ __volatile__(".set push\n"
> > +                       ".set noreorder\n"
> > +                       "1:\n"
> > +                       "ll %0, 0(%3)\n"
> > +                       "addu %1, %0, %2\n"
> > +                       "sc %1, 0(%3)\n"
> > +                       "beqz %1, 1b\n"
> > +                       "addu %1, %0, %2\n"
> > +                       ".set pop\n"
> > +                       : "=&r" (temp), "=&r" (temp2)
> > +                       : "Ir" (increment), "r" (ptr)
> > +                       : "memory");
> > +
> > +  return temp2;
> > +}
> >

On Thu, Sep 18, 2014 at 1:11 PM, Eric Christopher <echristo@gmail.com> wrote:
>
>
>
> On Wed, Sep 10, 2014 at 3:39 AM, Matthew Fortune <Matthew.Fortune@imgtec.com> wrote:
>>
>> > The patch is ok to commit.  I think I made a typo as other analogous
>> > code contains '|='.
>> >
>> > Thanks for fixing this, Robert.
>>
>> The LRA part of this is committed as r215119.
>>
>> Eric: Is the test OK to commit for MIPS? I suggest removing the
>> dg-skip-if as the test is for successful compilation, not inspecting
>> the generated code.
>>

Agreed. Sorry for the delay.

-eric

>
>>
>> Thanks,
>> Matthew
>>
>> >
>> > >
>> > > 2014-09-09  Robert Suchanek  <robert.suchanek@imgtec.com>
>> > >
>> > > gcc/testsuite/
>> > >     * gcc.target/mips/20140909.c: New test.
>>
>> > > diff --git a/gcc/testsuite/gcc.target/mips/20140909.c
>> > b/gcc/testsuite/gcc.target/mips/20140909.c
>> > > new file mode 100644
>> > > index 0000000..dbdfb30
>> > > --- /dev/null
>> > > +++ b/gcc/testsuite/gcc.target/mips/20140909.c
>> > > @@ -0,0 +1,20 @@
>> > > +/* { dg-do compile } */
>> > > +/* { dg-skip-if "code quality test" { *-*-* } { "-O0" } { "" } } */
>> > > +
>> > > +NOMIPS16 int NoBarrier_AtomicIncrement(volatile int* ptr, int
>> > increment) {
>> > > +  int temp, temp2;
>> > > +  __asm__ __volatile__(".set push\n"
>> > > +                       ".set noreorder\n"
>> > > +                       "1:\n"
>> > > +                       "ll %0, 0(%3)\n"
>> > > +                       "addu %1, %0, %2\n"
>> > > +                       "sc %1, 0(%3)\n"
>> > > +                       "beqz %1, 1b\n"
>> > > +                       "addu %1, %0, %2\n"
>> > > +                       ".set pop\n"
>> > > +                       : "=&r" (temp), "=&r" (temp2)
>> > > +                       : "Ir" (increment), "r" (ptr)
>> > > +                       : "memory");
>> > > +
>> > > +  return temp2;
>> > > +}
>> > >
>>
>