On 05/18/2017 08:37 AM, Bernd Edlinger wrote:
On 05/17/17 04:01, Daniel Santos wrote:
- if (ignore_outlined && cfun->machine->call_ms2sysv
- && in_hard_reg_set_p (stub_managed_regs, DImode, regno))
- return false;
+ if (ignore_outlined && cfun->machine->call_ms2sysv)
+ {
+ /* Registers who's save & restore will be managed by stubs
called from
+ pro/epilogue. */
+ HARD_REG_SET stub_managed_regs;
+ xlogue_layout::compute_stub_managed_regs (stub_managed_regs);
+ if (in_hard_reg_set_p (stub_managed_regs, DImode, regno))
+ return false;
+ }
+
if (crtl->drap_reg
&& regno == REGNO (crtl->drap_reg)
&& !cfun->machine->no_drap_save_restore)
This makes no sense. The entire purpose of stub_managed_regs is to
cache the result of xlogue_layout::compute_stub_managed_regs() and this
would unnecessarily repeat that calculation for each time
ix86_save_reg() is called. Since
xlogue_layout::compute_stub_managed_regs() calls ix86_save_reg many
times, this makes it even worse.Which registers are being saved
out-of-line and inline MUST be known at the time the stack layout is
determined. So stub_managed_regsshould either be left a TU static or
just moved to struct machine_function.
As an aside, I've noticed that xlogue_layout::compute_stub_managed_regs
is calling ix86_save_reg (which isn't trivial) more often than it really
has to, so I've refactored it.
Well, meanwhile I think the stub_managed_regs contain zero information
and need not be saved at all, because it can easily be reconstructed
from m->call_ms2sysv_extra_regs.
See the attached new version. Daniel does it work for you?
No, I'm not at all comfortable with you making so many seemingly
unnecessary changes to this code. (Although I wish I got this much
feedback during my RFCs! :) I can accept the changes to
is/count_stub_managed_reg (with some caveats), but I do not at all see
the rationale for changing m_stub_names to a static and adding another
dimension for the object instance -- from an OO standpoint, that's just
bad design. Can you please share your rationale for that?
Incidentally, half of the space in that array is wasted and can be
trimmed since a given instance of xlogue_layout either supports hard
frame pointers or doesn't, I just never got around to splitting that
apart. (The first three enum xlogue_stub values are for without HFP and
the last three for with.) Also, if we wanted to further reduce the
memory footprint of xlogue_layout objects, the offset field of struct
reginfo could be changed to int, and if we really wanted to go nuts then
16 bits would work for both of its fields.
So for is/count_stub_managed_reg(s), you are obviously much more
familiar with gcc, its passes and the i386 backend, but my knowledge
level makes me very uncomfortable having the result of
xlogue_layout::is_stub_managed_reg() determined in a way that has the
(apparent) potential to differ from from the state at the time the last
call to ix86_compute_frame_layout() was made; I just don't understand
well enough what all can change in between the last call to
ix86_compute_frame_layout() and the last call to
xlogue_layout::is_stub_managed_reg(). I like your
count_stub_managed_regs() is_stub_managed_regs() from a *performance*
standpoint (and I know I get too uptight about that kind of thing, so
appreciate that), but as to the change in scheme, I would have to trust
you if you assert that this will always behave consistently.
I also want to give you a little background on some of these seemingly
repetitive computations. One of my design goals was for the code to be
relatively easily to adapted to the management of out-of-line
pro/epilogue stubs for other possible scenarios where there are a lot of
clobbers and it could be useful. Granted, I don't have enough knowledge
of x86 architectures to identify situations other than this one (in
64-bit Wine) where it could be helpful and I know that x86 push/pops are
really small. So theoretically, struct machine_function's
"call_ms2sysv" could be changed to something like "outline_savres" and
any combination of clobbered registers for which there is a descent stub
could be used if it was a good choice. I also realize that nobody likes
complexity that isn't being used, and I respect that. So if you are
comfortable with this change and you believe you understand how it works
then I will agree to it, but I'll be trusting you well beyond my
knowledge level and ability to confidently predict the outcome (probably
what a programmer hates the most).
Thanks,
Daniel
