On Wed, Aug 16, 2017 at 06:23:13PM -0500, Segher Boessenkool wrote:
> Hi!
>
> On Wed, Aug 16, 2017 at 08:05:04AM +0930, Alan Modra wrote:
> > Repost with requested changes. I've extracted the logic that omits
> > frame saves from rs6000_frame_related to a new function, because it's
> > easier to document that way. The logic has been simplified a little
> > too: fixed_reg_p doesn't need to be called there.
> >
> > PR target/80938
> > * config/rs6000/rs6000.c (rs6000_savres_strategy): Revert 2017-08-09.
> > Don't use store multiple if only one reg needs saving.
> > (interesting_frame_related_regno): New function.
> > (rs6000_frame_related): Don't emit eh_frame for regs that
> > don't need saving.
> > (rs6000_emit_epilogue): Likewise.
>
> It's not just eh_frame, it's all call frame information.
Sure, I meant to fix that.
> > +/* This function is called when rs6000_frame_related is processing
> > + SETs within a PARALLEL, and returns whether the REGNO save ought to
> > + be marked RTX_FRAME_RELATED_P. The PARALLELs involved are those
> > + for out-of-line register save functions, store multiple, and the
> > + Darwin world_save. They may contain registers that don't really
> > + need saving. */
> > +
> > +static bool
> > +interesting_frame_related_regno (unsigned int regno)
> > +{
> > + /* Saves apparently of r0 are actually saving LR. */
> > + if (regno == 0)
> > + return true;
> > + /* If we see CR2 then we are here on a Darwin world save. Saves of
> > + CR2 signify the whole CR is being saved. */
> > + if (regno == CR2_REGNO)
> > + return true;
> > + /* Omit frame info for any user-defined global regs. If frame info
> > + is supplied for them, frame unwinding will restore a user reg.
> > + Also omit frame info for any reg we don't need to save, as that
> > + bloats eh_frame and can cause problems with shrink wrapping.
> > + Since global regs won't be seen as needing to be saved, both of
> > + these conditions are covered by save_reg_p. */
> > + return save_reg_p (regno);
> > +}
>
> The function name isn't so great, doesn't say what it does at all. Not
> that I can think of anything better.
>
> Maybe whatever is creating those instructions should set RTX_FRAME_RELATED_P
> by itself? Not sure if that is nicer.
>
> Both this CR2 and R0 handling are pretty nasty hacks. Could you add a
> comment saying that?
I wouldn't say the R0 handling is a nasty hack at all. You can't save
LR directly, storing to the stack must go via a gpr. I'm 100% sure
you know that, and so would anyone else working on powerpc gcc
support. It so happens that we use r0 in every case we hit this
code. *That* fact is commented. I don't really know what else you
want. Hmm, maybe I'm just too close to this code. I'll go with
expounding some of the things known, as follows.
/* Saves apparently of r0 are actually saving LR. It doesn't make
sense to substitute the regno here to test save_reg_p (LR_REGNO).
We *know* LR needs saving, and dwarf2cfi.c is able to deduce that
(set (mem) (r0)) is saving LR from a prior (set (r0) (lr)) marked
as frame related. */
(Incidentally, the dwarf2cfi.c behaviour is described by
In addition, if a register has previously been saved to a different
register,
Yup, great comment that one! Dates back to 2004, commit 60ea93bb72.)
The CR2 thing is a long-standing convention for frame info about CR, a
wart fixed by ELFv2. See elsewhere
/* CR register traditionally saved as CR2. */
and
/* In other ABIs, by convention, we use a single CR regnum to
represent the fact that all call-saved CR fields are saved.
We use CR2_REGNO to be compatible with gcc-2.95 on Linux. */
I'l go with:
/* If we see CR2 then we are here on a Darwin world save. Saves of
CR2 signify the whole CR is being saved. This is a long-standing
ABI wart fixed by ELFv2. As for r0/lr there is no need to check
that CR needs to be saved. */
> Okay for trunk with those improvements (eh_frame and hack comment).
> Thanks!
>
>
> Segher
--
Alan Modra
Australia Development Lab, IBM
