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
