diff mbox

Fix missing headers for plugin [was Miss head file diagnostic.h in plugin.h?]

Message ID AANLkTik4ztZmy7u26T1Oom1WWA=RPNi8PWdKi6wwoO-C@mail.gmail.com
State New
Headers show

Commit Message

Mingjie Xing Nov. 30, 2010, 1:43 a.m. UTC 
2010/11/30 Ralf Wildenhues <Ralf.Wildenhues@gmx.de>:
> But then why is hard-reg-set.h not listed in FUNCTION_H?  Generally,
> the *_H make macros in gcc/Makefile.in should correspond to directly
> included headers only (with some set of exceptions that I haven't really
> grokked yet, sorry).
>
> Thanks,
> Ralf

I bet you are right. Putting hard-reg-set.h in FUNCTION_H seems
reasonable.  Here's the updated patch.

Bootstrapped on ia64-redhat-linux and i486-linux-gnu.

2010-11-30  Mingjie Xing  <mingjie.xing@gmail.com>

         * gcc-plugin.h: Include coretypes.h.
         * Makefile.in (FUNCTION_H): Add hard-reg-set.h.

Thanks,
Mingjie

Comments

Ralf Wildenhues Nov. 30, 2010, 5:58 a.m. UTC | #1 
* Mingjie Xing wrote on Tue, Nov 30, 2010 at 02:43:45AM CET:
> 2010/11/30 Ralf Wildenhues:
> > But then why is hard-reg-set.h not listed in FUNCTION_H?  Generally,
> > the *_H make macros in gcc/Makefile.in should correspond to directly
> > included headers only (with some set of exceptions that I haven't really
> > grokked yet, sorry).

> I bet you are right. Putting hard-reg-set.h in FUNCTION_H seems
> reasonable.  Here's the updated patch.
> 
> Bootstrapped on ia64-redhat-linux and i486-linux-gnu.

OK then.

Thanks,
Ralf

> 2010-11-30  Mingjie Xing  <mingjie.xing@gmail.com>
> 
>          * gcc-plugin.h: Include coretypes.h.
>          * Makefile.in (FUNCTION_H): Add hard-reg-set.h.

> --- gcc-plugin.h	(revision 167194)
> +++ gcc-plugin.h	(working copy)
> @@ -26,6 +26,7 @@ along with GCC; see the file COPYING3.  
>  
>  #include "config.h"
>  #include "system.h"
> +#include "coretypes.h"
>  #include "highlev-plugin-common.h"
>  #include "hashtab.h"

> --- Makefile.in	(revision 167194)
> +++ Makefile.in	(working copy)
> @@ -907,7 +907,7 @@ ALIAS_H = alias.h coretypes.h
>  EMIT_RTL_H = emit-rtl.h
>  FLAGS_H = flags.h coretypes.h flag-types.h $(OPTIONS_H)
>  OPTIONS_H = options.h flag-types.h $(OPTIONS_H_EXTRA)
> -FUNCTION_H = function.h $(TREE_H) $(HASHTAB_H) vecprim.h $(TM_H)
> +FUNCTION_H = function.h $(TREE_H) $(HASHTAB_H) vecprim.h $(TM_H) hard-reg-set.h
>  EXPR_H = expr.h insn-config.h $(FUNCTION_H) $(RTL_H) $(FLAGS_H) $(TREE_H) $(MACHMODE_H) $(EMIT_RTL_H)
>  OPTABS_H = optabs.h insn-codes.h
>  REGS_H = regs.h $(MACHMODE_H) hard-reg-set.h
Mingjie Xing Nov. 30, 2010, 6:29 a.m. UTC | #2 
2010/11/30 Ralf Wildenhues <Ralf.Wildenhues@gmx.de>:
> * Mingjie Xing wrote on Tue, Nov 30, 2010 at 02:43:45AM CET:
>> 2010/11/30 Ralf Wildenhues:
>> > But then why is hard-reg-set.h not listed in FUNCTION_H?  Generally,
>> > the *_H make macros in gcc/Makefile.in should correspond to directly
>> > included headers only (with some set of exceptions that I haven't really
>> > grokked yet, sorry).
>
>> I bet you are right. Putting hard-reg-set.h in FUNCTION_H seems
>> reasonable.  Here's the updated patch.
>>
>> Bootstrapped on ia64-redhat-linux and i486-linux-gnu.
>
> OK then.
>
> Thanks,
> Ralf

Hi Ralf, further digging shows that there are many places where
hard-reg-set.h and $(FUNCTION_H) both appear.  For an example,

graph.o: graph.c $(SYSTEM_H) coretypes.h $(TM_H) $(TOPLEV_H)
$(DIAGNOSTIC_CORE_H) $(FLAGS_H) output.h \
    $(RTL_H) $(FUNCTION_H) hard-reg-set.h $(BASIC_BLOCK_H) graph.h
