On Fri, Jun 6, 2025 at 6:41 AM Jakub Jelinek <ja...@redhat.com> wrote:
>
> Hi!
>
> On top of the just posted patch, the following patch attempts to
> use value range to see if MSB is known to be false and for scalar integral
> extension in that case tries to expand both sign and zero extension and
> chooses based on RTX costs the cheaper one (if the costs are the same
> uses what it used before, TYPE_UNSIGNED (TREE_TYPE (treeop0)) based.
>
> The patch regresses the gcc.target/i386/pr78103-3.c test, will post
> a separate patch for that momentarily (with the intent that if all 3
> patches are approved, I'll commit the PR78103 related one before this one).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2025-06-06 Jakub Jelinek <ja...@redhat.com>
>
> PR middle-end/120434
> * expr.cc (expand_expr_real_2) <CASE_CONVERT>: If get_range_pos_neg
> at -O2 for scalar integer extension suggests the most significant
> bit of op0 is not set, try both unsigned and signed conversion and
> choose the cheaper one. If both are the same cost, choose one
> based on TYPE_UNSIGNED (TREE_TYPE (treeop0)).
>
> * gcc.target/i386/pr120434-2.c: New test.
>
> --- gcc/expr.cc.jj 2025-06-05 10:36:06.545069723 +0200
> +++ gcc/expr.cc 2025-06-05 19:14:45.601489036 +0200
> @@ -9885,14 +9885,68 @@ expand_expr_real_2 (const_sepops ops, rt
> op0 = gen_rtx_fmt_e (TYPE_UNSIGNED (TREE_TYPE (treeop0))
> ? ZERO_EXTEND : SIGN_EXTEND, mode, op0);
>
> + else if (SCALAR_INT_MODE_P (GET_MODE (op0))
> + && optimize >= 2
Maybe flag_expensive_optimizations instead?
Note the documentation for fexpensive-optimizations (should be cleaned
up to say kind of expensiveness, e.g compile time/compile memory
usage).
Thanks,
Andrew
> + && SCALAR_INT_MODE_P (mode)
> + && (GET_MODE_SIZE (as_a <scalar_int_mode> (mode))
> + > GET_MODE_SIZE (as_a <scalar_int_mode> (GET_MODE (op0))))
> + && get_range_pos_neg (treeop0,
> + currently_expanding_gimple_stmt) == 1)
> + {
> + /* If argument is known to be positive when interpreted
> + as signed, we can expand it as both sign and zero
> + extension. Choose the cheaper sequence in that case. */
> + bool speed_p = optimize_insn_for_speed_p ();
> + rtx uns_ret = NULL_RTX, sgn_ret = NULL_RTX;
> + do_pending_stack_adjust ();
> + start_sequence ();
> + if (target == NULL_RTX)
> + uns_ret = convert_to_mode (mode, op0, 1);
> + else
> + convert_move (target, op0, 1);
> + rtx_insn *uns_insns = end_sequence ();
> + start_sequence ();
> + if (target == NULL_RTX)
> + sgn_ret = convert_to_mode (mode, op0, 0);
> + else
> + convert_move (target, op0, 0);
> + rtx_insn *sgn_insns = end_sequence ();
> + unsigned uns_cost = seq_cost (uns_insns, speed_p);
> + unsigned sgn_cost = seq_cost (sgn_insns, speed_p);
> + bool was_tie = false;
> +
> + /* If costs are the same then use as tie breaker the other other
> + factor. */
> + if (uns_cost == sgn_cost)
> + {
> + uns_cost = seq_cost (uns_insns, !speed_p);
> + sgn_cost = seq_cost (sgn_insns, !speed_p);
> + was_tie = true;
> + }
> +
> + if (dump_file && (dump_flags & TDF_DETAILS))
> + fprintf (dump_file, ";; positive extension:%s unsigned cost: %u; "
> + "signed cost: %u\n",
> + was_tie ? " (needed tie breaker)" : "",
> + uns_cost, sgn_cost);
> + if (uns_cost < sgn_cost
> + || (uns_cost == sgn_cost && TYPE_UNSIGNED (TREE_TYPE
> (treeop0))))
> + {
> + emit_insn (uns_insns);
> + sgn_ret = uns_ret;
> + }
> + else
> + emit_insn (sgn_insns);
> + if (target == NULL_RTX)
> + op0 = sgn_ret;
> + else
> + op0 = target;
> + }
> else if (target == 0)
> - op0 = convert_to_mode (mode, op0,
> - TYPE_UNSIGNED (TREE_TYPE
> - (treeop0)));
> + op0 = convert_to_mode (mode, op0, TYPE_UNSIGNED (TREE_TYPE
> (treeop0)));
> else
> {
> - convert_move (target, op0,
> - TYPE_UNSIGNED (TREE_TYPE (treeop0)));
> + convert_move (target, op0, TYPE_UNSIGNED (TREE_TYPE (treeop0)));
> op0 = target;
> }
>
> --- gcc/testsuite/gcc.target/i386/pr120434-2.c.jj 2025-06-06
> 11:41:49.829197752 +0200
> +++ gcc/testsuite/gcc.target/i386/pr120434-2.c 2025-06-06 11:40:25.294236198
> +0200
> @@ -0,0 +1,15 @@
> +/* PR middle-end/120434 */
> +/* { dg-do compile { target lp64 } } */
> +/* { dg-options "-O2 -mtune=generic -masm=att" } */
> +/* { dg-final { scan-assembler-not "\tmovslq\t%edi, %rdi" } } */
> +/* { dg-final { scan-assembler "\tmovl\t%edi, %edi" } } */
> +
> +extern unsigned long long foo (unsigned long long x);
> +
> +unsigned long long
> +bar (int x)
> +{
> + if (x < 50)
> + return 0;
> + return foo (x);
> +}
>
> Jakub
>
