Hi,
On 30.12.2015 15:31, Richard Sandiford wrote:
> I think the problem is deeper than that though. The instructions that
> are triggering the ICE are only generated by the prologue, so this
> means that we're trying to lay out the frame again after the prologue
> has been generated, whereas it really needs to be fixed by then. (And
> even if recalculating it is a no-op, the operation is still too
> expensive to be repeated lightly.) Query functions like
> rtx_addr_can_trap_p(_1) shouldn't really be changing or recalculating
> the frame layout or other global state. I think we need to find a
> different way of getting the information. Maybe reload/LRA should use
> its own structures to calculate the range of "safe" stack and hfp
> offsets, then store them in crtl for rtx_addr_can_trap_p to use. AIUI,
> before reload, the only non-trapping uses of sp should be for outgoing
> arguments, so we can use a test based on the cumulative outgoing
> arguments size. I don't think the hfp should be used at all before
> reload, so we could conservatively return -1 for that case. Thanks,
> Richard
Yes, I agree, it _should_ be a no-op, but given the complexity of
mips_compute_frame_info it is probably better to use cached values after
reload completed.
Before reload_completed, rtx_addr_can_trap_p is just trying to do an
initial guess on the stack layout, it may well be possible that some
important information is still missing here. We will never call the
target callback from here, before reload_completed. It is however not
completely impossible that rtx_addr_can_trap_p is called while
lra/reload is already renaming the registers, see PR66614.
Of course the required information is already cached in
cfun->machine->frame, but rtx_addr_can_trap_p can't use the information
without a target callback. And I just want to avoid to invent a new
callback for that.
I looked at how other targets handle this situation and found that most
of them either have a more simple frame layout function or they keep the
previously cached values if they are ever called after reload_completed.
I found, previously mips_compute_frame_info was always called with
reload_completed=false, either from mips_frame_pointer_required or from
mips_initial_elimination_offset. Reload seems to call the
initial_elimination_offset repeatedly and seems to _expect_ the values
to change from time to time. However it does not seem to expect the
result from mips_frame_pointer_required to change after reload/LRA has
started. But as it seems, that may be possible for TARGET_MIPS16 if the
frame size happens to be very close to 0x7FFF. Well that could be
another story.
Anyways when mips_compute_frame_info is called with
reload_completed=true, we can as well use the cached values, as they
were immediately before reload_completed.
So what do you think of my new version of the patch?
Thanks
Bernd.
2015-12-30 Bernd Edlinger <bernd.edlin...@hotmail.de>
PR target/69012
* config/mips/mips.c (mips_compute_frame_info): Don't change the
frame info after reload completed.
Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c (revision 231954)
+++ gcc/config/mips/mips.c (working copy)
@@ -10321,6 +10321,10 @@ mips_compute_frame_info (void)
HOST_WIDE_INT offset, size;
unsigned int regno, i;
+ /* Don't change the frame info after reload completed. */
+ if (reload_completed)
+ return;
+
/* Set this function's interrupt properties. */
if (mips_interrupt_type_p (TREE_TYPE (current_function_decl)))
{
