Re: [PATCH 1/9] gensupport: Fix define_subst operand renumbering.

2016-03-01 Thread Andreas Krebbel 
On 03/01/2016 02:38 PM, James Greenhalgh wrote:
> On Tue, Mar 01, 2016 at 01:35:18PM +0100, Andreas Krebbel wrote:
>> On 03/01/2016 01:15 PM, James Greenhalgh wrote:
>>> On Tue, Mar 01, 2016 at 10:29:28AM +0100, Andreas Krebbel wrote:
 On 02/29/2016 02:36 PM, Bernd Schmidt wrote:
> On 02/29/2016 09:46 AM, Andreas Krebbel wrote:
>> Ok for mainline?
>>
>>  * gensupport.c (process_substs_on_one_elem): Split loop to
>>  complete mark_operands_used_in_match_dup on all expressions in the
>>  vector first.
>>  (adjust_operands_numbers): Inline into process_substs_on_one_elem
>>  and remove function.
>
> Didn't I approve this a while ago? Not sure it's appropriate for stage4 
> though; is this series fixing an important regression?

 Yes you did. I didn't commit it until the rest of the patch series was 
 ready to commit.  The patch
 series fixes a fundamental problem in the backend. The first iteration was 
 posted before stage 4 but
 it took me a few iterations to get it right.

 I've committed the patch now after retesting.
>>>
>>> This looks like it has caused failures in the following tests on an
>>> x86_64-none-linux-gnu build.
>>
>> I've regression tested the patch on x86_64 as well.  Are there specific
>> options required to enable these tests?
> 
> The bisect robot just builds a stage one compiler, configured with:
> 
>   ./configure --disable-bootstrap, --enable-languages=c,c++,fortran
>   --disable-multilib --disable-libsanitizer
> 
> My system GCC is a 5.2 from the release sources with:
> 
>   ../gcc-5.2.0/configure --with-bugurl='Good luck'
>  --enable-languages=c,c++,go,fortran,objc,obj-c++
>  --prefix=/work/install-gcc-5.2.0 --enable-shared
>  --enable-linker-build-id --without-included-gettext
>  --enable-threads=posix --enable-nls
>--enable-clocale=gnu --enable-libstdcxx-debug
>  --enable-libstdcxx-time=yes
>  --enable-gnu-unique-object --disable-libmudflap
>  --enable-plugin --with-system-zlib
>  --disable-browser-plugin --enable-java-awt=gtk
>  --enable-gtk-cairo --with-arch-directory=amd64
>  --enable-objc-gc --enable-multiarch
>  --disable-werror --with-arch-32=i686
>  --with-abi=m64 --with-multilib-list=m32,m64,mx32
>  --with-tune=native --enable-checking=release
>  --build=x86_64-linux-gnu --host=x86_64-linux-gnu
>  --target=x86_64-linux-gnu
> 
> I tried a full bootstrap at that revision and still see these failures.
> Who knows what state has been corrupted, or that you silently get away with,
> if this is an undefined behaviour somewhere :-). I haven't tried with a
> valgrind checking build to see what it can spot.

Ok. Thanks for the infos.  I'll try to have a look. I've reverted the patch now.

Bye,

-Andreas-

Re: [PATCH 1/9] gensupport: Fix define_subst operand renumbering.

2016-03-01 Thread James Greenhalgh 
On Tue, Mar 01, 2016 at 01:35:18PM +0100, Andreas Krebbel wrote:
> On 03/01/2016 01:15 PM, James Greenhalgh wrote:
> > On Tue, Mar 01, 2016 at 10:29:28AM +0100, Andreas Krebbel wrote:
> >> On 02/29/2016 02:36 PM, Bernd Schmidt wrote:
> >>> On 02/29/2016 09:46 AM, Andreas Krebbel wrote:
>  Ok for mainline?
> 
>   * gensupport.c (process_substs_on_one_elem): Split loop to
>   complete mark_operands_used_in_match_dup on all expressions in the
>   vector first.
>   (adjust_operands_numbers): Inline into process_substs_on_one_elem
>   and remove function.
> >>>
> >>> Didn't I approve this a while ago? Not sure it's appropriate for stage4 
> >>> though; is this series fixing an important regression?
> >>
> >> Yes you did. I didn't commit it until the rest of the patch series was 
> >> ready to commit.  The patch
> >> series fixes a fundamental problem in the backend. The first iteration was 
> >> posted before stage 4 but
> >> it took me a few iterations to get it right.
> >>
> >> I've committed the patch now after retesting.
> > 
> > This looks like it has caused failures in the following tests on an
> > x86_64-none-linux-gnu build.
> 
> I've regression tested the patch on x86_64 as well.  Are there specific
> options required to enable these tests?

The bisect robot just builds a stage one compiler, configured with:

  ./configure --disable-bootstrap, --enable-languages=c,c++,fortran
  --disable-multilib --disable-libsanitizer

