Re: Fix PR rtl-optimization/89795
On Wed, Sep 11, 2019 at 02:18:36PM +0200, Eric Botcazou wrote: > > What does it mean for > > (set (reg:QI) (const_int -128)) > > for example? This is the canonical form of moving positive 128 into a > > register as well, of course. > > Nothing, since word_register_operation_p returns false on CONST_INTs . > > > W_R_O needs many more restrictions if it can work at all, but those aren't > > documented. > > The problematic issue is more basic and pertains to mere SUBREGs. I'd say this is more basic, but heh... It's an example. If it isn't documented what W_R_O really *does*, how can we ever hope to make it works stably? Segher
Re: Fix PR rtl-optimization/89795
> What does it mean for > (set (reg:QI) (const_int -128)) > for example? This is the canonical form of moving positive 128 into a > register as well, of course. Nothing, since word_register_operation_p returns false on CONST_INTs . > W_R_O needs many more restrictions if it can work at all, but those aren't > documented. The problematic issue is more basic and pertains to mere SUBREGs. -- Eric Botcazou
Re: Fix PR rtl-optimization/89795
Hi Eric, On Wed, Sep 11, 2019 at 12:39:51PM +0200, Eric Botcazou wrote: > This fixes a wrong code generation on the ARM in very peculiar circumstances > ( > -O2 -fno-dce -fno-forward-propagate -fno-sched-pressure) present on all > active > branches in the form of a missing zero-extension. It turns out that not all > parts of the RTL middle-end agree on the assumptions valid on RISC machines > as > encoded by WORD_REGISTER_OPERATIONS, so the nonzero_bits machinery needs to > be > more conservative with them. As witnessed by the added XFAIL, this will add > back some redundant zero-extensions on WORD_REGISTER_OPERATIONS targets. But what are those assumptions, _exactly_? It's not clear to me. WORD_REGISTER_OPERATIONS Define this macro if operations between registers with integral mode smaller than a word are always performed on the entire register. Most RISC machines have this property and most CISC machines do not. What does it mean for (set (reg:QI) (const_int -128)) for example? This is the canonical form of moving positive 128 into a register as well, of course. On a target with 64-bit registers, does it mean that (set (reg:SI d) (mult:SI (reg:SI a) (reg:SI b))) is actually done as (set (reg:DI d) (mult:DI (reg:DI a) (reg:DI b))) ? I can't think of any hardware where that is true. W_R_O needs many more restrictions if it can work at all, but those aren't documented. Segher
Re: Fix PR rtl-optimization/89795
On Wed, Sep 11, 2019 at 12:39:51PM +0200, Eric Botcazou wrote:
> This fixes a wrong code generation on the ARM in very peculiar circumstances
> (
> -O2 -fno-dce -fno-forward-propagate -fno-sched-pressure) present on all
> active
> branches in the form of a missing zero-extension. It turns out that not all
> parts of the RTL middle-end agree on the assumptions valid on RISC machines
> as
> encoded by WORD_REGISTER_OPERATIONS, so the nonzero_bits machinery needs to
> be
> more conservative with them. As witnessed by the added XFAIL, this will add
> back some redundant zero-extensions on WORD_REGISTER_OPERATIONS targets.
>
> Tested on SPARC/Solaris and SPARC64/Linux, applied on all active branches.
Thanks. I've added testcase for the various PRs that are fixed by this
change, verified on armv4 and riscv64 on the first and last testcases that
the rtlanal.c change makes the desired change in the assembly, plus tested
on x86_64-linux -m32/-m64, committed to trunk as obvious.
2019-09-11 Jakub Jelinek
PR rtl-optimization/89435
PR rtl-optimization/89795
PR rtl-optimization/91720
* gcc.dg/pr89435.c: New test.
* gcc.dg/pr89795.c: New test.
* gcc.dg/pr91720.c: New test.
--- gcc/testsuite/gcc.dg/pr89435.c.jj 2019-09-11 13:34:09.630165981 +0200
+++ gcc/testsuite/gcc.dg/pr89435.c 2019-09-11 13:33:37.038664072 +0200
@@ -0,0 +1,21 @@
+/* PR rtl-optimization/89435 */
+/* { dg-do run } */
+/* { dg-options "-O1 -fno-forward-propagate -fno-tree-forwprop -fno-tree-ccp"
} */
+
+unsigned short a;
+unsigned int b, c, d, e, f;
+
+int
+main ()
+{
+#if __CHAR_BIT__ == 8 && __SIZEOF_INT__ == 4
+ unsigned char g = e = __builtin_mul_overflow_p (5, 542624702, 0);
+ d = __builtin_bswap64 (a);
+ b = __builtin_sub_overflow ((unsigned char) -e, (unsigned int) d, );
+ e = __builtin_mul_overflow (b, c, );
+ f = g + e;
+ if (f != 0xff)
+__builtin_abort ();
+#endif
+ return 0;
+}
--- gcc/testsuite/gcc.dg/pr89795.c.jj 2019-09-11 13:05:58.405033868 +0200
+++ gcc/testsuite/gcc.dg/pr89795.c 2019-09-11 13:04:52.508042619 +0200
@@ -0,0 +1,25 @@
+/* PR rtl-optimization/89795 */
+/* { dg-do run } */
+/* { dg-options "-O2 -fno-dce -fno-forward-propagate -fno-sched-pressure" } */
+
+unsigned char a;
+unsigned b, c, d;
+
+int
+main ()
+{
+#if __CHAR_BIT__ == 8
+ unsigned x;
+ int e, f;
+ unsigned char g;
+ e = __builtin_bswap32 (a);
+ f = __builtin_ffs (~(unsigned short) e);
+ a = __builtin_mul_overflow ((unsigned char) 0xf7, f, );
+ a |= __builtin_sub_overflow_p (c, 0, (unsigned char) 0);
+ d = g + b;
+ x = d;
+ if (x != 0xf7)
+__builtin_abort ();
+#endif
+ return 0;
+}
--- gcc/testsuite/gcc.dg/pr91720.c.jj 2019-09-11 13:06:08.909873060 +0200
+++ gcc/testsuite/gcc.dg/pr91720.c 2019-09-11 13:00:50.070754162 +0200
@@ -0,0 +1,22 @@
+/* PR rtl-optimization/91720 */
+/* { dg-do run } */
+/* { dg-options "-Og -fno-forward-propagate -frerun-cse-after-loop
-fno-tree-fre" } */
+
+unsigned a, b;
+
+int
+main ()
+{
+#if __CHAR_BIT__ == 8
+ unsigned c = 1;
+ unsigned long long d = 0;
+ unsigned char e = 0;
+ e = __builtin_sub_overflow (d, e, ) ? 0 : 0x80;
+ e = e << 7 | e >> c;
+ __builtin_memmove (, , 2);
+ b = e;
+ if (b != 0x40)
+__builtin_abort ();
+#endif
+ return 0;
+}
Jakub
Fix PR rtl-optimization/89795
This fixes a wrong code generation on the ARM in very peculiar circumstances (
-O2 -fno-dce -fno-forward-propagate -fno-sched-pressure) present on all active
branches in the form of a missing zero-extension. It turns out that not all
parts of the RTL middle-end agree on the assumptions valid on RISC machines as
encoded by WORD_REGISTER_OPERATIONS, so the nonzero_bits machinery needs to be
more conservative with them. As witnessed by the added XFAIL, this will add
back some redundant zero-extensions on WORD_REGISTER_OPERATIONS targets.
Tested on SPARC/Solaris and SPARC64/Linux, applied on all active branches.
2019-09-11 Eric Botcazou
PR rtl-optimization/89795
* rtlanal.c (nonzero_bits1) : Do not propagate results from
inner REGs to paradoxical SUBREGs if WORD_REGISTER_OPERATIONS is set.
2019-09-11 Eric Botcazou
* gcc.target/sparc/2016-1.c: XFAIL redundant zero-extension test.
--
Eric BotcazouIndex: rtlanal.c
===
--- rtlanal.c (revision 275484)
+++ rtlanal.c (working copy)
@@ -4810,7 +4810,7 @@ nonzero_bits1 (const_rtx x, scalar_int_m
|| ((extend_op = load_extend_op (inner_mode)) == SIGN_EXTEND
? val_signbit_known_set_p (inner_mode, nonzero)
: extend_op != ZERO_EXTEND)
- || (!MEM_P (SUBREG_REG (x)) && !REG_P (SUBREG_REG (x
+ || !MEM_P (SUBREG_REG (x)))
&& xmode_width > inner_width)
nonzero
|= (GET_MODE_MASK (GET_MODE (x)) & ~GET_MODE_MASK (inner_mode));
Index: testsuite/gcc.target/sparc/2016-1.c
===
--- testsuite/gcc.target/sparc/2016-1.c (revision 275484)
+++ testsuite/gcc.target/sparc/2016-1.c (working copy)
@@ -14,4 +14,4 @@ unsigned char ee_isdigit2(unsigned int i
return retval;
}
-/* { dg-final { scan-assembler-not "and\t%" } } */
+/* { dg-final { scan-assembler-not "and\t%" { xfail *-*-* } } } */
