Patchwork Objective C/C++ Compiler Drivers

login
register
mail settings
Submitter IainS
Date Sept. 4, 2010, 12:47 p.m.
Message ID <2B0DD900-561C-44FB-B3BC-E629E10EF0D1@sandoe-acoustics.co.uk>
Download mbox | patch
Permalink /patch/63774/
State New
Headers show

Comments

IainS - Sept. 4, 2010, 12:47 p.m.
Joseph, Phil,

On 4 Sep 2010, at 01:34, Philip Herron wrote:

> On 4 September 2010 01:29, Joseph S. Myers <joseph@codesourcery.com>  
> wrote:
>> On Sat, 4 Sep 2010, Philip Herron wrote:
>>
>>> Then does it make sense for a compiler driver to be generated from
>>> some kind of a config, i agree i've always found the compiler  
>>> drivers
>>> a frustrating thing, since for the most part all will do pretty
>>> similar things, most customizations would be add extra linker  
>>> options
>>> i guess. I am not sure if that means we would have a build system to
>>> generate it from a bash script and add extra options to the
>>> config-lang.in to generate this or something.
>>
>> I am suggesting something much simpler: incremental refactoring of  
>> the
>> existing code, moving shared utility functions to gcc.c, so that in  
>> the
>> end most drivers can call a shared function and pass a C structure  
>> to it.
>> Not anything requiring generator programs.
>>
>> For example, you might adapt the Fortran functions such as  
>> append_arg to
>> allocate space dynamically rather than hardcoding limits on the  
>> size of
>> the new array of options based on the old one, move them to gcc.c  
>> with
>> declarations in gcc.h and make all drivers use them.  That could be  
>> one of
>> the incremental patches.  Subsequent ones might try to identify  
>> what is
>> common about the logic to add language-specific libraries.
>>
>> There is an idea that we don't really want all these drivers at all  
>> - we
>> want "gcc" to link in the right libraries automatically for whatever
>> languages were used for the object files it is asked to link - which
>> certainly requires those libraries to be described by suitable
>> datastructures gcc.c can use.
>>
>> --
>> Joseph S. Myers
>> joseph@codesourcery.com
>>
>
> Ah ok i see what you mean now, thanks by the way. I'll play around
> with that now and see what happens and do it for the objc front-end
> first.

I am not really sure why the drivers don't append the lang-specific  
libs to the outfiles array in
lang_specific_pre_link ()?

I'm also not sure why gcj does it differently and inserts via a  
hijacked libgcc spec?

Assuming that there are Good Reasons for these......


@@ -80,13 +81,28 @@ objc/objc-act.o : objc/objc-act.c \
     $(LANGHOOKS_DEF_H) $(HASHTAB_H) $(C_PRAGMA_H) gt-objc-objc-act.h \
     $(GIMPLE_H) c-lang.h