My system GCC is a 5.2 from the release sources with:

  ../gcc-5.2.0/configure --with-bugurl='Good luck'
 --enable-languages=c,c++,go,fortran,objc,obj-c++
 --prefix=/work/install-gcc-5.2.0 --enable-shared
 --enable-linker-build-id --without-included-gettext
 --enable-threads=posix --enable-nls
 --enable-clocale=gnu --enable-libstdcxx-debug
 --enable-libstdcxx-time=yes
 --enable-gnu-unique-object --disable-libmudflap
 --enable-plugin --with-system-zlib
 --disable-browser-plugin --enable-java-awt=gtk
 --enable-gtk-cairo --with-arch-directory=amd64
 --enable-objc-gc --enable-multiarch
 --disable-werror --with-arch-32=i686
 --with-abi=m64 --with-multilib-list=m32,m64,mx32
 --with-tune=native --enable-checking=release
 --build=x86_64-linux-gnu --host=x86_64-linux-gnu
 --target=x86_64-linux-gnu

I tried a full bootstrap at that revision and still see these failures.
Who knows what state has been corrupted, or that you silently get away with,
if this is an undefined behaviour somewhere :-). I haven't tried with a
valgrind checking build to see what it can spot.

Thanks,
James

> > 
> > The failures are of this form:
> > 
> > In file included from 
> > /data/work/gcc-bisect-bot/build/gcc/include/immintrin.h:45:0,
> >  from 
> > /work/gcc-bisect-bot/gcc/gcc/testsuite/gcc.target/i386/avx512f-vfnmsubXXXps-1.c:12:
> > /data/work/gcc-bisect-bot/build/gcc/include/avx512fintrin.h: In function 
> > 'avx512f_test':
> > /data/work/gcc-bisect-bot/build/gcc/include/avx512fintrin.h:11666:10: 
> > internal compiler error: Segmentation fault
> > 0xb1a18f crash_signal
> > /work/gcc-bisect-bot/gcc/gcc/toplev.c:335
> > Please submit a full bug report,
> > with preprocessed source if appropriate.
> > Please include the complete backtrace with any bug report.
> > See  for instructions.
> > 
> > There are lots of them, so I'm just mentioning the unique names below.
> > 
> >   gcc.target/i386/avx512f-vfmaddsubXXXps-2.c
> >   gcc.target/i386/avx512f-vfmsubaddXXXps-2.c
> >   gcc.target/i386/avx512f-vfnmsubXXXps-2.c
> >   gcc.target/i386/avx512f-vfnmsubXXXps-1.c
> >   gcc.target/i386/avx-2.c
> >   gcc.target/i386/avx512vl-vshufpd-1.c
> >   gcc.target/i386/avx512f-vfmsubXXXpd-2.c
> >   gcc.target/i386/avx-1.c
> >   gcc.target/i386/avx512f-vfixupimmsd-1.c
> >   gcc.target/i386/avx512f-vfmsubXXXps-1.c
> >   gcc.target/i386/avx512f-vfmsubaddXXXpd-2.c
> >   gcc.target/i386/avx512f-vfmsubXXXps-2.c
> >   gcc.target/i386/avx512vl-vshufps-1.c
> >   gcc.target/i386/avx512f-vfmaddsubXXXps-1.c
> >   gcc.target/i386/avx512f-vfixupimmsd-2.c
> >   gcc.target/i386/avx512f-vfmaddXXXps-1.c
> >   gcc.target/i386/avx512f-vfmaddsubXXXpd-1.c
> >   gcc.target/i386/avx512f-vfmaddXXXpd-2.c
> >   gcc.target/i386/avx512f-vfmaddXXXps-2.c
> >   gcc.target/i386/avx512f-vfmaddsubXXXpd-2.c
> >   gcc.target/i386/sse-22.c
> >   gcc.target/i386/avx512f-vfmsubXXXpd-1.c
> >   gcc.target/i386/testround-1.c
> >   gcc.target/i386/sse-23.c
> >   gcc.target/i386/avx512f-vfmsubaddXXXpd-1.c
> >   gcc.target/i386/sse-22a.c
> >   gcc.target/i386/sse-25.c
> >   gcc.target/i386/sse-24.c
> >   gcc.tar

Re: [PATCH 1/9] gensupport: Fix define_subst operand renumbering.

2016-03-01 Thread Andreas Krebbel 
On 03/01/2016 01:15 PM, James Greenhalgh wrote:
> On Tue, Mar 01, 2016 at 10:29:28AM +0100, Andreas Krebbel wrote:
>> On 02/29/2016 02:36 PM, Bernd Schmidt wrote:
>>> On 02/29/2016 09:46 AM, Andreas Krebbel wrote:
 Ok for mainline?

* gensupport.c (process_substs_on_one_elem): Split loop to
complete mark_operands_used_in_match_dup on all expressions in the
vector first.
(adjust_operands_numbers): Inline into process_substs_on_one_elem
and remove function.
>>>
>>> Didn't I approve this a while ago? Not sure it's appropriate for stage4 
>>> though; is this series fixing an important regression?
>>
>> Yes you did. I didn't commit it until the rest of the patch series was ready 
>> to commit.  The patch
>> series fixes a fundamental problem in the backend. The first iteration was 
>> posted before stage 4 but
>> it took me a few iterations to get it right.
>>
>> I've committed the patch now after retesting.
> 
> This looks like it has caused failures in the following tests on an
> x86_64-none-linux-gnu build.

