| Message ID | alpine.DEB.2.20.1710281133200.6751@stedding.saclay.inria.fr |
|---|---|
| State | New |
| Headers | show |
| Series | [RFTesting] New POINTER_DIFF_EXPR | expand |
Adding some random cc: to people who might be affected. Hopefully I am not breaking any of your stuff... Ulrich Weigand (address space) Ilya Enkovich (pointer bound check) DJ Delorie (target with 24-bit partial mode pointer) If you want to give it a try, or just take a quick look to check that you are obviously not affected, that would be nice, but don't feel forced. Here is an updated version of the patch, with just a few more transformations in match.pd, to match some MINUS_EXPR optimizations I missed the first time: -(A-B), X-Z<Y-Z, (X-Z)-(Y-Z), Z-X<Z-Y, (Z-X)-(Z-Y), (A-B)+(C-A) The exact undefined-behavior status should probably be clarified more. First, I'd like to say that POINTER_DIFF_EXPR may only take 2 pointers into the same "object" (like in C/C++), so they differ by at most half the size of the address space, and a-b is not allowed to be the minimum negative value (so I can safely use b-a), and if we compute a-b and b-c, then a and c are in the same object so I can safely compute a-c, etc. Second, we probably need modes without undefined behavior, wrapping with either -fwrapv or a new -fwrapp, or sanitized. For the sanitized version, we could keep using POINTER_DIFF_EXPR and check TYPE_OVERFLOW_SANITIZED on the pointer type as we currently do for integers. For wrapping, either we say that POINTER_DIFF_EXPR has wrapping semantics when that option is in effect, or we do not use POINTER_DIFF_EXPR and instead cast to unsigned before applying a MINUS_EXPR (my preference). On Sat, 28 Oct 2017, Marc Glisse wrote: > Hello, > > first, if you are doing anything unusual with pointers (address spaces, > pointer/sizetype with weird sizes, instrumentation, etc), it would be great > if you could give this patch a try. It was bootstrapped and regtested on > powerpc64le-unknown-linux-gnu (gcc112), and a slightly older version on > x86_64-pc-linux-gnu (skylake laptop). I also built bare cross-compilers (no > sysroot or anything) for avr, m32c, alpha64-vms, s390-linux, and visium to > check that on trivial examples it behaves as expected (by the way, m32c seems > broken for unrelated reasons at the moment), but I wouldn't count that as > complete testing. > > This was previously discussed in the thread "Fix pointer diff (was: > -fsanitize=pointer-overflow support (PR sanitizer/80998))" ( > https://gcc.gnu.org/ml/gcc-patches/2017-10/msg02128.html for the latest > message). > > Front-ends other than C/C++ can be changed later (I took a quick look at > fortran and ada, but they are way too unfamiliar to me), I did not remove any > handling for the other representations. > > 2017-10-28 Marc Glisse <marc.glisse@inria.fr> > > gcc/c/ > * c-fold.c (c_fully_fold_internal): Handle POINTER_DIFF_EXPR. > * c-typeck.c (pointer_diff): Use POINTER_DIFF_EXPR. > > gcc/c-family/ > * c-pretty-print.c (pp_c_additive_expression, > c_pretty_printer::expression): Handle POINTER_DIFF_EXPR. > > gcc/cp/ > * constexpr.c (cxx_eval_constant_expression, > potential_constant_expression_1): Handle POINTER_DIFF_EXPR. > * cp-gimplify.c (cp_fold): Likewise. > * error.c (dump_expr): Likewise. > * typeck.c (cp_build_binary_op): Likewise. > (pointer_diff): Use POINTER_DIFF_EXPR. > > gcc/ > * doc/generic.texi: Document POINTER_DIFF_EXPR, update > POINTER_PLUS_EXPR. > * cfgexpand.c (expand_debug_expr): Handle POINTER_DIFF_EXPR. > * expr.c (expand_expr_real_2): Likewise. > * fold-const.c (const_binop, const_binop, > fold_addr_of_array_ref_difference, fold_binary_loc): Likewise. > * match.pd (X-X, P+(Q-P), &D-P, (P+N)-P, P-(P+N), (P+M)-(P+N), > P-Q==0): New transformations for POINTER_DIFF_EXPR, based on > MINUS_EXPR transformations. > * optabs-tree.c (optab_for_tree_code): Handle POINTER_DIFF_EXPR. > * tree-cfg.c (verify_expr, verify_gimple_assign_binary): Likewise. > * tree-inline.c (estimate_operator_cost): Likewise. > * tree-pretty-print.c (dump_generic_node, op_code_prio, > op_symbol_code): Likewise. > * tree-vect-stmts.c (vectorizable_operation): Likewise. > * tree-vrp.c (extract_range_from_binary_expr): Likewise. > * varasm.c (initializer_constant_valid_p_1): Likewise. > * tree.def: New tree code POINTER_DIFF_EXPR.
On Sat, Nov 11, 2017 at 12:44 AM, Marc Glisse <marc.glisse@inria.fr> wrote: > Adding some random cc: to people who might be affected. Hopefully I am not > breaking any of your stuff... > > Ulrich Weigand (address space) > Ilya Enkovich (pointer bound check) > DJ Delorie (target with 24-bit partial mode pointer) > > If you want to give it a try, or just take a quick look to check that you > are obviously not affected, that would be nice, but don't feel forced. > > Here is an updated version of the patch, with just a few more > transformations in match.pd, to match some MINUS_EXPR optimizations I missed > the first time: -(A-B), X-Z<Y-Z, (X-Z)-(Y-Z), Z-X<Z-Y, (Z-X)-(Z-Y), > (A-B)+(C-A) > > The exact undefined-behavior status should probably be clarified more. > First, I'd like to say that POINTER_DIFF_EXPR may only take 2 pointers into > the same "object" (like in C/C++), so they differ by at most half the size > of the address space, and a-b is not allowed to be the minimum negative > value (so I can safely use b-a), and if we compute a-b and b-c, then a and c > are in the same object so I can safely compute a-c, etc. Second, we probably > need modes without undefined behavior, wrapping with either -fwrapv or a new > -fwrapp, or sanitized. For the sanitized version, we could keep using > POINTER_DIFF_EXPR and check TYPE_OVERFLOW_SANITIZED on the pointer type as > we currently do for integers. For wrapping, either we say that > POINTER_DIFF_EXPR has wrapping semantics when that option is in effect, or > we do not use POINTER_DIFF_EXPR and instead cast to unsigned before applying > a MINUS_EXPR (my preference). CCing C/C++ FE folks as well. + /* It is better if the caller provides the return type. */ + if (code == POINTER_DIFF_EXPR) + { + offset_int res = wi::sub (wi::to_offset (arg1), + wi::to_offset (arg2)); + return force_fit_type (signed_type_for (TREE_TYPE (arg1)), res, 1, + TREE_OVERFLOW (arg1) | TREE_OVERFLOW (arg2)); + } + there's an overload that provides the type... (we should probably go over all callers and use that). There is already a folding that is only done when the type is available. So I'd simply remove the above from the non-type overload handling. What breaks then? With the patch it's of course somewhat ugly in that we need to deal with both MINUS_EXPR on pointers and POINTER_DIFF_EXPR - at least that's what + case POINTER_DIFF_EXPR: case MINUS_EXPR: + /* Fold &a[i] - &a[j] to i-j. */ + if (TREE_CODE (arg0) == ADDR_EXPR + && TREE_CODE (TREE_OPERAND (arg0, 0)) == ARRAY_REF + && TREE_CODE (arg1) == ADDR_EXPR + && TREE_CODE (TREE_OPERAND (arg1, 0)) == ARRAY_REF) + { + tree tem = fold_addr_of_array_ref_difference (loc, type, + TREE_OPERAND (arg0, 0), + TREE_OPERAND (arg1, 0), + code + == POINTER_DIFF_EXPR); suggests. But is that really so? The FEs today should never build a MINUS_EXPR with pointer operands and your patch also doesn't. I suppose we only arrive above because STRIP_NOPS strips the pointer to integer conversion? + case POINTER_DIFF_EXPR: + if (!POINTER_TYPE_P (TREE_TYPE (TREE_OPERAND (t, 0))) + || !POINTER_TYPE_P (TREE_TYPE (TREE_OPERAND (t, 1)))) + { + error ("invalid operand to pointer diff, operand is not a pointer"); + return t; + } + CHECK_OP (0, "invalid operand to pointer diff"); + CHECK_OP (1, "invalid operand to pointer diff"); + break; can you add || TREE_CODE (TREE_TYPE (t)) != INTEGER_TYPE || TYPE_UNSIGNED (TREE_TYPE (t)) ? Note that if the precision of the pointer type arguments are not equal to the precision of the return value then foldings like (simplify + (plus:c (pointer_diff @0 @1) (pointer_diff @2 @0)) + (if (TYPE_OVERFLOW_UNDEFINED (type) + && !TYPE_OVERFLOW_SANITIZED (TREE_TYPE (@0))) + (pointer_diff @2 @1))) might not be correct as we drop a possible truncation here. Shall we require (and document) that the result of the pointer difference in a POINTER_DIFF_EXPR has to be representable in the type of the expression, otherwise the behavior is undefined? Shall we allow an unsigned return type for differences known to be representable? Probably not... Does the behavior depend on TYPE_OVERFLOW_UNDEFINED? It would be nice to amend the generic.texi docs somewhat here. Ah, the gimple checking adds some more bits here that clarify things: + case POINTER_DIFF_EXPR: + { + if (!POINTER_TYPE_P (rhs1_type) + || !POINTER_TYPE_P (rhs2_type) + || !types_compatible_p (rhs1_type, rhs2_type) + || TREE_CODE (lhs_type) != INTEGER_TYPE + || TYPE_UNSIGNED (lhs_type) + || TYPE_PRECISION (lhs_type) != TYPE_PRECISION (rhs1_type)) + { + error ("type mismatch in pointer diff expression"); + debug_generic_stmt (lhs_type); + debug_generic_stmt (rhs1_type); + debug_generic_stmt (rhs2_type); + return true; + } I think the patch is ok for trunk, the FE bits should get approval from their maintainers. Joseph may have an idea about the address-space issue. With this in place we should be able to fix some of the issues Jakub ran into with pointer sanitizing? Also with this in place it would be nice to do the corresponding adjustment to POINTER_PLUS_EXPR, that is, require a _signed_ offset type that matches the precision of the other argument. Richard. > > > On Sat, 28 Oct 2017, Marc Glisse wrote: > >> Hello, >> >> first, if you are doing anything unusual with pointers (address spaces, >> pointer/sizetype with weird sizes, instrumentation, etc), it would be great >> if you could give this patch a try. It was bootstrapped and regtested on >> powerpc64le-unknown-linux-gnu (gcc112), and a slightly older version on >> x86_64-pc-linux-gnu (skylake laptop). I also built bare cross-compilers (no >> sysroot or anything) for avr, m32c, alpha64-vms, s390-linux, and visium to >> check that on trivial examples it behaves as expected (by the way, m32c >> seems broken for unrelated reasons at the moment), but I wouldn't count that >> as complete testing. >> >> This was previously discussed in the thread "Fix pointer diff (was: >> -fsanitize=pointer-overflow support (PR sanitizer/80998))" ( >> https://gcc.gnu.org/ml/gcc-patches/2017-10/msg02128.html for the latest >> message). >> >> Front-ends other than C/C++ can be changed later (I took a quick look at >> fortran and ada, but they are way too unfamiliar to me), I did not remove >> any handling for the other representations. >> >> 2017-10-28 Marc Glisse <marc.glisse@inria.fr> >> >> gcc/c/ >> * c-fold.c (c_fully_fold_internal): Handle POINTER_DIFF_EXPR. >> * c-typeck.c (pointer_diff): Use POINTER_DIFF_EXPR. >> >> gcc/c-family/ >> * c-pretty-print.c (pp_c_additive_expression, >> c_pretty_printer::expression): Handle POINTER_DIFF_EXPR. >> >> gcc/cp/ >> * constexpr.c (cxx_eval_constant_expression, >> potential_constant_expression_1): Handle POINTER_DIFF_EXPR. >> * cp-gimplify.c (cp_fold): Likewise. >> * error.c (dump_expr): Likewise. >> * typeck.c (cp_build_binary_op): Likewise. >> (pointer_diff): Use POINTER_DIFF_EXPR. >> >> gcc/ >> * doc/generic.texi: Document POINTER_DIFF_EXPR, update >> POINTER_PLUS_EXPR. >> * cfgexpand.c (expand_debug_expr): Handle POINTER_DIFF_EXPR. >> * expr.c (expand_expr_real_2): Likewise. >> * fold-const.c (const_binop, const_binop, >> fold_addr_of_array_ref_difference, fold_binary_loc): Likewise. >> * match.pd (X-X, P+(Q-P), &D-P, (P+N)-P, P-(P+N), (P+M)-(P+N), >> P-Q==0): New transformations for POINTER_DIFF_EXPR, based on >> MINUS_EXPR transformations. >> * optabs-tree.c (optab_for_tree_code): Handle POINTER_DIFF_EXPR. >> * tree-cfg.c (verify_expr, verify_gimple_assign_binary): Likewise. >> * tree-inline.c (estimate_operator_cost): Likewise. >> * tree-pretty-print.c (dump_generic_node, op_code_prio, >> op_symbol_code): Likewise. >> * tree-vect-stmts.c (vectorizable_operation): Likewise. >> * tree-vrp.c (extract_range_from_binary_expr): Likewise. >> * varasm.c (initializer_constant_valid_p_1): Likewise. >> * tree.def: New tree code POINTER_DIFF_EXPR. > > > -- > Marc Glisse
On Fri, Nov 17, 2017 at 7:56 AM, Richard Biener <richard.guenther@gmail.com> wrote: > On Sat, Nov 11, 2017 at 12:44 AM, Marc Glisse <marc.glisse@inria.fr> wrote: >> Adding some random cc: to people who might be affected. Hopefully I am not >> breaking any of your stuff... >> >> Ulrich Weigand (address space) >> Ilya Enkovich (pointer bound check) >> DJ Delorie (target with 24-bit partial mode pointer) >> >> If you want to give it a try, or just take a quick look to check that you >> are obviously not affected, that would be nice, but don't feel forced. >> >> Here is an updated version of the patch, with just a few more >> transformations in match.pd, to match some MINUS_EXPR optimizations I missed >> the first time: -(A-B), X-Z<Y-Z, (X-Z)-(Y-Z), Z-X<Z-Y, (Z-X)-(Z-Y), >> (A-B)+(C-A) >> >> The exact undefined-behavior status should probably be clarified more. >> First, I'd like to say that POINTER_DIFF_EXPR may only take 2 pointers into >> the same "object" (like in C/C++), so they differ by at most half the size >> of the address space, and a-b is not allowed to be the minimum negative >> value (so I can safely use b-a), and if we compute a-b and b-c, then a and c >> are in the same object so I can safely compute a-c, etc. Second, we probably >> need modes without undefined behavior, wrapping with either -fwrapv or a new >> -fwrapp, or sanitized. For the sanitized version, we could keep using >> POINTER_DIFF_EXPR and check TYPE_OVERFLOW_SANITIZED on the pointer type as >> we currently do for integers. For wrapping, either we say that >> POINTER_DIFF_EXPR has wrapping semantics when that option is in effect, or >> we do not use POINTER_DIFF_EXPR and instead cast to unsigned before applying >> a MINUS_EXPR (my preference). > > CCing C/C++ FE folks as well. > > + /* It is better if the caller provides the return type. */ > + if (code == POINTER_DIFF_EXPR) > + { > + offset_int res = wi::sub (wi::to_offset (arg1), > + wi::to_offset (arg2)); > + return force_fit_type (signed_type_for (TREE_TYPE (arg1)), res, 1, > + TREE_OVERFLOW (arg1) | TREE_OVERFLOW (arg2)); > + } > + > > there's an overload that provides the type... (we should probably go > over all callers > and use that). There is already a folding that is only done when the type is > available. So I'd simply remove the above from the non-type overload handling. > What breaks then? > > With the patch it's of course somewhat ugly in that we need to deal with both > MINUS_EXPR on pointers and POINTER_DIFF_EXPR - at least that's what > > + case POINTER_DIFF_EXPR: > case MINUS_EXPR: > + /* Fold &a[i] - &a[j] to i-j. */ > + if (TREE_CODE (arg0) == ADDR_EXPR > + && TREE_CODE (TREE_OPERAND (arg0, 0)) == ARRAY_REF > + && TREE_CODE (arg1) == ADDR_EXPR > + && TREE_CODE (TREE_OPERAND (arg1, 0)) == ARRAY_REF) > + { > + tree tem = fold_addr_of_array_ref_difference (loc, type, > + TREE_OPERAND (arg0, 0), > + TREE_OPERAND (arg1, 0), > + code > + == POINTER_DIFF_EXPR); > > suggests. But is that really so? The FEs today should never build > a MINUS_EXPR with pointer operands and your patch also doesn't. > I suppose we only arrive above because STRIP_NOPS strips the > pointer to integer conversion? > > + case POINTER_DIFF_EXPR: > + if (!POINTER_TYPE_P (TREE_TYPE (TREE_OPERAND (t, 0))) > + || !POINTER_TYPE_P (TREE_TYPE (TREE_OPERAND (t, 1)))) > + { > + error ("invalid operand to pointer diff, operand is not a pointer"); > + return t; > + } > + CHECK_OP (0, "invalid operand to pointer diff"); > + CHECK_OP (1, "invalid operand to pointer diff"); > + break; > > can you add > || TREE_CODE (TREE_TYPE (t)) != INTEGER_TYPE > || TYPE_UNSIGNED (TREE_TYPE (t)) > > ? Note that if the precision of the pointer type arguments are not equal to the > precision of the return value then foldings like > > (simplify > + (plus:c (pointer_diff @0 @1) (pointer_diff @2 @0)) > + (if (TYPE_OVERFLOW_UNDEFINED (type) > + && !TYPE_OVERFLOW_SANITIZED (TREE_TYPE (@0))) > + (pointer_diff @2 @1))) > > might not be correct as we drop a possible truncation here. Shall we > require (and document) that the result of the pointer difference in > a POINTER_DIFF_EXPR has to be representable in the type of the > expression, otherwise > the behavior is undefined? Shall we allow an unsigned return type for > differences known to be representable? Probably not... Does the behavior > depend on TYPE_OVERFLOW_UNDEFINED? It would be nice to amend > the generic.texi docs somewhat here. > > Ah, the gimple checking adds some more bits here that clarify things: > > + case POINTER_DIFF_EXPR: > + { > + if (!POINTER_TYPE_P (rhs1_type) > + || !POINTER_TYPE_P (rhs2_type) > + || !types_compatible_p (rhs1_type, rhs2_type) > + || TREE_CODE (lhs_type) != INTEGER_TYPE > + || TYPE_UNSIGNED (lhs_type) > + || TYPE_PRECISION (lhs_type) != TYPE_PRECISION (rhs1_type)) > + { > + error ("type mismatch in pointer diff expression"); > + debug_generic_stmt (lhs_type); > + debug_generic_stmt (rhs1_type); > + debug_generic_stmt (rhs2_type); > + return true; > + } > > > I think the patch is ok for trunk, the FE bits should get approval > from their maintainers. > Joseph may have an idea about the address-space issue. > > With this in place we should be able to fix some of the issues Jakub ran into > with pointer sanitizing? > > Also with this in place it would be nice to do the corresponding adjustment to > POINTER_PLUS_EXPR, that is, require a _signed_ offset type that matches > the precision of the other argument. > > Richard. > >> >> >> On Sat, 28 Oct 2017, Marc Glisse wrote: >> >>> Hello, >>> >>> first, if you are doing anything unusual with pointers (address spaces, >>> pointer/sizetype with weird sizes, instrumentation, etc), it would be great >>> if you could give this patch a try. It was bootstrapped and regtested on >>> powerpc64le-unknown-linux-gnu (gcc112), and a slightly older version on >>> x86_64-pc-linux-gnu (skylake laptop). I also built bare cross-compilers (no >>> sysroot or anything) for avr, m32c, alpha64-vms, s390-linux, and visium to >>> check that on trivial examples it behaves as expected (by the way, m32c >>> seems broken for unrelated reasons at the moment), but I wouldn't count that >>> as complete testing. >>> >>> This was previously discussed in the thread "Fix pointer diff (was: >>> -fsanitize=pointer-overflow support (PR sanitizer/80998))" ( >>> https://gcc.gnu.org/ml/gcc-patches/2017-10/msg02128.html for the latest >>> message). >>> >>> Front-ends other than C/C++ can be changed later (I took a quick look at >>> fortran and ada, but they are way too unfamiliar to me), I did not remove >>> any handling for the other representations. >>> >>> 2017-10-28 Marc Glisse <marc.glisse@inria.fr> >>> >>> gcc/cp/ >>> * constexpr.c (cxx_eval_constant_expression, >>> potential_constant_expression_1): Handle POINTER_DIFF_EXPR. >>> * cp-gimplify.c (cp_fold): Likewise. >>> * error.c (dump_expr): Likewise. >>> * typeck.c (cp_build_binary_op): Likewise. It's not clear to me that cp_build_binary_op needs to handle POINTER_DIFF_EXPR, it should get MINUS_EXPR and produce POINTER_DIFF_EXPR. >>> * tree.def: New tree code POINTER_DIFF_EXPR. The comment might mention that the value is the same for all pointer types, not divided by the size of the pointed-to type. Jason
On Fri, 17 Nov 2017, Richard Biener wrote:
> Joseph may have an idea about the address-space issue.
I'm not clear what the question is. The TR 18037 rule on subtractions
with address spaces is "For subtraction, if the two operands are pointers
into different address spaces, the address spaces must overlap." (as a
constraint). That should already be checked via comp_target_types, and
should have nothing to do with this patch.
On November 17, 2017 6:20:22 PM GMT+01:00, Joseph Myers <joseph@codesourcery.com> wrote: >On Fri, 17 Nov 2017, Richard Biener wrote: > >> Joseph may have an idea about the address-space issue. > >I'm not clear what the question is. The TR 18037 rule on subtractions >with address spaces is "For subtraction, if the two operands are >pointers >into different address spaces, the address spaces must overlap." (as a >constraint). That should already be checked via comp_target_types, and > >should have nothing to do with this patch. The question is what ptrdiff_t is for a specific address space. Or rather if that type may be dependent on the address space or if we can always use that of the default address space. Richard.
On Fri, 17 Nov 2017, Richard Biener wrote: > The question is what ptrdiff_t is for a specific address space. Or > rather if that type may be dependent on the address space or if we can > always use that of the default address space. ptrdiff_t is a fixed type which does not depend on the address space.
On Fri, 17 Nov 2017, Jason Merrill wrote: > It's not clear to me that cp_build_binary_op needs to handle > POINTER_DIFF_EXPR, it should get MINUS_EXPR and produce > POINTER_DIFF_EXPR. Indeed, I added POINTER_DIFF_EXPR in many places by looking for MINUS_EXPR, but this one is useless, regtesting passes without it. >>>> * tree.def: New tree code POINTER_DIFF_EXPR. > > The comment might mention that the value is the same for all pointer > types, not divided by the size of the pointed-to type. Good idea, thanks.
On Fri, 17 Nov 2017, Richard Biener wrote: > On Sat, Nov 11, 2017 at 12:44 AM, Marc Glisse <marc.glisse@inria.fr> wrote: > >> The exact undefined-behavior status should probably be clarified more. >> First, I'd like to say that POINTER_DIFF_EXPR may only take 2 pointers into >> the same "object" (like in C/C++), so they differ by at most half the size >> of the address space, and a-b is not allowed to be the minimum negative >> value (so I can safely use b-a), and if we compute a-b and b-c, then a and c >> are in the same object so I can safely compute a-c, etc. Second, we probably >> need modes without undefined behavior, wrapping with either -fwrapv or a new >> -fwrapp, or sanitized. For the sanitized version, we could keep using >> POINTER_DIFF_EXPR and check TYPE_OVERFLOW_SANITIZED on the pointer type as >> we currently do for integers. For wrapping, either we say that >> POINTER_DIFF_EXPR has wrapping semantics when that option is in effect, or >> we do not use POINTER_DIFF_EXPR and instead cast to unsigned before applying >> a MINUS_EXPR (my preference). > > CCing C/C++ FE folks as well. > > + /* It is better if the caller provides the return type. */ > + if (code == POINTER_DIFF_EXPR) > + { > + offset_int res = wi::sub (wi::to_offset (arg1), > + wi::to_offset (arg2)); > + return force_fit_type (signed_type_for (TREE_TYPE (arg1)), res, 1, > + TREE_OVERFLOW (arg1) | TREE_OVERFLOW (arg2)); > + } > + > > there's an overload that provides the type... (we should probably go > over all callers > and use that). There is already a folding that is only done when the type is > available. So I'd simply remove the above from the non-type overload handling. > What breaks then? I haven't checked yet. On the other hand, now that I require the same precision for pointers and their difference, using signed_type_for should be safe enough (only issue is if a front-end is unhappy that we changed the exact type for another 'equivalent' one). But I'll try removing the version with signed_type_for. > With the patch it's of course somewhat ugly in that we need to deal with both > MINUS_EXPR on pointers and POINTER_DIFF_EXPR - at least that's what > > + case POINTER_DIFF_EXPR: > case MINUS_EXPR: > + /* Fold &a[i] - &a[j] to i-j. */ > + if (TREE_CODE (arg0) == ADDR_EXPR > + && TREE_CODE (TREE_OPERAND (arg0, 0)) == ARRAY_REF > + && TREE_CODE (arg1) == ADDR_EXPR > + && TREE_CODE (TREE_OPERAND (arg1, 0)) == ARRAY_REF) > + { > + tree tem = fold_addr_of_array_ref_difference (loc, type, > + TREE_OPERAND (arg0, 0), > + TREE_OPERAND (arg1, 0), > + code > + == POINTER_DIFF_EXPR); > > suggests. But is that really so? The FEs today should never build > a MINUS_EXPR with pointer operands and your patch also doesn't. > I suppose we only arrive above because STRIP_NOPS strips the > pointer to integer conversion? I think so. And since I only patched 2 front-ends, others probably still generate casts to integers and MINUS_EXPR and can benefit from this transformation. Also, if we implement -fwrapp by casting to unsigned and doing MINUS_EXPR, that will remain useful. > + case POINTER_DIFF_EXPR: > + if (!POINTER_TYPE_P (TREE_TYPE (TREE_OPERAND (t, 0))) > + || !POINTER_TYPE_P (TREE_TYPE (TREE_OPERAND (t, 1)))) > + { > + error ("invalid operand to pointer diff, operand is not a pointer"); > + return t; > + } > + CHECK_OP (0, "invalid operand to pointer diff"); > + CHECK_OP (1, "invalid operand to pointer diff"); > + break; > > can you add > || TREE_CODE (TREE_TYPE (t)) != INTEGER_TYPE > || TYPE_UNSIGNED (TREE_TYPE (t)) I wasn't sure how much to duplicate between here and below. Added. > ? Note that if the precision of the pointer type arguments are not equal to the > precision of the return value then foldings like > > (simplify > + (plus:c (pointer_diff @0 @1) (pointer_diff @2 @0)) > + (if (TYPE_OVERFLOW_UNDEFINED (type) > + && !TYPE_OVERFLOW_SANITIZED (TREE_TYPE (@0))) > + (pointer_diff @2 @1))) > > might not be correct as we drop a possible truncation here. Shall we I simplified the transformations after deciding to require equal precision. > require (and document) that the result of the pointer difference in > a POINTER_DIFF_EXPR has to be representable in the type of the > expression, otherwise > the behavior is undefined? Shall we allow an unsigned return type for > differences known to be representable? Probably not... Does the behavior > depend on TYPE_OVERFLOW_UNDEFINED? It would be nice to amend > the generic.texi docs somewhat here. I was more precise in tree.def, copying details over to generic.texi now. But the overflow behavior is something I was asking about above (see citation at the beginning of this email). I am tempted to say that overflow is always undefined for POINTER_DIFF_EXPR and we should use MINUS_EXPR on unsigned integers when we want wrapping, but if people prefer to use POINTER_DIFF_EXPR in all cases and change its behavior depending on TYPE_OVERFLOW_UNDEFINED that would be ok with me. I probably wasn't consistent in match.pd and will need to tweak a few details once this is decided. > Ah, the gimple checking adds some more bits here that clarify things: > > + case POINTER_DIFF_EXPR: > + { > + if (!POINTER_TYPE_P (rhs1_type) > + || !POINTER_TYPE_P (rhs2_type) > + || !types_compatible_p (rhs1_type, rhs2_type) > + || TREE_CODE (lhs_type) != INTEGER_TYPE > + || TYPE_UNSIGNED (lhs_type) > + || TYPE_PRECISION (lhs_type) != TYPE_PRECISION (rhs1_type)) > + { > + error ("type mismatch in pointer diff expression"); > + debug_generic_stmt (lhs_type); > + debug_generic_stmt (rhs1_type); > + debug_generic_stmt (rhs2_type); > + return true; > + } > > > I think the patch is ok for trunk, the FE bits should get approval > from their maintainers. > Joseph may have an idea about the address-space issue. > > With this in place we should be able to fix some of the issues Jakub ran into > with pointer sanitizing? Some of them should be fixed without doing anything: pointer differences are not using MINUS_EXPR anymore in C/C++, so they are not sanitized and don't produce spurious errors when crossing the middle of the address space. On the other hand, we may want to sanitize POINTER_DIFF_EXPR specifically. > Also with this in place it would be nice to do the corresponding adjustment to > POINTER_PLUS_EXPR, that is, require a _signed_ offset type that matches > the precision of the other argument. Maybe next stage1... Too late for this year.
On November 18, 2017 11:20:42 AM GMT+01:00, Marc Glisse <marc.glisse@inria.fr> wrote: >On Fri, 17 Nov 2017, Richard Biener wrote: > >> On Sat, Nov 11, 2017 at 12:44 AM, Marc Glisse <marc.glisse@inria.fr> >wrote: >> >>> The exact undefined-behavior status should probably be clarified >more. >>> First, I'd like to say that POINTER_DIFF_EXPR may only take 2 >pointers into >>> the same "object" (like in C/C++), so they differ by at most half >the size >>> of the address space, and a-b is not allowed to be the minimum >negative >>> value (so I can safely use b-a), and if we compute a-b and b-c, then >a and c >>> are in the same object so I can safely compute a-c, etc. Second, we >probably >>> need modes without undefined behavior, wrapping with either -fwrapv >or a new >>> -fwrapp, or sanitized. For the sanitized version, we could keep >using >>> POINTER_DIFF_EXPR and check TYPE_OVERFLOW_SANITIZED on the pointer >type as >>> we currently do for integers. For wrapping, either we say that >>> POINTER_DIFF_EXPR has wrapping semantics when that option is in >effect, or >>> we do not use POINTER_DIFF_EXPR and instead cast to unsigned before >applying >>> a MINUS_EXPR (my preference). >> >> CCing C/C++ FE folks as well. >> >> + /* It is better if the caller provides the return type. */ >> + if (code == POINTER_DIFF_EXPR) >> + { >> + offset_int res = wi::sub (wi::to_offset (arg1), >> + wi::to_offset (arg2)); >> + return force_fit_type (signed_type_for (TREE_TYPE (arg1)), >res, 1, >> + TREE_OVERFLOW (arg1) | TREE_OVERFLOW >(arg2)); >> + } >> + >> >> there's an overload that provides the type... (we should probably go >> over all callers >> and use that). There is already a folding that is only done when the >type is >> available. So I'd simply remove the above from the non-type overload >handling. >> What breaks then? > >I haven't checked yet. On the other hand, now that I require the same >precision for pointers and their difference, using signed_type_for >should >be safe enough (only issue is if a front-end is unhappy that we changed > >the exact type for another 'equivalent' one). But I'll try removing the > >version with signed_type_for. > >> With the patch it's of course somewhat ugly in that we need to deal >with both >> MINUS_EXPR on pointers and POINTER_DIFF_EXPR - at least that's what >> >> + case POINTER_DIFF_EXPR: >> case MINUS_EXPR: >> + /* Fold &a[i] - &a[j] to i-j. */ >> + if (TREE_CODE (arg0) == ADDR_EXPR >> + && TREE_CODE (TREE_OPERAND (arg0, 0)) == ARRAY_REF >> + && TREE_CODE (arg1) == ADDR_EXPR >> + && TREE_CODE (TREE_OPERAND (arg1, 0)) == ARRAY_REF) >> + { >> + tree tem = fold_addr_of_array_ref_difference (loc, type, >> + TREE_OPERAND >(arg0, 0), >> + TREE_OPERAND >(arg1, 0), >> + code >> + == >POINTER_DIFF_EXPR); >> >> suggests. But is that really so? The FEs today should never build >> a MINUS_EXPR with pointer operands and your patch also doesn't. >> I suppose we only arrive above because STRIP_NOPS strips the >> pointer to integer conversion? > >I think so. And since I only patched 2 front-ends, others probably >still >generate casts to integers and MINUS_EXPR and can benefit from this >transformation. Also, if we implement -fwrapp by casting to unsigned >and >doing MINUS_EXPR, that will remain useful. > >> + case POINTER_DIFF_EXPR: >> + if (!POINTER_TYPE_P (TREE_TYPE (TREE_OPERAND (t, 0))) >> + || !POINTER_TYPE_P (TREE_TYPE (TREE_OPERAND (t, 1)))) >> + { >> + error ("invalid operand to pointer diff, operand is not a >pointer"); >> + return t; >> + } >> + CHECK_OP (0, "invalid operand to pointer diff"); >> + CHECK_OP (1, "invalid operand to pointer diff"); >> + break; >> >> can you add >> || TREE_CODE (TREE_TYPE (t)) != INTEGER_TYPE >> || TYPE_UNSIGNED (TREE_TYPE (t)) > >I wasn't sure how much to duplicate between here and below. Added. > >> ? Note that if the precision of the pointer type arguments are not >equal to the >> precision of the return value then foldings like >> >> (simplify >> + (plus:c (pointer_diff @0 @1) (pointer_diff @2 @0)) >> + (if (TYPE_OVERFLOW_UNDEFINED (type) >> + && !TYPE_OVERFLOW_SANITIZED (TREE_TYPE (@0))) >> + (pointer_diff @2 @1))) >> >> might not be correct as we drop a possible truncation here. Shall we > >I simplified the transformations after deciding to require equal >precision. > >> require (and document) that the result of the pointer difference in >> a POINTER_DIFF_EXPR has to be representable in the type of the >> expression, otherwise >> the behavior is undefined? Shall we allow an unsigned return type >for >> differences known to be representable? Probably not... Does the >behavior >> depend on TYPE_OVERFLOW_UNDEFINED? It would be nice to amend >> the generic.texi docs somewhat here. > >I was more precise in tree.def, copying details over to generic.texi >now. >But the overflow behavior is something I was asking about above (see >citation at the beginning of this email). I am tempted to say that >overflow >is always undefined for POINTER_DIFF_EXPR and we should use MINUS_EXPR >on >unsigned integers when we want wrapping, but if people prefer to use >POINTER_DIFF_EXPR in all cases and change its behavior depending on >TYPE_OVERFLOW_UNDEFINED that would be ok with me. I probably wasn't >consistent in match.pd and will need to tweak a few details once this >is >decided. Ah. Indeed using unsigned MINUS_EXPR sounds like a good idea - also for canonicalization. Likewise for POUNTER_PLUS_EXPR vs. PLUS_EXPR. >> Ah, the gimple checking adds some more bits here that clarify things: >> >> + case POINTER_DIFF_EXPR: >> + { >> + if (!POINTER_TYPE_P (rhs1_type) >> + || !POINTER_TYPE_P (rhs2_type) >> + || !types_compatible_p (rhs1_type, rhs2_type) >> + || TREE_CODE (lhs_type) != INTEGER_TYPE >> + || TYPE_UNSIGNED (lhs_type) >> + || TYPE_PRECISION (lhs_type) != TYPE_PRECISION >(rhs1_type)) >> + { >> + error ("type mismatch in pointer diff expression"); >> + debug_generic_stmt (lhs_type); >> + debug_generic_stmt (rhs1_type); >> + debug_generic_stmt (rhs2_type); >> + return true; >> + } >> >> >> I think the patch is ok for trunk, the FE bits should get approval >> from their maintainers. >> Joseph may have an idea about the address-space issue. >> >> With this in place we should be able to fix some of the issues Jakub >ran into >> with pointer sanitizing? > >Some of them should be fixed without doing anything: pointer >differences >are not using MINUS_EXPR anymore in C/C++, so they are not sanitized >and >don't produce spurious errors when crossing the middle of the address >space. On the other hand, we may want to sanitize POINTER_DIFF_EXPR >specifically. > >> Also with this in place it would be nice to do the corresponding >adjustment to >> POINTER_PLUS_EXPR, that is, require a _signed_ offset type that >matches >> the precision of the other argument. > >Maybe next stage1... Too late for this year. Agreed. Bin had patches to do this IIRC. Richard.
Hello, new version, regtested on powerpc64le-unknown-linux-gnu. The front-end parts are up for review. 2017-10-28 Marc Glisse <marc.glisse@inria.fr> gcc/c/ * c-fold.c (c_fully_fold_internal): Handle POINTER_DIFF_EXPR. * c-typeck.c (pointer_diff): Use POINTER_DIFF_EXPR. gcc/c-family/ * c-pretty-print.c (pp_c_additive_expression, c_pretty_printer::expression): Handle POINTER_DIFF_EXPR. gcc/cp/ * constexpr.c (cxx_eval_constant_expression, potential_constant_expression_1): Handle POINTER_DIFF_EXPR. * cp-gimplify.c (cp_fold): Likewise. * error.c (dump_expr): Likewise. * typeck.c (pointer_diff): Use POINTER_DIFF_EXPR. gcc/ * doc/generic.texi: Document POINTER_DIFF_EXPR, update POINTER_PLUS_EXPR. * cfgexpand.c (expand_debug_expr): Handle POINTER_DIFF_EXPR. * expr.c (expand_expr_real_2): Likewise. * fold-const.c (const_binop, fold_addr_of_array_ref_difference, fold_binary_loc): Likewise. * match.pd (X-X, P+(Q-P), &D-P, (P+N)-P, P-(P+N), (P+M)-(P+N), P-Q==0, -(A-B), X-Z<Y-Z, (X-Z)-(Y-Z), Z-X<Z-Y, (Z-X)-(Z-Y), (A-B)+(C-A)): New transformations for POINTER_DIFF_EXPR, based on MINUS_EXPR transformations. * optabs-tree.c (optab_for_tree_code): Handle POINTER_DIFF_EXPR. * tree-cfg.c (verify_expr, verify_gimple_assign_binary): Likewise. * tree-inline.c (estimate_operator_cost): Likewise. * tree-pretty-print.c (dump_generic_node, op_code_prio, op_symbol_code): Likewise. * tree-vect-stmts.c (vectorizable_operation): Likewise. * vr-values.c (extract_range_from_binary_expr): Likewise. * varasm.c (initializer_constant_valid_p_1): Likewise. * tree.def: New tree code POINTER_DIFF_EXPR.
On Sun, Nov 19, 2017 at 6:54 PM, Marc Glisse <marc.glisse@inria.fr> wrote: > new version, regtested on powerpc64le-unknown-linux-gnu. The front-end parts > are up for review. The C++ changes are OK. Jason
On 11/19/2017 04:54 PM, Marc Glisse wrote: > Hello, > > new version, regtested on powerpc64le-unknown-linux-gnu. The front-end > parts are up for review. > > 2017-10-28 Marc Glisse <marc.glisse@inria.fr> > > gcc/c/ > * c-fold.c (c_fully_fold_internal): Handle POINTER_DIFF_EXPR. > * c-typeck.c (pointer_diff): Use POINTER_DIFF_EXPR. > > gcc/c-family/ > * c-pretty-print.c (pp_c_additive_expression, > c_pretty_printer::expression): Handle POINTER_DIFF_EXPR. > > gcc/cp/ > * constexpr.c (cxx_eval_constant_expression, > potential_constant_expression_1): Handle POINTER_DIFF_EXPR. > * cp-gimplify.c (cp_fold): Likewise. > * error.c (dump_expr): Likewise. > * typeck.c (pointer_diff): Use POINTER_DIFF_EXPR. > > gcc/ > * doc/generic.texi: Document POINTER_DIFF_EXPR, update > POINTER_PLUS_EXPR. > * cfgexpand.c (expand_debug_expr): Handle POINTER_DIFF_EXPR. > * expr.c (expand_expr_real_2): Likewise. > * fold-const.c (const_binop, fold_addr_of_array_ref_difference, > fold_binary_loc): Likewise. > * match.pd (X-X, P+(Q-P), &D-P, (P+N)-P, P-(P+N), (P+M)-(P+N), > P-Q==0, -(A-B), X-Z<Y-Z, (X-Z)-(Y-Z), Z-X<Z-Y, (Z-X)-(Z-Y), > (A-B)+(C-A)): New transformations for POINTER_DIFF_EXPR, based on > MINUS_EXPR transformations. > * optabs-tree.c (optab_for_tree_code): Handle POINTER_DIFF_EXPR. > * tree-cfg.c (verify_expr, verify_gimple_assign_binary): Likewise. > * tree-inline.c (estimate_operator_cost): Likewise. > * tree-pretty-print.c (dump_generic_node, op_code_prio, > op_symbol_code): Likewise. > * tree-vect-stmts.c (vectorizable_operation): Likewise. > * vr-values.c (extract_range_from_binary_expr): Likewise. > * varasm.c (initializer_constant_valid_p_1): Likewise. > * tree.def: New tree code POINTER_DIFF_EXPR. > The front-end bits seem very reasonable to me. If the rest is ACK'd then you should consider the full patch ack'd. jeff
Hi Marc, On 20 November 2017 at 00:54, Marc Glisse <marc.glisse@inria.fr> wrote: > Hello, > > new version, regtested on powerpc64le-unknown-linux-gnu. The front-end parts > are up for review. > > 2017-10-28 Marc Glisse <marc.glisse@inria.fr> > > gcc/c/ > * c-fold.c (c_fully_fold_internal): Handle POINTER_DIFF_EXPR. > * c-typeck.c (pointer_diff): Use POINTER_DIFF_EXPR. > > gcc/c-family/ > * c-pretty-print.c (pp_c_additive_expression, > c_pretty_printer::expression): Handle POINTER_DIFF_EXPR. > > gcc/cp/ > * constexpr.c (cxx_eval_constant_expression, > potential_constant_expression_1): Handle POINTER_DIFF_EXPR. > * cp-gimplify.c (cp_fold): Likewise. > * error.c (dump_expr): Likewise. > * typeck.c (pointer_diff): Use POINTER_DIFF_EXPR. > > gcc/ > * doc/generic.texi: Document POINTER_DIFF_EXPR, update > POINTER_PLUS_EXPR. > * cfgexpand.c (expand_debug_expr): Handle POINTER_DIFF_EXPR. > * expr.c (expand_expr_real_2): Likewise. > * fold-const.c (const_binop, fold_addr_of_array_ref_difference, > fold_binary_loc): Likewise. > * match.pd (X-X, P+(Q-P), &D-P, (P+N)-P, P-(P+N), (P+M)-(P+N), > P-Q==0, -(A-B), X-Z<Y-Z, (X-Z)-(Y-Z), Z-X<Z-Y, (Z-X)-(Z-Y), > (A-B)+(C-A)): New transformations for POINTER_DIFF_EXPR, based on > MINUS_EXPR transformations. > * optabs-tree.c (optab_for_tree_code): Handle POINTER_DIFF_EXPR. > * tree-cfg.c (verify_expr, verify_gimple_assign_binary): Likewise. > * tree-inline.c (estimate_operator_cost): Likewise. > * tree-pretty-print.c (dump_generic_node, op_code_prio, > op_symbol_code): Likewise. > * tree-vect-stmts.c (vectorizable_operation): Likewise. > * vr-values.c (extract_range_from_binary_expr): Likewise. > * varasm.c (initializer_constant_valid_p_1): Likewise. > > * tree.def: New tree code POINTER_DIFF_EXPR. > Since you committed this patch (r255021), my cross-builds of GCC for aarch64-linux-gnu fail while building glibc: /tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/tools/bin/aarch64-none-linux-gnu-gcc fetch-value.c -c -std=gnu99 -fgnu89-inline -O2 -Wall -Winline -Wundef -Wwrite-string s -fmerge-all-constants -frounding-math -g -Wstrict-prototypes -fPIC -I../include -I/tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/gl ibc-1/nptl_db -I/tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/glibc-1 -I../sysdeps/unix/sysv/linux/aarch64/nptl -I../sysdeps/unix/sysv/lin ux/aarch64 -I../sysdeps/unix/sysv/linux/generic -I../sysdeps/unix/sysv/linux/wordsize-64 -I../nptl/sysdeps/unix/sysv/linux -I../nptl/sysdeps/pthread -I../sysdeps/pthread -I ../sysdeps/unix/sysv/linux -I../sysdeps/gnu -I../sysdeps/unix/inet -I../nptl/sysdeps/unix/sysv -I../sysdeps/unix/sysv -I../nptl/sysdeps/unix -I../sysdeps/unix -I../sysdeps /posix -I../sysdeps/aarch64/fpu -I../sysdeps/aarch64/nptl -I../sysdeps/aarch64 -I../sysdeps/wordsize-64 -I../sysdeps/ieee754/ldbl-128 -I../sysdeps/ieee754/dbl-64/wordsize-6 4 -I../sysdeps/ieee754/dbl-64 -I../sysdeps/ieee754/flt-32 -I../sysdeps/aarch64/soft-fp -I../sysdeps/ieee754 -I../sysdeps/generic -I../nptl -I.. -I../libio -I. -nostdinc -i system /tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/tools/lib/gcc/aarch64-none-linux-gnu/8.0.0/include -isystem /tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc /tools/lib/gcc/aarch64-none-linux-gnu/8.0.0/include-fixed -isystem /tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/sysroot-aarch64-none-linux-gnu/usr/include -D_LIBC_REE NTRANT -include ../include/libc-symbols.h -DPIC -DSHARED -DNOT_IN_libc=1 -DIS_IN_libthread_db=1 -DIN_LIB=libthread_db -o /tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccs rc/obj-aarch64-none-linux-gnu/glibc-1/nptl_db/fetch-value.os -MD -MP -MF /tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/glibc-1/nptl_db/fetch- value.os.dt -MT /tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/glibc-1/nptl_db/fetch-value.os during GIMPLE pass: vrp fetch-value.c: In function '_td_locate_field': fetch-value.c:44:1: internal compiler error: Segmentation fault _td_locate_field (td_thragent_t *ta, ^~~~~~~~~~~~~~~~ 0xb87c75 crash_signal /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/toplev.c:325 0x5b1934 contains_struct_check /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree.h:3459 0x5b1934 wi::to_wide(tree_node const*) /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree.h:5247 0xf05557 vr_values::two_valued_val_range_p(tree_node*, tree_node**, tree_node**) /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/vr-values.c:4051 0xf0ee48 vr_values::simplify_stmt_using_ranges(gimple_stmt_iterator*) /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/vr-values.c:4115 0xd7068f substitute_and_fold_dom_walker::before_dom_children(basic_block_def*) /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-ssa-propagate.c:1073 0x1204860 dom_walker::walk(basic_block_def*) /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/domwalk.c:308 0xd6e805 substitute_and_fold_engine::substitute_and_fold() /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-ssa-propagate.c:1173 0xe71b0b vrp_prop::vrp_finalize(bool) /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-vrp.c:6788 0xe81667 execute_vrp /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-vrp.c:6864 Please submit a full bug report, No such problem on arm targets. Can you have a look? Thanks, Christophe > -- > Marc Glisse
On Wed, 22 Nov 2017, Christophe Lyon wrote: > Since you committed this patch (r255021), my cross-builds of GCC for > aarch64-linux-gnu fail while building glibc: > /tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/tools/bin/aarch64-none-linux-gnu-gcc > fetch-value.c -c -std=gnu99 -fgnu89-inline -O2 -Wall -Winline -Wundef > -Wwrite-string > s -fmerge-all-constants -frounding-math -g -Wstrict-prototypes -fPIC > -I../include > -I/tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/gl > ibc-1/nptl_db -I/tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/glibc-1 > -I../sysdeps/unix/sysv/linux/aarch64/nptl -I../sysdeps/unix/sysv/lin > ux/aarch64 -I../sysdeps/unix/sysv/linux/generic > -I../sysdeps/unix/sysv/linux/wordsize-64 > -I../nptl/sysdeps/unix/sysv/linux -I../nptl/sysdeps/pthread > -I../sysdeps/pthread -I > ../sysdeps/unix/sysv/linux -I../sysdeps/gnu -I../sysdeps/unix/inet > -I../nptl/sysdeps/unix/sysv -I../sysdeps/unix/sysv > -I../nptl/sysdeps/unix -I../sysdeps/unix -I../sysdeps > /posix -I../sysdeps/aarch64/fpu -I../sysdeps/aarch64/nptl > -I../sysdeps/aarch64 -I../sysdeps/wordsize-64 > -I../sysdeps/ieee754/ldbl-128 -I../sysdeps/ieee754/dbl-64/wordsize-6 > 4 -I../sysdeps/ieee754/dbl-64 -I../sysdeps/ieee754/flt-32 > -I../sysdeps/aarch64/soft-fp -I../sysdeps/ieee754 > -I../sysdeps/generic -I../nptl -I.. -I../libio -I. -nostdinc -i > system /tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/tools/lib/gcc/aarch64-none-linux-gnu/8.0.0/include > -isystem /tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc > /tools/lib/gcc/aarch64-none-linux-gnu/8.0.0/include-fixed -isystem > /tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/sysroot-aarch64-none-linux-gnu/usr/include > -D_LIBC_REE > NTRANT -include ../include/libc-symbols.h -DPIC -DSHARED > -DNOT_IN_libc=1 -DIS_IN_libthread_db=1 -DIN_LIB=libthread_db -o > /tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccs > rc/obj-aarch64-none-linux-gnu/glibc-1/nptl_db/fetch-value.os -MD -MP > -MF /tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/glibc-1/nptl_db/fetch- > value.os.dt -MT > /tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/glibc-1/nptl_db/fetch-value.os > during GIMPLE pass: vrp > fetch-value.c: In function '_td_locate_field': > fetch-value.c:44:1: internal compiler error: Segmentation fault > _td_locate_field (td_thragent_t *ta, > ^~~~~~~~~~~~~~~~ > 0xb87c75 crash_signal > /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/toplev.c:325 > 0x5b1934 contains_struct_check > /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree.h:3459 > 0x5b1934 wi::to_wide(tree_node const*) > /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree.h:5247 > 0xf05557 vr_values::two_valued_val_range_p(tree_node*, tree_node**, tree_node**) > /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/vr-values.c:4051 > 0xf0ee48 vr_values::simplify_stmt_using_ranges(gimple_stmt_iterator*) > /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/vr-values.c:4115 > 0xd7068f substitute_and_fold_dom_walker::before_dom_children(basic_block_def*) > /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-ssa-propagate.c:1073 > 0x1204860 dom_walker::walk(basic_block_def*) > /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/domwalk.c:308 > 0xd6e805 substitute_and_fold_engine::substitute_and_fold() > /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-ssa-propagate.c:1173 > 0xe71b0b vrp_prop::vrp_finalize(bool) > /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-vrp.c:6788 > 0xe81667 execute_vrp > /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-vrp.c:6864 > Please submit a full bug report, Looks like PR 83104. (I am a bit curious what the code looks like, since the testcase in the PR is rather on the undefined side of things, though ok for something embedded)
On Wed, 22 Nov 2017, Marc Glisse wrote: > On Wed, 22 Nov 2017, Christophe Lyon wrote: > >> Since you committed this patch (r255021), my cross-builds of GCC for >> aarch64-linux-gnu fail while building glibc: >> /tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/tools/bin/aarch64-none-linux-gnu-gcc >> fetch-value.c -c -std=gnu99 -fgnu89-inline -O2 -Wall -Winline -Wundef >> -Wwrite-string >> s -fmerge-all-constants -frounding-math -g -Wstrict-prototypes -fPIC >> -I../include >> >> -I/tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/gl >> ibc-1/nptl_db >> -I/tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/glibc-1 >> -I../sysdeps/unix/sysv/linux/aarch64/nptl -I../sysdeps/unix/sysv/lin >> ux/aarch64 -I../sysdeps/unix/sysv/linux/generic >> -I../sysdeps/unix/sysv/linux/wordsize-64 >> -I../nptl/sysdeps/unix/sysv/linux -I../nptl/sysdeps/pthread >> -I../sysdeps/pthread -I >> ../sysdeps/unix/sysv/linux -I../sysdeps/gnu -I../sysdeps/unix/inet >> -I../nptl/sysdeps/unix/sysv -I../sysdeps/unix/sysv >> -I../nptl/sysdeps/unix -I../sysdeps/unix -I../sysdeps >> /posix -I../sysdeps/aarch64/fpu -I../sysdeps/aarch64/nptl >> -I../sysdeps/aarch64 -I../sysdeps/wordsize-64 >> -I../sysdeps/ieee754/ldbl-128 -I../sysdeps/ieee754/dbl-64/wordsize-6 >> 4 -I../sysdeps/ieee754/dbl-64 -I../sysdeps/ieee754/flt-32 >> -I../sysdeps/aarch64/soft-fp -I../sysdeps/ieee754 >> -I../sysdeps/generic -I../nptl -I.. -I../libio -I. -nostdinc -i >> system >> /tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/tools/lib/gcc/aarch64-none-linux-gnu/8.0.0/include >> -isystem /tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc >> /tools/lib/gcc/aarch64-none-linux-gnu/8.0.0/include-fixed -isystem >> /tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/sysroot-aarch64-none-linux-gnu/usr/include >> -D_LIBC_REE >> NTRANT -include ../include/libc-symbols.h -DPIC -DSHARED >> -DNOT_IN_libc=1 -DIS_IN_libthread_db=1 -DIN_LIB=libthread_db -o >> /tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccs >> rc/obj-aarch64-none-linux-gnu/glibc-1/nptl_db/fetch-value.os -MD -MP >> -MF >> /tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/glibc-1/nptl_db/fetch- >> value.os.dt -MT >> /tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/glibc-1/nptl_db/fetch-value.os >> during GIMPLE pass: vrp >> fetch-value.c: In function '_td_locate_field': >> fetch-value.c:44:1: internal compiler error: Segmentation fault >> _td_locate_field (td_thragent_t *ta, >> ^~~~~~~~~~~~~~~~ >> 0xb87c75 crash_signal >> /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/toplev.c:325 >> 0x5b1934 contains_struct_check >> /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree.h:3459 >> 0x5b1934 wi::to_wide(tree_node const*) >> /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree.h:5247 >> 0xf05557 vr_values::two_valued_val_range_p(tree_node*, tree_node**, >> tree_node**) >> /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/vr-values.c:4051 >> 0xf0ee48 vr_values::simplify_stmt_using_ranges(gimple_stmt_iterator*) >> /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/vr-values.c:4115 >> 0xd7068f >> substitute_and_fold_dom_walker::before_dom_children(basic_block_def*) >> /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-ssa-propagate.c:1073 >> 0x1204860 dom_walker::walk(basic_block_def*) >> /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/domwalk.c:308 >> 0xd6e805 substitute_and_fold_engine::substitute_and_fold() >> /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-ssa-propagate.c:1173 >> 0xe71b0b vrp_prop::vrp_finalize(bool) >> /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-vrp.c:6788 >> 0xe81667 execute_vrp >> /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-vrp.c:6864 >> Please submit a full bug report, > > Looks like PR 83104. > > (I am a bit curious what the code looks like, since the testcase in the PR is > rather on the undefined side of things, though ok for something embedded) if (idx != 0 && DB_DESC_NELEM (desc) != 0 && idx - (psaddr_t) 0 > DB_DESC_NELEM (desc)) "idx - (psaddr_t) 0": glibc should probably avoid doing pointer arithmetic with a null pointer, "(ptrdiff_t) idx" looks equivalent.
On 22 November 2017 at 09:29, Marc Glisse <marc.glisse@inria.fr> wrote: > On Wed, 22 Nov 2017, Marc Glisse wrote: > >> On Wed, 22 Nov 2017, Christophe Lyon wrote: >> >>> Since you committed this patch (r255021), my cross-builds of GCC for >>> aarch64-linux-gnu fail while building glibc: >>> >>> /tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/tools/bin/aarch64-none-linux-gnu-gcc >>> fetch-value.c -c -std=gnu99 -fgnu89-inline -O2 -Wall -Winline -Wundef >>> -Wwrite-string >>> s -fmerge-all-constants -frounding-math -g -Wstrict-prototypes -fPIC >>> -I../include >>> >>> >>> -I/tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/gl >>> ibc-1/nptl_db >>> -I/tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/glibc-1 >>> -I../sysdeps/unix/sysv/linux/aarch64/nptl -I../sysdeps/unix/sysv/lin >>> ux/aarch64 -I../sysdeps/unix/sysv/linux/generic >>> -I../sysdeps/unix/sysv/linux/wordsize-64 >>> -I../nptl/sysdeps/unix/sysv/linux -I../nptl/sysdeps/pthread >>> -I../sysdeps/pthread -I >>> ../sysdeps/unix/sysv/linux -I../sysdeps/gnu -I../sysdeps/unix/inet >>> -I../nptl/sysdeps/unix/sysv -I../sysdeps/unix/sysv >>> -I../nptl/sysdeps/unix -I../sysdeps/unix -I../sysdeps >>> /posix -I../sysdeps/aarch64/fpu -I../sysdeps/aarch64/nptl >>> -I../sysdeps/aarch64 -I../sysdeps/wordsize-64 >>> -I../sysdeps/ieee754/ldbl-128 -I../sysdeps/ieee754/dbl-64/wordsize-6 >>> 4 -I../sysdeps/ieee754/dbl-64 -I../sysdeps/ieee754/flt-32 >>> -I../sysdeps/aarch64/soft-fp -I../sysdeps/ieee754 >>> -I../sysdeps/generic -I../nptl -I.. -I../libio -I. -nostdinc -i >>> system >>> /tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/tools/lib/gcc/aarch64-none-linux-gnu/8.0.0/include >>> -isystem /tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc >>> /tools/lib/gcc/aarch64-none-linux-gnu/8.0.0/include-fixed -isystem >>> >>> /tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/sysroot-aarch64-none-linux-gnu/usr/include >>> -D_LIBC_REE >>> NTRANT -include ../include/libc-symbols.h -DPIC -DSHARED >>> -DNOT_IN_libc=1 -DIS_IN_libthread_db=1 -DIN_LIB=libthread_db -o >>> /tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccs >>> rc/obj-aarch64-none-linux-gnu/glibc-1/nptl_db/fetch-value.os -MD -MP >>> -MF >>> /tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/glibc-1/nptl_db/fetch- >>> value.os.dt -MT >>> >>> /tmp/6857183_6.tmpdir/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/glibc-1/nptl_db/fetch-value.os >>> during GIMPLE pass: vrp >>> fetch-value.c: In function '_td_locate_field': >>> fetch-value.c:44:1: internal compiler error: Segmentation fault >>> _td_locate_field (td_thragent_t *ta, >>> ^~~~~~~~~~~~~~~~ >>> 0xb87c75 crash_signal >>> >>> /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/toplev.c:325 >>> 0x5b1934 contains_struct_check >>> >>> /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree.h:3459 >>> 0x5b1934 wi::to_wide(tree_node const*) >>> >>> /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree.h:5247 >>> 0xf05557 vr_values::two_valued_val_range_p(tree_node*, tree_node**, >>> tree_node**) >>> >>> /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/vr-values.c:4051 >>> 0xf0ee48 vr_values::simplify_stmt_using_ranges(gimple_stmt_iterator*) >>> >>> /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/vr-values.c:4115 >>> 0xd7068f >>> substitute_and_fold_dom_walker::before_dom_children(basic_block_def*) >>> >>> /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-ssa-propagate.c:1073 >>> 0x1204860 dom_walker::walk(basic_block_def*) >>> >>> /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/domwalk.c:308 >>> 0xd6e805 substitute_and_fold_engine::substitute_and_fold() >>> >>> /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-ssa-propagate.c:1173 >>> 0xe71b0b vrp_prop::vrp_finalize(bool) >>> >>> /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-vrp.c:6788 >>> 0xe81667 execute_vrp >>> >>> /tmp/6857183_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-vrp.c:6864 >>> Please submit a full bug report, >> >> >> Looks like PR 83104. >> >> (I am a bit curious what the code looks like, since the testcase in the PR >> is rather on the undefined side of things, though ok for something embedded) > > > if (idx != 0 && DB_DESC_NELEM (desc) != 0 > && idx - (psaddr_t) 0 > DB_DESC_NELEM (desc)) > > "idx - (psaddr_t) 0": glibc should probably avoid doing pointer arithmetic > with a null pointer, "(ptrdiff_t) idx" looks equivalent. > The patch you attached in PR83104 works for me (at least the build completes, I haven't run the tests) Thanks, Christophe > -- > Marc Glisse
On Mon, 20 Nov 2017, Marc Glisse wrote: > new version, regtested on powerpc64le-unknown-linux-gnu. The front-end > parts are up for review. > > 2017-10-28 Marc Glisse <marc.glisse@inria.fr> > > gcc/c/ > * c-fold.c (c_fully_fold_internal): Handle POINTER_DIFF_EXPR. > * c-typeck.c (pointer_diff): Use POINTER_DIFF_EXPR. > > gcc/c-family/ > * c-pretty-print.c (pp_c_additive_expression, > c_pretty_printer::expression): Handle POINTER_DIFF_EXPR. > > gcc/cp/ > * constexpr.c (cxx_eval_constant_expression, > potential_constant_expression_1): Handle POINTER_DIFF_EXPR. > * cp-gimplify.c (cp_fold): Likewise. > * error.c (dump_expr): Likewise. > * typeck.c (pointer_diff): Use POINTER_DIFF_EXPR. I'm afraid this broke building Wine. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83164 ddeml.c: In function ‘DDEML_AddThunk’: ddeml.c:181:28: error: type mismatch in pointer diff expression static struct ddeml_thunk* DDEML_AddThunk(DWORD instId, DWORD pfn16) ^~~~~~~~~~~~~~ int struct HDDEDATA__ * (*<T1c96>) (DWORD, UINT, UINT, struct HCONV__ *, struct HSZ__ *, struct HSZ__ *, struct HDDEDATA__ *, ULONG_PTR, ULONG_PTR) DWORD * _6 = WDML_InvokeCallback16 - _5; ddeml.c:181:28: internal compiler error: verify_gimple failed David, in addition to the ICE diagnostics also looks, umm, rather confusing? Gerald
On Sun, 2017-11-26 at 01:11 +0100, Gerald Pfeifer wrote: > On Mon, 20 Nov 2017, Marc Glisse wrote: > > new version, regtested on powerpc64le-unknown-linux-gnu. The front- > > end > > parts are up for review. > > > > 2017-10-28 Marc Glisse <marc.glisse@inria.fr> > > > > gcc/c/ > > * c-fold.c (c_fully_fold_internal): Handle POINTER_DIFF_EXPR. > > * c-typeck.c (pointer_diff): Use POINTER_DIFF_EXPR. > > > > gcc/c-family/ > > * c-pretty-print.c (pp_c_additive_expression, > > c_pretty_printer::expression): Handle POINTER_DIFF_EXPR. > > > > gcc/cp/ > > * constexpr.c (cxx_eval_constant_expression, > > potential_constant_expression_1): Handle POINTER_DIFF_EXPR. > > * cp-gimplify.c (cp_fold): Likewise. > > * error.c (dump_expr): Likewise. > > * typeck.c (pointer_diff): Use POINTER_DIFF_EXPR. > > I'm afraid this broke building Wine. > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83164 > > > ddeml.c: In function ‘DDEML_AddThunk’: > ddeml.c:181:28: error: type mismatch in pointer diff expression > static struct ddeml_thunk* DDEML_AddThunk(DWORD instId, DWORD > pfn16) > ^~~~~~~~~~~~~~ > int > > struct HDDEDATA__ * (*<T1c96>) (DWORD, UINT, UINT, struct HCONV__ *, > struct HSZ__ *, struct HSZ__ *, struct HDDEDATA__ *, ULONG_PTR, > ULONG_PTR) > > DWORD * > > _6 = WDML_InvokeCallback16 - _5; > ddeml.c:181:28: internal compiler error: verify_gimple failed > > > David, in addition to the ICE diagnostics also looks, umm, > rather confusing? Indeed. I've added some comments about that to BZ. Thanks Dave
On Mon, Nov 27, 2017 at 10:41:42AM -0500, David Malcolm wrote: > On Sun, 2017-11-26 at 01:11 +0100, Gerald Pfeifer wrote: > > On Mon, 20 Nov 2017, Marc Glisse wrote: > > > new version, regtested on powerpc64le-unknown-linux-gnu. The front- > > > end > > > parts are up for review. > > > > > > 2017-10-28 Marc Glisse <marc.glisse@inria.fr> > > > > > > gcc/c/ > > > * c-fold.c (c_fully_fold_internal): Handle POINTER_DIFF_EXPR. > > > * c-typeck.c (pointer_diff): Use POINTER_DIFF_EXPR. > > > > > > gcc/c-family/ > > > * c-pretty-print.c (pp_c_additive_expression, > > > c_pretty_printer::expression): Handle POINTER_DIFF_EXPR. > > > > > > gcc/cp/ > > > * constexpr.c (cxx_eval_constant_expression, > > > potential_constant_expression_1): Handle POINTER_DIFF_EXPR. > > > * cp-gimplify.c (cp_fold): Likewise. > > > * error.c (dump_expr): Likewise. > > > * typeck.c (pointer_diff): Use POINTER_DIFF_EXPR. > > > > I'm afraid this broke building Wine. > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83164 > > > > > > ddeml.c: In function ‘DDEML_AddThunk’: > > ddeml.c:181:28: error: type mismatch in pointer diff expression > > static struct ddeml_thunk* DDEML_AddThunk(DWORD instId, DWORD > > pfn16) > > ^~~~~~~~~~~~~~ > > int > > > > struct HDDEDATA__ * (*<T1c96>) (DWORD, UINT, UINT, struct HCONV__ *, > > struct HSZ__ *, struct HSZ__ *, struct HDDEDATA__ *, ULONG_PTR, > > ULONG_PTR) > > > > DWORD * > > > > _6 = WDML_InvokeCallback16 - _5; > > ddeml.c:181:28: internal compiler error: verify_gimple failed > > > > > > David, in addition to the ICE diagnostics also looks, umm, > > rather confusing? This is a checking ICE, so in a non-buggy compiler it should never happen, so doesn't need to be too user friendly, it is enough that we can find out what's going on. > Indeed. I've added some comments about that to BZ. Jakub
Index: gcc/c/c-fold.c =================================================================== --- gcc/c/c-fold.c (revision 254183) +++ gcc/c/c-fold.c (working copy) @@ -238,20 +238,21 @@ c_fully_fold_internal (tree expr, bool i case COMPOUND_EXPR: case MODIFY_EXPR: case PREDECREMENT_EXPR: case PREINCREMENT_EXPR: case POSTDECREMENT_EXPR: case POSTINCREMENT_EXPR: case PLUS_EXPR: case MINUS_EXPR: case MULT_EXPR: case POINTER_PLUS_EXPR: + case POINTER_DIFF_EXPR: case TRUNC_DIV_EXPR: case CEIL_DIV_EXPR: case FLOOR_DIV_EXPR: case TRUNC_MOD_EXPR: case RDIV_EXPR: case EXACT_DIV_EXPR: case LSHIFT_EXPR: case RSHIFT_EXPR: case BIT_IOR_EXPR: case BIT_XOR_EXPR: Index: gcc/c/c-typeck.c =================================================================== --- gcc/c/c-typeck.c (revision 254183) +++ gcc/c/c-typeck.c (working copy) @@ -3772,21 +3772,21 @@ parser_build_binary_op (location_t locat && TREE_CODE (type2) == ENUMERAL_TYPE && TYPE_MAIN_VARIANT (type1) != TYPE_MAIN_VARIANT (type2)) warning_at (location, OPT_Wenum_compare, "comparison between %qT and %qT", type1, type2); return result; } /* Return a tree for the difference of pointers OP0 and OP1. - The resulting tree has type int. */ + The resulting tree has type ptrdiff_t. */ static tree pointer_diff (location_t loc, tree op0, tree op1) { tree restype = ptrdiff_type_node; tree result, inttype; 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)); @@ -3804,43 +3804,50 @@ pointer_diff (location_t loc, tree op0, to exist because the caller verified that comp_target_types returned non-zero. */ if (!addr_space_superset (as0, as1, &as_common)) gcc_unreachable (); common_type = common_pointer_type (TREE_TYPE (op0), TREE_TYPE (op1)); op0 = convert (common_type, op0); op1 = convert (common_type, op1); } - /* Determine integer type to perform computations in. This will usually + /* Determine integer type result of the subtraction. This will usually be the same as the result type (ptrdiff_t), but may need to be a wider type if pointers for the address space are wider than ptrdiff_t. */ if (TYPE_PRECISION (restype) < TYPE_PRECISION (TREE_TYPE (op0))) inttype = c_common_type_for_size (TYPE_PRECISION (TREE_TYPE (op0)), 0); else inttype = restype; if (TREE_CODE (target_type) == VOID_TYPE) pedwarn (loc, OPT_Wpointer_arith, "pointer of type %<void *%> used in subtraction"); if (TREE_CODE (target_type) == FUNCTION_TYPE) pedwarn (loc, OPT_Wpointer_arith, "pointer to a function used in subtraction"); - /* First do the subtraction as integers; - then drop through to build the divide operator. - Do not do default conversions on the minus operator - in case restype is a short type. */ - - op0 = build_binary_op (loc, - MINUS_EXPR, convert (inttype, op0), - convert (inttype, op1), false); + /* First do the subtraction, then build the divide operator + and only convert at the very end. + Do not do default conversions in case restype is a short type. */ + + /* POINTER_DIFF_EXPR requires a signed integer type of the same size as + pointers. If some platform cannot provide that, or has a larger + ptrdiff_type to support differences larger than half the address + space, cast the pointers to some larger integer type and do the + computations in that type. */ + if (TYPE_PRECISION (inttype) > TYPE_PRECISION (TREE_TYPE (op0))) + op0 = build_binary_op (loc, MINUS_EXPR, convert (inttype, op0), + convert (inttype, op1), false); + else + op0 = build2_loc (loc, POINTER_DIFF_EXPR, inttype, op0, op1); + /* This generates an error if op1 is pointer to incomplete type. */ if (!COMPLETE_OR_VOID_TYPE_P (TREE_TYPE (TREE_TYPE (orig_op1)))) error_at (loc, "arithmetic on pointer to an incomplete type"); op1 = c_size_in_bytes (target_type); if (pointer_to_zero_sized_aggr_p (TREE_TYPE (orig_op1))) error_at (loc, "arithmetic on pointer to an empty aggregate"); /* Divide by the size, in easiest possible way. */ Index: gcc/c-family/c-pretty-print.c =================================================================== --- gcc/c-family/c-pretty-print.c (revision 254183) +++ gcc/c-family/c-pretty-print.c (working copy) @@ -1869,20 +1869,21 @@ c_pretty_printer::multiplicative_express additive-expression - multiplicative-expression */ static void pp_c_additive_expression (c_pretty_printer *pp, tree e) { enum tree_code code = TREE_CODE (e); switch (code) { case POINTER_PLUS_EXPR: case PLUS_EXPR: + case POINTER_DIFF_EXPR: case MINUS_EXPR: pp_c_additive_expression (pp, TREE_OPERAND (e, 0)); pp_c_whitespace (pp); if (code == PLUS_EXPR || code == POINTER_PLUS_EXPR) pp_plus (pp); else pp_minus (pp); pp_c_whitespace (pp); pp->multiplicative_expression (TREE_OPERAND (e, 1)); break; @@ -2285,20 +2286,21 @@ c_pretty_printer::expression (tree e) case NE_EXPR: pp_c_equality_expression (this, e); break; case COND_EXPR: conditional_expression (e); break; case POINTER_PLUS_EXPR: case PLUS_EXPR: + case POINTER_DIFF_EXPR: case MINUS_EXPR: pp_c_additive_expression (this, e); break; case MODIFY_EXPR: case INIT_EXPR: assignment_expression (e); break; case COMPOUND_EXPR: Index: gcc/cfgexpand.c =================================================================== --- gcc/cfgexpand.c (revision 254183) +++ gcc/cfgexpand.c (working copy) @@ -4621,20 +4621,21 @@ expand_debug_expr (tree exp) /* We always sign-extend, regardless of the signedness of the operand, because the operand is always unsigned here even if the original C expression is signed. */ op1 = simplify_gen_unary (SIGN_EXTEND, op0_mode, op1, op1_mode); } /* Fall through. */ case PLUS_EXPR: return simplify_gen_binary (PLUS, mode, op0, op1); case MINUS_EXPR: + case POINTER_DIFF_EXPR: return simplify_gen_binary (MINUS, mode, op0, op1); case MULT_EXPR: return simplify_gen_binary (MULT, mode, op0, op1); case RDIV_EXPR: case TRUNC_DIV_EXPR: case EXACT_DIV_EXPR: if (unsignedp) return simplify_gen_binary (UDIV, mode, op0, op1); Index: gcc/cp/constexpr.c =================================================================== --- gcc/cp/constexpr.c (revision 254183) +++ gcc/cp/constexpr.c (working copy) @@ -4275,20 +4275,21 @@ cxx_eval_constant_expression (const cons return t; op1 = TREE_OPERAND (t, 1); r = cxx_eval_constant_expression (ctx, op1, lval, non_constant_p, overflow_p, jump_target); } } break; case POINTER_PLUS_EXPR: + case POINTER_DIFF_EXPR: case PLUS_EXPR: case MINUS_EXPR: case MULT_EXPR: case TRUNC_DIV_EXPR: case CEIL_DIV_EXPR: case FLOOR_DIV_EXPR: case ROUND_DIV_EXPR: case TRUNC_MOD_EXPR: case CEIL_MOD_EXPR: case ROUND_MOD_EXPR: @@ -5509,20 +5510,21 @@ potential_constant_expression_1 (tree t, && TYPE_POLYMORPHIC_P (TREE_TYPE (e))) { if (flags & tf_error) error_at (loc, "typeid-expression is not a constant expression " "because %qE is of polymorphic type", e); return false; } return true; } + case POINTER_DIFF_EXPR: case MINUS_EXPR: want_rval = true; goto binary; case LT_EXPR: case LE_EXPR: case GT_EXPR: case GE_EXPR: case EQ_EXPR: case NE_EXPR: Index: gcc/cp/cp-gimplify.c =================================================================== --- gcc/cp/cp-gimplify.c (revision 254183) +++ gcc/cp/cp-gimplify.c (working copy) @@ -2205,20 +2205,21 @@ cp_fold (tree x) case POSTINCREMENT_EXPR: case INIT_EXPR: case PREDECREMENT_EXPR: case PREINCREMENT_EXPR: case COMPOUND_EXPR: case MODIFY_EXPR: rval_ops = false; /* FALLTHRU */ case POINTER_PLUS_EXPR: case PLUS_EXPR: + case POINTER_DIFF_EXPR: case MINUS_EXPR: case MULT_EXPR: case TRUNC_DIV_EXPR: case CEIL_DIV_EXPR: case FLOOR_DIV_EXPR: case ROUND_DIV_EXPR: case TRUNC_MOD_EXPR: case CEIL_MOD_EXPR: case ROUND_MOD_EXPR: case RDIV_EXPR: Index: gcc/cp/error.c =================================================================== --- gcc/cp/error.c (revision 254183) +++ gcc/cp/error.c (working copy) @@ -2221,20 +2221,24 @@ dump_expr (cxx_pretty_printer *pp, tree default argument. Note we may have cleared out the first operand in expand_expr, so don't go killing ourselves. */ if (TREE_OPERAND (t, 1)) dump_expr (pp, TREE_OPERAND (t, 1), flags | TFF_EXPR_IN_PARENS); break; case POINTER_PLUS_EXPR: dump_binary_op (pp, "+", t, flags); break; + case POINTER_DIFF_EXPR: + dump_binary_op (pp, "-", t, flags); + break; + case INIT_EXPR: case MODIFY_EXPR: dump_binary_op (pp, assignment_operator_name_info[NOP_EXPR].name, t, flags); break; case PLUS_EXPR: case MINUS_EXPR: case MULT_EXPR: case TRUNC_DIV_EXPR: Index: gcc/cp/typeck.c =================================================================== --- gcc/cp/typeck.c (revision 254183) +++ gcc/cp/typeck.c (working copy) @@ -4305,20 +4305,21 @@ cp_build_binary_op (location_t location, converted = 1; break; } default: break; } } switch (code) { + case POINTER_DIFF_EXPR: case MINUS_EXPR: /* Subtraction of two similar pointers. We must subtract them as integers, then divide by object size. */ if (code0 == POINTER_TYPE && code1 == POINTER_TYPE && same_type_ignoring_top_level_qualifiers_p (TREE_TYPE (type0), TREE_TYPE (type1))) return pointer_diff (location, op0, op1, common_pointer_type (type0, type1), complain); /* In all other cases except pointer - int, the usual arithmetic rules apply. */ @@ -5362,21 +5363,21 @@ cp_pointer_int_sum (location_t loc, enum intop, complain & tf_warning_or_error); } /* Return a tree for the difference of pointers OP0 and OP1. The resulting tree has type int. */ static tree pointer_diff (location_t loc, tree op0, tree op1, tree ptrtype, tsubst_flags_t complain) { - tree result; + tree result, inttype; tree restype = ptrdiff_type_node; tree target_type = TREE_TYPE (ptrtype); if (!complete_type_or_else (target_type, NULL_TREE)) return error_mark_node; if (VOID_TYPE_P (target_type)) { if (complain & tf_error) permerror (loc, "ISO C++ forbids using pointer of " @@ -5394,28 +5395,45 @@ pointer_diff (location_t loc, tree op0, } if (TREE_CODE (target_type) == METHOD_TYPE) { if (complain & tf_error) permerror (loc, "ISO C++ forbids using pointer to " "a method in subtraction"); else return error_mark_node; } - /* First do the subtraction as integers; - then drop through to build the divide operator. */ + /* Determine integer type result of the subtraction. This will usually + be the same as the result type (ptrdiff_t), but may need to be a wider + type if pointers for the address space are wider than ptrdiff_t. */ + if (TYPE_PRECISION (restype) < TYPE_PRECISION (TREE_TYPE (op0))) + inttype = c_common_type_for_size (TYPE_PRECISION (TREE_TYPE (op0)), 0); + else + inttype = restype; - op0 = cp_build_binary_op (loc, - MINUS_EXPR, - cp_convert (restype, op0, complain), - cp_convert (restype, op1, complain), - complain); + /* First do the subtraction, then build the divide operator + and only convert at the very end. + Do not do default conversions in case restype is a short type. */ + + /* POINTER_DIFF_EXPR requires a signed integer type of the same size as + pointers. If some platform cannot provide that, or has a larger + ptrdiff_type to support differences larger than half the address + space, cast the pointers to some larger integer type and do the + computations in that type. */ + if (TYPE_PRECISION (inttype) > TYPE_PRECISION (TREE_TYPE (op0))) + op0 = cp_build_binary_op (loc, + MINUS_EXPR, + cp_convert (inttype, op0, complain), + cp_convert (inttype, op1, complain), + complain); + else + op0 = build2_loc (loc, POINTER_DIFF_EXPR, inttype, op0, op1); /* This generates an error if op1 is a pointer to an incomplete type. */ if (!COMPLETE_TYPE_P (TREE_TYPE (TREE_TYPE (op1)))) { if (complain & tf_error) error_at (loc, "invalid use of a pointer to an incomplete type in " "pointer arithmetic"); else return error_mark_node; } @@ -5427,23 +5445,23 @@ pointer_diff (location_t loc, tree op0, else return error_mark_node; } op1 = (TYPE_PTROB_P (ptrtype) ? size_in_bytes_loc (loc, target_type) : integer_one_node); /* Do the division. */ - result = build2_loc (loc, EXACT_DIV_EXPR, restype, op0, - cp_convert (restype, op1, complain)); - return result; + result = build2_loc (loc, EXACT_DIV_EXPR, inttype, op0, + cp_convert (inttype, op1, complain)); + return cp_convert (restype, result, complain); } /* Construct and perhaps optimize a tree representation for a unary operation. CODE, a tree_code, specifies the operation and XARG is the operand. */ tree build_x_unary_op (location_t loc, enum tree_code code, cp_expr xarg, tsubst_flags_t complain) { Index: gcc/doc/generic.texi =================================================================== --- gcc/doc/generic.texi (revision 254183) +++ gcc/doc/generic.texi (working copy) @@ -1217,20 +1217,21 @@ the byte offset of the field, but should @tindex RSHIFT_EXPR @tindex BIT_IOR_EXPR @tindex BIT_XOR_EXPR @tindex BIT_AND_EXPR @tindex TRUTH_ANDIF_EXPR @tindex TRUTH_ORIF_EXPR @tindex TRUTH_AND_EXPR @tindex TRUTH_OR_EXPR @tindex TRUTH_XOR_EXPR @tindex POINTER_PLUS_EXPR +@tindex POINTER_DIFF_EXPR @tindex PLUS_EXPR @tindex MINUS_EXPR @tindex MULT_EXPR @tindex MULT_HIGHPART_EXPR @tindex RDIV_EXPR @tindex TRUNC_DIV_EXPR @tindex FLOOR_DIV_EXPR @tindex CEIL_DIV_EXPR @tindex ROUND_DIV_EXPR @tindex TRUNC_MOD_EXPR @@ -1406,22 +1407,26 @@ always of @code{BOOLEAN_TYPE} or @code{I These nodes represent logical and, logical or, and logical exclusive or. They are strict; both arguments are always evaluated. There are no corresponding operators in C or C++, but the front end will sometimes generate these expressions anyhow, if it can tell that strictness does not matter. The type of the operands and that of the result are always of @code{BOOLEAN_TYPE} or @code{INTEGER_TYPE}. @item POINTER_PLUS_EXPR This node represents pointer arithmetic. The first operand is always a pointer/reference type. The second operand is always an unsigned -integer type compatible with sizetype. This is the only binary -arithmetic operand that can operate on pointer types. +integer type compatible with sizetype. This and POINTER_DIFF_EXPR are +the only binary arithmetic operators that can operate on pointer types. + +@item POINTER_DIFF_EXPR +This node represents pointer subtraction. The two operands always +have pointer/reference type. It returns a signed integer. @item PLUS_EXPR @itemx MINUS_EXPR @itemx MULT_EXPR These nodes represent various binary arithmetic operations. Respectively, these operations are addition, subtraction (of the second operand from the first) and multiplication. Their operands may have either integral or floating type, but there will never be case in which one operand is of floating type and the other is of integral type. Index: gcc/expr.c =================================================================== --- gcc/expr.c (revision 254183) +++ gcc/expr.c (working copy) @@ -8542,20 +8542,21 @@ expand_expr_real_2 (sepops ops, rtx targ if (op1 == const0_rtx) return op0; goto binop2; } expand_operands (treeop0, treeop1, subtarget, &op0, &op1, modifier); return REDUCE_BIT_FIELD (simplify_gen_binary (PLUS, mode, op0, op1)); case MINUS_EXPR: + case POINTER_DIFF_EXPR: do_minus: /* For initializers, we are allowed to return a MINUS of two symbolic constants. Here we handle all cases when both operands are constant. */ /* Handle difference of two symbolic constants, for the sake of an initializer. */ if ((modifier == EXPAND_SUM || modifier == EXPAND_INITIALIZER) && really_constant_p (treeop0) && really_constant_p (treeop1)) { Index: gcc/fold-const.c =================================================================== --- gcc/fold-const.c (revision 254183) +++ gcc/fold-const.c (working copy) @@ -1134,20 +1134,29 @@ const_binop (enum tree_code code, tree a STRIP_NOPS (arg1); STRIP_NOPS (arg2); if (TREE_CODE (arg1) == INTEGER_CST && TREE_CODE (arg2) == INTEGER_CST) { if (code == POINTER_PLUS_EXPR) return int_const_binop (PLUS_EXPR, arg1, fold_convert (TREE_TYPE (arg1), arg2)); + /* It is better if the caller provides the return type. */ + if (code == POINTER_DIFF_EXPR) + { + offset_int res = wi::sub (wi::to_offset (arg1), + wi::to_offset (arg2)); + return force_fit_type (signed_type_for (TREE_TYPE (arg1)), res, 1, + TREE_OVERFLOW (arg1) | TREE_OVERFLOW (arg2)); + } + return int_const_binop (code, arg1, arg2); } if (TREE_CODE (arg1) == REAL_CST && TREE_CODE (arg2) == REAL_CST) { machine_mode mode; REAL_VALUE_TYPE d1; REAL_VALUE_TYPE d2; REAL_VALUE_TYPE value; REAL_VALUE_TYPE result; @@ -1476,20 +1485,31 @@ const_binop (enum tree_code code, tree t switch (code) { case COMPLEX_EXPR: if ((TREE_CODE (arg1) == REAL_CST && TREE_CODE (arg2) == REAL_CST) || (TREE_CODE (arg1) == INTEGER_CST && TREE_CODE (arg2) == INTEGER_CST)) return build_complex (type, arg1, arg2); return NULL_TREE; + /* Use the return type, in case it might be larger than ptrdiff_t. */ + case POINTER_DIFF_EXPR: + if (TREE_CODE (arg1) == INTEGER_CST && TREE_CODE (arg2) == INTEGER_CST) + { + offset_int res = wi::sub (wi::to_offset (arg1), + wi::to_offset (arg2)); + return force_fit_type (type, res, 1, + TREE_OVERFLOW (arg1) | TREE_OVERFLOW (arg2)); + } + return NULL_TREE; + case VEC_PACK_TRUNC_EXPR: case VEC_PACK_FIX_TRUNC_EXPR: { unsigned int out_nelts, in_nelts, i; if (TREE_CODE (arg1) != VECTOR_CST || TREE_CODE (arg2) != VECTOR_CST) return NULL_TREE; in_nelts = VECTOR_CST_NELTS (arg1); @@ -8794,40 +8814,47 @@ fold_vec_perm (tree type, tree arg0, tre else return build_vector (type, out_elts); } /* Try to fold a pointer difference of type TYPE two address expressions of array references AREF0 and AREF1 using location LOC. Return a simplified expression for the difference or NULL_TREE. */ static tree fold_addr_of_array_ref_difference (location_t loc, tree type, - tree aref0, tree aref1) + tree aref0, tree aref1, + bool use_pointer_diff) { tree base0 = TREE_OPERAND (aref0, 0); tree base1 = TREE_OPERAND (aref1, 0); tree base_offset = build_int_cst (type, 0); /* If the bases are array references as well, recurse. If the bases are pointer indirections compute the difference of the pointers. If the bases are equal, we are set. */ if ((TREE_CODE (base0) == ARRAY_REF && TREE_CODE (base1) == ARRAY_REF && (base_offset - = fold_addr_of_array_ref_difference (loc, type, base0, base1))) + = fold_addr_of_array_ref_difference (loc, type, base0, base1, + use_pointer_diff))) || (INDIRECT_REF_P (base0) && INDIRECT_REF_P (base1) && (base_offset - = fold_binary_loc (loc, MINUS_EXPR, type, - fold_convert (type, TREE_OPERAND (base0, 0)), - fold_convert (type, - TREE_OPERAND (base1, 0))))) + = use_pointer_diff + ? fold_binary_loc (loc, POINTER_DIFF_EXPR, type, + TREE_OPERAND (base0, 0), + TREE_OPERAND (base1, 0)) + : fold_binary_loc (loc, MINUS_EXPR, type, + fold_convert (type, + TREE_OPERAND (base0, 0)), + fold_convert (type, + TREE_OPERAND (base1, 0))))) || operand_equal_p (base0, base1, OEP_ADDRESS_OF)) { tree op0 = fold_convert_loc (loc, type, TREE_OPERAND (aref0, 1)); tree op1 = fold_convert_loc (loc, type, TREE_OPERAND (aref1, 1)); tree esz = fold_convert_loc (loc, type, array_ref_element_size (aref0)); tree diff = fold_build2_loc (loc, MINUS_EXPR, type, op0, op1); return fold_build2_loc (loc, PLUS_EXPR, type, base_offset, fold_build2_loc (loc, MULT_EXPR, type, diff, esz)); @@ -9687,21 +9714,41 @@ fold_binary_loc (location_t loc, } return fold_convert_loc (loc, type, associate_trees (loc, var0, con0, code, atype)); } } return NULL_TREE; + case POINTER_DIFF_EXPR: case MINUS_EXPR: + /* Fold &a[i] - &a[j] to i-j. */ + if (TREE_CODE (arg0) == ADDR_EXPR + && TREE_CODE (TREE_OPERAND (arg0, 0)) == ARRAY_REF + && TREE_CODE (arg1) == ADDR_EXPR + && TREE_CODE (TREE_OPERAND (arg1, 0)) == ARRAY_REF) + { + tree tem = fold_addr_of_array_ref_difference (loc, type, + TREE_OPERAND (arg0, 0), + TREE_OPERAND (arg1, 0), + code + == POINTER_DIFF_EXPR); + if (tem) + return tem; + } + + /* Further transformations are not for pointers. */ + if (code == POINTER_DIFF_EXPR) + return NULL_TREE; + /* (-A) - B -> (-B) - A where B is easily negated and we can swap. */ if (TREE_CODE (arg0) == NEGATE_EXPR && negate_expr_p (op1)) return fold_build2_loc (loc, MINUS_EXPR, type, negate_expr (op1), fold_convert_loc (loc, type, TREE_OPERAND (arg0, 0))); /* Fold __complex__ ( x, 0 ) - __complex__ ( 0, y ) to __complex__ ( x, -y ). This is not the same for SNaNs or if @@ -9745,33 +9792,20 @@ fold_binary_loc (location_t loc, && ! TYPE_OVERFLOW_SANITIZED (type) && ((FLOAT_TYPE_P (type) /* Avoid this transformation if B is a positive REAL_CST. */ && (TREE_CODE (op1) != REAL_CST || REAL_VALUE_NEGATIVE (TREE_REAL_CST (op1)))) || INTEGRAL_TYPE_P (type))) return fold_build2_loc (loc, PLUS_EXPR, type, fold_convert_loc (loc, type, arg0), negate_expr (op1)); - /* Fold &a[i] - &a[j] to i-j. */ - if (TREE_CODE (arg0) == ADDR_EXPR - && TREE_CODE (TREE_OPERAND (arg0, 0)) == ARRAY_REF - && TREE_CODE (arg1) == ADDR_EXPR - && TREE_CODE (TREE_OPERAND (arg1, 0)) == ARRAY_REF) - { - tree tem = fold_addr_of_array_ref_difference (loc, type, - TREE_OPERAND (arg0, 0), - TREE_OPERAND (arg1, 0)); - if (tem) - return tem; - } - /* Handle (A1 * C1) - (A2 * C2) with A1, A2 or C1, C2 being the same or one. Make sure the type is not saturating and has the signedness of the stripped operands, as fold_plusminus_mult_expr will re-associate. ??? The latter condition should use TYPE_OVERFLOW_* flags instead. */ if ((TREE_CODE (arg0) == MULT_EXPR || TREE_CODE (arg1) == MULT_EXPR) && !TYPE_SATURATING (type) && TYPE_UNSIGNED (type) == TYPE_UNSIGNED (TREE_TYPE (arg0)) && TYPE_UNSIGNED (type) == TYPE_UNSIGNED (TREE_TYPE (arg1)) && (!FLOAT_TYPE_P (type) || flag_associative_math)) Index: gcc/match.pd =================================================================== --- gcc/match.pd (revision 254183) +++ gcc/match.pd (working copy) @@ -117,20 +117,23 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) /* Simplify x - x. This is unsafe for certain floats even in non-IEEE formats. In IEEE, it is unsafe because it does wrong for NaNs. Also note that operand_equal_p is always false if an operand is volatile. */ (simplify (minus @0 @0) (if (!FLOAT_TYPE_P (type) || !HONOR_NANS (type)) { build_zero_cst (type); })) +(simplify + (pointer_diff @@0 @0) + { build_zero_cst (type); }) (simplify (mult @0 integer_zerop@1) @1) /* Maybe fold x * 0 to 0. The expressions aren't the same when x is NaN, since x * 0 is also NaN. Nor are they the same in modes with signed zeros, since multiplying a negative value by 0 gives -0, not +0. */ (simplify @@ -1420,20 +1423,24 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) tem5 = ptr1 + tem4; and produce tem5 = ptr2; */ (simplify (pointer_plus @0 (convert?@2 (minus@3 (convert @1) (convert @0)))) /* Conditionally look through a sign-changing conversion. */ (if (TYPE_PRECISION (TREE_TYPE (@2)) == TYPE_PRECISION (TREE_TYPE (@3)) && ((GIMPLE && useless_type_conversion_p (type, TREE_TYPE (@1))) || (GENERIC && type == TREE_TYPE (@1)))) @1)) +(simplify + (pointer_plus @0 (convert?@2 (pointer_diff@3 @1 @@0))) + (if (TYPE_PRECISION (TREE_TYPE (@2)) >= TYPE_PRECISION (TREE_TYPE (@3))) + (convert @1))) /* Pattern match tem = (sizetype) ptr; tem = tem & algn; tem = -tem; ... = ptr p+ tem; and produce the simpler and easier to analyze with respect to alignment ... = ptr & ~algn; */ (simplify (pointer_plus @0 (negate (bit_and (convert @0) INTEGER_CST@1))) @@ -1446,20 +1453,34 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) (if (tree_nop_conversion_p (type, TREE_TYPE (@0))) (with { HOST_WIDE_INT diff; } (if (ptr_difference_const (@0, @1, &diff)) { build_int_cst_type (type, diff); })))) (simplify (minus (convert @0) (convert ADDR_EXPR@1)) (if (tree_nop_conversion_p (type, TREE_TYPE (@0))) (with { HOST_WIDE_INT diff; } (if (ptr_difference_const (@0, @1, &diff)) { build_int_cst_type (type, diff); })))) +(simplify + (pointer_diff (convert?@2 ADDR_EXPR@0) (convert?@3 @1)) + (if (tree_nop_conversion_p (TREE_TYPE(@2), TREE_TYPE (@0)) + && tree_nop_conversion_p (TREE_TYPE(@3), TREE_TYPE (@1))) + (with { HOST_WIDE_INT diff; } + (if (ptr_difference_const (@0, @1, &diff)) + { build_int_cst_type (type, diff); })))) +(simplify + (pointer_diff (convert?@2 @0) (convert?@3 ADDR_EXPR@1)) + (if (tree_nop_conversion_p (TREE_TYPE(@2), TREE_TYPE (@0)) + && tree_nop_conversion_p (TREE_TYPE(@3), TREE_TYPE (@1))) + (with { HOST_WIDE_INT diff; } + (if (ptr_difference_const (@0, @1, &diff)) + { build_int_cst_type (type, diff); })))) /* 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. */ (simplify (bit_and (convert? @0) INTEGER_CST@1) (if (POINTER_TYPE_P (TREE_TYPE (@0)) && tree_nop_conversion_p (type, TREE_TYPE (@0))) (with { @@ -1643,20 +1664,26 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) || (INTEGRAL_TYPE_P (TREE_TYPE (@0)) && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))) /* For pointer types, if the conversion of A to the final type requires a sign- or zero-extension, then we have to punt - it is not defined which one is correct. */ || (POINTER_TYPE_P (TREE_TYPE (@0)) && TREE_CODE (@1) == INTEGER_CST && tree_int_cst_sign_bit (@1) == 0)) (convert @1)))) + (simplify + (pointer_diff (pointer_plus @@0 @1) @0) + /* The second argument of pointer_plus must be interpreted as signed, and + thus sign-extended if necessary. */ + (with { tree stype = signed_type_for (TREE_TYPE (@1)); } + (convert (convert:stype @1)))) /* (T)P - (T)(P + A) -> -(T) A */ (for add (plus pointer_plus) (simplify (minus (convert @0) (convert (add @@0 @1))) (if (element_precision (type) <= element_precision (TREE_TYPE (@1)) /* For integer types, if A has a smaller type than T the result depends on the possible overflow in P + A. @@ -1667,20 +1694,26 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) || (INTEGRAL_TYPE_P (TREE_TYPE (@0)) && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))) /* For pointer types, if the conversion of A to the final type requires a sign- or zero-extension, then we have to punt - it is not defined which one is correct. */ || (POINTER_TYPE_P (TREE_TYPE (@0)) && TREE_CODE (@1) == INTEGER_CST && tree_int_cst_sign_bit (@1) == 0)) (negate (convert @1))))) + (simplify + (pointer_diff @0 (pointer_plus @@0 @1)) + /* The second argument of pointer_plus must be interpreted as signed, and + thus sign-extended if necessary. */ + (with { tree stype = signed_type_for (TREE_TYPE (@1)); } + (negate (convert (convert:stype @1))))) /* (T)(P + A) - (T)(P + B) -> (T)A - (T)B */ (for add (plus pointer_plus) (simplify (minus (convert (add @@0 @1)) (convert (add @0 @2))) (if (element_precision (type) <= element_precision (TREE_TYPE (@1)) /* For integer types, if A has a smaller type than T the result depends on the possible overflow in P + A. @@ -1693,20 +1726,26 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) /* For pointer types, if the conversion of A to the final type requires a sign- or zero-extension, then we have to punt - it is not defined which one is correct. */ || (POINTER_TYPE_P (TREE_TYPE (@0)) && TREE_CODE (@1) == INTEGER_CST && tree_int_cst_sign_bit (@1) == 0 && TREE_CODE (@2) == INTEGER_CST && tree_int_cst_sign_bit (@2) == 0)) (minus (convert @1) (convert @2))))))) + (simplify + (pointer_diff (pointer_plus @@0 @1) (pointer_plus @0 @2)) + /* The second argument of pointer_plus must be interpreted as signed, and + thus sign-extended if necessary. */ + (with { tree stype = signed_type_for (TREE_TYPE (@1)); } + (minus (convert (convert:stype @1)) (convert (convert:stype @2))))) /* Simplifications of MIN_EXPR, MAX_EXPR, fmin() and fmax(). */ (for minmax (min max FMIN FMAX) (simplify (minmax @0 @0) @0)) /* min(max(x,y),y) -> y. */ (simplify @@ -2690,20 +2729,25 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) /* Transform comparisons of the form X - Y CMP 0 to X CMP Y. ??? The transformation is valid for the other operators if overflow is undefined for the type, but performing it here badly interacts with the transformation in fold_cond_expr_with_comparison which attempts to synthetize ABS_EXPR. */ (for cmp (eq ne) (simplify (cmp (minus@2 @0 @1) integer_zerop) (if (single_use (@2)) (cmp @0 @1)))) +(for cmp (eq ne) + (simplify + (cmp (pointer_diff@2 @0 @1) integer_zerop) + (if (single_use (@2)) + (cmp @0 @1)))) /* Transform comparisons of the form X * C1 CMP 0 to X CMP 0 in the signed arithmetic case. That form is created by the compiler often enough for folding it to be of value. One example is in computing loop trip counts after Operator Strength Reduction. */ (for cmp (simple_comparison) scmp (swapped_simple_comparison) (simplify (cmp (mult@3 @0 INTEGER_CST@1) integer_zerop@2) /* Handle unfolded multiplication by zero. */ Index: gcc/optabs-tree.c =================================================================== --- gcc/optabs-tree.c (revision 254183) +++ gcc/optabs-tree.c (working copy) @@ -216,20 +216,21 @@ optab_for_tree_code (enum tree_code code trapv = INTEGRAL_TYPE_P (type) && TYPE_OVERFLOW_TRAPS (type); switch (code) { case POINTER_PLUS_EXPR: case PLUS_EXPR: if (TYPE_SATURATING (type)) return TYPE_UNSIGNED (type) ? usadd_optab : ssadd_optab; return trapv ? addv_optab : add_optab; + case POINTER_DIFF_EXPR: case MINUS_EXPR: if (TYPE_SATURATING (type)) return TYPE_UNSIGNED (type) ? ussub_optab : sssub_optab; return trapv ? subv_optab : sub_optab; case MULT_EXPR: if (TYPE_SATURATING (type)) return TYPE_UNSIGNED (type) ? usmul_optab : ssmul_optab; return trapv ? smulv_optab : smul_optab; Index: gcc/tree-cfg.c =================================================================== --- gcc/tree-cfg.c (revision 254183) +++ gcc/tree-cfg.c (working copy) @@ -3136,20 +3136,31 @@ verify_expr (tree *tp, int *walk_subtree POINTER_PLUS_EXPR. */ if (POINTER_TYPE_P (TREE_TYPE (t))) { error ("invalid operand to plus/minus, type is a pointer"); return t; } CHECK_OP (0, "invalid operand to binary operator"); CHECK_OP (1, "invalid operand to binary operator"); break; + case POINTER_DIFF_EXPR: + if (!POINTER_TYPE_P (TREE_TYPE (TREE_OPERAND (t, 0))) + || !POINTER_TYPE_P (TREE_TYPE (TREE_OPERAND (t, 1)))) + { + error ("invalid operand to pointer diff, operand is not a pointer"); + return t; + } + CHECK_OP (0, "invalid operand to pointer diff"); + CHECK_OP (1, "invalid operand to pointer diff"); + break; + case POINTER_PLUS_EXPR: /* Check to make sure the first operand is a pointer or reference type. */ if (!POINTER_TYPE_P (TREE_TYPE (TREE_OPERAND (t, 0)))) { error ("invalid operand to pointer plus, first operand is not a pointer"); return t; } /* Check to make sure the second operand is a ptrofftype. */ if (!ptrofftype_p (TREE_TYPE (TREE_OPERAND (t, 1)))) { @@ -3971,20 +3982,39 @@ verify_gimple_assign_binary (gassign *st error ("type mismatch in pointer plus expression"); debug_generic_stmt (lhs_type); debug_generic_stmt (rhs1_type); debug_generic_stmt (rhs2_type); return true; } return false; } + case POINTER_DIFF_EXPR: + { + if (!POINTER_TYPE_P (rhs1_type) + || !POINTER_TYPE_P (rhs2_type) + || !types_compatible_p (rhs1_type, rhs2_type) + || TREE_CODE (lhs_type) != INTEGER_TYPE + || TYPE_UNSIGNED (lhs_type) + || TYPE_PRECISION (lhs_type) != TYPE_PRECISION (rhs1_type)) + { + error ("type mismatch in pointer diff expression"); + debug_generic_stmt (lhs_type); + debug_generic_stmt (rhs1_type); + debug_generic_stmt (rhs2_type); + return true; + } + + return false; + } + case TRUTH_ANDIF_EXPR: case TRUTH_ORIF_EXPR: case TRUTH_AND_EXPR: case TRUTH_OR_EXPR: case TRUTH_XOR_EXPR: gcc_unreachable (); case LT_EXPR: case LE_EXPR: Index: gcc/tree-inline.c =================================================================== --- gcc/tree-inline.c (revision 254183) +++ gcc/tree-inline.c (working copy) @@ -3894,20 +3894,21 @@ estimate_operator_cost (enum tree_code c return 0; /* Assign cost of 1 to usual operations. ??? We may consider mapping RTL costs to this. */ case COND_EXPR: case VEC_COND_EXPR: case VEC_PERM_EXPR: case PLUS_EXPR: case POINTER_PLUS_EXPR: + case POINTER_DIFF_EXPR: case MINUS_EXPR: case MULT_EXPR: case MULT_HIGHPART_EXPR: case FMA_EXPR: case ADDR_SPACE_CONVERT_EXPR: case FIXED_CONVERT_EXPR: case FIX_TRUNC_EXPR: case NEGATE_EXPR: Index: gcc/tree-pretty-print.c =================================================================== --- gcc/tree-pretty-print.c (revision 254183) +++ gcc/tree-pretty-print.c (working copy) @@ -2301,20 +2301,21 @@ dump_generic_node (pretty_printer *pp, t pp_greater (pp); break; /* Binary arithmetic and logic expressions. */ case WIDEN_SUM_EXPR: case WIDEN_MULT_EXPR: case MULT_EXPR: case MULT_HIGHPART_EXPR: case PLUS_EXPR: case POINTER_PLUS_EXPR: + case POINTER_DIFF_EXPR: case MINUS_EXPR: case TRUNC_DIV_EXPR: case CEIL_DIV_EXPR: case FLOOR_DIV_EXPR: case ROUND_DIV_EXPR: case TRUNC_MOD_EXPR: case CEIL_MOD_EXPR: case FLOOR_MOD_EXPR: case ROUND_MOD_EXPR: case RDIV_EXPR: @@ -3535,20 +3536,21 @@ op_code_prio (enum tree_code code) case LROTATE_EXPR: case RROTATE_EXPR: case VEC_WIDEN_LSHIFT_HI_EXPR: case VEC_WIDEN_LSHIFT_LO_EXPR: case WIDEN_LSHIFT_EXPR: return 11; case WIDEN_SUM_EXPR: case PLUS_EXPR: case POINTER_PLUS_EXPR: + case POINTER_DIFF_EXPR: case MINUS_EXPR: return 12; case VEC_WIDEN_MULT_HI_EXPR: case VEC_WIDEN_MULT_LO_EXPR: case WIDEN_MULT_EXPR: case DOT_PROD_EXPR: case WIDEN_MULT_PLUS_EXPR: case WIDEN_MULT_MINUS_EXPR: case MULT_EXPR: @@ -3721,20 +3723,21 @@ op_symbol_code (enum tree_code code) return "w+"; case WIDEN_MULT_EXPR: return "w*"; case MULT_HIGHPART_EXPR: return "h*"; case NEGATE_EXPR: case MINUS_EXPR: + case POINTER_DIFF_EXPR: return "-"; case BIT_NOT_EXPR: return "~"; case TRUTH_NOT_EXPR: return "!"; case MULT_EXPR: case INDIRECT_REF: Index: gcc/tree-vect-stmts.c =================================================================== --- gcc/tree-vect-stmts.c (revision 254183) +++ gcc/tree-vect-stmts.c (working copy) @@ -5258,24 +5258,26 @@ vectorizable_operation (gimple *stmt, gi /* Is STMT a vectorizable binary/unary operation? */ if (!is_gimple_assign (stmt)) return false; if (TREE_CODE (gimple_assign_lhs (stmt)) != SSA_NAME) return false; code = gimple_assign_rhs_code (stmt); - /* For pointer addition, we should use the normal plus for - the vector addition. */ + /* For pointer addition and subtraction, we should use the normal + plus and minus for the vector operation. */ if (code == POINTER_PLUS_EXPR) code = PLUS_EXPR; + if (code == POINTER_DIFF_EXPR) + code = MINUS_EXPR; /* Support only unary or binary operations. */ op_type = TREE_CODE_LENGTH (code); if (op_type != unary_op && op_type != binary_op && op_type != ternary_op) { if (dump_enabled_p ()) dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location, "num. args = %d (not unary/binary/ternary op).\n", op_type); return false; Index: gcc/tree-vrp.c =================================================================== --- gcc/tree-vrp.c (revision 254183) +++ gcc/tree-vrp.c (working copy) @@ -3124,29 +3124,29 @@ extract_range_from_binary_expr (value_ra set_value_range (&n_vr0, VR_RANGE, op0, op0, NULL); extract_range_from_binary_expr_1 (vr, code, expr_type, &n_vr0, &vr1); } /* If we didn't derive a range for MINUS_EXPR, and op1's range is ~[op0,op0] or vice-versa, then we can derive a non-null range. This happens often for pointer subtraction. */ if (vr->type == VR_VARYING - && code == MINUS_EXPR + && (code == MINUS_EXPR || code == POINTER_DIFF_EXPR) && TREE_CODE (op0) == SSA_NAME && ((vr0.type == VR_ANTI_RANGE && vr0.min == op1 && vr0.min == vr0.max) || (vr1.type == VR_ANTI_RANGE && vr1.min == op0 && vr1.min == vr1.max))) - set_value_range_to_nonnull (vr, TREE_TYPE (op0)); + set_value_range_to_nonnull (vr, expr_type); } /* Extract range information from a unary operation CODE based on the range of its operand *VR0 with type OP0_TYPE with resulting type TYPE. The resulting range is stored in *VR. */ void extract_range_from_unary_expr (value_range *vr, enum tree_code code, tree type, value_range *vr0_, tree op0_type) Index: gcc/tree.def =================================================================== --- gcc/tree.def (revision 254183) +++ gcc/tree.def (working copy) @@ -669,20 +669,27 @@ DEFTREECODE (PLACEHOLDER_EXPR, "placehol /* Simple arithmetic. */ DEFTREECODE (PLUS_EXPR, "plus_expr", tcc_binary, 2) DEFTREECODE (MINUS_EXPR, "minus_expr", tcc_binary, 2) DEFTREECODE (MULT_EXPR, "mult_expr", tcc_binary, 2) /* Pointer addition. The first operand is always a pointer and the second operand is an integer of type sizetype. */ DEFTREECODE (POINTER_PLUS_EXPR, "pointer_plus_expr", tcc_binary, 2) +/* Pointer subtraction. The two arguments are pointers, and the result + is a signed integer of the same precision. Pointers are interpreted + as unsigned, the difference is computed as if in infinite signed + precision. Behavior is undefined if the difference does not fit in + the result type. */ +DEFTREECODE (POINTER_DIFF_EXPR, "pointer_diff_expr", tcc_binary, 2) + /* Highpart multiplication. For an integral type with precision B, returns bits [2B-1, B] of the full 2*B product. */ DEFTREECODE (MULT_HIGHPART_EXPR, "mult_highpart_expr", tcc_binary, 2) /* Division for integer result that rounds the quotient toward zero. */ DEFTREECODE (TRUNC_DIV_EXPR, "trunc_div_expr", tcc_binary, 2) /* Division for integer result that rounds it toward plus infinity. */ DEFTREECODE (CEIL_DIV_EXPR, "ceil_div_expr", tcc_binary, 2) Index: gcc/varasm.c =================================================================== --- gcc/varasm.c (revision 254183) +++ gcc/varasm.c (working copy) @@ -4601,20 +4601,21 @@ initializer_constant_valid_p_1 (tree val else /* Support narrowing pointer differences. */ ret = narrowing_initializer_constant_valid_p (value, endtype, NULL); if (cache) { cache[0] = value; cache[1] = ret; } return ret; + case POINTER_DIFF_EXPR: case MINUS_EXPR: if (TREE_CODE (endtype) == REAL_TYPE) return NULL_TREE; if (cache && cache[0] == value) return cache[1]; if (! INTEGRAL_TYPE_P (endtype) || TYPE_PRECISION (endtype) >= TYPE_PRECISION (TREE_TYPE (value))) { tree ncache[4] = { NULL_TREE, NULL_TREE, NULL_TREE, NULL_TREE }; tree valid0