* [PATCH] Disable the bitfield += optimization until it is fixed properly
@ 2004-07-05 20:10 Jakub Jelinek
2004-07-05 20:47 ` Roger Sayle
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Jakub Jelinek @ 2004-07-05 20:10 UTC (permalink / raw)
To: Richard Henderson, Roger Sayle; +Cc: gcc-patches
Hi!
While this bug didn't show up during bootstrap/make check, it shows up while
building e.g. GNOME.
I was completely wrong to use to_rtx's mode, which can very well be BLKmode
or whatever mode the structure with the bitfield has.
On the other side, when to_rtx is a MEM, there is no reason why bitpos +
bitsize can't be bigger than BITS_PER_WORD, it just shouldn't cross the
desired mode's boundary.
Can I commit this quick patch now so that others don't run into this bug,
or should I completely back up all expand_assignment changes and submit
a full new patch when I get it working properly?
One of the attached new testcases gets ICE with current CVS.
2004-07-05 Jakub Jelinek <jakub@redhat.com>
* expr.c (expand_assignment): Disable the bitfield += optimizations.
* gcc.c-torture/execute/20040629-1.c (FIELDS1, FIELDS2): Define to
nothing if not yet defined. Use it in b, c and d type definitions.
* gcc.c-torture/execute/20040705-1.c: New test.
* gcc.c-torture/execute/20040705-2.c: New test.
--- gcc/expr.c.jj 2004-07-03 18:43:54.000000000 +0200
+++ gcc/expr.c 2004-07-05 21:52:25.443377711 +0200
@@ -3854,7 +3854,7 @@ expand_assignment (tree to, tree from, i
MEM_KEEP_ALIAS_SET_P (to_rtx) = 1;
}
- while (mode1 == VOIDmode && !want_value
+ while (0 && mode1 == VOIDmode && !want_value
&& bitpos + bitsize <= BITS_PER_WORD
&& bitsize < BITS_PER_WORD
&& GET_MODE_BITSIZE (GET_MODE (to_rtx)) <= BITS_PER_WORD
--- gcc/testsuite/gcc.c-torture/execute/20040705-1.c.jj 2004-07-05 21:50:46.505754736 +0200
+++ gcc/testsuite/gcc.c-torture/execute/20040705-1.c 2004-07-05 21:50:40.931733733 +0200
@@ -0,0 +1,2 @@
+#define FIELDS1 long long l;
+#include "20040629-1.c"
--- gcc/testsuite/gcc.c-torture/execute/20040705-2.c.jj 2004-07-05 21:50:56.964917735 +0200
+++ gcc/testsuite/gcc.c-torture/execute/20040705-2.c 2004-07-05 21:51:03.660741710 +0200
@@ -0,0 +1,2 @@
+#define FIELDS2 long long l;
+#include "20040629-1.c"
--- gcc/testsuite/gcc.c-torture/execute/20040629-1.c.jj 2004-06-30 12:08:54.000000000 +0200
+++ gcc/testsuite/gcc.c-torture/execute/20040629-1.c 2004-07-05 21:50:05.343984194 +0200
@@ -4,9 +4,16 @@
extern void abort (void);
extern void exit (int);
-struct { unsigned int i : 6, j : 11, k : 15; } b;
-struct { unsigned int i : 5, j : 1, k : 26; } c;
-struct { unsigned int i : 16, j : 8, k : 8; } d;
+#ifndef FIELDS1
+#define FIELDS1
+#endif
+#ifndef FIELDS2
+#define FIELDS2
+#endif
+
+struct { FIELDS1 unsigned int i : 6, j : 11, k : 15; FIELDS2 } b;
+struct { FIELDS1 unsigned int i : 5, j : 1, k : 26; FIELDS2 } c;
+struct { FIELDS1 unsigned int i : 16, j : 8, k : 8; FIELDS2 } d;
unsigned int ret1 (void) { return b.i; }
unsigned int ret2 (void) { return b.j; }
Jakub
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Disable the bitfield += optimization until it is fixed properly
2004-07-05 20:10 [PATCH] Disable the bitfield += optimization until it is fixed properly Jakub Jelinek
@ 2004-07-05 20:47 ` Roger Sayle
2004-07-05 20:56 ` Eric Christopher
2004-07-09 23:22 ` [PATCH] Fix and reenable the bitfield += optimization Jakub Jelinek
2 siblings, 0 replies; 11+ messages in thread
From: Roger Sayle @ 2004-07-05 20:47 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Henderson, gcc-patches
On Mon, 5 Jul 2004, Jakub Jelinek wrote:
> Can I commit this quick patch now so that others don't run into this bug,
> or should I completely back up all expand_assignment changes and submit
> a full new patch when I get it working properly?
Sure, as a temporary solution. Perhaps GET_MODE (to_rtx) != BLKmode
would be sufficient? I'd be cautious of trying to read/write bitfields
in BLKmode MEMs or with bitpos beyond BITS_PER_WORD. I could never
figure out the mess of endian and alignment issues. However, if you
do manage to work out how to do it, GCC's signbit and copysign builtins
could benefit from it, especially for "long double"s. :>
Roger
--
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Disable the bitfield += optimization until it is fixed properly
2004-07-05 20:10 [PATCH] Disable the bitfield += optimization until it is fixed properly Jakub Jelinek
2004-07-05 20:47 ` Roger Sayle
@ 2004-07-05 20:56 ` Eric Christopher
2004-07-09 23:22 ` [PATCH] Fix and reenable the bitfield += optimization Jakub Jelinek
2 siblings, 0 replies; 11+ messages in thread
From: Eric Christopher @ 2004-07-05 20:56 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Henderson, Roger Sayle, gcc-patches
> Can I commit this quick patch now so that others don't run into this bug,
> or should I completely back up all expand_assignment changes and submit
> a full new patch when I get it working properly?
A comment would be useful at the 0 && point.
-eric
--
Eric Christopher <echristo@redhat.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] Fix and reenable the bitfield += optimization
2004-07-05 20:10 [PATCH] Disable the bitfield += optimization until it is fixed properly Jakub Jelinek
2004-07-05 20:47 ` Roger Sayle
2004-07-05 20:56 ` Eric Christopher
@ 2004-07-09 23:22 ` Jakub Jelinek
2004-07-14 17:48 ` [PATCH] Fix and reenable the bitfield += optimization (take 2) Jakub Jelinek
2 siblings, 1 reply; 11+ messages in thread
From: Jakub Jelinek @ 2004-07-09 23:22 UTC (permalink / raw)
To: Richard Henderson, Roger Sayle; +Cc: gcc-patches
Hi!
This patch reenables the bitfield += (or -=) optimization after fixing it.
The problems were:
a) incorrect assumption that GET_MODE (to_rtx) is the right mode
(the patch now uses get_best_mode for MEM_P)
b) missing alias set change to alias set 0, as the memory accesses might
change fields in different alias set
c) missing convert_modes for the += operand
Bootstrapped/regtested on
{i386,x86_64,ia64,ppc,ppc64,s390,s390x}-redhat-linux.
Ok to commit?
The only failure is in the new 20040709-2.c test on ppc64 -m64 at -O2
and -O3, ICE in a routine unrelated to the actualy optimization and the
compiler ICEs no matter whether this patch is in or not.
struct __attribute__((packed)) G { unsigned short i : 1, j : 1, k : 6; unsigned long long l; };
struct G retmeG (struct G x) { return x; }
is the simplified testcase.
I'll try to look at this soon and if I don't manage to do something about it
soon enough, will file a PR and submit a patch to xfail it, ok?
2004-07-10 Jakub Jelinek <jakub@redhat.com>
* expr.c (expand_assignment): Reenable bitfield += optimizations.
Use alias set 0 for memory, do proper mode calculations and adjust
address for memories.
* gcc.c-torture/execute/20040709-1.c: New test.
* gcc.c-torture/execute/20040709-2.c: New test.
--- gcc/expr.c.jj 2004-07-09 13:50:54.000000000 +0200
+++ gcc/expr.c 2004-07-09 13:59:52.087222284 +0200
@@ -3799,18 +3799,16 @@ expand_assignment (tree to, tree from, i
MEM_KEEP_ALIAS_SET_P (to_rtx) = 1;
}
- /* Disabled temporarily. GET_MODE (to_rtx) is often not the right
- mode. */
- while (0 && mode1 == VOIDmode && !want_value
- && bitpos + bitsize <= BITS_PER_WORD
- && bitsize < BITS_PER_WORD
+ /* Optimize bitfld op= val in certain cases. */
+ while (mode1 == VOIDmode && !want_value
+ && bitsize > 0 && bitsize < BITS_PER_WORD
&& GET_MODE_BITSIZE (GET_MODE (to_rtx)) <= BITS_PER_WORD
&& !TREE_SIDE_EFFECTS (to)
&& !TREE_THIS_VOLATILE (to))
{
tree src, op0, op1;
- rtx value;
- HOST_WIDE_INT count = bitpos;
+ rtx value, str_rtx = to_rtx;
+ HOST_WIDE_INT bitpos1 = bitpos;
optab binop;
src = from;
@@ -3826,47 +3824,89 @@ expand_assignment (tree to, tree from, i
if (! operand_equal_p (to, op0, 0))
break;
+ if (MEM_P (str_rtx))
+ {
+ enum machine_mode mode = GET_MODE (str_rtx);
+ HOST_WIDE_INT offset1;
+
+ if (GET_MODE_BITSIZE (mode) == 0
+ || GET_MODE_BITSIZE (mode) > BITS_PER_WORD)
+ mode = word_mode;
+ mode = get_best_mode (bitsize, bitpos1, MEM_ALIGN (str_rtx),
+ mode, 0);
+ if (mode == VOIDmode)
+ break;
+
+ offset1 = bitpos1;
+ bitpos1 %= GET_MODE_BITSIZE (mode);
+ offset1 = (offset1 - bitpos1) / BITS_PER_UNIT;
+ str_rtx = adjust_address (str_rtx, mode, offset1);
+ }
+ else if (!REG_P (str_rtx) && GET_CODE (str_rtx) != SUBREG)
+ break;
+
+ /* If the bit field covers the whole REG/MEM, store_field
+ will likely generate better code. */
+ if (bitsize >= GET_MODE_BITSIZE (GET_MODE (str_rtx)))
+ break;
+
+ /* We can't handle fields split accross multiple entities. */
+ if (bitpos1 + bitsize > GET_MODE_BITSIZE (GET_MODE (str_rtx)))
+ break;
+
if (BYTES_BIG_ENDIAN)
- count = GET_MODE_BITSIZE (GET_MODE (to_rtx)) - bitpos - bitsize;
+ bitpos1 = GET_MODE_BITSIZE (GET_MODE (str_rtx)) - bitpos1
+ - bitsize;
/* Special case some bitfield op= exp. */
switch (TREE_CODE (src))
{
case PLUS_EXPR:
case MINUS_EXPR:
- if (count <= 0)
- break;
-
/* For now, just optimize the case of the topmost bitfield
where we don't need to do any masking and also
1 bit bitfields where xor can be used.
We might win by one instruction for the other bitfields
too if insv/extv instructions aren't used, so that
can be added later. */
- if (count + bitsize != GET_MODE_BITSIZE (GET_MODE (to_rtx))
+ if (bitpos1 + bitsize != GET_MODE_BITSIZE (GET_MODE (str_rtx))
&& (bitsize != 1 || TREE_CODE (op1) != INTEGER_CST))
break;
- value = expand_expr (op1, NULL_RTX, VOIDmode, 0);
+ value = expand_expr (op1, NULL_RTX, GET_MODE (str_rtx), 0);
value = protect_from_queue (value, 0);
- to_rtx = protect_from_queue (to_rtx, 1);
+ str_rtx = protect_from_queue (str_rtx, 1);
+ value = convert_modes (GET_MODE (str_rtx),
+ TYPE_MODE (TREE_TYPE (op1)), value,
+ TYPE_UNSIGNED (TREE_TYPE (op1)));
+
+ /* We may be accessing data outside the field, which means
+ we can alias adjacent data. */
+ if (MEM_P (str_rtx))
+ {
+ str_rtx = shallow_copy_rtx (str_rtx);
+ set_mem_alias_set (str_rtx, 0);
+ set_mem_expr (str_rtx, 0);
+ }
+
binop = TREE_CODE (src) == PLUS_EXPR ? add_optab : sub_optab;
if (bitsize == 1
- && count + bitsize != GET_MODE_BITSIZE (GET_MODE (to_rtx)))
+ && bitpos1 + bitsize != GET_MODE_BITSIZE (GET_MODE (str_rtx)))
{
- value = expand_and (GET_MODE (to_rtx), value, const1_rtx,
+ value = expand_and (GET_MODE (str_rtx), value, const1_rtx,
NULL_RTX);
binop = xor_optab;
}
- value = expand_shift (LSHIFT_EXPR, GET_MODE (to_rtx),
- value, build_int_2 (count, 0),
+ value = expand_shift (LSHIFT_EXPR, GET_MODE (str_rtx),
+ value, build_int_2 (bitpos1, 0),
NULL_RTX, 1);
- result = expand_binop (GET_MODE (to_rtx), binop, to_rtx,
- value, to_rtx, 1, OPTAB_WIDEN);
- if (result != to_rtx)
- emit_move_insn (to_rtx, result);
+ result = expand_binop (GET_MODE (str_rtx), binop, str_rtx,
+ value, str_rtx, 1, OPTAB_WIDEN);
+ if (result != str_rtx)
+ emit_move_insn (str_rtx, result);
free_temp_slots ();
pop_temp_slots ();
return NULL_RTX;
+
default:
break;
}
--- gcc/testsuite/gcc.c-torture/execute/20040709-1.c.jj 2004-01-21 17:12:41.000000000 +0100
+++ gcc/testsuite/gcc.c-torture/execute/20040709-1.c 2004-07-06 11:55:57.000000000 +0200
@@ -0,0 +1,147 @@
+/* Test arithmetics on bitfields. */
+
+extern void abort (void);
+extern void exit (int);
+
+unsigned int
+myrnd (void)
+{
+ static unsigned int s = 1388815473;
+ s *= 1103515245;
+ s += 12345;
+ return (s / 65536) % 2048;
+}
+
+#define T(S) \
+struct S s##S; \
+struct S retme##S (struct S x) \
+{ \
+ return x; \
+} \
+ \
+unsigned int fn1##S (unsigned int x) \
+{ \
+ struct S y = s##S; \
+ y.k += x; \
+ y = retme##S (y); \
+ return y.k; \
+} \
+ \
+unsigned int fn2##S (unsigned int x) \
+{ \
+ struct S y = s##S; \
+ y.k += x; \
+ y.k %= 15; \
+ return y.k; \
+} \
+ \
+unsigned int retit##S (void) \
+{ \
+ return s##S.k; \
+} \
+ \
+unsigned int fn3##S (unsigned int x) \
+{ \
+ s##S.k += x; \
+ return retit##S (); \
+} \
+ \
+void test##S (void) \
+{ \
+ int i; \
+ unsigned int mask, v, a, r; \
+ struct S x; \
+ char *p = (char *) &s##S; \
+ for (i = 0; i < sizeof (s##S); ++i) \
+ *p++ = myrnd (); \
+ if (__builtin_classify_type (s##S.l) == 8) \
+ s##S.l = 5.25; \
+ s##S.k = -1; \
+ mask = s##S.k; \
+ v = myrnd (); \
+ a = myrnd (); \
+ s##S.k = v; \
+ x = s##S; \
+ r = fn1##S (a); \
+ if (x.i != s##S.i || x.j != s##S.j \
+ || x.k != s##S.k || x.l != s##S.l \
+ || ((v + a) & mask) != r) \
+ abort (); \
+ v = myrnd (); \
+ a = myrnd (); \
+ s##S.k = v; \
+ x = s##S; \
+ r = fn2##S (a); \
+ if (x.i != s##S.i || x.j != s##S.j \
+ || x.k != s##S.k || x.l != s##S.l \
+ || ((((v + a) & mask) % 15) & mask) != r) \
+ abort (); \
+ v = myrnd (); \
+ a = myrnd (); \
+ s##S.k = v; \
+ x = s##S; \
+ r = fn3##S (a); \
+ if (x.i != s##S.i || x.j != s##S.j \
+ || s##S.k != r || x.l != s##S.l \
+ || ((v + a) & mask) != r) \
+ abort (); \
+}
+
+struct A { unsigned int i : 6, l : 1, j : 10, k : 15; }; T(A)
+struct B { unsigned int i : 6, j : 11, k : 15; unsigned int l; }; T(B)
+struct C { unsigned int l; unsigned int i : 6, j : 11, k : 15; }; T(C)
+struct D { unsigned long long l : 6, i : 6, j : 23, k : 29; }; T(D)
+struct E { unsigned long long l, i : 12, j : 23, k : 29; }; T(E)
+struct F { unsigned long long i : 12, j : 23, k : 29, l; }; T(F)
+struct G { unsigned int i : 12, j : 13, k : 7; unsigned long long l; }; T(G)
+struct H { unsigned int i : 12, j : 11, k : 9; unsigned long long l; }; T(H)
+struct I { unsigned short i : 1, j : 6, k : 9; unsigned long long l; }; T(I)
+struct J { unsigned short i : 1, j : 8, k : 7; unsigned short l; }; T(J)
+struct K { unsigned int k : 6, l : 1, j : 10, i : 15; }; T(K)
+struct L { unsigned int k : 6, j : 11, i : 15; unsigned int l; }; T(L)
+struct M { unsigned int l; unsigned int k : 6, j : 11, i : 15; }; T(M)
+struct N { unsigned long long l : 6, k : 6, j : 23, i : 29; }; T(N)
+struct O { unsigned long long l, k : 12, j : 23, i : 29; }; T(O)
+struct P { unsigned long long k : 12, j : 23, i : 29, l; }; T(P)
+struct Q { unsigned int k : 12, j : 13, i : 7; unsigned long long l; }; T(Q)
+struct R { unsigned int k : 12, j : 11, i : 9; unsigned long long l; }; T(R)
+struct S { unsigned short k : 1, j : 6, i : 9; unsigned long long l; }; T(S)
+struct T { unsigned short k : 1, j : 8, i : 7; unsigned short l; }; T(T)
+struct U { unsigned short j : 6, k : 1, i : 9; unsigned long long l; }; T(U)
+struct V { unsigned short j : 8, k : 1, i : 7; unsigned short l; }; T(V)
+struct W { long double l; unsigned int k : 12, j : 13, i : 7; }; T(W)
+struct X { unsigned int k : 12, j : 13, i : 7; long double l; }; T(X)
+struct Y { unsigned int k : 12, j : 11, i : 9; long double l; }; T(Y)
+struct Z { long double l; unsigned int j : 13, i : 7, k : 12; }; T(Z)
+
+int
+main (void)
+{
+ testA ();
+ testB ();
+ testC ();
+ testD ();
+ testE ();
+ testF ();
+ testG ();
+ testH ();
+ testI ();
+ testJ ();
+ testK ();
+ testL ();
+ testM ();
+ testN ();
+ testO ();
+ testP ();
+ testQ ();
+ testR ();
+ testS ();
+ testT ();
+ testU ();
+ testV ();
+ testW ();
+ testX ();
+ testY ();
+ testZ ();
+ exit (0);
+}
--- gcc/testsuite/gcc.c-torture/execute/20040709-2.c.jj 2004-01-21 17:12:41.000000000 +0100
+++ gcc/testsuite/gcc.c-torture/execute/20040709-2.c 2004-07-09 10:49:17.000000000 +0200
@@ -0,0 +1,148 @@
+/* Test arithmetics on bitfields. */
+
+extern void abort (void);
+extern void exit (int);
+
+unsigned int
+myrnd (void)
+{
+ static unsigned int s = 1388815473;
+ s *= 1103515245;
+ s += 12345;
+ return (s / 65536) % 2048;
+}
+
+#define T(S) \
+struct S s##S; \
+struct S retme##S (struct S x) \
+{ \
+ return x; \
+} \
+ \
+unsigned int fn1##S (unsigned int x) \
+{ \
+ struct S y = s##S; \
+ y.k += x; \
+ y = retme##S (y); \
+ return y.k; \
+} \
+ \
+unsigned int fn2##S (unsigned int x) \
+{ \
+ struct S y = s##S; \
+ y.k += x; \
+ y.k %= 15; \
+ return y.k; \
+} \
+ \
+unsigned int retit##S (void) \
+{ \
+ return s##S.k; \
+} \
+ \
+unsigned int fn3##S (unsigned int x) \
+{ \
+ s##S.k += x; \
+ return retit##S (); \
+} \
+ \
+void test##S (void) \
+{ \
+ int i; \
+ unsigned int mask, v, a, r; \
+ struct S x; \
+ char *p = (char *) &s##S; \
+ for (i = 0; i < sizeof (s##S); ++i) \
+ *p++ = myrnd (); \
+ if (__builtin_classify_type (s##S.l) == 8) \
+ s##S.l = 5.25; \
+ s##S.k = -1; \
+ mask = s##S.k; \
+ v = myrnd (); \
+ a = myrnd (); \
+ s##S.k = v; \
+ x = s##S; \
+ r = fn1##S (a); \
+ if (x.i != s##S.i || x.j != s##S.j \
+ || x.k != s##S.k || x.l != s##S.l \
+ || ((v + a) & mask) != r) \
+ abort (); \
+ v = myrnd (); \
+ a = myrnd (); \
+ s##S.k = v; \
+ x = s##S; \
+ r = fn2##S (a); \
+ if (x.i != s##S.i || x.j != s##S.j \
+ || x.k != s##S.k || x.l != s##S.l \
+ || ((((v + a) & mask) % 15) & mask) != r) \
+ abort (); \
+ v = myrnd (); \
+ a = myrnd (); \
+ s##S.k = v; \
+ x = s##S; \
+ r = fn3##S (a); \
+ if (x.i != s##S.i || x.j != s##S.j \
+ || s##S.k != r || x.l != s##S.l \
+ || ((v + a) & mask) != r) \
+ abort (); \
+}
+
+#define pck __attribute__((packed))
+struct pck A { unsigned short i : 1, l : 1, j : 3, k : 11; }; T(A)
+struct pck B { unsigned short i : 4, j : 1, k : 11; unsigned int l; }; T(B)
+struct pck C { unsigned int l; unsigned short i : 4, j : 1, k : 11; }; T(C)
+struct pck D { unsigned long long l : 6, i : 6, j : 23, k : 29; }; T(D)
+struct pck E { unsigned long long l, i : 12, j : 23, k : 29; }; T(E)
+struct pck F { unsigned long long i : 12, j : 23, k : 29, l; }; T(F)
+struct pck G { unsigned short i : 1, j : 1, k : 6; unsigned long long l; }; T(G)
+struct pck H { unsigned short i : 6, j : 2, k : 8; unsigned long long l; }; T(H)
+struct pck I { unsigned short i : 1, j : 6, k : 1; unsigned long long l; }; T(I)
+struct pck J { unsigned short i : 1, j : 8, k : 7; unsigned short l; }; T(J)
+struct pck K { unsigned int k : 6, l : 1, j : 10, i : 15; }; T(K)
+struct pck L { unsigned int k : 6, j : 11, i : 15; unsigned int l; }; T(L)
+struct pck M { unsigned int l; unsigned short k : 6, j : 11, i : 15; }; T(M)
+struct pck N { unsigned long long l : 6, k : 6, j : 23, i : 29; }; T(N)
+struct pck O { unsigned long long l, k : 12, j : 23, i : 29; }; T(O)
+struct pck P { unsigned long long k : 12, j : 23, i : 29, l; }; T(P)
+struct pck Q { unsigned short k : 12, j : 1, i : 3; unsigned long long l; }; T(Q)
+struct pck R { unsigned short k : 2, j : 11, i : 3; unsigned long long l; }; T(R)
+struct pck S { unsigned short k : 1, j : 6, i : 9; unsigned long long l; }; T(S)
+struct pck T { unsigned short k : 1, j : 8, i : 7; unsigned short l; }; T(T)
+struct pck U { unsigned short j : 6, k : 1, i : 9; unsigned long long l; }; T(U)
+struct pck V { unsigned short j : 8, k : 1, i : 7; unsigned short l; }; T(V)
+struct pck W { long double l; unsigned int k : 12, j : 13, i : 7; }; T(W)
+struct pck X { unsigned int k : 12, j : 13, i : 7; long double l; }; T(X)
+struct pck Y { unsigned int k : 12, j : 11, i : 9; long double l; }; T(Y)
+struct pck Z { long double l; unsigned int j : 13, i : 7, k : 12; }; T(Z)
+
+int
+main (void)
+{
+ testA ();
+ testB ();
+ testC ();
+ testD ();
+ testE ();
+ testF ();
+ testG ();
+ testH ();
+ testI ();
+ testJ ();
+ testK ();
+ testL ();
+ testM ();
+ testN ();
+ testO ();
+ testP ();
+ testQ ();
+ testR ();
+ testS ();
+ testT ();
+ testU ();
+ testV ();
+ testW ();
+ testX ();
+ testY ();
+ testZ ();
+ exit (0);
+}
Jakub
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] Fix and reenable the bitfield += optimization (take 2)
2004-07-09 23:22 ` [PATCH] Fix and reenable the bitfield += optimization Jakub Jelinek
@ 2004-07-14 17:48 ` Jakub Jelinek
2004-07-14 22:00 ` Roger Sayle
2004-07-15 0:39 ` Richard Henderson
0 siblings, 2 replies; 11+ messages in thread
From: Jakub Jelinek @ 2004-07-14 17:48 UTC (permalink / raw)
To: Richard Henderson, Roger Sayle; +Cc: gcc-patches
Hi!
This patch reenables the bitfield += (or -=) optimization after fixing it.
The problems were:
a) incorrect assumption that GET_MODE (to_rtx) is the right mode
(the patch now uses get_best_mode for MEM_P)
b) missing alias set change to alias set 0, as the memory accesses might
change fields in different alias set
c) missing convert_modes for the += operand
Bootstrapped/regtested on
{i386,x86_64,ia64,ppc,ppc64,s390,s390x}-redhat-linux.
I haven't managed to write an .x file for the unrelated ppc64 unaligned
handling bug (PR target/16491) which shows up in the new 20040709-2.c
test (how do I XFAIL {ppc64,powerpc64}-*-* 64-bit compilation at -O2/-O3?
Given that ppc* can be configured both as defaulting to -m32 or -m64,
I guess tcl would need to run the compiler once more, see what macros it
defines in -dD -E -xc /dev/null and act based on it),
so instead I added a hack like:
#ifdef __powerpc64__
/* Temporary hack for broken PPC64 unaligned handling PR rtl-optimization/13674 */
# define pck
#else
# define pck __attribute__((packed))
#endif
and would add a comment to that PR so that as soon as it is fixed this
testcase hack should be removed.
Ok to commit?
2004-07-14 Jakub Jelinek <jakub@redhat.com>
* expr.c (expand_assignment): Reenable bitfield += optimizations.
Use alias set 0 for memory, do proper mode calculations and adjust
address for memories.
* gcc.c-torture/execute/20040709-1.c: New test.
* gcc.c-torture/execute/20040709-2.c: New test.
--- gcc/expr.c.jj 2004-07-09 13:50:54.000000000 +0200
+++ gcc/expr.c 2004-07-09 13:59:52.087222284 +0200
@@ -3799,18 +3799,16 @@ expand_assignment (tree to, tree from, i
MEM_KEEP_ALIAS_SET_P (to_rtx) = 1;
}
- /* Disabled temporarily. GET_MODE (to_rtx) is often not the right
- mode. */
- while (0 && mode1 == VOIDmode && !want_value
- && bitpos + bitsize <= BITS_PER_WORD
- && bitsize < BITS_PER_WORD
+ /* Optimize bitfld op= val in certain cases. */
+ while (mode1 == VOIDmode && !want_value
+ && bitsize > 0 && bitsize < BITS_PER_WORD
&& GET_MODE_BITSIZE (GET_MODE (to_rtx)) <= BITS_PER_WORD
&& !TREE_SIDE_EFFECTS (to)
&& !TREE_THIS_VOLATILE (to))
{
tree src, op0, op1;
- rtx value;
- HOST_WIDE_INT count = bitpos;
+ rtx value, str_rtx = to_rtx;
+ HOST_WIDE_INT bitpos1 = bitpos;
optab binop;
src = from;
@@ -3826,47 +3824,89 @@ expand_assignment (tree to, tree from, i
if (! operand_equal_p (to, op0, 0))
break;
+ if (MEM_P (str_rtx))
+ {
+ enum machine_mode mode = GET_MODE (str_rtx);
+ HOST_WIDE_INT offset1;
+
+ if (GET_MODE_BITSIZE (mode) == 0
+ || GET_MODE_BITSIZE (mode) > BITS_PER_WORD)
+ mode = word_mode;
+ mode = get_best_mode (bitsize, bitpos1, MEM_ALIGN (str_rtx),
+ mode, 0);
+ if (mode == VOIDmode)
+ break;
+
+ offset1 = bitpos1;
+ bitpos1 %= GET_MODE_BITSIZE (mode);
+ offset1 = (offset1 - bitpos1) / BITS_PER_UNIT;
+ str_rtx = adjust_address (str_rtx, mode, offset1);
+ }
+ else if (!REG_P (str_rtx) && GET_CODE (str_rtx) != SUBREG)
+ break;
+
+ /* If the bit field covers the whole REG/MEM, store_field
+ will likely generate better code. */
+ if (bitsize >= GET_MODE_BITSIZE (GET_MODE (str_rtx)))
+ break;
+
+ /* We can't handle fields split accross multiple entities. */
+ if (bitpos1 + bitsize > GET_MODE_BITSIZE (GET_MODE (str_rtx)))
+ break;
+
if (BYTES_BIG_ENDIAN)
- count = GET_MODE_BITSIZE (GET_MODE (to_rtx)) - bitpos - bitsize;
+ bitpos1 = GET_MODE_BITSIZE (GET_MODE (str_rtx)) - bitpos1
+ - bitsize;
/* Special case some bitfield op= exp. */
switch (TREE_CODE (src))
{
case PLUS_EXPR:
case MINUS_EXPR:
- if (count <= 0)
- break;
-
/* For now, just optimize the case of the topmost bitfield
where we don't need to do any masking and also
1 bit bitfields where xor can be used.
We might win by one instruction for the other bitfields
too if insv/extv instructions aren't used, so that
can be added later. */
- if (count + bitsize != GET_MODE_BITSIZE (GET_MODE (to_rtx))
+ if (bitpos1 + bitsize != GET_MODE_BITSIZE (GET_MODE (str_rtx))
&& (bitsize != 1 || TREE_CODE (op1) != INTEGER_CST))
break;
- value = expand_expr (op1, NULL_RTX, VOIDmode, 0);
+ value = expand_expr (op1, NULL_RTX, GET_MODE (str_rtx), 0);
value = protect_from_queue (value, 0);
- to_rtx = protect_from_queue (to_rtx, 1);
+ str_rtx = protect_from_queue (str_rtx, 1);
+ value = convert_modes (GET_MODE (str_rtx),
+ TYPE_MODE (TREE_TYPE (op1)), value,
+ TYPE_UNSIGNED (TREE_TYPE (op1)));
+
+ /* We may be accessing data outside the field, which means
+ we can alias adjacent data. */
+ if (MEM_P (str_rtx))
+ {
+ str_rtx = shallow_copy_rtx (str_rtx);
+ set_mem_alias_set (str_rtx, 0);
+ set_mem_expr (str_rtx, 0);
+ }
+
binop = TREE_CODE (src) == PLUS_EXPR ? add_optab : sub_optab;
if (bitsize == 1
- && count + bitsize != GET_MODE_BITSIZE (GET_MODE (to_rtx)))
+ && bitpos1 + bitsize != GET_MODE_BITSIZE (GET_MODE (str_rtx)))
{
- value = expand_and (GET_MODE (to_rtx), value, const1_rtx,
+ value = expand_and (GET_MODE (str_rtx), value, const1_rtx,
NULL_RTX);
binop = xor_optab;
}
- value = expand_shift (LSHIFT_EXPR, GET_MODE (to_rtx),
- value, build_int_2 (count, 0),
+ value = expand_shift (LSHIFT_EXPR, GET_MODE (str_rtx),
+ value, build_int_2 (bitpos1, 0),
NULL_RTX, 1);
- result = expand_binop (GET_MODE (to_rtx), binop, to_rtx,
- value, to_rtx, 1, OPTAB_WIDEN);
- if (result != to_rtx)
- emit_move_insn (to_rtx, result);
+ result = expand_binop (GET_MODE (str_rtx), binop, str_rtx,
+ value, str_rtx, 1, OPTAB_WIDEN);
+ if (result != str_rtx)
+ emit_move_insn (str_rtx, result);
free_temp_slots ();
pop_temp_slots ();
return NULL_RTX;
+
default:
break;
}
--- gcc/testsuite/gcc.c-torture/execute/20040709-1.c.jj 2004-01-21 17:12:41.000000000 +0100
+++ gcc/testsuite/gcc.c-torture/execute/20040709-1.c 2004-07-06 11:55:57.000000000 +0200
@@ -0,0 +1,147 @@
+/* Test arithmetics on bitfields. */
+
+extern void abort (void);
+extern void exit (int);
+
+unsigned int
+myrnd (void)
+{
+ static unsigned int s = 1388815473;
+ s *= 1103515245;
+ s += 12345;
+ return (s / 65536) % 2048;
+}
+
+#define T(S) \
+struct S s##S; \
+struct S retme##S (struct S x) \
+{ \
+ return x; \
+} \
+ \
+unsigned int fn1##S (unsigned int x) \
+{ \
+ struct S y = s##S; \
+ y.k += x; \
+ y = retme##S (y); \
+ return y.k; \
+} \
+ \
+unsigned int fn2##S (unsigned int x) \
+{ \
+ struct S y = s##S; \
+ y.k += x; \
+ y.k %= 15; \
+ return y.k; \
+} \
+ \
+unsigned int retit##S (void) \
+{ \
+ return s##S.k; \
+} \
+ \
+unsigned int fn3##S (unsigned int x) \
+{ \
+ s##S.k += x; \
+ return retit##S (); \
+} \
+ \
+void test##S (void) \
+{ \
+ int i; \
+ unsigned int mask, v, a, r; \
+ struct S x; \
+ char *p = (char *) &s##S; \
+ for (i = 0; i < sizeof (s##S); ++i) \
+ *p++ = myrnd (); \
+ if (__builtin_classify_type (s##S.l) == 8) \
+ s##S.l = 5.25; \
+ s##S.k = -1; \
+ mask = s##S.k; \
+ v = myrnd (); \
+ a = myrnd (); \
+ s##S.k = v; \
+ x = s##S; \
+ r = fn1##S (a); \
+ if (x.i != s##S.i || x.j != s##S.j \
+ || x.k != s##S.k || x.l != s##S.l \
+ || ((v + a) & mask) != r) \
+ abort (); \
+ v = myrnd (); \
+ a = myrnd (); \
+ s##S.k = v; \
+ x = s##S; \
+ r = fn2##S (a); \
+ if (x.i != s##S.i || x.j != s##S.j \
+ || x.k != s##S.k || x.l != s##S.l \
+ || ((((v + a) & mask) % 15) & mask) != r) \
+ abort (); \
+ v = myrnd (); \
+ a = myrnd (); \
+ s##S.k = v; \
+ x = s##S; \
+ r = fn3##S (a); \
+ if (x.i != s##S.i || x.j != s##S.j \
+ || s##S.k != r || x.l != s##S.l \
+ || ((v + a) & mask) != r) \
+ abort (); \
+}
+
+struct A { unsigned int i : 6, l : 1, j : 10, k : 15; }; T(A)
+struct B { unsigned int i : 6, j : 11, k : 15; unsigned int l; }; T(B)
+struct C { unsigned int l; unsigned int i : 6, j : 11, k : 15; }; T(C)
+struct D { unsigned long long l : 6, i : 6, j : 23, k : 29; }; T(D)
+struct E { unsigned long long l, i : 12, j : 23, k : 29; }; T(E)
+struct F { unsigned long long i : 12, j : 23, k : 29, l; }; T(F)
+struct G { unsigned int i : 12, j : 13, k : 7; unsigned long long l; }; T(G)
+struct H { unsigned int i : 12, j : 11, k : 9; unsigned long long l; }; T(H)
+struct I { unsigned short i : 1, j : 6, k : 9; unsigned long long l; }; T(I)
+struct J { unsigned short i : 1, j : 8, k : 7; unsigned short l; }; T(J)
+struct K { unsigned int k : 6, l : 1, j : 10, i : 15; }; T(K)
+struct L { unsigned int k : 6, j : 11, i : 15; unsigned int l; }; T(L)
+struct M { unsigned int l; unsigned int k : 6, j : 11, i : 15; }; T(M)
+struct N { unsigned long long l : 6, k : 6, j : 23, i : 29; }; T(N)
+struct O { unsigned long long l, k : 12, j : 23, i : 29; }; T(O)
+struct P { unsigned long long k : 12, j : 23, i : 29, l; }; T(P)
+struct Q { unsigned int k : 12, j : 13, i : 7; unsigned long long l; }; T(Q)
+struct R { unsigned int k : 12, j : 11, i : 9; unsigned long long l; }; T(R)
+struct S { unsigned short k : 1, j : 6, i : 9; unsigned long long l; }; T(S)
+struct T { unsigned short k : 1, j : 8, i : 7; unsigned short l; }; T(T)
+struct U { unsigned short j : 6, k : 1, i : 9; unsigned long long l; }; T(U)
+struct V { unsigned short j : 8, k : 1, i : 7; unsigned short l; }; T(V)
+struct W { long double l; unsigned int k : 12, j : 13, i : 7; }; T(W)
+struct X { unsigned int k : 12, j : 13, i : 7; long double l; }; T(X)
+struct Y { unsigned int k : 12, j : 11, i : 9; long double l; }; T(Y)
+struct Z { long double l; unsigned int j : 13, i : 7, k : 12; }; T(Z)
+
+int
+main (void)
+{
+ testA ();
+ testB ();
+ testC ();
+ testD ();
+ testE ();
+ testF ();
+ testG ();
+ testH ();
+ testI ();
+ testJ ();
+ testK ();
+ testL ();
+ testM ();
+ testN ();
+ testO ();
+ testP ();
+ testQ ();
+ testR ();
+ testS ();
+ testT ();
+ testU ();
+ testV ();
+ testW ();
+ testX ();
+ testY ();
+ testZ ();
+ exit (0);
+}
--- gcc/testsuite/gcc.c-torture/execute/20040709-2.c.jj 2004-01-21 17:12:41.000000000 +0100
+++ gcc/testsuite/gcc.c-torture/execute/20040709-2.c 2004-07-09 10:49:17.000000000 +0200
@@ -0,0 +1,153 @@
+/* Test arithmetics on bitfields. */
+
+extern void abort (void);
+extern void exit (int);
+
+unsigned int
+myrnd (void)
+{
+ static unsigned int s = 1388815473;
+ s *= 1103515245;
+ s += 12345;
+ return (s / 65536) % 2048;
+}
+
+#define T(S) \
+struct S s##S; \
+struct S retme##S (struct S x) \
+{ \
+ return x; \
+} \
+ \
+unsigned int fn1##S (unsigned int x) \
+{ \
+ struct S y = s##S; \
+ y.k += x; \
+ y = retme##S (y); \
+ return y.k; \
+} \
+ \
+unsigned int fn2##S (unsigned int x) \
+{ \
+ struct S y = s##S; \
+ y.k += x; \
+ y.k %= 15; \
+ return y.k; \
+} \
+ \
+unsigned int retit##S (void) \
+{ \
+ return s##S.k; \
+} \
+ \
+unsigned int fn3##S (unsigned int x) \
+{ \
+ s##S.k += x; \
+ return retit##S (); \
+} \
+ \
+void test##S (void) \
+{ \
+ int i; \
+ unsigned int mask, v, a, r; \
+ struct S x; \
+ char *p = (char *) &s##S; \
+ for (i = 0; i < sizeof (s##S); ++i) \
+ *p++ = myrnd (); \
+ if (__builtin_classify_type (s##S.l) == 8) \
+ s##S.l = 5.25; \
+ s##S.k = -1; \
+ mask = s##S.k; \
+ v = myrnd (); \
+ a = myrnd (); \
+ s##S.k = v; \
+ x = s##S; \
+ r = fn1##S (a); \
+ if (x.i != s##S.i || x.j != s##S.j \
+ || x.k != s##S.k || x.l != s##S.l \
+ || ((v + a) & mask) != r) \
+ abort (); \
+ v = myrnd (); \
+ a = myrnd (); \
+ s##S.k = v; \
+ x = s##S; \
+ r = fn2##S (a); \
+ if (x.i != s##S.i || x.j != s##S.j \
+ || x.k != s##S.k || x.l != s##S.l \
+ || ((((v + a) & mask) % 15) & mask) != r) \
+ abort (); \
+ v = myrnd (); \
+ a = myrnd (); \
+ s##S.k = v; \
+ x = s##S; \
+ r = fn3##S (a); \
+ if (x.i != s##S.i || x.j != s##S.j \
+ || s##S.k != r || x.l != s##S.l \
+ || ((v + a) & mask) != r) \
+ abort (); \
+}
+
+#ifdef __powerpc64__
+/* Temporary hack for broken PPC64 unaligned handling PR rtl-optimization/13674 */
+# define pck
+#else
+# define pck __attribute__((packed))
+#endif
+struct pck A { unsigned short i : 1, l : 1, j : 3, k : 11; }; T(A)
+struct pck B { unsigned short i : 4, j : 1, k : 11; unsigned int l; }; T(B)
+struct pck C { unsigned int l; unsigned short i : 4, j : 1, k : 11; }; T(C)
+struct pck D { unsigned long long l : 6, i : 6, j : 23, k : 29; }; T(D)
+struct pck E { unsigned long long l, i : 12, j : 23, k : 29; }; T(E)
+struct pck F { unsigned long long i : 12, j : 23, k : 29, l; }; T(F)
+struct pck G { unsigned short i : 1, j : 1, k : 6; unsigned long long l; }; T(G)
+struct pck H { unsigned short i : 6, j : 2, k : 8; unsigned long long l; }; T(H)
+struct pck I { unsigned short i : 1, j : 6, k : 1; unsigned long long l; }; T(I)
+struct pck J { unsigned short i : 1, j : 8, k : 7; unsigned short l; }; T(J)
+struct pck K { unsigned int k : 6, l : 1, j : 10, i : 15; }; T(K)
+struct pck L { unsigned int k : 6, j : 11, i : 15; unsigned int l; }; T(L)
+struct pck M { unsigned int l; unsigned short k : 6, j : 11, i : 15; }; T(M)
+struct pck N { unsigned long long l : 6, k : 6, j : 23, i : 29; }; T(N)
+struct pck O { unsigned long long l, k : 12, j : 23, i : 29; }; T(O)
+struct pck P { unsigned long long k : 12, j : 23, i : 29, l; }; T(P)
+struct pck Q { unsigned short k : 12, j : 1, i : 3; unsigned long long l; }; T(Q)
+struct pck R { unsigned short k : 2, j : 11, i : 3; unsigned long long l; }; T(R)
+struct pck S { unsigned short k : 1, j : 6, i : 9; unsigned long long l; }; T(S)
+struct pck T { unsigned short k : 1, j : 8, i : 7; unsigned short l; }; T(T)
+struct pck U { unsigned short j : 6, k : 1, i : 9; unsigned long long l; }; T(U)
+struct pck V { unsigned short j : 8, k : 1, i : 7; unsigned short l; }; T(V)
+struct pck W { long double l; unsigned int k : 12, j : 13, i : 7; }; T(W)
+struct pck X { unsigned int k : 12, j : 13, i : 7; long double l; }; T(X)
+struct pck Y { unsigned int k : 12, j : 11, i : 9; long double l; }; T(Y)
+struct pck Z { long double l; unsigned int j : 13, i : 7, k : 12; }; T(Z)
+
+int
+main (void)
+{
+ testA ();
+ testB ();
+ testC ();
+ testD ();
+ testE ();
+ testF ();
+ testG ();
+ testH ();
+ testI ();
+ testJ ();
+ testK ();
+ testL ();
+ testM ();
+ testN ();
+ testO ();
+ testP ();
+ testQ ();
+ testR ();
+ testS ();
+ testT ();
+ testU ();
+ testV ();
+ testW ();
+ testX ();
+ testY ();
+ testZ ();
+ exit (0);
+}
Jakub
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix and reenable the bitfield += optimization (take 2)
2004-07-14 17:48 ` [PATCH] Fix and reenable the bitfield += optimization (take 2) Jakub Jelinek
@ 2004-07-14 22:00 ` Roger Sayle
2004-07-15 0:39 ` Richard Henderson
1 sibling, 0 replies; 11+ messages in thread
From: Roger Sayle @ 2004-07-14 22:00 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Henderson, gcc-patches
On Wed, 14 Jul 2004, Jakub Jelinek wrote:
> 2004-07-14 Jakub Jelinek <jakub@redhat.com>
>
> * expr.c (expand_assignment): Reenable bitfield += optimizations.
> Use alias set 0 for memory, do proper mode calculations and adjust
> address for memories.
>
> * gcc.c-torture/execute/20040709-1.c: New test.
> * gcc.c-torture/execute/20040709-2.c: New test.
If RTH doesn't have any objections, this is OK for mainline.
Roger
--
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix and reenable the bitfield += optimization (take 2)
2004-07-14 17:48 ` [PATCH] Fix and reenable the bitfield += optimization (take 2) Jakub Jelinek
2004-07-14 22:00 ` Roger Sayle
@ 2004-07-15 0:39 ` Richard Henderson
2004-07-15 0:55 ` Jakub Jelinek
1 sibling, 1 reply; 11+ messages in thread
From: Richard Henderson @ 2004-07-15 0:39 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Roger Sayle, gcc-patches
On Wed, Jul 14, 2004 at 04:02:06AM -0400, Jakub Jelinek wrote:
> Bootstrapped/regtested on
> {i386,x86_64,ia64,ppc,ppc64,s390,s390x}-redhat-linux.
The patch looks fine.
I assume you're testing this on 3.4 or something? You
shouldn't be able to trigger this on mainline at all,
after jsm28's bitfield patch.
r~
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix and reenable the bitfield += optimization (take 2)
2004-07-15 0:39 ` Richard Henderson
@ 2004-07-15 0:55 ` Jakub Jelinek
2004-07-15 1:07 ` Richard Henderson
0 siblings, 1 reply; 11+ messages in thread
From: Jakub Jelinek @ 2004-07-15 0:55 UTC (permalink / raw)
To: Richard Henderson, Roger Sayle, gcc-patches
On Wed, Jul 14, 2004 at 10:36:01AM -0700, Richard Henderson wrote:
> On Wed, Jul 14, 2004 at 04:02:06AM -0400, Jakub Jelinek wrote:
> > Bootstrapped/regtested on
> > {i386,x86_64,ia64,ppc,ppc64,s390,s390x}-redhat-linux.
>
> The patch looks fine.
>
> I assume you're testing this on 3.4 or something? You
> shouldn't be able to trigger this on mainline at all,
> after jsm28's bitfield patch.
I tested both on 3.4.1-rh and 3.5 20040709 (which is after Joseph's
bitfield commit). It was before Paolo's QUEUED removal, so
I have to remove those two protect_from_queue calls.
By "trigger this" you mean the PPC64 unaligned bug (that one is
present solely on the mainline, not on 3.4 branch or earlier),
or the optimization (which certainly triggers many times in the
testcases I have added
(gcc.c-torture/execute/2004{0629-1,0705-1,0705-2,0709-1,0709-2}.c)
on all the arches?
Jakub
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix and reenable the bitfield += optimization (take 2)
2004-07-15 0:55 ` Jakub Jelinek
@ 2004-07-15 1:07 ` Richard Henderson
2004-07-15 8:34 ` Jakub Jelinek
0 siblings, 1 reply; 11+ messages in thread
From: Richard Henderson @ 2004-07-15 1:07 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Roger Sayle, gcc-patches
On Wed, Jul 14, 2004 at 01:47:24PM -0400, Jakub Jelinek wrote:
> By "trigger this" you mean the PPC64 unaligned bug (that one is
> present solely on the mainline, not on 3.4 branch or earlier),
> or the optimization (which certainly triggers many times in the
> testcases I have added
> (gcc.c-torture/execute/2004{0629-1,0705-1,0705-2,0709-1,0709-2}.c)
> on all the arches?
I meant the test cases. When I was looking at execute/20040629-1.c
on alpha, after Joseph's patch, we would have
c.k = (unnamed type)((unsigned int)c.k + 1)
where (unnamed type) is the 26 bit type of the bitfield.
r~
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix and reenable the bitfield += optimization (take 2)
2004-07-15 1:07 ` Richard Henderson
@ 2004-07-15 8:34 ` Jakub Jelinek
2004-07-15 12:07 ` Richard Henderson
0 siblings, 1 reply; 11+ messages in thread
From: Jakub Jelinek @ 2004-07-15 8:34 UTC (permalink / raw)
To: Richard Henderson, Roger Sayle; +Cc: gcc-patches
On Wed, Jul 14, 2004 at 11:04:43AM -0700, Richard Henderson wrote:
> On Wed, Jul 14, 2004 at 01:47:24PM -0400, Jakub Jelinek wrote:
> > By "trigger this" you mean the PPC64 unaligned bug (that one is
> > present solely on the mainline, not on 3.4 branch or earlier),
> > or the optimization (which certainly triggers many times in the
> > testcases I have added
> > (gcc.c-torture/execute/2004{0629-1,0705-1,0705-2,0709-1,0709-2}.c)
> > on all the arches?
>
> I meant the test cases. When I was looking at execute/20040629-1.c
> on alpha, after Joseph's patch, we would have
>
> c.k = (unnamed type)((unsigned int)c.k + 1)
>
> where (unnamed type) is the 26 bit type of the bitfield.
Say for
struct { unsigned int i : 6, j : 11, k : 15; } b;
void foo (int x)
{
b.k = b.k + x;
}
from is:
<nop_expr 0x2a97ca14c0
type <integer_type 0x2a97daa460 unsigned HI
size <integer_cst 0x2a97c1f180 constant invariant 16>
unit size <integer_cst 0x2a97c9a870 constant invariant 2>
align 16 symtab 0 alias set -1 precision 15 min <integer_cst 0x2a97da9450 0> max <integer_cst 0x2a97da9480 32767>>
arg 0 <nop_expr 0x2a97ca1480
type <integer_type 0x2a97c0c8c0 short unsigned int unsigned HI
size <integer_cst 0x2a97c0a690 constant invariant 16>
unit size <integer_cst 0x2a97c0a870 constant invariant 2>
align 16 symtab 0 alias set -1 precision 16 min <integer_cst 0x2a97c0a9c0 0> max <integer_cst 0x2a97c0a9f0 65535>>
arg 0 <plus_expr 0x2a97c0b960 type <integer_type 0x2a97c0c7e0 short int>
arg 0 <convert_expr 0x2a97ca1440 type <integer_type 0x2a97c0c7e0 short int>
arg 0 <component_ref 0x2a97c0b8c0 type <integer_type 0x2a97daa460>
arg 0 <var_decl 0x2a97daa540 b> arg 1 <field_decl 0x2a97daa000 k>>>
arg 1 <convert_expr 0x2a97ca13c0 type <integer_type 0x2a97c0c7e0 short int>
arg 0 <parm_decl 0x2a97daa620 x>>>>>
My code in expand_assignment does:
tree src = from;
STRIP_NOPS (src);
and because HImode of the unnamed bitfield type is the same as
HImode of the PLUS_EXPR, src is now PLUS_EXPR and the optimization
can do its stuff.
Either it is ok for STRIP_NOPS to skip over these unnamed bitfield types
as long as TYPE_MODEs match, or not, but in either case everywhere in the code, not
just here. In fact, if STRIP_NOPS was not allowed to skip over them, I think
expand_assignment could still skip over them as long as precision is not smaller
than bitfield's own precision.
I'm not sure what are C++ enum cast supposed to do in:
enum E { e0 = 0, e1 = 0x1fff };
struct B { unsigned int i : 6, j : 11, k : 15; } b;
void foo (int x)
{
b.k = (enum E) (b.k + x);
}
but I get the same behaviour before and after my patch (that is, the enum cast
is ignored instead of doing & 0x1fff on the result of addition).
Jakub
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix and reenable the bitfield += optimization (take 2)
2004-07-15 8:34 ` Jakub Jelinek
@ 2004-07-15 12:07 ` Richard Henderson
0 siblings, 0 replies; 11+ messages in thread
From: Richard Henderson @ 2004-07-15 12:07 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Roger Sayle, gcc-patches
On Wed, Jul 14, 2004 at 07:49:40PM -0400, Jakub Jelinek wrote:
> Either it is ok for STRIP_NOPS to skip over these unnamed bitfield types
> as long as TYPE_MODEs match, or not, but in either case everywhere in the
> code, not just here.
Yes. I believe it's invalid for STRIP_NOPs to skip them.
We really need to clean up the usage of NOP_EXPR vs CONVERT_EXPR.
The documentation says that NOP_EXPR generates no code, which would
be fine, but is clearly *not* the case for many of the places it
gets used.
> I think expand_assignment could still skip over them as long as
> precision is not smaller than bitfield's own precision.
Yes.
> I'm not sure what are C++ enum cast supposed to do in:
I don't know off hand what the story with C++ enumerators or
bitfields are, at present.
r~
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2004-07-15 0:02 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-07-05 20:10 [PATCH] Disable the bitfield += optimization until it is fixed properly Jakub Jelinek
2004-07-05 20:47 ` Roger Sayle
2004-07-05 20:56 ` Eric Christopher
2004-07-09 23:22 ` [PATCH] Fix and reenable the bitfield += optimization Jakub Jelinek
2004-07-14 17:48 ` [PATCH] Fix and reenable the bitfield += optimization (take 2) Jakub Jelinek
2004-07-14 22:00 ` Roger Sayle
2004-07-15 0:39 ` Richard Henderson
2004-07-15 0:55 ` Jakub Jelinek
2004-07-15 1:07 ` Richard Henderson
2004-07-15 8:34 ` Jakub Jelinek
2004-07-15 12:07 ` Richard Henderson
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).