This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH]: Clean up driver processing for IMA

Okay, here's the updated patch and Changelog. Since I didn't receive much feedback, I decided to go
with option 1 below.

Is this okay to commit to 3.5?

-- Caroline Tice
ctice@apple.com

2004-03-22 Caroline Tice <ctice@apple.com>

* gcc.c (combine_flag): New global variable, for new driver option.
(struct compiler): Add two new fields, to be used when
combining multiple input files in a single pass (IMA).
(default_compilers): Add values for the new fields to all
compiler entries. Modify the "@c" compiler entry for doing IMA
properly with "-save-temps" and the "combine" flag.
(option_map): Add new driver option, "--combine", to tell driver
to pass multiple input files to compiler at one time.
(have_o_argbuf_index): New global variable.
(store_arg): Modify to assign value to have_o_argbuf_index.
(struct infile): Add three new fields, to help with IMA.
(display_help): Add help for new "combine" option.
(process_command): Remove local variable have_o; add code to check
for new "combine" option; remove assignment to combine_inputs.
(do_spec_1): Modify to deal with IMA better.
(main): Make variable 'lang_n_infiles' local to entire function
rather than to a single block. Use flag combine_flag to
determine whether to do IMA or not; Modify loop initializing
infiles to deal properly with linker files.
Add code for doing preprocessing in presence of
IMA with "-save-temps" flag. Modify "main" loop to handle
multiple input files, in multiple languages, with or without
preprocessing, gracefully.
* toplev.c (set_src_pwd): Modify to not complain if attempting to
re-set it to same directory it's previously been set to (avoid
irritating, meaningless warning messages when doing IMA with
save-temps).
* doc/inboke.texi: Add "-combine" to list of Overall Options;
remove documentation about IMA that is no longer accurate; Add
documentation explaining what "-combine" does.
* ada/lang-specs.h: Add initialization values for new fields in
"struct compiler".
* cp/lang-specs.h: Likewise.
* f/lang-specs.h: Likewise.
* java/lang-specs.h: Likewise.
* objc/lang-specs.h: Likewise.
* treelang/lang-specs.h: Likewise.

Attachment: gcc5-imi3.txt
Description: Text document



On Mar 23, 2004, at 9:32 AM, Caroline Tice wrote:


I am preparing an updated patch which will contain modified compiler specs for all the various language
lang-specs.h files, as well as documenting the changed driver options. I am also willing to change the driver
options to whatever this community thinks is the most appropriate way for the driver/IMA to work, but I need some
clarification on that. As I see it, there are three viable possibilities (leaving it exactly as it is is *not* okay, in my
opinion, because: it doesn't work as documented; it doesn't work at all with -save-temps'; you are forced to invoke
the driver at least twice to get an executable out). The options I am considering are:

1). Leave things as they are with the original patch I submitted, i.e. IMA is only turned on if you
pass a "-combine" flag to the driver.

2). Make IMA be turned on all the time by default, and create an explicit flag (-nocombine? -noima?)
to turn it off (there needs to be some way to turn it off, because, as Dale mentioned below, having it
on all the times causes some tests to fail; and sometimes you really want separate .s files for debugging
purposes).

3). Make IMA be turned on all the time, period.

Please let me know (preferably soon) which option seems best. Thank you.

-- Caroline Tice
ctice@apple.com

On Mar 22, 2004, at 5:03 PM, Dale Johannesen wrote:

On Mar 22, 2004, at 4:10 PM, Caroline Tice wrote:
On Mar 22, 2004, at 3:57 PM, Daniel Jacobowitz wrote:
On Mon, Mar 22, 2004 at 03:48:44PM -0800, Caroline Tice wrote:

This patch cleans up the way the gcc driver passes multiple input files
to the compiler. In particular, it
adds a new flag "combine". When this flag is passed to the compiler
driver, the driver attempts to pass
(for example) all the *.c files to the cc1 compiler together, allowing
for IMA . This patch also fixes the
driver so that "-save-temps" works properly with "-combine", and so
that "-combine" can also handle
linker files and source files for multiple languages (for example if
you pass it a combination of c and
c++ files, it will attempt to pass all the c files to cc1 at once, but
will pass the c++ files individually to
cc1plus, then pass the appropriate options to the assembler/linker). I
have currently not modified the c++ or objective-c compiler specs to
cope with multiple source files at once.

I have tested this on an Apple G4 running apple-darwin, and an i386
running Linux. It has bootstrapped
and I am in the middle of running the DejaGnu tests. (I also tested
various combinations of the
"-combine" "-save-temps" "-S" and "-c" flags on a multiple file C
program).

Assuming it passes all the tests is this ok to commit to gcc 3.5?

I didn't review the patch, but I can still tell you that a patch whose
purpose is to change option handling is not OK without matching
documentation updates.

Does this change gcc -o foo foo.c bar.c back to not using IMA?


As far as I can tell, the line above *NEVER* used IMA. IMA was ONLY turned on if you added the "-c" flag as
well as the "-o" flag. I had assumed that the reason for this was that people did not want IMA turned on most
of the time.

No, it's a bug, as Geoff explains here:
http://gcc.gnu.org/ml/gcc-patches/2003-07/msg01458.html
Nobody ever fixed it though, that's why you're doing it!
The entire thread above is probably worth reading. I thought the idea of
a command line switch had more support, but it seems this may not be
necessary for acceptance. (Personally I like it, but Geoff doesn't. and I
should probably mention he's not here this week to explain what he does
think. Note that a correctly working IMA will break a certain amount of existing
code, including several SPECmarks and testsuite programs. [Or catch bugs
that weren't caught before, depending on your viewpoint.] That is why I
think it should be on a switch.)

(And it also never worked with the -save-temps flag). So it seemed reasonable to make this
assumption explicit and add a flag that actually gives direct control for turning on/off IMA. If people would rather
just have it on all the time by default, that would be easy enough to correct.

You are right, I forgot to add documentation for the "combine" option, but I will be happy to do so.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]