diff mbox series

Makefile.in: Add variable TM_P_H2 for TM_P_H dependency [PR111021]

Message ID e94df26f-39d2-4426-3fd1-f88946f34378@linux.ibm.com
State Unresolved
Headers
Series Makefile.in: Add variable TM_P_H2 for TM_P_H dependency [PR111021] |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Kewen.Lin Aug. 16, 2023, 2:31 a.m. UTC 
  Hi,

As PR111021 shows, the below ${port}-protos.h include tree.h
for code_helper and tree_code:

  arm/arm-protos.h:#include "tree.h"
  cris/cris-protos.h:#include "tree.h"  (H-P removed this in r14-3218)
  microblaze/microblaze-protos.h:#include "tree.h"
  rl78/rl78-protos.h:#include "tree.h"
  stormy16/stormy16-protos.h:#include "tree.h"

, when compiling build/gencondmd.cc, the include hierarchy
makes it depend on tm_p.h -> ${port}-protos.h -> tree.h,
which further includes (depends on) some files that are
generated during the building, such as: all-tree.def,
tree-check.h and so on.  The previous commit r14-3215
should already force build/gencondmd.cc to depend on
${TREE_H}, so the reported build failure should be gone.

But for a long term maintenance, especially one day some
build/xxx.cc requires tm_p.h but not recog.h, the ${TREE_H}
dependence could be missed and a build failure will show
up.  So this patch is to add one variable under section
"# Shorthand variables for dependency lists.", to explicit
indicate tm_p.h which includes ${port}-protos.h should
depend on ${TREE_H}.  Then any new build/xxx.cc depending
on tm_p.h will be able to consider ${TREE_H}.

Note that the existing ${TM_P_H} variable is also used for
"generated_files", it isn't dedicated for dependencies, so
a variable named ${TM_P_H2} is proposed and put under the
"# Shorthand variables for dependency lists.", also the
only use as dependence is updated accordingly.

It's tested with cross-builds for the affected ports with
steps:

  1) dropped the fix r14-3215;
  2) reproduced the build failure with serial build;
  3) applied this patch, serial built and verified all passed;
  4) added back r14-3215, serial built and verified all passed;

Is it ok for trunk?

BR,
Kewen
-----
	PR bootstrap/111021

gcc/ChangeLog:

	* Makefile.in (TM_P_H2): New variable for tm_p.h dependence.
---
 gcc/Makefile.in | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

 # Now figure out from those variables how to compile and link.
@@ -2905,7 +2906,7 @@ build/inchash.o : inchash.cc $(BCONFIG_H) $(SYSTEM_H) $(CORETYPES_H)	\
   $(HASHTAB_H) inchash.h
 build/gencondmd.o : build/gencondmd.cc $(BCONFIG_H) $(SYSTEM_H)		\
   $(CORETYPES_H) $(GTM_H) insn-constants.h				\
-  $(filter-out insn-flags.h, $(RTL_H) $(TM_P_H) $(FUNCTION_H) $(REGS_H) \
+  $(filter-out insn-flags.h, $(RTL_H) $(TM_P_H2) $(FUNCTION_H) $(REGS_H) \
   $(RECOG_H) output.h $(FLAGS_H) $(RESOURCE_H) toplev.h $(DIAGNOSTIC_CORE_H) reload.h 	\
   $(EXCEPT_H) tm-constrs.h)
 # This pulls in tm-pred.h which contains inline functions wrapping up
--
2.39.1

Comments

Kewen.Lin Aug. 16, 2023, 5:43 a.m. UTC | #1 
on 2023/8/16 10:31, Kewen.Lin via Gcc-patches wrote:
> Hi,
> 
> As PR111021 shows, the below ${port}-protos.h include tree.h
> for code_helper and tree_code:
> 
>   arm/arm-protos.h:#include "tree.h"
>   cris/cris-protos.h:#include "tree.h"  (H-P removed this in r14-3218)
>   microblaze/microblaze-protos.h:#include "tree.h"
>   rl78/rl78-protos.h:#include "tree.h"
>   stormy16/stormy16-protos.h:#include "tree.h"
> 
> , when compiling build/gencondmd.cc, the include hierarchy
> makes it depend on tm_p.h -> ${port}-protos.h -> tree.h,
> which further includes (depends on) some files that are
> generated during the building, such as: all-tree.def,
> tree-check.h and so on.  The previous commit r14-3215
> should already force build/gencondmd.cc to depend on
> ${TREE_H}, so the reported build failure should be gone.
> 
> But for a long term maintenance, especially one day some
> build/xxx.cc requires tm_p.h but not recog.h, the ${TREE_H}
> dependence could be missed and a build failure will show
> up.  So this patch is to add one variable under section
> "# Shorthand variables for dependency lists.", to explicit
> indicate tm_p.h which includes ${port}-protos.h should
> depend on ${TREE_H}.  Then any new build/xxx.cc depending
> on tm_p.h will be able to consider ${TREE_H}.
> 
> Note that the existing ${TM_P_H} variable is also used for
> "generated_files", it isn't dedicated for dependencies, so
> a variable named ${TM_P_H2} is proposed and put under the
> "# Shorthand variables for dependency lists.", also the
> only use as dependence is updated accordingly.

I did some more checkings and found that not all files in
$(generated_files) are **generated**, some of them actually
sit in source directory, I misinterpreted it from its name,
I think we can just update the existing ${TM_P_H} instead of
adding a new variable.

I'll post a new patch after some testings, sorry for noise!

BR,
Kewen
diff mbox series

Patch

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 9dddb65b45d..192dc76f294 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1062,6 +1062,7 @@  RTL_SSA_H = $(PRETTY_PRINT_H) insn-config.h splay-tree-utils.h \
 	    rtl-ssa/changes.h rtl-ssa/functions.h rtl-ssa/is-a.inl \
 	    rtl-ssa/access-utils.h rtl-ssa/insn-utils.h rtl-ssa/movement.h \
 	    rtl-ssa/change-utils.h rtl-ssa/member-fns.inl
+TM_P_H2 = $(TM_P_H) $(TREE_H)

 #