+objc-spec.o: $(srcdir)/objc/objc-spec.c $(CONFIG_H) $(SYSTEM_H) \
+  coretypes.h $(DIAGNOSTIC_H) $(TREE_H) $(FLAGS_H) toplev.h  
langhooks.h $(TM_H)
+	$(CC) $(GPY_CFLAGS) $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) $ 
(DRIVER_DEFINES) \
+		$(INCLUDES) $(LDFLAGS) -c -o $@ $(srcdir)/objc/objc-spec.c
+
+OBJC_D_OBJS = $(GCC_OBJS) objc-spec.o version.o prefix.o intl.o
+g-objc$(exeext): $(OBJC_D_OBJS) $(EXTRA_GCC_OBJS) $(LIBDEPS)
+	$(LINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ \
+	  $(OBJC_D_OBJS) $(EXTRA_GCC_OBJS) $(LIBS)
+
+# Create a version of the objc driver which calls the cross-compiler.
+objc-cross$(exeext): g-objc$(exeext)
+	-rm -f objc-cross$(exeext)
+	cp g-objc$(exeext) objc-cross$(exeext)
+
  objc.srcextra:

  #
  # Build hooks:

-objc.all.cross:
-objc.start.encap:
+objc.all.cross: objc-cross$(exeext)
+objc.start.encap: g-objc$(exeext)
  objc.rest.encap:
  objc.info:
  objc.install-info:
@@ -96,6 +112,7 @@ objc.install-pdf:
  objc.html:
  objc.install-html:
  objc.man:
+objc.install-man:
  objc.srcinfo:
  objc.srcman:
  objc.install-plugin:
@@ -109,12 +126,18 @@ lang_checks += check-objc
  #
  # Install hooks:
  # cc1obj is installed elsewhere as part of $(COMPILERS).
+# Handled here too with the new objc-spec.c driver.

-objc.install-common:
+objc.install-common: installdirs
+	-rm -f $(DESTDIR)$(bindir)/$(OBJC_INSTALL_NAME)$(exeext)
+	-$(INSTALL_PROGRAM) g-objc$(exeext) $(DESTDIR)$(bindir)/$ 
(OBJC_INSTALL_NAME)$(exeext)
+	-chmod a+x $(DESTDIR)$(bindir)/$(OBJC_INSTALL_NAME)$(exeext)

-objc.install-man:
+objc.install-plugin:

  objc.uninstall:
+	-rm -rf $(DESTDIR)/$(bindir)/$(OBJC_INSTALL_NAME)$(exeext)
+
  #
  # Clean hooks:
  # A lot of the ancillary files are deleted by the main makefile.
@@ -144,3 +167,6 @@ objc.stageprofile: stageprofile-start
  	-mv objc/*$(objext) stageprofile/objc
  objc.stagefeedback: stagefeedback-start
  	-mv objc/*$(objext) stagefeedback/objc
+
+config.status: objc/config-lang.in
+
Joseph S. Myers - Sept. 4, 2010, 3:05 p.m.
On Sat, 4 Sep 2010, IainS wrote:

> What do you think of these as common functionality to be collected up?

1. Functions to append either copied or new arguments to the new array of 
arguments without needing an error-prone calculation of the exact size of 
that array.  Only Fortran has this at present, but it would clearly be 
good for all drivers.

2. Description of how the mapping from file extensions to languages 
differs from the default (for example, g++ compiles .c files as C++).  
Should be able to go in a datastructure.  Consider having some structure 
provided by the drivers that is used by lookup_compiler to override its 
defaults rather than inserting -x options on the command line.

3. Logic for when the language's libraries need to be appended and for 
putting them in the right sequence (see what gfortranspec.c does about 
arranging the sequence -lgfortran -lm).  It might be better for this to go 
not in lang_specific_driver but later, in gcc.c after it's determined the 
language for each input file; that way, lang_specific_driver wouldn't need 
to look at file suffixes and track -x options to work out whether a 
particular input file is a source file for its own language.

4. Related to that, logic identifying which option (OPT_ value) if any 
causes the static version of which library to be linked in, and 
controlling whether shared libgcc is the default for a particular driver 
or not.

5. There may be extra or different libraries for static linking or 
profiling; see what g++spec.c does.

> 1/ determining that the action is PCH
> (and ensuring that no link commands are put into the driver line -- these
> cause collect2 to be invoked which barfs because there is no linking to do).

Yes, I suppose so.

> 3/ consider that the input files might be supplied from a file list and that
> should be considered before quitting with a "no files, not prepared to ... "

@files are expanded as the very first stage in processing argv, so 
lang_specific_driver does not need to consider them at all.
Philip Herron - Sept. 5, 2010, 2:54 a.m.
On 4 September 2010 16:05, Joseph S. Myers <joseph@codesourcery.com> wrote:
> On Sat, 4 Sep 2010, IainS wrote:
>
>> What do you think of these as common functionality to be collected up?
>
> 1. Functions to append either copied or new arguments to the new array of
> arguments without needing an error-prone calculation of the exact size of
> that array.  Only Fortran has this at present, but it would clearly be
> good for all drivers.
>
> 2. Description of how the mapping from file extensions to languages
> differs from the default (for example, g++ compiles .c files as C++).
> Should be able to go in a datastructure.  Consider having some structure
> provided by the drivers that is used by lookup_compiler to override its
> defaults rather than inserting -x options on the command line.
>
> 3. Logic for when the language's libraries need to be appended and for
> putting them in the right sequence (see what gfortranspec.c does about
> arranging the sequence -lgfortran -lm).  It might be better for this to go
> not in lang_specific_driver but later, in gcc.c after it's determined the
> language for each input file; that way, lang_specific_driver wouldn't need
> to look at file suffixes and track -x options to work out whether a
> particular input file is a source file for its own language.
>
> 4. Related to that, logic identifying which option (OPT_ value) if any
> causes the static version of which library to be linked in, and
> controlling whether shared libgcc is the default for a particular driver
> or not.
>
> 5. There may be extra or different libraries for static linking or
> profiling; see what g++spec.c does.
>
>> 1/ determining that the action is PCH
>> (and ensuring that no link commands are put into the driver line -- these
>> cause collect2 to be invoked which barfs because there is no linking to do).
>
> Yes, I suppose so.
>
>> 3/ consider that the input files might be supplied from a file list and that
>> should be considered before quitting with a "no files, not prepared to ... "
>
> @files are expanded as the very first stage in processing argv, so
> lang_specific_driver does not need to consider them at all.
>
> --
> Joseph S. Myers
> joseph@codesourcery.com
>

I played around there and i think i have a good idea of what it could
look like now i'll aim to clean it up and send a patch around tomorrow
night since i would need feedback on what it looks like.

--Phil
Philip Herron - Sept. 8, 2010, 4:20 a.m.
Hey all

I've built up a set of functions in making compiler drivers much
simpler now i would like some feedback on the patch it gives objc a
working compiler driver now and thanks to IainS for your Make-lang.in
to fix my compiler driver name and installing routines.

The routines follow the idea of the Fortran front-end in logic but its
much simpler in adding argument i think, once you do the initial check
it sets up plenty of variables then you tell it what your runtime
library is and if you need the math lib and if its a specific lib or
not. Then you transform the arguments into a new array and once that's
done you are free to do what you will.

I was unsure on where i should be sticking these routines so i just
created a file along side the objc driver (driver-utils.c) and put the
code in there, i had some problems moving things into gcc.c then i got
a little confused in what i was doing since it didn't seem to really
like my extern'd variables when the static lib was create it
complained about missing variable declarations which the driver was
going to declare and i got a bit messed up so i just started clean and
hope some of you guys could shed some light on where things might take
place better and hopefuly some more feedback on this not sure if its
quite what you all had in mind or not... :)

--Phil
Joseph S. Myers - Sept. 8, 2010, 11:06 a.m.
On Wed, 8 Sep 2010, Philip Herron wrote:

> I was unsure on where i should be sticking these routines so i just
> created a file along side the objc driver (driver-utils.c) and put the
> code in there, i had some problems moving things into gcc.c then i got
> a little confused in what i was doing since it didn't seem to really
> like my extern'd variables when the static lib was create it
> complained about missing variable declarations which the driver was
> going to declare and i got a bit messed up so i just started clean and

That is far too vague a description to be able to help at all.  Shared 
code for drivers can go in gcc.c with extern declarations in gcc.h.

> hope some of you guys could shed some light on where things might take
> place better and hopefuly some more feedback on this not sure if its
> quite what you all had in mind or not... :)

I don't want to see a patch that starts by implementing a new driver in 
terms of new functions.  I want to see a patch that creates new functions 
and switches existing drivers to using them, with *no change in the 
behavior of those existing drivers except for changes that you carefully 
explain and justify in the body of the message*.  Also, please keep to the 
GNU Coding Standards from the start; comments and code following standard 
formatting, comments on functions detailing the semantics of all 
parameters and the return type, etc.; otherwise the patch is not 
effectively reviewable.

The "no change in behavior" part means starting small and being very 
careful with each incremental patch, and thinking carefully and clearly 
about exactly what the existing code in each driver does so you can 
understand and explain exactly what is changed as a consequence of moving 
to unified infrastructure.
Philip Herron - Sept. 8, 2010, 2:40 p.m.
On 8 September 2010 12:06, Joseph S. Myers <joseph@codesourcery.com> wrote:
> On Wed, 8 Sep 2010, Philip Herron wrote:
>
>> I was unsure on where i should be sticking these routines so i just
>> created a file along side the objc driver (driver-utils.c) and put the
>> code in there, i had some problems moving things into gcc.c then i got
>> a little confused in what i was doing since it didn't seem to really
>> like my extern'd variables when the static lib was create it
>> complained about missing variable declarations which the driver was
>> going to declare and i got a bit messed up so i just started clean and
>
> That is far too vague a description to be able to help at all.  Shared
> code for drivers can go in gcc.c with extern declarations in gcc.h.
>
>> hope some of you guys could shed some light on where things might take
>> place better and hopefuly some more feedback on this not sure if its
>> quite what you all had in mind or not... :)
>
> I don't want to see a patch that starts by implementing a new driver in
> terms of new functions.  I want to see a patch that creates new functions
> and switches existing drivers to using them, with *no change in the
> behavior of those existing drivers except for changes that you carefully
> explain and justify in the body of the message*.  Also, please keep to the
> GNU Coding Standards from the start; comments and code following standard
> formatting, comments on functions detailing the semantics of all
> parameters and the return type, etc.; otherwise the patch is not
> effectively reviewable.
>
> The "no change in behavior" part means starting small and being very
> careful with each incremental patch, and thinking carefully and clearly
> about exactly what the existing code in each driver does so you can
> understand and explain exactly what is changed as a consequence of moving
> to unified infrastructure.
>

I'm not quite sure does that mean you looked at it or not? And does
that mean you want a patch that starts implementing/changing the
existing compiler drivers to use these new shared functions?

I just put these shared functions in a separate file along side the
objc front-end for a first pass since i would have liked to get some
feedback on the functions them selves since i would be the C/C++
front-end might have more complicated switches etc going on which i
may not handle correctly, but i can look into that now, as well as
move the shared code into gcc.c.

I think i need to fix some things in it anyways i'll try and document
the functions etc with some examples etc along side the next patch +
fix the formatting.
Joseph S. Myers - Sept. 8, 2010, 4:05 p.m.
On Wed, 8 Sep 2010, Philip Herron wrote:

> I'm not quite sure does that mean you looked at it or not? And does

I began looking at it but rapidly found it wasn't in a state that was 
worth my while looking at.

When sending patches to the lists, the patches should be aimed primarily 
at a human reader - what's important is making it clear to people what the 
patch does and why, and the details of the code itself should be 
secondary; readability is the most important attribute of almost all code 
that anyone is ever going to have to read or maintain in future.  I look 
first at:

* The patch write-up in the body of the message.  For changes to 
complicated code with subtle effects on semantics in existing corner 
cases, I often find it takes several hours' analysis before starting work 
on the patch to identify these cases, and a few hours more to write up the 
details of the patch (having kept notes as I went along of issues found 
during implementation); there are plenty of examples in my option handling 
patch series, e.g. 
<http://gcc.gnu.org/ml/gcc-patches/2010-08/msg01399.html>.

* The changes to the documentation.  This sort of thing often may not make 
any changes that would require adjusting user documentation - making the 
write-up increasingly important.

* The new testcases or changes to testcases.  The sort of corner cases 
such a patch affects may be hard to cover in the testsuite - making it 
increasingly important to detail them in the write-up.

* The internal datastructures and interfaces and comments in the patch 
that provide a high-level overview of the design being used.  At the point 
where I saw new functions without their parameters explained, and comments 
not formatted in the standard way, I stopped looking at the patch.

Only after this point - if everything above is satisfactory - do the 
details of the code become relevant.  Code extracts showing the 
datastructures and function prototypes with full detailed comments are 
much more useful than function bodies without explanation.

> that mean you want a patch that starts implementing/changing the
> existing compiler drivers to use these new shared functions?

Exactly.  Whenever you find too many corner cases of behavior might be 
affected, that's a sign that you need smaller incremental patches - 
possibly having a patch that explicitly fixes a corner case first, before 
a patch introducing the use of new infrastructure without changing the 
behavior on that case.  The minimal step might be a new interface for 
appending new or modified options to an array (that automatically resizes) 
- with no changes at all to what options get appended in what conditions, 
just the removal of code in the various drivers that tries to work out 
exactly how many options it will end up adding, in order to allocate 
space, separately from adding those options.

Patch

==== as for factoring:

What do you think of these as common functionality to be collected up?

1/ determining that the action is PCH
(and ensuring that no link commands are put into the driver line --  
these cause collect2 to be invoked which barfs because there is no  
linking to do).
[g++ fails on this as of now]

2/  the repeated sequence of actions for a 'static-xxxx' flag:
NOTE: when encountering a "static-xxxx" flag - please do not remove  
this (or , specifically, ensure that it is replaced when linking) so  
that targets that use spec substitution (i.e. those that do not  
support Bstatic/Bdynamic) for static libs can see it.
[g++ fails on this too, at present, fortran has it right]

3/ consider that the input files might be supplied from a file list  
and that should be considered before quitting with a "no files, not  
prepared to ... "

Phil, your spec file works for me on i686-darwin9 (for NeXT and gnu- 
runtimes) with the modified Makefile fragment below.
cheers,
Iain

Index: gcc/objc/Make-lang.in
===================================================================
--- gcc/objc/Make-lang.in	(revision 163837)
+++ gcc/objc/Make-lang.in	(working copy)
@@ -39,13 +39,14 @@ 
  #
  # Define the names for selecting Objective-C in LANGUAGES.
  objc: cc1obj$(exeext)
+.phony: objc

-# Tell GNU make to ignore these if they exist.
-.PHONY: objc
-
-# Use maximal warnings for this front end.
+# Use strict warnings for this front end.
  objc-warn = $(STRICT_WARN)

+OBJC_INSTALL_NAME := $(shell echo objc|sed '$(program_transform_name)')
+OBJC_TARGET_INSTALL_NAME := $(target_noncanonical)-$(shell echo objc| 
sed '$(program_transform_name)')
+
  # Language-specific object files for Objective C.
  OBJC_OBJS = objc/objc-lang.o objc/objc-act.o