Robin Dapp <rd...@linux.ibm.com> writes:
> When then and else are reversed, we would swap new_val and old_val.
> The same has to be done for our new code paths.
> Also, emit_conditional_move may perform swapping. In case we need to
> swap, the cc comparison also needs to be swapped and for this we pass
> the reversed cc comparison directly. An alternative would be to pass
> the constituents of the compare directly, but the functions have more
> than enough parameters already.
The changes to emit_conditional_move shouldn't be needed if we pass
the condition directly to a new overload, as per the 6/9 comments.
Let me know if that's not true.
> ---
> gcc/ifcvt.c | 37 ++++++++++++++++++++++++++-----------
> gcc/optabs.c | 4 +++-
> gcc/optabs.h | 2 +-
> 3 files changed, 30 insertions(+), 13 deletions(-)
>
> diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
> index 09bf443656c..d3959b870f6 100644
> --- a/gcc/ifcvt.c
> +++ b/gcc/ifcvt.c
> @@ -3313,7 +3315,7 @@ noce_convert_multiple_sets (struct noce_if_info
> *if_info)
> */
>
> rtx_insn *seq, *seq1 = 0, *seq2 = 0;
> - rtx temp_dest, temp_dest1, temp_dest2;
> + rtx temp_dest, temp_dest1 = NULL_RTX, temp_dest2 = NULL_RTX;
> unsigned int cost1 = 0, cost2 = 0;
> bool speed_p = if_info->speed_p;
> push_topmost_sequence ();
> @@ -3324,7 +3326,13 @@ noce_convert_multiple_sets (struct noce_if_info
> *if_info)
> temp_dest1 = noce_emit_cmove (if_info, temp, cond_code,
> x, y, new_val, old_val, NULL_RTX);
> else
> - noce_emit_move_insn (target, new_val);
> + {
> + temp_dest1 = target;
> + if (if_info->then_else_reversed)
> + noce_emit_move_insn (target, old_val);
> + else
> + noce_emit_move_insn (target, new_val);
> + }
>
> /* If we failed to expand the conditional move, drop out and don't
> try to continue. */
> @@ -3352,9 +3360,15 @@ noce_convert_multiple_sets (struct noce_if_info
> *if_info)
> start_sequence ();
> if (!need_no_cmovs.get (insn))
> temp_dest2 = noce_emit_cmove (if_info, target, cond_code,
> - x, y, new_val, old_val, cc_cmp);
> + x, y, new_val, old_val, cc_cmp, rev_cc_cmp);
> else
> - noce_emit_move_insn (target, new_val);
> + {
> + temp_dest2 = target;
> + if (if_info->then_else_reversed)
> + noce_emit_move_insn (target, old_val);
> + else
> + noce_emit_move_insn (target, new_val);
> + }
>
> /* If we failed to expand the conditional move, drop out and don't
> try to continue. */
> @@ -3386,7 +3400,8 @@ noce_convert_multiple_sets (struct noce_if_info
> *if_info)
> /* We do not require a temp register, so remove it from the list. */
> seq = seq2;
> temp_dest = temp_dest2;
> - temps_created.remove (temp);
> + if (temps_created.get (temp))
> + temps_created.remove (temp);
> }
> else
> {
> @@ -3436,7 +3451,8 @@ noce_convert_multiple_sets (struct noce_if_info
> *if_info)
> /* Mark all our temporaries and targets as used. */
> for (int i = 0; i < count; i++)
> {
> - set_used_flags (temporaries[i]);
> + if (temps_created.get(targets[i]))
> + set_used_flags (temporaries[i]);
> set_used_flags (targets[i]);
> }
>
> @@ -3941,7 +3957,7 @@ check_need_temps (basic_block bb, hash_map<rtx, bool>
> *need_temp, rtx cond)
>
> FOR_BB_INSNS (bb, insn)
> {
> - rtx set, src, dest;
> + rtx set, dest;
>
> if (!active_insn_p (insn))
> continue;
> @@ -3950,7 +3966,6 @@ check_need_temps (basic_block bb, hash_map<rtx, bool>
> *need_temp, rtx cond)
> if (set == NULL_RTX)
> continue;
>
> - src = SET_SRC (set);
> dest = SET_DEST (set);
>
> /* noce_emit_cmove will emit the condition check every time it is
> called
It looks like these changes are really fixes for previous patches in the
series. It'd be clearer to fold them into the relevant previous patch
if possible.
Thanks,
Richard
