On Wed, Oct 1, 2014 at 8:57 PM, Vladimir Makarov <vmaka...@redhat.com> wrote:
> The problem is in code introduced by Bernd in IRA and caller-saves.c in
> 2012. It is basically an optimization for functions returning always the
> same result as one argument (e.g. memcpy returning 1st argument).
>
> There are two possible solutions. First one is to prohibit the
> optimizations when there is a parallel in SET. Second one is to go deeper
> if the call result is guaranteed in the first element which is true for the
> patch.
I suspect that the first solution will regress
gcc.target/i386/retarg.c on i686 - that testcase test if referred
optimization is effective. All things equal, I think we should go with
the second solution.
> For the first solution, the patch would be
>
> Index: lra-constraints.c
> ===================================================================
> --- lra-constraints.c (revision 215748)
> +++ lra-constraints.c (working copy)
> @@ -5348,16 +5348,19 @@
> 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;
>
>
> For the second solution, the patch is
>
>
> Index: lra-constraints.c
> ===================================================================
> --- lra-constraints.c (revision 215748)
> +++ lra-constraints.c (working copy)
> @@ -5348,16 +5348,25 @@
> 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 (GET_CODE (dest) == PARALLEL)
> + {
> + dest = XVECEXP (dest, 0, 0);
> + if (GET_CODE (dest) == EXPR_LIST)
> + dest = XEXP (dest, 0);
> + }
> + 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);
> + }
> }
> }
>
>
> The first patch is safer but the second one is ok too. I have no particular
> preferences. Whatever we choose, analogous code in caller-saves.c should be
> changed too.
Uros.
