Re: [PATCH] [i386] Recompute the frame layout less often

[Daniel Santos <daniel.santos@pobox.com>]

On 05/14/2017 11:31 AM, Bernd Edlinger wrote:
> Hi Daniel,
>
> there is one thing I don't understand in your patch:
> That is, it introduces a static value:
>
> /* Registers who's save & restore will be managed by stubs called from
>      pro/epilogue.  */
> static HARD_REG_SET GTY(()) stub_managed_regs;
>
> This seems to be set as a side effect of ix86_compute_frame_layout,
> and depends on the register usage of the current function.
> But values that depend on the current function need usually be
> attached to cfun->machine, because the passes can run in parallel
> unless I am completely mistaken, and the stub_managed_regs may
> therefore be computed from a different function.
>
>
> Bernd.

I'm relatively new to GCC and still learning.  However, there are quite 
a lot of static TU variables in i386.c like this.  I am not aware of gcc 
having parallelism support, but if it were to be added then all of these 
TU variables should probably be moved to some class or struct (like 
cfun->machine) to reduce the number of TLS lookups required (which I 
presume is a little more expensive than a this/offset calculation).  
Having this (as well as other variables) in such a struct is better 
design IMO, but as I said, I'm still learning GCC's architecture, idioms 
and patterns.  (I should add that I don't really understand the GTY 
memory management either. :)

To be clear on class xlogue_layout, the only instances of this class are 
const and could be shared across multiple threads.  It is dependent upon 
the cfun->machine as well as the global struct rtl_data crtl, but is not 
so entangled that were these proper C++ classes (with private data) that 
it would need to be a friend -- it only needs read-access to their data 
members.

To be honest, it's a strange feeling programming in a mixture of C and 
C++ idioms, but I know it was only recently converted to C++ so I think 
it's better to try to use only one or the other in a given function.  
But if I were going to do this all OO, then ix86_compute_frame_layout 
would be a member function of ix86_frame (which would be a 
specialization of some generic "frame" class), machine_function would be 
class ix86_machine_function with it's own compute_frame_layout that 
called ix86_frame::compute_frame_layout, etc.  If I really wanted to go 
nuts, I would consider making class function, et.al. template classes 
with machine_function and machine_function_state part of the object 
instead of pointers to separate objects to reduce accesses down to a 
single this/offset, but now I I'm *really* digressing...

Please free to move it.

Thanks,
Daniel