I've regression tested the patch on x86_64 as well.  Are there specific options 
required to enable
these tests?

Sorry for the breakage. I'll revert the patch together with the affected parts 
of my S/390 patchset.
I'll just need a couple of hours to re-test on S/390.

Bye,

-Andreas-

> 
> The failures are of this form:
> 
> In file included from 
> /data/work/gcc-bisect-bot/build/gcc/include/immintrin.h:45:0,
>  from 
> /work/gcc-bisect-bot/gcc/gcc/testsuite/gcc.target/i386/avx512f-vfnmsubXXXps-1.c:12:
> /data/work/gcc-bisect-bot/build/gcc/include/avx512fintrin.h: In function 
> 'avx512f_test':
> /data/work/gcc-bisect-bot/build/gcc/include/avx512fintrin.h:11666:10: 
> internal compiler error: Segmentation fault
> 0xb1a18f crash_signal
>   /work/gcc-bisect-bot/gcc/gcc/toplev.c:335
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See  for instructions.
> 
> There are lots of them, so I'm just mentioning the unique names below.
> 
>   gcc.target/i386/avx512f-vfmaddsubXXXps-2.c
>   gcc.target/i386/avx512f-vfmsubaddXXXps-2.c
>   gcc.target/i386/avx512f-vfnmsubXXXps-2.c
>   gcc.target/i386/avx512f-vfnmsubXXXps-1.c
>   gcc.target/i386/avx-2.c
>   gcc.target/i386/avx512vl-vshufpd-1.c
>   gcc.target/i386/avx512f-vfmsubXXXpd-2.c
>   gcc.target/i386/avx-1.c
>   gcc.target/i386/avx512f-vfixupimmsd-1.c
>   gcc.target/i386/avx512f-vfmsubXXXps-1.c
>   gcc.target/i386/avx512f-vfmsubaddXXXpd-2.c
>   gcc.target/i386/avx512f-vfmsubXXXps-2.c
>   gcc.target/i386/avx512vl-vshufps-1.c
>   gcc.target/i386/avx512f-vfmaddsubXXXps-1.c
>   gcc.target/i386/avx512f-vfixupimmsd-2.c
>   gcc.target/i386/avx512f-vfmaddXXXps-1.c
>   gcc.target/i386/avx512f-vfmaddsubXXXpd-1.c
>   gcc.target/i386/avx512f-vfmaddXXXpd-2.c
>   gcc.target/i386/avx512f-vfmaddXXXps-2.c
>   gcc.target/i386/avx512f-vfmaddsubXXXpd-2.c
>   gcc.target/i386/sse-22.c
>   gcc.target/i386/avx512f-vfmsubXXXpd-1.c
>   gcc.target/i386/testround-1.c
>   gcc.target/i386/sse-23.c
>   gcc.target/i386/avx512f-vfmsubaddXXXpd-1.c
>   gcc.target/i386/sse-22a.c
>   gcc.target/i386/sse-25.c
>   gcc.target/i386/sse-24.c
>   gcc.target/i386/avx512f-vfnmsubXXXpd-2.c
>   gcc.target/i386/sse-14.c
>   gcc.target/i386/avx512f-vfixupimmss-1.c
>   gcc.target/i386/avx512f-vfnmaddXXXpd-1.c
>   gcc.target/i386/avx512f-vfnmaddXXXps-2.c
>   gcc.target/i386/avx512f-vfixupimmpd-2.c
>   gcc.target/i386/avx512f-vfnmaddXXXpd-2.c
>   gcc.target/i386/sse-13.c
>   gcc.target/i386/avx512f-vfixupimmps-1.c
>   gcc.target/i386/avx512f-vfnmsubXXXpd-1.c
>   gcc.target/i386/avx512f-vfnmaddXXXps-1.c
>   gcc.target/i386/avx512f-vfixupimmps-2.c
>   gcc.target/i386/avx512f-vfmaddXXXpd-1.c
>   gcc.target/i386/testimm-10.c
>   gcc.target/i386/avx512f-vfmsubaddXXXps-1.c
>   gcc.target/i386/avx512f-vfixupimmss-2.c
>   gcc.target/i386/avx512f-vfixupimmpd-1.c
> 
> Author: krebbel 
> Date:   Tue Mar 1 09:19:14 2016 +
> 
> gensupport: Fix define_subst operand renumbering.
> 
> When processing substitutions the operands are renumbered.  To find a
> free operand number the array used_operands_numbers is used.
> Currently this array is used to assign new numbers before all the
> RTXes in the vector have been processed.  I did run into problems with
> this for insns where a match_dup occurred in a later (use ...) operand
> referring to an earlier operand (e.g. s390.md "setmem_long").
> 
> The patch splits the loop doing the processing into two in order to
> have all the operand numbers collected already when assigning new
> numbers.
> 
> gcc/ChangeLog:
> 
> 2016-03-01  Andreas Krebbel  
> 
>   * gensupport.c (process_substs_on_one_elem): Split loop to
>   complete mark_operands_used_in_match_dup on all expressions in the
>   vector first.
>   (adjust_op

Re: [PATCH 1/9] gensupport: Fix define_subst operand renumbering.

2016-03-01 Thread Bernd Schmidt 

On 03/01/2016 01:15 PM, James Greenhalgh wrote:

On Tue, Mar 01, 2016 at 10:29:28AM +0100, Andreas Krebbel wrote:

On 02/29/2016 02:36 PM, Bernd Schmidt wrote:

Didn't I approve this a while ago? Not sure it's appropriate for stage4
though; is this series fixing an important regression?


Yes you did. I didn't commit it until the rest of the patch series was ready to 
commit.  The patch
series fixes a fundamental problem in the backend. The first iteration was 
posted before stage 4 but
it took me a few iterations to get it right.

I've committed the patch now after retesting.


This looks like it has caused failures in the following tests on an
x86_64-none-linux-gnu build.


My inclination would be not to change this code in stage4, it looks too 
fragile.



Bernd

Re: [PATCH 1/9] gensupport: Fix define_subst operand renumbering.

2016-03-01 Thread James Greenhalgh 
On Tue, Mar 01, 2016 at 10:29:28AM +0100, Andreas Krebbel wrote:
> On 02/29/2016 02:36 PM, Bernd Schmidt wrote:
> > On 02/29/2016 09:46 AM, Andreas Krebbel wrote:
> >> Ok for mainline?
> >>
> >>* gensupport.c (process_substs_on_one_elem): Split loop to
> >>complete mark_operands_used_in_match_dup on all expressions in the
> >>vector first.
> >>(adjust_operands_numbers): Inline into process_substs_on_one_elem
> >>and remove function.
> > 
> > Didn't I approve this a while ago? Not sure it's appropriate for stage4 
> > though; is this series fixing an important regression?
> 
> Yes you did. I didn't commit it until the rest of the patch series was ready 
> to commit.  The patch
> series fixes a fundamental problem in the backend. The first iteration was 
> posted before stage 4 but
> it took me a few iterations to get it right.
> 
> I've committed the patch now after retesting.

This looks like it has caused failures in the following tests on an
x86_64-none-linux-gnu build.

The failures are of this form:

In file included from 
/data/work/gcc-bisect-bot/build/gcc/include/immintrin.h:45:0,
 from 
/work/gcc-bisect-bot/gcc/gcc/testsuite/gcc.target/i386/avx512f-vfnmsubXXXps-1.c:12:
/data/work/gcc-bisect-bot/build/gcc/include/avx512fintrin.h: In function 
'avx512f_test':
/data/work/gcc-bisect-bot/build/gcc/include/avx512fintrin.h:11666:10: internal 
compiler error: Segmentation fault
0xb1a18f crash_signal
/work/gcc-bisect-bot/gcc/gcc/toplev.c:335
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See  for instructions.

There are lots of them, so I'm just mentioning the unique names below.

  gcc.target/i386/avx512f-vfmaddsubXXXps-2.c
  gcc.target/i386/avx512f-vfmsubaddXXXps-2.c
  gcc.target/i386/avx512f-vfnmsubXXXps-2.c
  gcc.target/i386/avx512f-vfnmsubXXXps-1.c
  gcc.target/i386/avx-2.c
  gcc.target/i386/avx512vl-vshufpd-1.c
  gcc.target/i386/avx512f-vfmsubXXXpd-2.c
  gcc.target/i386/avx-1.c
  gcc.target/i386/avx512f-vfixupimmsd-1.c
  gcc.target/i386/avx512f-vfmsubXXXps-1.c
  gcc.target/i386/avx512f-vfmsubaddXXXpd-2.c
  gcc.target/i386/avx512f-vfmsubXXXps-2.c
  gcc.target/i386/avx512vl-vshufps-1.c
  gcc.target/i386/avx512f-vfmaddsubXXXps-1.c
  gcc.target/i386/avx512f-vfixupimmsd-2.c
  gcc.target/i386/avx512f-vfmaddXXXps-1.c
  gcc.target/i386/avx512f-vfmaddsubXXXpd-1.c
  gcc.target/i386/avx512f-vfmaddXXXpd-2.c
  gcc.target/i386/avx512f-vfmaddXXXps-2.c
  gcc.target/i386/avx512f-vfmaddsubXXXpd-2.c
  gcc.target/i386/sse-22.c
  gcc.target/i386/avx512f-vfmsubXXXpd-1.c
  gcc.target/i386/testround-1.c
  gcc.target/i386/sse-23.c
  gcc.target/i386/avx512f-vfmsubaddXXXpd-1.c
  gcc.target/i386/sse-22a.c
  gcc.target/i386/sse-25.c
  gcc.target/i386/sse-24.c
  gcc.target/i386/avx512f-vfnmsubXXXpd-2.c
  gcc.target/i386/sse-14.c
  gcc.target/i386/avx512f-vfixupimmss-1.c
  gcc.target/i386/avx512f-vfnmaddXXXpd-1.c
  gcc.target/i386/avx512f-vfnmaddXXXps-2.c
  gcc.target/i386/avx512f-vfixupimmpd-2.c
  gcc.target/i386/avx512f-vfnmaddXXXpd-2.c
  gcc.target/i386/sse-13.c
  gcc.target/i386/avx512f-vfixupimmps-1.c
  gcc.target/i386/avx512f-vfnmsubXXXpd-1.c
  gcc.target/i386/avx512f-vfnmaddXXXps-1.c
  gcc.target/i386/avx512f-vfixupimmps-2.c
  gcc.target/i386/avx512f-vfmaddXXXpd-1.c
  gcc.target/i386/testimm-10.c
  gcc.target/i386/avx512f-vfmsubaddXXXps-1.c
  gcc.target/i386/avx512f-vfixupimmss-2.c
  gcc.target/i386/avx512f-vfixupimmpd-1.c

Author: krebbel 
Date:   Tue Mar 1 09:19:14 2016 +

gensupport: Fix define_subst operand renumbering.

When processing substitutions the operands are renumbered.  To find a
free operand number the array used_operands_numbers is used.
Currently this array is used to assign new numbers before all the
RTXes in the vector have been processed.  I did run into problems with
this for insns where a match_dup occurred in a later (use ...) operand
referring to an earlier operand (e.g. s390.md "setmem_long").

The patch splits the loop doing the processing into two in order to
have all the operand numbers collected already when assigning new
numbers.

gcc/ChangeLog:

2016-03-01  Andreas Krebbel  

* gensupport.c (process_substs_on_one_elem): Split loop to
complete mark_operands_used_in_match_dup on all expressions in the
vector first.
(adjust_operands_numbers): Inline into process_substs_on_one_elem
and remove function.

svn+ssh://gcc.gnu.org/svn/gcc/trunk@233841

Thanks,
James

> 
> -Andreas-
>

Re: [PATCH 1/9] gensupport: Fix define_subst operand renumbering.

2016-03-01 Thread Andreas Krebbel 
On 02/29/2016 02:36 PM, Bernd Schmidt wrote:
> On 02/29/2016 09:46 AM, Andreas Krebbel wrote:
>> Ok for mainline?
>>
>>  * gensupport.c (process_substs_on_one_elem): Split loop to
>>  complete mark_operands_used_in_match_dup on all expressions in the
>>  vector first.
>>  (adjust_operands_numbers): Inline into process_substs_on_one_elem
>>  and remove function.
> 
> Didn't I approve this a while ago? Not sure it's appropriate for stage4 
> though; is this series fixing an important regression?

Yes you did. I didn't commit it until the rest of the patch series was ready to 
commit.  The patch
series fixes a fundamental problem in the backend. The first iteration was 
posted before stage 4 but
it took me a few iterations to get it right.

I've committed the patch now after retesting.

-Andreas-

Re: [PATCH 1/9] gensupport: Fix define_subst operand renumbering.

2016-02-29 Thread Bernd Schmidt 

On 02/29/2016 09:46 AM, Andreas Krebbel wrote:

Ok for mainline?

* gensupport.c (process_substs_on_one_elem): Split loop to
complete mark_operands_used_in_match_dup on all expressions in the
vector first.
(adjust_operands_numbers): Inline into process_substs_on_one_elem
and remove function.


Didn't I approve this a while ago? Not sure it's appropriate for stage4 
though; is this series fixing an important regression?



Bernd

[PATCH 1/9] gensupport: Fix define_subst operand renumbering.

2016-02-29 Thread Andreas Krebbel 
When processing substitutions the operands are renumbered.  To find a
free operand number the array used_operands_numbers is used.
Currently this array is used to assign new numbers before all the
RTXes in the vector have been processed.  I did run into problems with
this for insns where a match_dup occurred in a later (use ...) operand
referring to an earlier operand (e.g. s390.md "setmem_long").

The patch splits the loop doing the processing into two in order to
have all the operand numbers collected already when assigning new
numbers.

Bootstrapped and regtested on s390, s390x, and x86_64.

Ok for mainline?

Bye,

-Andreas-

gcc/ChangeLog:

2016-02-29  Andreas Krebbel  

* gensupport.c (process_substs_on_one_elem): Split loop to
complete mark_operands_used_in_match_dup on all expressions in the
vector first.
(adjust_operands_numbers): Inline into process_substs_on_one_elem
and remove function.
---
 gcc/gensupport.c | 45 -
 1 file changed, 20 insertions(+), 25 deletions(-)

diff --git a/gcc/gensupport.c b/gcc/gensupport.c
index 8c5a1ab..de29579 100644
--- a/gcc/gensupport.c
+++ b/gcc/gensupport.c
@@ -126,7 +126,10 @@ static const char * duplicate_each_alternative (const char 
* str, int n_dup);
 
 typedef const char * (*constraints_handler_t) (const char *, int);
 static rtx alter_constraints (rtx, int, constraints_handler_t);
-static rtx adjust_operands_numbers (rtx);
+
+static void mark_operands_used_in_match_dup (rtx);
+static void renumerate_operands_in_pattern (rtx);
+
 static rtx replace_duplicating_operands_in_pattern (rtx);
 
 /* Make a version of gen_rtx_CONST_INT so that GEN_INT can be used in
@@ -1844,7 +1847,18 @@ process_substs_on_one_elem (struct queue_elem *elem,
  subst_pattern = alter_constraints (subst_pattern, alternatives,
 duplicate_each_alternative);
 
- subst_pattern = adjust_operands_numbers (subst_pattern);
+ mark_operands_used_in_match_dup (subst_pattern);
+ RTVEC_ELT (subst_pattern_vec, j) = subst_pattern;
+   }
+
+  for (j = 0; j < XVECLEN (subst_elem->data, 3); j++)
+   {
+ subst_pattern = RTVEC_ELT (subst_pattern_vec, j);
+
+ /* The number of MATCH_OPERANDs in the output pattern might
+change.  This routine assigns new numbers to the
+MATCH_OPERAND expressions to avoid collisions.  */
+ renumerate_operands_in_pattern (subst_pattern);
 
  /* Substitute match_dup and match_op_dup in the new pattern and
 duplicate constraints.  */
@@ -1857,7 +1871,6 @@ process_substs_on_one_elem (struct queue_elem *elem,
  if (GET_CODE (elem->data) == DEFINE_EXPAND)
remove_constraints (subst_pattern);
 
- RTVEC_ELT (subst_pattern_vec, j) = subst_pattern;
}
   XVEC (elem->data, 1) = subst_pattern_vec;
 
@@ -1927,7 +1940,7 @@ mark_operands_from_match_dup (rtx pattern)
 }
 }
 