$(OBSTACK_H) \
    $(CONFIG_H) $(EMIT_RTL_H)

Do you think that after put hard-reg-set.h in FUNCTION_H, these will
need more cleanup?

Thanks,
Mingjie
Mingjie Xing Nov. 30, 2010, 7:14 a.m. UTC | #3 
> 2010/11/30 Ralf Wildenhues <Ralf.Wildenhues@gmx.de>:
>> * Mingjie Xing wrote on Tue, Nov 30, 2010 at 02:43:45AM CET:
>>> 2010/11/30 Ralf Wildenhues:
>>> > But then why is hard-reg-set.h not listed in FUNCTION_H?  Generally,
>>> > the *_H make macros in gcc/Makefile.in should correspond to directly
>>> > included headers only (with some set of exceptions that I haven't really
>>> > grokked yet, sorry).
>>
>>> I bet you are right. Putting hard-reg-set.h in FUNCTION_H seems
>>> reasonable.  Here's the updated patch.
>>>
>>> Bootstrapped on ia64-redhat-linux and i486-linux-gnu.
>>
>> OK then.
>>
>> Thanks,
>> Ralf

Committed revision 167290.

Thanks,
Mingjie
Joseph Myers Nov. 30, 2010, 11:50 a.m. UTC | #4 
On Tue, 30 Nov 2010, Mingjie Xing wrote:

> Hi Ralf, further digging shows that there are many places where
> hard-reg-set.h and $(FUNCTION_H) both appear.  For an example,
> 
> graph.o: graph.c $(SYSTEM_H) coretypes.h $(TM_H) $(TOPLEV_H)
> $(DIAGNOSTIC_CORE_H) $(FLAGS_H) output.h \
>     $(RTL_H) $(FUNCTION_H) hard-reg-set.h $(BASIC_BLOCK_H) graph.h
> $(OBSTACK_H) \
>     $(CONFIG_H) $(EMIT_RTL_H)
> 
> Do you think that after put hard-reg-set.h in FUNCTION_H, these will
> need more cleanup?

No.  If a file directly includes hard-reg-set.h then it is correct that 
hard-reg-set.h be listed explicitly in its dependencies, whether or not it 
also indirectly includes hard-reg-set.h.  And if a file uses interfaces 
from hard-reg-set.h it is correct that it includes it explicitly rather 
than relying on another header happening to include it indirectly.

These dependencies don't tend to be kept up to date very reliably, 
however.  Since there is now a GNU make 3.82 release, I wonder if for 4.7 
we could remove the manually maintained dependencies and arrange things so 
that with GNU make 3.82 or later you get automatic dependency generation 
(see Tom's patch that had to be reverted a while back because of bugs in 
older GNU make releases) and with older GNU make you get a safe but stupid 
dependency of all .o files on all headers?
Ralf Wildenhues Dec. 1, 2010, 7 a.m. UTC | #5 
* Joseph S. Myers wrote on Tue, Nov 30, 2010 at 12:50:02PM CET:
> Since there is now a GNU make 3.82 release, I wonder if for 4.7 
> we could remove the manually maintained dependencies and arrange things so 
> that with GNU make 3.82 or later you get automatic dependency generation 
> (see Tom's patch that had to be reverted a while back because of bugs in 
> older GNU make releases) and with older GNU make you get a safe but stupid 
> dependency of all .o files on all headers?

Sounds like an idea worth pursuing.

Thanks,
Ralf
diff mbox

Patch

Index: gcc-plugin.h
===================================================================
--- gcc-plugin.h	(revision 167194)
+++ gcc-plugin.h	(working copy)
@@ -26,6 +26,7 @@  along with GCC; see the file COPYING3.  
 
 #include "config.h"
 #include "system.h"
+#include "coretypes.h"
 #include "highlev-plugin-common.h"
 #include "hashtab.h"
 
Index: Makefile.in
===================================================================
--- Makefile.in	(revision 167194)
+++ Makefile.in	(working copy)
@@ -907,7 +907,7 @@  ALIAS_H = alias.h coretypes.h
 EMIT_RTL_H = emit-rtl.h
 FLAGS_H = flags.h coretypes.h flag-types.h $(OPTIONS_H)
 OPTIONS_H = options.h flag-types.h $(OPTIONS_H_EXTRA)
-FUNCTION_H = function.h $(TREE_H) $(HASHTAB_H) vecprim.h $(TM_H)
+FUNCTION_H = function.h $(TREE_H) $(HASHTAB_H) vecprim.h $(TM_H) hard-reg-set.h
 EXPR_H = expr.h insn-config.h $(FUNCTION_H) $(RTL_H) $(FLAGS_H) $(TREE_H) $(MACHMODE_H) $(EMIT_RTL_H)
 OPTABS_H = optabs.h insn-codes.h
 REGS_H = regs.h $(MACHMODE_H) hard-reg-set.h