Re: PATCH: mips16/nomips16 function attributes, version N

[Sandra Loosemore <sandra@codesourcery.com>]

Richard Sandiford wrote:

> I suppose I was hoping that, one day, the compiler would be in a
> "mode-agnostic" state when outside the top-level push_cfun/pop_cfun
> pair, except for explicit exceptions like TARGET_CPU_CPP_BUILTINS.
> However, that would mean deferring backend_init until the first call
> to your new hook, which would probably be even more work than what
> you've done so far.  Even then, the compiler would not be in a
> mode-agnostic state between a top-level pop_cfun and the next
> push_cfun.  I suppose that day will only come "when" (= if) all
> mode-specific data is in an easily-switchable struct.

Right.  In a perfect world, what we'd do here is just have two objects 
encapsulating all the back end state for each mode, and all we'd have to 
swap is a pointer.  But reverse-engineering that onto the existing code 
would be very nasty, and likely introduce many bugs.

> As it is, the comments you made on internal IRC suggest that this
> reinitialisation is very expensive, so I suppose it's not something
> we want to foist on normal -mips16 users for the sake of an internal
> mental model.

To give a rough idea of the magnitude of the performance hit, my 
previous version of these patches that invoked the reset hook only once 
per function ran about 20% slower in -mflip-mips16 mode than without, 
judging just by time stamps on log files from running the gcc testsuite. 
  With the new version and the hook being invoked every time cfun is 
set, that's gone up to a factor of 3.  Of course -mflip-mips16 is 
designed to be pathological; in practice, use of these attributes is 
going to be exceptional, and we should optimize for the common case instead.

As I noted in my comments about the other part of the patch, I think 
some of the places where cfun is being set to NULL are really not 
necessary (they seem to be signalling that a processing phase is done 
rather than a real function context switch), and getting rid of those 
places would speed things up.  Again, it's only a concern for things 
that aren't using the default mode.

> From the segfault you describe, it sounds like we currently assume that
> calling this hook is a no-op until the first time we pass a function
> with attributes.  (Or at least, it sounds like that rule would avoid the
> situations where things go wrong.)  If so, I think we should mention
> that in the tm.texi documentation.

Yes, I am worried about this too.  I already had to add a hack to 
prevent the hook from being invoked recursively; maybe I need to 
generalize that with something to prevent it from being invoked before 
initial initialization :-) is complete, and/or put an assertion in 
target_reinit ().  I'm going to poke around at this once I've tested the 
other changes you suggested.

-Sandra