Re: [PATCH] Fix build on darwin (was Re: [PATCH] gcc.c: Split up the driver's main into smaller functions)
Hi David, On 3 Oct 2014, at 20:15, David Malcolm wrote: On Fri, 2014-10-03 at 11:02 -0400, David Malcolm wrote: The main function for the driver in gcc.c has grown from ~200 lines in its original form (way back in r262) to ~1000 lines today, with a dozen locals (if we include the params). The following patch splits it up into 15 smaller functions, moving the various locals into the places where they're needed, so we can easily see e.g where argc/argv get read vs written. The functions are private methods of a new driver class to give an extra level of encapsualation beyond just being static in gcc.c, and so that we can hide some state as member data inside the driver instance. Turning them into named functions/methods also makes it easier to talk about the different phases of main, and put breakpoints on them. Bootstrappedregrtested on x86_64-unknown-linux-gnu (Fedora 20). OK for trunk? gcc/ChangeLog: * gcc.c (class driver): New class. (main): Reimplement in terms of driver::main, moving most of the locals to be locals within individual methods of class driver. The remaining locals explicit_link_files, decoded_options and decoded_options_count are used by multiple driver:: methods, and so become member data. Doing so isolates the argc/argv reads and writes. Replace goto out with a special exit code from new method driver::prepare_infiles. Split out the old implementation of main into the following... (driver::main): New function, corresponding to the old main implementation. (driver::set_progname): New function, taken from the old main implementation. (driver::expand_at_files): Likewise. (driver::decode_argv): Likewise. (driver::global_initializations): Likewise. (driver::build_multilib_strings): Likewise. (driver::set_up_specs): Likewise. (driver::putenv_COLLECT_GCC): Likewise. (driver::maybe_putenv_COLLECT_LTO_WRAPPER): Likewise. (driver::handle_unrecognized_options): Likewise. (driver::maybe_print_and_exit): Likewise. (driver::prepare_infiles): Likewise. (driver::do_spec_on_infiles): Likewise. (driver::maybe_run_linker): Likewise. (driver::final_actions): Likewise. (driver::get_exit_code): Likewise. I committed this as r215861 after the approval, but it turns out to break the build of trunk's gcc.c on darwin, due to the const on driver::global_initializations preventing GCC_DRIVER_HOST_INITIALIZATION from modifying decoded_options_count and decoded_options: In file included from ./tm.h:16:0, from ../../src/gcc/gcc.c:34: ../../src/gcc/gcc.c: In member function ‘void driver::global_initializations() const’: ../../src/gcc/config/darwin.h:911:63: error: invalid conversion from ‘const unsigned int*’ to ‘unsigned int*’ [-fpermissive] darwin_driver_init (decoded_options_count, decoded_options) ^ ../../src/gcc/gcc.c:6862:3: note: in expansion of macro ‘GCC_DRIVER_HOST_INITIALIZATION’ GCC_DRIVER_HOST_INITIALIZATION; ^ ../../src/gcc/config/darwin.h:911:63: error: invalid conversion from ‘cl_decoded_option* const*’ to ‘cl_decoded_option**’ [-fpermissive] darwin_driver_init (decoded_options_count, decoded_options) ^ ../../src/gcc/gcc.c:6862:3: note: in expansion of macro ‘GCC_DRIVER_HOST_INITIALIZATION’ GCC_DRIVER_HOST_INITIALIZATION; ^ The attached patch appears to fix it; with it, gcc.c compiles (in a simple smoketest of make gcc.o in stage1 with --target=powerpc64-darwin). Am bootstrapping it on x86_64, fwiw; iains [CCed] said on IRC that he's attempting a bootstrap with it. Works for me (and seems, as you said on irc, retrospectively obvious) thanks! Iain Sorry about the breakage. gcc/ChangeLog: * gcc.c (driver::global_initializations): Remove const so that GCC_DRIVER_HOST_INITIALIZATION can modify decoded_options and decoded_options_count. fix-darwin.patch
Re: [PATCH] Fix build on darwin (was Re: [PATCH] gcc.c: Split up the driver's main into smaller functions)
On 10/03/14 13:15, David Malcolm wrote: On Fri, 2014-10-03 at 11:02 -0400, David Malcolm wrote: The main function for the driver in gcc.c has grown from ~200 lines in its original form (way back in r262) to ~1000 lines today, with a dozen locals (if we include the params). The following patch splits it up into 15 smaller functions, moving the various locals into the places where they're needed, so we can easily see e.g where argc/argv get read vs written. The functions are private methods of a new driver class to give an extra level of encapsualation beyond just being static in gcc.c, and so that we can hide some state as member data inside the driver instance. Turning them into named functions/methods also makes it easier to talk about the different phases of main, and put breakpoints on them. Bootstrappedregrtested on x86_64-unknown-linux-gnu (Fedora 20). OK for trunk? gcc/ChangeLog: * gcc.c (class driver): New class. (main): Reimplement in terms of driver::main, moving most of the locals to be locals within individual methods of class driver. The remaining locals explicit_link_files, decoded_options and decoded_options_count are used by multiple driver:: methods, and so become member data. Doing so isolates the argc/argv reads and writes. Replace goto out with a special exit code from new method driver::prepare_infiles. Split out the old implementation of main into the following... (driver::main): New function, corresponding to the old main implementation. (driver::set_progname): New function, taken from the old main implementation. (driver::expand_at_files): Likewise. (driver::decode_argv): Likewise. (driver::global_initializations): Likewise. (driver::build_multilib_strings): Likewise. (driver::set_up_specs): Likewise. (driver::putenv_COLLECT_GCC): Likewise. (driver::maybe_putenv_COLLECT_LTO_WRAPPER): Likewise. (driver::handle_unrecognized_options): Likewise. (driver::maybe_print_and_exit): Likewise. (driver::prepare_infiles): Likewise. (driver::do_spec_on_infiles): Likewise. (driver::maybe_run_linker): Likewise. (driver::final_actions): Likewise. (driver::get_exit_code): Likewise. I committed this as r215861 after the approval, but it turns out to break the build of trunk's gcc.c on darwin, due to the const on driver::global_initializations preventing GCC_DRIVER_HOST_INITIALIZATION from modifying decoded_options_count and decoded_options: In file included from ./tm.h:16:0, from ../../src/gcc/gcc.c:34: ../../src/gcc/gcc.c: In member function ‘void driver::global_initializations() const’: ../../src/gcc/config/darwin.h:911:63: error: invalid conversion from ‘const unsigned int*’ to ‘unsigned int*’ [-fpermissive] darwin_driver_init (decoded_options_count, decoded_options) ^ ../../src/gcc/gcc.c:6862:3: note: in expansion of macro ‘GCC_DRIVER_HOST_INITIALIZATION’ GCC_DRIVER_HOST_INITIALIZATION; ^ ../../src/gcc/config/darwin.h:911:63: error: invalid conversion from ‘cl_decoded_option* const*’ to ‘cl_decoded_option**’ [-fpermissive] darwin_driver_init (decoded_options_count, decoded_options) ^ ../../src/gcc/gcc.c:6862:3: note: in expansion of macro ‘GCC_DRIVER_HOST_INITIALIZATION’ GCC_DRIVER_HOST_INITIALIZATION; ^ The attached patch appears to fix it; with it, gcc.c compiles (in a simple smoketest of make gcc.o in stage1 with --target=powerpc64-darwin). Am bootstrapping it on x86_64, fwiw; iains [CCed] said on IRC that he's attempting a bootstrap with it. Sorry about the breakage. gcc/ChangeLog: * gcc.c (driver::global_initializations): Remove const so that GCC_DRIVER_HOST_INITIALIZATION can modify decoded_options and decoded_options_count. OK. jeff
