Richard Biener <richard.guent...@gmail.com> writes: > On Mon, Mar 3, 2014 at 11:01 PM, Richard Sandiford > <rdsandif...@googlemail.com> wrote: >> AIUI: >> >> (1) The main point of http://gcc.gnu.org/ml/gcc-patches/2012-10/msg01172.html >> was that we had: >> >> emit_move_insn (virtual_stack_vars_rtx, hard_frame_pointer_rtx); >> /* This might change the hard frame pointer in ways that aren't >> apparent to early optimization passes, so force a clobber. */ >> emit_clobber (hard_frame_pointer_rtx); >> >> and, after elimination, the clobber killed the assignment to the >> hard frame pointer. Which it could. :-) >> >> (2) Aassuming for simplicity that STARTING_FRAME_OFFSET == 0, we end up >> with an assignment like: >> >> (set frame_pointer_rtx hard_frame_pointer_rtx) >> >> And the problem is that frame_pointer_rtx often isn't a real register: >> it gets eliminated to hard_frame_pointer_rtx+X, where X isn't known >> until RA time. I.e. the assignment is really: >> >> (set (plus hard_frame_pointer_rtx X) hard_frame_pointer_rtx) >> >> which becomes: >> >> (set hard_frame_pointer_rtx (plus hard_frame_pointer_rtx -X)) >> >> Before RA it isn't obvious that the assignment clobbers >> hard_frame_pointer_rtx, so it would seem to most passes that >> frame_pointer_rtx and hard_frame_pointer_rtx are equivalent >> after the set. That was why the clobber was there. >> >> (3) We chose to fix this by deleting the explicit clobber and relying on >> the magicness of unspec_volatile to imply the clobber instead. >> >> But I don't think (3) is a good idea. We should instead fix the dataflow >> so that it's accurate. >> >> Long term it would be good to have a different representation for (2), > > An UNSPEC that sets frame_pointer_rtx, uses and clobbers > hard_frame_pointer_rtx. > > In fact the plain move_insn is a lie and the clobber should have been > at least in a parallel with the set of frame_pointer_rtx ... > > ?
Yeah, the plain move is lie, which is why ideally I'd like to change it. But the problem is that the elimination code specifically expects this kind of pattern to be used. It eliminates the target of the move to reg+offset and treats the new insn as being an assignment to reg with offset subtracted from the source. It needs to be something simple like a plain move or a: (set (reg) (plus (reg) (const_int))) for subtracting offset to work correctly. So changing the representation of the move would mean changing the way that those eliminations are handled too. Not 4.9 material :-) >> but I don't have any bright ideas what that might be. Until then I think >> we can model the dataflow accurately by having a use as well as a clobber >> of hard_frame_pointer_rtx. I went back to r192682: > > Indeed. > >> http://gcc.gnu.org/ml/gcc-testresults/2012-10/msg02350.html >> >> and saw the -m32 tests fail without the builtins.c part of the patch. >> They passed after it. I then went to r200643: >> >> 2013-07-03 Hans-Peter Nilsson <h...@bitrange.com> >> >> PR middle-end/55030 >> * stmt.c (expand_nl_goto_receiver): Remove almost-copy of >> expand_builtin_setjmp_receiver. >> (expand_label): Adjust, call expand_builtin_setjmp_receiver >> with NULL for the label parameter. >> * builtins.c (expand_builtin_setjmp_receiver): Don't clobber >> the frame-pointer. Adjust comments. >> [HAVE_builtin_setjmp_receiver]: Emit builtin_setjmp_receiver >> only if LABEL is non-NULL. >> >> and applied the full patch. The tests still passed, despite the removal >> of the volatile checks. (To recap, the volatile checks touched here >> were the same ones touched by: >> >> http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00868.html >> >> Rather than reverting that part I'm removing the check entirely, >> since we seem to agree that the original asm handling wasn't necessary.) >> >> I'll run a full test overnight, but does this look like it might be >> a way out, at least for 4.9? > > Shouldn't the use and clobber be part of the "move" and thus wrapped > inside a parallel? Or am I misunderstanding how parallel works? > What keeps those three insns adjacent otherwise? Strict adjacency doesn't really matter. We just need to kill the equivalence between the soft and hard frame pointers for anything after the clobber. I did wonder about using an empty asm that takes the old hard frame pointer as input and produces a new hard frame pointer as output, but normal asms aren't allowed to change the frame pointer. Thanks, Richard