-/* This is a subroutine of adjust_operands_numbers.
+/* This is a subroutine of process_substs_on_one_elem.
It goes through all expressions in PATTERN and when MATCH_DUP is
met, all MATCH_OPERANDs inside it is marked as occupied.  The
process of marking is done by routin mark_operands_from_match_dup.  */
@@ -1973,10 +1986,9 @@ find_first_unused_number_of_operand ()
   return MAX_OPERANDS;
 }
 
-/* This is subroutine of adjust_operands_numbers.
-   It visits all expressions in PATTERN and assigns not-occupied
-   operand indexes to MATCH_OPERANDs and MATCH_OPERATORs of this
-   PATTERN.  */
+/* This is a subroutine of process_substs_on_one_elem.  It visits all
+   expressions in PATTERN and assigns not-occupied operand indexes to
+   MATCH_OPERANDs and MATCH_OPERATORs of this PATTERN.  */
 static void
 renumerate_operands_in_pattern (rtx pattern)
 {
@@ -2011,23 +2023,6 @@ renumerate_operands_in_pattern (rtx pattern)
 }
 }
 
-/* If output pattern of define_subst contains MATCH_DUP, then this
-   expression would be replaced with the pattern, matched with
-   MATCH_OPERAND from input pattern.  This pattern could contain any
-   number of MATCH_OPERANDs, MATCH_OPERATORs etc., so it's possible
-   that a MATCH_OPERAND from output_pattern (if any) would have the
-   same number, as MATCH_OPERAND from copied pattern.  To avoid such
-   indexes overlapping, we assign new indexes to MATCH_OPERANDs,
-   laying in the output pattern outside of MATCH_DUPs.  */
-static rtx
-adjust_operands_numbers (rtx pattern)
-{
-  mark_operands_used_in_match_dup (pattern);
-
-  renumerate_operands_in_pattern (pattern);
-
-  return pattern;
-}
 
 /* Generate RTL expression
(match_dup OPNO)
-- 
1.9.1

[PATCH 1/9] gensupport: Fix define_subst operand renumbering.

2016-02-23 Thread Andreas Krebbel 
When processing substitutions the operands are renumbered.  To find a
free operand number the array used_operands_numbers is used.
Currently this array is used to assign new numbers before all the
RTXes in the vector have been processed.  I did run into problems with
this for insns where a match_dup occurred in a later (use ...) operand
referring to an earlier operand (e.g. s390.md "setmem_long").

The patch splits the loop doing the processing into two in order to
have all the operand numbers collected already when assigning new
numbers.

Bootstrapped and regtested on s390, s390x, and x86_64.

Ok for mainline?

Bye,

-Andreas-

gcc/ChangeLog:

* gensupport.c (process_substs_on_one_elem): Split loop to
complete mark_operands_used_in_match_dup on all expressions in the
vector first.
(adjust_operands_numbers): Inline into process_substs_on_one_elem
and remove function.
---
 gcc/gensupport.c | 45 -
 1 file changed, 20 insertions(+), 25 deletions(-)

diff --git a/gcc/gensupport.c b/gcc/gensupport.c
index 8c5a1ab..de29579 100644
--- a/gcc/gensupport.c
+++ b/gcc/gensupport.c
@@ -126,7 +126,10 @@ static const char * duplicate_each_alternative (const char 
* str, int n_dup);
 
 typedef const char * (*constraints_handler_t) (const char *, int);
 static rtx alter_constraints (rtx, int, constraints_handler_t);
-static rtx adjust_operands_numbers (rtx);
+
+static void mark_operands_used_in_match_dup (rtx);
+static void renumerate_operands_in_pattern (rtx);
+
 static rtx replace_duplicating_operands_in_pattern (rtx);
 
 /* Make a version of gen_rtx_CONST_INT so that GEN_INT can be used in
@@ -1844,7 +1847,18 @@ process_substs_on_one_elem (struct queue_elem *elem,
  subst_pattern = alter_constraints (subst_pattern, alternatives,
 duplicate_each_alternative);
 
- subst_pattern = adjust_operands_numbers (subst_pattern);
+ mark_operands_used_in_match_dup (subst_pattern);
+ RTVEC_ELT (subst_pattern_vec, j) = subst_pattern;
+   }
+
+  for (j = 0; j < XVECLEN (subst_elem->data, 3); j++)
+   {
+ subst_pattern = RTVEC_ELT (subst_pattern_vec, j);
+
+ /* The number of MATCH_OPERANDs in the output pattern might
+change.  This routine assigns new numbers to the
+MATCH_OPERAND expressions to avoid collisions.  */
+ renumerate_operands_in_pattern (subst_pattern);
 
  /* Substitute match_dup and match_op_dup in the new pattern and
 duplicate constraints.  */
