Re: rs6000 stack_tie mishap again

[Olivier Hainque <hainque@adacore.com>]

> On Mar 28, 2016, at 05:01 , Segher Boessenkool <segher@kernel.crashing.org> 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.

>> We have observed this with a gcc 4.7 back-end and weren't able to reproduce
>> with a more recent version.
> 
> This makes it not a regression and thus out of scope for GCC 6.  We're
> supposed to stabilise things now ;-)

Sure, no problem if this is only for gcc 7. I posted the message
while the details were still fresh in my mind.

>> 		  if (could_be_prologue_epilogue
>> 		      && prologue_epilogue_contains (insn))
>> 		    continue;
>> 
>> The motivation for this is unclear to me.
> 
> Alan linked to the history.

Right


>  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.

> 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.

>> 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.

Thanks for your feedback,

Olivier