Patchwork Fix expansion of comparisons into signed type with 1-bit precision (PR middle-end/48973)

login
register
mail settings
Submitter Jakub Jelinek
Date May 12, 2011, 2:36 p.m.
Message ID <20110512143631.GI17079@tyan-ft48-01.lab.bos.redhat.com>
Download mbox | patch
Permalink /patch/95321/
State New
Headers show

Comments

Jakub Jelinek - May 12, 2011, 2:36 p.m.
Hi!

The read from a 1-bit signed bitfield initialized by a comparison
is optimized into the comparison, which has that 1-bit signed bitfield
comparison.  Unfortunately that is still expanded as setting the result
to 0 resp. 1 instead of this case 0 resp. -1 QImode pseudo, which is then
sign extended into SImode for the comparison.

Fixed by special casing expansion in that case.  I think it is rare enough
we can just ignore the fold_single_bit_test optimization in that case,
rather than having two versions thereof.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/4.6?

2011-05-12  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/48973
	* expr.c (expand_expr_real_2) <case LT_EXPR>: If do_store_flag
	failed and the comparison has a single bit signed type, use
	constm1_rtx instead of const1_rtx for true value.
	(do_store_flag): If ops->type is single bit signed type, disable
	signel bit test optimization and pass -1 instead of 1 as last
	parameter to emit_store_flag_force.

	* gcc.c-torture/execute/pr48973-1.c: New test.
	* gcc.c-torture/execute/pr48973-2.c: New test.


	Jakub
H.J. Lu - May 23, 2011, 1:06 p.m.
On Thu, May 12, 2011 at 7:36 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> The read from a 1-bit signed bitfield initialized by a comparison
> is optimized into the comparison, which has that 1-bit signed bitfield
> comparison.  Unfortunately that is still expanded as setting the result
> to 0 resp. 1 instead of this case 0 resp. -1 QImode pseudo, which is then
> sign extended into SImode for the comparison.
>
> Fixed by special casing expansion in that case.  I think it is rare enough
> we can just ignore the fold_single_bit_test optimization in that case,
> rather than having two versions thereof.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/4.6?
>
> 2011-05-12  Jakub Jelinek  <jakub@redhat.com>
>
>        PR middle-end/48973
>        * expr.c (expand_expr_real_2) <case LT_EXPR>: If do_store_flag
>        failed and the comparison has a single bit signed type, use
>        constm1_rtx instead of const1_rtx for true value.
>        (do_store_flag): If ops->type is single bit signed type, disable
>        signel bit test optimization and pass -1 instead of 1 as last
>        parameter to emit_store_flag_force.
>
>        * gcc.c-torture/execute/pr48973-1.c: New test.
>        * gcc.c-torture/execute/pr48973-2.c: New test.
>

This caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49123

H.J.
Jakub Jelinek - May 23, 2011, 1:21 p.m.
On Mon, May 23, 2011 at 06:06:55AM -0700, H.J. Lu wrote:
> On Thu, May 12, 2011 at 7:36 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> > The read from a 1-bit signed bitfield initialized by a comparison
> > is optimized into the comparison, which has that 1-bit signed bitfield
> > comparison.  Unfortunately that is still expanded as setting the result
> > to 0 resp. 1 instead of this case 0 resp. -1 QImode pseudo, which is then
> > sign extended into SImode for the comparison.
> >
> > Fixed by special casing expansion in that case.  I think it is rare enough
> > we can just ignore the fold_single_bit_test optimization in that case,
> > rather than having two versions thereof.
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/4.6?
> >
> > 2011-05-12  Jakub Jelinek  <jakub@redhat.com>
> >
> >        PR middle-end/48973
> >        * expr.c (expand_expr_real_2) <case LT_EXPR>: If do_store_flag
> >        failed and the comparison has a single bit signed type, use
> >        constm1_rtx instead of const1_rtx for true value.
> >        (do_store_flag): If ops->type is single bit signed type, disable
> >        signel bit test optimization and pass -1 instead of 1 as last
> >        parameter to emit_store_flag_force.
> >
> >        * gcc.c-torture/execute/pr48973-1.c: New test.
> >        * gcc.c-torture/execute/pr48973-2.c: New test.
> >
> 
> This caused:
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49123

Can't reproduce, all tests succeed just fine, both on the trunk and in 4.6,
both x86_64-linux {-m32,-m64} and 32-bit HWI i686-linux, including -flto.

	Jakub
H.J. Lu - May 23, 2011, 1:39 p.m.
On Mon, May 23, 2011 at 6:21 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Mon, May 23, 2011 at 06:06:55AM -0700, H.J. Lu wrote:
>> On Thu, May 12, 2011 at 7:36 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>> > The read from a 1-bit signed bitfield initialized by a comparison
>> > is optimized into the comparison, which has that 1-bit signed bitfield
>> > comparison.  Unfortunately that is still expanded as setting the result
>> > to 0 resp. 1 instead of this case 0 resp. -1 QImode pseudo, which is then
>> > sign extended into SImode for the comparison.
>> >
>> > Fixed by special casing expansion in that case.  I think it is rare enough
>> > we can just ignore the fold_single_bit_test optimization in that case,
>> > rather than having two versions thereof.
>> >
>> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/4.6?
>> >
>> > 2011-05-12  Jakub Jelinek  <jakub@redhat.com>
>> >
>> >        PR middle-end/48973
>> >        * expr.c (expand_expr_real_2) <case LT_EXPR>: If do_store_flag
>> >        failed and the comparison has a single bit signed type, use
>> >        constm1_rtx instead of const1_rtx for true value.
>> >        (do_store_flag): If ops->type is single bit signed type, disable
>> >        signel bit test optimization and pass -1 instead of 1 as last
>> >        parameter to emit_store_flag_force.
>> >
>> >        * gcc.c-torture/execute/pr48973-1.c: New test.
>> >        * gcc.c-torture/execute/pr48973-2.c: New test.
>> >
>>
>> This caused:
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49123
>
> Can't reproduce, all tests succeed just fine, both on the trunk and in 4.6,
> both x86_64-linux {-m32,-m64} and 32-bit HWI i686-linux, including -flto.
>

