Re: PR82943 - Suggested patch to fix

[Harald Anlauf]

Am 20.01.24 um 23:42 schrieb Alexander Westbrooks:

Based on what I am reading here, I can either do the DCO path or the copy
right form path? Or is it both, where I send in the copy right forms and
then on every commit I put the DCO?


I thought the text is pretty clear.  As already mentioned,

  https://gcc.gnu.org/contribute.html#legal

gives you the options.  One of these options is:

"Alternatively, a contributor can certify the Developer Certificate of
Origin for their contribution by adding the Signed-off-by: tag to their
submission."

If you opt for this variant,

  https://gcc.gnu.org/dco.html

tells you everything.  Basically (but please read yourself):

"The sign-off is a simple line at the end of the explanation for the
patch, which certifies that you wrote it or otherwise have the right to
pass it on as a free software patch. ..."

[...]

"then you just add a line saying:

Signed-off-by: Random J Developer 

using your real name (sorry, no pseudonyms or anonymous contributions.) ..."

I think this would be sufficient.



On Sat, Jan 20, 2024 at 3:40 PM Harald Anlauf  wrote:


Am 20.01.24 um 21:37 schrieb Jerry D:

On 1/20/24 12:08 PM, Harald Anlauf wrote:

Am 20.01.24 um 20:08 schrieb Jerry D:

On 1/20/24 10:46 AM, Alexander Westbrooks wrote:

Hello and Happy New Year!

I wanted to follow up on this patch I made to address PR82943 for
GFortran. Has anyone had a chance to review it?

Thanks,

Alexander Westbrooks



Inserting myself in here just a little bit.  I will apply and test

today

if I can. Paul is unavailable for a few weeks. Harald can chime in.

Do you have commit rights for gcc?


I am not aware of Alex having a copyright assignment on file,
or a DCO certificate, and the patch is not signed off.
But I might be wrong.


--- snip ---

I do not mind committing this but need clarifications regarding the
copyright (copyleft?) rules in this case. In the past we have allowed
small contributions like this. This may be a little more than minor.


It is.  This is why I pointed to:

https://gcc.gnu.org/dco.html


Regardless it appears to do the job!

Jerry

Re: PR82943 - Suggested patch to fix

[Alexander Westbrooks]

Based on what I am reading here, I can either do the DCO path or the copy
right form path? Or is it both, where I send in the copy right forms and
then on every commit I put the DCO?

On Sat, Jan 20, 2024 at 3:40 PM Harald Anlauf  wrote:

> Am 20.01.24 um 21:37 schrieb Jerry D:
> > On 1/20/24 12:08 PM, Harald Anlauf wrote:
> >> Am 20.01.24 um 20:08 schrieb Jerry D:
> >>> On 1/20/24 10:46 AM, Alexander Westbrooks wrote:
>  Hello and Happy New Year!
> 
>  I wanted to follow up on this patch I made to address PR82943 for
>  GFortran. Has anyone had a chance to review it?
> 
>  Thanks,
> 
>  Alexander Westbrooks
> 
> >>>
> >>> Inserting myself in here just a little bit.  I will apply and test
> today
> >>> if I can. Paul is unavailable for a few weeks. Harald can chime in.
> >>>
> >>> Do you have commit rights for gcc?
> >>
> >> I am not aware of Alex having a copyright assignment on file,
> >> or a DCO certificate, and the patch is not signed off.
> >> But I might be wrong.
> >>
> > --- snip ---
> >
> > I do not mind committing this but need clarifications regarding the
> > copyright (copyleft?) rules in this case. In the past we have allowed
> > small contributions like this. This may be a little more than minor.
>
> It is.  This is why I pointed to:
>
> https://gcc.gnu.org/dco.html
>
> > Regardless it appears to do the job!
> >
> > Jerry
> >
> >
>
>

Re: PR82943 - Suggested patch to fix

[Harald Anlauf]

Am 20.01.24 um 21:37 schrieb Jerry D:

On 1/20/24 12:08 PM, Harald Anlauf wrote:

Am 20.01.24 um 20:08 schrieb Jerry D:

On 1/20/24 10:46 AM, Alexander Westbrooks wrote:

Hello and Happy New Year!

I wanted to follow up on this patch I made to address PR82943 for
GFortran. Has anyone had a chance to review it?

Thanks,

Alexander Westbrooks



Inserting myself in here just a little bit.  I will apply and test today
if I can. Paul is unavailable for a few weeks. Harald can chime in.

Do you have commit rights for gcc?


I am not aware of Alex having a copyright assignment on file,
or a DCO certificate, and the patch is not signed off.
But I might be wrong.


--- snip ---

I do not mind committing this but need clarifications regarding the
copyright (copyleft?) rules in this case. In the past we have allowed
small contributions like this. This may be a little more than minor.


It is.  This is why I pointed to:

https://gcc.gnu.org/dco.html


Regardless it appears to do the job!

Jerry

Re: PR82943 - Suggested patch to fix

[Jerry D]

On 1/20/24 12:08 PM, Harald Anlauf wrote:

Am 20.01.24 um 20:08 schrieb Jerry D:

On 1/20/24 10:46 AM, Alexander Westbrooks wrote:

Hello and Happy New Year!

I wanted to follow up on this patch I made to address PR82943 for
GFortran. Has anyone had a chance to review it?

Thanks,

Alexander Westbrooks



Inserting myself in here just a little bit.  I will apply and test today
if I can. Paul is unavailable for a few weeks. Harald can chime in.

Do you have commit rights for gcc?


I am not aware of Alex having a copyright assignment on file,
or a DCO certificate, and the patch is not signed off.
But I might be wrong.


--- snip ---

I do not mind committing this but need clarifications regarding the 
copyright (copyleft?) rules in this case. In the past we have allowed 
small contributions like this. This may be a little more than minor.


Regardless it appears to do the job!

Jerry

Re: PR82943 - Suggested patch to fix

[Harald Anlauf]

Am 20.01.24 um 20:08 schrieb Jerry D:

On 1/20/24 10:46 AM, Alexander Westbrooks wrote:

Hello and Happy New Year!

I wanted to follow up on this patch I made to address PR82943 for
GFortran. Has anyone had a chance to review it?

Thanks,

Alexander Westbrooks



Inserting myself in here just a little bit.  I will apply and test today
if I can. Paul is unavailable for a few weeks. Harald can chime in.

Do you have commit rights for gcc?


I am not aware of Alex having a copyright assignment on file,
or a DCO certificate, and the patch is not signed off.
But I might be wrong.


Your efforts are appreciated.

Regards,

Jerry

Re: PR82943 - Suggested patch to fix

[Jerry D]

On 1/20/24 11:08 AM, Jerry D wrote:

On 1/20/24 10:46 AM, Alexander Westbrooks wrote:

Hello and Happy New Year!

I wanted to follow up on this patch I made to address PR82943 for 
GFortran. Has anyone had a chance to review it?


Thanks,

Alexander Westbrooks



Inserting myself in here just a little bit.  I will apply and test today 
if I can. Paul is unavailable for a few weeks. Harald can chime in.


Do you have commit rights for gcc?

Your efforts are appreciated.

Regards,

Jerry





I did send you an invite to our Mattermost workspace.

I did apply your patch. Some comments.

The ChangeLog files are auto generated so do not get manually changed 
with a patch.  The push process to the gcc git repository will generate 
those from the git commit log.  If the git commit log is not formatted 
correctly the push will be rejected.


The patch attempts to create a PDT_33.f03 test case. There is one 
already there so probably rename that one.


In resolve.cc There was a compile error that looked like an extra '&&' 
in the conditional.  I deleted that and all compiled ok and all 
regression tested OK, including your new test cases and the existing 
PDT_33.f03 case.  I did not try your new test case yet for PDT_33.