@@ -1857,7 +1871,6 @@ process_substs_on_one_elem (struct queue_elem *elem,
  if (GET_CODE (elem->data) == DEFINE_EXPAND)
remove_constraints (subst_pattern);
 
- RTVEC_ELT (subst_pattern_vec, j) = subst_pattern;
}
   XVEC (elem->data, 1) = subst_pattern_vec;
 
@@ -1927,7 +1940,7 @@ mark_operands_from_match_dup (rtx pattern)
 }
 }
 
-/* This is a subroutine of adjust_operands_numbers.
+/* This is a subroutine of process_substs_on_one_elem.
It goes through all expressions in PATTERN and when MATCH_DUP is
met, all MATCH_OPERANDs inside it is marked as occupied.  The
process of marking is done by routin mark_operands_from_match_dup.  */
@@ -1973,10 +1986,9 @@ find_first_unused_number_of_operand ()
   return MAX_OPERANDS;
 }
 
-/* This is subroutine of adjust_operands_numbers.
-   It visits all expressions in PATTERN and assigns not-occupied
-   operand indexes to MATCH_OPERANDs and MATCH_OPERATORs of this
-   PATTERN.  */
+/* This is a subroutine of process_substs_on_one_elem.  It visits all
+   expressions in PATTERN and assigns not-occupied operand indexes to
+   MATCH_OPERANDs and MATCH_OPERATORs of this PATTERN.  */
 static void
 renumerate_operands_in_pattern (rtx pattern)
 {
@@ -2011,23 +2023,6 @@ renumerate_operands_in_pattern (rtx pattern)
 }
 }
 
