* PATCH: fold BIT_AND_EXPR's with ADDR_EXPR operands
@ 2007-09-11 6:13 Ollie Wild
2007-09-11 9:56 ` Richard Guenther
0 siblings, 1 reply; 11+ messages in thread
From: Ollie Wild @ 2007-09-11 6:13 UTC (permalink / raw)
To: GCC Patches; +Cc: Mark Mitchell
[-- Attachment #1: Type: text/plain, Size: 1081 bytes --]
This patch adds support for folding BIT_AND_EXPR's with pointer
operands derived from ADDR_EXPR's.
If a BIT_AND_EXPR contains (a) a pointer operand derived from an
ADDR_EXPR and (b) an INTEGER_CST operand whose value is smaller than
the alignment of the addressed object, then it is possible to fold the
expression into a single INTEGER_CST. This is important to the C++
front end because (on some architectures) this construct appears in
pointers to non-virtual member functions. This patch allows
dereferencing of these objects to be simplified into simple function
calls where the referenced member function is known.
Tested with a full bootstrap and testsuite on i686-pc-linux-gnu.
Ollie
2007-09-10 Ollie Wild <aaw@google.com>
fold-const.c (fold_binary): Fold BIT_AND_EXPR's with a pointer operand.
(get_pointer_modulus_and_residue): New function.
(get_lvalue_alignment): New function.
2007-09-10 Ollie Wild <aaw@google.com>
gcc.dg/fold-bitand-1.c: New test.
gcc.dg/fold-bitand-2.c: New test.
gcc.dg/fold-bitand-3.c: New test.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fold-bitand.patch --]
[-- Type: text/x-patch; name="fold-bitand.patch", Size: 5643 bytes --]
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index a6fb08b..8fb6798 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -9383,6 +9383,67 @@ fold_mult_zconjz (tree type, tree expr)
}
+/* Subroutine of get_pointer_modulus_and_residue. Computes the MODULUS and
+ RESIDUE of the storage for the supplied lvalue expression. MODULUS is
+ a power of 2. */
+
+static void
+get_lvalue_alignment (tree expr, unsigned int *modulus, unsigned int *residue)
+{
+ *modulus = 1;
+ *residue = 0;
+
+ if (DECL_P (expr))
+ *modulus = DECL_ALIGN_UNIT (expr);
+ else if (TREE_CODE (expr) == COMPONENT_REF)
+ {
+ tree field = TREE_OPERAND (expr, 1);
+
+ get_lvalue_alignment (TREE_OPERAND (expr, 0), modulus, residue);
+ *residue += TREE_INT_CST_LOW (DECL_FIELD_OFFSET (field))
+ + TREE_INT_CST_LOW (DECL_FIELD_BIT_OFFSET (field))
+ / BITS_PER_UNIT;
+ }
+}
+
+
+/* Subroutine of fold_binary. Computes the MODULUS and RESIDUE of a pointer
+ expression. MODULUS is a power of 2. */
+
+static void
+get_pointer_modulus_and_residue (tree expr, unsigned int *modulus,
+ unsigned int *residue)
+{
+ *modulus = 1;
+ *residue = 0;
+
+ switch (TREE_CODE (expr))
+ {
+ case ADDR_EXPR:
+ get_lvalue_alignment (TREE_OPERAND (expr, 0), modulus, residue);
+ break;
+
+ case POINTER_PLUS_EXPR:
+ {
+ tree op1 = TREE_OPERAND (expr, 1);
+
+ if (TREE_CODE (op1) == INTEGER_CST)
+ {
+ tree op0 = TREE_OPERAND (expr, 0);
+
+ STRIP_NOPS (op0);
+ get_pointer_modulus_and_residue (op0, modulus, residue);
+ *residue += TREE_INT_CST_LOW (op1);
+ }
+ }
+ break;
+
+ default:
+ break;
+ }
+}
+
+
/* Fold a binary expression of code CODE and type TYPE with operands
OP0 and OP1. Return the folded expression if folding is
successful. Otherwise, return NULL_TREE. */
@@ -10912,6 +10973,24 @@ fold_binary (enum tree_code code, tree type, tree op0, tree op1)
TREE_OPERAND (arg1, 0)));
}
+ /* If arg0 is derived from the address of an object or function, we may
+ be able to fold this expression using the object or function's
+ alignment. */
+ if (POINTER_TYPE_P (TREE_TYPE (arg0)) &&
+ TREE_CODE (arg1) == INTEGER_CST && TREE_INT_CST_HIGH (arg1) == 0)
+ {
+ unsigned int modulus, residue;
+ unsigned HOST_WIDE_INT low = TREE_INT_CST_LOW (arg1);
+
+ get_pointer_modulus_and_residue (arg0, &modulus, &residue);
+
+ /* This works because modulus is a power of 2. If this weren't the
+ case, we'd have to replace it by its greatest power-of-2
+ divisor: modulus & -modulus. */
+ if (low < modulus)
+ return build_int_cst (type, residue & low);
+ }
+
goto associate;
case RDIV_EXPR:
diff --git a/gcc/testsuite/gcc.dg/fold-bitand-1.c b/gcc/testsuite/gcc.dg/fold-bitand-1.c
new file mode 100644
index 0000000..cd9ed64
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/fold-bitand-1.c
@@ -0,0 +1,34 @@
+/* { dg-do compile } */
+/* { dg-options "-fdump-tree-original" } */
+
+char c1 __attribute__ ((aligned (1)));
+char c2 __attribute__ ((aligned (2)));
+char c4 __attribute__ ((aligned (4)));
+char c8 __attribute__ ((aligned (8)));
+
+unsigned f1(void)
+{
+ return 3 & (unsigned)&c1;
+}
+
+unsigned f2(void)
+{
+ return 3 & (unsigned)&c2;
+}
+
+unsigned f3(void)
+{
+ return 3 & (unsigned)&c4;
+}
+
+unsigned f4(void)
+{
+ return 3 & (unsigned)&c8;
+}
+
+/* { dg-final { scan-tree-dump-times "\&c1 \& 3" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "\&c2 \& 3" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "\&c4 \& 3" 0 "original" } } */
+/* { dg-final { scan-tree-dump-times "\&c8 \& 3" 0 "original" } } */
+/* { dg-final { scan-tree-dump-times "return 0" 2 "original" } } */
+/* { dg-final { cleanup-tree-dump "original" } } */
diff --git a/gcc/testsuite/gcc.dg/fold-bitand-2.c b/gcc/testsuite/gcc.dg/fold-bitand-2.c
new file mode 100644
index 0000000..edd6ba1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/fold-bitand-2.c
@@ -0,0 +1,42 @@
+/* { dg-do compile } */
+/* { dg-options "-fdump-tree-original" } */
+
+struct {
+ char c1;
+ char c2;
+ char c3;
+ char c4;
+} s __attribute__ ((aligned (4)));
+
+unsigned f1 (void)
+{
+ return 3 & (unsigned)&s.c1;
+}
+
+unsigned f2 (void)
+{
+ return 3 & (unsigned)&s.c2;
+}
+
+unsigned f3 (void)
+{
+ return 3 & (unsigned)&s.c3;
+}
+
+unsigned f4 (void)
+{
+ return 3 & (unsigned)&s.c4;
+}
+
+unsigned f5 (void)
+{
+ return 4 & (unsigned)&s.c1;
+}
+
+/* { dg-final { scan-tree-dump-times "\& 3" 0 "original" } } */
+/* { dg-final { scan-tree-dump-times "\& 4" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "return 0" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "return 1" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "return 2" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "return 3" 1 "original" } } */
+/* { dg-final { cleanup-tree-dump "original" } } */
diff --git a/gcc/testsuite/gcc.dg/fold-bitand-3.c b/gcc/testsuite/gcc.dg/fold-bitand-3.c
new file mode 100644
index 0000000..e6b8e9a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/fold-bitand-3.c
@@ -0,0 +1,25 @@
+/* { dg-do compile } */
+/* { dg-options "-fdump-tree-original" } */
+
+char c[4] __attribute__ ((aligned (4)));
+
+struct S {
+ char c1;
+ char c2;
+ char c3;
+ char c4;
+};
+
+int f1 (void)
+{
+ return 3 & (unsigned long)&c[1];
+}
+
+int f2 (void)
+{
+ return 3 & (unsigned long)&((struct S *)&c)->c2;
+}
+
+/* { dg-final { scan-tree-dump-times "\& 3" 0 "original" } } */
+/* { dg-final { scan-tree-dump-times "return 1" 2 "original" } } */
+/* { dg-final { cleanup-tree-dump "original" } } */
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH: fold BIT_AND_EXPR's with ADDR_EXPR operands
2007-09-11 6:13 PATCH: fold BIT_AND_EXPR's with ADDR_EXPR operands Ollie Wild
@ 2007-09-11 9:56 ` Richard Guenther
2007-09-18 9:02 ` Ollie Wild
0 siblings, 1 reply; 11+ messages in thread
From: Richard Guenther @ 2007-09-11 9:56 UTC (permalink / raw)
To: Ollie Wild; +Cc: GCC Patches, Mark Mitchell
On 9/11/07, Ollie Wild <aaw@google.com> wrote:
> This patch adds support for folding BIT_AND_EXPR's with pointer
> operands derived from ADDR_EXPR's.
>
> If a BIT_AND_EXPR contains (a) a pointer operand derived from an
> ADDR_EXPR and (b) an INTEGER_CST operand whose value is smaller than
> the alignment of the addressed object, then it is possible to fold the
> expression into a single INTEGER_CST. This is important to the C++
> front end because (on some architectures) this construct appears in
> pointers to non-virtual member functions. This patch allows
> dereferencing of these objects to be simplified into simple function
> calls where the referenced member function is known.
Can you consolidate your get_lvalue_alignment with
builtins.c:get_pointer_alignment?
It seems to have more sophisticated handling of ADDR_EXPRs.
Thanks,
Richard.
>
> Tested with a full bootstrap and testsuite on i686-pc-linux-gnu.
>
> Ollie
>
>
> 2007-09-10 Ollie Wild <aaw@google.com>
>
> fold-const.c (fold_binary): Fold BIT_AND_EXPR's with a pointer operand.
> (get_pointer_modulus_and_residue): New function.
> (get_lvalue_alignment): New function.
>
> 2007-09-10 Ollie Wild <aaw@google.com>
>
> gcc.dg/fold-bitand-1.c: New test.
> gcc.dg/fold-bitand-2.c: New test.
> gcc.dg/fold-bitand-3.c: New test.
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH: fold BIT_AND_EXPR's with ADDR_EXPR operands
2007-09-11 9:56 ` Richard Guenther
@ 2007-09-18 9:02 ` Ollie Wild
2007-09-18 9:10 ` Jakub Jelinek
0 siblings, 1 reply; 11+ messages in thread
From: Ollie Wild @ 2007-09-18 9:02 UTC (permalink / raw)
To: Richard Guenther; +Cc: GCC Patches, Mark Mitchell
[-- Attachment #1: Type: text/plain, Size: 1688 bytes --]
On 9/11/07, Richard Guenther <richard.guenther@gmail.com> wrote:
>
> Can you consolidate your get_lvalue_alignment with
> builtins.c:get_pointer_alignment?
> It seems to have more sophisticated handling of ADDR_EXPRs.
I don't think it makes sense to merge these two functions, as
get_lvalue_alignment tries to calculate the largest modulus yielding a
constant residue, whereas get_pointer_alignment seeks the smallest
modulus yielding a zero residue. Furthermore, get_pointer_alignment
utilizes TYPE_ALIGN, which is merely suggestive on some architectures
and hence unsuitable for constant folding.
That said, the get_inner_reference function which
get_pointer_alignment uses for offset calculations is perfect. I've
attached a new patch which adds calls to get_inner_reference for
offset calculations and handles MULT_EXPR operands of
POINTER_PLUS_EXPR. The latter allows it to handle code like
(unsigned)&a[i] & 3, where the array index is variable.
In theory, I could also handle MULT_EXPR's returned from
get_inner_reference. However, it isn't clear to me how to generate
such an expression, and I refuse to write code I can't test. If
someone knows how to write a simple test program for this scenario,
please let me know.
Tested with a full bootstrap and testsuite on i686-pc-linux-gnu.
Ollie
2007-09-17 Ollie Wild <aaw@google.com>
fold-const.c (fold_binary): Fold BIT_AND_EXPR's with a pointer operand.
(get_pointer_modulus_and_residue): New function.
2007-09-07 Ollie Wild <aaw@google.com>
gcc.dg/fold-bitand-1.c: New test.
gcc.dg/fold-bitand-2.c: New test.
gcc.dg/fold-bitand-3.c: New test.
gcc.dg/fold-bitand-4.c: New test.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fold-bitand.patch --]
[-- Type: text/x-patch; name="fold-bitand.patch", Size: 7440 bytes --]
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index fb664fe..09f65a2 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -9384,6 +9384,97 @@ fold_mult_zconjz (tree type, tree expr)
}
+/* Subroutine of fold_binary. Computes the MODULUS and RESIDUE of a pointer
+ expression. MODULUS is a power of 2. */
+
+static void
+get_pointer_modulus_and_residue (tree expr, unsigned HOST_WIDE_INT *modulus,
+ unsigned HOST_WIDE_INT *residue)
+{
+ switch (TREE_CODE (expr))
+ {
+ case ADDR_EXPR:
+ *residue = 0;
+
+ expr = TREE_OPERAND (expr, 0);
+ if (handled_component_p (expr))
+ {
+ HOST_WIDE_INT bitsize, bitpos;
+ tree offset;
+ enum machine_mode mode;
+ int unsignedp, volatilep;
+
+ expr = get_inner_reference (expr, &bitsize, &bitpos, &offset,
+ &mode, &unsignedp, &volatilep, false);
+ *residue = bitpos / BITS_PER_UNIT;
+ if (offset)
+ {
+ if (TREE_CODE (offset) == INTEGER_CST)
+ *residue += TREE_INT_CST_LOW (offset);
+ else
+ expr = NULL;
+ }
+ }
+
+ if (expr && DECL_P (expr))
+ *modulus = DECL_ALIGN_UNIT (expr);
+ else
+ *modulus = 1;
+
+ break;
+
+ case POINTER_PLUS_EXPR:
+ {
+ tree op0, op1;
+
+ op0 = TREE_OPERAND (expr, 0);
+ STRIP_NOPS (op0);
+ get_pointer_modulus_and_residue (op0, modulus, residue);
+
+ op1 = TREE_OPERAND (expr, 1);
+ STRIP_NOPS (op1);
+ switch (TREE_CODE (op1))
+ {
+ case INTEGER_CST:
+ *residue += TREE_INT_CST_LOW (op1);
+ break;
+
+ case MULT_EXPR:
+ op1 = TREE_OPERAND (op1, 1);
+ STRIP_NOPS (op1);
+ if (TREE_CODE (op1) == INTEGER_CST)
+ {
+ unsigned HOST_WIDE_INT align;
+
+ /* Compute the greatest power-of-2 divisor of op1. */
+ align = TREE_INT_CST_LOW (op1);
+ align &= -align;
+
+ /* If align is non-zero and less than *modulus, replace
+ *modulus with align., If align is 0, then either op1 is 0
+ or the greatest power-of-2 divisor of op1 doesn't fit in an
+ unsigned HOST_WIDE_INT. In either case, no additional
+ constraint is imposed. */
+ if (align)
+ *modulus = MIN (*modulus, align);
+ }
+ break;
+
+ default:
+ *modulus = 1;
+ *residue = 0;
+ }
+ }
+ break;
+
+ default:
+ *modulus = 1;
+ *residue = 0;
+ break;
+ }
+}
+
+
/* Fold a binary expression of code CODE and type TYPE with operands
OP0 and OP1. Return the folded expression if folding is
successful. Otherwise, return NULL_TREE. */
@@ -10913,6 +11004,24 @@ fold_binary (enum tree_code code, tree type, tree op0, tree op1)
TREE_OPERAND (arg1, 0)));
}
+ /* If arg0 is derived from the address of an object or function, we may
+ be able to fold this expression using the object or function's
+ alignment. */
+ if (POINTER_TYPE_P (TREE_TYPE (arg0)) &&
+ TREE_CODE (arg1) == INTEGER_CST && TREE_INT_CST_HIGH (arg1) == 0)
+ {
+ unsigned HOST_WIDE_INT modulus, residue;
+ unsigned HOST_WIDE_INT low = TREE_INT_CST_LOW (arg1);
+
+ get_pointer_modulus_and_residue (arg0, &modulus, &residue);
+
+ /* This works because modulus is a power of 2. If this weren't the
+ case, we'd have to replace it by its greatest power-of-2
+ divisor: modulus & -modulus. */
+ if (low < modulus)
+ return build_int_cst (type, residue & low);
+ }
+
goto associate;
case RDIV_EXPR:
diff --git a/gcc/testsuite/gcc.dg/fold-bitand-1.c b/gcc/testsuite/gcc.dg/fold-bitand-1.c
new file mode 100644
index 0000000..cd9ed64
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/fold-bitand-1.c
@@ -0,0 +1,34 @@
+/* { dg-do compile } */
+/* { dg-options "-fdump-tree-original" } */
+
+char c1 __attribute__ ((aligned (1)));
+char c2 __attribute__ ((aligned (2)));
+char c4 __attribute__ ((aligned (4)));
+char c8 __attribute__ ((aligned (8)));
+
+unsigned f1(void)
+{
+ return 3 & (unsigned)&c1;
+}
+
+unsigned f2(void)
+{
+ return 3 & (unsigned)&c2;
+}
+
+unsigned f3(void)
+{
+ return 3 & (unsigned)&c4;
+}
+
+unsigned f4(void)
+{
+ return 3 & (unsigned)&c8;
+}
+
+/* { dg-final { scan-tree-dump-times "\&c1 \& 3" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "\&c2 \& 3" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "\&c4 \& 3" 0 "original" } } */
+/* { dg-final { scan-tree-dump-times "\&c8 \& 3" 0 "original" } } */
+/* { dg-final { scan-tree-dump-times "return 0" 2 "original" } } */
+/* { dg-final { cleanup-tree-dump "original" } } */
diff --git a/gcc/testsuite/gcc.dg/fold-bitand-2.c b/gcc/testsuite/gcc.dg/fold-bitand-2.c
new file mode 100644
index 0000000..edd6ba1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/fold-bitand-2.c
@@ -0,0 +1,42 @@
+/* { dg-do compile } */
+/* { dg-options "-fdump-tree-original" } */
+
+struct {
+ char c1;
+ char c2;
+ char c3;
+ char c4;
+} s __attribute__ ((aligned (4)));
+
+unsigned f1 (void)
+{
+ return 3 & (unsigned)&s.c1;
+}
+
+unsigned f2 (void)
+{
+ return 3 & (unsigned)&s.c2;
+}
+
+unsigned f3 (void)
+{
+ return 3 & (unsigned)&s.c3;
+}
+
+unsigned f4 (void)
+{
+ return 3 & (unsigned)&s.c4;
+}
+
+unsigned f5 (void)
+{
+ return 4 & (unsigned)&s.c1;
+}
+
+/* { dg-final { scan-tree-dump-times "\& 3" 0 "original" } } */
+/* { dg-final { scan-tree-dump-times "\& 4" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "return 0" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "return 1" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "return 2" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "return 3" 1 "original" } } */
+/* { dg-final { cleanup-tree-dump "original" } } */
diff --git a/gcc/testsuite/gcc.dg/fold-bitand-3.c b/gcc/testsuite/gcc.dg/fold-bitand-3.c
new file mode 100644
index 0000000..5ba2695
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/fold-bitand-3.c
@@ -0,0 +1,25 @@
+/* { dg-do compile } */
+/* { dg-options "-fdump-tree-original" } */
+
+char c[4] __attribute__ ((aligned (4)));
+
+struct S {
+ char c1;
+ char c2;
+ char c3;
+ char c4;
+};
+
+int f1 (void)
+{
+ return 3 & (unsigned)&c[1];
+}
+
+int f2 (void)
+{
+ return 3 & (unsigned)&((struct S *)&c)->c2;
+}
+
+/* { dg-final { scan-tree-dump-times "\& 3" 0 "original" } } */
+/* { dg-final { scan-tree-dump-times "return 1" 2 "original" } } */
+/* { dg-final { cleanup-tree-dump "original" } } */
diff --git a/gcc/testsuite/gcc.dg/fold-bitand-4.c b/gcc/testsuite/gcc.dg/fold-bitand-4.c
new file mode 100644
index 0000000..e405942
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/fold-bitand-4.c
@@ -0,0 +1,44 @@
+/* { dg-do compile } */
+/* { dg-options "-fdump-tree-original" } */
+
+typedef char char4[4] __attribute__ ((aligned (4)));
+char4 c4[4] __attribute__ ((aligned (16)));
+
+typedef char char16[16] __attribute__ ((aligned (16)));
+char16 c16[4] __attribute__ ((aligned (4)));
+
+int f1 (void)
+{
+ /* 12 */
+ return 15 & (unsigned)&c4[3];
+}
+
+int f2 (int i)
+{
+ /* Indeterminate */
+ return 15 & (unsigned)&c4[i];
+}
+
+int f3 (int i)
+{
+ /* 0 */
+ return 3 & (unsigned)&c4[i];
+}
+
+int f4 (int i)
+{
+ /* Indeterminate */
+ return 7 & (unsigned)&c16[i];
+}
+
+int f5 (int i)
+{
+ /* 0 */
+ return 3 & (unsigned)&c16[i];
+}
+
+/* { dg-final { scan-tree-dump-times "return 12" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "\& 15" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "return 0" 2 "original" } } */
+/* { dg-final { scan-tree-dump-times "\& 7" 1 "original" } } */
+/* { dg-final { cleanup-tree-dump "original" } } */
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH: fold BIT_AND_EXPR's with ADDR_EXPR operands
2007-09-18 9:02 ` Ollie Wild
@ 2007-09-18 9:10 ` Jakub Jelinek
2007-09-19 6:57 ` Ollie Wild
0 siblings, 1 reply; 11+ messages in thread
From: Jakub Jelinek @ 2007-09-18 9:10 UTC (permalink / raw)
To: Ollie Wild; +Cc: Richard Guenther, GCC Patches, Mark Mitchell
On Mon, Sep 17, 2007 at 11:53:43PM -0700, Ollie Wild wrote:
> 2007-09-07 Ollie Wild <aaw@google.com>
>
> gcc.dg/fold-bitand-1.c: New test.
> gcc.dg/fold-bitand-2.c: New test.
> gcc.dg/fold-bitand-3.c: New test.
> gcc.dg/fold-bitand-4.c: New test.
The tests shouldn't cast pointers to (unsigned), but to uintptr_t
or at least size_t, otherwise it is -Wpointer-to-int-cast material
on LP64 architectures.
Jakub
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH: fold BIT_AND_EXPR's with ADDR_EXPR operands
2007-09-18 9:10 ` Jakub Jelinek
@ 2007-09-19 6:57 ` Ollie Wild
2007-09-19 9:19 ` Richard Guenther
0 siblings, 1 reply; 11+ messages in thread
From: Ollie Wild @ 2007-09-19 6:57 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Guenther, GCC Patches, Mark Mitchell
[-- Attachment #1: Type: text/plain, Size: 803 bytes --]
On 9/18/07, Jakub Jelinek <jakub@redhat.com> wrote:
>
> The tests shouldn't cast pointers to (unsigned), but to uintptr_t
> or at least size_t, otherwise it is -Wpointer-to-int-cast material
> on LP64 architectures.
I've attached an updated patch. After some consideration, I've opted
to use __SIZE_TYPE__, which is consistent with several other tests
(e.g. array-10.c) and avoids problems with missing stdint.h headers.
Ollie
2007-09-18 Ollie Wild <aaw@google.com>
fold-const.c (fold_binary): Fold BIT_AND_EXPR's with a pointer operand.
(get_pointer_modulus_and_residue): New function.
2007-09-18 Ollie Wild <aaw@google.com>
gcc.dg/fold-bitand-1.c: New test.
gcc.dg/fold-bitand-2.c: New test.
gcc.dg/fold-bitand-3.c: New test.
gcc.dg/fold-bitand-4.c: New test.
[-- Attachment #2: fold-bitand.patch --]
[-- Type: application/octet-stream, Size: 7520 bytes --]
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index fb664fe..09f65a2 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -9384,6 +9384,97 @@ fold_mult_zconjz (tree type, tree expr)
}
+/* Subroutine of fold_binary. Computes the MODULUS and RESIDUE of a pointer
+ expression. MODULUS is a power of 2. */
+
+static void
+get_pointer_modulus_and_residue (tree expr, unsigned HOST_WIDE_INT *modulus,
+ unsigned HOST_WIDE_INT *residue)
+{
+ switch (TREE_CODE (expr))
+ {
+ case ADDR_EXPR:
+ *residue = 0;
+
+ expr = TREE_OPERAND (expr, 0);
+ if (handled_component_p (expr))
+ {
+ HOST_WIDE_INT bitsize, bitpos;
+ tree offset;
+ enum machine_mode mode;
+ int unsignedp, volatilep;
+
+ expr = get_inner_reference (expr, &bitsize, &bitpos, &offset,
+ &mode, &unsignedp, &volatilep, false);
+ *residue = bitpos / BITS_PER_UNIT;
+ if (offset)
+ {
+ if (TREE_CODE (offset) == INTEGER_CST)
+ *residue += TREE_INT_CST_LOW (offset);
+ else
+ expr = NULL;
+ }
+ }
+
+ if (expr && DECL_P (expr))
+ *modulus = DECL_ALIGN_UNIT (expr);
+ else
+ *modulus = 1;
+
+ break;
+
+ case POINTER_PLUS_EXPR:
+ {
+ tree op0, op1;
+
+ op0 = TREE_OPERAND (expr, 0);
+ STRIP_NOPS (op0);
+ get_pointer_modulus_and_residue (op0, modulus, residue);
+
+ op1 = TREE_OPERAND (expr, 1);
+ STRIP_NOPS (op1);
+ switch (TREE_CODE (op1))
+ {
+ case INTEGER_CST:
+ *residue += TREE_INT_CST_LOW (op1);
+ break;
+
+ case MULT_EXPR:
+ op1 = TREE_OPERAND (op1, 1);
+ STRIP_NOPS (op1);
+ if (TREE_CODE (op1) == INTEGER_CST)
+ {
+ unsigned HOST_WIDE_INT align;
+
+ /* Compute the greatest power-of-2 divisor of op1. */
+ align = TREE_INT_CST_LOW (op1);
+ align &= -align;
+
+ /* If align is non-zero and less than *modulus, replace
+ *modulus with align., If align is 0, then either op1 is 0
+ or the greatest power-of-2 divisor of op1 doesn't fit in an
+ unsigned HOST_WIDE_INT. In either case, no additional
+ constraint is imposed. */
+ if (align)
+ *modulus = MIN (*modulus, align);
+ }
+ break;
+
+ default:
+ *modulus = 1;
+ *residue = 0;
+ }
+ }
+ break;
+
+ default:
+ *modulus = 1;
+ *residue = 0;
+ break;
+ }
+}
+
+
/* Fold a binary expression of code CODE and type TYPE with operands
OP0 and OP1. Return the folded expression if folding is
successful. Otherwise, return NULL_TREE. */
@@ -10913,6 +11004,24 @@ fold_binary (enum tree_code code, tree type, tree op0, tree op1)
TREE_OPERAND (arg1, 0)));
}
+ /* If arg0 is derived from the address of an object or function, we may
+ be able to fold this expression using the object or function's
+ alignment. */
+ if (POINTER_TYPE_P (TREE_TYPE (arg0)) &&
+ TREE_CODE (arg1) == INTEGER_CST && TREE_INT_CST_HIGH (arg1) == 0)
+ {
+ unsigned HOST_WIDE_INT modulus, residue;
+ unsigned HOST_WIDE_INT low = TREE_INT_CST_LOW (arg1);
+
+ get_pointer_modulus_and_residue (arg0, &modulus, &residue);
+
+ /* This works because modulus is a power of 2. If this weren't the
+ case, we'd have to replace it by its greatest power-of-2
+ divisor: modulus & -modulus. */
+ if (low < modulus)
+ return build_int_cst (type, residue & low);
+ }
+
goto associate;
case RDIV_EXPR:
diff --git a/gcc/testsuite/gcc.dg/fold-bitand-1.c b/gcc/testsuite/gcc.dg/fold-bitand-1.c
new file mode 100644
index 0000000..413ed67
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/fold-bitand-1.c
@@ -0,0 +1,34 @@
+/* { dg-do compile } */
+/* { dg-options "-fdump-tree-original" } */
+
+char c1 __attribute__ ((aligned (1)));
+char c2 __attribute__ ((aligned (2)));
+char c4 __attribute__ ((aligned (4)));
+char c8 __attribute__ ((aligned (8)));
+
+unsigned f1(void)
+{
+ return 3 & (__SIZE_TYPE__)&c1;
+}
+
+unsigned f2(void)
+{
+ return 3 & (__SIZE_TYPE__)&c2;
+}
+
+unsigned f3(void)
+{
+ return 3 & (__SIZE_TYPE__)&c4;
+}
+
+unsigned f4(void)
+{
+ return 3 & (__SIZE_TYPE__)&c8;
+}
+
+/* { dg-final { scan-tree-dump-times "\&c1 \& 3" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "\&c2 \& 3" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "\&c4 \& 3" 0 "original" } } */
+/* { dg-final { scan-tree-dump-times "\&c8 \& 3" 0 "original" } } */
+/* { dg-final { scan-tree-dump-times "return 0" 2 "original" } } */
+/* { dg-final { cleanup-tree-dump "original" } } */
diff --git a/gcc/testsuite/gcc.dg/fold-bitand-2.c b/gcc/testsuite/gcc.dg/fold-bitand-2.c
new file mode 100644
index 0000000..de2e674
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/fold-bitand-2.c
@@ -0,0 +1,42 @@
+/* { dg-do compile } */
+/* { dg-options "-fdump-tree-original" } */
+
+struct {
+ char c1;
+ char c2;
+ char c3;
+ char c4;
+} s __attribute__ ((aligned (4)));
+
+unsigned f1 (void)
+{
+ return 3 & (__SIZE_TYPE__)&s.c1;
+}
+
+unsigned f2 (void)
+{
+ return 3 & (__SIZE_TYPE__)&s.c2;
+}
+
+unsigned f3 (void)
+{
+ return 3 & (__SIZE_TYPE__)&s.c3;
+}
+
+unsigned f4 (void)
+{
+ return 3 & (__SIZE_TYPE__)&s.c4;
+}
+
+unsigned f5 (void)
+{
+ return 4 & (__SIZE_TYPE__)&s.c1;
+}
+
+/* { dg-final { scan-tree-dump-times "\& 3" 0 "original" } } */
+/* { dg-final { scan-tree-dump-times "\& 4" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "return 0" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "return 1" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "return 2" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "return 3" 1 "original" } } */
+/* { dg-final { cleanup-tree-dump "original" } } */
diff --git a/gcc/testsuite/gcc.dg/fold-bitand-3.c b/gcc/testsuite/gcc.dg/fold-bitand-3.c
new file mode 100644
index 0000000..43d765b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/fold-bitand-3.c
@@ -0,0 +1,25 @@
+/* { dg-do compile } */
+/* { dg-options "-fdump-tree-original" } */
+
+char c[4] __attribute__ ((aligned (4)));
+
+struct S {
+ char c1;
+ char c2;
+ char c3;
+ char c4;
+};
+
+int f1 (void)
+{
+ return 3 & (__SIZE_TYPE__)&c[1];
+}
+
+int f2 (void)
+{
+ return 3 & (__SIZE_TYPE__)&((struct S *)&c)->c2;
+}
+
+/* { dg-final { scan-tree-dump-times "\& 3" 0 "original" } } */
+/* { dg-final { scan-tree-dump-times "return 1" 2 "original" } } */
+/* { dg-final { cleanup-tree-dump "original" } } */
diff --git a/gcc/testsuite/gcc.dg/fold-bitand-4.c b/gcc/testsuite/gcc.dg/fold-bitand-4.c
new file mode 100644
index 0000000..7d82426
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/fold-bitand-4.c
@@ -0,0 +1,44 @@
+/* { dg-do compile } */
+/* { dg-options "-fdump-tree-original" } */
+
+typedef char char4[4] __attribute__ ((aligned (4)));
+char4 c4[4] __attribute__ ((aligned (16)));
+
+typedef char char16[16] __attribute__ ((aligned (16)));
+char16 c16[4] __attribute__ ((aligned (4)));
+
+int f1 (void)
+{
+ /* 12 */
+ return 15 & (__SIZE_TYPE__)&c4[3];
+}
+
+int f2 (int i)
+{
+ /* Indeterminate */
+ return 15 & (__SIZE_TYPE__)&c4[i];
+}
+
+int f3 (int i)
+{
+ /* 0 */
+ return 3 & (__SIZE_TYPE__)&c4[i];
+}
+
+int f4 (int i)
+{
+ /* Indeterminate */
+ return 7 & (__SIZE_TYPE__)&c16[i];
+}
+
+int f5 (int i)
+{
+ /* 0 */
+ return 3 & (__SIZE_TYPE__)&c16[i];
+}
+
+/* { dg-final { scan-tree-dump-times "return 12" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "\& 15" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "return 0" 2 "original" } } */
+/* { dg-final { scan-tree-dump-times "\& 7" 1 "original" } } */
+/* { dg-final { cleanup-tree-dump "original" } } */
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH: fold BIT_AND_EXPR's with ADDR_EXPR operands
2007-09-19 6:57 ` Ollie Wild
@ 2007-09-19 9:19 ` Richard Guenther
2007-09-20 20:33 ` Ollie Wild
0 siblings, 1 reply; 11+ messages in thread
From: Richard Guenther @ 2007-09-19 9:19 UTC (permalink / raw)
To: Ollie Wild; +Cc: Jakub Jelinek, GCC Patches, Mark Mitchell
On 9/19/07, Ollie Wild <aaw@google.com> wrote:
> On 9/18/07, Jakub Jelinek <jakub@redhat.com> wrote:
> >
> > The tests shouldn't cast pointers to (unsigned), but to uintptr_t
> > or at least size_t, otherwise it is -Wpointer-to-int-cast material
> > on LP64 architectures.
>
> I've attached an updated patch. After some consideration, I've opted
> to use __SIZE_TYPE__, which is consistent with several other tests
> (e.g. array-10.c) and avoids problems with missing stdint.h headers.
+ /* If arg0 is derived from the address of an object or function, we may
+ be able to fold this expression using the object or function's
+ alignment. */
+ if (POINTER_TYPE_P (TREE_TYPE (arg0)) &&
+ TREE_CODE (arg1) == INTEGER_CST && TREE_INT_CST_HIGH (arg1) == 0)
+ {
coding style says the && goes to the next line. Instead of
TREE_CODE (arg1) == INTEGER_CST && TREE_INT_CST_HIGH (arg1) == 0 use
host_integer_p (arg1, 1).
The description of get_pointer_modulus_and_residue isn't exactly clear
to me. It looks
like it should compute the biggest power-of-two N and M so that p % N == M? But
this doesn't have a single solution - so what is it exactly computing?
Please clarify
that in the functions description.
+ expr = get_inner_reference (expr, &bitsize, &bitpos, &offset,
+ &mode, &unsignedp, &volatilep, false);
+ *residue = bitpos / BITS_PER_UNIT;
+ if (offset)
+ {
+ if (TREE_CODE (offset) == INTEGER_CST)
+ *residue += TREE_INT_CST_LOW (offset);
+ else
+ expr = NULL;
an INTEGER_CST offset is always accounted for in bitpos, no need to repeat that
here, just bail out. As for the function I would make it return the
modulus, that would
make the flow easier.
+ if (expr && DECL_P (expr))
+ *modulus = DECL_ALIGN_UNIT (expr);
+ else
expr can't be non-NULL here. I don't see how you protect against a
residue bigger
than modulus? But I guess it doesn't matter, still the names are
confusing then.
I would still like you to re-investigate using get_pointer_alignment.
Basically you
would split out a helper function to get the pointer expression with
its ADDR_EXPR
stripped off and do
base = get_inner_reference (expr, ...);
base_align = get_pointer_alignment_1 (base, -1);
where base_align yields your modulus and from the get_inner_reference result
you compute the residue. And yes, the result from get_pointer_alignment has
to be useful for folding as strict alignment targets rely on it for
correct code.
Thanks,
Richard.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH: fold BIT_AND_EXPR's with ADDR_EXPR operands
2007-09-19 9:19 ` Richard Guenther
@ 2007-09-20 20:33 ` Ollie Wild
2007-09-20 21:11 ` Richard Guenther
0 siblings, 1 reply; 11+ messages in thread
From: Ollie Wild @ 2007-09-20 20:33 UTC (permalink / raw)
To: Richard Guenther; +Cc: Jakub Jelinek, GCC Patches, Mark Mitchell
On 9/19/07, Richard Guenther <richard.guenther@gmail.com> wrote:
> + /* If arg0 is derived from the address of an object or function, we may
> + be able to fold this expression using the object or function's
> + alignment. */
> + if (POINTER_TYPE_P (TREE_TYPE (arg0)) &&
> + TREE_CODE (arg1) == INTEGER_CST && TREE_INT_CST_HIGH (arg1) == 0)
> + {
>
> coding style says the && goes to the next line. Instead of
> TREE_CODE (arg1) == INTEGER_CST && TREE_INT_CST_HIGH (arg1) == 0 use
> host_integer_p (arg1, 1).
OK.
> The description of get_pointer_modulus_and_residue isn't exactly clear
> to me. It looks
> like it should compute the biggest power-of-two N and M so that p % N == M? But
> this doesn't have a single solution - so what is it exactly computing?
> Please clarify
> that in the functions description.
It computes a power-of-two N and (arbitrary) M such that N divides (p
- M). This tells us that p and M have log2(N) least significant
digits in common. M is essentially arbitrary so long as these
constraints are met. Given the information at our disposal, we choose
N as large as possible such that a constant M can be determined.
I'll update the comments.
>
> + expr = get_inner_reference (expr, &bitsize, &bitpos, &offset,
> + &mode, &unsignedp, &volatilep, false);
> + *residue = bitpos / BITS_PER_UNIT;
> + if (offset)
> + {
> + if (TREE_CODE (offset) == INTEGER_CST)
> + *residue += TREE_INT_CST_LOW (offset);
> + else
> + expr = NULL;
>
> an INTEGER_CST offset is always accounted for in bitpos, no need to repeat that
> here, just bail out.
I'm pretty sure offset *can* be an INTEGER_CST. The end of
get_inner_reference looks something like:
if (host_integerp (offset, 0)) {
/* move offset into *pbitbos and return */
}
*pbitpos = tree_low_cst (bit_offset, 0);
*poffset = offset;
If a constant offset is too large to store in a HOST_WIDE_INT, it is
returned in offset instead. For our purposes, we can safely ignore
the high order bits.
Or is there some other constraint which limits the size of offsets?
> As for the function I would make it return the modulus, that would
> make the flow easier.
OK.
> + if (expr && DECL_P (expr))
> + *modulus = DECL_ALIGN_UNIT (expr);
> + else
>
> expr can't be non-NULL here.
If the "if (handled_component_p (expr))" branch gets executed and the
offset returned from get_inner_reference is not an INTEGER_CST or
NULL, then expr gets set to NULL. In all other cases, it is non-NULL.
I guess it would be easier to understand if I just returned rather
than setting expr to NULL.
> I don't see how you protect against a residue bigger
> than modulus? But I guess it doesn't matter, still the names are
> confusing then.
Since it doesn't affect the results of subsequent operations, I don't
try to limit the size of residue. Technically, it's an (arbitrary)
representative of a residue class modulo *modulus. I could put
"*residue %= *modulus" before returning if you think that would be
more comprehensible. Alternatively, I'll just make the comments
clearer on this point.
> I would still like you to re-investigate using get_pointer_alignment.
> Basically you
> would split out a helper function to get the pointer expression with
> its ADDR_EXPR
> stripped off and do
>
> base = get_inner_reference (expr, ...);
> base_align = get_pointer_alignment_1 (base, -1);
>
> where base_align yields your modulus and from the get_inner_reference result
> you compute the residue. And yes, the result from get_pointer_alignment has
> to be useful for folding as strict alignment targets rely on it for
> correct code.
Just to make sure I'm following you, are you suggesting that I should
pull the code that calculates alignments for DECL's, CONSTANT_CLASS's,
VIEW_CONVERT_EXPR's, and INDIRECT_REF's into a new function,
get_pointer_alignment_1? Here's some example code which illustrates
my concern about using get_pointer_alignment for folding.
struct S {
int i;
};
static const char c[8] __attribute__ ((aligned (4)));
static const struct S *ps = (struct S *)&c[1];
unsigned f (void)
{
return 3 & (__SIZE_TYPE__)&(ps->i);
}
On machines with strict alignment requirements, treating &c[1] as a
struct S * is illegal, but on x86, this compiles fine without
triggering a warning. This generates an ADDR_EXPR whose base is an
INDIRECT_REF. Using the logic from get_pointer_alignment, we set the
alignment to TYPE_ALIGN (TREE_TYPE (indirect_ref_expr)) which on x86
is 32 (4 bytes). If we incorporated this logic into
get_pointer_modulus_and_residue, we'd fold the BITAND_EXPR to 0, but
the correct value is 1.
Ollie
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH: fold BIT_AND_EXPR's with ADDR_EXPR operands
2007-09-20 20:33 ` Ollie Wild
@ 2007-09-20 21:11 ` Richard Guenther
2007-09-23 8:00 ` Ollie Wild
0 siblings, 1 reply; 11+ messages in thread
From: Richard Guenther @ 2007-09-20 21:11 UTC (permalink / raw)
To: Ollie Wild; +Cc: Jakub Jelinek, GCC Patches, Mark Mitchell
On 9/20/07, Ollie Wild <aaw@google.com> wrote:
> On 9/19/07, Richard Guenther <richard.guenther@gmail.com> wrote:
>
> > + /* If arg0 is derived from the address of an object or function, we may
> > + be able to fold this expression using the object or function's
> > + alignment. */
> > + if (POINTER_TYPE_P (TREE_TYPE (arg0)) &&
> > + TREE_CODE (arg1) == INTEGER_CST && TREE_INT_CST_HIGH (arg1) == 0)
> > + {
> >
> > coding style says the && goes to the next line. Instead of
> > TREE_CODE (arg1) == INTEGER_CST && TREE_INT_CST_HIGH (arg1) == 0 use
> > host_integer_p (arg1, 1).
>
> OK.
>
> > The description of get_pointer_modulus_and_residue isn't exactly clear
> > to me. It looks
> > like it should compute the biggest power-of-two N and M so that p % N == M? But
> > this doesn't have a single solution - so what is it exactly computing?
> > Please clarify
> > that in the functions description.
>
> It computes a power-of-two N and (arbitrary) M such that N divides (p
> - M). This tells us that p and M have log2(N) least significant
> digits in common. M is essentially arbitrary so long as these
> constraints are met. Given the information at our disposal, we choose
> N as large as possible such that a constant M can be determined.
>
> I'll update the comments.
Thanks.
> >
> > + expr = get_inner_reference (expr, &bitsize, &bitpos, &offset,
> > + &mode, &unsignedp, &volatilep, false);
> > + *residue = bitpos / BITS_PER_UNIT;
> > + if (offset)
> > + {
> > + if (TREE_CODE (offset) == INTEGER_CST)
> > + *residue += TREE_INT_CST_LOW (offset);
> > + else
> > + expr = NULL;
> >
> > an INTEGER_CST offset is always accounted for in bitpos, no need to repeat that
> > here, just bail out.
>
> I'm pretty sure offset *can* be an INTEGER_CST. The end of
> get_inner_reference looks something like:
>
> if (host_integerp (offset, 0)) {
> /* move offset into *pbitbos and return */
> }
>
> *pbitpos = tree_low_cst (bit_offset, 0);
> *poffset = offset;
>
> If a constant offset is too large to store in a HOST_WIDE_INT, it is
> returned in offset instead. For our purposes, we can safely ignore
> the high order bits.
>
> Or is there some other constraint which limits the size of offsets?
No, you are right.
> > As for the function I would make it return the modulus, that would
> > make the flow easier.
>
> OK.
>
> > + if (expr && DECL_P (expr))
> > + *modulus = DECL_ALIGN_UNIT (expr);
> > + else
> >
> > expr can't be non-NULL here.
>
> If the "if (handled_component_p (expr))" branch gets executed and the
> offset returned from get_inner_reference is not an INTEGER_CST or
> NULL, then expr gets set to NULL. In all other cases, it is non-NULL.
> I guess it would be easier to understand if I just returned rather
> than setting expr to NULL.
Doh, indeed, I missed that.
> > I don't see how you protect against a residue bigger
> > than modulus? But I guess it doesn't matter, still the names are
> > confusing then.
>
> Since it doesn't affect the results of subsequent operations, I don't
> try to limit the size of residue. Technically, it's an (arbitrary)
> representative of a residue class modulo *modulus. I could put
> "*residue %= *modulus" before returning if you think that would be
> more comprehensible. Alternatively, I'll just make the comments
> clearer on this point.
Comments are enough.
> > I would still like you to re-investigate using get_pointer_alignment.
> > Basically you
> > would split out a helper function to get the pointer expression with
> > its ADDR_EXPR
> > stripped off and do
> >
> > base = get_inner_reference (expr, ...);
> > base_align = get_pointer_alignment_1 (base, -1);
> >
> > where base_align yields your modulus and from the get_inner_reference result
> > you compute the residue. And yes, the result from get_pointer_alignment has
> > to be useful for folding as strict alignment targets rely on it for
> > correct code.
>
> Just to make sure I'm following you, are you suggesting that I should
> pull the code that calculates alignments for DECL's, CONSTANT_CLASS's,
> VIEW_CONVERT_EXPR's, and INDIRECT_REF's into a new function,
> get_pointer_alignment_1? Here's some example code which illustrates
> my concern about using get_pointer_alignment for folding.
>
> struct S {
> int i;
> };
>
> static const char c[8] __attribute__ ((aligned (4)));
> static const struct S *ps = (struct S *)&c[1];
>
> unsigned f (void)
> {
> return 3 & (__SIZE_TYPE__)&(ps->i);
> }
>
> On machines with strict alignment requirements, treating &c[1] as a
> struct S * is illegal, but on x86, this compiles fine without
> triggering a warning. This generates an ADDR_EXPR whose base is an
> INDIRECT_REF. Using the logic from get_pointer_alignment, we set the
> alignment to TYPE_ALIGN (TREE_TYPE (indirect_ref_expr)) which on x86
> is 32 (4 bytes). If we incorporated this logic into
> get_pointer_modulus_and_residue, we'd fold the BITAND_EXPR to 0, but
> the correct value is 1.
Hm, right. So much for less code duplication :/
There's one extra thing,
+ case POINTER_PLUS_EXPR:
+ {
+ tree op0, op1;
...
+ case MULT_EXPR:
+ op1 = TREE_OPERAND (op1, 1);
+ STRIP_NOPS (op1);
+ if (TREE_CODE (op1) == INTEGER_CST)
+ {
there shouldn't be NOPs if op1 is INTEGER_CST, so the STRIP_NOPS
is not necessary. What happens if op1 is not INTEGER_CST? Don't
we need to set modulus/residue to 1/0? For example
int a, n, m;
if (&a + n*m & 3)
...
if n == m == 1 then we leave modulus/residue to that of &a which is
wrong.
Otherwise the patch should be ok after you incorporated the changes
(please re-post it).
Thanks for your patience,
Richard.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH: fold BIT_AND_EXPR's with ADDR_EXPR operands
2007-09-20 21:11 ` Richard Guenther
@ 2007-09-23 8:00 ` Ollie Wild
2007-09-23 14:08 ` Richard Guenther
0 siblings, 1 reply; 11+ messages in thread
From: Ollie Wild @ 2007-09-23 8:00 UTC (permalink / raw)
To: Richard Guenther; +Cc: Jakub Jelinek, GCC Patches, Mark Mitchell
[-- Attachment #1: Type: text/plain, Size: 710 bytes --]
On 9/20/07, Richard Guenther <richard.guenther@gmail.com> wrote:
> Otherwise the patch should be ok after you incorporated the changes
> (please re-post it).
I've attached an updated patch. In addition to the changes we've
discussed, I converted the switch statements to if-else chains as I
believe this makes the code more legible.
Ollie
2007-09-22 Ollie Wild <aaw@google.com>
fold-const.c (fold_binary): Fold BIT_AND_EXPR's with a pointer operand.
(get_pointer_modulus_and_residue): New function.
2007-09-22 Ollie Wild <aaw@google.com>
gcc.dg/fold-bitand-1.c: New test.
gcc.dg/fold-bitand-2.c: New test.
gcc.dg/fold-bitand-3.c: New test.
gcc.dg/fold-bitand-4.c: New test.
[-- Attachment #2: fold-bitand.txt --]
[-- Type: text/plain, Size: 8004 bytes --]
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index fb664fe..5f0ee21 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -9384,6 +9384,97 @@ fold_mult_zconjz (tree type, tree expr)
}
+/* Subroutine of fold_binary. If P is the value of EXPR, computes
+ power-of-two M and (arbitrary) N such that M divides (P-N). This condition
+ guarantees that P and N have the same least significant log2(M) bits.
+ N is not otherwise constrained. In particular, N is not normalized to
+ 0 <= N < M as is common. In general, the precise value of P is unknown.
+ M is chosen as large as possible such that constant N can be determined.
+
+ Returns M and sets *RESIDUE to N. */
+
+static unsigned HOST_WIDE_INT
+get_pointer_modulus_and_residue (tree expr, unsigned HOST_WIDE_INT *residue)
+{
+ enum tree_code code;
+
+ *residue = 0;
+
+ code = TREE_CODE (expr);
+ if (code == ADDR_EXPR)
+ {
+ expr = TREE_OPERAND (expr, 0);
+ if (handled_component_p (expr))
+ {
+ HOST_WIDE_INT bitsize, bitpos;
+ tree offset;
+ enum machine_mode mode;
+ int unsignedp, volatilep;
+
+ expr = get_inner_reference (expr, &bitsize, &bitpos, &offset,
+ &mode, &unsignedp, &volatilep, false);
+ *residue = bitpos / BITS_PER_UNIT;
+ if (offset)
+ {
+ if (TREE_CODE (offset) == INTEGER_CST)
+ *residue += TREE_INT_CST_LOW (offset);
+ else
+ /* We don't handle more complicated offset expressions. */
+ return 1;
+ }
+ }
+
+ if (DECL_P (expr))
+ return DECL_ALIGN_UNIT (expr);
+ }
+ else if (code == POINTER_PLUS_EXPR)
+ {
+ tree op0, op1;
+ unsigned HOST_WIDE_INT modulus;
+ enum tree_code inner_code;
+
+ op0 = TREE_OPERAND (expr, 0);
+ STRIP_NOPS (op0);
+ modulus = get_pointer_modulus_and_residue (op0, residue);
+
+ op1 = TREE_OPERAND (expr, 1);
+ STRIP_NOPS (op1);
+ inner_code = TREE_CODE (op1);
+ if (inner_code == INTEGER_CST)
+ {
+ *residue += TREE_INT_CST_LOW (op1);
+ return modulus;
+ }
+ else if (inner_code == MULT_EXPR)
+ {
+ op1 = TREE_OPERAND (op1, 1);
+ if (TREE_CODE (op1) == INTEGER_CST)
+ {
+ unsigned HOST_WIDE_INT align;
+
+ /* Compute the greatest power-of-2 divisor of op1. */
+ align = TREE_INT_CST_LOW (op1);
+ align &= -align;
+
+ /* If align is non-zero and less than *modulus, replace
+ *modulus with align., If align is 0, then either op1 is 0
+ or the greatest power-of-2 divisor of op1 doesn't fit in an
+ unsigned HOST_WIDE_INT. In either case, no additional
+ constraint is imposed. */
+ if (align)
+ modulus = MIN (modulus, align);
+
+ return modulus;
+ }
+ }
+ }
+
+ /* If we get here, we were unable to determine anything useful about the
+ expression. */
+ return 1;
+}
+
+
/* Fold a binary expression of code CODE and type TYPE with operands
OP0 and OP1. Return the folded expression if folding is
successful. Otherwise, return NULL_TREE. */
@@ -10913,6 +11004,23 @@ fold_binary (enum tree_code code, tree type, tree op0, tree op1)
TREE_OPERAND (arg1, 0)));
}
+ /* If arg0 is derived from the address of an object or function, we may
+ be able to fold this expression using the object or function's
+ alignment. */
+ if (POINTER_TYPE_P (TREE_TYPE (arg0)) && host_integerp (arg1, 1))
+ {
+ unsigned HOST_WIDE_INT modulus, residue;
+ unsigned HOST_WIDE_INT low = TREE_INT_CST_LOW (arg1);
+
+ modulus = get_pointer_modulus_and_residue (arg0, &residue);
+
+ /* This works because modulus is a power of 2. If this weren't the
+ case, we'd have to replace it by its greatest power-of-2
+ divisor: modulus & -modulus. */
+ if (low < modulus)
+ return build_int_cst (type, residue & low);
+ }
+
goto associate;
case RDIV_EXPR:
diff --git a/gcc/testsuite/gcc.dg/fold-bitand-1.c b/gcc/testsuite/gcc.dg/fold-bitand-1.c
new file mode 100644
index 0000000..413ed67
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/fold-bitand-1.c
@@ -0,0 +1,34 @@
+/* { dg-do compile } */
+/* { dg-options "-fdump-tree-original" } */
+
+char c1 __attribute__ ((aligned (1)));
+char c2 __attribute__ ((aligned (2)));
+char c4 __attribute__ ((aligned (4)));
+char c8 __attribute__ ((aligned (8)));
+
+unsigned f1(void)
+{
+ return 3 & (__SIZE_TYPE__)&c1;
+}
+
+unsigned f2(void)
+{
+ return 3 & (__SIZE_TYPE__)&c2;
+}
+
+unsigned f3(void)
+{
+ return 3 & (__SIZE_TYPE__)&c4;
+}
+
+unsigned f4(void)
+{
+ return 3 & (__SIZE_TYPE__)&c8;
+}
+
+/* { dg-final { scan-tree-dump-times "\&c1 \& 3" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "\&c2 \& 3" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "\&c4 \& 3" 0 "original" } } */
+/* { dg-final { scan-tree-dump-times "\&c8 \& 3" 0 "original" } } */
+/* { dg-final { scan-tree-dump-times "return 0" 2 "original" } } */
+/* { dg-final { cleanup-tree-dump "original" } } */
diff --git a/gcc/testsuite/gcc.dg/fold-bitand-2.c b/gcc/testsuite/gcc.dg/fold-bitand-2.c
new file mode 100644
index 0000000..de2e674
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/fold-bitand-2.c
@@ -0,0 +1,42 @@
+/* { dg-do compile } */
+/* { dg-options "-fdump-tree-original" } */
+
+struct {
+ char c1;
+ char c2;
+ char c3;
+ char c4;
+} s __attribute__ ((aligned (4)));
+
+unsigned f1 (void)
+{
+ return 3 & (__SIZE_TYPE__)&s.c1;
+}
+
+unsigned f2 (void)
+{
+ return 3 & (__SIZE_TYPE__)&s.c2;
+}
+
+unsigned f3 (void)
+{
+ return 3 & (__SIZE_TYPE__)&s.c3;
+}
+
+unsigned f4 (void)
+{
+ return 3 & (__SIZE_TYPE__)&s.c4;
+}
+
+unsigned f5 (void)
+{
+ return 4 & (__SIZE_TYPE__)&s.c1;
+}
+
+/* { dg-final { scan-tree-dump-times "\& 3" 0 "original" } } */
+/* { dg-final { scan-tree-dump-times "\& 4" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "return 0" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "return 1" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "return 2" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "return 3" 1 "original" } } */
+/* { dg-final { cleanup-tree-dump "original" } } */
diff --git a/gcc/testsuite/gcc.dg/fold-bitand-3.c b/gcc/testsuite/gcc.dg/fold-bitand-3.c
new file mode 100644
index 0000000..43d765b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/fold-bitand-3.c
@@ -0,0 +1,25 @@
+/* { dg-do compile } */
+/* { dg-options "-fdump-tree-original" } */
+
+char c[4] __attribute__ ((aligned (4)));
+
+struct S {
+ char c1;
+ char c2;
+ char c3;
+ char c4;
+};
+
+int f1 (void)
+{
+ return 3 & (__SIZE_TYPE__)&c[1];
+}
+
+int f2 (void)
+{
+ return 3 & (__SIZE_TYPE__)&((struct S *)&c)->c2;
+}
+
+/* { dg-final { scan-tree-dump-times "\& 3" 0 "original" } } */
+/* { dg-final { scan-tree-dump-times "return 1" 2 "original" } } */
+/* { dg-final { cleanup-tree-dump "original" } } */
diff --git a/gcc/testsuite/gcc.dg/fold-bitand-4.c b/gcc/testsuite/gcc.dg/fold-bitand-4.c
new file mode 100644
index 0000000..7d82426
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/fold-bitand-4.c
@@ -0,0 +1,44 @@
+/* { dg-do compile } */
+/* { dg-options "-fdump-tree-original" } */
+
+typedef char char4[4] __attribute__ ((aligned (4)));
+char4 c4[4] __attribute__ ((aligned (16)));
+
+typedef char char16[16] __attribute__ ((aligned (16)));
+char16 c16[4] __attribute__ ((aligned (4)));
+
+int f1 (void)
+{
+ /* 12 */
+ return 15 & (__SIZE_TYPE__)&c4[3];
+}
+
+int f2 (int i)
+{
+ /* Indeterminate */
+ return 15 & (__SIZE_TYPE__)&c4[i];
+}
+
+int f3 (int i)
+{
+ /* 0 */
+ return 3 & (__SIZE_TYPE__)&c4[i];
+}
+
+int f4 (int i)
+{
+ /* Indeterminate */
+ return 7 & (__SIZE_TYPE__)&c16[i];
+}
+
+int f5 (int i)
+{
+ /* 0 */
+ return 3 & (__SIZE_TYPE__)&c16[i];
+}
+
+/* { dg-final { scan-tree-dump-times "return 12" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "\& 15" 1 "original" } } */
+/* { dg-final { scan-tree-dump-times "return 0" 2 "original" } } */
+/* { dg-final { scan-tree-dump-times "\& 7" 1 "original" } } */
+/* { dg-final { cleanup-tree-dump "original" } } */
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH: fold BIT_AND_EXPR's with ADDR_EXPR operands
2007-09-23 8:00 ` Ollie Wild
@ 2007-09-23 14:08 ` Richard Guenther
2007-09-24 0:08 ` Ollie Wild
0 siblings, 1 reply; 11+ messages in thread
From: Richard Guenther @ 2007-09-23 14:08 UTC (permalink / raw)
To: Ollie Wild; +Cc: Jakub Jelinek, GCC Patches, Mark Mitchell
On 9/23/07, Ollie Wild <aaw@google.com> wrote:
> On 9/20/07, Richard Guenther <richard.guenther@gmail.com> wrote:
>
> > Otherwise the patch should be ok after you incorporated the changes
> > (please re-post it).
>
> I've attached an updated patch. In addition to the changes we've
> discussed, I converted the switch statements to if-else chains as I
> believe this makes the code more legible.
This is ok for mainline if it passed bootstrap & regtest.
Thanks,
Richard.
> Ollie
>
> 2007-09-22 Ollie Wild <aaw@google.com>
>
> fold-const.c (fold_binary): Fold BIT_AND_EXPR's with a pointer operand.
> (get_pointer_modulus_and_residue): New function.
>
> 2007-09-22 Ollie Wild <aaw@google.com>
>
> gcc.dg/fold-bitand-1.c: New test.
> gcc.dg/fold-bitand-2.c: New test.
> gcc.dg/fold-bitand-3.c: New test.
> gcc.dg/fold-bitand-4.c: New test.
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH: fold BIT_AND_EXPR's with ADDR_EXPR operands
2007-09-23 14:08 ` Richard Guenther
@ 2007-09-24 0:08 ` Ollie Wild
0 siblings, 0 replies; 11+ messages in thread
From: Ollie Wild @ 2007-09-24 0:08 UTC (permalink / raw)
To: Richard Guenther; +Cc: Jakub Jelinek, GCC Patches, Mark Mitchell
On 9/23/07, Richard Guenther <richard.guenther@gmail.com> wrote:
>
> This is ok for mainline if it passed bootstrap & regtest.
It did. Submitting now.
Thanks.
Ollie
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2007-09-23 19:58 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-09-11 6:13 PATCH: fold BIT_AND_EXPR's with ADDR_EXPR operands Ollie Wild
2007-09-11 9:56 ` Richard Guenther
2007-09-18 9:02 ` Ollie Wild
2007-09-18 9:10 ` Jakub Jelinek
2007-09-19 6:57 ` Ollie Wild
2007-09-19 9:19 ` Richard Guenther
2007-09-20 20:33 ` Ollie Wild
2007-09-20 21:11 ` Richard Guenther
2007-09-23 8:00 ` Ollie Wild
2007-09-23 14:08 ` Richard Guenther
2007-09-24 0:08 ` Ollie Wild
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).