Does your linker support LTO?
Jeff Law - May 23, 2011, 4:17 p.m.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 05/12/11 08:36, Jakub Jelinek wrote:
> Hi!
> 
> The read from a 1-bit signed bitfield initialized by a comparison
> is optimized into the comparison, which has that 1-bit signed bitfield
> comparison.  Unfortunately that is still expanded as setting the result
> to 0 resp. 1 instead of this case 0 resp. -1 QImode pseudo, which is then
> sign extended into SImode for the comparison.
> 
> Fixed by special casing expansion in that case.  I think it is rare enough
> we can just ignore the fold_single_bit_test optimization in that case,
> rather than having two versions thereof.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/4.6?
> 
> 2011-05-12  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/48973
> 	* expr.c (expand_expr_real_2) <case LT_EXPR>: If do_store_flag
> 	failed and the comparison has a single bit signed type, use
> 	constm1_rtx instead of const1_rtx for true value.
> 	(do_store_flag): If ops->type is single bit signed type, disable
> 	signel bit test optimization and pass -1 instead of 1 as last
> 	parameter to emit_store_flag_force.
> 
> 	* gcc.c-torture/execute/pr48973-1.c: New test.
> 	* gcc.c-torture/execute/pr48973-2.c: New test.
OK.
jeff
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJN2oiJAAoJEBRtltQi2kC7M8sH/2BN1N5CH8ts1+qxjVT2LaRm
HJIeSqO9mC+PCLpP5VW1POCfUi72r5gqXokrYSe+LFRoRriFMwpoKCjR2dcYu1yz
XKkSQo3rb/+FENKITmqn93z6MCRTj97b5oVTG3T8Uf/xIPBc8r0HHqCjOcpud1Eh
YuZNqSVGA8gaxBfWq9HeOyumSbeXp897/+k55uX5L2jhp9ja0Oynf6TCg9gZjEhU
avyRR09o2E+wldaeTxI5H45Y7WDgcSBtYDOLP2SGjrcicf0BwdACmlyWo8KK6ydu
nmS+R6EOO+8WB+W0l8BP9tG82tUVHsQj/Rhy6I1XlByGVHzZdha5BqXW9rnKoLw=
=0wsi
-----END PGP SIGNATURE-----

Patch

--- gcc/expr.c.jj	2011-05-11 19:39:04.000000000 +0200
+++ gcc/expr.c	2011-05-12 10:46:52.000000000 +0200
@@ -8105,7 +8105,10 @@  expand_expr_real_2 (sepops ops, rtx targ
       op1 = gen_label_rtx ();
       jumpifnot_1 (code, treeop0, treeop1, op1, -1);
 
-      emit_move_insn (target, const1_rtx);
+      if (TYPE_PRECISION (type) == 1 && !TYPE_UNSIGNED (type))
+	emit_move_insn (target, constm1_rtx);
+      else
+	emit_move_insn (target, const1_rtx);
 
       emit_label (op1);
       return target;
@@ -10050,7 +10053,8 @@  do_store_flag (sepops ops, rtx target, e
 
   if ((code == NE || code == EQ)
       && TREE_CODE (arg0) == BIT_AND_EXPR && integer_zerop (arg1)
-      && integer_pow2p (TREE_OPERAND (arg0, 1)))
+      && integer_pow2p (TREE_OPERAND (arg0, 1))
+      && (TYPE_PRECISION (ops->type) != 1 || TYPE_UNSIGNED (ops->type)))
     {
       tree type = lang_hooks.types.type_for_mode (mode, unsignedp);
       return expand_expr (fold_single_bit_test (loc,
@@ -10070,7 +10074,9 @@  do_store_flag (sepops ops, rtx target, e
 
   /* Try a cstore if possible.  */
   return emit_store_flag_force (target, code, op0, op1,
-			        operand_mode, unsignedp, 1);
+				operand_mode, unsignedp,
+				(TYPE_PRECISION (ops->type) == 1
+				 && !TYPE_UNSIGNED (ops->type)) ? -1 : 1);
 }
 
 
--- gcc/testsuite/gcc.c-torture/execute/pr48973-1.c.jj	2011-05-12 10:53:49.000000000 +0200
+++ gcc/testsuite/gcc.c-torture/execute/pr48973-1.c	2011-05-12 10:53:12.000000000 +0200
@@ -0,0 +1,20 @@ 
+/* PR middle-end/48973 */
+
+extern void abort (void);
+struct S { int f : 1; } s;
+int v = -1;
+
+void
+foo (unsigned int x)
+{
+  if (x != -1U)
+    abort ();
+}
+
+int
+main ()
+{
+  s.f = (v & 1) > 0;
+  foo (s.f);
+  return 0;
+}
--- gcc/testsuite/gcc.c-torture/execute/pr48973-2.c.jj	2011-05-12 10:53:52.000000000 +0200
+++ gcc/testsuite/gcc.c-torture/execute/pr48973-2.c	2011-05-12 10:53:37.000000000 +0200
@@ -0,0 +1,14 @@ 
+/* PR middle-end/48973 */
+
+extern void abort (void);
+struct S { int f : 1; } s;
+int v = -1;
+
+int
+main ()
+{
+  s.f = v < 0;
+  if ((unsigned int) s.f != -1U)
+    abort ();
+  return 0;
+}