On Mon, 4 Aug 2014, Marek Polacek wrote:
> This PR is about bogus overflow warning that we issue for e.g.
> int *q = &i + 1;
> q - (q - 1);
> because pointer_diff receives p - (p + -1U) which gets simplified to
> 1U - with overflow. We could drop the overflow flag to suppress the
> warning, but I think we should just remove the optimization
> altogether. First, FE shouldn't perform such transformations at
> all. Second, C++ FE has its own pointer_diff function that doesn't
> do such optimization. With this patch, the C FE will generate the
> same expression as the C++ FE. It's true that we should try to
> optimize this, but not in the front end. It ought to be easy to
> write a pattern for match-and-simplify that would handle this.
I think that tree-ssa-forwprop.c already simplifies this in
associate_plusminus with (T)(P + A) - (T)P -> (T)A. Well,
maybe not - but then the code should be massages to handle it.
Can you double-check and do that?
Otherwise I agree with removing this (and other) "premature"
optimizations scattered throughout the frontends (and convert.c).
Thanks,
Richard.
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2014-08-04 Marek Polacek <pola...@redhat.com>
>
> PR c/61240
> * c-typeck.c (pointer_diff): Remove P - (P + CST) optimization.
>
> * gcc.dg/pr61240.c: New test.
>
> diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
> index fe9440c..5f46944 100644
> --- gcc/c/c-typeck.c
> +++ gcc/c/c-typeck.c
> @@ -3460,7 +3460,6 @@ pointer_diff (location_t loc, tree op0, tree op1)
> addr_space_t as0 = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (op0)));
> addr_space_t as1 = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (op1)));
> tree target_type = TREE_TYPE (TREE_TYPE (op0));
> - tree con0, con1, lit0, lit1;
> tree orig_op1 = op1;
>
> /* If the operands point into different address spaces, we need to
> @@ -3490,7 +3489,6 @@ pointer_diff (location_t loc, tree op0, tree op1)
> else
> inttype = restype;
>
> -
> if (TREE_CODE (target_type) == VOID_TYPE)
> pedwarn (loc, OPT_Wpointer_arith,
> "pointer of type %<void *%> used in subtraction");
> @@ -3498,50 +3496,6 @@ pointer_diff (location_t loc, tree op0, tree op1)
> pedwarn (loc, OPT_Wpointer_arith,
> "pointer to a function used in subtraction");
>
> - /* If the conversion to ptrdiff_type does anything like widening or
> - converting a partial to an integral mode, we get a convert_expression
> - that is in the way to do any simplifications.
> - (fold-const.c doesn't know that the extra bits won't be needed.
> - split_tree uses STRIP_SIGN_NOPS, which leaves conversions to a
> - different mode in place.)
> - So first try to find a common term here 'by hand'; we want to cover
> - at least the cases that occur in legal static initializers. */
> - if (CONVERT_EXPR_P (op0)
> - && (TYPE_PRECISION (TREE_TYPE (op0))
> - == TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (op0, 0)))))
> - con0 = TREE_OPERAND (op0, 0);
> - else
> - con0 = op0;
> - if (CONVERT_EXPR_P (op1)
> - && (TYPE_PRECISION (TREE_TYPE (op1))
> - == TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (op1, 0)))))
> - con1 = TREE_OPERAND (op1, 0);
> - else
> - con1 = op1;
> -
> - if (TREE_CODE (con0) == POINTER_PLUS_EXPR)
> - {
> - lit0 = TREE_OPERAND (con0, 1);
> - con0 = TREE_OPERAND (con0, 0);
> - }
> - else
> - lit0 = integer_zero_node;
> -
> - if (TREE_CODE (con1) == POINTER_PLUS_EXPR)
> - {
> - lit1 = TREE_OPERAND (con1, 1);
> - con1 = TREE_OPERAND (con1, 0);
> - }
> - else
> - lit1 = integer_zero_node;
> -
> - if (operand_equal_p (con0, con1, 0))
> - {
> - op0 = lit0;
> - op1 = lit1;
> - }
> -
> -
> /* First do the subtraction as integers;
> then drop through to build the divide operator.
> Do not do default conversions on the minus operator
> diff --git gcc/testsuite/gcc.dg/pr61240.c gcc/testsuite/gcc.dg/pr61240.c
> index e69de29..e90c070 100644
> --- gcc/testsuite/gcc.dg/pr61240.c
> +++ gcc/testsuite/gcc.dg/pr61240.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +
> +void
> +foo (void)
> +{
> + volatile __PTRDIFF_TYPE__ t;
> + int i;
> + int *p = &i;
> + int *q = &i + 1;
> + t = q - (q - 1);
> + t = (q - 1) - q; /* { dg-warning "integer overflow in expression" } */
> + t = p - (p - 1);
> + t = (p - 1) - p ; /* { dg-warning "integer overflow in expression" } */
> +}
>
> Marek
>
>
--
Richard Biener <rguent...@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer
