On 12/02/13 15:51, Jakub Jelinek wrote:
Hi!
On Sat, Nov 30, 2013 at 12:38:30PM +0100, Eric Botcazou wrote:
Rather than adding do_pending_stack_adjust () in all the places, especially
when it isn't clear whether emit_conditional_move will be called at all and
whether it will actually do do_pending_stack_adjust (), I chose to add
two new functions to save/restore the pending stack adjustment state,
so that when instruction sequence is thrown away (either by doing
start_sequence/end_sequence around it and not emitting it, or
delete_insns_since) the state can be restored, and have changed all the
places that IMHO need it for emit_conditional_move.
Why not do it in emit_conditional_move directly then? The code thinks it's
clever to do:
do_pending_stack_adjust ();
last = get_last_insn ();
prepare_cmp_insn (XEXP (comparison, 0), XEXP (comparison, 1),
GET_CODE (comparison), NULL_RTX, unsignedp, OPTAB_WIDEN,
&comparison, &cmode);
[...]
delete_insns_since (last);
return NULL_RTX;
but apparently not, so why not delete the stack adjustment as well and restore
the state afterwards?
On Sat, Nov 30, 2013 at 09:10:35AM +0100, Richard Biener wrote:
The idea is good but I'd like to see a struct rather than an array for the
storage.
So, this patch attempts to include both of the proposed changes.
Furthermore, I've noticed that calls.c has been saving/restoring those
two values by hand, so now it can use the new APIs for that too.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
What about 4.8 branch? I could create an alternative patch for 4.8,
keep everything as is and just save/restore the two fields by hand in
emit_conditional_move like calls.c used to do it.
2013-12-02 Jakub Jelinek <ja...@redhat.com>
PR target/58864
* dojump.c (save_pending_stack_adjust, restore_pending_stack_adjust):
New functions.
* expr.h (struct saved_pending_stack_adjust): New type.
(save_pending_stack_adjust, restore_pending_stack_adjust): New
prototypes.
* optabs.c (emit_conditional_move): Call save_pending_stack_adjust
and get_last_insn before do_pending_stack_adjust, call
restore_pending_stack_adjust after delete_insns_since.
* expr.c (expand_expr_real_2): Don't call do_pending_stack_adjust
before calling emit_conditional_move.
* expmed.c (expand_sdiv_pow2): Likewise.
* calls.c (expand_call): Use {save,restore}_pending_stack_adjust.
* g++.dg/opt/pr58864.C: New test.
OK.
Branch maintainers call on how they want to deal with this in 4.8.
jeff
