On Mon, Mar 28, 2016 at 12:45:03PM +0200, Olivier Hainque wrote:
> > The normal -m32 compiler here generates code like
> >
> > lwz 11,0(1)
> >
> > and try as I might I cannot get it to fail. Maybe because the GPR11
> > setup here involves a load?
>
> You need to have had r11 last used to designate a global
> symbol as part of the function body (in order to have base_term
> designate a symbol_ref etc), and then have the scheduler
> decide that moving across is a good idea. It's certainly not
> an easy combination to trigger.
Yes, I did that (with some asm's). Like this:
===
void g(int, char *);
int dum;
void f(int x)
{
char big[200000];
g(x, big);
g(x, big);
register void *p asm("r11") = &dum;
asm("" : : "r"(p));
}
===
and the end of that becomes
===
bl g # 11 *call_nonlocal_sysvsi/1 [length = 4]
lis 11,dum@ha # 12 elf_high [length = 4]
la 11,dum@l(11) # 13 elf_low [length = 4]
lwz 11,0(1) # 33 *movsi_internal1/3 [length = 4]
lwz 0,4(11) # 34 *movsi_internal1/3 [length = 4]
lwz 31,-4(11) # 36 *movsi_internal1/3 [length = 4]
mr 1,11 # 38 *movsi_internal1/1 [length = 4]
mtlr 0 # 35 *movsi_internal1/9 [length = 4]
blr # 39 *return_internal_si [length = 4]
===
> > It seems clear that just considering the
> > prologue is enough to fix the original problem (frame pointer setup),
> > and problems like yours cannot happen in the prologue.
>
> Right. What is unclear is if it's correct to consider only
> prologues here. ISTM we arrange to produce CFI for epilogues
> as well today, at least in some circumstances, and maybe the
> issue Richard had with prologues at the time would need to
> be addressed for epilogues as well today.
Correct is to do alias analysis in both the prologue and epilogue.
If we don't, ports have to do all kinds of stack ties etc. to fake it.
Currently we do neither, so doing just one is a step in the right
direction.
> > Better would be not to have this hack at all.
>
> Sure.
>
> >> My rough understanding is that we probably really care about frame_related
> >> insns only here, at least on targets where the flag is supposed to be set
> >> accurately.
> >
> > On targets with DWARF2 unwind info the flag should be set on those insns
> > that need unwind info. This does not include all insns in the epilogue
> > that access the frame, so I don't think this will help you?
>
> My idea was that, maybe, the insns we need to see for alias analysis
> are precisely those for which the bit should not be set, which happened
> to be exactly the case in the problematic situation we hit. But as I said,
> I'm not 100% convinced the reasoning is globally correct.
All the register restores do not have the flag set, in most cases.
But they can in others (say, a target that does not optimise the CFI
stuff very well).
> >> This is the basis of the proposed patch, which essentially disconnects the
> >> shortcut above for !frame_related insns on targets which need precise alias
> >> analysis within epilogues, as indicated by a new target hook.
> >
> > Eww. Isn't that really all targets that schedule the epilogue at all?
>
> Yes. Most of them use stronger barriers which the dependency analyser
> knows about, so aren't affected by this.
>
> That's a possible alternative approach for rs6000.
A clobber of scratch should work yes. It's really heavy handed though,
we can move the GPR1 restore quite freely otherwise (in shrink-wrap),
but perhaps allowing scheduling to move it doesn't buy us much at all.
This doesn't solve the problem however that other dependencies in the
epilogue can be messed up in similar ways.
Segher
