Re: RFA: Use promote_mode to handle mismatched modes in out-of-ssa

[Richard Guenther <richard dot guenther at gmail dot com>]

On Sat, Aug 8, 2009 at 10:31 AM, Richard
Sandiford<rdsandiford@googlemail.com> wrote:
> Register promotion can cause an rtx to have a different mode from the
> associated TYPE_MODE. ?tree-out-of-ssa.c handles this as follows:
>
> ?mode = GET_MODE (SA.partition_to_pseudo[dest]);
> ?x = expand_expr (src, SA.partition_to_pseudo[dest], mode, EXPAND_NORMAL);
> ?if (GET_MODE (x) != VOIDmode && GET_MODE (x) != mode)
> ? ?x = convert_to_mode (mode, x, TYPE_UNSIGNED (TREE_TYPE (src)));
> ?if (CONSTANT_P (x) && GET_MODE (x) == VOIDmode
> ? ? ?&& mode != TYPE_MODE (TREE_TYPE (src)))
> ? ?x = convert_modes (mode, TYPE_MODE (TREE_TYPE (src)),
> ? ? ? ? ? ? ? ? ? ? ? ? ?x, TYPE_UNSIGNED (TREE_TYPE (src)));
> ?if (x != SA.partition_to_pseudo[dest])
> ? ?emit_move_insn (SA.partition_to_pseudo[dest], x);
>
> The problem here is that the signedness of the conversion is always
> taken from the type, whereas the target is allowed to choose its own
> signedness. ?SUBREG_PROMOTED_UNSIGNED_P guarantees that a promoted
> register is always extended in the same (target-defined) way,
> so it's important to be consistent.
>
> The patch below fixes this by using promote_mode, and asserting that
> the promoted mode is indeed the same as the partition register's.
>
> The code currently passes the partition register and destination
> mode to expand_expr even if the source has a different mode.
> I think this is dangerous for the same reason: there's no guarantee
> that expand_expr will use the right signedness. ?(The MEM case in
> particular is suspicious.)
>
> Also, the convert_* calls seem overly complex. ?convert_to_mode
> is simply a wrapper around convert_modes, so we might as well
> use the latter for all cases.
>
> Boostrapped & regression-tested on x86_64-linux-gnu. ?Also
> regression-tested on mipsisa64-elf, where it fixes a boatload
> of execution failures. ?OK to install?

Ok.

Thanks,
Richard.

> Richard
>
>
> gcc/
> ? ? ? ?* tree-out-of-ssa.c (insert_value_copy_on_edge): If the source
> ? ? ? ?and destination have different modes, Use promote_mode to
> ? ? ? ?determine the signedness of the conversion. ?Assert that the
> ? ? ? ?promoted source mode matches the destination mode. ?Don't pass
> ? ? ? ?the destination and destination mode to expand_expr if the source
> ? ? ? ?mode is different. ?Simplify conversion logic.
>
> Index: gcc/tree-outof-ssa.c
> ===================================================================
> --- gcc/tree-outof-ssa.c ? ? ? ?2009-08-07 21:56:17.000000000 +0100
> +++ gcc/tree-outof-ssa.c ? ? ? ?2009-08-07 21:57:00.000000000 +0100
> @@ -195,7 +195,9 @@ insert_partition_copy_on_edge (edge e, i
> ?insert_value_copy_on_edge (edge e, int dest, tree src, source_location locus)
> ?{
> ? rtx seq, x;
> - ?enum machine_mode mode;
> + ?enum machine_mode dest_mode, src_mode;
> + ?int unsignedp;
> +
> ? if (dump_file && (dump_flags & TDF_DETAILS))
> ? ? {
> ? ? ? fprintf (dump_file,
> @@ -214,14 +216,21 @@ insert_value_copy_on_edge (edge e, int d
> ? ? set_curr_insn_source_location (locus);
>
> ? start_sequence ();
> - ?mode = GET_MODE (SA.partition_to_pseudo[dest]);
> - ?x = expand_expr (src, SA.partition_to_pseudo[dest], mode, EXPAND_NORMAL);
> - ?if (GET_MODE (x) != VOIDmode && GET_MODE (x) != mode)
> - ? ?x = convert_to_mode (mode, x, TYPE_UNSIGNED (TREE_TYPE (src)));
> - ?if (CONSTANT_P (x) && GET_MODE (x) == VOIDmode
> - ? ? ?&& mode != TYPE_MODE (TREE_TYPE (src)))
> - ? ?x = convert_modes (mode, TYPE_MODE (TREE_TYPE (src)),
> - ? ? ? ? ? ? ? ? ? ? ? ? x, TYPE_UNSIGNED (TREE_TYPE (src)));
> +
> + ?src_mode = TYPE_MODE (TREE_TYPE (src));
> + ?unsignedp = TYPE_UNSIGNED (TREE_TYPE (src));
> + ?dest_mode = promote_mode (TREE_TYPE (src), src_mode, &unsignedp, 0);
> + ?gcc_assert (dest_mode == GET_MODE (SA.partition_to_pseudo[dest]));
> +
> + ?if (src_mode != dest_mode)
> + ? ?{
> + ? ? ?x = expand_expr (src, NULL, src_mode, EXPAND_NORMAL);
> + ? ? ?x = convert_modes (dest_mode, src_mode, x, unsignedp);
> + ? ?}
> + ?else
> + ? ?x = expand_expr (src, SA.partition_to_pseudo[dest],
> + ? ? ? ? ? ? ? ? ? ?dest_mode, EXPAND_NORMAL);
> +
> ? if (x != SA.partition_to_pseudo[dest])
> ? ? emit_move_insn (SA.partition_to_pseudo[dest], x);
> ? seq = get_insns ();
>