So next steps are walk you through using the git scripts to make commits 
with the logs formatted correctly. (assuming no one else chimes in with 
any other comment about code changes themselves..


Regards,

Jerry

Re: PR82943 - Suggested patch to fix

[Jerry D]

On 1/20/24 10:46 AM, Alexander Westbrooks wrote:

Hello and Happy New Year!

I wanted to follow up on this patch I made to address PR82943 for 
GFortran. Has anyone had a chance to review it?


Thanks,

Alexander Westbrooks



Inserting myself in here just a little bit.  I will apply and test today 
if I can. Paul is unavailable for a few weeks. Harald can chime in.


Do you have commit rights for gcc?

Your efforts are appreciated.

Regards,

Jerry

Re: PR82943 - Suggested patch to fix

[Alexander Westbrooks]

Hello and Happy New Year!

I wanted to follow up on this patch I made to address PR82943 for GFortran.
Has anyone had a chance to review it?

Thanks,

Alexander Westbrooks

On Thu, Jun 29, 2023 at 10:38 PM Alexander Westbrooks 
wrote:

> Hello,
>
> I have finished my testing, and updated my patch and relevant Changelogs.
> I added 4 new tests and all the existing tests in the current testsuite
> for gfortran passed or failed as expected. Do I need to attach the test
> results here?
>
> The platform I tested on was a Docker container running in Docker Desktop,
> running the "mcr.microsoft.com/devcontainers/universal:2-linux" image.
>
> I also made sure that my code changes followed the coding standards.
> Please let me know if there is anything else that I need to do. I don't
> have write-access to the repository.
>
> Thanks,
>
> Alexander
>
> On Wed, Jun 28, 2023 at 4:14 PM Harald Anlauf  wrote:
>
>> Hi Alex,
>>
>> welcome to the gfortran community.  It is great that you are trying
>> to get actively involved.
>>
>> You already did quite a few things right: patches shall be sent to
>> the gcc-patches ML, but Fortran reviewers usually notice them only
>> where they are copied to the fortran ML.
>>
>> There are some general recommendations on the formatting of C code,
>> like indentation, of the patches, and of the commit log entries.
>>
>> Regarding coding standards, see https://www.gnu.org/prep/standards/ .
>>
>> Regarding testcases, a recommendation is to have a look at
>> existing testcases, e.g. in gcc/testsuite/gfortran.dg/, and then
>> decide if the testcase shall test the compile-time or run-time
>> behaviour, and add the necessary dejagnu directives.
>>
>> You should also verify if your patch passes regression testing.
>> For changes to gfortran, it is usually sufficient to run
>>
>> make check-fortran -j 
>>
>> where  is the number of parallel tests.
>> You would need to report also the platform where you tested on.
>>
>> There is also a legal issue to consider before non-trivial patches can
>> be accepted for incorporation: https://gcc.gnu.org/contribute.html#legal
>>
>> If your patch is accepted and if you do not have write-access to the
>> repository, one of the maintainers will likely take care of it.
>> If you become a regular contributor, you will probably want to consider
>> getting write access.
>>
>> Cheers,
>> Harald
>>
>>
>>
>> On 6/24/23 19:17, Alexander Westbrooks via Gcc-patches wrote:
>> > Hello,
>> >
>> > I am new to the GFortran community. Over the past two weeks I created a
>> > patch that should fix PR82943 for GFortran. I have attached it to this
>> > email. The patch allows the code below to compile successfully. I am
>> > working on creating test cases next, but I am new to the process so it
>> may
>> > take me some time. After I make test cases, do I email them to you as
>> well?
>> > Do I need to make a pull-request on github in order to get the patch
>> > reviewed?
>> >
>> > Thank you,
>> >
>> > Alexander Westbrooks
>> >
>> > module testmod
>> >
>> >  public :: foo
>> >
>> >  type, public :: tough_lvl_0(a, b)
>> >  integer, kind :: a = 1
>> >  integer, len :: b
>> >  contains
>> >  procedure :: foo
>> >  end type
>> >
>> >  type, public, EXTENDS(tough_lvl_0) :: tough_lvl_1 (c)
>> >  integer, len :: c
>> >  contains
>> >  procedure :: bar
>> >  end type
>> >
>> >  type, public, EXTENDS(tough_lvl_1) :: tough_lvl_2 (d)
>> >  integer, len :: d
>> >  contains
>> >  procedure :: foobar
>> >  end type
>> >
>> > contains
>> >  subroutine foo(this)
>> >  class(tough_lvl_0(1,*)), intent(inout) :: this
>> >  end subroutine
>> >
>> >  subroutine bar(this)
>> >  class(tough_lvl_1(1,*,*)), intent(inout) :: this
>> >  end subroutine
>> >
>> >  subroutine foobar(this)
>> >  class(tough_lvl_2(1,*,*,*)), intent(inout) :: this
>> >  end subroutine
>> >
>> > end module
>> >
>> > PROGRAM testprogram
>> >  USE testmod
>> >
>> >  TYPE(tough_lvl_0(1,5)) :: test_pdt_0
>> >  TYPE(tough_lvl_1(1,5,6))   :: test_pdt_1
>> >  TYPE(tough_lvl_2(1,5,6,7)) :: test_pdt_2
>> >
>> >  CALL test_pdt_0%foo()
>> >
>> >  CALL test_pdt_1%foo()
>> >  CALL test_pdt_1%bar()
>> >
>> >  CALL test_pdt_2%foo()
>> >  CALL test_pdt_2%bar()
>> >  CALL test_pdt_2%foobar()
>> >
>> >
>> > END PROGRAM testprogram
>>
>>

Re: PR82943 - Suggested patch to fix

[Alexander Westbrooks via Gcc-patches]

Hello,

I wanted to follow up on this, and ask what the next steps would be to
incorporate this patch?

Thanks,

Alexander Westbrooks


On Thu, Jun 29, 2023 at 10:38 PM Alexander Westbrooks 
wrote:

> Hello,
>
> I have finished my testing, and updated my patch and relevant Changelogs.
> I added 4 new tests and all the existing tests in the current testsuite
> for gfortran passed or failed as expected. Do I need to attach the test
> results here?
>
> The platform I tested on was a Docker container running in Docker Desktop,
> running the "mcr.microsoft.com/devcontainers/universal:2-linux" image.
>
> I also made sure that my code changes followed the coding standards.
> Please let me know if there is anything else that I need to do. I don't
> have write-access to the repository.
>
> Thanks,
>
> Alexander
>
> On Wed, Jun 28, 2023 at 4:14 PM Harald Anlauf  wrote:
>
>> Hi Alex,
>>
>> welcome to the gfortran community.  It is great that you are trying
>> to get actively involved.
>>
>> You already did quite a few things right: patches shall be sent to
>> the gcc-patches ML, but Fortran reviewers usually notice them only
>> where they are copied to the fortran ML.
>>
>> There are some general recommendations on the formatting of C code,
>> like indentation, of the patches, and of the commit log entries.
>>
>> Regarding coding standards, see https://www.gnu.org/prep/standards/ .
>>
>> Regarding testcases, a recommendation is to have a look at
>> existing testcases, e.g. in gcc/testsuite/gfortran.dg/, and then
>> decide if the testcase shall test the compile-time or run-time
>> behaviour, and add the necessary dejagnu directives.
>>
>> You should also verify if your patch passes regression testing.
>> For changes to gfortran, it is usually sufficient to run
>>
>> make check-fortran -j 
>>
>> where  is the number of parallel tests.
>> You would need to report also the platform where you tested on.
>>
>> There is also a legal issue to consider before non-trivial patches can
>> be accepted for incorporation: https://gcc.gnu.org/contribute.html#legal
>>
>> If your patch is accepted and if you do not have write-access to the
>> repository, one of the maintainers will likely take care of it.
>> If you become a regular contributor, you will probably want to consider
>> getting write access.
>>
>> Cheers,
>> Harald
>>
>>
>>
>> On 6/24/23 19:17, Alexander Westbrooks via Gcc-patches wrote:
>> > Hello,
>> >
>> > I am new to the GFortran community. Over the past two weeks I created a
>> > patch that should fix PR82943 for GFortran. I have attached it to this
>> > email. The patch allows the code below to compile successfully. I am
>> > working on creating test cases next, but I am new to the process so it
>> may
>> > take me some time. After I make test cases, do I email them to you as
>> well?
>> > Do I need to make a pull-request on github in order to get the patch
>> > reviewed?
>> >
>> > Thank you,
>> >
>> > Alexander Westbrooks
>> >
>> > module testmod
>> >
>> >  public :: foo
>> >
>> >  type, public :: tough_lvl_0(a, b)
>> >  integer, kind :: a = 1
>> >  integer, len :: b
>> >  contains
>> >  procedure :: foo
>> >  end type
>> >
>> >  type, public, EXTENDS(tough_lvl_0) :: tough_lvl_1 (c)
>> >  integer, len :: c
>> >  contains
>> >  procedure :: bar
>> >  end type
>> >
>> >  type, public, EXTENDS(tough_lvl_1) :: tough_lvl_2 (d)
>> >  integer, len :: d
>> >  contains
>> >  procedure :: foobar
>> >  end type
>> >
>> > contains
>> >  subroutine foo(this)
>> >  class(tough_lvl_0(1,*)), intent(inout) :: this
>> >  end subroutine
>> >
>> >  subroutine bar(this)
>> >  class(tough_lvl_1(1,*,*)), intent(inout) :: this
>> >  end subroutine
>> >
>> >  subroutine foobar(this)
>> >  class(tough_lvl_2(1,*,*,*)), intent(inout) :: this
>> >  end subroutine
>> >
>> > end module
>> >
>> > PROGRAM testprogram
>> >  USE testmod
>> >
>> >  TYPE(tough_lvl_0(1,5)) :: test_pdt_0
>> >  TYPE(tough_lvl_1(1,5,6))   :: test_pdt_1
>> >  TYPE(tough_lvl_2(1,5,6,7)) :: test_pdt_2
>> >
>> >  CALL test_pdt_0%foo()
>> >
>> >  CALL test_pdt_1%foo()
>> >  CALL test_pdt_1%bar()
>> >
>> >  CALL test_pdt_2%foo()
>> >  CALL test_pdt_2%bar()
>> >  CALL test_pdt_2%foobar()
>> >
>> >
>> > END PROGRAM testprogram
>>
>>

Re: PR82943 - Suggested patch to fix

[Paul Richard Thomas via Gcc-patches]

Hi All,

I have gone through the PDT problem reports and made sure that they
block PR82173.

To my utter astonishment (i) There might be only one duplicate; and
(ii) Only 82649, 84119, 90218, 95541, 99079, 102901 & 105380 (out of
50 PRs) depend on the representation.

Regards

Paul

Re: PR82943 - Suggested patch to fix

[Paul Richard Thomas via Gcc-patches]

Hi Alexander,

I suggest that you take a look at PR82649 before going too far down
the road of fixing PDT bugs. This PR underlines just how wrong the PDT
representation is - mea culpa!

The mechanics for constructing PDTs are in
decl.cc(gfc_get_pdt_instance). They need to be turned inside out to
create a container, not unlike the class containers, with
{data-field(assumed rank array?), kind and len parameters}. This will
then trigger all manner of failures in trans-***.cc in particular.

It had been my intention to turn to PDTs after I complete my scourge
of associate construct bugs. If you want to take this on, please do so
and I will give you all the help that I can. You will see from PR82649
that I have been promising to get on to this for a long time but have
not had the time thus far :-( If you want to get on with parsing bugs
to start with, please be my guest!

I notice that searching for PDT in PR title lines generates 48 hits,
while the PDT meta-bug PR82173 only has 28 blockers. I will get on
with the housekeeping this weekend by updating PR82173 and eliminating
duplicates.

Welcome, Alexander!

Paul

On Fri, 30 Jun 2023 at 05:42, Steve Kargl via Fortran
 wrote:
>
> On Thu, Jun 29, 2023 at 10:38:42PM -0500, Alexander Westbrooks via Fortran 
> wrote:
> > I have finished my testing, and updated my patch and relevant Changelogs. I
> > added 4 new tests and all the existing tests in the current testsuite
> > for gfortran passed or failed as expected. Do I need to attach the test
> > results here?
>
> Yes.  It helps others also do testing to have one self-contained
> patch (which I don't know to generate with git and new files :-( ).
> It may also be a good idea to attach the patch and test cases to
> the PR in bugzilla so that they don't accidentally get lost.
>
> > The platform I tested on was a Docker container running in Docker Desktop,
> > running the "mcr.microsoft.com/devcontainers/universal:2-linux" image.
> >
> > I also made sure that my code changes followed the coding standards. Please
> > let me know if there is anything else that I need to do. I don't have
> > write-access to the repository.
>
> See the legal link that Harald provided.  At one time, one needed to
> assign copyright to the FSF with a wet-ink signature on some form.
> Now, I think you just need to attest that you have the right to
> provide the code to the gcc project.
>
> PS: Welcome to the gfortran development world.  Don't be put off
> if there is a delay in getting feedback/review.  There are too
> few contributors and too little time.   If a week passes simply
> ping the mailing list.  I'll try to carve out some time to look
> over your patch this weekend.
>
> --
> steve
>
>
> >
> > Thanks,
> >
> > Alexander
> >
> > On Wed, Jun 28, 2023 at 4:14 PM Harald Anlauf  wrote:
> >
> > > Hi Alex,
> > >
> > > welcome to the gfortran community.  It is great that you are trying
> > > to get actively involved.
> > >
> > > You already did quite a few things right: patches shall be sent to
> > > the gcc-patches ML, but Fortran reviewers usually notice them only
> > > where they are copied to the fortran ML.
> > >
> > > There are some general recommendations on the formatting of C code,
> > > like indentation, of the patches, and of the commit log entries.
> > >
> > > Regarding coding standards, see https://www.gnu.org/prep/standards/ .
> > >
> > > Regarding testcases, a recommendation is to have a look at
> > > existing testcases, e.g. in gcc/testsuite/gfortran.dg/, and then
> > > decide if the testcase shall test the compile-time or run-time
> > > behaviour, and add the necessary dejagnu directives.
> > >
> > > You should also verify if your patch passes regression testing.
> > > For changes to gfortran, it is usually sufficient to run
> > >
> > > make check-fortran -j 
> > >
> > > where  is the number of parallel tests.
> > > You would need to report also the platform where you tested on.
> > >
> > > There is also a legal issue to consider before non-trivial patches can
> > > be accepted for incorporation: https://gcc.gnu.org/contribute.html#legal
> > >
> > > If your patch is accepted and if you do not have write-access to the
> > > repository, one of the maintainers will likely take care of it.
> > > If you become a regular contributor, you will probably want to consider
> > > getting write access.
> > >
> > > Cheers,
> > > Harald
> > >
> > >
> > >
> > > On 6/24/23 19:17, Alexander Westbrooks via Gcc-patches wrote:
> > > > Hello,
> > > >
> > > > I am new to the GFortran community. Over the past two weeks I created a
> > > > patch that should fix PR82943 for GFortran. I have attached it to this
> > > > email. The patch allows the code below to compile successfully. I am
> > > > working on creating test cases next, but I am new to the process so it
> > > may
> > > > take me some time. After I make test cases, do I email them to you as
> > > well?
> > > > Do I need to make a pull-request on github in order to get the patch
> > > > revi

Re: PR82943 - Suggested patch to fix

[Steve Kargl via Gcc-patches]

On Thu, Jun 29, 2023 at 10:38:42PM -0500, Alexander Westbrooks via Fortran 
wrote:
> I have finished my testing, and updated my patch and relevant Changelogs. I
> added 4 new tests and all the existing tests in the current testsuite
> for gfortran passed or failed as expected. Do I need to attach the test
> results here?

Yes.  It helps others also do testing to have one self-contained
patch (which I don't know to generate with git and new files :-( ).
It may also be a good idea to attach the patch and test cases to
the PR in bugzilla so that they don't accidentally get lost.

> The platform I tested on was a Docker container running in Docker Desktop,
> running the "mcr.microsoft.com/devcontainers/universal:2-linux" image.
> 
> I also made sure that my code changes followed the coding standards. Please
> let me know if there is anything else that I need to do. I don't have
> write-access to the repository.

See the legal link that Harald provided.  At one time, one needed to
assign copyright to the FSF with a wet-ink signature on some form.
Now, I think you just need to attest that you have the right to
provide the code to the gcc project.

PS: Welcome to the gfortran development world.  Don't be put off
if there is a delay in getting feedback/review.  There are too 
few contributors and too little time.   If a week passes simply
ping the mailing list.  I'll try to carve out some time to look
over your patch this weekend.

-- 
steve


> 
> Thanks,
> 
> Alexander
> 
> On Wed, Jun 28, 2023 at 4:14 PM Harald Anlauf  wrote:
> 
> > Hi Alex,
> >
> > welcome to the gfortran community.  It is great that you are trying
> > to get actively involved.
> >
> > You already did quite a few things right: patches shall be sent to
> > the gcc-patches ML, but Fortran reviewers usually notice them only
> > where they are copied to the fortran ML.
> >
> > There are some general recommendations on the formatting of C code,
> > like indentation, of the patches, and of the commit log entries.
> >
> > Regarding coding standards, see https://www.gnu.org/prep/standards/ .
> >
> > Regarding testcases, a recommendation is to have a look at
> > existing testcases, e.g. in gcc/testsuite/gfortran.dg/, and then
> > decide if the testcase shall test the compile-time or run-time
> > behaviour, and add the necessary dejagnu directives.
> >
> > You should also verify if your patch passes regression testing.
> > For changes to gfortran, it is usually sufficient to run
> >
> > make check-fortran -j 
> >
> > where  is the number of parallel tests.
> > You would need to report also the platform where you tested on.
> >
> > There is also a legal issue to consider before non-trivial patches can
> > be accepted for incorporation: https://gcc.gnu.org/contribute.html#legal
> >
> > If your patch is accepted and if you do not have write-access to the
> > repository, one of the maintainers will likely take care of it.
> > If you become a regular contributor, you will probably want to consider
> > getting write access.
> >
> > Cheers,
> > Harald
> >
> >
> >
> > On 6/24/23 19:17, Alexander Westbrooks via Gcc-patches wrote:
> > > Hello,
> > >
> > > I am new to the GFortran community. Over the past two weeks I created a
> > > patch that should fix PR82943 for GFortran. I have attached it to this
> > > email. The patch allows the code below to compile successfully. I am
> > > working on creating test cases next, but I am new to the process so it
> > may
> > > take me some time. After I make test cases, do I email them to you as
> > well?
> > > Do I need to make a pull-request on github in order to get the patch
> > > reviewed?
> > >
> > > Thank you,
> > >
> > > Alexander Westbrooks
> > >
> > > module testmod
> > >
> > >  public :: foo
> > >
> > >  type, public :: tough_lvl_0(a, b)
> > >  integer, kind :: a = 1
> > >  integer, len :: b
> > >  contains
> > >  procedure :: foo
> > >  end type
> > >
> > >  type, public, EXTENDS(tough_lvl_0) :: tough_lvl_1 (c)
> > >  integer, len :: c
> > >  contains
> > >  procedure :: bar
> > >  end type
> > >
> > >  type, public, EXTENDS(tough_lvl_1) :: tough_lvl_2 (d)
> > >  integer, len :: d
> > >  contains
> > >  procedure :: foobar
> > >  end type
> > >
> > > contains
> > >  subroutine foo(this)
> > >  class(tough_lvl_0(1,*)), intent(inout) :: this
> > >  end subroutine
> > >
> > >  subroutine bar(this)
> > >  class(tough_lvl_1(1,*,*)), intent(inout) :: this
> > >  end subroutine
> > >
> > >  subroutine foobar(this)
> > >  class(tough_lvl_2(1,*,*,*)), intent(inout) :: this
> > >  end subroutine
> > >
> > > end module
> > >
> > > PROGRAM testprogram
> > >  USE testmod
> > >
> > >  TYPE(tough_lvl_0(1,5)) :: test_pdt_0
> > >  TYPE(tough_lvl_1(1,5,6))   :: test_pdt_1
> > >  TYPE(tough_lvl_2(1,5,6,7)) :: test_pdt_2
> > >
> > >  CALL test_pdt_0%foo()
> > >
> > >  

Re: PR82943 - Suggested patch to fix

[Alexander Westbrooks via Gcc-patches]

Hello,

I have finished my testing, and updated my patch and relevant Changelogs. I
added 4 new tests and all the existing tests in the current testsuite
for gfortran passed or failed as expected. Do I need to attach the test
results here?

The platform I tested on was a Docker container running in Docker Desktop,
running the "mcr.microsoft.com/devcontainers/universal:2-linux" image.

I also made sure that my code changes followed the coding standards. Please
let me know if there is anything else that I need to do. I don't have
write-access to the repository.

Thanks,

Alexander

On Wed, Jun 28, 2023 at 4:14 PM Harald Anlauf  wrote:

> Hi Alex,
>
> welcome to the gfortran community.  It is great that you are trying
> to get actively involved.
>
> You already did quite a few things right: patches shall be sent to
> the gcc-patches ML, but Fortran reviewers usually notice them only
> where they are copied to the fortran ML.
>
> There are some general recommendations on the formatting of C code,
> like indentation, of the patches, and of the commit log entries.
>
> Regarding coding standards, see https://www.gnu.org/prep/standards/ .
>
> Regarding testcases, a recommendation is to have a look at
> existing testcases, e.g. in gcc/testsuite/gfortran.dg/, and then
> decide if the testcase shall test the compile-time or run-time
> behaviour, and add the necessary dejagnu directives.
>
> You should also verify if your patch passes regression testing.
> For changes to gfortran, it is usually sufficient to run
>
> make check-fortran -j 
>
> where  is the number of parallel tests.
> You would need to report also the platform where you tested on.
>
> There is also a legal issue to consider before non-trivial patches can
> be accepted for incorporation: https://gcc.gnu.org/contribute.html#legal
>
> If your patch is accepted and if you do not have write-access to the
> repository, one of the maintainers will likely take care of it.
> If you become a regular contributor, you will probably want to consider
> getting write access.
>
> Cheers,
> Harald
>
>
>
> On 6/24/23 19:17, Alexander Westbrooks via Gcc-patches wrote:
> > Hello,
> >
> > I am new to the GFortran community. Over the past two weeks I created a
> > patch that should fix PR82943 for GFortran. I have attached it to this
> > email. The patch allows the code below to compile successfully. I am
> > working on creating test cases next, but I am new to the process so it
> may
> > take me some time. After I make test cases, do I email them to you as
> well?
> > Do I need to make a pull-request on github in order to get the patch
> > reviewed?
> >
> > Thank you,
> >
> > Alexander Westbrooks
> >
> > module testmod
> >
> >  public :: foo
> >
> >  type, public :: tough_lvl_0(a, b)
> >  integer, kind :: a = 1
> >  integer, len :: b
> >  contains
> >  procedure :: foo
> >  end type
> >
> >  type, public, EXTENDS(tough_lvl_0) :: tough_lvl_1 (c)
> >  integer, len :: c
> >  contains
> >  procedure :: bar
> >  end type
> >
> >  type, public, EXTENDS(tough_lvl_1) :: tough_lvl_2 (d)
> >  integer, len :: d
> >  contains
> >  procedure :: foobar
> >  end type
> >
> > contains
> >  subroutine foo(this)
> >  class(tough_lvl_0(1,*)), intent(inout) :: this
> >  end subroutine
> >
> >  subroutine bar(this)
> >  class(tough_lvl_1(1,*,*)), intent(inout) :: this
> >  end subroutine
> >
> >  subroutine foobar(this)
> >  class(tough_lvl_2(1,*,*,*)), intent(inout) :: this
> >  end subroutine
> >
> > end module
> >
> > PROGRAM testprogram
> >  USE testmod
> >
> >  TYPE(tough_lvl_0(1,5)) :: test_pdt_0
> >  TYPE(tough_lvl_1(1,5,6))   :: test_pdt_1
> >  TYPE(tough_lvl_2(1,5,6,7)) :: test_pdt_2
> >
> >  CALL test_pdt_0%foo()
> >
> >  CALL test_pdt_1%foo()
> >  CALL test_pdt_1%bar()
> >
> >  CALL test_pdt_2%foo()
> >  CALL test_pdt_2%bar()
> >  CALL test_pdt_2%foobar()
> >
> >
> > END PROGRAM testprogram
>
>


0001-Fix-fortran-PR82943-PR86148-and-PR86268.patch
Description: Binary data

Re: PR82943 - Suggested patch to fix

[Harald Anlauf via Gcc-patches]

Hi Alex,

welcome to the gfortran community.  It is great that you are trying
to get actively involved.

You already did quite a few things right: patches shall be sent to
the gcc-patches ML, but Fortran reviewers usually notice them only
where they are copied to the fortran ML.

There are some general recommendations on the formatting of C code,
like indentation, of the patches, and of the commit log entries.

Regarding coding standards, see https://www.gnu.org/prep/standards/ .

Regarding testcases, a recommendation is to have a look at
existing testcases, e.g. in gcc/testsuite/gfortran.dg/, and then
decide if the testcase shall test the compile-time or run-time
behaviour, and add the necessary dejagnu directives.

You should also verify if your patch passes regression testing.
For changes to gfortran, it is usually sufficient to run

make check-fortran -j 

where  is the number of parallel tests.
You would need to report also the platform where you tested on.

There is also a legal issue to consider before non-trivial patches can
be accepted for incorporation: https://gcc.gnu.org/contribute.html#legal

If your patch is accepted and if you do not have write-access to the
repository, one of the maintainers will likely take care of it.
If you become a regular contributor, you will probably want to consider
getting write access.

Cheers,
Harald



On 6/24/23 19:17, Alexander Westbrooks via Gcc-patches wrote:

Hello,

I am new to the GFortran community. Over the past two weeks I created a
patch that should fix PR82943 for GFortran. I have attached it to this
email. The patch allows the code below to compile successfully. I am
working on creating test cases next, but I am new to the process so it may
take me some time. After I make test cases, do I email them to you as well?
Do I need to make a pull-request on github in order to get the patch
reviewed?

Thank you,

Alexander Westbrooks

module testmod

 public :: foo

 type, public :: tough_lvl_0(a, b)
 integer, kind :: a = 1
 integer, len :: b
 contains
 procedure :: foo
 end type

 type, public, EXTENDS(tough_lvl_0) :: tough_lvl_1 (c)
 integer, len :: c
 contains
 procedure :: bar
 end type

 type, public, EXTENDS(tough_lvl_1) :: tough_lvl_2 (d)
 integer, len :: d
 contains
 procedure :: foobar
 end type

contains
 subroutine foo(this)
 class(tough_lvl_0(1,*)), intent(inout) :: this
 end subroutine

 subroutine bar(this)
 class(tough_lvl_1(1,*,*)), intent(inout) :: this
 end subroutine

 subroutine foobar(this)
 class(tough_lvl_2(1,*,*,*)), intent(inout) :: this
 end subroutine

end module

PROGRAM testprogram
 USE testmod

 TYPE(tough_lvl_0(1,5)) :: test_pdt_0
 TYPE(tough_lvl_1(1,5,6))   :: test_pdt_1
 TYPE(tough_lvl_2(1,5,6,7)) :: test_pdt_2

 CALL test_pdt_0%foo()

 CALL test_pdt_1%foo()
 CALL test_pdt_1%bar()

 CALL test_pdt_2%foo()
 CALL test_pdt_2%bar()
 CALL test_pdt_2%foobar()


END PROGRAM testprogram

PR82943 - Suggested patch to fix

[Alexander Westbrooks via Gcc-patches]

Hello,

I am new to the GFortran community. Over the past two weeks I created a
patch that should fix PR82943 for GFortran. I have attached it to this
email. The patch allows the code below to compile successfully. I am
working on creating test cases next, but I am new to the process so it may
take me some time. After I make test cases, do I email them to you as well?
Do I need to make a pull-request on github in order to get the patch
reviewed?

Thank you,

Alexander Westbrooks

module testmod

public :: foo

type, public :: tough_lvl_0(a, b)
integer, kind :: a = 1
integer, len :: b
contains
procedure :: foo
end type

type, public, EXTENDS(tough_lvl_0) :: tough_lvl_1 (c)
integer, len :: c
contains
procedure :: bar
end type

type, public, EXTENDS(tough_lvl_1) :: tough_lvl_2 (d)
integer, len :: d
contains
procedure :: foobar
end type

contains
subroutine foo(this)
class(tough_lvl_0(1,*)), intent(inout) :: this
end subroutine

subroutine bar(this)
class(tough_lvl_1(1,*,*)), intent(inout) :: this
end subroutine

subroutine foobar(this)
class(tough_lvl_2(1,*,*,*)), intent(inout) :: this
end subroutine

end module

PROGRAM testprogram
USE testmod

TYPE(tough_lvl_0(1,5)) :: test_pdt_0
TYPE(tough_lvl_1(1,5,6))   :: test_pdt_1
TYPE(tough_lvl_2(1,5,6,7)) :: test_pdt_2

CALL test_pdt_0%foo()

CALL test_pdt_1%foo()
CALL test_pdt_1%bar()

CALL test_pdt_2%foo()
CALL test_pdt_2%bar()
CALL test_pdt_2%foobar()


END PROGRAM testprogram


0001-bug-patch-PR82943.patch
Description: Binary data

Re: [PATCH v2 0/2] Series of patch to fix PR106594

[Richard Sandiford via Gcc-patches]

Ping

https://gcc.gnu.org/pipermail/gcc-patches/2023-March/613640.html

Richard Sandiford  writes:
> This series of patches fixes PR106594, an aarch64 regression in which
> we fail to combine an extension into an address.  The first patch just
> refactors code.  The second patch contains the actual fix.
>
> The cover note for the second patch describes the problem and the fix.
>
> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
>
> Richard

[PATCH v2 0/2] Series of patch to fix PR106594

[Richard Sandiford via Gcc-patches]

This series of patches fixes PR106594, an aarch64 regression in which
we fail to combine an extension into an address.  The first patch just
refactors code.  The second patch contains the actual fix.

The cover note for the second patch describes the problem and the fix.

Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

Richard

Re: realpath() patch to fix symlinks resolution for win32

[Martin Liška]

On 2/11/23 22:14, Gerald Pfeifer wrote:

On Sat, 11 Feb 2023, NightStrike wrote:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108350

I would have expected the PR to have been automatically updated based on
the commit email. Any idea why that didn't happen? Not to change the state
to closed, but to add the commit information as a reply.


I assume the fact that the PR reference was spelt as
   PR/108350
without a slash, not a blank, after "PR" may be responsible for the
missing Bugzilla comment.

The documented format - per gcc.gnu.org/codingconventions.html - is
   PR component/12345


It's likely what happens.



Martin? (By the way, where does one best have a look at those hooks?
.git/hooks in the main repository isn't it, it appears?)


I know that server hooks (gcc.gnu.org:/home/gccadmin/hooks-bin) do contain:
email-to-bugzilla-filtered:BUGZILLA_CMD = ("/sourceware/infra/bin/email-to-bugzilla", 
"-G", "gcc")

thus I guess it's /sourceware/infra/bin/email-to-bugzilla what makes the 
filtering of commits
to bugzilla.

Martin



Gerald

Re: realpath() patch to fix symlinks resolution for win32

[Gerald Pfeifer]

On Sat, 11 Feb 2023, NightStrike wrote:
 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108350
> I would have expected the PR to have been automatically updated based on
> the commit email. Any idea why that didn't happen? Not to change the state
> to closed, but to add the commit information as a reply.

I assume the fact that the PR reference was spelt as
  PR/108350
without a slash, not a blank, after "PR" may be responsible for the 
missing Bugzilla comment.

The documented format - per gcc.gnu.org/codingconventions.html - is
  PR component/12345

Martin? (By the way, where does one best have a look at those hooks?
.git/hooks in the main repository isn't it, it appears?)

Gerald

Re: realpath() patch to fix symlinks resolution for win32

[NightStrike via Gcc-patches]

On Sat, Feb 11, 2023 at 3:52 AM Gerald Pfeifer  wrote:
>
> On Sat, 11 Feb 2023, Jonathan Yong via Gcc-patches wrote:
> >> could you please close the corresponding BR too?:
> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108350
> > I can't close it, but I put a note that it has been committed.
>
> I closed the report.
>
> Gerald

I would have expected the PR to have been automatically updated based on
the commit email. Any idea why that didn't happen? Not to change the state
to closed, but to add the commit information as a reply.

Re: realpath() patch to fix symlinks resolution for win32

[Jonathan Yong via Gcc-patches]

On 2/11/23 08:52, Gerald Pfeifer wrote:

On Sat, 11 Feb 2023, Jonathan Yong via Gcc-patches wrote:

could you please close the corresponding BR too?:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108350

I can't close it, but I put a note that it has been committed.


I closed the report.

Gerald


Thanks.

Re: realpath() patch to fix symlinks resolution for win32

[Gerald Pfeifer]

On Sat, 11 Feb 2023, Jonathan Yong via Gcc-patches wrote:
>> could you please close the corresponding BR too?:
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108350
> I can't close it, but I put a note that it has been committed.

I closed the report.

Gerald

Re: realpath() patch to fix symlinks resolution for win32

[Jonathan Yong via Gcc-patches]

On 2/11/23 07:28, i.nix...@autistici.org wrote:


Thank you Jonathan!

could you please close the corresponding BR too?:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108350



best!


I can't close it, but I put a note that it has been committed.

Re: realpath() patch to fix symlinks resolution for win32

[i.nixman--- via Gcc-patches]

On 2023-02-11 06:32, Jonathan Yong wrote:

On 2/6/23 06:40, Jonathan Yong wrote:

On 1/18/23 10:44, i.nixman--- via Gcc-patches wrote:

hello again!

the final version of the path for 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108350


successfully bootstraped for x86_64-mingw32 and x86_64-linux.

could anyone apply it please?



best!


Looks good to me, please supply the appropriate changelog.


Pushed to master.



Thank you Jonathan!

could you please close the corresponding BR too?:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108350



best!

Re: realpath() patch to fix symlinks resolution for win32

[Jonathan Yong via Gcc-patches]

On 2/6/23 06:40, Jonathan Yong wrote:

On 1/18/23 10:44, i.nixman--- via Gcc-patches wrote:

hello again!

the final version of the path for 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108350


successfully bootstraped for x86_64-mingw32 and x86_64-linux.

could anyone apply it please?



best!


Looks good to me, please supply the appropriate changelog.


Pushed to master.

Re: realpath() patch to fix symlinks resolution for win32

[Jonathan Yong via Gcc-patches]

On 1/18/23 10:44, i.nixman--- via Gcc-patches wrote:

hello again!

the final version of the path for 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108350


successfully bootstraped for x86_64-mingw32 and x86_64-linux.

could anyone apply it please?



best!


Looks good to me, please supply the appropriate changelog.

Re: realpath() patch to fix symlinks resolution for win32

[i.nixman--- via Gcc-patches]

hello,

could someone look at patch attached please?

https://gcc.gnu.org/pipermail/gcc-patches/2023-January/610147.html

realpath() patch to fix symlinks resolution for win32

[i.nixman--- via Gcc-patches]

hello again!

the final version of the path for 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108350


successfully bootstraped for x86_64-mingw32 and x86_64-linux.

could anyone apply it please?



best!
diff --git a/libiberty/lrealpath.c b/libiberty/lrealpath.c
index 3c7053b0b70..a1ad074d00e 100644
--- a/libiberty/lrealpath.c
+++ b/libiberty/lrealpath.c
@@ -68,8 +68,135 @@ extern char *canonicalize_file_name (const char *);
   /* cygwin has realpath, so it won't get here.  */ 
 # if defined (_WIN32)
 #  define WIN32_LEAN_AND_MEAN
-#  include  /* for GetFullPathName */
-# endif
+#  include  /* for GetFullPathName/GetFinalPathNameByHandle/
+  CreateFile/CloseHandle */
+#  define WIN32_REPLACE_SLASHES(_ptr, _len) \
+ for (unsigned i = 0; i != (_len); ++i) \
+   if ((_ptr)[i] == '\\') (_ptr)[i] = '/';
+
+#  define WIN32_UNC_PREFIX "//?/UNC/"
+#  define WIN32_UNC_PREFIX_LEN (sizeof(WIN32_UNC_PREFIX)-1)
+#  define WIN32_IS_UNC_PREFIX(ptr) \
+  (0 == memcmp(ptr, WIN32_UNC_PREFIX, WIN32_UNC_PREFIX_LEN))
+
+#  define WIN32_NON_UNC_PREFIX "//?/"
+#  define WIN32_NON_UNC_PREFIX_LEN (sizeof(WIN32_NON_UNC_PREFIX)-1)
+#  define WIN32_IS_NON_UNC_PREFIX(ptr) \
+  (0 == memcmp(ptr, WIN32_NON_UNC_PREFIX, WIN32_NON_UNC_PREFIX_LEN))
+
+/* Get full path name without symlinks resolution.
+   It also converts all forward slashes to back slashes.
+*/
+char* get_full_path_name(const char *filename) {
+  DWORD len;
+  char *buf, *ptr, *res;
+
+  /* determining the required buffer size.
+ from the man: `If the lpBuffer buffer is too small to contain
+ the path, the return value is the size, in TCHARs, of the buffer
+ that is required to hold the path _and_the_terminating_null_character_`
+  */
+  len = GetFullPathName(filename, 0, NULL, NULL);
+
+  if ( len == 0 )
+return strdup(filename);
+
+  buf = (char *)malloc(len);
+
+  /* no point to check the result again */
+  len = GetFullPathName(filename, len, buf, NULL);
+  buf[len] = 0;
+
+  /* replace slashes */
+  WIN32_REPLACE_SLASHES(buf, len);
+
+  /* calculate offset based on prefix type */
+  len = WIN32_IS_UNC_PREFIX(buf)
+? (WIN32_UNC_PREFIX_LEN - 2)
+: WIN32_IS_NON_UNC_PREFIX(buf)
+  ? WIN32_NON_UNC_PREFIX_LEN
+  : 0
+  ;
+
+  ptr = buf + len;
+  if ( WIN32_IS_UNC_PREFIX(buf) ) {
+ptr[0] = '/';
+ptr[1] = '/';
+  }
+
+  res = strdup(ptr);
+
+  free(buf);
+
+  return res;
+}
+
+# if _WIN32_WINNT >= 0x0600
+
+/* Get full path name WITH symlinks resolution.
+   It also converts all forward slashes to back slashes.
+*/
+char* get_final_path_name(HANDLE fh) {
+  DWORD len;
+  char *buf, *ptr, *res;
+
+  /* determining the required buffer size.
+ from the  man: `If the function fails because lpszFilePath is too
+ small to hold the string plus the terminating null character,
+ the return value is the required buffer size, in TCHARs. This
+ value _includes_the_size_of_the_terminating_null_character_`.
+ but in my testcase I have path with 26 chars, the function
+ returns 26 also, ie without the trailing zero-char...
+  */
+  len = GetFinalPathNameByHandle(
+ fh
+,NULL
+,0
+,FILE_NAME_NORMALIZED | VOLUME_NAME_DOS
+  );
+
+  if ( len == 0 )
+return NULL;
+
+  len += 1; /* for zero-char */
+  buf = (char *)malloc(len);
+
+  /* no point to check the result again */
+  len = GetFinalPathNameByHandle(
+ fh
+,buf
+,len
+,FILE_NAME_NORMALIZED | VOLUME_NAME_DOS
+  );
+  buf[len] = 0;
+
+  /* replace slashes */
+  WIN32_REPLACE_SLASHES(buf, len);
+
+  /* calculate offset based on prefix type */
+  len = WIN32_IS_UNC_PREFIX(buf)
+? (WIN32_UNC_PREFIX_LEN - 2)
+: WIN32_IS_NON_UNC_PREFIX(buf)
+  ? WIN32_NON_UNC_PREFIX_LEN
+  : 0
+  ;
+
+  ptr = buf + len;
+  if ( WIN32_IS_UNC_PREFIX(buf) ) {
+ptr[0] = '/';
+ptr[1] = '/';
+  }
+
+  res = strdup(ptr);
+
+  free(buf);
+
+  return res;
+}
+
+# endif // _WIN32_WINNT >= 0x0600
+
+# endif // _WIN32
 #endif
 
 char *
@@ -128,30 +255,52 @@ lrealpath (const char *filename)
   }
 #endif
 
-  /* The MS Windows method.  If we don't have realpath, we assume we
- don't have symlinks and just canonicalize to a Windows absolute
- path.  GetFullPath converts ../ and ./ in relative paths to
- absolute paths, filling in current drive if one is not given
- or using the current directory of a specified drive (eg, "E:foo").
- It also converts all forward slashes to back slashes.  */
+  /* The MS Windows method */
 #if defined (_WIN32)
   {
-char buf[MAX_PATH];
-char* basename;
-DWORD len = GetFullPathName (filename, MAX_PATH, buf, &basename);
-if (len == 0 || len > MAX_PATH - 1)
-  return strdup (filename);
-else
-  {
-	/* The file system is case-preserving but case-insensitive,
-	   Canonicalize to lowercase, using the codepage associated
-	   with the process locale.  */
-CharLowerBuff (buf, len);
-return strdup (buf);
-  }
-  }
-#endif
+char

Re: realpath() patch to fix symlinks resolution for win32

[i.nixman--- via Gcc-patches]

updated patch in attachments.
diff --git a/libiberty/lrealpath.c b/libiberty/lrealpath.c
index 3c7053b0b70..8858619e854 100644
--- a/libiberty/lrealpath.c
+++ b/libiberty/lrealpath.c
@@ -68,8 +68,138 @@ extern char *canonicalize_file_name (const char *);
   /* cygwin has realpath, so it won't get here.  */ 
 # if defined (_WIN32)
 #  define WIN32_LEAN_AND_MEAN
-#  include  /* for GetFullPathName */
-# endif
+#  include  /* for GetFullPathName/GetFinalPathNameByHandle/
+  CreateFile/CloseHandle */
+#  define WIN32_REPLACE_SLASHES(_ptr, _len) \
+ for (unsigned i = 0; i != (_len); ++i) \
+   if ((_ptr)[i] == '\\') (_ptr)[i] = '/';
+
+#  define WIN32_UNC_PREFIX "//?/UNC/"
+#  define WIN32_UNC_PREFIX_LEN (sizeof(WIN32_UNC_PREFIX)-1)
+#  define WIN32_IS_UNC_PREFIX(ptr) \
+  (0 == memcmp(ptr, WIN32_UNC_PREFIX, WIN32_UNC_PREFIX_LEN))
+
+#  define WIN32_NON_UNC_PREFIX "//?/"
+#  define WIN32_NON_UNC_PREFIX_LEN (sizeof(WIN32_NON_UNC_PREFIX)-1)
+#  define WIN32_IS_NON_UNC_PREFIX(ptr) \
+  (0 == memcmp(ptr, WIN32_NON_UNC_PREFIX, WIN32_NON_UNC_PREFIX_LEN))
+
+# define WIN32_IS_SPECIAL_NUL_FILE(ptr) \
+  (0 == strcasecmp(ptr, "NUL"))
+
+/* Get full path name without symlinks resolution.
+   It also converts all forward slashes to back slashes.
+*/
+char* get_full_path_name(const char *filename) {
+  DWORD len;
+  char *buf, *ptr, *res;
+
+  /* determining the required buffer size.
+ from the man: `If the lpBuffer buffer is too small to contain
+ the path, the return value is the size, in TCHARs, of the buffer
+ that is required to hold the path _and_the_terminating_null_character_`
+  */
+  len = GetFullPathName(filename, 0, NULL, NULL);
+
+  if ( len == 0 )
+return strdup(filename);
+
+  buf = (char *)malloc(len);
+
+  /* no point to check the result again */
+  len = GetFullPathName(filename, len, buf, NULL);
+  buf[len] = 0;
+
+  /* replace slashes */
+  WIN32_REPLACE_SLASHES(buf, len);
+
+  /* calculate offset based on prefix type */
+  len = WIN32_IS_UNC_PREFIX(buf)
+? (WIN32_UNC_PREFIX_LEN - 2)
+: WIN32_IS_NON_UNC_PREFIX(buf)
+  ? WIN32_NON_UNC_PREFIX_LEN
+  : 0
+  ;
+
+  ptr = buf + len;
+  if ( WIN32_IS_UNC_PREFIX(buf) ) {
+ptr[0] = '/';
+ptr[1] = '/';
+  }
+
+  res = strdup(ptr);
+
+  free(buf);
+
+  return res;
+}
+
+# if _WIN32_WINNT >= 0x0600
+
+/* Get full path name WITH symlinks resolution.
+   It also converts all forward slashes to back slashes.
+*/
+char* get_final_path_name(HANDLE fh) {
+  DWORD len;
+  char *buf, *ptr, *res;
+
+  /* determining the required buffer size.
+ from the  man: `If the function fails because lpszFilePath is too
+ small to hold the string plus the terminating null character,
+ the return value is the required buffer size, in TCHARs. This
+ value _includes_the_size_of_the_terminating_null_character_`.
+ but in my testcase I have path with 26 chars, the function
+ returns 26 also, ie without the trailing zero-char...
+  */
+  len = GetFinalPathNameByHandle(
+ fh
+,NULL
+,0
+,FILE_NAME_NORMALIZED | VOLUME_NAME_DOS
+  );
+
+  if ( len == 0 )
+return NULL;
+
+  len += 1; /* for zero-char */
+  buf = (char *)malloc(len);
+
+  /* no point to check the result again */
+  len = GetFinalPathNameByHandle(
+ fh
+,buf
+,len
+,FILE_NAME_NORMALIZED | VOLUME_NAME_DOS
+  );
+  buf[len] = 0;
+
+  /* replace slashes */
+  WIN32_REPLACE_SLASHES(buf, len);
+
+  /* calculate offset based on prefix type */
+  len = WIN32_IS_UNC_PREFIX(buf)
+? (WIN32_UNC_PREFIX_LEN - 2)
+: WIN32_IS_NON_UNC_PREFIX(buf)
+  ? WIN32_NON_UNC_PREFIX_LEN
+  : 0
+  ;
+
+  ptr = buf + len;
+  if ( WIN32_IS_UNC_PREFIX(buf) ) {
+ptr[0] = '/';
+ptr[1] = '/';
+  }
+
+  res = strdup(ptr);
+
+  free(buf);
+
+  return res;
+}
+
+# endif // _WIN32_WINNT >= 0x0600
+
+# endif // _WIN32
 #endif
 
 char *
@@ -128,30 +258,51 @@ lrealpath (const char *filename)
   }
 #endif
 
-  /* The MS Windows method.  If we don't have realpath, we assume we
- don't have symlinks and just canonicalize to a Windows absolute
- path.  GetFullPath converts ../ and ./ in relative paths to
- absolute paths, filling in current drive if one is not given
- or using the current directory of a specified drive (eg, "E:foo").
- It also converts all forward slashes to back slashes.  */
+  /* The MS Windows method */
 #if defined (_WIN32)
   {
-char buf[MAX_PATH];
-char* basename;
-DWORD len = GetFullPathName (filename, MAX_PATH, buf, &basename);
-if (len == 0 || len > MAX_PATH - 1)
-  return strdup (filename);
-else
-  {
-	/* The file system is case-preserving but case-insensitive,
-	   Canonicalize to lowercase, using the codepage associated
-	   with the process locale.  */
-CharLowerBuff (buf, len);
-return strdup (buf);
+char *res;
+/* For Windows Vista and greater */
+#if _WIN32_WINNT >= 0x0600
+
+/* For some reason the function rec

lrealpath() patch to fix symlinks resolution for win32

[i.nixman--- via Gcc-patches]

hello,

I just finished with https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108350

could anyone review and apply the attached patch please?


GCC master was succesfully bootstrapped as x86_64-w64-mingw32.



best!
diff --git a/libiberty/lrealpath.c b/libiberty/lrealpath.c
index 3c7053b0b70..8858619e854 100644
--- a/libiberty/lrealpath.c
+++ b/libiberty/lrealpath.c
@@ -68,8 +68,138 @@ extern char *canonicalize_file_name (const char *);
   /* cygwin has realpath, so it won't get here.  */ 
 # if defined (_WIN32)
 #  define WIN32_LEAN_AND_MEAN
-#  include  /* for GetFullPathName */
-# endif
+#  include  /* for GetFullPathName/GetFinalPathNameByHandle/
+  CreateFile/CloseHandle */
+#  define WIN32_REPLACE_SLASHES(_ptr, _len) \
+ for (unsigned i = 0; i != (_len); ++i) \
+   if ((_ptr)[i] == '\\') (_ptr)[i] = '/';
+
+#  define WIN32_UNC_PREFIX "//?/UNC/"
+#  define WIN32_UNC_PREFIX_LEN (sizeof(WIN32_UNC_PREFIX)-1)
+#  define WIN32_IS_UNC_PREFIX(ptr) \
+  (0 == memcmp(ptr, WIN32_UNC_PREFIX, WIN32_UNC_PREFIX_LEN))
+
+#  define WIN32_NON_UNC_PREFIX "//?/"
+#  define WIN32_NON_UNC_PREFIX_LEN (sizeof(WIN32_NON_UNC_PREFIX)-1)
+#  define WIN32_IS_NON_UNC_PREFIX(ptr) \
+  (0 == memcmp(ptr, WIN32_NON_UNC_PREFIX, WIN32_NON_UNC_PREFIX_LEN))
+
+# define WIN32_IS_SPECIAL_NUL_FILE(ptr) \
+  (0 == memcmp(ptr, "NUL", 3) || 0 == memcmp(ptr, "nul", 3))
+
+/* Get full path name without symlinks resolution.
+   It also converts all forward slashes to back slashes.
+*/
+char* get_full_path_name(const char *filename) {
+  DWORD len;
+  char *buf, *ptr, *res;
+
+  /* determining the required buffer size.
+ from the man: `If the lpBuffer buffer is too small to contain
+ the path, the return value is the size, in TCHARs, of the buffer
+ that is required to hold the path _and_the_terminating_null_character_`
+  */
+  len = GetFullPathName(filename, 0, NULL, NULL);
+
+  if ( len == 0 )
+return strdup(filename);
+
+  buf = (char *)malloc(len);
+
+  /* no point to check the result again */
+  len = GetFullPathName(filename, len, buf, NULL);
+  buf[len] = 0;
+
+  /* replace slashes */
+  WIN32_REPLACE_SLASHES(buf, len);
+
+  /* calculate offset based on prefix type */
+  len = WIN32_IS_UNC_PREFIX(buf)
+? (WIN32_UNC_PREFIX_LEN - 2)
+: WIN32_IS_NON_UNC_PREFIX(buf)
+  ? WIN32_NON_UNC_PREFIX_LEN
+  : 0
+  ;
+
+  ptr = buf + len;
+  if ( WIN32_IS_UNC_PREFIX(buf) ) {
+ptr[0] = '/';
+ptr[1] = '/';
+  }
+
+  res = strdup(ptr);
+
+  free(buf);
+
+  return res;
+}
+
+# if _WIN32_WINNT >= 0x0600
+
+/* Get full path name WITH symlinks resolution.
+   It also converts all forward slashes to back slashes.
+*/
+char* get_final_path_name(HANDLE fh) {
+  DWORD len;
+  char *buf, *ptr, *res;
+
+  /* determining the required buffer size.
+ from the  man: `If the function fails because lpszFilePath is too
+ small to hold the string plus the terminating null character,
+ the return value is the required buffer size, in TCHARs. This
+ value _includes_the_size_of_the_terminating_null_character_`.
+ but in my testcase I have path with 26 chars, the function
+ returns 26 also, ie without the trailing zero-char...
+  */
+  len = GetFinalPathNameByHandle(
+ fh
+,NULL
+,0
+,FILE_NAME_NORMALIZED | VOLUME_NAME_DOS
+  );
+
+  if ( len == 0 )
+return NULL;
+
+  len += 1; /* for zero-char */
+  buf = (char *)malloc(len);
+
+  /* no point to check the result again */
+  len = GetFinalPathNameByHandle(
+ fh
+,buf
+,len
+,FILE_NAME_NORMALIZED | VOLUME_NAME_DOS
+  );
+  buf[len] = 0;
+
+  /* replace slashes */
+  WIN32_REPLACE_SLASHES(buf, len);
+
+  /* calculate offset based on prefix type */
+  len = WIN32_IS_UNC_PREFIX(buf)
+? (WIN32_UNC_PREFIX_LEN - 2)
+: WIN32_IS_NON_UNC_PREFIX(buf)
+  ? WIN32_NON_UNC_PREFIX_LEN
+  : 0
+  ;
+
+  ptr = buf + len;
+  if ( WIN32_IS_UNC_PREFIX(buf) ) {
+ptr[0] = '/';
+ptr[1] = '/';
+  }
+
+  res = strdup(ptr);
+
+  free(buf);
+
+  return res;
+}
+
+# endif // _WIN32_WINNT >= 0x0600
+
+# endif // _WIN32
 #endif
 
 char *
@@ -128,30 +258,51 @@ lrealpath (const char *filename)
   }
 #endif
 
-  /* The MS Windows method.  If we don't have realpath, we assume we
- don't have symlinks and just canonicalize to a Windows absolute
- path.  GetFullPath converts ../ and ./ in relative paths to
- absolute paths, filling in current drive if one is not given
- or using the current directory of a specified drive (eg, "E:foo").
- It also converts all forward slashes to back slashes.  */
+  /* The MS Windows method */
 #if defined (_WIN32)
   {
-char buf[MAX_PATH];
-char* basename;
-DWORD len = GetFullPathName (filename, MAX_PATH, buf, &basename);
-if (len == 0 || len > MAX_PATH - 1)
-  return strdup (filename);
-else
-  {
-	/* The file system is case-preserving but case-insensitive,
-	   Canonicalize to lowercase, using the codepage associated
-	   with the 

[committed] IRA: patch to fix PR97847

[Vladimir Makarov via Gcc-patches]

The following patch fixes

  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97847

The patch was successfully bootstrapped and tested on x86-64 and ppc64.


[PR97847] IRA: Skip abnormal critical edge splitting

PPC64 can generate jumps with clobbered pseudo-regs and a BB with
such jump can have abnormal output edges.  IRA hits an assert when trying
to split abnormal critical edge to deal with asm goto output reloads
later.  The patch just skips splitting abnormal edges.  It is assumed
that asm-goto with output reloads can not be in BB with output abnormal edges.

gcc/ChangeLog:

	PR target/97847
	* ira.c (ira): Skip abnormal critical edge splitting.

diff --git a/gcc/ira.c b/gcc/ira.c
index 725b0ff0276..f0bdbc8cf56 100644
--- a/gcc/ira.c
+++ b/gcc/ira.c
@@ -5433,12 +5433,22 @@ ira (FILE *f)
 	  for (int i = 0; i < recog_data.n_operands; i++)
 	if (recog_data.operand_type[i] != OP_IN)
 	  {
+		bool skip_p = false;
+		FOR_EACH_EDGE (e, ei, bb->succs)
+		  if (EDGE_CRITICAL_P (e)
+		  && e->dest != EXIT_BLOCK_PTR_FOR_FN (cfun)
+		  && (e->flags & EDGE_ABNORMAL))
+		{
+		  skip_p = true;
+		  break;
+		}
+		if (skip_p)
+		  break;
 		output_jump_reload_p = true;
 		FOR_EACH_EDGE (e, ei, bb->succs)
 		  if (EDGE_CRITICAL_P (e)
 		  && e->dest != EXIT_BLOCK_PTR_FOR_FN (cfun))
 		{
-		  ira_assert (!(e->flags & EDGE_ABNORMAL));
 		  start_sequence ();
 		  /* We need to put some no-op insn here.  We can
 			 not put a note as commit_edges insertion will

[committed] LRA: patch to fix PR97969

[Vladimir Makarov via Gcc-patches]

The following patch fixes

  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97969

The patch was successfully bootstrapped on x86-64.

[PR97969] LRA: Transform pattern `plus (plus (hard reg, const),	pseudo)` after elimination

LRA can loop infinitely on targets without `reg + imm` insns.  Register elimination
on such targets can increase register pressure resulting in permanent
stack size increase and changing elimination offset.  To avoid such situation, a simple
transformation can be done to avoid register pressure increase after
generating reload insns containing eliminated hard regs.

gcc/ChangeLog:

	PR target/97969
	* lra-eliminations.c (eliminate_regs_in_insn): Add transformation
	of pattern 'plus (plus (hard reg, const), pseudo)'.

gcc/testsuite/ChangeLog:

	PR target/97969
	* gcc.target/arm/pr97969.c: New.

diff --git a/gcc/lra-eliminations.c b/gcc/lra-eliminations.c
index b28f3c410d3..ebcadd1006c 100644
--- a/gcc/lra-eliminations.c
+++ b/gcc/lra-eliminations.c
@@ -885,7 +885,7 @@ eliminate_regs_in_insn (rtx_insn *insn, bool replace_p, bool first_p,
 			poly_int64 update_sp_offset)
 {
   int icode = recog_memoized (insn);
-  rtx old_set = single_set (insn);
+  rtx set, old_set = single_set (insn);
   bool validate_p;
   int i;
   rtx substed_operand[MAX_RECOG_OPERANDS];
@@ -1038,6 +1038,32 @@ eliminate_regs_in_insn (rtx_insn *insn, bool replace_p, bool first_p,
   for (i = 0; i < static_id->n_dups; i++)
 *id->dup_loc[i] = substed_operand[(int) static_id->dup_num[i]];
 
+  /* Transform plus (plus (hard reg, const), pseudo) to plus (plus (pseudo,
+ const), hard reg) in order to keep insn containing eliminated register
+ after all reloads calculating its offset.  This permits to keep register
+ pressure under control and helps to avoid LRA cycling in patalogical
+ cases.  */
+  if (! replace_p && (set = single_set (insn)) != NULL
+  && GET_CODE (SET_SRC (set)) == PLUS
+  && GET_CODE (XEXP (SET_SRC (set), 0)) == PLUS)
+{
+  rtx reg1, reg2, op1, op2;
+  
+  reg1 = op1 = XEXP (XEXP (SET_SRC (set), 0), 0);
+  reg2 = op2 = XEXP (SET_SRC (set), 1);
+  if (GET_CODE (reg1) == SUBREG)
+	reg1 = SUBREG_REG (reg1);
+  if (GET_CODE (reg2) == SUBREG)
+	reg2 = SUBREG_REG (reg2);
+  if (REG_P (reg1) && REG_P (reg2)
+	  && REGNO (reg1) < FIRST_PSEUDO_REGISTER
+	  && REGNO (reg2) >= FIRST_PSEUDO_REGISTER)
+	{
+	  XEXP (XEXP (SET_SRC (set), 0), 0) = op2;
+	  XEXP (SET_SRC (set), 1) = op1;
+	}
+}
+
   /* If we had a move insn but now we don't, re-recognize it.
  This will cause spurious re-recognition if the old move had a
  PARALLEL since the new one still will, but we can't call
diff --git a/gcc/testsuite/gcc.target/arm/pr97969.c b/gcc/testsuite/gcc.target/arm/pr97969.c
new file mode 100644
index 000..714a1d18870
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr97969.c
@@ -0,0 +1,54 @@
+/* { dg-do compile } */
+/* { dg-options "-std=c99 -fno-omit-frame-pointer -mthumb -w -Os" } */
+
+typedef a[23];
+enum { b };
+typedef struct {
+  int c;
+  char *e;
+  char f
+} d;
+typedef enum { g = 1 } h;
+typedef struct {
+  h i;
+  int j
+} k;
+typedef struct {
+  a l;
+  int a;
+  int m;
+  int n;
+  int o;
+  short p;
+  int q;
+  k r;
+  char e;
+  char *s;
+  d t;
+  d *u;
+  short v;
+  int w
+} aa;
+c(char x, int y, char z, int ab) {
+  aa ac;
+  ac.r.i = 0;
+  d ad;
+  ac.t = ad;
+  ac.u = 0;
+  ae(&ac.v, 0, 0);
+  ac.w = 0;
+  af(&ac, x + y, z, z + ab);
+  if (ag(0))
+return 0;
+  if (x)
+ac.s = z + ab;
+  else
+ac.s = x + y;
+  ac.o |= g;
+  if (!setjmp()) {
+ah(ac);
+ai(b);
+ac.e = z + ab;
+aj(ac);
+  }
+}

Re: [committed] patch to fix PR97978

[Vladimir Makarov via Gcc-patches]

On 2021-01-07 6:01 a.m., Richard Sandiford wrote:

Vladimir Makarov via Gcc-patches  writes:

The following fixes

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97978

The patch was successfully bootstrapped on x86-64.

Can you explain this a bit more?  The assert fires if the register
allocation is inconsistent with the conflict information.  What causes
the inconsistency in this case, and why is it OK for the inconsistency
to persist until the next lra_assign pass?  Does something fix up the
inconsistency later, or is the inconsistent information never used?


Sorry, Richard.  I should have described it in more details in the bugzilla.

Hard register splitting was added to remove reload failures to find a 
hard register after the first scheduling.  Although unfortunately it 
does not remove all the 1st insn scheduling problems.


A pseudo (p1) lives through insn and one insn alternative requires a 
specific hard reg (dx in this case) although there is no explicit hard 
register in the insn (simply another pseudo p2 in the insn requires a 
class containing only dx reg).  So the pseudo p1 does not have conflict 
with dx for now and p1 was assigned to dx.


Now constraint sub-pass choose the insn alternative and the next assign 
sub-pass creates a reload pseudo rp2 for p2 and tries to assign dx to 
rp2.  It fails and we made hard reg splitting to assign dx to a reload 
pseudo of rp2.


p3<-dx

insn (rp2)

dx<-p3

After this transformation p1 now got dx as a conflicting reg and 
allocation is incorrect at this stage as p1 assigned to dx living 
through the insns conflicts explicitly with dx.


Next assign subpass is trying to assign dx to rp2 by spilling p1 (and 
assigning another hard reg to p1).


At the end p2,rp2,p3 get dx and all trivial moves (dx <- dx) are removed.

That is explanation when we have an incorrect allocation.

I also should say that LRA has a final register allocation check.  The 
check in assign sub-pass is for earlier bug recognition during initial 
LRA development and probably is not necessary anymore.  So another 
solution would be remove the check in assign sub-pass at all.  But I 
decided not to do this as it still permits to find some bugs earlier.



Is there no chance of lra_split_hard_reg_for updating the information
itself, to keep everything self-consistent?
I think it would be the old reload approach (ignoring sub-pass 
separation and making all in on place).  To correct allocations right in 
lra_split_hard_reg_for we would need to check all pseudos living through 
the new insns (and for this we need correct live info) and spill pseudos 
conflicting with split hard reg.  But we don't need to do this as right 
after splitting sub-pass we have live info sub-pass and assign sub-pass 
which make all of this.

   Bypassing the check for
every pseudo register seems like quite a big hammer.

I'm not saying this is the wrong fix.  I just think it would help
to have more commentary explaining the situation.

Re: [committed] patch to fix PR97978

[Richard Sandiford via Gcc-patches]

Vladimir Makarov via Gcc-patches  writes:
> The following fixes
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97978
>
> The patch was successfully bootstrapped on x86-64.

Can you explain this a bit more?  The assert fires if the register
allocation is inconsistent with the conflict information.  What causes
the inconsistency in this case, and why is it OK for the inconsistency
to persist until the next lra_assign pass?  Does something fix up the
inconsistency later, or is the inconsistent information never used?

Is there no chance of lra_split_hard_reg_for updating the information
itself, to keep everything self-consistent?  Bypassing the check for
every pseudo register seems like quite a big hammer.

I'm not saying this is the wrong fix.  I just think it would help
to have more commentary explaining the situation.

Thanks,
Richard
>
>
> commit fbf9b2b634e376516cd21d7aa92ef3fd5778aa10 (HEAD -> master)
> Author: Vladimir N. Makarov 
> Date:   Wed Jan 6 14:48:53 2021 -0500
>
> [PR97978] LRA: Permit temporary allocation incorrectness after hard reg 
> split.
>
> LRA can crash when a hard register was split and the same hard register
> was assigned on the previous assignment sub-pass.  The following
> patch fixes this problem.
> 
> gcc/ChangeLog:
> 
> PR rtl-optimization/97978
> * lra-int.h (lra_hard_reg_split_p): New external.
> * lra.c (lra_hard_reg_split_p): New global.
> (lra): Set up lra_hard_reg_split_p after splitting a hard reg.
> * lra-assigns.c (lra_assign): Don't check allocation correctness
> after hard reg splitting.
> 
> gcc/testsuite/ChangeLog:
> 
> PR rtl-optimization/97978
> * gcc.target/i386/pr97978.c: New.
>
> diff --git a/gcc/lra-assigns.c b/gcc/lra-assigns.c
> index 9335e4c876e..c6a941fe663 100644
> --- a/gcc/lra-assigns.c
> +++ b/gcc/lra-assigns.c
> @@ -1636,10 +1636,11 @@ lra_assign (bool &fails_p)
>bitmap_initialize (&all_spilled_pseudos, ®_obstack);
>create_live_range_start_chains ();
>setup_live_pseudos_and_spill_after_risky_transforms (&all_spilled_pseudos);
> -  if (! lra_asm_error_p && flag_checking)
> -/* Check correctness of allocation for call-crossed pseudos but
> -   only when there are no asm errors as in the case of errors the
> -   asm is removed and it can result in incorrect allocation.  */
> +  if (! lra_hard_reg_split_p && ! lra_asm_error_p && flag_checking)
> +/* Check correctness of allocation but only when there are no hard reg
> +   splits and asm errors as in the case of errors explicit insns 
> involving
> +   hard regs are added or the asm is removed and this can result in
> +   incorrect allocation.  */
>  for (i = FIRST_PSEUDO_REGISTER; i < max_regno; i++)
>if (lra_reg_info[i].nrefs != 0
> && reg_renumber[i] >= 0
> diff --git a/gcc/lra-int.h b/gcc/lra-int.h
> index 75ba6560bcc..1b8f7b6ae61 100644
> --- a/gcc/lra-int.h
> +++ b/gcc/lra-int.h
> @@ -273,6 +273,7 @@ typedef class lra_insn_recog_data *lra_insn_recog_data_t;
>  
>  extern FILE *lra_dump_file;
>  
> +extern bool lra_hard_reg_split_p;
>  extern bool lra_asm_error_p;
>  extern bool lra_reg_spill_p;
>  
> diff --git a/gcc/lra.c b/gcc/lra.c
> index 380a21ac2ac..aa49de6f154 100644
> --- a/gcc/lra.c
> +++ b/gcc/lra.c
> @@ -2211,6 +2211,9 @@ bitmap_head lra_subreg_reload_pseudos;
>  /* File used for output of LRA debug information.  */
>  FILE *lra_dump_file;
>  
> +/* True if we split hard reg after the last constraint sub-pass.  */
> +bool lra_hard_reg_split_p;
> +
>  /* True if we found an asm error.  */
>  bool lra_asm_error_p;
>  
> @@ -2359,6 +2362,7 @@ lra (FILE *f)
> if (live_p)
>   lra_clear_live_ranges ();
> bool fails_p;
> +   lra_hard_reg_split_p = false;
> do
>   {
> /* We need live ranges for lra_assign -- so build them.
> @@ -2403,6 +2407,7 @@ lra (FILE *f)
> live_p = false;
> if (! lra_split_hard_reg_for ())
>   break;
> +   lra_hard_reg_split_p = true;
>   }
>   }
> while (fails_p);
> diff --git a/gcc/testsuite/gcc.target/i386/pr97978.c 
> b/gcc/testsuite/gcc.target/i386/pr97978.c
> new file mode 100644
> index 000..263bca8708d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr97978.c
> @@ -0,0 +1,22 @@
> +/* { dg-do compile } */
> +/* { dg-options "-Os -fno-PIC" } */
> +int sg;
> +long int kk;
> +
> +void
> +bp (int jz, int tj, long int li)
> +{
> +  if (jz == 0 || tj == 0)
> +__builtin_unreachable ();
> +
> +  kk = li;
> +}
> +
> +void
> +qp (void)
> +{
> +  ++kk;
> +
> +  for (;;)
> +bp (1l / sg, 0, ~0u);
> +}

[committed] patch to fix PR97978

[Vladimir Makarov via Gcc-patches]

The following fixes

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97978

The patch was successfully bootstrapped on x86-64.


commit fbf9b2b634e376516cd21d7aa92ef3fd5778aa10 (HEAD -> master)
Author: Vladimir N. Makarov 
Date:   Wed Jan 6 14:48:53 2021 -0500

[PR97978] LRA: Permit temporary allocation incorrectness after hard reg split.

LRA can crash when a hard register was split and the same hard register
was assigned on the previous assignment sub-pass.  The following
patch fixes this problem.

gcc/ChangeLog:

PR rtl-optimization/97978
* lra-int.h (lra_hard_reg_split_p): New external.
* lra.c (lra_hard_reg_split_p): New global.
(lra): Set up lra_hard_reg_split_p after splitting a hard reg.
* lra-assigns.c (lra_assign): Don't check allocation correctness
after hard reg splitting.

gcc/testsuite/ChangeLog:

PR rtl-optimization/97978
* gcc.target/i386/pr97978.c: New.

diff --git a/gcc/lra-assigns.c b/gcc/lra-assigns.c
index 9335e4c876e..c6a941fe663 100644
--- a/gcc/lra-assigns.c
+++ b/gcc/lra-assigns.c
@@ -1636,10 +1636,11 @@ lra_assign (bool &fails_p)
   bitmap_initialize (&all_spilled_pseudos, ®_obstack);
   create_live_range_start_chains ();
   setup_live_pseudos_and_spill_after_risky_transforms (&all_spilled_pseudos);
-  if (! lra_asm_error_p && flag_checking)
-/* Check correctness of allocation for call-crossed pseudos but
-   only when there are no asm errors as in the case of errors the
-   asm is removed and it can result in incorrect allocation.  */
+  if (! lra_hard_reg_split_p && ! lra_asm_error_p && flag_checking)
+/* Check correctness of allocation but only when there are no hard reg
+   splits and asm errors as in the case of errors explicit insns involving
+   hard regs are added or the asm is removed and this can result in
+   incorrect allocation.  */
 for (i = FIRST_PSEUDO_REGISTER; i < max_regno; i++)
   if (lra_reg_info[i].nrefs != 0
 	  && reg_renumber[i] >= 0
diff --git a/gcc/lra-int.h b/gcc/lra-int.h
index 75ba6560bcc..1b8f7b6ae61 100644
--- a/gcc/lra-int.h
+++ b/gcc/lra-int.h
@@ -273,6 +273,7 @@ typedef class lra_insn_recog_data *lra_insn_recog_data_t;
 
 extern FILE *lra_dump_file;
 
+extern bool lra_hard_reg_split_p;
 extern bool lra_asm_error_p;
 extern bool lra_reg_spill_p;
 
diff --git a/gcc/lra.c b/gcc/lra.c
index 380a21ac2ac..aa49de6f154 100644
--- a/gcc/lra.c
+++ b/gcc/lra.c
@@ -2211,6 +2211,9 @@ bitmap_head lra_subreg_reload_pseudos;
 /* File used for output of LRA debug information.  */
 FILE *lra_dump_file;
 
+/* True if we split hard reg after the last constraint sub-pass.  */
+bool lra_hard_reg_split_p;
+
 /* True if we found an asm error.  */
 bool lra_asm_error_p;
 
@@ -2359,6 +2362,7 @@ lra (FILE *f)
 	  if (live_p)
 	lra_clear_live_ranges ();
 	  bool fails_p;
+	  lra_hard_reg_split_p = false;
 	  do
 	{
 	  /* We need live ranges for lra_assign -- so build them.
@@ -2403,6 +2407,7 @@ lra (FILE *f)
 		  live_p = false;
 		  if (! lra_split_hard_reg_for ())
 		break;
+		  lra_hard_reg_split_p = true;
 		}
 	}
 	  while (fails_p);
diff --git a/gcc/testsuite/gcc.target/i386/pr97978.c b/gcc/testsuite/gcc.target/i386/pr97978.c
new file mode 100644
index 000..263bca8708d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr97978.c
@@ -0,0 +1,22 @@
+/* { dg-do compile } */
+/* { dg-options "-Os -fno-PIC" } */
+int sg;
+long int kk;
+
+void
+bp (int jz, int tj, long int li)
+{
+  if (jz == 0 || tj == 0)
+__builtin_unreachable ();
+
+  kk = li;
+}
+
+void
+qp (void)
+{
+  ++kk;
+
+  for (;;)
+bp (1l / sg, 0, ~0u);
+}

[COMMITTED] patch to fix PR97933

[Vladimir Makarov via Gcc-patches]

The following patch fixes

  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97933

The patch was successfully bootstrapped on x86-64 and s390x.


commit fb352136db34a7adbc7be6a1e4e90b56bc632ebd (HEAD -> master)
Author: Vladimir N. Makarov 
Date:   Tue Nov 24 11:25:16 2020 -0500

[PR97933] LRA: find correctly last empty dest block.

gcc/

2020-11-24  Vladimir Makarov  

PR bootstrap/97933
* lra.c (lra_process_new_insns): Stop on the first real insn after
head of e->dest.

diff --git a/gcc/lra.c b/gcc/lra.c
index b318cfd7456..4ec0f466376 100644
--- a/gcc/lra.c
+++ b/gcc/lra.c
@@ -1908,11 +1908,9 @@ lra_process_new_insns (rtx_insn *insn, rtx_insn *before, rtx_insn *after,
 		  tmp = NEXT_INSN (tmp);
 		if (NOTE_INSN_BASIC_BLOCK_P (tmp))
 		  tmp = NEXT_INSN (tmp);
-		for (curr = tmp;
-		 curr != NULL
-		   && (!INSN_P (curr) || BLOCK_FOR_INSN (curr) == e->dest);
-		 curr = NEXT_INSN (curr))
-		  ;
+		for (curr = tmp; curr != NULL; curr = NEXT_INSN (curr))
+		  if (INSN_P (curr))
+		break;
 		/* Do not put reload insns if it is the last BB
 		   without actual insns.  In this case the reload insns
 		   can get null BB after emitting.  */

[committed] patch to fix arm sync-3.c failure after submitting patch to deal with scratches in IRA

[Vladimir Makarov via Gcc-patches]

After submitting the patch dealing with insn scratches in IRA, a report 
came that sync-3.c started to fail with x86_64-aarch64 cross-compiler.  
The following patch fixes this problem.  The patch was successfully 
bootstrapped on x86-64.


commit 885cbb4a0a677299de34d9e413818df5bb8272b1
Author: Vladimir N. Makarov 
Date:   Mon Nov 2 16:52:17 2020 -0500

Expand reg_equiv when scratches are removed.

gcc/ChangeLog:

* ira.c (ira_remove_scratches): Rename to remove_scratches.  Make
it static and returning flag of any change.
(ira.c): Call ira_expand_reg_equiv in case of removing scratches.

diff --git a/gcc/ira.c b/gcc/ira.c
index 682d092c2f5..bc94e15a50e 100644
--- a/gcc/ira.c
+++ b/gcc/ira.c
@@ -5214,7 +5214,8 @@ contains_X_constraint_p (const char *str)
   return false;
 }
   
-/* Change INSN's scratches into pseudos and save their location.  */
+/* Change INSN's scratches into pseudos and save their location.
+   Return true if we changed any scratch.  */
 bool
 ira_remove_insn_scratches (rtx_insn *insn, bool all_p, FILE *dump_file,
 			   rtx (*get_reg) (rtx original))
@@ -5245,17 +5246,19 @@ ira_remove_insn_scratches (rtx_insn *insn, bool all_p, FILE *dump_file,
 }
 
 /* Return new register of the same mode as ORIGINAL.  Used in
-   ira_remove_scratches.  */
+   remove_scratches.  */
 static rtx
 get_scratch_reg (rtx original)
 {
   return gen_reg_rtx (GET_MODE (original));
 }
 
-/* Change scratches into pseudos and save their location.  */
-void
-ira_remove_scratches (void)
+/* Change scratches into pseudos and save their location.  Return true
+   if we changed any scratch.  */
+static bool
+remove_scratches (void)
 {
+  bool change_p = false;
   basic_block bb;
   rtx_insn *insn;
 
@@ -5266,8 +5269,12 @@ ira_remove_scratches (void)
 FOR_BB_INSNS (bb, insn)
 if (INSN_P (insn)
 	&& ira_remove_insn_scratches (insn, false, ira_dump_file, get_scratch_reg))
-  /* Because we might use DF, we need to keep DF info up to date.  */
-  df_insn_rescan (insn);
+  {
+	/* Because we might use DF, we need to keep DF info up to date.  */
+	df_insn_rescan (insn);
+	change_p = true;
+  }
+  return change_p;
 }
 
 /* Changes pseudos created by function remove_scratches onto scratches.	 */
@@ -5514,8 +5521,8 @@ ira (FILE *f)
   end_alias_analysis ();
   free (reg_equiv);
 
-  if (ira_use_lra_p)
-ira_remove_scratches ();
+  if (ira_use_lra_p && remove_scratches ())
+ira_expand_reg_equiv ();
 
   if (resize_reg_info () && flag_ira_loop_pressure)
 ira_set_pseudo_classes (true, ira_dump_file);

[PUSHED] Patch to fix a LRA ICE [PR 97313]

[Vladimir Makarov via Gcc-patches]

The following patch has been committed into the main line.  The patch fixes

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97313

The patch was successfully bootstrapped and tested on x86-64.


gcc/ChangeLog:

2020-10-09  Vladimir Makarov  

	PR rtl-optimization/97313
	* lra-constraints.c (match_reload): Don't keep strict_low_part in
	reloads for non-registers.

gcc/testsuite/ChangeLog:

2020-10-09  Vladimir Makarov  

	PR rtl-optimization/97313
	* gcc.target/i386/pr97313.c: New.

diff --git a/gcc/lra-constraints.c b/gcc/lra-constraints.c
index 301c912cb21..f761d7dfe3c 100644
--- a/gcc/lra-constraints.c
+++ b/gcc/lra-constraints.c
@@ -1132,8 +1132,13 @@ match_reload (signed char out, signed char *ins, signed char *outs,
   narrow_reload_pseudo_class (out_rtx, goal_class);
   if (find_reg_note (curr_insn, REG_UNUSED, out_rtx) == NULL_RTX)
 {
+  reg = SUBREG_P (out_rtx) ? SUBREG_REG (out_rtx) : out_rtx;
   start_sequence ();
-  if (out >= 0 && curr_static_id->operand[out].strict_low)
+  /* If we had strict_low_part, use it also in reload to keep other
+	 parts unchanged but do it only for regs as strict_low_part
+	 has no sense for memory and probably there is no insn pattern
+	 to match the reload insn in memory case.  */
+  if (out >= 0 && curr_static_id->operand[out].strict_low && REG_P (reg))
 	out_rtx = gen_rtx_STRICT_LOW_PART (VOIDmode, out_rtx);
   lra_emit_move (out_rtx, copy_rtx (new_out_reg));
   emit_insn (*after);
diff --git a/gcc/testsuite/gcc.target/i386/pr97313.c b/gcc/testsuite/gcc.target/i386/pr97313.c
new file mode 100644
index 000..ef93cf1cca8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr97313.c
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fPIE" } */
+
+typedef struct {
+  int unspecified : 1;
+  int secure : 1;
+} MemTxAttrs;
+
+enum { MSCAllowNonSecure } tz_msc_read_pdata;
+
+int tz_msc_read_s_0;
+int tz_msc_check();
+int address_space_ldl_le();
+
+void tz_msc_read(MemTxAttrs attrs) {
+  int as = tz_msc_read_s_0;
+  long long data;
+  switch (tz_msc_check()) {
+  case MSCAllowNonSecure:
+attrs.secure = attrs.unspecified = 0;
+data = address_space_ldl_le(as, attrs);
+  }
+  tz_msc_read_pdata = data;
+}

Re: patch to fix PR93564

[Vladimir Makarov]

On 2020-02-24 4:54 a.m., Christophe Lyon wrote:

Hi,

On Sun, 23 Feb 2020 at 22:26, Vladimir Makarov  wrote:

The following patch is for

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93564

The patch was successfully bootstrapped on x86-64 and benchmarked on
SPEC2000.



It seems this patch causes regression on some arm cores (seen on
cortex-a15 and cortex-57).
When configuring
--target arm-none-linux-gnueabihf
--with-cpu cortex-a57
--with-fpu crypto-neon-fp-armv8

gcc.target/arm/unaligned-memcpy-2.c: ldrd found 1 times
FAIL: gcc.target/arm/unaligned-memcpy-2.c scan-assembler-times ldrd 0
gcc.target/arm/unaligned-memcpy-2.c: strd found 2 times
FAIL: gcc.target/arm/unaligned-memcpy-2.c scan-assembler-times strd 1

This one regresses only on cortex-a15:
gcc.target/arm/unaligned-memcpy-3.c: ldrd found 2 times
FAIL: gcc.target/arm/unaligned-memcpy-3.c scan-assembler-times ldrd 1
gcc.target/arm/unaligned-memcpy-3.c: strd found 1 times
FAIL: gcc.target/arm/unaligned-memcpy-3.c scan-assembler-times strd 0


I've just submitted a patch which I hope will solve some of the failures.

Re: patch to fix PR93564

[Vladimir Makarov]

On 2020-02-27 7:33 a.m., Andrew Stubbs wrote:

On 26/02/2020 15:16, Andrew Stubbs wrote:
The problem appears to be that the high-part of a register pair is 
not marked as "ever live".  I'm trying to figure out whether this is 
some kind of target-specific issue that has merely been exposed, but 
it's difficult to see what's going on. I'm pretty sure I've never 
seen this one before.


I'm now pretty sure your patch didn't cause this issue so much as 
expose it.


Either way, it's fixed now.

Thank you for informing me about this.  Such heuristic changes should 
not affect generated code correctness unless they trigger a hidden bug 
in RA or machine-depended code used by RA.

Re: patch to fix PR93564

[Andrew Stubbs]

On 26/02/2020 15:16, Andrew Stubbs wrote:
The problem appears to be that the high-part of a register pair is not 
marked as "ever live".  I'm trying to figure out whether this is some 
kind of target-specific issue that has merely been exposed, but it's 
difficult to see what's going on. I'm pretty sure I've never seen this 
one before.


I'm now pretty sure your patch didn't cause this issue so much as expose it.

Either way, it's fixed now.

Andrew

Re: patch to fix PR93564

[Andrew Stubbs]

On 23/02/2020 21:25, Vladimir Makarov wrote:

The following patch is for

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93564

The patch was successfully bootstrapped on x86-64 and benchmarked on 
SPEC2000.


Since this patch I get an ICE with checking enabled, for amdgcn-amdhsa:

during RTL pass: reload
dump file: /scratch/astubbs/amd/upstreamB/tmp/target.283r.reload
../gcc.c-torture/execute/ieee/compare-fp-1.c: In function 'ieq':
../gcc.c-torture/execute/ieee/compare-fp-1.c:33:1: internal 
compiler error: in lra_constraints, at lra-constraints.c:5051

   33 | }
  | ^
0x103d7cf lra_constraints(bool)
/gcc/lra-constraints.c:5051
0x102541a lra(_IO_FILE*)
/gcc/lra.c:2437
0xfcc9c9 do_reload
/gcc/ira.c:5523
0xfcce50 execute
/gcc/ira.c:5709
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See  for instructions.

This happens at all optimization levels (but not -O0).

The problem appears to be that the high-part of a register pair is not 
marked as "ever live".  I'm trying to figure out whether this is some 
kind of target-specific issue that has merely been exposed, but it's 
difficult to see what's going on. I'm pretty sure I've never seen this 
one before.


Andrew

Re: Patch to fix PR93272

[Matthias Klose]

On 1/28/20 9:52 PM, Vladimir Makarov wrote:
> The following patch fixes
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93272
> 
> The patch was successfully tested and bootstrapped on x86_64.
> 
> Unfortunately it is hard to create a test case for the patch.  So there is no
> test for this PR.

the bug report asks for a backport to the gcc-8 and gcc-9 branches.

Re: patch to fix PR93564

[Christophe Lyon]

Hi,

On Sun, 23 Feb 2020 at 22:26, Vladimir Makarov  wrote:
>
> The following patch is for
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93564
>
> The patch was successfully bootstrapped on x86-64 and benchmarked on
> SPEC2000.
>


It seems this patch causes regression on some arm cores (seen on
cortex-a15 and cortex-57).
When configuring
--target arm-none-linux-gnueabihf
--with-cpu cortex-a57
--with-fpu crypto-neon-fp-armv8

gcc.target/arm/unaligned-memcpy-2.c: ldrd found 1 times
FAIL: gcc.target/arm/unaligned-memcpy-2.c scan-assembler-times ldrd 0
gcc.target/arm/unaligned-memcpy-2.c: strd found 2 times
FAIL: gcc.target/arm/unaligned-memcpy-2.c scan-assembler-times strd 1

This one regresses only on cortex-a15:
gcc.target/arm/unaligned-memcpy-3.c: ldrd found 2 times
FAIL: gcc.target/arm/unaligned-memcpy-3.c scan-assembler-times ldrd 1
gcc.target/arm/unaligned-memcpy-3.c: strd found 1 times
FAIL: gcc.target/arm/unaligned-memcpy-3.c scan-assembler-times strd 0

Christophe

patch to fix PR93564

[Vladimir Makarov]

The following patch is for

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93564

The patch was successfully bootstrapped on x86-64 and benchmarked on 
SPEC2000.



commit 3133bed5d0327e8a9cd0a601b7ecdb9de4fc825d
Author: Vladimir N. Makarov 
Date:   Sun Feb 23 16:20:05 2020 -0500

Changing cost propagation and ordering colorable bucket heuristics for PR93564.

2020-02-23  Vladimir Makarov  

PR rtl-optimization/93564
* ira-color.c (struct update_cost_queue_elem): New member start.
(queue_update_cost, get_next_update_cost): Add new arg start.
(allocnos_conflict_p): New function.
(update_costs_from_allocno): Add new arg conflict_cost_update_p.
Add checking conflicts with allocnos_conflict_p.
(update_costs_from_prefs, restore_costs_from_copies): Adjust
update_costs_from_allocno calls.
(update_conflict_hard_regno_costs): Add checking conflicts with
allocnos_conflict_p.  Adjust calls of queue_update_cost and
get_next_update_cost.
(assign_hard_reg): Adjust calls of queue_update_cost.  Add
debugging print.
(bucket_allocno_compare_func): Restore previous version.

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index cea52f00523..b3b9d92a1ec 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,20 @@
+2020-02-23  Vladimir Makarov  
+
+	PR rtl-optimization/93564
+	* ira-color.c (struct update_cost_queue_elem): New member start.
+	(queue_update_cost, get_next_update_cost): Add new arg start.
+	(allocnos_conflict_p): New function.
+	(update_costs_from_allocno): Add new arg conflict_cost_update_p.
+	Add checking conflicts with allocnos_conflict_p.
+	(update_costs_from_prefs, restore_costs_from_copies): Adjust
+	update_costs_from_allocno calls.
+	(update_conflict_hard_regno_costs): Add checking conflicts with
+	allocnos_conflict_p.  Adjust calls of queue_update_cost and
+	get_next_update_cost.
+	(assign_hard_reg): Adjust calls of queue_update_cost.  Add
+	debugging print.
+	(bucket_allocno_compare_func): Restore previous version.
+
 2020-02-21  John David Anglin  
 
 	* gcc/config/pa/pa.c (pa_function_value): Fix check for word and
diff --git a/gcc/ira-color.c b/gcc/ira-color.c
index 0bcc80463b0..0ffdd192020 100644
--- a/gcc/ira-color.c
+++ b/gcc/ira-color.c
@@ -1199,6 +1199,10 @@ struct update_cost_queue_elem
  connecting this allocno to the one being allocated.  */
   int divisor;
 
+  /* Allocno from which we started chaining costs of connected
+ allocnos. */
+  ira_allocno_t start;
+
   /* Allocno from which we are chaining costs of connected allocnos.
  It is used not go back in graph of allocnos connected by
  copies.  */
@@ -1258,10 +1262,11 @@ start_update_cost (void)
   update_cost_queue = NULL;
 }
 
-/* Add (ALLOCNO, FROM, DIVISOR) to the end of update_cost_queue, unless
+/* Add (ALLOCNO, START, FROM, DIVISOR) to the end of update_cost_queue, unless
ALLOCNO is already in the queue, or has NO_REGS class.  */
 static inline void
-queue_update_cost (ira_allocno_t allocno, ira_allocno_t from, int divisor)
+queue_update_cost (ira_allocno_t allocno, ira_allocno_t start,
+		   ira_allocno_t from, int divisor)
 {
   struct update_cost_queue_elem *elem;
 
@@ -1270,6 +1275,7 @@ queue_update_cost (ira_allocno_t allocno, ira_allocno_t from, int divisor)
   && ALLOCNO_CLASS (allocno) != NO_REGS)
 {
   elem->check = update_cost_check;
+  elem->start = start;
   elem->from = from;
   elem->divisor = divisor;
   elem->next = NULL;
@@ -1282,10 +1288,11 @@ queue_update_cost (ira_allocno_t allocno, ira_allocno_t from, int divisor)
 }
 
 /* Try to remove the first element from update_cost_queue.  Return
-   false if the queue was empty, otherwise make (*ALLOCNO, *FROM,
-   *DIVISOR) describe the removed element.  */
+   false if the queue was empty, otherwise make (*ALLOCNO, *START,
+   *FROM, *DIVISOR) describe the removed element.  */
 static inline bool
-get_next_update_cost (ira_allocno_t *allocno, ira_allocno_t *from, int *divisor)
+get_next_update_cost (ira_allocno_t *allocno, ira_allocno_t *start,
+		  ira_allocno_t *from, int *divisor)
 {
   struct update_cost_queue_elem *elem;
 
@@ -1294,6 +1301,7 @@ get_next_update_cost (ira_allocno_t *allocno, ira_allocno_t *from, int *divisor)
 
   *allocno = update_cost_queue;
   elem = &update_cost_queue_elems[ALLOCNO_NUM (*allocno)];
+  *start = elem->start;
   *from = elem->from;
   *divisor = elem->divisor;
   update_cost_queue = elem->next;
@@ -1325,18 +1333,41 @@ update_allocno_cost (ira_allocno_t allocno, int hard_regno,
   return true;
 }
 
+/* Return TRUE if allocnos A1 and A2 conflicts. Here we are
+   interesting only in conflicts of allocnos with intersected allocno
+   classes. */
+static bool
+allocnos_conflict_p (ira_allocno_t a1, ira_allocno_t a2)
+{
+  ira_object_t obj, conflict_obj;
+  ira_object_conflict_iterator oci;
+  int word, nwo

Re: Patch to fix PR93561

[Segher Boessenkool]

Hi!

On Thu, Feb 06, 2020 at 05:16:14PM -0500, Vladimir Makarov wrote:
> --- a/gcc/lra-assigns.c
> +++ b/gcc/lra-assigns.c
> @@ -964,6 +964,8 @@ spill_for (int regno, bitmap spilled_pseudo_bitmap, bool 
> first_p)
>bitmap_clear (&spill_pseudos_bitmap);
>for (j = hard_regno_nregs (hard_regno, mode) - 1; j >= 0; j--)
>   {
> +  if (hard_regno + j >= FIRST_PSEUDO_REGISTER)
> + break;

if (!HARD_REGISTER_NUM_P (hard_regno + j))
?


Segher

Patch to fix PR93561

[Vladimir Makarov]

The following patch fixes

  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93561

The patch was successfully bootstrapped on x86-64.

commit d26f37a16e3ed3d75a93ffb1da10c44c36a8a36d (HEAD -> master)
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 1754aa76399..aec58a06529 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,10 @@
+2020-02-06  
+  	Vladimir Makarov  
+
+	PR rtl-optimization/93561
+	* lra-assigns.c (spill_for): Check that tested hard regno is not out of
+	hard register range.
+
 2020-02-06  Richard Sandiford  
 
 	* config/aarch64/aarch64.md (aarch64_movk): Add a type
diff --git a/gcc/lra-assigns.c b/gcc/lra-assigns.c
index 031ce402c32..40e323c2a64 100644
--- a/gcc/lra-assigns.c
+++ b/gcc/lra-assigns.c
@@ -964,6 +964,8 @@ spill_for (int regno, bitmap spilled_pseudo_bitmap, bool first_p)
   bitmap_clear (&spill_pseudos_bitmap);
   for (j = hard_regno_nregs (hard_regno, mode) - 1; j >= 0; j--)
 	{
+  if (hard_regno + j >= FIRST_PSEUDO_REGISTER)
+	break;
 	  if (try_hard_reg_pseudos_check[hard_regno + j] != curr_pseudo_check)
 	continue;
 	  lra_assert (!bitmap_empty_p (&try_hard_reg_pseudos[hard_regno + j]));

patch to fix PR91333

[Vladimir Makarov]

The following patch fixes

  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91333

The patch was successfully bootstrapped and tested on x86-64.

The patch changes order of putting colorable allocnos on the stack and 
consequently changes order of assigning hard registers to the allocnos 
(they all will get a hard register).  This can change the assignment and 
eliminate some moves involving hard registers.


I tried many different heuristics of ordering colorable allocnos based 
on different allocno (and corresponding thread) features like living 
only in BB, live range, the allocno preferences etc. But surprisingly 
this simplest patch worked the best.   The patch results in the same 
SPEC2000 rates.



commit 9ba4fc60a8d4c06130dedb7a6df1c72d972c4eb6
Author: Vladimir N. Makarov 
Date:   Fri Jan 31 14:26:26 2020 -0500

Fix for PR 91333 - suboptimal register allocation for inline asm

2020-01-31  Vladimir Makarov  

PR rtl-optimization/91333
* ira-color.c (bucket_allocno_compare_func): Move conflict hard
reg preferences comparison up.

2020-01-31  Vladimir Makarov  

PR rtl-optimization/91333
* gcc.target/i386/pr91333.c: New.

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 022865d005d..f4141157e97 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2020-01-31  Vladimir Makarov  
+
+	PR rtl-optimization/91333
+	* ira-color.c (bucket_allocno_compare_func): Move conflict hard
+	reg preferences comparison up.
+
 2020-01-31  Andrew Stubbs  
 
 	* config/gcn/gcn-valu.md (addv64di3_exec): Allow one '0' in each
diff --git a/gcc/ira-color.c b/gcc/ira-color.c
index 4a726dc7c1b..51c4afd6391 100644
--- a/gcc/ira-color.c
+++ b/gcc/ira-color.c
@@ -2251,6 +2251,11 @@ bucket_allocno_compare_func (const void *v1p, const void *v2p)
   ira_allocno_t t2 = ALLOCNO_COLOR_DATA (a2)->first_thread_allocno;
   int cl1 = ALLOCNO_CLASS (a1), cl2 = ALLOCNO_CLASS (a2);
 
+  /* Push allocnos with minimal conflict_allocno_hard_prefs first.  */
+  pref1 = ALLOCNO_COLOR_DATA (a1)->conflict_allocno_hard_prefs;
+  pref2 = ALLOCNO_COLOR_DATA (a2)->conflict_allocno_hard_prefs;
+  if ((diff = pref1 - pref2) != 0)
+return diff;
   freq1 = ALLOCNO_COLOR_DATA (t1)->thread_freq;
   freq2 = ALLOCNO_COLOR_DATA (t2)->thread_freq;
   if ((diff = freq1 - freq2) != 0)
@@ -2276,11 +2281,6 @@ bucket_allocno_compare_func (const void *v1p, const void *v2p)
   a2_num = ALLOCNO_COLOR_DATA (a2)->available_regs_num;
   if ((diff = a2_num - a1_num) != 0)
 return diff;
-  /* Push allocnos with minimal conflict_allocno_hard_prefs first.  */
-  pref1 = ALLOCNO_COLOR_DATA (a1)->conflict_allocno_hard_prefs;
-  pref2 = ALLOCNO_COLOR_DATA (a2)->conflict_allocno_hard_prefs;
-  if ((diff = pref1 - pref2) != 0)
-return diff;
   return ALLOCNO_NUM (a2) - ALLOCNO_NUM (a1);
 }
 
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index f95d2d4e069..edc250d8b40 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2020-01-31  Vladimir Makarov  
+
+	PR rtl-optimization/91333
+	* gcc.target/i386/pr91333.c: New.
+
 2020-01-31  Tobias Burnus  
 
 	PR fortran/93462
diff --git a/gcc/testsuite/gcc.target/i386/pr91333.c b/gcc/testsuite/gcc.target/i386/pr91333.c
new file mode 100644
index 000..41fc328698e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr91333.c
@@ -0,0 +1,14 @@
+/* { dg-do compile { target x86_64-*-* } } */
+/* { dg-options "-O2 -mavx" } */
+/* { dg-final { scan-assembler-times "vmovapd" 2 } } */
+
+static inline double g (double x){
+  asm volatile ("" : "+x" (x));
+  return x;
+}
+static inline double f (double a, double b){
+  return g (g (a) + g (b));
+}
+double h (double a, double b){
+  return f (f (a, a), f (b, b));
+}

Patch to fix PR93272

[Vladimir Makarov]

The following patch fixes

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93272

The patch was successfully tested and bootstrapped on x86_64.

Unfortunately it is hard to create a test case for the patch.  So there 
is no test for this PR.



commit 5c8a1211b9873a1b69ef7b2fddae181535bc3b0a (HEAD -> master, origin/master, origin/HEAD)
Author: Vladimir N. Makarov 
Date:   Tue Jan 28 15:43:44 2020 -0500

Fix for PR93272 - LRA: EH reg allocated to hold local variable

2020-01-28  Vladimir Makarov  

PR rtl-optimization/93272
* ira-lives.c (process_out_of_region_eh_regs): New function.
(process_bb_node_lives): Call it.

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 326250b9b96..8d60dcf0864 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2020-01-28  Vladimir Makarov  
+
+	PR rtl-optimization/93272
+	* ira-lives.c (process_out_of_region_eh_regs): New function.
+	(process_bb_node_lives): Call it.
+
 2020-01-28  Jan Hubicka  
 
 	* coverage.c (read_counts_file): Make error message lowercase.
diff --git a/gcc/ira-lives.c b/gcc/ira-lives.c
index 155f825ea8d..f776fd2342f 100644
--- a/gcc/ira-lives.c
+++ b/gcc/ira-lives.c
@@ -1160,6 +1160,50 @@ non_conflicting_reg_copy_p (rtx_insn *insn)
   return SET_SRC (set);
 }
 
+#ifdef EH_RETURN_DATA_REGNO
+
+/* Add EH return hard registers as conflict hard registers to allocnos
+   living at end of BB.  For most allocnos it is already done in
+   process_bb_node_lives when we processing input edges but it does
+   not work when and EH edge is edge out of the current region.  This
+   function covers such out of region edges. */
+static void
+process_out_of_region_eh_regs (basic_block bb)
+{
+  edge e;
+  edge_iterator ei;
+  unsigned int i;
+  bitmap_iterator bi;
+  bool eh_p = false;
+
+  FOR_EACH_EDGE (e, ei, bb->succs)
+if ((e->flags & EDGE_EH)
+	&& IRA_BB_NODE (e->dest)->parent != IRA_BB_NODE (bb)->parent)
+  eh_p = true;
+
+  if (! eh_p)
+return;
+
+  EXECUTE_IF_SET_IN_BITMAP (df_get_live_out (bb), FIRST_PSEUDO_REGISTER, i, bi)
+{
+  ira_allocno_t a = ira_curr_regno_allocno_map[i];
+  for (int n = ALLOCNO_NUM_OBJECTS (a) - 1; n >= 0; n--)
+	{
+	  ira_object_t obj = ALLOCNO_OBJECT (a, n);
+	  for (int k = 0; ; k++)
+	{
+	  unsigned int regno = EH_RETURN_DATA_REGNO (k);
+	  if (regno == INVALID_REGNUM)
+		break;
+	  SET_HARD_REG_BIT (OBJECT_CONFLICT_HARD_REGS (obj), regno);
+	  SET_HARD_REG_BIT (OBJECT_TOTAL_CONFLICT_HARD_REGS (obj), regno);
+	}
+	}
+}
+}
+
+#endif
+
 /* Process insns of the basic block given by its LOOP_TREE_NODE to
update allocno live ranges, allocno hard register conflicts,
intersected calls, and register pressure info for allocnos for the
@@ -1213,6 +1257,10 @@ process_bb_node_lives (ira_loop_tree_node_t loop_tree_node)
   EXECUTE_IF_SET_IN_BITMAP (reg_live_out, FIRST_PSEUDO_REGISTER, j, bi)
 	mark_pseudo_regno_live (j);
 
+#ifdef EH_RETURN_DATA_REGNO
+  process_out_of_region_eh_regs (bb);
+#endif
+
   freq = REG_FREQ_FROM_BB (bb);
   if (freq == 0)
 	freq = 1;

patch to fix PR93207

[Vladimir Makarov]

The following patch fixes

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93027

The patch was successfully tested bootstrapped on x86-64.

Committed as r280133

Index: ChangeLog
===
--- ChangeLog	(revision 280132)
+++ ChangeLog	(working copy)
@@ -1,3 +1,10 @@
+2020-01-10  Vladimir Makarov  
+
+	PR inline-asm/93207
+	* lra-constraints.c (match_reload): Permit input operands have the
+	same mode as output while other input operands have a different
+	mode.
+
 2020-01-10  Wilco Dijkstra  
 
 	PR tree-optimization/90838
Index: lra-constraints.c
===
--- lra-constraints.c	(revision 280132)
+++ lra-constraints.c	(working copy)
@@ -1054,12 +1054,15 @@ match_reload (signed char out, signed ch
   curr_insn_input_reloads[curr_insn_input_reloads_num].match_p = true;
   curr_insn_input_reloads[curr_insn_input_reloads_num++].reg = new_in_reg;
   for (i = 0; (in = ins[i]) >= 0; i++)
-{
-  lra_assert
-	(GET_MODE (*curr_id->operand_loc[in]) == VOIDmode
-	 || GET_MODE (new_in_reg) == GET_MODE (*curr_id->operand_loc[in]));
+if (GET_MODE (*curr_id->operand_loc[in]) == VOIDmode
+	|| GET_MODE (new_in_reg) == GET_MODE (*curr_id->operand_loc[in]))
   *curr_id->operand_loc[in] = new_in_reg;
-}
+else
+  {
+	lra_assert
+	  (GET_MODE (new_out_reg) == GET_MODE (*curr_id->operand_loc[in]));
+	*curr_id->operand_loc[in] = new_out_reg;
+  }
   lra_update_dups (curr_id, ins);
   if (out < 0)
 return;
Index: testsuite/ChangeLog
===
--- testsuite/ChangeLog	(revision 280132)
+++ testsuite/ChangeLog	(working copy)
@@ -1,3 +1,8 @@
+2020-01-10  Vladimir Makarov  
+
+	PR inline-asm/93207
+	* gcc.target/i386/pr93207.c: New test.
+
 2020-01-10  Wilco Dijkstra  
 
 	* testsuite/gcc.target/aarch64/pr90838.c: New test.
Index: testsuite/gcc.target/i386/pr93207.c
===
--- testsuite/gcc.target/i386/pr93207.c	(nonexistent)
+++ testsuite/gcc.target/i386/pr93207.c	(working copy)
@@ -0,0 +1,14 @@
+/* PR inline-asm/93207 */
+/* { dg-do compile } */
+/* { dg-options "-O0" } */
+
+int main (void) {
+  int f = 0, w;
+
+  asm volatile(
+""
+: "+m&l"(f)
+: "0a"(&w)
+  );
+  return 0;
+}

patch to fix PR92796

[Vladimir Makarov]

The following patch fixes

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92796

The patch was successfully bootstrapped and tested on x86-64.

Committed as r279204


Index: ChangeLog
===
--- ChangeLog	(revision 279200)
+++ ChangeLog	(working copy)
@@ -1,3 +1,18 @@
+2019-12-10  Vladimir Makarov  
+
+	PR rtl-optimization/92796
+	* lra-int.h (lra_risky_transformations_p): Rename to
+	check_and_force_assignment_correctness_p.
+	* lra-assigns.c: Ditto.
+	(lra_assign): Reset check_and_force_assignment_correctness_p.
+	* lra-constraints.c (lra_risky_transformations_p): Rename to
+	check_and_force_assignment_correctness_p.
+	(lra_constraints): Set up check_and_force_assignment_correctness_p
+	only for the 1st sub-pass.
+	* lra-eliminations.c (process_insn_for_elimination): Set up
+	check_and_force_assignment_correctness_p if the insn chnaged its
+	code.
+
 2019-12-10  Jakub Jelinek  
 
 	PR rtl-optimization/92882
Index: lra-assigns.c
===
--- lra-assigns.c	(revision 278608)
+++ lra-assigns.c	(working copy)
@@ -1131,7 +1131,7 @@ static int *sorted_pseudos;
 /* The constraints pass is allowed to create equivalences between
pseudos that make the current allocation "incorrect" (in the sense
that pseudos are assigned to hard registers from their own conflict
-   sets).  The global variable lra_risky_transformations_p says
+   sets).  The global variable check_and_force_assignment_correctness_p says
whether this might have happened.
 
Process pseudos assigned to hard registers (less frequently used
@@ -1152,7 +1152,7 @@ setup_live_pseudos_and_spill_after_risky
   bitmap_iterator bi;
   int max_regno = max_reg_num ();
 
-  if (! lra_risky_transformations_p)
+  if (! check_and_force_assignment_correctness_p)
 {
   for (i = FIRST_PSEUDO_REGISTER; i < max_regno; i++)
 	if (reg_renumber[i] >= 0 && lra_reg_info[i].nrefs > 0)
@@ -1690,6 +1690,8 @@ lra_assign (bool &fails_p)
 internal_error
   ("maximum number of LRA assignment passes is achieved (%d)",
LRA_MAX_ASSIGNMENT_ITERATION_NUMBER);
+  /* Reset the assignment correctness flag: */
+  check_and_force_assignment_correctness_p = false;
   return no_spills_p;
 }
 
Index: lra-constraints.c
===
--- lra-constraints.c	(revision 278608)
+++ lra-constraints.c	(working copy)
@@ -4665,11 +4665,14 @@ loc_equivalence_callback (rtx loc, const
 /* The current iteration number of this LRA pass.  */
 int lra_constraint_iter;
 
-/* True if we substituted equiv which needs checking register
-   allocation correctness because the equivalent value contains
-   allocatable hard registers or when we restore multi-register
-   pseudo.  */
-bool lra_risky_transformations_p;
+/* True if we should during assignment sub-pass check assignment
+   correctness for all pseudos and spill some of them to correct
+   conflicts.  It can be necessary when we substitute equiv which
+   needs checking register allocation correctness because the
+   equivalent value contains allocatable hard registers, or when we
+   restore multi-register pseudo, or when we change the insn code and
+   its operand became INOUT operand when it was IN one before.  */
+bool check_and_force_assignment_correctness_p;
 
 /* Return true if REGNO is referenced in more than one block.  */
 static bool
@@ -4811,14 +4814,14 @@ lra_constraints (bool first_p)
   changed_p = false;
   if (pic_offset_table_rtx
   && REGNO (pic_offset_table_rtx) >= FIRST_PSEUDO_REGISTER)
-lra_risky_transformations_p = true;
-  else
+check_and_force_assignment_correctness_p = true;
+  else if (first_p)
 /* On the first iteration we should check IRA assignment
correctness.  In rare cases, the assignments can be wrong as
early clobbers operands are ignored in IRA or usages of
paradoxical sub-registers are not taken into account by
IRA.  */
-lra_risky_transformations_p = first_p;
+check_and_force_assignment_correctness_p = true;
   new_insn_uid_start = get_max_uid ();
   new_regno_start = first_p ? lra_constraint_new_regno_start : max_reg_num ();
   /* Mark used hard regs for target stack size calulations.  */
@@ -4994,7 +4997,7 @@ lra_constraints (bool first_p)
 		  dump_insn_slim (lra_dump_file, curr_insn);
 		}
 		  if (contains_reg_p (x, true, false))
-		lra_risky_transformations_p = true;
+		check_and_force_assignment_correctness_p = true;
 		  lra_set_insn_deleted (curr_insn);
 		  continue;
 		}
@@ -5507,7 +5510,7 @@ need_for_split_p (HARD_REG_SET potential
 	   /* Don't split call clobbered hard regs living through
 	  calls, otherwise we might have a check problem in the
 	  assign sub-pass as in the most cases (exception is a
-	  situation when lra_risky_transformations_p value is
+	  situation when check_and_force_assignment_correctness_p 

Patch to fix PR92176

[Vladimir Makarov]

The following patch fixes

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92176

The patch was tested on i686, x86-64, and ppc64.

Committed as r279061.


Index: ChangeLog
===
--- ChangeLog	(revision 279060)
+++ ChangeLog	(working copy)
@@ -1,3 +1,10 @@
+2019-12-06  Andreas Krebbel  
+	Vladimir Makarov  
+
+	PR rtl-optimization/92176
+	* lra.c (simplify_subreg_regno): Don't permit unconditional
+	changing mode for LRA too.
+
 2019-12-06  Richard Sandiford  
 
 	* target.h (TCTX_ALLOCATION, TCTX_DEALLOCATION, TCTX_EXCEPTIONS)
Index: rtlanal.c
===
--- rtlanal.c	(revision 279060)
+++ rtlanal.c	(working copy)
@@ -3951,9 +3951,7 @@ simplify_subreg_regno (unsigned int xreg
   /* Give the backend a chance to disallow the mode change.  */
   if (GET_MODE_CLASS (xmode) != MODE_COMPLEX_INT
   && GET_MODE_CLASS (xmode) != MODE_COMPLEX_FLOAT
-  && !REG_CAN_CHANGE_MODE_P (xregno, xmode, ymode)
-  /* We can use mode change in LRA for some transformations.  */
-  && ! lra_in_progress)
+  && !REG_CAN_CHANGE_MODE_P (xregno, xmode, ymode))
 return -1;
 
   /* We shouldn't simplify stack-related registers.  */
Index: testsuite/ChangeLog
===
--- testsuite/ChangeLog	(revision 279060)
+++ testsuite/ChangeLog	(working copy)
@@ -1,3 +1,9 @@
+2019-12-06  Andreas Krebbel  
+	Vladimir Makarov  
+
+	PR rtl-optimization/92176
+	* gcc.target/s390/pr92176.c: New test.
+
 2019-12-06  Martin Sebor  
 
 	* gcc.dg/Wstringop-overflow-23.c: Use the correct argument type.
Index: testsuite/gcc.target/s390/pr92176.c
===
--- testsuite/gcc.target/s390/pr92176.c	(nonexistent)
+++ testsuite/gcc.target/s390/pr92176.c	(working copy)
@@ -0,0 +1,33 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -march=z13 -mzarch" } */
+
+int a = 5, b, c, d, g, h, k, l, m, o;
+static int e[7];
+int *volatile i = &d;
+long long j;
+
+short p(int f, int dummy) {
+  k = 0 != (*e = m);
+  j = 0;
+  for (; j < 59; j = j + 1)
+*i |= b;
+  g = 1;
+  for (; g <= 4; g++) {
+o = 0;
+for (; o <= 4; o++)
+  i = (int * volatile)(long)l;
+  }
+  return 42;
+}
+
+void
+q() {
+  char *n = (char*)&b;
+
+  (*n = a) == p(e[6], c);
+  for (; h;)
+for (;;)
+  ;
+}
+
+/* { dg-final { scan-assembler-not {(?n)^\tvsteb\t.+,0$} } } */

Patch to fix PR92283

[Vladimir Makarov]

The following patch fixes

   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92283

The patch has no test because it is very hard to reproduce PR and check 
the patch even on a specific GCC revision.


The patch was successfully bootstrapped and tested on x86-64.

Committed as r278865


Index: ChangeLog
===
--- ChangeLog	(revision 278864)
+++ ChangeLog	(working copy)
@@ -1,3 +1,9 @@
+2019-11-29  Vladimir Makarov  
+
+	PR rtl-optimization/92283
+	* lra.c (lra): Update reg notes after inheritance sub-pass and
+	before constraint sub-pass.
+
 2019-11-29  Richard Biener  
 
 	PR tree-optimization/91003
Index: lra.c
===
--- lra.c	(revision 278864)
+++ lra.c	(working copy)
@@ -2473,7 +2473,7 @@ lra (FILE *f)
 		 But don't remove dead insns or change global live
 		 info as we can undo inheritance transformations after
 		 inheritance pseudo assigning.  */
-	  lra_create_live_ranges (true, false);
+	  lra_create_live_ranges (true, !lra_simple_p);
 	  live_p = true;
 	  /* If we don't spill non-reload and non-inheritance
 		 pseudos, there is no sense to run memory-memory move
@@ -2514,6 +2514,11 @@ lra (FILE *f)
 		}
 	}
 	  while (fails_p);
+	  if (! live_p) {
+	/* We need the correct reg notes for work of constraint sub-pass.  */
+	lra_create_live_ranges (true, true);
+	live_p = true;
+	  }
 	}
   /* Don't clear optional reloads bitmap until all constraints are
 	 satisfied as we need to differ them from regular reloads.  */

Re: Ping: RFA: patch to fix PR90007

[Vladimir Makarov]

On 2019-11-27 3:31 a.m., Richard Biener wrote:

On Tue, Nov 26, 2019 at 3:57 PM Vladimir Makarov  wrote:

This is the patch removing discrepancy between recog and LRA insn
recognition:

https://gcc.gnu.org/ml/gcc-patches/2019-11/msg01842.html

OK.


Thank you, Richard.  Committed as r278770.

Re: Ping: RFA: patch to fix PR90007

[Richard Biener]

On Tue, Nov 26, 2019 at 3:57 PM Vladimir Makarov  wrote:
>
> This is the patch removing discrepancy between recog and LRA insn
> recognition:
>
> https://gcc.gnu.org/ml/gcc-patches/2019-11/msg01842.html

OK.

Ping: RFA: patch to fix PR90007

[Vladimir Makarov]

This is the patch removing discrepancy between recog and LRA insn 
recognition:


https://gcc.gnu.org/ml/gcc-patches/2019-11/msg01842.html

Index: ChangeLog
===
--- ChangeLog	(revision 278451)
+++ ChangeLog	(working copy)
@@ -1,3 +1,9 @@
+2019-11-19  Vladimir Makarov  
+
+	PR rtl-optimization/90007
+	* recog.c (constrain_operands): Permit hard registers too for
+	memory when LRA is used.
+
 2019-11-19  Martin Liska  
 
 	* toplev.c (general_init): Move the call...
Index: recog.c
===
--- recog.c	(revision 278413)
+++ recog.c	(working copy)
@@ -2757,10 +2757,9 @@ constrain_operands (int strict, alternat
 			   /* Before reload, accept what reload can turn
   into a mem.  */
 			   || (strict < 0 && CONSTANT_P (op))
-			   /* Before reload, accept a pseudo,
+			   /* Before reload, accept a pseudo or hard register,
   since LRA can turn it into a mem.  */
-			   || (strict < 0 && targetm.lra_p () && REG_P (op)
-   && REGNO (op) >= FIRST_PSEUDO_REGISTER)
+			   || (strict < 0 && targetm.lra_p () && REG_P (op))
 			   /* During reload, accept a pseudo  */
 			   || (reload_in_progress && REG_P (op)
    && REGNO (op) >= FIRST_PSEUDO_REGISTER)))
Index: testsuite/ChangeLog
===
--- testsuite/ChangeLog	(revision 278451)
+++ testsuite/ChangeLog	(working copy)
@@ -1,3 +1,8 @@
+2019-11-19  Vladimir Makarov  
+
+	PR rtl-optimization/90007
+	* gcc.target/i386/pr90007.c: New test.
+
 2019-11-15  Andrew Sutton  
 
 	PR c++/89913
Index: testsuite/gcc.target/i386/pr90007.c
===
--- testsuite/gcc.target/i386/pr90007.c	(nonexistent)
+++ testsuite/gcc.target/i386/pr90007.c	(working copy)
@@ -0,0 +1,15 @@
+/* PR rtl-optimization/90007 */
+/* { dg-do compile { target x86_64-*-* } } */
+/* { dg-options "-march=bdver1 -mfpmath=387 -O1 -fschedule-insns -fselective-scheduling" } */
+
+void
+qj (int b9, int r9, int k4, int k0, int e7)
+{
+  (void) b9;
+  (void) r9;
+  (void) k4;
+
+  while (!!k0 == e7 * 1.1)
+{
+}
+}

Re: RFA; patch to fix PR90007

[Alexander Monakov]

On Wed, 20 Nov 2019, Alexander Monakov wrote:

> On Wed, 20 Nov 2019, Richard Biener wrote:
> 
> > On Tue, Nov 19, 2019 at 5:07 PM Vladimir Makarov  
> > wrote:
> > >
> > > The following patch fixes
> > >
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90007
> > >
> > > Sometime ago a code which permits LRA to reload hard register into
> > > memory as it did for pseudo were added.  But this LRA possibility was
> > > not reflected in recog.c.  The following patch fixes this discrepancy
> > > and as a result fixes PR90007.
> > >
> > > OK to commit?
> > 
> > I guess the change is OK but for the bug itself it sounds like
> > selective scheduling doesn't properly recognize insns it
> > proagates into (and avoids doing that then)?  That is,
> > selective scheduling creates invalid RTL?
> 
> We validate the substitution by invoking validate_replace_rtx_part_nosimplify
> from substitute_reg_in_expr.  I think that should be sufficient?  I see 
> similar
> code in haifa-sched uses attempt_change, which also ultimately uses
> apply_change_group.

Although looking at this more, I see that we specifically arrange for a call to
constrain_operands in replace_src_with_reg_ok_p (with a comment), but here in
substitute_reg_in_expr we have a '???' comment that seems to mention that
theoretically there might be a problem, but it never came up in practice.

So there's an inconsistency in sel-sched as well.

Alexander

Re: RFA; patch to fix PR90007

[Vladimir Makarov]

On 2019-11-20 5:06 a.m., Richard Biener wrote:

On Tue, Nov 19, 2019 at 5:07 PM Vladimir Makarov  wrote:

The following patch fixes

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90007

Sometime ago a code which permits LRA to reload hard register into
memory as it did for pseudo were added.  But this LRA possibility was
not reflected in recog.c.  The following patch fixes this discrepancy
and as a result fixes PR90007.

OK to commit?

I guess the change is OK but for the bug itself it sounds like
selective scheduling doesn't properly recognize insns it
proagates into (and avoids doing that then)?  That is,
selective scheduling creates invalid RTL?

It is hard for me to say what should be enough for new insn validation 
in any scheduler code.  I think recog code would be a safe choice as it 
is already used in DFA.


On the other hand using non-recog code for insn validation helped to 
find the code discrepancy between recog and LRA.


In any case I believe that the patch should be committed anyway and it 
fixes the problem.

Re: RFA; patch to fix PR90007

[Alexander Monakov]

On Wed, 20 Nov 2019, Richard Biener wrote:

> On Tue, Nov 19, 2019 at 5:07 PM Vladimir Makarov  wrote:
> >
> > The following patch fixes
> >
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90007
> >
> > Sometime ago a code which permits LRA to reload hard register into
> > memory as it did for pseudo were added.  But this LRA possibility was
> > not reflected in recog.c.  The following patch fixes this discrepancy
> > and as a result fixes PR90007.
> >
> > OK to commit?
> 
> I guess the change is OK but for the bug itself it sounds like
> selective scheduling doesn't properly recognize insns it
> proagates into (and avoids doing that then)?  That is,
> selective scheduling creates invalid RTL?

We validate the substitution by invoking validate_replace_rtx_part_nosimplify
from substitute_reg_in_expr.  I think that should be sufficient?  I see similar
code in haifa-sched uses attempt_change, which also ultimately uses
apply_change_group.

FWIW, here's gdb session transcript showing that the substituted insn is
greenlighted:

Breakpoint 1, substitute_reg_in_expr (expr=0x248ee98, insn=0x7791b880, 
undo=false) at /home/monoid/gcc-vecdoc/gcc/sel-sched.c:743
743   bool has_rhs = VINSN_RHS (*vi) != NULL;
(gdb) call debug_expr(expr)
[((17;r95=flt(r98);)type:set;count:3;)prio:8;orig_bb:2;]
(gdb) call debug_insn(insn)
([((35;r98=r8;)type:use;count:1;)prio:9;orig_bb:2;];seqno:2;)
(gdb) b validate_replace_rtx_part_nosimplify
Breakpoint 2 at 0xcc1cfb: file /home/monoid/gcc-vecdoc/gcc/recog.c, line 835.
(gdb) c
Continuing.

Breakpoint 2, validate_replace_rtx_part_nosimplify 
(from=from@entry=0x77aa2588, to=to@entry=0x77a9acd8, 
where=0x77aa2d00, insn=insn@entry=0x7791b940) at 
/home/monoid/gcc-vecdoc/gcc/recog.c:835
835   validate_replace_rtx_1 (where, from, to, insn, false);
(gdb) call debug_rtx(from)
(reg:SI 98)
(gdb) call debug_rtx(to)
(reg:SI 36 r8 [ e7 ])
(gdb) call debug_rtx(*where)
(float:DF (reg:SI 98))
(gdb) call debug_rtx(insn)
(insn 39 0 0 (set (reg:DF 95)
(float:DF (reg:SI 98))) 167 {*floatsidf2}
 (expr_list:REG_DEAD (reg:SI 98)
(nil)))
(gdb) fin
Run till exit from #0  validate_replace_rtx_part_nosimplify 
(from=from@entry=0x77aa2588, to=to@entry=0x77a9acd8, 
where=0x77aa2d00, insn=insn@entry=0x7791b940) at 
/home/monoid/gcc-vecdoc/gcc/recog.c:835
substitute_reg_in_expr (expr=0x248ee98, insn=, undo=) at /home/monoid/gcc-vecdoc/gcc/sel-sched.c:783
783   if (new_insn_valid)
Value returned is $1 = 1
(gdb) call debug_rtx(new_insn)
(insn 39 0 0 (set (reg:DF 95)
(float:DF (reg:SI 36 r8 [ e7 ]))) 167 {*floatsidf2}
 (expr_list:REG_DEAD (reg:SI 98)
(nil)))


Alexander

Re: RFA; patch to fix PR90007

[Richard Biener]

On Tue, Nov 19, 2019 at 5:07 PM Vladimir Makarov  wrote:
>
> The following patch fixes
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90007
>
> Sometime ago a code which permits LRA to reload hard register into
> memory as it did for pseudo were added.  But this LRA possibility was
> not reflected in recog.c.  The following patch fixes this discrepancy
> and as a result fixes PR90007.
>
> OK to commit?

I guess the change is OK but for the bug itself it sounds like
selective scheduling doesn't properly recognize insns it
proagates into (and avoids doing that then)?  That is,
selective scheduling creates invalid RTL?

Thanks,
Richard.

>

RFA; patch to fix PR90007

[Vladimir Makarov]

The following patch fixes

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90007

Sometime ago a code which permits LRA to reload hard register into 
memory as it did for pseudo were added.  But this LRA possibility was 
not reflected in recog.c.  The following patch fixes this discrepancy 
and as a result fixes PR90007.


OK to commit?

Index: ChangeLog
===
--- ChangeLog	(revision 278451)
+++ ChangeLog	(working copy)
@@ -1,3 +1,9 @@
+2019-11-19  Vladimir Makarov  
+
+	PR rtl-optimization/90007
+	* recog.c (constrain_operands): Permit hard registers too for
+	memory when LRA is used.
+
 2019-11-19  Martin Liska  
 
 	* toplev.c (general_init): Move the call...
Index: recog.c
===
--- recog.c	(revision 278413)
+++ recog.c	(working copy)
@@ -2757,10 +2757,9 @@ constrain_operands (int strict, alternat
 			   /* Before reload, accept what reload can turn
   into a mem.  */
 			   || (strict < 0 && CONSTANT_P (op))
-			   /* Before reload, accept a pseudo,
+			   /* Before reload, accept a pseudo or hard register,
   since LRA can turn it into a mem.  */
-			   || (strict < 0 && targetm.lra_p () && REG_P (op)
-   && REGNO (op) >= FIRST_PSEUDO_REGISTER)
+			   || (strict < 0 && targetm.lra_p () && REG_P (op))
 			   /* During reload, accept a pseudo  */
 			   || (reload_in_progress && REG_P (op)
    && REGNO (op) >= FIRST_PSEUDO_REGISTER)))
Index: testsuite/ChangeLog
===
--- testsuite/ChangeLog	(revision 278451)
+++ testsuite/ChangeLog	(working copy)
@@ -1,3 +1,8 @@
+2019-11-19  Vladimir Makarov  
+
+	PR rtl-optimization/90007
+	* gcc.target/i386/pr90007.c: New test.
+
 2019-11-15  Andrew Sutton  
 
 	PR c++/89913
Index: testsuite/gcc.target/i386/pr90007.c
===
--- testsuite/gcc.target/i386/pr90007.c	(nonexistent)
+++ testsuite/gcc.target/i386/pr90007.c	(working copy)
@@ -0,0 +1,15 @@
+/* PR rtl-optimization/90007 */
+/* { dg-do compile { target x86_64-*-* } } */
+/* { dg-options "-march=bdver1 -mfpmath=387 -O1 -fschedule-insns -fselective-scheduling" } */
+
+void
+qj (int b9, int r9, int k4, int k0, int e7)
+{
+  (void) b9;
+  (void) r9;
+  (void) k4;
+
+  while (!!k0 == e7 * 1.1)
+{
+}
+}

C++ PATCH to fix typo in test name

[Marek Polacek]

Applying as obvious:

A  +initlist-array1.C
> moved from initlist-arrray1.C
D   initlist-arrray1.C
> moved to initlist-array1.C

--
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA

patch to fix PR91223

[Vladimir Makarov]

The following patch fixes

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91223

The patch was successfully bootstrapped and tested on x86-64.

Committed as rev. 273810.

Index: ChangeLog
===
--- ChangeLog	(revision 273809)
+++ ChangeLog	(working copy)
@@ -1,3 +1,9 @@
+2019-07-25  Vladimir Makarov  
+
+	PR rtl-optimization/91223
+	* lra-constraints.c (process_alt_operands): Fail for unsuccessful
+	matching with INOUT operand.
+
 2019-07-25  Eric Botcazou  
 
 	* stmt.c (expand_case): Try to narrow the index type if it's larger
Index: lra-constraints.c
===
--- lra-constraints.c	(revision 273809)
+++ lra-constraints.c	(working copy)
@@ -2171,6 +2171,14 @@ process_alt_operands (int only_alternati
 		  }
 		else
 		  {
+			/* If the operands do not match and one
+			   operand is INOUT, we can not match them.
+			   Try other possibilities, e.g. other
+			   alternatives or commutative operand
+			   exchange.  */
+			if (curr_static_id->operand[nop].type == OP_INOUT
+			|| curr_static_id->operand[m].type == OP_INOUT)
+			  break;
 			/* Operands don't match.  If the operands are
 			   different user defined explicit hard
 			   registers, then we cannot make them match
Index: testsuite/ChangeLog
===
--- testsuite/ChangeLog	(revision 273809)
+++ testsuite/ChangeLog	(working copy)
@@ -1,3 +1,8 @@
+2019-07-25  Vladimir Makarov  
+
+	PR rtl-optimization/91223
+	* gcc.target/i386/pr91223.c: New test.
+
 2019-07-25  Iain Sandoe  
 
 	PR gcov-profile/91087
Index: testsuite/gcc.target/i386/pr91223.c
===
--- testsuite/gcc.target/i386/pr91223.c	(nonexistent)
+++ testsuite/gcc.target/i386/pr91223.c	(working copy)
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-Og -fno-tree-fre" } */
+int a;
+void fn2(short, short);
+
+void fn1(void) {
+  short b[8];
+  b[0] |= a & 3;
+  b[1] = a;
+  fn2(b[0], b[1]);
+}

Re: [PATH] Patch to fix -finstrument-functions-exclude-function-list handling of namespaces and escaped commas

[Jeff Law]

On 7/4/19 2:18 AM, Oliver Browne wrote:
> See below for modified patch, indentation and newline for curly braces
> style applied, and commented out chunk removed. Apologies, indentation
> and newline for scope are not they way I normally write things, habits
> got the better of me, and I forgot to remove the commented out chunk
> before submission.
> 
> Adding the scope braces and writing the for loop in ordinary C instead
> of relying on a macro are changes made for the sake of
> maintainability.
The right way to do this in GCC is to use the facilities we have
designed for these purposes.  In some cases there are constraints that
those facilities enforce and doing an open-coded implementation will not
work the way you want (the immediate use iterators immediately come to
mind) and in other cases the facilities we've built may be much faster
(the bitmap iterators) and in some cases it's just syntatic sugar, but
the consistency with the rest of the codebase is important.

When you don't follow conventions, the maintainer looking at your patch
has to disentangle the real change from your formatting and convention
changes, then apply the fix by hand, possibly getting it wrong in the
process (made even more possible by the lack of a test in the patch).
THen because they made changes, they'll have to sanity tests to make
sure they didn't goof anything with a typo, etc which takes even more of
their limited time.

If you make it easy to review the patch by following conventions, not
making extraneous changes, include tests, indicate that you regression
tested your patch, etc, then it becomes very easy for a maintainer to
look at the patch and gate it in.

Anyway, I've fixed up your patch to follow existing conventions,
bootstrapped and regression tested it and installed the patch on the trunk.

jeff

patch to fix PR91102

[Vladimir Makarov]

  The following patch fixes

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91102

  The patch was bootstrapped and tested on x86-64

Committed as rev. 273357


Index: ChangeLog
===
--- ChangeLog	(revision 273356)
+++ ChangeLog	(working copy)
@@ -1,3 +1,9 @@
+2019-07-10  Vladimir Makarov  
+
+	PR target/91102
+	* lra-constraints.c (process_alt_operands): Don't match user
+	defined regs only if they are early clobbers.
+
 2019-07-10  Marc Glisse  
 
 	* wide-int.h (wi::lshift): Reject negative values for the fast path.
Index: lra-constraints.c
===
--- lra-constraints.c	(revision 273356)
+++ lra-constraints.c	(working copy)
@@ -2172,8 +2172,9 @@ process_alt_operands (int only_alternati
 		else
 		  {
 			/* Operands don't match.  If the operands are
-			   different user defined explicit hard registers,
-			   then we cannot make them match.  */
+			   different user defined explicit hard
+			   registers, then we cannot make them match
+			   when one is early clobber operand.  */
 			if ((REG_P (*curr_id->operand_loc[nop])
 			 || SUBREG_P (*curr_id->operand_loc[nop]))
 			&& (REG_P (*curr_id->operand_loc[m])
@@ -2192,9 +2193,17 @@ process_alt_operands (int only_alternati
 && REG_P (m_reg)
 && HARD_REGISTER_P (m_reg)
 && REG_USERVAR_P (m_reg))
-			  break;
+			  {
+int i;
+
+for (i = 0; i < early_clobbered_regs_num; i++)
+  if (m == early_clobbered_nops[i])
+break;
+if (i < early_clobbered_regs_num
+|| early_clobber_p)
+  break;
+			  }
 			  }
-
 			/* Both operands must allow a reload register,
 			   otherwise we cannot make them match.  */
 			if (curr_alt[m] == NO_REGS)
Index: testsuite/ChangeLog
===
--- testsuite/ChangeLog	(revision 273356)
+++ testsuite/ChangeLog	(working copy)
@@ -1,3 +1,8 @@
+2019-07-10  Vladimir Makarov  
+
+	PR target/91102
+	* gcc.target/aarch64/pr91102.c: New test.
+
 2019-07-10  Richard Biener  
 
 	PR tree-optimization/91126
Index: testsuite/gcc.target/aarch64/pr91102.c
===
--- testsuite/gcc.target/aarch64/pr91102.c	(nonexistent)
+++ testsuite/gcc.target/aarch64/pr91102.c	(working copy)
@@ -0,0 +1,26 @@
+/* PR target/91102 */
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+int
+foo (long d, long l)
+{
+  register long e asm ("x1") = d;
+  register long f asm("x2") = l;
+  asm ("" : : "r" (e), "r" (f));
+  return 3;
+}
+
+struct T { int i; int j; };
+union S { long h; struct T t; };
+
+void
+bar (union S b)
+{
+  while (1)
+{
+  union S c = b;
+  c.t.j++;
+  b.h = foo (b.h, c.h);
+}
+}

Re: [PATH] Patch to fix -finstrument-functions-exclude-function-list handling of namespaces and escaped commas

[Oliver Browne]

See below for modified patch, indentation and newline for curly braces
style applied, and commented out chunk removed. Apologies, indentation
and newline for scope are not they way I normally write things, habits
got the better of me, and I forgot to remove the commented out chunk
before submission.

Adding the scope braces and writing the for loop in ordinary C instead
of relying on a macro are changes made for the sake of
maintainability. This feature is rarely touched (as evidenced by it
having the bugs I'm fixing for so long) and so it is more likely any
patches submitted to it will be submitted by people who a) are not
maintainers of GCC, and so are not familiar with the various macros
etc "normally" used and b) don't have the time / patience to
familiarize themselves with the code base before fixing just the bit
they need to work for whatever they're doing. For people like that
(people like me), having the code be written in as obvious a way as
possible and clearly marking scope of branches makes making fixes
easier (adding a quick printf for debugging is painless, etc), which
is important for low usage features.

Index: gimplify.c
===
--- gimplify.c  2019-06-12 19:07:26.872077000 +0100
+++ gimplify.c  2019-07-04 08:53:08.76842 +0100
@@ -13987,11 +13987,16 @@ flag_instrument_functions_exclude_p (tre
 {
   const char *name;
-  int i;
+  unsigned int i;
   char *s;

-  name = lang_hooks.decl_printable_name (fndecl, 0);
-  FOR_EACH_VEC_ELT (*v, i, s)
-   if (strstr (name, s) != NULL)
- return true;
+  name = lang_hooks.decl_printable_name (fndecl, 1);
+ for(i = 0; i < v->length(); i++)
+ {
+   s = (*v)[i];
+   if(strstr(name, s) != NULL)
+   {
+ return(true);
+   }
+ }
 }
Index: opts.c
===
--- opts.c  2019-06-12 19:10:04.354612000 +0100
+++ opts.c  2019-07-04 08:55:25.287523000 +0100
@@ -263,7 +263,9 @@ add_comma_separated_to_vector (void **pv
*w++ = *r++;
 }
+  *w = '\0';
   if (*token_start != '\0')
+  {
 v->safe_push (token_start);
-
+  }
   *pvec = v;
 }

On Thu, Jul 4, 2019 at 12:31 AM Jeff Law  wrote:
>
> On 6/12/19 12:25 PM, Oliver Browne wrote:
> > Patch fixes following PRs:
> > c++/90816 - -finstrument-functions-exclude-function-list improperly
> > handles namespace/class definitions
> > c++/90809 - -finstrument-functions-exclude-function-list mishandles
> > comma escaping
> >
> > Fixes as follows:
> > At flag_instrument_functions_exclude_p [gimplify.c]
> > Using lang_hooks.decl_printable_name (fndecl, 1) to get namespace /
> > class information as part of printable name to allow for
> > inclusion of namespace / class specification when passing symbols to
> > -finstrument-functions-exclude-function-list. Was
> > previously lang_hooks.decl_printable_name (fndecl, 0).
> >
> > At add_comma_separated_to_vector [opts.c]
> > Added writing of a null character to w after primary loop finishes, to
> > account for offset between r and w when r reaches end of
> > passed string.
> >
> > from Oliver Browne 
> > PR c++/90816
> > PR c++/90809
> >  * gimplify.c (flag_instrument_functions_exclude_p): include namespace
> >information as part of decl name
> >  * opts.c (add_comma_separated_to_vector): add null character to correct
> >position in last token added to token vector
> > Index: gimplify.c
> > ===
> > --- gimplify.c 2019-06-12 19:07:26.872077000 +0100
> > +++ gimplify.c 2019-06-12 18:55:10.609255000 +0100
> > @@ -13987,11 +13987,17 @@ flag_instrument_functions_exclude_p (tre
> >  {
> >const char *name;
> > -  int i;
> > +  unsigned int i;
> >char *s;
> >
> > -  name = lang_hooks.decl_printable_name (fndecl, 0);
> > -  FOR_EACH_VEC_ELT (*v, i, s)
> > +  name = lang_hooks.decl_printable_name (fndecl, 1);
> > +   for(i = 0; i < v->length(); i++){
> > + s = (*v)[i];
> > + if(strstr(name, s) != NULL){
> > +   return(true);
> > + }
> > +   }
> > +/*  FOR_EACH_VEC_ELT (*v, i, s)
> >   if (strstr (name, s) != NULL)
> > -   return true;
> > + return true;*/
> >  }
> So why did you drop the FOR_EACH_VEC_ELT and open-code the loop?  I
> don't see that as being a necessary change.  Leaving the
> FOR_EACH_VEC_ELT in place would also avoid the mis-formatting you've
> introduced as well as removing clutter of a commented-out hunk of code.
>
> >
> > @@ -14278,3 +14284,3 @@ gimplify_hasher::equal (const elt_t *p1,
> >
> >return true;
> > -}
> > \ No newline at end of file
> > +}
> > Index: opts.c
> > ===
> > --- opts.c 2019-06-12 19:10:04.354612000 +0100
> > +++ opts.c 2019-06-12 18:53:43.675852000 +0100
> > @@ -263,7 +263,8 @@ add_comma_separated_t

Re: [PATH] Patch to fix -finstrument-functions-exclude-function-list handling of namespaces and escaped commas

[Jeff Law]

On 6/12/19 12:25 PM, Oliver Browne wrote:
> Patch fixes following PRs:
> c++/90816 - -finstrument-functions-exclude-function-list improperly
> handles namespace/class definitions
> c++/90809 - -finstrument-functions-exclude-function-list mishandles
> comma escaping
> 
> Fixes as follows:
> At flag_instrument_functions_exclude_p [gimplify.c]
> Using lang_hooks.decl_printable_name (fndecl, 1) to get namespace /
> class information as part of printable name to allow for
> inclusion of namespace / class specification when passing symbols to
> -finstrument-functions-exclude-function-list. Was
> previously lang_hooks.decl_printable_name (fndecl, 0).
> 
> At add_comma_separated_to_vector [opts.c]
> Added writing of a null character to w after primary loop finishes, to
> account for offset between r and w when r reaches end of
> passed string.
> 
> from Oliver Browne 
> PR c++/90816
> PR c++/90809
>  * gimplify.c (flag_instrument_functions_exclude_p): include namespace
>information as part of decl name
>  * opts.c (add_comma_separated_to_vector): add null character to correct
>position in last token added to token vector
> Index: gimplify.c
> ===
> --- gimplify.c 2019-06-12 19:07:26.872077000 +0100
> +++ gimplify.c 2019-06-12 18:55:10.609255000 +0100
> @@ -13987,11 +13987,17 @@ flag_instrument_functions_exclude_p (tre
>  {
>const char *name;
> -  int i;
> +  unsigned int i;
>char *s;
> 
> -  name = lang_hooks.decl_printable_name (fndecl, 0);
> -  FOR_EACH_VEC_ELT (*v, i, s)
> +  name = lang_hooks.decl_printable_name (fndecl, 1);
> +   for(i = 0; i < v->length(); i++){
> + s = (*v)[i];
> + if(strstr(name, s) != NULL){
> +   return(true);
> + }
> +   }
> +/*  FOR_EACH_VEC_ELT (*v, i, s)
>   if (strstr (name, s) != NULL)
> -   return true;
> + return true;*/
>  }
So why did you drop the FOR_EACH_VEC_ELT and open-code the loop?  I
don't see that as being a necessary change.  Leaving the
FOR_EACH_VEC_ELT in place would also avoid the mis-formatting you've
introduced as well as removing clutter of a commented-out hunk of code.

> 
> @@ -14278,3 +14284,3 @@ gimplify_hasher::equal (const elt_t *p1,
> 
>return true;
> -}
> \ No newline at end of file
> +}
> Index: opts.c
> ===
> --- opts.c 2019-06-12 19:10:04.354612000 +0100
> +++ opts.c 2019-06-12 18:53:43.675852000 +0100
> @@ -263,7 +263,8 @@ add_comma_separated_to_vector (void **pv
>   *w++ = *r++;
>  }
> -  if (*token_start != '\0')
> +  *w = '\0';
> +  if (*token_start != '\0'){
>  v->safe_push (token_start);
> -
> +  }
>*pvec = v;
So why introduce the unnecessary { } scope?  And why do it in a way that
is different than 99% of the other code in GCC (where the { and } will
be on individual lines with 2 spaces of indention?

Jeff

[PATH] Patch to fix -finstrument-functions-exclude-function-list handling of namespaces and escaped commas

[Oliver Browne]

Patch fixes following PRs:
c++/90816 - -finstrument-functions-exclude-function-list improperly
handles namespace/class definitions
c++/90809 - -finstrument-functions-exclude-function-list mishandles
comma escaping

Fixes as follows:
At flag_instrument_functions_exclude_p [gimplify.c]
Using lang_hooks.decl_printable_name (fndecl, 1) to get namespace /
class information as part of printable name to allow for
inclusion of namespace / class specification when passing symbols to
-finstrument-functions-exclude-function-list. Was
previously lang_hooks.decl_printable_name (fndecl, 0).

At add_comma_separated_to_vector [opts.c]
Added writing of a null character to w after primary loop finishes, to
account for offset between r and w when r reaches end of
passed string.

from Oliver Browne 
PR c++/90816
PR c++/90809
 * gimplify.c (flag_instrument_functions_exclude_p): include namespace
   information as part of decl name
 * opts.c (add_comma_separated_to_vector): add null character to correct
   position in last token added to token vector
Index: gimplify.c
===
--- gimplify.c 2019-06-12 19:07:26.872077000 +0100
+++ gimplify.c 2019-06-12 18:55:10.609255000 +0100
@@ -13987,11 +13987,17 @@ flag_instrument_functions_exclude_p (tre
 {
   const char *name;
-  int i;
+  unsigned int i;
   char *s;

-  name = lang_hooks.decl_printable_name (fndecl, 0);
-  FOR_EACH_VEC_ELT (*v, i, s)
+  name = lang_hooks.decl_printable_name (fndecl, 1);
+   for(i = 0; i < v->length(); i++){
+ s = (*v)[i];
+ if(strstr(name, s) != NULL){
+   return(true);
+ }
+   }
+/*  FOR_EACH_VEC_ELT (*v, i, s)
  if (strstr (name, s) != NULL)
-   return true;
+ return true;*/
 }

@@ -14278,3 +14284,3 @@ gimplify_hasher::equal (const elt_t *p1,

   return true;
-}
\ No newline at end of file
+}
Index: opts.c
===
--- opts.c 2019-06-12 19:10:04.354612000 +0100
+++ opts.c 2019-06-12 18:53:43.675852000 +0100
@@ -263,7 +263,8 @@ add_comma_separated_to_vector (void **pv
  *w++ = *r++;
 }
-  if (*token_start != '\0')
+  *w = '\0';
+  if (*token_start != '\0'){
 v->safe_push (token_start);
-
+  }
   *pvec = v;
 }
@@ -3151,3 +3152,3 @@ option_name (diagnostic_context *context
   else
 return NULL;
-}
\ No newline at end of file
+}

Re: [PATCH][Preprocessor][Version 3]patch to fix PR 90581

[Richard Biener]

On Mon, Jun 3, 2019 at 6:46 PM Qing Zhao  wrote:
>
> Hi,
>
> this is the 3rd version of the patch, which fixed the issues Paolo raised in 
> the previous email.
>
> Okay for trunk?

OK.

Thanks,
Richard.

> thanks.
>
> gcc/ChangeLog:
>
> 2019-06-03  qing zhao  
>
>* doc/cppopts.texi: Add document for -fmax-include-depth.
>* doc/invoke.texi (Preprocessor Options): List -fmax-include-depth.
>
> libcpp/ChangeLog:
>
> 2019-06-03  qing zhao  
>
>* directives.c (do_include_common): Replace CPP_STACK_MAX with
>CPP_OPTION (pfile, max_include_depth).
>* include/cpplib.h (struct cpp_options): Add new field
>max_include_depth.
>* init.c (cpp_create_reader): Initiate new field max_include_depth.
>* internal.h: Delete CPP_STACK_MAX.
>
> gcc/c-family/ChangeLog:
>
> 2019-06-03  qing zhao  
>
>* c-opts.c (c_common_handle_option): Handle -fmax-include-depth.
>* c.opt: Add new option -fmax-include-depth.
>
> gcc/testsuite/ChangeLog:
>
> 2019-06-03  qing zhao  
>
>* c-c++-common/cpp/fmax-include-depth-1a.h: New test.
>* c-c++-common/cpp/fmax-include-depth-1b.h: New test.
>* c-c++-common/cpp/fmax-include-depth.c: New test.
>
>
>
>

[PATCH][Preprocessor][Version 3]patch to fix PR 90581

[Qing Zhao]

Hi,

this is the 3rd version of the patch, which fixed the issues Paolo raised in 
the previous email.

Okay for trunk?

thanks.

gcc/ChangeLog:

2019-06-03  qing zhao  

   * doc/cppopts.texi: Add document for -fmax-include-depth.
   * doc/invoke.texi (Preprocessor Options): List -fmax-include-depth.

libcpp/ChangeLog:

2019-06-03  qing zhao  

   * directives.c (do_include_common): Replace CPP_STACK_MAX with
   CPP_OPTION (pfile, max_include_depth).
   * include/cpplib.h (struct cpp_options): Add new field
   max_include_depth.
   * init.c (cpp_create_reader): Initiate new field max_include_depth.
   * internal.h: Delete CPP_STACK_MAX.

gcc/c-family/ChangeLog:

2019-06-03  qing zhao  

   * c-opts.c (c_common_handle_option): Handle -fmax-include-depth.
   * c.opt: Add new option -fmax-include-depth.

gcc/testsuite/ChangeLog:

2019-06-03  qing zhao  

   * c-c++-common/cpp/fmax-include-depth-1a.h: New test.
   * c-c++-common/cpp/fmax-include-depth-1b.h: New test.
   * c-c++-common/cpp/fmax-include-depth.c: New test.



0001-PR-preprocessor-90581.patch
Description: Binary data

Re: [PATCH][Preprocessor][Version 3]patch to fix PR 90581

[Qing Zhao]

Hi, Paolo,

thanks for the comment.

I will fix these issues.

Qing


> On Jun 3, 2019, at 10:03 AM, Paolo Carlini  wrote:
> 
> Hi,
> 
> a few minor comments.
> 
> On 03/06/19 16:49, Qing Zhao wrote:
>> +fmax-include-depth=
>> +C ObjC C++ ObjC++ Joined RejectNegative UInteger
>> +fmax-include-depth= Set the maximum number of depth of nested 
>> #include.
> 
> I think you want something like "Set the maximum depth of nested #include".
> 
>> +@item -fmax-include-depth=@var{depth}
>> +@opindex fmax-include-depth
>> +Set the maximum number of depth of the nested include. The default is 200.
> 
> Likewise, plus include or #include?
> 
> In any case the wording should be consistent with the actual cpp_error 
> message below in your attachment, thus, in particular, no "number".
> 
>> +  /* the maximum depths for nested #include.  */
> 
> Upper case "The". "for" or "of"? Again, consistency.
> 
> Paolo.
> 
> 

Re: [PATCH][Preprocessor][Version 2]patch to fix PR 90581

[Paolo Carlini]

Hi,

a few minor comments.

On 03/06/19 16:49, Qing Zhao wrote:

+fmax-include-depth=
+C ObjC C++ ObjC++ Joined RejectNegative UInteger
+fmax-include-depth= Set the maximum number of depth of nested #include.


I think you want something like "Set the maximum depth of nested #include".


+@item -fmax-include-depth=@var{depth}
+@opindex fmax-include-depth
+Set the maximum number of depth of the nested include. The default is 200.


Likewise, plus include or #include?

In any case the wording should be consistent with the actual cpp_error 
message below in your attachment, thus, in particular, no "number".



+  /* the maximum depths for nested #include.  */


Upper case "The". "for" or "of"? Again, consistency.

Paolo.

[PATCH][Preprocessor][Version 2]patch to fix PR 90581

[Qing Zhao]

Hi, this is the 2nd version of the patch.

the main changes are:

1. some typo.
2. enhance the error message to provide user idea on how to increase the 
limit.

bootstrap and regression tested on X86, no issue.

Okay for trunk?

thanks.

Qing

gcc/ChangeLog:

2019-06-03  qing zhao  

* doc/cppopts.texi: Add document for -fmax-include-depth.
* doc/invoke.texi (Preprocessor Options): List -fmax-include-depth.

libcpp/ChangeLog:

2019-06-03  qing zhao  

* directives.c (do_include_common): Replace CPP_STACK_MAX with
CPP_OPTION (pfile, max_include_depth).
* include/cpplib.h (struct cpp_options): Add new field
max_include_depth.
* init.c (cpp_create_reader): Initiate new field max_include_depth.
* internal.h: Delete CPP_STACK_MAX.

gcc/c-family/ChangeLog:

2019-06-03  qing zhao  

* c-opts.c (c_common_handle_option): Handle -fmax-include-depth.
* c.opt: Add new option -fmax-include-depth.

gcc/testsuite/ChangeLog:

2019-06-03  qing zhao  

* c-c++-common/cpp/fmax-include-depth-1a.h: New test.
* c-c++-common/cpp/fmax-include-depth-1b.h: New test.
* c-c++-common/cpp/fmax-include-depth.c: New test.



0001-PR-preprocessor-90581.patch
Description: Binary data

Re: [PATCH][Preprocessor]patch to fix PR 90581

[Richard Biener]

On Thu, May 30, 2019 at 10:46 PM David Malcolm  wrote:
>
> On Thu, 2019-05-30 at 11:23 -0500, Qing Zhao wrote:
> > Hi,
> >
> > PR 90581 (provide an option to adjust the maximum depth of nested
> > #include)
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90581
> >
> > is to add a new cpp option -fmax-inlcude-depth to set the maximum
> > depth of nested #include.
> >
> > '-fmax-include-depth=DEPTH'
> >  Set the maximum depth of the nested include.  The default value
> > is
> >  200.
> >
> > Please check the attached patch.
> > I have done bootstrap and regression test on X86, no any issue.
> >
> > thanks a lot.
> >
> > Qing.
> >
> Thanks for working on this.  It's looking promising, but I agree that a
> param would be better than an option.

Not sure - this is for language limits and we do have existing
like -ftemplate-backtrace-limit and -ftemplate-depth.

> One idea that occurred to me looking at the patch...
>
> > index 3ee8bc4..480c282 100644
> > --- a/libcpp/directives.c
> > +++ b/libcpp/directives.c
> > @@ -831,7 +831,7 @@ do_include_common (cpp_reader *pfile, enum include_type 
> > type)
> >  }
> >
> >/* Prevent #include recursion.  */
> > -  if (pfile->line_table->depth >= CPP_STACK_MAX)
> > +  if (pfile->line_table->depth >= CPP_OPTION (pfile, max_include_depth))
> >  cpp_error (pfile, CPP_DL_ERROR, "#include nested too deeply");
>
> ...a nice usability tweak here would be to give a hint about the new
> param, to give the user an idea on how to increase the limit.
>
> Maybe something like:
>
> cpp_error (pfile, CPP_DL_ERROR,
>   "%<#include%> nested too deeply (depth %i);"
>   " use %<--param max-include-depth=LIMIT%> to support deeper 
> nesting",
>   pfile->line_table->depth);
>
> (though probably that would be better as a followup "note")
>
> Dave

Re: [PATCH][Preprocessor]patch to fix PR 90581

[Qing Zhao]

> On May 30, 2019, at 3:46 PM, David Malcolm  wrote:
> 
> On Thu, 2019-05-30 at 11:23 -0500, Qing Zhao wrote:
>> Hi,
>> 
>> PR 90581 (provide an option to adjust the maximum depth of nested
>> #include)
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90581
>> 
>> is to add a new cpp option -fmax-inlcude-depth to set the maximum
>> depth of nested #include.
>> 
>> '-fmax-include-depth=DEPTH'
>> Set the maximum depth of the nested include.  The default value
>> is
>> 200.
>> 
>> Please check the attached patch.
>> I have done bootstrap and regression test on X86, no any issue.
>> 
>> thanks a lot.
>> 
>> Qing.
>> 
> Thanks for working on this.  It's looking promising, but I agree that a
> param would be better than an option.

I just checked both gcc source code and gcc documentation. 
looks like that 

—param name=value 

is an option for “Options that Control Optimization”.

Since this new option is for preprocessor, I think that it might not be fit in?

let me know if I miss anything here.

> 
> One idea that occurred to me looking at the patch...
> 
>> index 3ee8bc4..480c282 100644
>> --- a/libcpp/directives.c
>> +++ b/libcpp/directives.c
>> @@ -831,7 +831,7 @@ do_include_common (cpp_reader *pfile, enum include_type 
>> type)
>> }
>> 
>>   /* Prevent #include recursion.  */
>> -  if (pfile->line_table->depth >= CPP_STACK_MAX)
>> +  if (pfile->line_table->depth >= CPP_OPTION (pfile, max_include_depth))
>> cpp_error (pfile, CPP_DL_ERROR, "#include nested too deeply");
> 
> ...a nice usability tweak here would be to give a hint about the new
> param, to give the user an idea on how to increase the limit.

Yes, I agree.
will fix this.

Thanks.

Qing
> 
> Maybe something like:
> 
> cpp_error (pfile, CPP_DL_ERROR,
> "%<#include%> nested too deeply (depth %i);"
> " use %<--param max-include-depth=LIMIT%> to support deeper nesting",
> pfile->line_table->depth);
> 
> (though probably that would be better as a followup "note")
> 
> Dave

Re: [PATCH][Preprocessor]patch to fix PR 90581

[David Malcolm]

On Thu, 2019-05-30 at 11:23 -0500, Qing Zhao wrote:
> Hi,
> 
> PR 90581 (provide an option to adjust the maximum depth of nested
> #include)
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90581
> 
> is to add a new cpp option -fmax-inlcude-depth to set the maximum
> depth of nested #include.
> 
> '-fmax-include-depth=DEPTH'
>  Set the maximum depth of the nested include.  The default value
> is
>  200.
> 
> Please check the attached patch.
> I have done bootstrap and regression test on X86, no any issue.
> 
> thanks a lot.
> 
> Qing.
> 
Thanks for working on this.  It's looking promising, but I agree that a
param would be better than an option.

One idea that occurred to me looking at the patch...

> index 3ee8bc4..480c282 100644
> --- a/libcpp/directives.c
> +++ b/libcpp/directives.c
> @@ -831,7 +831,7 @@ do_include_common (cpp_reader *pfile, enum include_type 
> type)
>  }
>  
>/* Prevent #include recursion.  */
> -  if (pfile->line_table->depth >= CPP_STACK_MAX)
> +  if (pfile->line_table->depth >= CPP_OPTION (pfile, max_include_depth))
>  cpp_error (pfile, CPP_DL_ERROR, "#include nested too deeply");

...a nice usability tweak here would be to give a hint about the new
param, to give the user an idea on how to increase the limit.

Maybe something like:

cpp_error (pfile, CPP_DL_ERROR,
  "%<#include%> nested too deeply (depth %i);"
  " use %<--param max-include-depth=LIMIT%> to support deeper nesting",
  pfile->line_table->depth);

(though probably that would be better as a followup "note")

Dave

Re: [PATCH][Preprocessor]patch to fix PR 90581

[Qing Zhao]

> On May 30, 2019, at 3:38 PM, David Malcolm  wrote:
> 
> On Thu, 2019-05-30 at 15:17 -0500, Qing Zhao wrote:
>>> On May 30, 2019, at 2:13 PM, Bernhard Reutner-Fischer >> gmail.com> wrote:
>>> 
>>> On 30 May 2019 18:23:43 CEST, Qing Zhao 
>>> wrote:
 Hi,
 
 PR 90581 (provide an option to adjust the maximum depth of nested
 #include)
 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90581
 
 is to add a new cpp option -fmax-inlcude-depth
>>> 
>>> Typo inl vs inc.
>> 
>> Okay, will fix this.
>> 
>>> 
>>> Why isn't this a param?
>> 
>> do you mean to change “-fmax-include-depth=” to “-param max-include-
>> depth=“?
>> 
> 
> That sounds like a good idea to me.

can -param be accepted by preprocessor? 

I didn’t see any -param in gcc/c-family/c.opt.

where should I define a -param for preprocessor?

thanks.

Qing
> 
>>> Wouldn't a param ease range checking not to overflow the uint max
>>> and maybe automagically provide diagnostics for out of range input?
> 
> (indeed)
> 
> 

Re: [PATCH][Preprocessor]patch to fix PR 90581

[David Malcolm]

On Thu, 2019-05-30 at 15:17 -0500, Qing Zhao wrote:
> > On May 30, 2019, at 2:13 PM, Bernhard Reutner-Fischer  > gmail.com> wrote:
> > 
> > On 30 May 2019 18:23:43 CEST, Qing Zhao 
> > wrote:
> > > Hi,
> > > 
> > > PR 90581 (provide an option to adjust the maximum depth of nested
> > > #include)
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90581
> > > 
> > > is to add a new cpp option -fmax-inlcude-depth
> > 
> > Typo inl vs inc.
> 
> Okay, will fix this.
> 
> > 
> > Why isn't this a param?
> 
> do you mean to change “-fmax-include-depth=” to “-param max-include-
> depth=“?
> 

That sounds like a good idea to me.

> > Wouldn't a param ease range checking not to overflow the uint max
> > and maybe automagically provide diagnostics for out of range input?

(indeed)

Re: [PATCH][Preprocessor]patch to fix PR 90581

[Qing Zhao]

> On May 30, 2019, at 2:13 PM, Bernhard Reutner-Fischer  
> wrote:
> 
> On 30 May 2019 18:23:43 CEST, Qing Zhao  wrote:
>> Hi,
>> 
>> PR 90581 (provide an option to adjust the maximum depth of nested
>> #include)
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90581
>> 
>> is to add a new cpp option -fmax-inlcude-depth
> 
> Typo inl vs inc.

Okay, will fix this.

> 
> Why isn't this a param?

do you mean to change “-fmax-include-depth=” to “-param max-include-depth=“?

> Wouldn't a param ease range checking not to overflow the uint max and maybe 
> automagically provide diagnostics for out of range input?
> 
> In the docs you mix "number" and "depth”.

will fix this.

thanks for the feedback.

Qing
> 
> thanks,
> 
>> to set the maximum depth
>> of nested #include.
>> 
>> '-fmax-include-depth=DEPTH'
>>Set the maximum depth of the nested include.  The default value is
>>200.
>> 
>> Please check the attached patch.
>> I have done bootstrap and regression test on X86, no any issue.
>> 
>> thanks a lot.
>> 
>> Qing.
>> 
>> 
>> 
>> gcc/ChangeLog:
>> 
>> 2019-05-30  qing zhao  
>> 
>>   * doc/cppopts.texi: Add document for -fmax-include-depth.
>>   * doc/invoke.texi (Preprocessor Options): List -fmax-include-depth.
>> 
>> libcpp/ChangeLog:
>> 
>> 2019-05-30  qing zhao  
>> 
>>   * directives.c (do_include_common): Replace CPP_STACK_MAX with
>>   CPP_OPTION (pfile, max_include_depth).
>>   * include/cpplib.h (struct cpp_options): Add new field
>>   max_include_depth.
>>   * init.c (cpp_create_reader): Initiate new field max_include_depth.
>>   * internal.h: Delete CPP_STACK_MAX.
>> 
>> gcc/c-family/ChangeLog:
>> 
>> 2019-05-30  qing zhao  
>> 
>>  * c-opts.c (c_common_handle_option): Handle -fmax-include-depth.
>>   * c.opt: Add new option -fmax-include-depth.
>> 
>> gcc/testsuite/ChangeLog:
>> 
>> 2019-05-30  qing zhao  
>> 
>>   * c-c++-common/cpp/fmax-include-depth-1a.h: New test.
>>   * c-c++-common/cpp/fmax-include-depth-1b.h: New test.
>>   * c-c++-common/cpp/fmax-include-depth.c: New test.
> 

Re: [PATCH][Preprocessor]patch to fix PR 90581

[Bernhard Reutner-Fischer]

On 30 May 2019 18:23:43 CEST, Qing Zhao  wrote:
>Hi,
>
>PR 90581 (provide an option to adjust the maximum depth of nested
>#include)
>https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90581
>
>is to add a new cpp option -fmax-inlcude-depth

Typo inl vs inc.

Why isn't this a param?
Wouldn't a param ease range checking not to overflow the uint max and maybe 
automagically provide diagnostics for out of range input?

In the docs you mix "number" and "depth".

thanks,

> to set the maximum depth
>of nested #include.
>
>'-fmax-include-depth=DEPTH'
> Set the maximum depth of the nested include.  The default value is
> 200.
>
>Please check the attached patch.
>I have done bootstrap and regression test on X86, no any issue.
>
>thanks a lot.
>
>Qing.
>
>
>
>gcc/ChangeLog:
>
>2019-05-30  qing zhao  
>
>* doc/cppopts.texi: Add document for -fmax-include-depth.
>* doc/invoke.texi (Preprocessor Options): List -fmax-include-depth.
>
>libcpp/ChangeLog:
>
>2019-05-30  qing zhao  
>
>* directives.c (do_include_common): Replace CPP_STACK_MAX with
>CPP_OPTION (pfile, max_include_depth).
>* include/cpplib.h (struct cpp_options): Add new field
>max_include_depth.
>* init.c (cpp_create_reader): Initiate new field max_include_depth.
>* internal.h: Delete CPP_STACK_MAX.
>
>gcc/c-family/ChangeLog:
>
>2019-05-30  qing zhao  
>
>   * c-opts.c (c_common_handle_option): Handle -fmax-include-depth.
>* c.opt: Add new option -fmax-include-depth.
>
>gcc/testsuite/ChangeLog:
>
>2019-05-30  qing zhao  
>
>* c-c++-common/cpp/fmax-include-depth-1a.h: New test.
>* c-c++-common/cpp/fmax-include-depth-1b.h: New test.
>* c-c++-common/cpp/fmax-include-depth.c: New test.

C++ PATCH to fix typo in cp-tree.h

[Marek Polacek]

Noticed this is -> if typo.  Applying to trunk as obvious.

2019-05-30  Marek Polacek  

* cp-tree.h (TYPE_HAS_NONTRIVIAL_DESTRUCTOR): Fix a typo.

diff --git gcc/cp/cp-tree.h gcc/cp/cp-tree.h
index 7a74fd4fac5..edd59d5f000 100644
--- gcc/cp/cp-tree.h
+++ gcc/cp/cp-tree.h
@@ -4295,7 +4295,7 @@ more_aggr_init_expr_args_p (const 
aggr_init_expr_arg_iterator *iter)
 /* Nonzero for _TYPE node means that this type does not have a trivial
destructor.  Therefore, destroying an object of this type will
involve a call to a destructor.  This can apply to objects of
-   ARRAY_TYPE is the type of the elements needs a destructor.  */
+   ARRAY_TYPE if the type of the elements needs a destructor.  */
 #define TYPE_HAS_NONTRIVIAL_DESTRUCTOR(NODE) \
   (TYPE_LANG_FLAG_4 (NODE))
 

[PATCH][Preprocessor]patch to fix PR 90581

[Qing Zhao]

Hi,

PR 90581 (provide an option to adjust the maximum depth of nested #include)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90581

is to add a new cpp option -fmax-inlcude-depth to set the maximum depth of 
nested #include.

'-fmax-include-depth=DEPTH'
 Set the maximum depth of the nested include.  The default value is
 200.

Please check the attached patch.
I have done bootstrap and regression test on X86, no any issue.

thanks a lot.

Qing.



gcc/ChangeLog:

2019-05-30  qing zhao  

* doc/cppopts.texi: Add document for -fmax-include-depth.
* doc/invoke.texi (Preprocessor Options): List -fmax-include-depth.

libcpp/ChangeLog:

2019-05-30  qing zhao  

* directives.c (do_include_common): Replace CPP_STACK_MAX with
CPP_OPTION (pfile, max_include_depth).
* include/cpplib.h (struct cpp_options): Add new field
max_include_depth.
* init.c (cpp_create_reader): Initiate new field max_include_depth.
* internal.h: Delete CPP_STACK_MAX.

gcc/c-family/ChangeLog:

2019-05-30  qing zhao  

* c-opts.c (c_common_handle_option): Handle -fmax-include-depth.
* c.opt: Add new option -fmax-include-depth.

gcc/testsuite/ChangeLog:

2019-05-30  qing zhao  

* c-c++-common/cpp/fmax-include-depth-1a.h: New test.
* c-c++-common/cpp/fmax-include-depth-1b.h: New test.
* c-c++-common/cpp/fmax-include-depth.c: New test.



0001-PR-preprocessor-90581.patch
Description: Binary data

Re: C++ PATCH to fix naked inform call for strong using directive

[Marek Polacek]

On Mon, May 20, 2019 at 01:44:47PM -0400, Jason Merrill wrote:
> On 5/20/19 11:56 AM, Marek Polacek wrote:
> > This patch fixes a naked inform call that can be seen with -w.
> > 
> > While at it, I added missing quotes; I'm surprisesd this wasn't fixed by the
> > recent patch Martin submitted.
> > 
> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> 
> OK.  We might also change "may" to "can" in the inform.

Done in the following, thanks.

Bootstrapped/regtested on x86_64-linux, applying to trunk.

2019-05-20  Marek Polacek  

* name-lookup.c (finish_using_directive): Don't issue inform() if the
warning didn't trigger.  Add quoting.  Tweak the inform message.

* g++.dg/lookup/strong-using2.C: New test.

diff --git gcc/cp/name-lookup.c gcc/cp/name-lookup.c
index f7952eebe0c..476ba509231 100644
--- gcc/cp/name-lookup.c
+++ gcc/cp/name-lookup.c
@@ -7258,10 +7258,10 @@ finish_using_directive (tree target, tree attribs)
if (current_binding_level->kind == sk_namespace
&& is_attribute_p ("strong", name))
  {
-   warning (0, "strong using directive no longer supported");
-   if (CP_DECL_CONTEXT (target) == current_namespace)
+   if (warning (0, "% using directive no longer supported")
+   && CP_DECL_CONTEXT (target) == current_namespace)
  inform (DECL_SOURCE_LOCATION (target),
- "you may use an inline namespace instead");
+ "you can use an inline namespace instead");
  }
else
  warning (OPT_Wattributes, "%qD attribute directive ignored", name);
diff --git gcc/testsuite/g++.dg/lookup/strong-using2.C 
gcc/testsuite/g++.dg/lookup/strong-using2.C
new file mode 100644
index 000..17284949645
--- /dev/null
+++ gcc/testsuite/g++.dg/lookup/strong-using2.C
@@ -0,0 +1,11 @@
+// { dg-do compile { target c++11 } }
+// { dg-options "-w" }
+
+namespace A
+{
+  namespace B // { dg-bogus "inline namespace" }
+  {
+  }
+
+  using namespace B __attribute__ ((strong)); // { dg-bogus "no longer 
supported" }
+}

Re: C++ PATCH to fix naked inform call for strong using directive

[Jason Merrill]

On 5/20/19 11:56 AM, Marek Polacek wrote:

This patch fixes a naked inform call that can be seen with -w.

While at it, I added missing quotes; I'm surprisesd this wasn't fixed by the
recent patch Martin submitted.

Bootstrapped/regtested on x86_64-linux, ok for trunk?


OK.  We might also change "may" to "can" in the inform.

Jason

Re: C++ PATCH to fix ext/utf8-2.C with -std=c++2a

[Jason Merrill]

On 5/20/19 12:23 PM, Marek Polacek wrote:

On Mon, May 20, 2019 at 12:18:18PM -0400, Marek Polacek wrote:

This test fails in C++2a since r271338, because the types in the "aka" differ:
in C++17 and below we print:
   {aka 'const char [3]'}
while in C++2a:
   {aka 'const char8_t [3]'}

We can make the regexp accept both.


...because we'll print "char8_t" in other modes too, with -fchar8_t.


OK.

Jason

Re: C++ PATCH to fix ext/utf8-2.C with -std=c++2a

[Marek Polacek]

On Mon, May 20, 2019 at 12:18:18PM -0400, Marek Polacek wrote:
> This test fails in C++2a since r271338, because the types in the "aka" differ:
> in C++17 and below we print:
>   {aka 'const char [3]'}
> while in C++2a:
>   {aka 'const char8_t [3]'}
> 
> We can make the regexp accept both.

...because we'll print "char8_t" in other modes too, with -fchar8_t.

Marek

C++ PATCH to fix ext/utf8-2.C with -std=c++2a

[Marek Polacek]

This test fails in C++2a since r271338, because the types in the "aka" differ:
in C++17 and below we print:
  {aka 'const char [3]'}
while in C++2a:
  {aka 'const char8_t [3]'}

We can make the regexp accept both.

Tested on x86_64-linux, ok for trunk?

2019-05-20  Marek Polacek  

* g++.dg/ext/utf8-2.C: Accept both "char" and "char8_t" in aka.

diff --git gcc/testsuite/g++.dg/ext/utf8-2.C gcc/testsuite/g++.dg/ext/utf8-2.C
index 1db5c383fd6..5ce13fbe6be 100644
--- gcc/testsuite/g++.dg/ext/utf8-2.C
+++ gcc/testsuite/g++.dg/ext/utf8-2.C
@@ -12,16 +12,16 @@ const char16_t  s1[]= u8"ab";   // { dg-error 
"from a string literal with type arr
 const char32_t  s2[]= u8"ab";  // { dg-error "from a string literal 
with type array of .char." }
 const wchar_t   s3[]= u8"ab";  // { dg-error "from a string literal 
with type array of .char." }
 
-const u8_char_t  t0[0]   = u8"ab"; // { dg-error "initializer-string for 
'const u8_char_t \\\[0]' {aka 'const char \\\[0]'} is too long" }
-const u8_char_t  t1[1]   = u8"ab"; // { dg-error "initializer-string for 
'const u8_char_t \\\[1]' {aka 'const char \\\[1]'} is too long" }
-const u8_char_t  t2[2]   = u8"ab"; // { dg-error "initializer-string for 
'const u8_char_t \\\[2]' {aka 'const char \\\[2]'} is too long" }
+const u8_char_t  t0[0]   = u8"ab"; // { dg-error "initializer-string for 
'const u8_char_t \\\[0]' {aka 'const \(char|char8_t\) \\\[0]'} is too long" }
+const u8_char_t  t1[1]   = u8"ab"; // { dg-error "initializer-string for 
'const u8_char_t \\\[1]' {aka 'const \(char|char8_t\) \\\[1]'} is too long" }
+const u8_char_t  t2[2]   = u8"ab"; // { dg-error "initializer-string for 
'const u8_char_t \\\[2]' {aka 'const \(char|char8_t\) \\\[2]'} is too long" }
 const u8_char_t  t3[3]   = u8"ab";
 const u8_char_t  t4[4]   = u8"ab";
 
-const u8_char_t  u0[0]   = u8"\u2160.";// { dg-error 
"initializer-string for 'const u8_char_t \\\[0]' {aka 'const char \\\[0]'} is 
too long" }
-const u8_char_t  u1[1]   = u8"\u2160.";// { dg-error 
"initializer-string for 'const u8_char_t \\\[1]' {aka 'const char \\\[1]'} is 
too long" }
-const u8_char_t  u2[2]   = u8"\u2160.";// { dg-error 
"initializer-string for 'const u8_char_t \\\[2]' {aka 'const char \\\[2]'} is 
too long" }
-const u8_char_t  u3[3]   = u8"\u2160.";// { dg-error 
"initializer-string for 'const u8_char_t \\\[3]' {aka 'const char \\\[3]'} is 
too long" }
-const u8_char_t  u4[4]   = u8"\u2160.";// { dg-error 
"initializer-string for 'const u8_char_t \\\[4]' {aka 'const char \\\[4]'} is 
too long" }
+const u8_char_t  u0[0]   = u8"\u2160.";// { dg-error 
"initializer-string for 'const u8_char_t \\\[0]' {aka 'const \(char|char8_t\) 
\\\[0]'} is too long" }
+const u8_char_t  u1[1]   = u8"\u2160.";// { dg-error 
"initializer-string for 'const u8_char_t \\\[1]' {aka 'const \(char|char8_t\) 
\\\[1]'} is too long" }
+const u8_char_t  u2[2]   = u8"\u2160.";// { dg-error 
"initializer-string for 'const u8_char_t \\\[2]' {aka 'const \(char|char8_t\) 
\\\[2]'} is too long" }
+const u8_char_t  u3[3]   = u8"\u2160.";// { dg-error 
"initializer-string for 'const u8_char_t \\\[3]' {aka 'const \(char|char8_t\) 
\\\[3]'} is too long" }
+const u8_char_t  u4[4]   = u8"\u2160.";// { dg-error 
"initializer-string for 'const u8_char_t \\\[4]' {aka 'const \(char|char8_t\) 
\\\[4]'} is too long" }
 const u8_char_t  u5[5]   = u8"\u2160.";
 const u8_char_t  u6[6]   = u8"\u2160.";

C++ PATCH to fix naked inform call for strong using directive

[Marek Polacek]

This patch fixes a naked inform call that can be seen with -w.

While at it, I added missing quotes; I'm surprisesd this wasn't fixed by the
recent patch Martin submitted. 

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2019-05-20  Marek Polacek  

* name-lookup.c (finish_using_directive): Don't issue inform() if the
warning didn't trigger.  Add quoting.

* g++.dg/lookup/strong-using2.C: New test.

diff --git gcc/cp/name-lookup.c gcc/cp/name-lookup.c
index f7952eebe0c..fe3d5414e7c 100644
--- gcc/cp/name-lookup.c
+++ gcc/cp/name-lookup.c
@@ -7258,8 +7258,8 @@ finish_using_directive (tree target, tree attribs)
if (current_binding_level->kind == sk_namespace
&& is_attribute_p ("strong", name))
  {
-   warning (0, "strong using directive no longer supported");
-   if (CP_DECL_CONTEXT (target) == current_namespace)
+   if (warning (0, "% using directive no longer supported")
+   && CP_DECL_CONTEXT (target) == current_namespace)
  inform (DECL_SOURCE_LOCATION (target),
  "you may use an inline namespace instead");
  }
diff --git gcc/testsuite/g++.dg/lookup/strong-using2.C 
gcc/testsuite/g++.dg/lookup/strong-using2.C
new file mode 100644
index 000..0c497245df0
--- /dev/null
+++ gcc/testsuite/g++.dg/lookup/strong-using2.C
@@ -0,0 +1,11 @@
+// { dg-do compile { target c++11 } }
+// { dg-options "-w" }
+
+namespace A
+{
+  namespace B // { dg-bogus "inline namespace" }
+  {
+  }
+
+  using namespace B __attribute__ ((strong)); // { dg-bogus "no longer 
supported" }
+}

Re: RFA: Patch to fix severe recursion in d_count_templates_scopes (PR 89394)

[Jeff Law]

On 3/21/19 6:04 AM, Nick Clifton wrote:
> Hi Ian,
> 
>   Attached is a proposed patch to fix PR 89394, which contains an
>   artificial mangled name that triggers excessive recursion in
>   d_count_templates_scopes.  The patch uses the same recursion limit
>   that is already in place for d_print_comp, which I hope will be
>   acceptable.
> 
>   There is one frag in the patch which is not directly related to this
>   recursion problem however.  It extends the check in
>   cplus_demangle_fill_name so that names with a negative length are
>   rejected.  I had originally thought that the excessive recursion was
>   due to a negative length string, although further investigation proved
>   this guess to be wrong.  I felt that leaving the check in however
>   would still be a good idea.
> 
>   Tested with no regressions with an x86_64-linux-gnu toolchain, as well
>   as against the testcase in PR 89394.
> 
>   OK to apply ?
> 
> Cheers
>   Nick
> 
> libiberty/ChangeLog
> 2019-03-21  Nick Clifton  
> 
>   PR 89394
>   * cp-demangle.c (cplus_demangle_fill_name): Reject negative
>   lengths.
>   (d_count_templates_scopes): Replace num_templates and num_scopes
>   parameters with a struct d_print_info pointer parameter.  Adjust
>   body of the function accordingly.  Add recursion counter and check
>   that the recursion limit is not reached.
>   (d_print_init): Pass dpi parameter to d_count_templates_scopes.
>   Reset recursion counter afterwards, unless the recursion limit was
>   reached.
> It's actually a fairly trivial patch once you know that d_print_init
sets up dpi :-)

Given some folks seem to think this is a security issue, I'm going to
ACK for gcc-9 even though it's not a regression.

Jeff

[PATCH] Proposed patch to fix bug id, 89796 on bugzilla

[Nicholas Krause]

Not sure if this is a correct fix to this bug found here:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89796 but
comments are welcome. If a backtrace is required please
let me know.

Signed-off-by: Nicholas Krause 
---
 gcc/cp/constraint.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 9884eb0db50..a78d0a9a49b 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -1882,7 +1882,7 @@ tsubst_requires_expr (tree t, tree args,
   tree parms = TREE_OPERAND (t, 0);
   if (parms)
 {
-  parms = tsubst_constraint_variables (parms, args, complain, in_decl);
+  parms = tsubst_constraint_variables (PARM_CONSTR_PARMS (parms), args, 
complain, in_decl);
   if (parms == error_mark_node)
 return error_mark_node;
 }
-- 
2.17.1

Patch to fix PR89676

[Vladimir Makarov]

  The following patch fixes

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89676

  The patch was successfully bootstrapped and tested on x86-64.

  Committed as rev. 269878.

Index: ChangeLog
===
--- ChangeLog	(revision 269876)
+++ ChangeLog	(working copy)
@@ -1,3 +1,9 @@
+2019-03-22  Vladimir Makarov  
+
+	PR rtl-optimization/89676
+	* lra-constraints.c (curr_insn_transform): Do match reload for
+	early clobbers even if the match was successful.
+
 2019-03-22  Jakub Jelinek  
 
 	PR c++/87481
Index: lra-constraints.c
===
--- lra-constraints.c	(revision 269876)
+++ lra-constraints.c	(working copy)
@@ -4256,6 +4256,20 @@ curr_insn_transform (bool check_only_p)
 			   || MEM_P (SET_DEST (curr_insn_set))
 			   || GET_CODE (SET_DEST (curr_insn_set)) == SUBREG
 	optional_p = true;
+	  else if (goal_alt_matched[i][0] != -1
+		   && curr_static_id->operand[i].type == OP_OUT
+		   && (curr_static_id->operand_alternative
+		   [goal_alt_number * n_operands + i].earlyclobber))
+	{
+	  /* Generate reloads for output and matched inputs.  This
+		 is the easiest way to avoid creation of non-existing
+		 conflicts in lra-lives.c.  */
+	  match_reload (i, goal_alt_matched[i], outputs, goal_alt[i], &before,
+			&after, TRUE);
+	  outputs[n_outputs++] = i;
+	  outputs[n_outputs] = -1;
+	  continue;
+	}
 	  else
 	continue;
 	}
Index: testsuite/ChangeLog
===
--- testsuite/ChangeLog	(revision 269876)
+++ testsuite/ChangeLog	(working copy)
@@ -1,3 +1,8 @@
+2019-03-22  Vladimir Makarov  
+
+	PR rtl-optimization/89676
+	* gcc.target/i386/pr89676.c: New.
+
 2019-03-22  Jakub Jelinek  
 
 	PR c++/60702
Index: testsuite/gcc.target/i386/pr89676.c
===
--- testsuite/gcc.target/i386/pr89676.c	(nonexistent)
+++ testsuite/gcc.target/i386/pr89676.c	(working copy)
@@ -0,0 +1,10 @@
+/* PR rtl-optimization/89676 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -m32 -march=i686" } */
+unsigned long long
+foo (unsigned long long i)
+{
+  return i << 3;
+}
+
+/* { dg-final { scan-assembler-times "movl" 2 } } */

RFA: Patch to fix severe recursion in d_count_templates_scopes (PR 89394)

[Nick Clifton]

Hi Ian,

  Attached is a proposed patch to fix PR 89394, which contains an
  artificial mangled name that triggers excessive recursion in
  d_count_templates_scopes.  The patch uses the same recursion limit
  that is already in place for d_print_comp, which I hope will be
  acceptable.

  There is one frag in the patch which is not directly related to this
  recursion problem however.  It extends the check in
  cplus_demangle_fill_name so that names with a negative length are
  rejected.  I had originally thought that the excessive recursion was
  due to a negative length string, although further investigation proved
  this guess to be wrong.  I felt that leaving the check in however
  would still be a good idea.

  Tested with no regressions with an x86_64-linux-gnu toolchain, as well
  as against the testcase in PR 89394.

  OK to apply ?

Cheers
  Nick

libiberty/ChangeLog
2019-03-21  Nick Clifton  

PR 89394
* cp-demangle.c (cplus_demangle_fill_name): Reject negative
lengths.
(d_count_templates_scopes): Replace num_templates and num_scopes
parameters with a struct d_print_info pointer parameter.  Adjust
body of the function accordingly.  Add recursion counter and check
that the recursion limit is not reached.
(d_print_init): Pass dpi parameter to d_count_templates_scopes.
Reset recursion counter afterwards, unless the recursion limit was
reached.

Index: libiberty/cp-demangle.c
===
--- libiberty/cp-demangle.c	(revision 269832)
+++ libiberty/cp-demangle.c	(working copy)
@@ -861,7 +861,7 @@
 int
 cplus_demangle_fill_name (struct demangle_component *p, const char *s, int len)
 {
-  if (p == NULL || s == NULL || len == 0)
+  if (p == NULL || s == NULL || len <= 0)
 return 0;
   p->d_printing = 0;
   p->type = DEMANGLE_COMPONENT_NAME;
@@ -4061,7 +4061,7 @@
are larger than the actual numbers encountered.  */
 
 static void
-d_count_templates_scopes (int *num_templates, int *num_scopes,
+d_count_templates_scopes (struct d_print_info *dpi,
 			  const struct demangle_component *dc)
 {
   if (dc == NULL)
@@ -4081,13 +4081,13 @@
   break;
 
 case DEMANGLE_COMPONENT_TEMPLATE:
-  (*num_templates)++;
+  dpi->num_copy_templates++;
   goto recurse_left_right;
 
 case DEMANGLE_COMPONENT_REFERENCE:
 case DEMANGLE_COMPONENT_RVALUE_REFERENCE:
   if (d_left (dc)->type == DEMANGLE_COMPONENT_TEMPLATE_PARAM)
-	(*num_scopes)++;
+	dpi->num_saved_scopes++;
   goto recurse_left_right;
 
 case DEMANGLE_COMPONENT_QUAL_NAME:
@@ -4152,42 +4152,42 @@
 case DEMANGLE_COMPONENT_TAGGED_NAME:
 case DEMANGLE_COMPONENT_CLONE:
 recurse_left_right:
-  d_count_templates_scopes (num_templates, num_scopes,
-d_left (dc));
-  d_count_templates_scopes (num_templates, num_scopes,
-d_right (dc));
+  /* PR 89394 - Check for too much recursion.  */
+  if (dpi->recursion > DEMANGLE_RECURSION_LIMIT)
+	/* FIXME: There ought to be a way to report to the
+	   user that the recursion limit has been reached.  */
+	return;
+
+  ++ dpi->recursion;
+  d_count_templates_scopes (dpi, d_left (dc));
+  d_count_templates_scopes (dpi, d_right (dc));
+  -- dpi->recursion;
   break;
 
 case DEMANGLE_COMPONENT_CTOR:
-  d_count_templates_scopes (num_templates, num_scopes,
-dc->u.s_ctor.name);
+  d_count_templates_scopes (dpi, dc->u.s_ctor.name);
   break;
 
 case DEMANGLE_COMPONENT_DTOR:
-  d_count_templates_scopes (num_templates, num_scopes,
-dc->u.s_dtor.name);
+  d_count_templates_scopes (dpi, dc->u.s_dtor.name);
   break;
 
 case DEMANGLE_COMPONENT_EXTENDED_OPERATOR:
-  d_count_templates_scopes (num_templates, num_scopes,
-dc->u.s_extended_operator.name);
+  d_count_templates_scopes (dpi, dc->u.s_extended_operator.name);
   break;
 
 case DEMANGLE_COMPONENT_FIXED_TYPE:
-  d_count_templates_scopes (num_templates, num_scopes,
-dc->u.s_fixed.length);
+  d_count_templates_scopes (dpi, dc->u.s_fixed.length);
   break;
 
 case DEMANGLE_COMPONENT_GLOBAL_CONSTRUCTORS:
 case DEMANGLE_COMPONENT_GLOBAL_DESTRUCTORS:
-  d_count_templates_scopes (num_templates, num_scopes,
-d_left (dc));
+  d_count_templates_scopes (dpi, d_left (dc));
   break;
 
 case DEMANGLE_COMPONENT_LAMBDA:
 case DEMANGLE_COMPONENT_DEFAULT_ARG:
-  d_count_templates_scopes (num_templates, num_scopes,
-dc->u.s_unary_num.sub);
+  d_count_templates_scopes (dpi, dc->u.s_unary_num.sub);
   break;
 }
 }
@@ -4222,8 +4222,12 @@
   dpi->next_copy_template = 0;
   dpi->num_copy_templates = 0;
 
-  d_count_templates_scopes (&dpi->num_copy_templates,
-			&dpi->num_saved_scopes, dc);
+  d_count_templates_scopes (dpi, dc);
+  /* If we did 

Re: patch to fix PR85860

[Uros Bizjak]

Hello!

> The following patch fixes
>
>  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85860
>
> The patch was successfully bootstrapped and tested on x86-64.
>
> Committed as rev. 269662 to trunk and as rev. 269663 to gcc-8-branch.

Index: testsuite/gcc.target/i386/pr85860.c
===
--- testsuite/gcc.target/i386/pr85860.c (nonexistent)
+++ testsuite/gcc.target/i386/pr85860.c (working copy)
@@ -0,0 +1,23 @@
+/* { dg-do compile { target lp64 } } */

{ target int128 }

+/* { dg-options "-O2 -fno-guess-branch-probability
-flive-range-shrinkage -mbmi2" } */

Uros.

patch to fix PR85860

[Vladimir Makarov]

The following patch fixes

  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85860

The patch was successfully bootstrapped and tested on x86-64.

Committed as rev. 269662 to trunk and as rev. 269663 to gcc-8-branch.


Index: ChangeLog
===
--- ChangeLog	(revision 269661)
+++ ChangeLog	(working copy)
@@ -1,3 +1,9 @@
+2019-03-13  Vladimir Makarov  
+
+	PR target/85860
+	* lra-constraints.c (inherit_in_ebb): Update
+	potential_reload_hard_regs along with live_hard_regs.
+
 2019-03-13  Jakub Jelinek  
 
 	PR debug/89498
Index: lra-constraints.c
===
--- lra-constraints.c	(revision 269651)
+++ lra-constraints.c	(working copy)
@@ -6365,6 +6365,7 @@ inherit_in_ebb (rtx_insn *head, rtx_insn
 			add_to_hard_reg_set (&s, PSEUDO_REGNO_MODE (dst_regno),
 	 reg_renumber[dst_regno]);
 		  AND_COMPL_HARD_REG_SET (live_hard_regs, s);
+		  AND_COMPL_HARD_REG_SET (potential_reload_hard_regs, s);
 		}
 		  /* We should invalidate potential inheritance or
 		 splitting for the current insn usages to the next
Index: testsuite/ChangeLog
===
--- testsuite/ChangeLog	(revision 269661)
+++ testsuite/ChangeLog	(working copy)
@@ -1,3 +1,8 @@
+2019-03-13  Vladimir Makarov  
+
+	PR target/85860
+	* gcc.target/i386/pr85860.c: New.
+
 2019-03-13  Marek Polacek  
 
 	PR c++/89686 - mixing init-capture and simple-capture in lambda.
Index: testsuite/gcc.target/i386/pr85860.c
===
--- testsuite/gcc.target/i386/pr85860.c	(nonexistent)
+++ testsuite/gcc.target/i386/pr85860.c	(working copy)
@@ -0,0 +1,23 @@
+/* { dg-do compile { target lp64 } } */
+/* { dg-options "-O2 -fno-guess-branch-probability -flive-range-shrinkage -mbmi2" } */
+
+int a, b, c, d, e;
+
+extern int bar(void);
+
+__int128
+foo (unsigned g, int h, long i, __int128 j, short k, __int128 l)
+{
+  unsigned __int128 m = j;
+  do
+{
+  j %= 5;
+  c = c >> (m & 31);
+  e = __builtin_sub_overflow (b, 0, &m);
+  d = bar ();
+  l *= __builtin_mul_overflow_p ((unsigned) d, ~(unsigned __int128) 1,
+ (unsigned __int128) 0);
+}
+  while (a);
+  return m + j + k + l;
+}

Re: C++ PATCH to fix eb82.C

[Jason Merrill]

On 2/17/19 11:54 AM, Marek Polacek wrote:

On Sat, Feb 16, 2019 at 03:54:21PM -0500, Marek Polacek wrote:

I noticed this test fails in c++2a since the implementation of P0846
landed in r265734.  Since it's in g++.old-deja/, I never noticted the
fail (but I don't see any others).  This patch tweaks a dg-error in
order to make it pass in c++2a also.

Tested on x86_64-linux, ok for trunk?

2019-02-16  Marek Polacek  

* g++.old-deja/g++.robertl/eb82.C: Tweak dg-error.

diff --git gcc/testsuite/g++.old-deja/g++.robertl/eb82.C 
gcc/testsuite/g++.old-deja/g++.robertl/eb82.C
index 9bf0398cd0a..fc2bf7866fe 100644
--- gcc/testsuite/g++.old-deja/g++.robertl/eb82.C
+++ gcc/testsuite/g++.old-deja/g++.robertl/eb82.C
@@ -9,5 +9,5 @@ double val  () // { dg-error "" } bogus code
  
  int main ()

  {
-   printf ("%d\n", val<(int)3> ()); // { dg-error "" } val undeclared
+   printf ("%d\n", val<(int)3> ()); // { dg-error "" "" { target c++17_down } 
} val undeclared
  }


Actually I'll just go ahead with this, should be obvious anyway.


I had also noticed this test failing, and when investigating noticed 
that the remaining error strangely talked about a partial 
specialization.  This patch fixes that:



commit 848fa7b9ab2a55d4d3bbf791c828fc3ce60d61fa
Author: Jason Merrill 
Date:   Mon Feb 18 10:05:31 2019 -1000

Improve diagnostic for redundant template arguments in declaration.

* pt.c (check_explicit_specialization): If the declarator is a
template-id, only check whether the arguments are dependent.

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 48cbf3d9892..d8be92ddca4 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -2849,7 +2849,7 @@ check_explicit_specialization (tree declarator,
 	  /* This case handles bogus declarations like template <>
 	 template  void f(); */
 
-	  if (!uses_template_parms (declarator))
+	  if (!uses_template_parms (TREE_OPERAND (declarator, 1)))
 	error ("template-id %qD in declaration of primary template",
 		   declarator);
 	  else if (variable_template_p (TREE_OPERAND (declarator, 0)))
diff --git a/gcc/testsuite/g++.old-deja/g++.robertl/eb82.C b/gcc/testsuite/g++.old-deja/g++.robertl/eb82.C
index fc2bf7866fe..d4c5985cd8c 100644
--- a/gcc/testsuite/g++.old-deja/g++.robertl/eb82.C
+++ b/gcc/testsuite/g++.old-deja/g++.robertl/eb82.C
@@ -2,7 +2,8 @@
 #include 
 
 template 
-double val  () // { dg-error "" } bogus code
+double val  () // { dg-error "expected" "" { target c++17_down } } bogus code
+// { dg-error "template-id .val. in declaration of primary template" "" { target c++2a } .-1 }
 {  
return (double) n1;
 }

Re: C++ PATCH to fix eb82.C

[Marek Polacek]

On Sat, Feb 16, 2019 at 03:54:21PM -0500, Marek Polacek wrote:
> I noticed this test fails in c++2a since the implementation of P0846
> landed in r265734.  Since it's in g++.old-deja/, I never noticted the
> fail (but I don't see any others).  This patch tweaks a dg-error in
> order to make it pass in c++2a also.
> 
> Tested on x86_64-linux, ok for trunk?
> 
> 2019-02-16  Marek Polacek  
> 
>   * g++.old-deja/g++.robertl/eb82.C: Tweak dg-error.
> 
> diff --git gcc/testsuite/g++.old-deja/g++.robertl/eb82.C 
> gcc/testsuite/g++.old-deja/g++.robertl/eb82.C
> index 9bf0398cd0a..fc2bf7866fe 100644
> --- gcc/testsuite/g++.old-deja/g++.robertl/eb82.C
> +++ gcc/testsuite/g++.old-deja/g++.robertl/eb82.C
> @@ -9,5 +9,5 @@ double val  () // { dg-error "" } bogus code
>  
>  int main ()
>  {
> -   printf ("%d\n", val<(int)3> ()); // { dg-error "" } val undeclared
> +   printf ("%d\n", val<(int)3> ()); // { dg-error "" "" { target c++17_down 
> } } val undeclared
>  }

Actually I'll just go ahead with this, should be obvious anyway.

Marek

C++ PATCH to fix eb82.C

[Marek Polacek]

I noticed this test fails in c++2a since the implementation of P0846
landed in r265734.  Since it's in g++.old-deja/, I never noticted the
fail (but I don't see any others).  This patch tweaks a dg-error in
order to make it pass in c++2a also.

Tested on x86_64-linux, ok for trunk?

2019-02-16  Marek Polacek  

* g++.old-deja/g++.robertl/eb82.C: Tweak dg-error.

diff --git gcc/testsuite/g++.old-deja/g++.robertl/eb82.C 
gcc/testsuite/g++.old-deja/g++.robertl/eb82.C
index 9bf0398cd0a..fc2bf7866fe 100644
--- gcc/testsuite/g++.old-deja/g++.robertl/eb82.C
+++ gcc/testsuite/g++.old-deja/g++.robertl/eb82.C
@@ -9,5 +9,5 @@ double val  () // { dg-error "" } bogus code
 
 int main ()
 {
-   printf ("%d\n", val<(int)3> ()); // { dg-error "" } val undeclared
+   printf ("%d\n", val<(int)3> ()); // { dg-error "" "" { target c++17_down } 
} val undeclared
 }

patch to fix PR88560

[Vladimir Makarov]

  The following patch fixes

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88560

  The patch was bootstrapped and tested on x86-64 and ppc64.  It was 
also tested on ARM by Tamar Christina.  The patch changes expected 
generated code for one test on ppc64 but in a better way.  I'll send a 
patch fixing the test later.


Committed as rev. 268705


Index: ChangeLog
===
--- ChangeLog	(revision 268704)
+++ ChangeLog	(working copy)
@@ -1,3 +1,9 @@
+2019-02-08  Vladimir Makarov  
+
+	PR middle-end/88560
+	* lra-constraints.c (process_alt_operands): Don't increase reject
+	for memory when offset memory is required.
+
 2019-02-08  Robin Dapp  
 
 	* config/s390/vector.md: Implement vector copysign.
Index: lra-constraints.c
===
--- lra-constraints.c	(revision 268704)
+++ lra-constraints.c	(working copy)
@@ -2796,29 +2796,32 @@ process_alt_operands (int only_alternati
 			  (GET_MODE (op), this_alternative, cl)
 		losers++;
 
-	  /* Input reloads can be inherited more often than output
-		 reloads can be removed, so penalize output
-		 reloads.  */
-	  if (!REG_P (op) || curr_static_id->operand[nop].type != OP_IN)
-		{
-		  if (lra_dump_file != NULL)
-		fprintf
-		  (lra_dump_file,
-		   "%d Non input pseudo reload: reject++\n",
-		   nop);
-		  reject++;
-		}
-
 	  if (MEM_P (op) && offmemok)
 		addr_losers++;
-	  else if (curr_static_id->operand[nop].type == OP_INOUT)
+	  else
 		{
-		  if (lra_dump_file != NULL)
-		fprintf
-		  (lra_dump_file,
-		   "%d Input/Output reload: reject+=%d\n",
-		   nop, LRA_LOSER_COST_FACTOR);
-		  reject += LRA_LOSER_COST_FACTOR;
+		  /* Input reloads can be inherited more often than
+		 output reloads can be removed, so penalize output
+		 reloads.  */
+		  if (!REG_P (op) || curr_static_id->operand[nop].type != OP_IN)
+		{
+		  if (lra_dump_file != NULL)
+			fprintf
+			  (lra_dump_file,
+			   "%d Non input pseudo reload: reject++\n",
+			   nop);
+		  reject++;
+		}
+
+		  if (curr_static_id->operand[nop].type == OP_INOUT)
+		{
+		  if (lra_dump_file != NULL)
+			fprintf
+			  (lra_dump_file,
+			   "%d Input/Output reload: reject+=%d\n",
+			   nop, LRA_LOSER_COST_FACTOR);
+		  reject += LRA_LOSER_COST_FACTOR;
+		}
 		}
 	}
 

patch to fix PR89225

[Vladimir Makarov]

  The following patch fixes

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89225

  The patch was bootstrapped and tested on x86-64.

  Committed as rev. 268597.

Index: ChangeLog
===
--- ChangeLog	(revision 268596)
+++ ChangeLog	(working copy)
@@ -1,3 +1,9 @@
+2019-02-06  Vladimir Makarov  
+
+	PR rtl-optimization/89225
+	* lra-constaints.c (simplify_operand_subreg): Add subreg mode
+	sizes check.
+
 2019-02-06  Eric Botcazou  
 
 	* config/i386/i386.c (ix86_expand_prologue): Emit a memory blockage
Index: lra-constraints.c
===
--- lra-constraints.c	(revision 268424)
+++ lra-constraints.c	(working copy)
@@ -1533,9 +1533,12 @@ simplify_operand_subreg (int nop, machin
 	 a word.
 
 	 If valid memory becomes invalid after subreg elimination
-	 we still have to reload memory.
+	 and address might be different we still have to reload
+	 memory.
 	  */
-	  if ((! addr_was_valid || addr_is_valid)
+	  if ((! addr_was_valid
+	   || addr_is_valid
+	   || known_eq (GET_MODE_SIZE (mode), GET_MODE_SIZE (innermode)))
 	  && !(maybe_ne (GET_MODE_PRECISION (mode),
 			 GET_MODE_PRECISION (innermode))
 		   && known_le (GET_MODE_SIZE (mode), UNITS_PER_WORD)
Index: testsuite/ChangeLog
===
--- testsuite/ChangeLog	(revision 268596)
+++ testsuite/ChangeLog	(working copy)
@@ -1,3 +1,8 @@
+2019-02-06  Vladimir Makarov  
+
+	PR rtl-optimization/89225
+	* gcc.target/powerpc/pr89225.c: New.
+
 2019-02-06  Eric Botcazou  
 
 	* gnat.dg/opt76.adb: New test.
Index: testsuite/gcc.target/powerpc/pr89225.c
===
--- testsuite/gcc.target/powerpc/pr89225.c	(nonexistent)
+++ testsuite/gcc.target/powerpc/pr89225.c	(working copy)
@@ -0,0 +1,73 @@
+/* { dg-do compile  { target { powerpc*-*-* && lp64 } } } */
+/* { dg-options "-O2 -fstack-protector-strong -mlong-double-128" } */
+
+extern long double foo (long double);
+extern double bar (double);
+typedef long long int64_t;
+typedef unsigned long long uint64_t;
+typedef union { int64_t i[2]; long double x; double d[2]; } mynumber;
+static const double t512 = 0x1p512, tm256 = 0x1p-256, two54 = 0x1p54, twom54 = 0x1p-54;
+
+long double
+foo (long double x)
+{
+  static const long double big = 134217728.0, big1 = 134217729.0;
+  long double t, s, i;
+  mynumber a, c;
+  uint64_t k, l;
+  int64_t m, n;
+  double d;
+
+  a.x = x;
+  k = a.i[0] & 0x7fffL;
+
+  if (k > 0x000fL && k < 0x7ff0L)
+{
+  if (x < 0)
+	return (big1 - big1) / (big - big);
+  l = (k & 0x001fL) | 0x3fe0L;
+  if ((a.i[1] & 0x7fffL) != 0)
+	{
+	  n = (int64_t) ((l - k) * 2) >> 53;
+	  m = (a.i[1] >> 52) & 0x7ff;
+	  if (m == 0)
+	{
+	  a.d[1] *= two54;
+	  m = ((a.i[1] >> 52) & 0x7ff) - 54;
+	}
+	  m += n;
+	  if (m > 0)
+	a.i[1] = (a.i[1] & 0x800fL) | (m << 52);
+	  else if (m <= -54)
+	{
+	  a.i[1] &= 0x8000L;
+	}
+	  else
+	{
+	  m += 54;
+	  a.i[1] = (a.i[1] & 0x800fL) | (m << 52);
+	  a.d[1] *= twom54;
+	}
+	}
+  a.i[0] = l;
+  s = a.x;
+  d = bar (a.d[0]);
+  c.i[0] = 0x2000L + ((k & 0x7fe0L) >> 1);
+  c.i[1] = 0;
+  i = d;
+  t = 0.5L * (i + s / i);
+  i = 0.5L * (t + s / t);
+  return c.x * i;
+}
+  else
+{
+  if (k >= 0x7ff0L)
+
+	return x * x + x;
+  if (x == 0)
+	return x;
+  if (x < 0)
+	return (big1 - big1) / (big - big);
+  return tm256 * foo (x * t512);
+}
+}

patch to fix PR87246

[Vladimir Makarov]

  The following patch fixes

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87246

  The patch was successfully bootstrapped and tested on x86-64 and ppc64.

  Committed as rev. 268404

Index: ChangeLog
===
--- ChangeLog	(revision 268403)
+++ ChangeLog	(working copy)
@@ -1,3 +1,9 @@
+2019-01-30  Vladimir Makarov  
+
+	PR rtl-optimization/87246
+	* lra-constraints.c (simplify_operand_subreg): Reload memory
+	in subreg if the address became invalid.
+
 2019-01-30  Bill Schmidt  
 
 	PR target/87064
Index: lra-constraints.c
===
--- lra-constraints.c	(revision 268117)
+++ lra-constraints.c	(working copy)
@@ -1497,10 +1497,11 @@ simplify_operand_subreg (int nop, machin
   alter_subreg (curr_id->operand_loc[nop], false);
   rtx subst = *curr_id->operand_loc[nop];
   lra_assert (MEM_P (subst));
-
+  const bool addr_is_valid = valid_address_p (GET_MODE (subst),
+		  XEXP (subst, 0),
+		  MEM_ADDR_SPACE (subst));
   if (!addr_was_valid
-	  || valid_address_p (GET_MODE (subst), XEXP (subst, 0),
-			  MEM_ADDR_SPACE (subst))
+	  || addr_is_valid
 	  || ((get_constraint_type (lookup_constraint
 (curr_static_id->operand[nop].constraint))
 	   != CT_SPECIAL_MEMORY)
@@ -1529,12 +1530,17 @@ simplify_operand_subreg (int nop, machin
 	 data into a register when the inner is narrower than outer or
 	 missing important data from memory when the inner is wider than
 	 outer.  This rule only applies to modes that are no wider than
-	 a word.  */
-	  if (!(maybe_ne (GET_MODE_PRECISION (mode),
-			  GET_MODE_PRECISION (innermode))
-		&& known_le (GET_MODE_SIZE (mode), UNITS_PER_WORD)
-		&& known_le (GET_MODE_SIZE (innermode), UNITS_PER_WORD)
-		&& WORD_REGISTER_OPERATIONS)
+	 a word.
+
+	 If valid memory becomes invalid after subreg elimination
+	 we still have to reload memory.
+	  */
+	  if ((! addr_was_valid || addr_is_valid)
+	  && !(maybe_ne (GET_MODE_PRECISION (mode),
+			 GET_MODE_PRECISION (innermode))
+		   && known_le (GET_MODE_SIZE (mode), UNITS_PER_WORD)
+		   && known_le (GET_MODE_SIZE (innermode), UNITS_PER_WORD)
+		   && WORD_REGISTER_OPERATIONS)
 	  && (!(MEM_ALIGN (subst) < GET_MODE_ALIGNMENT (mode)
 		&& targetm.slow_unaligned_access (mode, MEM_ALIGN (subst)))
 		  || (MEM_ALIGN (reg) < GET_MODE_ALIGNMENT (innermode)
@@ -1553,7 +1559,7 @@ simplify_operand_subreg (int nop, machin
 	  enum reg_class rclass
 	= (enum reg_class) targetm.preferred_reload_class (reg, ALL_REGS);
 	  if (get_reload_reg (curr_static_id->operand[nop].type, innermode,
-			  reg, rclass, TRUE, "slow mem", &new_reg))
+			  reg, rclass, TRUE, "slow/invalid mem", &new_reg))
 	{
 	  bool insert_before, insert_after;
 	  bitmap_set_bit (&lra_subreg_reload_pseudos, REGNO (new_reg));
@@ -1572,7 +1578,7 @@ simplify_operand_subreg (int nop, machin
 	  rclass
 	= (enum reg_class) targetm.preferred_reload_class (reg, ALL_REGS);
 	  if (get_reload_reg (curr_static_id->operand[nop].type, mode, reg,
-			  rclass, TRUE, "slow mem", &new_reg))
+			  rclass, TRUE, "slow/invalid mem", &new_reg))
 	{
 	  bool insert_before, insert_after;
 	  bitmap_set_bit (&lra_subreg_reload_pseudos, REGNO (new_reg));
@@ -1585,7 +1591,7 @@ simplify_operand_subreg (int nop, machin
 	}
 	  *curr_id->operand_loc[nop] = new_reg;
 	  lra_process_new_insns (curr_insn, before, after,
- "Inserting slow mem reload");
+ "Inserting slow/invalid mem reload");
 	  return true;
 	}
 
Index: testsuite/ChangeLog
===
--- testsuite/ChangeLog	(revision 268403)
+++ testsuite/ChangeLog	(working copy)
@@ -1,3 +1,8 @@
+2019-01-30  Vladimir Makarov  
+
+	PR rtl-optimization/87246
+	* gcc.target/i386/pr87246.c: New.
+
 2019-01-30  Marek Polacek  
 
 	PR c++/89119 - ICE with value-initialization in template.
@@ -15,7 +20,7 @@
 	* gcc.target/powerpc/vec-extract-uint128-1.c: New test.
 	* gcc.target/powerpc/vec-extract-ulong-1.c: New test.
 	* gcc.target/powerpc/vec-extract-ushort-1.c: New test.
-	
+
 2019-01-30  Richard Biener  
 
 	PR tree-optimization/89111
Index: testsuite/gcc.target/i386/pr87246.c
===
--- testsuite/gcc.target/i386/pr87246.c	(nonexistent)
+++ testsuite/gcc.target/i386/pr87246.c	(working copy)
@@ -0,0 +1,22 @@
+/* PR rtl-optimization/87246 */
+/* { dg-do compile { target int128 } } */
+/* { dg-options "-O2 -w -fnon-call-exceptions -fno-split-wide-types" } */
+
+__int128 zd;
+int c1;
+
+void
+s2 (__int128 *qv)
+{
+  if (*qv != 0)
+{
+  zd = 0;
+  c1 = c1 <= *qv;
+}
+}
+
+void
+lt (unsigned int vb)
+{
+  s2 (vb + 1);
+}