On Thu, Oct 1, 2020 at 8:02 PM Andrew MacLeod <amacl...@redhat.com> 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
