Bernd Edlinger <bernd.edlin...@hotmail.de> writes: > 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?
Hi Bernd, I don't see any problem with this change from a MIPS backend perspective. As to whether rtx_addr_can_trap_p should be using initial_elimination_offset then I don't have any particular opinion. Richard: Any objections to using this fix so we can solve the PR leaving debate on the original rtx_addr_can_trap_p as a separate issue? Thanks, Matthew
