| Message ID | 20141002143000.GH28697@msticlxl57.ims.intel.com |
|---|---|
| State | New |
| Headers | show |
On 10/02/14 08:30, Ilya Enkovich wrote: > Hi, > > This patch adds a check for call destination register for a call return value optimization based on REG_RETURNED note. This solves some ICE issues for MPX codes. > > Bootstrapped and tested on linux-x86_64. OK for trunk? > > Thanks, > Ilya > -- > 2014-10-02 Ilya Enkovich <ilya.enkovich@intel.com> > > * lra-constraints.c (inherit_in_ebb): Handle calls with > multiple return values. > * caller-save.c (save_call_clobbered_regs): Likewise. This is fine. Please put a comment before the new if (REG_P (...))) conditional which notes that a CALL_INSN may have multiple return values expressed in an EXPR_LIST. That way if someone looks at this code a few years down the line, they'll know exactly why the code checks REG_P rather than just using the result blindly. Did this fix all the multiple-return issues you encountered? Jeff
2014-10-02 22:30 GMT+04:00 Jeff Law <law@redhat.com>: > On 10/02/14 08:30, Ilya Enkovich wrote: >> >> Hi, >> >> This patch adds a check for call destination register for a call return >> value optimization based on REG_RETURNED note. This solves some ICE issues >> for MPX codes. >> >> Bootstrapped and tested on linux-x86_64. OK for trunk? >> >> Thanks, >> Ilya >> -- >> 2014-10-02 Ilya Enkovich <ilya.enkovich@intel.com> >> >> * lra-constraints.c (inherit_in_ebb): Handle calls with >> multiple return values. >> * caller-save.c (save_call_clobbered_regs): Likewise. > > This is fine. Please put a comment before the new if (REG_P (...))) > conditional which notes that a CALL_INSN may have multiple return values > expressed in an EXPR_LIST. That way if someone looks at this code a few > years down the line, they'll know exactly why the code checks REG_P rather > than just using the result blindly. > > Did this fix all the multiple-return issues you encountered? With this fix almost all benchmarks compile successfully but there is still one ICE in LRA. I described it in MPX ABI thread. Ilya > > Jeff >
diff --git a/gcc/caller-save.c b/gcc/caller-save.c index 11df2ec..f983165 100644 --- a/gcc/caller-save.c +++ b/gcc/caller-save.c @@ -879,8 +879,11 @@ save_call_clobbered_regs (void) if (GET_CODE (pat) == PARALLEL) pat = XVECEXP (pat, 0, 0); dest = SET_DEST (pat); - newpat = gen_rtx_SET (VOIDmode, cheap, copy_rtx (dest)); - chain = insert_one_insn (chain, 0, -1, newpat); + if (REG_P (dest)) + { + newpat = gen_rtx_SET (VOIDmode, cheap, copy_rtx (dest)); + chain = insert_one_insn (chain, 0, -1, newpat); + } } } last = chain; diff --git a/gcc/lra-constraints.c b/gcc/lra-constraints.c index 5f68399..cd54abd 100644 --- a/gcc/lra-constraints.c +++ b/gcc/lra-constraints.c @@ -5348,16 +5348,19 @@ inherit_in_ebb (rtx_insn *head, rtx_insn *tail) if (GET_CODE (pat) == PARALLEL) pat = XVECEXP (pat, 0, 0); dest = SET_DEST (pat); - start_sequence (); - emit_move_insn (cheap, copy_rtx (dest)); - restore = get_insns (); - end_sequence (); - lra_process_new_insns (curr_insn, NULL, restore, - "Inserting call parameter restore"); - /* We don't need to save/restore of the pseudo from - this call. */ - usage_insns[regno].calls_num = calls_num; - bitmap_set_bit (&check_only_regs, regno); + if (REG_P (dest)) + { + start_sequence (); + emit_move_insn (cheap, copy_rtx (dest)); + restore = get_insns (); + end_sequence (); + lra_process_new_insns (curr_insn, NULL, restore, + "Inserting call parameter restore"); + /* We don't need to save/restore of the pseudo from + this call. */ + usage_insns[regno].calls_num = calls_num; + bitmap_set_bit (&check_only_regs, regno); + } } } to_inherit_num = 0;