https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64715
--- Comment #15 from Richard Biener <rguenth at gcc dot gnu.org> --- (In reply to Jakub Jelinek from comment #14) > Created attachment 35073 [details] > WIP patch > > So, on top of what you've committed, here is my unfinished attempt to > disable for __bos undesirable transformations. On the: > int > main () > { > struct A { char buf1[9]; char buf2[1]; } a; > char *p = a.buf1; > char *q = p + 1; > char *r = q + 4; > char *t = r - 1; > strcpy (t, str1 + 5); > return 0; > } > main of this PRs testcase, the match.pd snippet doesn't work (genmatch > thinks ADDR_EXPR operand must be a SSA_NAME, where that is not valid > gimple), we end up with > q_2 = &a.buf1 + 1; > t_4 = &MEM[(void *)q_2 + 3B]; that's from forwprop which runs into an ordering issue here. IMHO the whole POINTER_PLUS_EXPR handling in that first loop is now "premature". Disabling it yields q_2 = &a.buf1 + 1; r_3 = &a.buf1 + 5; t_4 = &a.buf1 + 4; str1.0_6 = str1; _7 = str1.0_6 + 5; _11 = __builtin_object_size (t_4, 1); Note that I would have expected CCP to already do all this... it's unfortunate that it can't consider '&a.buf1 + 1' a constant, simplifying it along the way. But changing that is too much work at this point. > which would be better expressed as > t_4 = &a.buf1 + 4; > and then FRE folds that into: > t_4 = &MEM[(void *)&a + 4B]; Yep, and it still does that. For the same reason as CCP (make the constant lattice work). Same awkward fix as for CCP: Index: gcc/tree-ssa-sccvn.c =================================================================== --- gcc/tree-ssa-sccvn.c (revision 221624) +++ gcc/tree-ssa-sccvn.c (working copy) @@ -85,6 +85,7 @@ along with GCC; see the file COPYING3. #include "ipa-ref.h" #include "plugin-api.h" #include "cgraph.h" +#include "tree-pass.h" /* This algorithm is based on the SCC algorithm presented by Keith Cooper and L. Taylor Simpson in "SCC-Based Value numbering" @@ -879,7 +880,7 @@ copy_reference_ops_from_ref (tree ref, v trick here). */ && (TREE_CODE (orig) != ADDR_EXPR || off != 0 - || cfun->after_inlining)) + || (cfun->curr_properties & PROP_gimple_ebos))) temp.off = off.to_shwi (); } } @@ -3374,7 +3375,9 @@ simplify_binary_expression (gimple stmt) if (code == POINTER_PLUS_EXPR && tree_fits_uhwi_p (op1) && TREE_CODE (op0) == ADDR_EXPR - && is_gimple_min_invariant (op0)) + && is_gimple_min_invariant (op0) + && (!handled_component_p (TREE_OPERAND (op0, 0)) + || (cfun->curr_properties & PROP_gimple_ebos))) return build_invariant_address (TREE_TYPE (op0), TREE_OPERAND (op0, 0), tree_to_uhwi (op1)); > so if we went this way, we'd need to change genmatch to handle ADDR_EXPRs > specially, and FRE to avoid forwarding into &MEM if that would lose info. I think trying to disable the pointer-plus-expr forwprop is more reasonable. I'll see what we get as fallout from that. So maybe we can "propagate" PROP_gimple_ebos up the cgraph in local-pure-const to at least not pessimize functions that do not have __bos and won't get it either via inlining (unfortunately local_pure_const is only run after early opts, so it isn't really the suitable place to do this - cgraph build maybe). > Or, as discussed on IRC we can consider MEM_REFs where first operand isn't > just > SSA_NAME or ADDR_EXPR of a decl, but allow also ADDR_EXPR of > handled_component_p (with base being a decl or MEM_REF with SSA_NAME first > operand). > > Or add a new tree code, like OFFSETTED_ADDR_EXPR which would be like > ADDR_EXPR, but with integer offset address to the ADDR_EXPR.