> > I also disabled type matching done by operand_equal_p and cleaned up the > > conditional of MEM_REF into multiple ones - for example it was passing > > OEP_ADDRESS_OF when comparing TYPE_SIZE which is quite a nonsense. > > > > I wonder what to do about OPE_CONSTANT_ADDRESS_OF. This flag does not seem > > to be used at all in current tree nor documented somehow. > > It is used and (un-)documented as OEP_ADDRESS_OF, see the ADDR_EXPR case: > > case ADDR_EXPR: > return operand_equal_p (TREE_OPERAND (arg0, 0), TREE_OPERAND (arg1, 0), > TREE_CONSTANT (arg0) && TREE_CONSTANT (arg1) > ? OEP_CONSTANT_ADDRESS_OF | OEP_ADDRESS_OF : 0); > > So it's OEP_ADDRESS_OF but for constant addresses.
Yep, I noticed it, but just after reading the sources for few times. The use is well hidden :) Here is updated patch adding Richard's feedback and also making most of OEP_ADDRESS_OF special cases to also handle OEP_CONSTANT_ADDRESS_OF. OEP_CONSTANT_ADDRESS_OF means we care about address and we know the whole expr is constant, so it is stronger than OEP_ADDRESS_OF. I also added documentation and cleaned up handling of ADDR_EXPR. There are two cases handing ADDR_EXPR. One handles TREE_CONSTANT (arg0) && TREE_CONSTANT (arg1) and the other the rest of cases, so we do not need the conditional in the code quoted above. This will hopefully make it more obvious when the OEP_CONSTANT_ADDRESS_OF is set and used. I also added sanity check that OEP_ADDRESSOF|OEP_CONSTANT_ADDRESS_OF is not used for INTEGER_CST and NOP_EXPR. There are many other cases where we can't take address, but this seems strong enough to catch the wrong recursion which forgets to clear the flag and forced me to fix the OEP_CONSTANT_ADDRESS_OF handling. I wonder if the INDIRECT_REF also needs the TBAA check that we do for MEM_REF? While we lower that early, I think we can still unify the code like in case of cond ? ref_alias_set_1 : ref_alias_set_2 Bootstrapped/regtested x86_64-linux, OK? Honza * fold-const.c (operand_equal_p): Document OEP_ADDRESS_OF and OEP_CONSTANT_ADDRESS_OF; make most of OEP_ADDRESS_OF special cases to also handle OEP_CONSTANT_ADDRESS_OF; skip type checking for OPE_*ADDRESS_OF. Index: fold-const.c =================================================================== --- fold-const.c (revision 228131) +++ fold-const.c (working copy) @@ -2691,7 +2691,12 @@ combine_comparisons (location_t loc, If OEP_PURE_SAME is set, then pure functions with identical arguments are considered the same. It is used when the caller has other ways - to ensure that global memory is unchanged in between. */ + to ensure that global memory is unchanged in between. + + If OEP_ADDRESS_OF/OEP_CONSTANT_ADDRESS_OF is set, we are actually comparing + addresses of objects, not values of expressions. OEP_CONSTANT_ADDRESS_OF + is used for ADDR_EXPR with TREE_CONSTANT flag set and we further ignore + any side effects on SAVE_EXPRs then. */ int operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags) @@ -2710,31 +2715,48 @@ operand_equal_p (const_tree arg0, const_ /* Check equality of integer constants before bailing out due to precision differences. */ if (TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) == INTEGER_CST) - return tree_int_cst_equal (arg0, arg1); + { + /* Address of INTEGER_CST is not defined; check that we did not forget + to drop the OEP_ADDRESS_OF/OEP_CONSTANT_ADDRESS_OF flags. */ + gcc_checking_assert (!(flags + & (OEP_ADDRESS_OF | OEP_CONSTANT_ADDRESS_OF))); + return tree_int_cst_equal (arg0, arg1); + } - /* If both types don't have the same signedness, then we can't consider - them equal. We must check this before the STRIP_NOPS calls - because they may change the signedness of the arguments. As pointers - strictly don't have a signedness, require either two pointers or - two non-pointers as well. */ - if (TYPE_UNSIGNED (TREE_TYPE (arg0)) != TYPE_UNSIGNED (TREE_TYPE (arg1)) - || POINTER_TYPE_P (TREE_TYPE (arg0)) != POINTER_TYPE_P (TREE_TYPE (arg1))) - return 0; + if (!(flags & (OEP_ADDRESS_OF | OEP_CONSTANT_ADDRESS_OF))) + { + /* If both types don't have the same signedness, then we can't consider + them equal. We must check this before the STRIP_NOPS calls + because they may change the signedness of the arguments. As pointers + strictly don't have a signedness, require either two pointers or + two non-pointers as well. */ + if (TYPE_UNSIGNED (TREE_TYPE (arg0)) != TYPE_UNSIGNED (TREE_TYPE (arg1)) + || POINTER_TYPE_P (TREE_TYPE (arg0)) + != POINTER_TYPE_P (TREE_TYPE (arg1))) + return 0; - /* We cannot consider pointers to different address space equal. */ - if (POINTER_TYPE_P (TREE_TYPE (arg0)) && POINTER_TYPE_P (TREE_TYPE (arg1)) - && (TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg0))) - != TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg1))))) - return 0; + /* We cannot consider pointers to different address space equal. */ + if (POINTER_TYPE_P (TREE_TYPE (arg0)) + && POINTER_TYPE_P (TREE_TYPE (arg1)) + && (TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg0))) + != TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg1))))) + return 0; - /* If both types don't have the same precision, then it is not safe - to strip NOPs. */ - if (element_precision (TREE_TYPE (arg0)) - != element_precision (TREE_TYPE (arg1))) - return 0; + /* If both types don't have the same precision, then it is not safe + to strip NOPs. */ + if (element_precision (TREE_TYPE (arg0)) + != element_precision (TREE_TYPE (arg1))) + return 0; - STRIP_NOPS (arg0); - STRIP_NOPS (arg1); + STRIP_NOPS (arg0); + STRIP_NOPS (arg1); + } + else + /* Addresses of NOP_EXPR (and many other things) are not well defined. + Check that we did not forget to drop the + OEP_ADDRESS_OF/OEP_CONSTANT_ADDRESS_OF flags. */ + gcc_checking_assert (TREE_CODE (arg0) != NOP_EXPR + && TREE_CODE (arg1) != NOP_EXPR); /* In case both args are comparisons but with different comparison code, try to swap the comparison operands of one arg to produce @@ -2757,7 +2779,7 @@ operand_equal_p (const_tree arg0, const_ /* NOP_EXPR and CONVERT_EXPR are considered equal. */ if (CONVERT_EXPR_P (arg0) && CONVERT_EXPR_P (arg1)) ; - else if (flags & OEP_ADDRESS_OF) + else if (flags & (OEP_ADDRESS_OF | OEP_CONSTANT_ADDRESS_OF)) { /* If we are interested in comparing addresses ignore MEM_REF wrappings of the base that can appear just for @@ -2858,9 +2880,10 @@ operand_equal_p (const_tree arg0, const_ TREE_STRING_LENGTH (arg0))); case ADDR_EXPR: + /* Be sure we pass right ADDRESS_OF flag. */ + gcc_checking_assert (TREE_CONSTANT (arg0) && TREE_CONSTANT (arg1)); return operand_equal_p (TREE_OPERAND (arg0, 0), TREE_OPERAND (arg1, 0), - TREE_CONSTANT (arg0) && TREE_CONSTANT (arg1) - ? OEP_CONSTANT_ADDRESS_OF | OEP_ADDRESS_OF : 0); + OEP_CONSTANT_ADDRESS_OF); default: break; } @@ -2922,7 +2945,7 @@ operand_equal_p (const_tree arg0, const_ switch (TREE_CODE (arg0)) { case INDIRECT_REF: - if (!(flags & OEP_ADDRESS_OF) + if (!(flags & (OEP_ADDRESS_OF | OEP_CONSTANT_ADDRESS_OF)) && (TYPE_ALIGN (TREE_TYPE (arg0)) != TYPE_ALIGN (TREE_TYPE (arg1)))) return 0; @@ -2935,27 +2958,34 @@ operand_equal_p (const_tree arg0, const_ case TARGET_MEM_REF: case MEM_REF: - /* Require equal access sizes, and similar pointer types. - We can have incomplete types for array references of - variable-sized arrays from the Fortran frontend - though. Also verify the types are compatible. */ - if (!((TYPE_SIZE (TREE_TYPE (arg0)) == TYPE_SIZE (TREE_TYPE (arg1)) - || (TYPE_SIZE (TREE_TYPE (arg0)) - && TYPE_SIZE (TREE_TYPE (arg1)) - && operand_equal_p (TYPE_SIZE (TREE_TYPE (arg0)), - TYPE_SIZE (TREE_TYPE (arg1)), flags))) - && types_compatible_p (TREE_TYPE (arg0), TREE_TYPE (arg1)) - && ((flags & OEP_ADDRESS_OF) - || (alias_ptr_types_compatible_p - (TREE_TYPE (TREE_OPERAND (arg0, 1)), - TREE_TYPE (TREE_OPERAND (arg1, 1))) - && (MR_DEPENDENCE_CLIQUE (arg0) - == MR_DEPENDENCE_CLIQUE (arg1)) - && (MR_DEPENDENCE_BASE (arg0) - == MR_DEPENDENCE_BASE (arg1)) - && (TYPE_ALIGN (TREE_TYPE (arg0)) - == TYPE_ALIGN (TREE_TYPE (arg1))))))) - return 0; + if (!(flags & (OEP_ADDRESS_OF | OEP_CONSTANT_ADDRESS_OF))) + { + /* Require equal access sizes */ + if (TYPE_SIZE (TREE_TYPE (arg0)) != TYPE_SIZE (TREE_TYPE (arg1)) + && (!TYPE_SIZE (TREE_TYPE (arg0)) + || !TYPE_SIZE (TREE_TYPE (arg1)) + || !operand_equal_p (TYPE_SIZE (TREE_TYPE (arg0)), + TYPE_SIZE (TREE_TYPE (arg1)), + flags))) + return 0; + /* Verify that access happens in similar types. */ + if (!types_compatible_p (TREE_TYPE (arg0), TREE_TYPE (arg1))) + return 0; + /* Verify that accesses are TBAA compatible. */ + if (flag_strict_aliasing + && (!alias_ptr_types_compatible_p + (TREE_TYPE (TREE_OPERAND (arg0, 1)), + TREE_TYPE (TREE_OPERAND (arg1, 1))) + || (MR_DEPENDENCE_CLIQUE (arg0) + != MR_DEPENDENCE_CLIQUE (arg1)) + || (MR_DEPENDENCE_BASE (arg0) + != MR_DEPENDENCE_BASE (arg1)))) + return 0; + /* Verify that alignment is compatible. */ + if (TYPE_ALIGN (TREE_TYPE (arg0)) + != TYPE_ALIGN (TREE_TYPE (arg1))) + return 0; + } flags &= ~(OEP_CONSTANT_ADDRESS_OF|OEP_ADDRESS_OF); return (OP_SAME (0) && OP_SAME (1) /* TARGET_MEM_REF require equal extra operands. */ @@ -3001,6 +3031,8 @@ operand_equal_p (const_tree arg0, const_ switch (TREE_CODE (arg0)) { case ADDR_EXPR: + /* Be sure we pass right ADDRESS_OF flag. */ + gcc_checking_assert (!(TREE_CONSTANT (arg0) && TREE_CONSTANT (arg1))); return operand_equal_p (TREE_OPERAND (arg0, 0), TREE_OPERAND (arg1, 0), flags | OEP_ADDRESS_OF);