* [PATCH] expand: Fix up expand_doubleword_mod on 32-bit targets [PR98229]
@ 2020-12-11 10:16 Jakub Jelinek
2020-12-11 10:34 ` Richard Biener
0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2020-12-11 10:16 UTC (permalink / raw)
To: Richard Biener, Jeff Law; +Cc: gcc-patches
Hi!
As the testcase shows, for 32-bit word size we can end up with op1
up to 0xffffffff (0x100000000 % 0xffffffff == 1 and so we use bit == 32
for that), but the CONST_INT we got from caller is for DImode in that case
and not valid for SImode operations.
The following patch canonicalizes the two spots where the constant needs
canonicalization.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2020-12-11 Jakub Jelinek <jakub@redhat.com>
PR rtl-optimization/98229
* optabs.c (expand_doubleword_mod): Canonicalize op1 and
1 - INTVAL (op1) as word_mode constants when used in
word_mode arithmetics.
* gcc.c-torture/compile/pr98229.c: New test.
--- gcc/optabs.c.jj 2020-12-02 11:40:47.574338088 +0100
+++ gcc/optabs.c 2020-12-10 19:00:46.151434474 +0100
@@ -1081,7 +1081,8 @@ expand_doubleword_mod (machine_mode mode
return NULL_RTX;
}
}
- rtx remainder = expand_divmod (1, TRUNC_MOD_EXPR, word_mode, sum, op1,
+ rtx op1w = GEN_INT (trunc_int_for_mode (INTVAL (op1), word_mode));
+ rtx remainder = expand_divmod (1, TRUNC_MOD_EXPR, word_mode, sum, op1w,
NULL_RTX, 1, OPTAB_DIRECT);
if (remainder == NULL_RTX)
return NULL_RTX;
@@ -1098,8 +1099,9 @@ expand_doubleword_mod (machine_mode mode
if (mask == NULL_RTX)
return NULL_RTX;
}
- mask = expand_simple_binop (word_mode, AND, mask,
- GEN_INT (1 - INTVAL (op1)),
+ rtx oneminusop1
+ = GEN_INT (trunc_int_for_mode (1 - INTVAL (op1), word_mode));
+ mask = expand_simple_binop (word_mode, AND, mask, oneminusop1,
NULL_RTX, 1, OPTAB_DIRECT);
if (mask == NULL_RTX)
return NULL_RTX;
--- gcc/testsuite/gcc.c-torture/compile/pr98229.c.jj 2020-12-10 19:08:25.502328354 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr98229.c 2020-12-10 19:08:10.062499943 +0100
@@ -0,0 +1,7 @@
+/* PR rtl-optimization/98229 */
+
+unsigned long long
+foo (unsigned long long x)
+{
+ return x % ~0U;
+}
Jakub
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] expand: Fix up expand_doubleword_mod on 32-bit targets [PR98229]
2020-12-11 10:16 [PATCH] expand: Fix up expand_doubleword_mod on 32-bit targets [PR98229] Jakub Jelinek
@ 2020-12-11 10:34 ` Richard Biener
2020-12-11 10:57 ` [PATCH] expand, v2: " Jakub Jelinek
0 siblings, 1 reply; 4+ messages in thread
From: Richard Biener @ 2020-12-11 10:34 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Jeff Law, gcc-patches
On Fri, 11 Dec 2020, Jakub Jelinek wrote:
> Hi!
>
> As the testcase shows, for 32-bit word size we can end up with op1
> up to 0xffffffff (0x100000000 % 0xffffffff == 1 and so we use bit == 32
> for that), but the CONST_INT we got from caller is for DImode in that case
> and not valid for SImode operations.
>
> The following patch canonicalizes the two spots where the constant needs
> canonicalization.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Shouldn't we simply use gen_int_mode?
> 2020-12-11 Jakub Jelinek <jakub@redhat.com>
>
> PR rtl-optimization/98229
> * optabs.c (expand_doubleword_mod): Canonicalize op1 and
> 1 - INTVAL (op1) as word_mode constants when used in
> word_mode arithmetics.
>
> * gcc.c-torture/compile/pr98229.c: New test.
>
> --- gcc/optabs.c.jj 2020-12-02 11:40:47.574338088 +0100
> +++ gcc/optabs.c 2020-12-10 19:00:46.151434474 +0100
> @@ -1081,7 +1081,8 @@ expand_doubleword_mod (machine_mode mode
> return NULL_RTX;
> }
> }
> - rtx remainder = expand_divmod (1, TRUNC_MOD_EXPR, word_mode, sum, op1,
> + rtx op1w = GEN_INT (trunc_int_for_mode (INTVAL (op1), word_mode));
> + rtx remainder = expand_divmod (1, TRUNC_MOD_EXPR, word_mode, sum, op1w,
> NULL_RTX, 1, OPTAB_DIRECT);
> if (remainder == NULL_RTX)
> return NULL_RTX;
> @@ -1098,8 +1099,9 @@ expand_doubleword_mod (machine_mode mode
> if (mask == NULL_RTX)
> return NULL_RTX;
> }
> - mask = expand_simple_binop (word_mode, AND, mask,
> - GEN_INT (1 - INTVAL (op1)),
> + rtx oneminusop1
> + = GEN_INT (trunc_int_for_mode (1 - INTVAL (op1), word_mode));
> + mask = expand_simple_binop (word_mode, AND, mask, oneminusop1,
> NULL_RTX, 1, OPTAB_DIRECT);
> if (mask == NULL_RTX)
> return NULL_RTX;
> --- gcc/testsuite/gcc.c-torture/compile/pr98229.c.jj 2020-12-10 19:08:25.502328354 +0100
> +++ gcc/testsuite/gcc.c-torture/compile/pr98229.c 2020-12-10 19:08:10.062499943 +0100
> @@ -0,0 +1,7 @@
> +/* PR rtl-optimization/98229 */
> +
> +unsigned long long
> +foo (unsigned long long x)
> +{
> + return x % ~0U;
> +}
>
> Jakub
>
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] expand, v2: Fix up expand_doubleword_mod on 32-bit targets [PR98229]
2020-12-11 10:34 ` Richard Biener
@ 2020-12-11 10:57 ` Jakub Jelinek
2020-12-11 11:39 ` Richard Biener
0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2020-12-11 10:57 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
On Fri, Dec 11, 2020 at 11:34:01AM +0100, Richard Biener wrote:
> Shouldn't we simply use gen_int_mode?
You're right, that is shorter. So like this?
BTW, looking at gen_int_mode, wouldn't it be better to add an overload
for HOST_WIDE_INT first argument so that it wouldn't need to go through
poly_int? Or would that then be ambiguous with int, unsigned etc.
arguments?
2020-12-10 Jakub Jelinek <jakub@redhat.com>
PR rtl-optimization/98229
* optabs.c (expand_doubleword_mod): Canonicalize op1 and
1 - INTVAL (op1) as word_mode constants when used in
word_mode arithmetics.
* gcc.c-torture/compile/pr98229.c: New test.
--- gcc/optabs.c.jj 2020-12-11 00:30:41.892272376 +0100
+++ gcc/optabs.c 2020-12-11 11:51:15.902855312 +0100
@@ -1081,7 +1081,8 @@ expand_doubleword_mod (machine_mode mode
return NULL_RTX;
}
}
- rtx remainder = expand_divmod (1, TRUNC_MOD_EXPR, word_mode, sum, op1,
+ rtx remainder = expand_divmod (1, TRUNC_MOD_EXPR, word_mode, sum,
+ gen_int_mode (INTVAL (op1), word_mode),
NULL_RTX, 1, OPTAB_DIRECT);
if (remainder == NULL_RTX)
return NULL_RTX;
@@ -1099,7 +1100,8 @@ expand_doubleword_mod (machine_mode mode
return NULL_RTX;
}
mask = expand_simple_binop (word_mode, AND, mask,
- GEN_INT (1 - INTVAL (op1)),
+ gen_int_mode (1 - INTVAL (op1),
+ word_mode),
NULL_RTX, 1, OPTAB_DIRECT);
if (mask == NULL_RTX)
return NULL_RTX;
--- gcc/testsuite/gcc.c-torture/compile/pr98229.c.jj 2020-12-11 11:49:34.518993120 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr98229.c 2020-12-11 11:49:34.517993131 +0100
@@ -0,0 +1,7 @@
+/* PR rtl-optimization/98229 */
+
+unsigned long long
+foo (unsigned long long x)
+{
+ return x % ~0U;
+}
Jakub
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] expand, v2: Fix up expand_doubleword_mod on 32-bit targets [PR98229]
2020-12-11 10:57 ` [PATCH] expand, v2: " Jakub Jelinek
@ 2020-12-11 11:39 ` Richard Biener
0 siblings, 0 replies; 4+ messages in thread
From: Richard Biener @ 2020-12-11 11:39 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On Fri, 11 Dec 2020, Jakub Jelinek wrote:
> On Fri, Dec 11, 2020 at 11:34:01AM +0100, Richard Biener wrote:
> > Shouldn't we simply use gen_int_mode?
>
> You're right, that is shorter. So like this?
Yes, OK.
> BTW, looking at gen_int_mode, wouldn't it be better to add an overload
> for HOST_WIDE_INT first argument so that it wouldn't need to go through
> poly_int? Or would that then be ambiguous with int, unsigned etc.
> arguments?
Not sure but marshalling as poly-int64 should be cheap.
Richard.
> 2020-12-10 Jakub Jelinek <jakub@redhat.com>
>
> PR rtl-optimization/98229
> * optabs.c (expand_doubleword_mod): Canonicalize op1 and
> 1 - INTVAL (op1) as word_mode constants when used in
> word_mode arithmetics.
>
> * gcc.c-torture/compile/pr98229.c: New test.
>
> --- gcc/optabs.c.jj 2020-12-11 00:30:41.892272376 +0100
> +++ gcc/optabs.c 2020-12-11 11:51:15.902855312 +0100
> @@ -1081,7 +1081,8 @@ expand_doubleword_mod (machine_mode mode
> return NULL_RTX;
> }
> }
> - rtx remainder = expand_divmod (1, TRUNC_MOD_EXPR, word_mode, sum, op1,
> + rtx remainder = expand_divmod (1, TRUNC_MOD_EXPR, word_mode, sum,
> + gen_int_mode (INTVAL (op1), word_mode),
> NULL_RTX, 1, OPTAB_DIRECT);
> if (remainder == NULL_RTX)
> return NULL_RTX;
> @@ -1099,7 +1100,8 @@ expand_doubleword_mod (machine_mode mode
> return NULL_RTX;
> }
> mask = expand_simple_binop (word_mode, AND, mask,
> - GEN_INT (1 - INTVAL (op1)),
> + gen_int_mode (1 - INTVAL (op1),
> + word_mode),
> NULL_RTX, 1, OPTAB_DIRECT);
> if (mask == NULL_RTX)
> return NULL_RTX;
> --- gcc/testsuite/gcc.c-torture/compile/pr98229.c.jj 2020-12-11 11:49:34.518993120 +0100
> +++ gcc/testsuite/gcc.c-torture/compile/pr98229.c 2020-12-11 11:49:34.517993131 +0100
> @@ -0,0 +1,7 @@
> +/* PR rtl-optimization/98229 */
> +
> +unsigned long long
> +foo (unsigned long long x)
> +{
> + return x % ~0U;
> +}
>
> Jakub
>
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-12-11 11:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-11 10:16 [PATCH] expand: Fix up expand_doubleword_mod on 32-bit targets [PR98229] Jakub Jelinek
2020-12-11 10:34 ` Richard Biener
2020-12-11 10:57 ` [PATCH] expand, v2: " Jakub Jelinek
2020-12-11 11:39 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).