Re: [PATCH] Fix build of ppc64 target.
On Fri, Oct 02, 2020 at 10:46:12AM +0200, Aldy Hernandez wrote: > On 10/2/20 2:17 AM, David Edelsohn wrote: > >On Thu, Oct 1, 2020 at 8:02 PM Andrew MacLeod wrote: > >Thanks for investigating. And I definitely am not suggesting that you > >delay the great progress on Ranger to flatten and compact tree-vrp.h > >and ssa.h immediately. > > > >Inclusion of any header file in tree-ssa-propagate.h is new, which > >surprised me because of the GCC strategy for headers. As you and Aldy > >continue to develop Ranger, I wanted to alert you to the fragility of > >the current header design. The rs6000 port is a very effective > >canary! > > Indeed. Actually, I had been using a ppc64le-linux machine as my > primary testing target for quite a while. However, a few weeks ago ppc > bootstrap broke and I switched back to x86-64, because I was too lazy to > investigate why. > > I'll test more regularly on ppc if it's back in working order. It is. We keep a close eye on it; it usually bootstraps (for at least the configs we test) all the time. Especially during stage 1 things do break from time to time, but that isn't unique to Power ;-) There isn't a rule that we can just revert any patches that break bootstrap on primary targets (and I am not suggesting there should be), so breakage now and then is unavoidable. Maybe some CI thing can help make stage 1 a less bumpy road. Segher
Re: [PATCH] Fix build of ppc64 target.
On 10/2/20 2:17 AM, David Edelsohn wrote: On Thu, Oct 1, 2020 at 8:02 PM Andrew MacLeod wrote: Hi, Andrew Thanks for investigating. And I definitely am not suggesting that you delay the great progress on Ranger to flatten and compact tree-vrp.h and ssa.h immediately. Inclusion of any header file in tree-ssa-propagate.h is new, which surprised me because of the GCC strategy for headers. As you and Aldy continue to develop Ranger, I wanted to alert you to the fragility of the current header design. The rs6000 port is a very effective canary! Indeed. Actually, I had been using a ppc64le-linux machine as my primary testing target for quite a while. However, a few weeks ago ppc bootstrap broke and I switched back to x86-64, because I was too lazy to investigate why. I'll test more regularly on ppc if it's back in working order. BTW, thanks for fixing the header glitch Martin. Aldy
Re: [PATCH] Fix build of ppc64 target.
On Thu, Oct 1, 2020 at 8:02 PM Andrew MacLeod wrote: > > On 10/1/20 5:30 PM, David Edelsohn wrote: > >>> * config/rs6000/rs6000-call.c: Include value-range.h. > >>> * config/rs6000/rs6000.c: Likewise. > >> This is okay for trunk, thanks! (It is trivial and obvious as well, so > >> please just commit things like this without prior approval.) > > This patch is not the correct long-term solution, as I explained to > > Martin on IRC. If it is approved as a work-around, it should be > > stated that it is a band-aid. Equally obvious is including > > value-range.h in tree-ssa-propagate.h. > > > > The tree-ssa-propagate.h, value-query.h and value-range.h headers > > currently are in an inconsistent state. > > > > GCC has worked to move towards a "flat" header files model to detangle > > the header dependencies in GCC. Most headers don't include other > > headers. In fact Andrew worked on the header reduction effort. > > > > As part of the recent Ranger infrastructure patches, Aldy included > > value-query.h in tree-ssa-propagate.h, but value-query.h depends on > > the irange type defined in value-range.h. I presume that the other > > uses of tree-ssa-propagate.h that refer to the irange methods also > > include value-range.h from some other dependency. > > > > I don't know which solution Aldy and Andrew prefer. > > tree-ssa-propagate.h could include value-range.h or users of > > tree-ssa-propagate.h that need Ranger could include tree-query.h. Or > > tree-query.h needs to be self-contained and provide the irange type. > > Or tree-query.h and tree-range.h need to be combined. The current > > interdependency of the headers does not seem like a wise choice. > > > > Thanks, David > > > Sorry David, I'm guessing its fixed for the moment. via workaround. > > Its not a problem in any other file because the other 29 files all > include the core "ssa.h" header file, which includes tree-vrp.h, which > is where value-range.h comes from. > > in fact, I see rs6000-call.c has: > > #include "tree-ssa-propagate.h" > #include "tree-vrp.h" > #include "tree-ssanames.h" > > which is out of order. if we swap them to be: > > #include "tree-vrp.h" > #include "tree-ssa-propagate.h" > #include "tree-ssanames.h" > > the it compiles just fine. Thats probably the right fix for the moment. > > I see it includes a number of things that ssa.h brings in, so it could > strategically be changed to: > > diff --git a/gcc/config/rs6000/rs6000-call.c > b/gcc/config/rs6000/rs6000-call.c > index a8b520834c7..1ae4df61af3 100644 > --- a/gcc/config/rs6000/rs6000-call.c > +++ b/gcc/config/rs6000/rs6000-call.c > @@ -57,7 +57,7 @@ > #include "gimplify.h" > #include "gimple-fold.h" > #include "gimple-iterator.h" > -#include "gimple-ssa.h" > +#include "ssa.h" > #include "builtins.h" > #include "tree-vector-builder.h" > #if TARGET_XCOFF > @@ -65,8 +65,6 @@ > #endif > #include "ppc-auxv.h" > #include "tree-ssa-propagate.h" > -#include "tree-vrp.h" > -#include "tree-ssanames.h" > #include "targhooks.h" > #include "opts.h" > > > I think tree-vrp.h really needs to be flattened too. It shouldn't be > including value-range.h, and that should be pushed into ssa.h as well. > I think we'll probably be including value-query.h from the ssa.h header > long term as well... ssa.h should be a prerequisite for any file which > uses tree-ssa-propagate.h... > > The rest of that wont be sorted out for a bit, I'm seeing tree-vrp.h is > included a few places which ssa.h is not.. so there is a bit of > shuffling to do to make the long term solution "work" :-P > > I guess its time to fix any bit-rot and rerun the tools to co-ordinate this. > > Sorry for the hassle. Hi, Andrew Thanks for investigating. And I definitely am not suggesting that you delay the great progress on Ranger to flatten and compact tree-vrp.h and ssa.h immediately. Inclusion of any header file in tree-ssa-propagate.h is new, which surprised me because of the GCC strategy for headers. As you and Aldy continue to develop Ranger, I wanted to alert you to the fragility of the current header design. The rs6000 port is a very effective canary! Thanks, David
Re: [PATCH] Fix build of ppc64 target.
On 10/1/20 5:30 PM, David Edelsohn wrote: * config/rs6000/rs6000-call.c: Include value-range.h. * config/rs6000/rs6000.c: Likewise. This is okay for trunk, thanks! (It is trivial and obvious as well, so please just commit things like this without prior approval.) This patch is not the correct long-term solution, as I explained to Martin on IRC. If it is approved as a work-around, it should be stated that it is a band-aid. Equally obvious is including value-range.h in tree-ssa-propagate.h. The tree-ssa-propagate.h, value-query.h and value-range.h headers currently are in an inconsistent state. GCC has worked to move towards a "flat" header files model to detangle the header dependencies in GCC. Most headers don't include other headers. In fact Andrew worked on the header reduction effort. As part of the recent Ranger infrastructure patches, Aldy included value-query.h in tree-ssa-propagate.h, but value-query.h depends on the irange type defined in value-range.h. I presume that the other uses of tree-ssa-propagate.h that refer to the irange methods also include value-range.h from some other dependency. I don't know which solution Aldy and Andrew prefer. tree-ssa-propagate.h could include value-range.h or users of tree-ssa-propagate.h that need Ranger could include tree-query.h. Or tree-query.h needs to be self-contained and provide the irange type. Or tree-query.h and tree-range.h need to be combined. The current interdependency of the headers does not seem like a wise choice. Thanks, David Sorry David, I'm guessing its fixed for the moment. via workaround. Its not a problem in any other file because the other 29 files all include the core "ssa.h" header file, which includes tree-vrp.h, which is where value-range.h comes from. in fact, I see rs6000-call.c has: #include "tree-ssa-propagate.h" #include "tree-vrp.h" #include "tree-ssanames.h" which is out of order. if we swap them to be: #include "tree-vrp.h" #include "tree-ssa-propagate.h" #include "tree-ssanames.h" the it compiles just fine. Thats probably the right fix for the moment. I see it includes a number of things that ssa.h brings in, so it could strategically be changed to: diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c index a8b520834c7..1ae4df61af3 100644 --- a/gcc/config/rs6000/rs6000-call.c +++ b/gcc/config/rs6000/rs6000-call.c @@ -57,7 +57,7 @@ #include "gimplify.h" #include "gimple-fold.h" #include "gimple-iterator.h" -#include "gimple-ssa.h" +#include "ssa.h" #include "builtins.h" #include "tree-vector-builder.h" #if TARGET_XCOFF @@ -65,8 +65,6 @@ #endif #include "ppc-auxv.h" #include "tree-ssa-propagate.h" -#include "tree-vrp.h" -#include "tree-ssanames.h" #include "targhooks.h" #include "opts.h" I think tree-vrp.h really needs to be flattened too. It shouldn't be including value-range.h, and that should be pushed into ssa.h as well. I think we'll probably be including value-query.h from the ssa.h header long term as well... ssa.h should be a prerequisite for any file which uses tree-ssa-propagate.h... The rest of that wont be sorted out for a bit, I'm seeing tree-vrp.h is included a few places which ssa.h is not.. so there is a bit of shuffling to do to make the long term solution "work" :-P I guess its time to fix any bit-rot and rerun the tools to co-ordinate this. Sorry for the hassle. Andrew
Re: [PATCH] Fix build of ppc64 target.
> > * config/rs6000/rs6000-call.c: Include value-range.h. > > * config/rs6000/rs6000.c: Likewise. > > This is okay for trunk, thanks! (It is trivial and obvious as well, so > please just commit things like this without prior approval.) This patch is not the correct long-term solution, as I explained to Martin on IRC. If it is approved as a work-around, it should be stated that it is a band-aid. Equally obvious is including value-range.h in tree-ssa-propagate.h. The tree-ssa-propagate.h, value-query.h and value-range.h headers currently are in an inconsistent state. GCC has worked to move towards a "flat" header files model to detangle the header dependencies in GCC. Most headers don't include other headers. In fact Andrew worked on the header reduction effort. As part of the recent Ranger infrastructure patches, Aldy included value-query.h in tree-ssa-propagate.h, but value-query.h depends on the irange type defined in value-range.h. I presume that the other uses of tree-ssa-propagate.h that refer to the irange methods also include value-range.h from some other dependency. I don't know which solution Aldy and Andrew prefer. tree-ssa-propagate.h could include value-range.h or users of tree-ssa-propagate.h that need Ranger could include tree-query.h. Or tree-query.h needs to be self-contained and provide the irange type. Or tree-query.h and tree-range.h need to be combined. The current interdependency of the headers does not seem like a wise choice. Thanks, David
Re: [PATCH] Fix build of ppc64 target.
On Thu, Oct 01, 2020 at 08:59:12PM +0200, Martin Liška wrote: > Since a889e06ac68 the following fails. > > In file included from ../../gcc/tree-ssa-propagate.h:25:0, > from ../../gcc/config/rs6000/rs6000.c:78: > ../../gcc/value-query.h:90:31: error: ‘irange’ has not been declared >virtual bool range_of_expr (irange , tree name, gimple * = NULL) = 0; >^~ > ../../gcc/value-query.h:91:31: error: ‘irange’ has not been declared >virtual bool range_on_edge (irange , edge, tree name); >^~ > ../../gcc/value-query.h:92:31: error: ‘irange’ has not been declared >virtual bool range_of_stmt (irange , gimple *, tree name = NULL); >^~ > In file included from ../../gcc/tree-ssa-propagate.h:25:0, > from ../../gcc/config/rs6000/rs6000-call.c:67: > ../../gcc/value-query.h:90:31: error: ‘irange’ has not been declared >virtual bool range_of_expr (irange , tree name, gimple * = NULL) = 0; >^~ > ../../gcc/value-query.h:91:31: error: ‘irange’ has not been declared >virtual bool range_on_edge (irange , edge, tree name); >^~ > ../../gcc/value-query.h:92:31: error: ‘irange’ has not been declared >virtual bool range_of_stmt (irange , gimple *, tree name = NULL); > > Ready for master? This is okay for trunk, thanks! (It is trivial and obvious as well, so please just commit things like this without prior approval.) Segher > * config/rs6000/rs6000-call.c: Include value-range.h. > * config/rs6000/rs6000.c: Likewise.
[PATCH] Fix build of ppc64 target.
Since a889e06ac68 the following fails. In file included from ../../gcc/tree-ssa-propagate.h:25:0, from ../../gcc/config/rs6000/rs6000.c:78: ../../gcc/value-query.h:90:31: error: ‘irange’ has not been declared virtual bool range_of_expr (irange , tree name, gimple * = NULL) = 0; ^~ ../../gcc/value-query.h:91:31: error: ‘irange’ has not been declared virtual bool range_on_edge (irange , edge, tree name); ^~ ../../gcc/value-query.h:92:31: error: ‘irange’ has not been declared virtual bool range_of_stmt (irange , gimple *, tree name = NULL); ^~ In file included from ../../gcc/tree-ssa-propagate.h:25:0, from ../../gcc/config/rs6000/rs6000-call.c:67: ../../gcc/value-query.h:90:31: error: ‘irange’ has not been declared virtual bool range_of_expr (irange , tree name, gimple * = NULL) = 0; ^~ ../../gcc/value-query.h:91:31: error: ‘irange’ has not been declared virtual bool range_on_edge (irange , edge, tree name); ^~ ../../gcc/value-query.h:92:31: error: ‘irange’ has not been declared virtual bool range_of_stmt (irange , gimple *, tree name = NULL); Ready for master? Thanks, Martin gcc/ChangeLog: * config/rs6000/rs6000-call.c: Include value-range.h. * config/rs6000/rs6000.c: Likewise. --- gcc/config/rs6000/rs6000-call.c | 1 + gcc/config/rs6000/rs6000.c | 1 + 2 files changed, 2 insertions(+) diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c index a8b520834c7..d10119bd6bf 100644 --- a/gcc/config/rs6000/rs6000-call.c +++ b/gcc/config/rs6000/rs6000-call.c @@ -64,6 +64,7 @@ #include "xcoffout.h" /* get declarations of xcoff_*_section_name */ #endif #include "ppc-auxv.h" +#include "value-range.h" #include "tree-ssa-propagate.h" #include "tree-vrp.h" #include "tree-ssanames.h" diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index 375fff59928..6a05f84a021 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -75,6 +75,7 @@ #endif #include "case-cfn-macros.h" #include "ppc-auxv.h" +#include "value-range.h" #include "tree-ssa-propagate.h" #include "tree-vrp.h" #include "tree-ssanames.h" -- 2.28.0
