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



Reply via email to