[Bug tree-optimization/64715] [5 Regression] __builtin_object_size (..., 1) fails to locate subobject

[rguenth at gcc dot gnu.org]

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.