-/* If output pattern of define_subst contains MATCH_DUP, then this
-   expression would be replaced with the pattern, matched with
-   MATCH_OPERAND from input pattern.  This pattern could contain any
-   number of MATCH_OPERANDs, MATCH_OPERATORs etc., so it's possible
-   that a MATCH_OPERAND from output_pattern (if any) would have the
-   same number, as MATCH_OPERAND from copied pattern.  To avoid such
-   indexes overlapping, we assign new indexes to MATCH_OPERANDs,
-   laying in the output pattern outside of MATCH_DUPs.  */
-static rtx
-adjust_operands_numbers (rtx pattern)
-{
-  mark_operands_used_in_match_dup (pattern);
-
-  renumerate_operands_in_pattern (pattern);
-
-  return pattern;
-}
 
 /* Generate RTL expression
(match_dup OPNO)
-- 
1.9.1

Re: [PATCH 1/9] gensupport: Fix define_subst operand renumbering.

2016-01-18 Thread Bernd Schmidt 

On 01/14/2016 05:33 PM, Andreas Krebbel wrote:

When processing substitutions the operands are renumbered.  To find a
free operand number the array used_operands_numbers is used to record
the operand numbers already in use.  Currently this array is used to
assign new numbers *before* all the RTXes in the vector have been
processed.



* gensupport.c (process_substs_on_one_elem): Split loop to
complete mark_operands_used_in_match_dup on all expressions in the
vector first.
(adjust_operands_numbers): Inline into process_substs_on_one_elem
and remove function.


Mostly ok, I think. As an aside, all the define_subst stuff in 
gensupport looks rather suspiciously clunky and the comments are in 
broken English. We should fix this stuff at some point.



@@ -1976,6 +1986,14 @@ find_first_unused_number_of_operand ()
 It visits all expressions in PATTERN and assigns not-occupied
 operand indexes to MATCH_OPERANDs and MATCH_OPERATORs of this
 PATTERN.  */
