On Wed, Nov 13, 2024 at 11:25 AM H.J. Lu <hjl.to...@gmail.com> wrote:
>
> On Wed, Nov 13, 2024 at 10:23 AM Hongtao Liu <crazy...@gmail.com> wrote:
> >
> > On Wed, Nov 13, 2024 at 8:29 AM H.J. Lu <hjl.to...@gmail.com> wrote:
> > >
> > > On Wed, Nov 13, 2024 at 5:57 AM H.J. Lu <hjl.to...@gmail.com> wrote:
> > > >
> > > > On Tue, Nov 12, 2024 at 9:30 PM Richard Biener
> > > > <richard.guent...@gmail.com> wrote:
> > > > >
> > > > > On Tue, Nov 12, 2024 at 1:49 PM H.J. Lu <hjl.to...@gmail.com> wrote:
> > > > > >
> > > > > > When passing 0xff as an unsigned char function argument, the C
> > > > > > frontend
> > > > > > promotion will promote it to int:
> > > > > >
> > > > > > <integer_cst 0x7fffe6aa23a8 type <integer_type 0x7fffe98225e8 int>
> > > > > > constant 255>
> > > > > >
> > > > > > and expand_normal always returns the rtx value using the
> > > > > > sign-extended
> > > > > > representation,
> > > > > >
> > > > > > (const_int 255 [0xff])
> > > > > >
> > > > > > If the C frontend doesn't promote unsigned char to int,
> > > > > > expand_normal will
> > > > > > get
> > > > > >
> > > > > > <integer_cst 0x7fffe9824018 type <integer_type 0x7fffe9822348
> > > > > > unsigned char > co
> > > > > > nstant 255>
> > > > > >
> > > > > > and return
> > > > > >
> > > > > > (const_int -1 [0xffffffffffffffff])
> > > > >
> > > > > that looks wrong to me, but in other places we ensure
> > > > > to use trunc_int_for_mode (), not some odd function like
> > > > > you introduce here?
> > > >
> > > > I opened:
> > > >
> > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117547
> > > >
> > >
> > > Here is the v2 patch with reference to PR target/117547.
> > >
> > > When passing 0xff as an unsigned char function argument with the C
> > > frontend
> > > promotion, expand_normal gets
> > >
> > > <integer_cst 0x7fffe6aa23a8 type <integer_type 0x7fffe98225e8 int>
> > > constant 255>
> > >
> > > and returns the rtx value using the sign-extended representation:
> > >
> > > (const_int 255 [0xff])
> > >
> > > Without the C frontend promotion, expand_normal gets
> > >
> > > <integer_cst 0x7fffe9824018 type <integer_type 0x7fffe9822348 unsigned
> > > char > co
> > > nstant 255>
> > >
> > > and returns
> > >
> > > (const_int -1 [0xffffffffffffffff])
> > >
> > > which doesn't work with the IN_RANGE predicates. Extract the 8-bit/16-bit
> > > integer constants to always return
> > >
> > > (const_int 255 [0xff])
> > >
> > > so that the return value can be used with the IN_RANGE predicates, like
> > > const_0_to_255_operand, without the C frontend promotion.
> > + if (TREE_CODE (arg) == INTEGER_CST)
> > + {
> > + tree type = TREE_TYPE (arg);
> > + if (INTEGRAL_TYPE_P (type)
> > + && TYPE_PRECISION (type) < TYPE_PRECISION (integer_type_node))
> > + {
> > + HOST_WIDE_INT cst = TREE_INT_CST_LOW (arg);
> > + return GEN_INT (cst);
> > + }
> > + }
> > +
> >
> > Shouldn't we guard this with TYPE_UNSIGNED (type)?
>
> Good idea. I will add it.
>
> > I'm also worried that GEN_INT may trigger ICE like PR96262.
> > Can we use gen_int_mode instead, or we must use GEN_INT here?
>
> gen_int_mode (255, QImode) returns
>
> (const_int -1 [0xffffffffffffffff])
>
> unless we use gen_int_mode (255, SImode)
>
Here is the v3 patch which adds the check with TYPE_UNSIGNED (type).
OK for master?
H.J.
---
When passing 0xff as an unsigned char function argument with the C frontend
promotion, expand_normal gets
<integer_cst 0x7fffe6aa23a8 type <integer_type 0x7fffe98225e8 int> constant 255>
and returns the rtx value using the sign-extended representation:
(const_int 255 [0xff])
Without the C frontend promotion, expand_normal gets
<integer_cst 0x7fffe9824018 type <integer_type 0x7fffe9822348 unsigned char > co
nstant 255>
and returns
(const_int -1 [0xffffffffffffffff])
which doesn't work with the IN_RANGE predicates. Extract the unsigned
char/short integer constants to always return
(const_int 255 [0xff])
so that the return value can be used with the IN_RANGE predicates, like
const_0_to_255_operand, without the C frontend promotion.
PR target/117547
* config/i386/i386-expand.cc (ix86_expand_integer_cst_argument):
New function.
(ix86_expand_args_builtin): Call ix86_expand_integer_cst_argument
to expand the argument before calling fixup_modeless_constant.
(ix86_expand_round_builtin): Likewise.
(ix86_expand_special_args_builtin): Likewise.
(ix86_expand_builtin): Likewise.
--
H.J.
From 56e0af83427730f76cc638d112314e1058d12a3b Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 12 Nov 2024 09:03:31 +0800
Subject: [PATCH v3] i386: Add ix86_expand_integer_cst_argument
When passing 0xff as an unsigned char function argument with the C frontend
promotion, expand_normal gets
<integer_cst 0x7fffe6aa23a8 type <integer_type 0x7fffe98225e8 int> constant 255>
and returns the rtx value using the sign-extended representation:
(const_int 255 [0xff])
Without the C frontend promotion, expand_normal gets
<integer_cst 0x7fffe9824018 type <integer_type 0x7fffe9822348 unsigned char > constant 255>
and returns
(const_int -1 [0xffffffffffffffff])
which doesn't work with the IN_RANGE predicates. Extract the unsigned
char/short integer constants to always return
(const_int 255 [0xff])
so that the return value can be used with the IN_RANGE predicates, like
const_0_to_255_operand, without the C frontend promotion.
PR target/117547
* config/i386/i386-expand.cc (ix86_expand_integer_cst_argument):
New function.
(ix86_expand_args_builtin): Call ix86_expand_integer_cst_argument
to expand the argument before calling fixup_modeless_constant.
(ix86_expand_round_builtin): Likewise.
(ix86_expand_special_args_builtin): Likewise.
(ix86_expand_builtin): Likewise.
Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
---
gcc/config/i386/i386-expand.cc | 56 +++++++++++++++++++++++++++++++---
1 file changed, 51 insertions(+), 5 deletions(-)
diff --git a/gcc/config/i386/i386-expand.cc b/gcc/config/i386/i386-expand.cc
index a6e6e738a52..8be479e2ffe 100644
--- a/gcc/config/i386/i386-expand.cc
+++ b/gcc/config/i386/i386-expand.cc
@@ -11243,6 +11243,52 @@ fixup_modeless_constant (rtx x, machine_mode mode)
return x;
}
+/* Expand the outgoing argument ARG and extract INTEGER_CST rtx value
+ suitable for the IN_RANGE predicates. */
+
+static rtx
+ix86_expand_integer_cst_argument (tree arg)
+{
+ /* When passing 0xff as an unsigned char function argument with the
+ C frontend promotion, expand_normal gets
+
+ <integer_cst 0x7fffe6aa23a8 type <integer_type 0x7fffe98225e8 int> constant 255>
+
+ and returns the rtx value using the sign-extended representation:
+
+ (const_int 255 [0xff])
+
+ Without the C frontend promotion, expand_normal gets
+
+ <integer_cst 0x7fffe9824018 type <integer_type 0x7fffe9822348 unsigned char > constant 255>
+
+ and returns
+
+ (const_int -1 [0xffffffffffffffff])
+
+ which doesn't work with the IN_RANGE predicates. Extract the
+ unsigned char/short integer constants to always return
+
+ (const_int 255 [0xff])
+
+ so that the return value can be used with the IN_RANGE predicates,
+ like const_0_to_255_operand, without the C frontend promotion. */
+
+ if (TREE_CODE (arg) == INTEGER_CST)
+ {
+ tree type = TREE_TYPE (arg);
+ if (INTEGRAL_TYPE_P (type)
+ && TYPE_UNSIGNED (type)
+ && TYPE_PRECISION (type) < TYPE_PRECISION (integer_type_node))
+ {
+ HOST_WIDE_INT cst = TREE_INT_CST_LOW (arg);
+ return GEN_INT (cst);
+ }
+ }
+
+ return expand_normal (arg);
+}
+
/* Subroutine of ix86_expand_builtin to take care of insns with
variable number of operands. */
@@ -12135,7 +12181,7 @@ ix86_expand_args_builtin (const struct builtin_description *d,
for (i = 0; i < nargs; i++)
{
tree arg = CALL_EXPR_ARG (exp, i);
- rtx op = expand_normal (arg);
+ rtx op = ix86_expand_integer_cst_argument (arg);
machine_mode mode = insn_p->operand[i + 1].mode;
/* Need to fixup modeless constant before testing predicate. */
op = fixup_modeless_constant (op, mode);
@@ -12870,7 +12916,7 @@ ix86_expand_round_builtin (const struct builtin_description *d,
for (i = 0; i < nargs; i++)
{
tree arg = CALL_EXPR_ARG (exp, i);
- rtx op = expand_normal (arg);
+ rtx op = ix86_expand_integer_cst_argument (arg);
machine_mode mode = insn_p->operand[i + 1].mode;
bool match = insn_p->operand[i + 1].predicate (op, mode);
@@ -13355,7 +13401,7 @@ ix86_expand_special_args_builtin (const struct builtin_description *d,
machine_mode mode = insn_p->operand[i + 1].mode;
arg = CALL_EXPR_ARG (exp, i + arg_adjust);
- op = expand_normal (arg);
+ op = ix86_expand_integer_cst_argument (arg);
if (i == memory)
{
@@ -15497,7 +15543,7 @@ rdseed_step:
op0 = expand_normal (arg0);
op1 = expand_normal (arg1);
op2 = expand_normal (arg2);
- op3 = expand_normal (arg3);
+ op3 = ix86_expand_integer_cst_argument (arg3);
op4 = expand_normal (arg4);
/* Note the arg order is different from the operand order. */
mode0 = insn_data[icode].operand[1].mode;
@@ -15712,7 +15758,7 @@ rdseed_step:
arg3 = CALL_EXPR_ARG (exp, 3);
arg4 = CALL_EXPR_ARG (exp, 4);
op0 = expand_normal (arg0);
- op1 = expand_normal (arg1);
+ op1 = ix86_expand_integer_cst_argument (arg1);
op2 = expand_normal (arg2);
op3 = expand_normal (arg3);
op4 = expand_normal (arg4);
--
2.47.0
