On Wed, Mar 23, 2016 at 05:04:39PM +0100, Olivier Hainque wrote:
> The visible effect is a powerpc-eabispe compiler (no red-zone) producing an
> epilogue sequence like
>
> addi %r11,%r1,184 # temp pointer within frame
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?
> addi %r1,%r11,104 # release frame
>
> evldd %r21,16(%r11) # restore registers
> ... # ...
> evldd %r31,96(%r11) # ...
>
> blr # return
> 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 ;-)
> if (! writep)
> {
> base = find_base_term (mem_addr);
> if (base && (GET_CODE (base) == LABEL_REF
> || (GET_CODE (base) == SYMBOL_REF
> && CONSTANT_POOL_ADDRESS_P (base))))
> return 0;
> }
>
>
> with
>
> (gdb) pr mem_addr
> (plus:SI (reg:SI 11 11)
> (const_int 96 [0x60]))
>
> and
>
> (gdb) pr base
> (symbol_ref/u:SI ("*.LC0") [flags 0x82])
>
> coming from insn 710, despite 894 in between. Ug.
Yeah that is just Wrong.
> The reason why 894 is not accounted in the base ref computation is because it
> is part of the epilogue sequence, and init_alias_analysis has:
>
> /* Walk the insns adding values to the new_reg_base_value array. */
> for (i = 0; i < rpo_cnt; i++)
> { ...
> if (could_be_prologue_epilogue
> && prologue_epilogue_contains (insn))
> continue;
>
> The motivation for this is unclear to me.
Alan linked to the history. 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.
Better would be not to have this hack at all.
> 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?
> 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?
> On the key insn at hand, the frame_related bit was cleared on purpose,
> per:
>
> https://gcc.gnu.org/ml/gcc-patches/2011-11/msg00543.html
>
> "1) Marking an instruction setting up r11 for use by _save64gpr_* as
> frame-related results in r11 being set as the cfa for the rest of the
> function. That's bad when r11 gets used for other things later.
> "
And that is correct.
> So, aside from the dependency issue which needs to be fixed somehow, I
> think it would make sense to consider using a strong blockage mecanism in
> expand_epilogue.
It would be very nice if we could directly express "the set of GPR1 should
stay behind any frame accesses", yeah.
Segher
