This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] Fix PR69553
- From: Richard Biener <rguenther at suse dot de>
- To: gcc-patches at gcc dot gnu dot org
- Date: Wed, 17 Feb 2016 10:09:24 +0100 (CET)
- Subject: [PATCH] Fix PR69553
- Authentication-results: sourceware.org; auth=none
Honza added skipping for the (broken) type-comparing code in
operand_equal_p for OEP_ADDRESS_OF. This exposes the fact that
both IMAGPART_EXPR and ARRAY_REF lack comparison of their offsetting
effect.
In fact the issue is somewhat latent as for example nothing
verified before that we don't compare two ARRAY_REFs with different
low bound. Looking at TYPE_PRECISION or TYPE_UNSIGNED for ARRAY_TYPEs
certainly doesn't do this properly.
So the following adds proper comparison of offsetting effects.
Unfortunately due to PLACEHOLDER_EXPR handling both
array_ref_low_bound and array_ref_element_size are mutating (ugh)
and probably more costly than needed. COMPONENT_REF gets away
with comparing FIELD_DECLs for pointer equality so eventually
we could get away with comparing TYPE_DOMAIN for pointer equality
as well as TYPE_SIZE. In the end we should require ops 2 and 3
to be always present (even if integer constants), that would simplify
things a lot. Or maybe have simpler variants for after gimplification.
Or remove PLACEHOLDER_EXPR from GENERIC and force Ada to lower those.
Well.
The following patch at least fixes the wrong-code issue Honza exposed.
We still need to fix the bogus type compatibility check, but that
doesn't look like a regression. Maybe sth like
Index: gcc/fold-const.c
===================================================================
*** gcc/fold-const.c (revision 233447)
--- gcc/fold-const.c (working copy)
*************** operand_equal_p (const_tree arg0, const_
*** 2779,2784 ****
--- 2779,2790 ----
if (!(flags & OEP_ADDRESS_OF))
{
+ if (AGGREGATE_TYPE_P (TREE_TYPE (arg0))
+ != AGGREGATE_TYPE_P (TREE_TYPE (arg1)))
+ return 0;
+
+ if (! AGGREGATE_TYPE_P (TREE_TYPE (arg0)))
+ {
/* If both types don't have the same signedness, then we can't
consider
them equal. We must check this before the STRIP_NOPS calls
because they may change the signedness of the arguments. As
pointers
*************** operand_equal_p (const_tree arg0, const_
*** 2798,2803 ****
--- 2804,2810 ----
STRIP_NOPS (arg0);
STRIP_NOPS (arg1);
}
+ }
#if 0
/* FIXME: Fortran FE currently produce ADDR_EXPR of NOP_EXPR. Enable
the
sanity check once the issue is solved. */
but well...
Bootstrap and regtest running on x86_64-unknown-linux-gnu.
I'm not too happy about calling the mutating array_ref_low_bound
or array_ref_element_size here but I'm not sure what reliable
alternative we'd have (compare TYPE_SIZE and TYPE_DOMAIN TYPE_MIN_VALUE?).
Richard.
2016-02-17 Richard Biener <rguenther@suse.de>
PR middle-end/69553
* fold-const.c (operand_equal_p): Properly compare offsets for
IMAGPART_EXPR and ARRAY_REF.
* g++.dg/torture/pr69553.C: New testcase.
Index: gcc/fold-const.c
===================================================================
*** gcc/fold-const.c (revision 233483)
--- gcc/fold-const.c (working copy)
*************** operand_equal_p (const_tree arg0, const_
*** 3008,3015 ****
flags &= ~OEP_ADDRESS_OF;
return OP_SAME (0);
- case REALPART_EXPR:
case IMAGPART_EXPR:
case VIEW_CONVERT_EXPR:
return OP_SAME (0);
--- 3008,3022 ----
flags &= ~OEP_ADDRESS_OF;
return OP_SAME (0);
case IMAGPART_EXPR:
+ /* Require the same offset. */
+ if (!operand_equal_p (TYPE_SIZE (TREE_TYPE (arg0)),
+ TYPE_SIZE (TREE_TYPE (arg1)),
+ flags & ~OEP_ADDRESS_OF))
+ return 0;
+
+ /* Fallthru. */
+ case REALPART_EXPR:
case VIEW_CONVERT_EXPR:
return OP_SAME (0);
*************** operand_equal_p (const_tree arg0, const_
*** 3049,3065 ****
case ARRAY_REF:
case ARRAY_RANGE_REF:
- /* Operands 2 and 3 may be null.
- Compare the array index by value if it is constant first as we
- may have different types but same value here. */
if (!OP_SAME (0))
return 0;
flags &= ~OEP_ADDRESS_OF;
return ((tree_int_cst_equal (TREE_OPERAND (arg0, 1),
TREE_OPERAND (arg1, 1))
|| OP_SAME (1))
&& OP_SAME_WITH_NULL (2)
! && OP_SAME_WITH_NULL (3));
case COMPONENT_REF:
/* Handle operand 2 the same as for ARRAY_REF. Operand 0
--- 3056,3084 ----
case ARRAY_REF:
case ARRAY_RANGE_REF:
if (!OP_SAME (0))
return 0;
flags &= ~OEP_ADDRESS_OF;
+ /* Compare the array index by value if it is constant first as we
+ may have different types but same value here. */
return ((tree_int_cst_equal (TREE_OPERAND (arg0, 1),
TREE_OPERAND (arg1, 1))
|| OP_SAME (1))
&& OP_SAME_WITH_NULL (2)
! && OP_SAME_WITH_NULL (3)
! /* Compare low bound and element size as with OEP_ADDRESS_OF
! we have to account for the offset of the ref. */
! && (TREE_TYPE (TREE_OPERAND (arg0, 0))
! == TREE_TYPE (TREE_OPERAND (arg1, 0))
! || (operand_equal_p (array_ref_low_bound
! (CONST_CAST_TREE (arg0)),
! array_ref_low_bound
! (CONST_CAST_TREE (arg1)), flags)
! && operand_equal_p (array_ref_element_size
! (CONST_CAST_TREE (arg0)),
! array_ref_element_size
! (CONST_CAST_TREE (arg1)),
! flags))));
case COMPONENT_REF:
/* Handle operand 2 the same as for ARRAY_REF. Operand 0
Index: gcc/testsuite/g++.dg/torture/pr69553.C
===================================================================
*** gcc/testsuite/g++.dg/torture/pr69553.C (revision 0)
--- gcc/testsuite/g++.dg/torture/pr69553.C (revision 0)
***************
*** 0 ****
--- 1,35 ----
+ // { dg-do run }
+ template <typename _Tp, long _Nm> struct A {
+ typedef _Tp _Type[_Nm];
+ static _Tp &_S_ref(const _Type &p1, int p2) {
+ return const_cast<_Tp &>(p1[p2]);
+ }
+ };
+ template <typename _Tp, long _Nm> struct B {
+ typedef A<_Tp, _Nm> _AT_Type;
+ typename _AT_Type::_Type _M_elems;
+ _Tp &operator[](long p1) const { return _AT_Type::_S_ref(_M_elems, p1); }
+ };
+ int t;
+ void foo(int p1, int &p2) {
+ if ((t & 1) == 0) {
+ if (p1 != 1)
+ __builtin_abort();
+ if (p2 != 2)
+ __builtin_abort();
+ }
+ t++;
+ }
+ __attribute__((noinline))
+ void test1(const B<int, 2> &p1) { foo(p1[0], p1[1]); }
+ void test(B<B<int, 2>, 2> &p1) {
+ test1(p1[0]);
+ test1(p1[1]);
+ foo(p1[0][0], p1[0][1]);
+ }
+ int main() {
+ B<B<int, 2>, 2> t;
+ t[0][0] = 1;
+ t[0][1] = 2;
+ test(t);
+ }