+/* If output pattern of define_subst contains MATCH_DUP, then this
+   expression would be replaced with the pattern, matched with
+   MATCH_OPERAND from input pattern.  This pattern could contain any
+   number of MATCH_OPERANDs, MATCH_OPERATORs etc., so it's possible
+   that a MATCH_OPERAND from output_pattern (if any) would have the
+   same number, as MATCH_OPERAND from copied pattern.  To avoid such
+   indexes overlapping, we assign new indexes to MATCH_OPERANDs,
+   laying in the output pattern outside of MATCH_DUPs.  */
  static void
  renumerate_operands_in_pattern (rtx pattern)
  {


If you want to keep this comment, you might want to move it inside the 
function (or into the caller). Ok with or without any such change - this 
looks a bit weird but I don't know what's best.



Bernd

[PATCH 1/9] gensupport: Fix define_subst operand renumbering.

2016-01-14 Thread Andreas Krebbel 
When processing substitutions the operands are renumbered.  To find a
free operand number the array used_operands_numbers is used to record
the operand numbers already in use.  Currently this array is used to
assign new numbers *before* all the RTXes in the vector have been
processed.

I did run into problems with this for insns where a match_dup occurred
in a later (use ...) operand referring to an earlier operand
(e.g. s390.md "setmem_long").

The patch splits the loop doing the processing into two in order to
have all the operand numbers recorded already when assigning new
numbers.

Bootstrapped and regtested on s390, s390x, and x86_64.

Ok for mainline?

Bye,

-Andreas-

gcc/ChangeLog:

2016-01-14  Andreas Krebbel  

* gensupport.c (process_substs_on_one_elem): Split loop to
complete mark_operands_used_in_match_dup on all expressions in the
vector first.
(adjust_operands_numbers): Inline into process_substs_on_one_elem
and remove function.
---
 gcc/gensupport.c | 41 +
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/gcc/gensupport.c b/gcc/gensupport.c
index 596b885..8ace425 100644
--- a/gcc/gensupport.c
+++ b/gcc/gensupport.c
@@ -126,7 +126,10 @@ static const char * duplicate_each_alternative (const char 
* str, int n_dup);
 
 typedef const char * (*constraints_handler_t) (const char *, int);
 static rtx alter_constraints (rtx, int, constraints_handler_t);
-static rtx adjust_operands_numbers (rtx);
+
+static void mark_operands_used_in_match_dup (rtx);
+static void renumerate_operands_in_pattern (rtx);
+
 static rtx replace_duplicating_operands_in_pattern (rtx);
 
 /* Make a version of gen_rtx_CONST_INT so that GEN_INT can be used in
@@ -1843,7 +1846,15 @@ process_substs_on_one_elem (struct queue_elem *elem,
  subst_pattern = alter_constraints (subst_pattern, alternatives,
 duplicate_each_alternative);
 
- subst_pattern = adjust_operands_numbers (subst_pattern);
+ mark_operands_used_in_match_dup (subst_pattern);
+ RTVEC_ELT (subst_pattern_vec, j) = subst_pattern;
+   }
+
+  for (j = 0; j < XVECLEN (subst_elem->data, 3); j++)
+   {
+ subst_pattern = RTVEC_ELT (subst_pattern_vec, j);
+
+ renumerate_operands_in_pattern (subst_pattern);
 
  /* Substitute match_dup and match_op_dup in the new pattern and
 duplicate constraints.  */
@@ -1856,7 +1867,6 @@ process_substs_on_one_elem (struct queue_elem *elem,
  if (GET_CODE (elem->data) == DEFINE_EXPAND)
remove_constraints (subst_pattern);
 
- RTVEC_ELT (subst_pattern_vec, j) = subst_pattern;
}
   XVEC (elem->data, 1) = subst_pattern_vec;
 
@@ -1976,6 +1986,14 @@ find_first_unused_number_of_operand ()
It visits all expressions in PATTERN and assigns not-occupied
operand indexes to MATCH_OPERANDs and MATCH_OPERATORs of this
PATTERN.  */
+/* If output pattern of define_subst contains MATCH_DUP, then this
+   expression would be replaced with the pattern, matched with
+   MATCH_OPERAND from input pattern.  This pattern could contain any
+   number of MATCH_OPERANDs, MATCH_OPERATORs etc., so it's possible
+   that a MATCH_OPERAND from output_pattern (if any) would have the
+   same number, as MATCH_OPERAND from copied pattern.  To avoid such
+   indexes overlapping, we assign new indexes to MATCH_OPERANDs,
+   laying in the output pattern outside of MATCH_DUPs.  */
 static void
 renumerate_operands_in_pattern (rtx pattern)
 {
@@ -2010,23 +2028,6 @@ renumerate_operands_in_pattern (rtx pattern)
 }
 }
 
-/* If output pattern of define_subst contains MATCH_DUP, then this
-   expression would be replaced with the pattern, matched with
-   MATCH_OPERAND from input pattern.  This pattern could contain any
-   number of MATCH_OPERANDs, MATCH_OPERATORs etc., so it's possible
-   that a MATCH_OPERAND from output_pattern (if any) would have the
-   same number, as MATCH_OPERAND from copied pattern.  To avoid such
-   indexes overlapping, we assign new indexes to MATCH_OPERANDs,
-   laying in the output pattern outside of MATCH_DUPs.  */
-static rtx
-adjust_operands_numbers (rtx pattern)
-{
-  mark_operands_used_in_match_dup (pattern);
-
-  renumerate_operands_in_pattern (pattern);
-
-  return pattern;
-}
 
 /* Generate RTL expression
(match_dup OPNO)
-- 
1.9.1