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: mips16/nomips16 function attributes, version N


Richard Sandiford wrote:
Thanks for the updated patch.  This is mostly looking good to me,
but I think it needs another round.

Sandra Loosemore <sandra@codesourcery.com> writes:
* I've implemented Richard's suggestion of taking out the call to mips_set_mips16_mode in override_options, and just waiting until the per-function hook has been invoked for the first time.

Hmm. The idea was that -mips16 should just specify the default for attributeless functions, so I was thinking that you should still keep the code that clears the MIPS16 flag.

You'd need to change TARGET_CPU_CPP_BUILTINS to use mips_base_mips16
rather than TARGET_MIPS16.  You'd also need to keep the code that sets
targetm.have_tls in override_options, and use a new "base" variable to
make that the default.  But would much more need to change?

I don't understand why you think clearing the MIPS16 flag in override_options is The Right Thing To Do. That means that the normal
first-time target initialization (the call to backend_init_target() from toplev.c) all happens in the "wrong" mode when -mips16 is passed. Trying to compensate for that by removing the first-time condition on the call to target_reinit() in mips_set_mips16_mode() causes a segfault, I think because it is happening before all the normal back end initialization is complete. I haven't tried to poke at it in detail yet, because even if I can resolve that order-dependency issue, why initialize everything the wrong way to start with and then have to go back and immediately reinitialize everything the other way, when we can more easily just do it right the first time?


I'm now thinking that the correct thing to do is to restore this to the way it was before, with the explicit call to mips_set_mips16_mode in override_options, so that the order dependency of the first-time mips16-specific initialization actions is made explicit. It's awfully hard to tell from reading the code just when that hook function is going to be invoked for the first time....

I didn't have any issues with your other comments -- thanks for your usual thorough review. :-)

-Sandra


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