| Message ID | 20190614084950.j4vmohkzuo3ql4aa@kam.mff.cuni.cz |
|---|---|
| State | New |
| Headers | show |
| Series | Remove nonoverlapping_component_refs_of_decl_p | expand |
On Fri, 14 Jun 2019, Jan Hubicka wrote: > Hi, > this patch removes nonoverlapping_component_refs_of_decl_p and replaces > it by nonoverlapping_component_refs_p. Those functions are basically > equivalent, however the first assumes that the outer type of memory access > is type of decl which is not true in the gimple memory model. > > nonoverlapping_component_refs_p may wind up more expensive by doing > quick sort. If the outer types match, we could use the same walk as in > nonoverlapping_component_refs_of_decl_p (and I can easilly implement it > in there saing some qsort for the indirect oracles too) but since > nonoverlapping_component_refs_p already handles common case of shallow > access paths and does not show in profiles I am not convinced it is > worth the code duplication. > > I think the oracle is mostly redudnant with aliasing_component_refs - in fact I > have troubles constructing testcases it would catch that the second would not > except for expliting the -1 corner cases of same_types_for_tbaa, so I decided > to leave this for later. The only case where this function is stronger is the > case of match of types in the middle of access path. This require some > non-const indexed array refs to exploit it. Perhaps we could integrate both > tests and do nonoverlapping_component_refs_p only if one of many handled > components walks we do earlier concludes that it has chance to suceed. > > Point of this patch is however to fix the code duplication and also add > missing checks for view converts - I don't see how it would be safe to use > the access paths here when it is not in the other two oracles. nonoverlapping_component_refs_of_decl_p was added by Eric before I moved nonoverlapping_component_refs_p from RTL to trees. It's nice to see them merged. Still I'd like to see it done in two steps, first making them more equivalent by adding missing checks, best with actually failing testcases (as said, GIMPLE testcases with arbitrary typed/TBAAed accesses are easy to write but even a C testcase should eventually work here). > I implemented assert checking that whenever nonoverlapping_component_refs_of_decl_p > so does nonoverlapping_component_refs_p and found two issues: > 1) the first does not give up seeing handled component that is not > COMPONENT_REF while other does. > This prevents it to disambiguate for example > foo.bar.array[1]; > from > foo.baz.array[1]; > where bar and baz are fields of structure foo. This is valid True. Copied from the RTL routine ;) > 2) It gives up upon seeing bitfield while it may disambiguate later in the > patch like. > foo.bar.bitfield1; > from > foo.baz.bitfield2; Not sure, it should first see bar/baz and disambiguate on that. > > Here we can not tell that bitfield1 is different from bitfield2 (at least > we do not do so currently claiming that it is not different for RTL, but > I think in that case we should not see bitfield in the access path) We do - this was added for actual miscompiles. We could of course make sure to strip components from MEM_EXPRs. This is all about RTL memory accesses being byte-granular while the GIMPLE alias oracle doing bit-granular disambiguations so the check is also on the conservative side. > but we can still disambiguate based on bar/baz. And we already should? Note nonoverlapping_component_refs_of_decl_p is quite cheap compared to nonoverlapping_component_refs_p (which at least is no longer quadratic as it was in the RTL implementation). As you said it has several correctness issues (explicit VIEW_CONVERT_EXPR also comes to my mind here). > Patch changes > Alias oracle query stats: refs_may_alias_p: 3021539 disambiguations, 3321136 queries ref_maybe_used_by_call_p: 7118 disambiguations, 3047133 queries call_may_clobber_ref_p: 817 disambiguations, 817 queries aliasing_component_ref_p: 2050 disambiguations, 19908 queries TBAA oracle: 1419961 disambiguations 2916692 queries 555158 are in alias set 0 575103 queries asked about the same object 0 queries asked about the same alias set 0 access volatile 252874 are dependent in the DAG 113596 are aritificially in conflict with void * > > to > > Alias oracle query stats: > refs_may_alias_p: 3021521 disambiguations, 3321121 queries > ref_maybe_used_by_call_p: 7118 disambiguations, 3047115 queries > call_may_clobber_ref_p: 817 disambiguations, 817 queries > nonoverlapping_component_refs_p: 25 disambiguations, 83528 queries > aliasing_component_refs_p: 2050 disambiguations, 19908 queries > TBAA oracle: 1419961 disambiguations 2916690 queries > 555158 are in alias set 0 > 575103 queries asked about the same object > 0 queries asked about the same alias set > 0 access volatile > 252872 are dependent in the DAG > 113596 are aritificially in conflict with void * > > So at least for tramp3d nonoverlapping_component_refs_p doesn't do any miracles. > There are fewer disambiguations caused by the new view convert verification. > > It however also causes: > > FAIL: gcc.dg/vect/pr66142.c -flto -ffat-lto-objects scan-tree-dump-times vect "vectorized 1 loops in function" 1 > FAIL: gcc.dg/vect/pr66142.c scan-tree-dump-times vect "vectorized 1 loops in function" 1 > > Test does: > > struct A { float x, y; }; > struct B { struct A t, w; }; > > static inline float > bar (const struct B *x) > { > struct A r; > float b, c, d; > r.x = x->t.x; > r.y = x->t.y; > > which gets inlined to > > void > foo (float *a, float *b, float *c) > { > int i; > float z = 0.0f; > float u = *a; > #pragma omp simd > for (i = 0; i < 32; i++) > { > float x = b[i]; > float y = c[i]; > struct B r; > r.t.x = 1.0f; > r.t.y = u; > r.w.x = x; > r.w.y = y; > z += bar (&r); > } > *a = z; > } > > SIMD code turns struct B r into array of size 32. > > The first difference is fre1. Here we give up on the oracle because of: > > @@ -488,15 +505,10 @@ > D.2200[_20].t.y = u_13; > D.2200[_20].w.x = x_22; > D.2200[_20].w.y = y_24; > - _30 = MEM <struct B[32]> [(const struct B *)&D.2200][_20].t.x; > - _34 = MEM <struct B[32]> [(const struct B *)&D.2200][_20].t.y; > - _35 = MEM <struct B[32]> [(const struct B *)&D.2200][_20].w.x; > - _36 = _30 * _35; > - _38 = y_24 * _34; > - b_39 = _36 + _38; > - _40 = _30 * _30; > - _41 = b_39 + _40; > - _42 = _34 * _34; > + _38 = u_13 * y_24; > + b_39 = x_22 + _38; > + _41 = b_39 + 1.0e+0; > + _42 = u_13 * u_13; > > It is truly silly to not optimize this, but there is a type mismatch > between "struct B[32]" and "struct B" which we perceive as a view > convert (same_type_for_tbaa_p return false which is bit ridiculout > because precisely this case could be supported). > (We would miss this if struct B was dynamically allocated previously > too.) I've noted elsewhere that what we consider a VIEW_CONVERT_EXPR should be improved (independently, maybe as a first step), factoring a mem_is_view_converting () check. > However in general we could run into this scenario since the type > mismatch is a result of forwprop1 handling > > D.2200[_20].t.x = 1.0e+0; > D.2200[_20].t.y = u_13; > D.2200[_20].w.x = x_22; > D.2200[_20].w.y = y_24; > _29 = &D.2200[_20]; > _30 = MEM[(const struct B *)_29].t.x; > _34 = MEM[(const struct B *)_29].t.y; > _35 = MEM[(const struct B *)_29].w.x; > _36 = _30 * _35; > _37 = MEM[(const struct B *)_29].w.y; > > So if there was outer struct, then the view convert would indeed happen. No, a forward propagation should never result in a view-convert perceived MEM unless the MEM was already view-converting. But the issue is that the special trick in forwprop that does /* If the RHS is a plain dereference and the value type is the same as that of the pointed-to type of the address we can put the dereferenced address on the RHS preserving the original alias-type. */ perserves the original alias-type. It's been too long that I remember all the details ;) > Any ideas how to solve this? I guess one option would be to delay such lossy > forward subtitutions for one of later forwprops/PRE since for most of SSA > optimizers the earlier sequence should be transparent enough and there > is ahope that one can put constant in. As said our notion what is a view-conversion and what not should probably allow simple component cases. Richard. > Shall I XFAIL the testcase? > > Bootstrapped/regtested x86_64-linux, OK? > > Honza > > > * tree-ssa-alias.c (alias_stats): Add > nonoverlapping_component_refs_p_may_alias and > nonoverlapping_component_refs_p_no_alias. > (dump_alias_stats): Dump it. > (nonoverlapping_component_refs_of_decl_p): Remove. > (nonoverlapping_component_refs_p): Walk all handled components; > do not give up on diambiguation on first failed match. > (decl_refs_may_alias_p): Watch for view converts; > use nonoverlapping_component_refs_of_decl_p. > > Index: tree-ssa-alias.c > =================================================================== > --- tree-ssa-alias.c (revision 272283) > +++ tree-ssa-alias.c (working copy) > @@ -100,6 +100,8 @@ static struct { > unsigned HOST_WIDE_INT call_may_clobber_ref_p_no_alias; > unsigned HOST_WIDE_INT aliasing_component_refs_p_may_alias; > unsigned HOST_WIDE_INT aliasing_component_refs_p_no_alias; > + unsigned HOST_WIDE_INT nonoverlapping_component_refs_p_may_alias; > + unsigned HOST_WIDE_INT nonoverlapping_component_refs_p_no_alias; > } alias_stats; > > void > @@ -124,7 +126,13 @@ dump_alias_stats (FILE *s) > alias_stats.call_may_clobber_ref_p_no_alias, > alias_stats.call_may_clobber_ref_p_no_alias > + alias_stats.call_may_clobber_ref_p_may_alias); > - fprintf (s, " aliasing_component_ref_p: " > + fprintf (s, " nonoverlapping_component_refs_p: " > + HOST_WIDE_INT_PRINT_DEC" disambiguations, " > + HOST_WIDE_INT_PRINT_DEC" queries\n", > + alias_stats.nonoverlapping_component_refs_p_no_alias, > + alias_stats.nonoverlapping_component_refs_p_no_alias > + + alias_stats.nonoverlapping_component_refs_p_may_alias); > + fprintf (s, " aliasing_component_refs_p: " > HOST_WIDE_INT_PRINT_DEC" disambiguations, " > HOST_WIDE_INT_PRINT_DEC" queries\n", > alias_stats.aliasing_component_refs_p_no_alias, > @@ -1029,106 +1037,6 @@ aliasing_component_refs_p (tree ref1, > return false; > } > > -/* Return true if we can determine that component references REF1 and REF2, > - that are within a common DECL, cannot overlap. */ > - > -static bool > -nonoverlapping_component_refs_of_decl_p (tree ref1, tree ref2) > -{ > - auto_vec<tree, 16> component_refs1; > - auto_vec<tree, 16> component_refs2; > - > - /* Create the stack of handled components for REF1. */ > - while (handled_component_p (ref1)) > - { > - component_refs1.safe_push (ref1); > - ref1 = TREE_OPERAND (ref1, 0); > - } > - if (TREE_CODE (ref1) == MEM_REF) > - { > - if (!integer_zerop (TREE_OPERAND (ref1, 1))) > - return false; > - ref1 = TREE_OPERAND (TREE_OPERAND (ref1, 0), 0); > - } > - > - /* Create the stack of handled components for REF2. */ > - while (handled_component_p (ref2)) > - { > - component_refs2.safe_push (ref2); > - ref2 = TREE_OPERAND (ref2, 0); > - } > - if (TREE_CODE (ref2) == MEM_REF) > - { > - if (!integer_zerop (TREE_OPERAND (ref2, 1))) > - return false; > - ref2 = TREE_OPERAND (TREE_OPERAND (ref2, 0), 0); > - } > - > - /* Bases must be either same or uncomparable. */ > - gcc_checking_assert (ref1 == ref2 > - || (DECL_P (ref1) && DECL_P (ref2) > - && compare_base_decls (ref1, ref2) != 0)); > - > - /* Pop the stacks in parallel and examine the COMPONENT_REFs of the same > - rank. This is sufficient because we start from the same DECL and you > - cannot reference several fields at a time with COMPONENT_REFs (unlike > - with ARRAY_RANGE_REFs for arrays) so you always need the same number > - of them to access a sub-component, unless you're in a union, in which > - case the return value will precisely be false. */ > - while (true) > - { > - do > - { > - if (component_refs1.is_empty ()) > - return false; > - ref1 = component_refs1.pop (); > - } > - while (!RECORD_OR_UNION_TYPE_P (TREE_TYPE (TREE_OPERAND (ref1, 0)))); > - > - do > - { > - if (component_refs2.is_empty ()) > - return false; > - ref2 = component_refs2.pop (); > - } > - while (!RECORD_OR_UNION_TYPE_P (TREE_TYPE (TREE_OPERAND (ref2, 0)))); > - > - /* Beware of BIT_FIELD_REF. */ > - if (TREE_CODE (ref1) != COMPONENT_REF > - || TREE_CODE (ref2) != COMPONENT_REF) > - return false; > - > - tree field1 = TREE_OPERAND (ref1, 1); > - tree field2 = TREE_OPERAND (ref2, 1); > - > - /* ??? We cannot simply use the type of operand #0 of the refs here > - as the Fortran compiler smuggles type punning into COMPONENT_REFs > - for common blocks instead of using unions like everyone else. */ > - tree type1 = DECL_CONTEXT (field1); > - tree type2 = DECL_CONTEXT (field2); > - > - /* We cannot disambiguate fields in a union or qualified union. */ > - if (type1 != type2 || TREE_CODE (type1) != RECORD_TYPE) > - return false; > - > - if (field1 != field2) > - { > - /* A field and its representative need to be considered the > - same. */ > - if (DECL_BIT_FIELD_REPRESENTATIVE (field1) == field2 > - || DECL_BIT_FIELD_REPRESENTATIVE (field2) == field1) > - return false; > - /* Different fields of the same record type cannot overlap. > - ??? Bitfields can overlap at RTL level so punt on them. */ > - if (DECL_BIT_FIELD (field1) && DECL_BIT_FIELD (field2)) > - return false; > - return true; > - } > - } > - > - return false; > -} > - > /* qsort compare function to sort FIELD_DECLs after their > DECL_FIELD_CONTEXT TYPE_UID. */ > > @@ -1154,40 +1062,66 @@ nonoverlapping_component_refs_p (const_t > { > if (!flag_strict_aliasing > || !x || !y > - || TREE_CODE (x) != COMPONENT_REF > - || TREE_CODE (y) != COMPONENT_REF) > - return false; > + || !handled_component_p (x) > + || !handled_component_p (y)) > + { > + ++alias_stats.nonoverlapping_component_refs_p_may_alias; > + return false; > + } > > auto_vec<const_tree, 16> fieldsx; > - while (TREE_CODE (x) == COMPONENT_REF) > + while (handled_component_p (x)) > { > - tree field = TREE_OPERAND (x, 1); > - tree type = DECL_FIELD_CONTEXT (field); > - if (TREE_CODE (type) == RECORD_TYPE) > - fieldsx.safe_push (field); > + if (TREE_CODE (x) == COMPONENT_REF) > + { > + tree field = TREE_OPERAND (x, 1); > + tree type = DECL_FIELD_CONTEXT (field); > + if (TREE_CODE (type) == RECORD_TYPE) > + fieldsx.safe_push (field); > + } > x = TREE_OPERAND (x, 0); > } > if (fieldsx.length () == 0) > - return false; > + { > + ++alias_stats.nonoverlapping_component_refs_p_may_alias; > + return false; > + } > auto_vec<const_tree, 16> fieldsy; > - while (TREE_CODE (y) == COMPONENT_REF) > + while (handled_component_p (y)) > { > - tree field = TREE_OPERAND (y, 1); > - tree type = DECL_FIELD_CONTEXT (field); > - if (TREE_CODE (type) == RECORD_TYPE) > - fieldsy.safe_push (TREE_OPERAND (y, 1)); > + if (TREE_CODE (y) == COMPONENT_REF) > + { > + tree field = TREE_OPERAND (y, 1); > + tree type = DECL_FIELD_CONTEXT (field); > + if (TREE_CODE (type) == RECORD_TYPE) > + fieldsy.safe_push (TREE_OPERAND (y, 1)); > + } > y = TREE_OPERAND (y, 0); > } > if (fieldsy.length () == 0) > - return false; > + { > + ++alias_stats.nonoverlapping_component_refs_p_may_alias; > + return false; > + } > > /* Most common case first. */ > if (fieldsx.length () == 1 > && fieldsy.length () == 1) > - return ((DECL_FIELD_CONTEXT (fieldsx[0]) > - == DECL_FIELD_CONTEXT (fieldsy[0])) > - && fieldsx[0] != fieldsy[0] > - && !(DECL_BIT_FIELD (fieldsx[0]) && DECL_BIT_FIELD (fieldsy[0]))); > + { > + if ((DECL_FIELD_CONTEXT (fieldsx[0]) > + == DECL_FIELD_CONTEXT (fieldsy[0])) > + && fieldsx[0] != fieldsy[0] > + && !(DECL_BIT_FIELD (fieldsx[0]) && DECL_BIT_FIELD (fieldsy[0]))) > + { > + ++alias_stats.nonoverlapping_component_refs_p_no_alias; > + return true; > + } > + else > + { > + ++alias_stats.nonoverlapping_component_refs_p_may_alias; > + return false; > + } > + } > > if (fieldsx.length () == 2) > { > @@ -1215,19 +1149,26 @@ nonoverlapping_component_refs_p (const_t > if (typex == typey) > { > /* We're left with accessing different fields of a structure, > - no possible overlap. */ > + no possible overlap. > + If we can not disambiguate, do not give up and try to continue: > + it is possible that there are multiple matching types on the > + access patch and each of them may result in disambiguation. */ > if (fieldx != fieldy) > { > /* A field and its representative need to be considered the > same. */ > if (DECL_BIT_FIELD_REPRESENTATIVE (fieldx) == fieldy > || DECL_BIT_FIELD_REPRESENTATIVE (fieldy) == fieldx) > - return false; > + ; > /* Different fields of the same record type cannot overlap. > ??? Bitfields can overlap at RTL level so punt on them. */ > - if (DECL_BIT_FIELD (fieldx) && DECL_BIT_FIELD (fieldy)) > - return false; > - return true; > + else if (DECL_BIT_FIELD (fieldx) && DECL_BIT_FIELD (fieldy)) > + ; > + else > + { > + ++alias_stats.nonoverlapping_component_refs_p_no_alias; > + return true; > + } > } > } > if (TYPE_UID (typex) < TYPE_UID (typey)) > @@ -1245,10 +1186,11 @@ nonoverlapping_component_refs_p (const_t > } > while (1); > > + ++alias_stats.nonoverlapping_component_refs_p_may_alias; > return false; > } > > > /* Return true if two memory references based on the variables BASE1 > and BASE2 constrained to [OFFSET1, OFFSET1 + MAX_SIZE1) and > [OFFSET2, OFFSET2 + MAX_SIZE2) may alias. REF1 and REF2 > @@ -1271,11 +1212,41 @@ decl_refs_may_alias_p (tree ref1, tree b > if (!ranges_maybe_overlap_p (offset1, max_size1, offset2, max_size2)) > return false; > > + /* Access path oracle below needs to know both references. */ > + if (!ref1 || !ref2) > + return true; > + > + /* If the decl is accessed via a MEM_REF, reconstruct the base > + we can use for TBAA. */ > + tree dbase1 = ref1; > + > + while (handled_component_p (dbase1)) > + dbase1 = TREE_OPERAND (dbase1, 0); > + if (TREE_CODE (dbase1) == MEM_REF > + || TREE_CODE (dbase1) == TARGET_MEM_REF) > + { > + tree ptrtype1 = TREE_TYPE (TREE_OPERAND (dbase1, 1)); > + /* If first reference is view-converted, give up now. */ > + if (same_type_for_tbaa (TREE_TYPE (dbase1), TREE_TYPE (ptrtype1)) != 1) > + return true; > + } > + > + tree dbase2 = ref2; > + > + while (handled_component_p (dbase2)) > + dbase2 = TREE_OPERAND (dbase2, 0); > + if (TREE_CODE (dbase2) == MEM_REF > + || TREE_CODE (dbase2) == TARGET_MEM_REF) > + { > + tree ptrtype2 = TREE_TYPE (TREE_OPERAND (dbase2, 1)); > + /* If second reference is view-converted, give up now. */ > + if (same_type_for_tbaa (TREE_TYPE (dbase2), TREE_TYPE (ptrtype2)) != 1) > + return true; > + } > + > /* For components with variable position, the above test isn't sufficient, > so we disambiguate component references manually. */ > - if (ref1 && ref2 > - && handled_component_p (ref1) && handled_component_p (ref2) > - && nonoverlapping_component_refs_of_decl_p (ref1, ref2)) > + if (nonoverlapping_component_refs_p (ref1, ref2)) > return false; > > return true; >
> nonoverlapping_component_refs_of_decl_p was added by Eric before > I moved nonoverlapping_component_refs_p from RTL to trees. It's > nice to see them merged. > > Still I'd like to see it done in two steps, first making them > more equivalent by adding missing checks, best with actually > failing testcases (as said, GIMPLE testcases with arbitrary > typed/TBAAed accesses are easy to write but even a C testcase > should eventually work here). This is good idea given that the extra view convert checks hit the problem. I am going to re-test the part of patch which adds strenghtens nonoverlapping_component_refs_p. With testcases I have problem that those I can write are also handled by access path oracle. I will try to dig into this more :) > > > I implemented assert checking that whenever nonoverlapping_component_refs_of_decl_p > > so does nonoverlapping_component_refs_p and found two issues: > > 1) the first does not give up seeing handled component that is not > > COMPONENT_REF while other does. > > This prevents it to disambiguate for example > > foo.bar.array[1]; > > from > > foo.baz.array[1]; > > where bar and baz are fields of structure foo. This is valid > > True. Copied from the RTL routine ;) > > > 2) It gives up upon seeing bitfield while it may disambiguate later in the > > patch like. > > foo.bar.bitfield1; > > from > > foo.baz.bitfield2; > > Not sure, it should first see bar/baz and disambiguate on that. It qsort according to the TYPE_UIDs. Are those guaranteed to be in right order? > > > > > Here we can not tell that bitfield1 is different from bitfield2 (at least > > we do not do so currently claiming that it is not different for RTL, but > > I think in that case we should not see bitfield in the access path) > > We do - this was added for actual miscompiles. We could of course make > sure to strip components from MEM_EXPRs. This is all about RTL > memory accesses being byte-granular while the GIMPLE alias oracle > doing bit-granular disambiguations so the check is also on the > conservative side. > > > but we can still disambiguate based on bar/baz. > > And we already should? > > Note nonoverlapping_component_refs_of_decl_p is quite cheap > compared to nonoverlapping_component_refs_p (which at least is > no longer quadratic as it was in the RTL implementation). > As you said it has several correctness issues (explicit > VIEW_CONVERT_EXPR also comes to my mind here). Yes, we could recover the cost by making nonoverlapping_component_refs_p to do the parallel walk in case outer types are the same as I mentioned. I can impement that. > > However in general we could run into this scenario since the type > > mismatch is a result of forwprop1 handling > > > > D.2200[_20].t.x = 1.0e+0; > > D.2200[_20].t.y = u_13; > > D.2200[_20].w.x = x_22; > > D.2200[_20].w.y = y_24; > > _29 = &D.2200[_20]; > > _30 = MEM[(const struct B *)_29].t.x; > > _34 = MEM[(const struct B *)_29].t.y; > > _35 = MEM[(const struct B *)_29].w.x; > > _36 = _30 * _35; > > _37 = MEM[(const struct B *)_29].w.y; > > > > So if there was outer struct, then the view convert would indeed happen. > > No, a forward propagation should never result in a view-convert > perceived MEM unless the MEM was already view-converting. But the > issue is that the special trick in forwprop that does > > /* If the RHS is a plain dereference and the value type is the same as > that of the pointed-to type of the address we can put the > dereferenced address on the RHS preserving the original alias-type. */ > > perserves the original alias-type. It's been too long that I remember > all the details ;) I think I understand the code. Normally if you have ptr = &foo.bar; ptr->baz; and we fold it together, we produce MEM_REF which is based on foo but offets to bar and from that access path starts. This is done by the code in forwardprop just preceeding this. If ptr = &foo.array[i]; the code fails and we end up constructing MEM_REF with the full access path that in gimple memory model does not garantee that foo is actually of type of foo and thus the ptrtype of memory access is just *ptr. This makes alias oracles to give up. Either we can delay this folding or we can invent way to represent folded memory reference preserving the point of access path from which it is reliable. > > > Any ideas how to solve this? I guess one option would be to delay such lossy > > forward subtitutions for one of later forwprops/PRE since for most of SSA > > optimizers the earlier sequence should be transparent enough and there > > is ahope that one can put constant in. > > As said our notion what is a view-conversion and what not should > probably allow simple component cases. Can you be bit more specific? :) I will break up the patch and start with adding the statistics to both variants and not giving up on ARRAY_REFs. Will look into remaining issues incrementally. Honza
On Fri, 14 Jun 2019, Jan Hubicka wrote: > > nonoverlapping_component_refs_of_decl_p was added by Eric before > > I moved nonoverlapping_component_refs_p from RTL to trees. It's > > nice to see them merged. > > > > Still I'd like to see it done in two steps, first making them > > more equivalent by adding missing checks, best with actually > > failing testcases (as said, GIMPLE testcases with arbitrary > > typed/TBAAed accesses are easy to write but even a C testcase > > should eventually work here). > > This is good idea given that the extra view convert checks hit the > problem. I am going to re-test the part of patch which adds strenghtens > nonoverlapping_component_refs_p. With testcases I have problem that > those I can write are also handled by access path oracle. I will try to > dig into this more :) > > > > > I implemented assert checking that whenever nonoverlapping_component_refs_of_decl_p > > > so does nonoverlapping_component_refs_p and found two issues: > > > 1) the first does not give up seeing handled component that is not > > > COMPONENT_REF while other does. > > > This prevents it to disambiguate for example > > > foo.bar.array[1]; > > > from > > > foo.baz.array[1]; > > > where bar and baz are fields of structure foo. This is valid > > > > True. Copied from the RTL routine ;) > > > > > 2) It gives up upon seeing bitfield while it may disambiguate later in the > > > patch like. > > > foo.bar.bitfield1; > > > from > > > foo.baz.bitfield2; > > > > Not sure, it should first see bar/baz and disambiguate on that. > > It qsort according to the TYPE_UIDs. Are those guaranteed to be in right > order? Ah, no, of course not. I guess the early out here should be a "ignore this match" instead. > > > > > > > > Here we can not tell that bitfield1 is different from bitfield2 (at least > > > we do not do so currently claiming that it is not different for RTL, but > > > I think in that case we should not see bitfield in the access path) > > > > We do - this was added for actual miscompiles. We could of course make > > sure to strip components from MEM_EXPRs. This is all about RTL > > memory accesses being byte-granular while the GIMPLE alias oracle > > doing bit-granular disambiguations so the check is also on the > > conservative side. > > > > > but we can still disambiguate based on bar/baz. > > > > And we already should? > > > > Note nonoverlapping_component_refs_of_decl_p is quite cheap > > compared to nonoverlapping_component_refs_p (which at least is > > no longer quadratic as it was in the RTL implementation). > > As you said it has several correctness issues (explicit > > VIEW_CONVERT_EXPR also comes to my mind here). > > Yes, we could recover the cost by making nonoverlapping_component_refs_p > to do the parallel walk in case outer types are the same as I mentioned. > I can impement that. Not sure if worth the effort - add some statistics detecting this case vs. the others. Limiting the number of components to track would be better I guess, we size the vec<>s to 16 components so simply give up after pushing too many? Again, statistics might be interesting. But as you say I also never saw this high in the profile... > > > However in general we could run into this scenario since the type > > > mismatch is a result of forwprop1 handling > > > > > > D.2200[_20].t.x = 1.0e+0; > > > D.2200[_20].t.y = u_13; > > > D.2200[_20].w.x = x_22; > > > D.2200[_20].w.y = y_24; > > > _29 = &D.2200[_20]; > > > _30 = MEM[(const struct B *)_29].t.x; > > > _34 = MEM[(const struct B *)_29].t.y; > > > _35 = MEM[(const struct B *)_29].w.x; > > > _36 = _30 * _35; > > > _37 = MEM[(const struct B *)_29].w.y; > > > > > > So if there was outer struct, then the view convert would indeed happen. > > > > No, a forward propagation should never result in a view-convert > > perceived MEM unless the MEM was already view-converting. But the > > issue is that the special trick in forwprop that does > > > > /* If the RHS is a plain dereference and the value type is the same as > > that of the pointed-to type of the address we can put the > > dereferenced address on the RHS preserving the original alias-type. */ > > > > perserves the original alias-type. It's been too long that I remember > > all the details ;) > > I think I understand the code. Normally if you have > > ptr = &foo.bar; > ptr->baz; > and we fold it together, we produce MEM_REF which is based on foo but > offets to bar and from that access path starts. > This is done by the code in forwardprop just preceeding this. > > If > ptr = &foo.array[i]; > the code fails and we end up constructing MEM_REF with the full access > path that in gimple memory model does not garantee that foo is actually > of type of foo and thus the ptrtype of memory access is just *ptr. > > This makes alias oracles to give up. Yes. But it also helps removing TREE_ADDRESSABLE and thus makes the alias oracle never consider a pointer-based ref alias this one... > Either we can delay this folding or we can invent way to represent > folded memory reference preserving the point of access path from which > it is reliable. The point is that the access was ptr->baz and foo.array[i] is just address-compute (thus not part of the path). I've already suggested you can walk from the base looking for TBAA type to value type match and likely that's the correct start for access-path based analysis. > > > > > Any ideas how to solve this? I guess one option would be to delay such lossy > > > forward subtitutions for one of later forwprops/PRE since for most of SSA > > > optimizers the earlier sequence should be transparent enough and there > > > is ahope that one can put constant in. > > > > As said our notion what is a view-conversion and what not should > > probably allow simple component cases. > > Can you be bit more specific? :) I think a 'view-conversion' from component (TBAA type) to vector/array/complex of component (value type) is not a real 'view-conversion' and we can just treat it as fine for access-path based analysis. The only think breaking access-path based analyses is view-conversions to (different) structure types? That is, a first step would be to break out same_type_for_tbaa (TREE_TYPE (base1), TREE_TYPE (ptrtype1)) != 1 into a may_be_view_conversion_p (TREE_TYPE (base1), TREE_TYPE (ptrtype1)) predicate which we then can amend to strip ARRAY/VECTOR_TYPEs? IIRC one of your patches did this but to same_type_for_tbaa but I think it only applies to the case where we look whether we can trust an access path. Of course we need to avoid disambiguating MEM<int>[(int *)&d] with MEM<int[]>[(int *)&d][i] but I think we're fine because of that other handling you don't like ;) > I will break up the patch and start with adding the statistics to both variants > and not giving up on ARRAY_REFs. > Will look into remaining issues incrementally. > > Honza >
Hi,
I am heading to lunch, thanks for all the reviews!
This is the cut down version of patch I am testing and will be back in
about an hour.
Honza
* tree-ssa-alias.c (alias_stats): Add
nonoverlapping_component_refs_p_may_alias,
nonoverlapping_component_refs_p_no_alias,
nonoverlapping_component_refs_of_decl_p_may_alias,
nonoverlapping_component_refs_of_decl_p_no_alias.
(dump_alias_stats): Dump them.
(nonoverlapping_component_refs_of_decl_p): Add stats.
(nonoverlapping_component_refs_p): Add stats; do not stop on first
ARRAY_REF.
Index: tree-ssa-alias.c
===================================================================
--- tree-ssa-alias.c (revision 272283)
+++ tree-ssa-alias.c (working copy)
@@ -100,6 +100,10 @@ static struct {
unsigned HOST_WIDE_INT call_may_clobber_ref_p_no_alias;
unsigned HOST_WIDE_INT aliasing_component_refs_p_may_alias;
unsigned HOST_WIDE_INT aliasing_component_refs_p_no_alias;
+ unsigned HOST_WIDE_INT nonoverlapping_component_refs_p_may_alias;
+ unsigned HOST_WIDE_INT nonoverlapping_component_refs_p_no_alias;
+ unsigned HOST_WIDE_INT nonoverlapping_component_refs_of_decl_p_may_alias;
+ unsigned HOST_WIDE_INT nonoverlapping_component_refs_of_decl_p_no_alias;
} alias_stats;
void
@@ -124,7 +128,19 @@ dump_alias_stats (FILE *s)
alias_stats.call_may_clobber_ref_p_no_alias,
alias_stats.call_may_clobber_ref_p_no_alias
+ alias_stats.call_may_clobber_ref_p_may_alias);
- fprintf (s, " aliasing_component_ref_p: "
+ fprintf (s, " nonoverlapping_component_refs_p: "
+ HOST_WIDE_INT_PRINT_DEC" disambiguations, "
+ HOST_WIDE_INT_PRINT_DEC" queries\n",
+ alias_stats.nonoverlapping_component_refs_p_no_alias,
+ alias_stats.nonoverlapping_component_refs_p_no_alias
+ + alias_stats.nonoverlapping_component_refs_p_may_alias);
+ fprintf (s, " nonoverlapping_component_refs_of_decl_p: "
+ HOST_WIDE_INT_PRINT_DEC" disambiguations, "
+ HOST_WIDE_INT_PRINT_DEC" queries\n",
+ alias_stats.nonoverlapping_component_refs_of_decl_p_no_alias,
+ alias_stats.nonoverlapping_component_refs_of_decl_p_no_alias
+ + alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias);
+ fprintf (s, " aliasing_component_refs_p: "
HOST_WIDE_INT_PRINT_DEC" disambiguations, "
HOST_WIDE_INT_PRINT_DEC" queries\n",
alias_stats.aliasing_component_refs_p_no_alias,
@@ -1047,7 +1063,10 @@ nonoverlapping_component_refs_of_decl_p
if (TREE_CODE (ref1) == MEM_REF)
{
if (!integer_zerop (TREE_OPERAND (ref1, 1)))
- return false;
+ {
+ ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias;
+ return false;
+ }
ref1 = TREE_OPERAND (TREE_OPERAND (ref1, 0), 0);
}
@@ -1060,7 +1079,10 @@ nonoverlapping_component_refs_of_decl_p
if (TREE_CODE (ref2) == MEM_REF)
{
if (!integer_zerop (TREE_OPERAND (ref2, 1)))
- return false;
+ {
+ ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias;
+ return false;
+ }
ref2 = TREE_OPERAND (TREE_OPERAND (ref2, 0), 0);
}
@@ -1080,7 +1102,10 @@ nonoverlapping_component_refs_of_decl_p
do
{
if (component_refs1.is_empty ())
- return false;
+ {
+ ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias;
+ return false;
+ }
ref1 = component_refs1.pop ();
}
while (!RECORD_OR_UNION_TYPE_P (TREE_TYPE (TREE_OPERAND (ref1, 0))));
@@ -1088,7 +1113,10 @@ nonoverlapping_component_refs_of_decl_p
do
{
if (component_refs2.is_empty ())
- return false;
+ {
+ ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias;
+ return false;
+ }
ref2 = component_refs2.pop ();
}
while (!RECORD_OR_UNION_TYPE_P (TREE_TYPE (TREE_OPERAND (ref2, 0))));
@@ -1096,7 +1124,10 @@ nonoverlapping_component_refs_of_decl_p
/* Beware of BIT_FIELD_REF. */
if (TREE_CODE (ref1) != COMPONENT_REF
|| TREE_CODE (ref2) != COMPONENT_REF)
- return false;
+ {
+ ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias;
+ return false;
+ }
tree field1 = TREE_OPERAND (ref1, 1);
tree field2 = TREE_OPERAND (ref2, 1);
@@ -1109,7 +1140,10 @@ nonoverlapping_component_refs_of_decl_p
/* We cannot disambiguate fields in a union or qualified union. */
if (type1 != type2 || TREE_CODE (type1) != RECORD_TYPE)
- return false;
+ {
+ ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias;
+ return false;
+ }
if (field1 != field2)
{
@@ -1117,15 +1151,23 @@ nonoverlapping_component_refs_of_decl_p
same. */
if (DECL_BIT_FIELD_REPRESENTATIVE (field1) == field2
|| DECL_BIT_FIELD_REPRESENTATIVE (field2) == field1)
- return false;
+ {
+ ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias;
+ return false;
+ }
/* Different fields of the same record type cannot overlap.
??? Bitfields can overlap at RTL level so punt on them. */
if (DECL_BIT_FIELD (field1) && DECL_BIT_FIELD (field2))
- return false;
+ {
+ ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias;
+ return false;
+ }
+ ++alias_stats.nonoverlapping_component_refs_of_decl_p_no_alias;
return true;
}
}
+ ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias;
return false;
}
@@ -1154,17 +1196,23 @@ nonoverlapping_component_refs_p (const_t
{
if (!flag_strict_aliasing
|| !x || !y
- || TREE_CODE (x) != COMPONENT_REF
- || TREE_CODE (y) != COMPONENT_REF)
- return false;
+ || !handled_component_p (x)
+ || !handled_component_p (y))
+ {
+ ++alias_stats.nonoverlapping_component_refs_p_may_alias;
+ return false;
+ }
auto_vec<const_tree, 16> fieldsx;
- while (TREE_CODE (x) == COMPONENT_REF)
+ while (handled_component_p (x))
{
- tree field = TREE_OPERAND (x, 1);
- tree type = DECL_FIELD_CONTEXT (field);
- if (TREE_CODE (type) == RECORD_TYPE)
- fieldsx.safe_push (field);
+ if (TREE_CODE (x) == COMPONENT_REF)
+ {
+ tree field = TREE_OPERAND (x, 1);
+ tree type = DECL_FIELD_CONTEXT (field);
+ if (TREE_CODE (type) == RECORD_TYPE)
+ fieldsx.safe_push (field);
+ }
x = TREE_OPERAND (x, 0);
}
if (fieldsx.length () == 0)
@@ -1172,22 +1220,39 @@ nonoverlapping_component_refs_p (const_t
auto_vec<const_tree, 16> fieldsy;
while (TREE_CODE (y) == COMPONENT_REF)
{
- tree field = TREE_OPERAND (y, 1);
- tree type = DECL_FIELD_CONTEXT (field);
- if (TREE_CODE (type) == RECORD_TYPE)
- fieldsy.safe_push (TREE_OPERAND (y, 1));
+ if (TREE_CODE (y) == COMPONENT_REF)
+ {
+ tree field = TREE_OPERAND (y, 1);
+ tree type = DECL_FIELD_CONTEXT (field);
+ if (TREE_CODE (type) == RECORD_TYPE)
+ fieldsy.safe_push (TREE_OPERAND (y, 1));
+ }
y = TREE_OPERAND (y, 0);
}
if (fieldsy.length () == 0)
- return false;
+ {
+ ++alias_stats.nonoverlapping_component_refs_p_may_alias;
+ return false;
+ }
/* Most common case first. */
if (fieldsx.length () == 1
&& fieldsy.length () == 1)
- return ((DECL_FIELD_CONTEXT (fieldsx[0])
- == DECL_FIELD_CONTEXT (fieldsy[0]))
- && fieldsx[0] != fieldsy[0]
- && !(DECL_BIT_FIELD (fieldsx[0]) && DECL_BIT_FIELD (fieldsy[0])));
+ {
+ if ((DECL_FIELD_CONTEXT (fieldsx[0])
+ == DECL_FIELD_CONTEXT (fieldsy[0]))
+ && fieldsx[0] != fieldsy[0]
+ && !(DECL_BIT_FIELD (fieldsx[0]) && DECL_BIT_FIELD (fieldsy[0])))
+ {
+ ++alias_stats.nonoverlapping_component_refs_p_no_alias;
+ return true;
+ }
+ else
+ {
+ ++alias_stats.nonoverlapping_component_refs_p_may_alias;
+ return false;
+ }
+ }
if (fieldsx.length () == 2)
{
@@ -1222,11 +1287,18 @@ nonoverlapping_component_refs_p (const_t
same. */
if (DECL_BIT_FIELD_REPRESENTATIVE (fieldx) == fieldy
|| DECL_BIT_FIELD_REPRESENTATIVE (fieldy) == fieldx)
- return false;
+ {
+ ++alias_stats.nonoverlapping_component_refs_p_may_alias;
+ return false;
+ }
/* Different fields of the same record type cannot overlap.
??? Bitfields can overlap at RTL level so punt on them. */
if (DECL_BIT_FIELD (fieldx) && DECL_BIT_FIELD (fieldy))
- return false;
+ {
+ ++alias_stats.nonoverlapping_component_refs_p_may_alias;
+ return false;
+ }
+ ++alias_stats.nonoverlapping_component_refs_p_no_alias;
return true;
}
}
@@ -1245,6 +1317,7 @@ nonoverlapping_component_refs_p (const_t
}
while (1);
+ ++alias_stats.nonoverlapping_component_refs_p_may_alias;
return false;
}
On Fri, 14 Jun 2019, Jan Hubicka wrote: > Hi, > I am heading to lunch, thanks for all the reviews! > This is the cut down version of patch I am testing and will be back in > about an hour. > > Honza > > * tree-ssa-alias.c (alias_stats): Add > nonoverlapping_component_refs_p_may_alias, > nonoverlapping_component_refs_p_no_alias, > nonoverlapping_component_refs_of_decl_p_may_alias, > nonoverlapping_component_refs_of_decl_p_no_alias. > (dump_alias_stats): Dump them. > (nonoverlapping_component_refs_of_decl_p): Add stats. > (nonoverlapping_component_refs_p): Add stats; do not stop on first > ARRAY_REF. > Index: tree-ssa-alias.c > =================================================================== > --- tree-ssa-alias.c (revision 272283) > +++ tree-ssa-alias.c (working copy) > @@ -100,6 +100,10 @@ static struct { > unsigned HOST_WIDE_INT call_may_clobber_ref_p_no_alias; > unsigned HOST_WIDE_INT aliasing_component_refs_p_may_alias; > unsigned HOST_WIDE_INT aliasing_component_refs_p_no_alias; > + unsigned HOST_WIDE_INT nonoverlapping_component_refs_p_may_alias; > + unsigned HOST_WIDE_INT nonoverlapping_component_refs_p_no_alias; > + unsigned HOST_WIDE_INT nonoverlapping_component_refs_of_decl_p_may_alias; > + unsigned HOST_WIDE_INT nonoverlapping_component_refs_of_decl_p_no_alias; > } alias_stats; > > void > @@ -124,7 +128,19 @@ dump_alias_stats (FILE *s) > alias_stats.call_may_clobber_ref_p_no_alias, > alias_stats.call_may_clobber_ref_p_no_alias > + alias_stats.call_may_clobber_ref_p_may_alias); > - fprintf (s, " aliasing_component_ref_p: " > + fprintf (s, " nonoverlapping_component_refs_p: " > + HOST_WIDE_INT_PRINT_DEC" disambiguations, " > + HOST_WIDE_INT_PRINT_DEC" queries\n", > + alias_stats.nonoverlapping_component_refs_p_no_alias, > + alias_stats.nonoverlapping_component_refs_p_no_alias > + + alias_stats.nonoverlapping_component_refs_p_may_alias); > + fprintf (s, " nonoverlapping_component_refs_of_decl_p: " > + HOST_WIDE_INT_PRINT_DEC" disambiguations, " > + HOST_WIDE_INT_PRINT_DEC" queries\n", > + alias_stats.nonoverlapping_component_refs_of_decl_p_no_alias, > + alias_stats.nonoverlapping_component_refs_of_decl_p_no_alias > + + alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias); > + fprintf (s, " aliasing_component_refs_p: " > HOST_WIDE_INT_PRINT_DEC" disambiguations, " > HOST_WIDE_INT_PRINT_DEC" queries\n", > alias_stats.aliasing_component_refs_p_no_alias, > @@ -1047,7 +1063,10 @@ nonoverlapping_component_refs_of_decl_p > if (TREE_CODE (ref1) == MEM_REF) > { > if (!integer_zerop (TREE_OPERAND (ref1, 1))) > - return false; > + { > + ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias; > + return false; > + } > ref1 = TREE_OPERAND (TREE_OPERAND (ref1, 0), 0); > } > > @@ -1060,7 +1079,10 @@ nonoverlapping_component_refs_of_decl_p > if (TREE_CODE (ref2) == MEM_REF) > { > if (!integer_zerop (TREE_OPERAND (ref2, 1))) > - return false; > + { > + ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias; > + return false; > + } > ref2 = TREE_OPERAND (TREE_OPERAND (ref2, 0), 0); > } > > @@ -1080,7 +1102,10 @@ nonoverlapping_component_refs_of_decl_p > do > { > if (component_refs1.is_empty ()) > - return false; > + { > + ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias; > + return false; > + } > ref1 = component_refs1.pop (); > } > while (!RECORD_OR_UNION_TYPE_P (TREE_TYPE (TREE_OPERAND (ref1, 0)))); > @@ -1088,7 +1113,10 @@ nonoverlapping_component_refs_of_decl_p > do > { > if (component_refs2.is_empty ()) > - return false; > + { > + ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias; > + return false; > + } > ref2 = component_refs2.pop (); > } > while (!RECORD_OR_UNION_TYPE_P (TREE_TYPE (TREE_OPERAND (ref2, 0)))); > @@ -1096,7 +1124,10 @@ nonoverlapping_component_refs_of_decl_p > /* Beware of BIT_FIELD_REF. */ > if (TREE_CODE (ref1) != COMPONENT_REF > || TREE_CODE (ref2) != COMPONENT_REF) > - return false; > + { > + ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias; > + return false; > + } > > tree field1 = TREE_OPERAND (ref1, 1); > tree field2 = TREE_OPERAND (ref2, 1); > @@ -1109,7 +1140,10 @@ nonoverlapping_component_refs_of_decl_p > > /* We cannot disambiguate fields in a union or qualified union. */ > if (type1 != type2 || TREE_CODE (type1) != RECORD_TYPE) > - return false; > + { > + ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias; > + return false; > + } > > if (field1 != field2) > { > @@ -1117,15 +1151,23 @@ nonoverlapping_component_refs_of_decl_p > same. */ > if (DECL_BIT_FIELD_REPRESENTATIVE (field1) == field2 > || DECL_BIT_FIELD_REPRESENTATIVE (field2) == field1) > - return false; > + { > + ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias; > + return false; > + } > /* Different fields of the same record type cannot overlap. > ??? Bitfields can overlap at RTL level so punt on them. */ > if (DECL_BIT_FIELD (field1) && DECL_BIT_FIELD (field2)) > - return false; > + { > + ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias; > + return false; > + } > + ++alias_stats.nonoverlapping_component_refs_of_decl_p_no_alias; > return true; > } > } > > + ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias; > return false; > } > > @@ -1154,17 +1196,23 @@ nonoverlapping_component_refs_p (const_t > { > if (!flag_strict_aliasing > || !x || !y > - || TREE_CODE (x) != COMPONENT_REF > - || TREE_CODE (y) != COMPONENT_REF) > - return false; > + || !handled_component_p (x) > + || !handled_component_p (y)) > + { > + ++alias_stats.nonoverlapping_component_refs_p_may_alias; > + return false; > + } > > auto_vec<const_tree, 16> fieldsx; > - while (TREE_CODE (x) == COMPONENT_REF) > + while (handled_component_p (x)) > { > - tree field = TREE_OPERAND (x, 1); > - tree type = DECL_FIELD_CONTEXT (field); > - if (TREE_CODE (type) == RECORD_TYPE) > - fieldsx.safe_push (field); > + if (TREE_CODE (x) == COMPONENT_REF) > + { > + tree field = TREE_OPERAND (x, 1); > + tree type = DECL_FIELD_CONTEXT (field); > + if (TREE_CODE (type) == RECORD_TYPE) > + fieldsx.safe_push (field); > + } I think this is still error-prone since we look past VIEW_CONVERT_EXPRs in the path so we happily disambiguate, say, struct X { int e; int d; }; struct Y { int d; int e; }; VIEW_CONVERT<X>(p->b).d VIEW_CONVERT<Y>(q->b).e where in reality only the access paths from the base up to the first view-conversion are relevant for path-based analysis. So upon seeing a VIEW_CONVERT (or BIT_FIELD_REF which has the same issue) simply truncate the vector. As said, it's a pre-existing issue but you are extending things to possibly handle more stuff... Otherwise looks good. > x = TREE_OPERAND (x, 0); > } > if (fieldsx.length () == 0) > @@ -1172,22 +1220,39 @@ nonoverlapping_component_refs_p (const_t > auto_vec<const_tree, 16> fieldsy; > while (TREE_CODE (y) == COMPONENT_REF) > { > - tree field = TREE_OPERAND (y, 1); > - tree type = DECL_FIELD_CONTEXT (field); > - if (TREE_CODE (type) == RECORD_TYPE) > - fieldsy.safe_push (TREE_OPERAND (y, 1)); > + if (TREE_CODE (y) == COMPONENT_REF) > + { > + tree field = TREE_OPERAND (y, 1); > + tree type = DECL_FIELD_CONTEXT (field); > + if (TREE_CODE (type) == RECORD_TYPE) > + fieldsy.safe_push (TREE_OPERAND (y, 1)); > + } > y = TREE_OPERAND (y, 0); > } > if (fieldsy.length () == 0) > - return false; > + { > + ++alias_stats.nonoverlapping_component_refs_p_may_alias; > + return false; > + } > > /* Most common case first. */ > if (fieldsx.length () == 1 > && fieldsy.length () == 1) > - return ((DECL_FIELD_CONTEXT (fieldsx[0]) > - == DECL_FIELD_CONTEXT (fieldsy[0])) > - && fieldsx[0] != fieldsy[0] > - && !(DECL_BIT_FIELD (fieldsx[0]) && DECL_BIT_FIELD (fieldsy[0]))); > + { > + if ((DECL_FIELD_CONTEXT (fieldsx[0]) > + == DECL_FIELD_CONTEXT (fieldsy[0])) > + && fieldsx[0] != fieldsy[0] > + && !(DECL_BIT_FIELD (fieldsx[0]) && DECL_BIT_FIELD (fieldsy[0]))) > + { > + ++alias_stats.nonoverlapping_component_refs_p_no_alias; > + return true; > + } > + else > + { > + ++alias_stats.nonoverlapping_component_refs_p_may_alias; > + return false; > + } > + } > > if (fieldsx.length () == 2) > { > @@ -1222,11 +1287,18 @@ nonoverlapping_component_refs_p (const_t > same. */ > if (DECL_BIT_FIELD_REPRESENTATIVE (fieldx) == fieldy > || DECL_BIT_FIELD_REPRESENTATIVE (fieldy) == fieldx) > - return false; > + { > + ++alias_stats.nonoverlapping_component_refs_p_may_alias; > + return false; > + } > /* Different fields of the same record type cannot overlap. > ??? Bitfields can overlap at RTL level so punt on them. */ > if (DECL_BIT_FIELD (fieldx) && DECL_BIT_FIELD (fieldy)) > - return false; > + { > + ++alias_stats.nonoverlapping_component_refs_p_may_alias; > + return false; > + } > + ++alias_stats.nonoverlapping_component_refs_p_no_alias; > return true; > } > } > @@ -1245,6 +1317,7 @@ nonoverlapping_component_refs_p (const_t > } > while (1); > > + ++alias_stats.nonoverlapping_component_refs_p_may_alias; > return false; > } > >
> > I think this is still error-prone since we look past > VIEW_CONVERT_EXPRs in the path so we happily disambiguate, > say, > > struct X { int e; int d; }; > struct Y { int d; int e; }; > > VIEW_CONVERT<X>(p->b).d > VIEW_CONVERT<Y>(q->b).e > > where in reality only the access paths from the base up to > the first view-conversion are relevant for path-based > analysis. > > So upon seeing a VIEW_CONVERT (or BIT_FIELD_REF which > has the same issue) simply truncate the vector. > > As said, it's a pre-existing issue but you are extending > things to possibly handle more stuff... > > Otherwise looks good. OK, i added the check and also managed to construct testcase that we do not optimize otherwise. I think same view convert bug ought to exist also in aliasing_component_refs_p and IMO ao_ref_alias set should also look for VCEs otherwise I can't make sense of /* First defer to TBAA if possible. */ if (tbaa_p && flag_strict_aliasing && !alias_sets_conflict_p (ao_ref_alias_set (ref1), ao_ref_alias_set (ref2))) return false; Honza * gcc.dg/tree-ssa/alias-access-path-2.c: New testcase. * tree-ssa-alias.c (alias_stats): Add nonoverlapping_component_refs_p_may_alias, nonoverlapping_component_refs_p_no_alias, nonoverlapping_component_refs_of_decl_p_may_alias, nonoverlapping_component_refs_of_decl_p_no_alias. (dump_alias_stats): Dump them. (nonoverlapping_component_refs_of_decl_p): Add stats. (nonoverlapping_component_refs_p): Add stats; do not stop on first ARRAY_REF. Index: testsuite/gcc.dg/tree-ssa/alias-access-path-2.c =================================================================== --- testsuite/gcc.dg/tree-ssa/alias-access-path-2.c (nonexistent) +++ testsuite/gcc.dg/tree-ssa/alias-access-path-2.c (working copy) @@ -0,0 +1,22 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-fre3" } */ +struct a { + int val; +}; +struct b { + struct a a[10],a2[10]; +}; +struct c { + struct b b[10]; +} *cptr; + +struct d {struct c c;} *dptr; + +int +test (int i, int j, int k, int l) +{ + cptr->b[i].a[j].val=123; + dptr->c.b[k].a2[l].val=2; + return cptr->b[i].a[j].val; +} +/* { dg-final { scan-tree-dump-times "return 123" 1 "fre3"} } */ Index: tree-ssa-alias.c =================================================================== --- tree-ssa-alias.c (revision 272283) +++ tree-ssa-alias.c (working copy) @@ -100,6 +100,10 @@ static struct { unsigned HOST_WIDE_INT call_may_clobber_ref_p_no_alias; unsigned HOST_WIDE_INT aliasing_component_refs_p_may_alias; unsigned HOST_WIDE_INT aliasing_component_refs_p_no_alias; + unsigned HOST_WIDE_INT nonoverlapping_component_refs_p_may_alias; + unsigned HOST_WIDE_INT nonoverlapping_component_refs_p_no_alias; + unsigned HOST_WIDE_INT nonoverlapping_component_refs_of_decl_p_may_alias; + unsigned HOST_WIDE_INT nonoverlapping_component_refs_of_decl_p_no_alias; } alias_stats; void @@ -124,7 +128,19 @@ dump_alias_stats (FILE *s) alias_stats.call_may_clobber_ref_p_no_alias, alias_stats.call_may_clobber_ref_p_no_alias + alias_stats.call_may_clobber_ref_p_may_alias); - fprintf (s, " aliasing_component_ref_p: " + fprintf (s, " nonoverlapping_component_refs_p: " + HOST_WIDE_INT_PRINT_DEC" disambiguations, " + HOST_WIDE_INT_PRINT_DEC" queries\n", + alias_stats.nonoverlapping_component_refs_p_no_alias, + alias_stats.nonoverlapping_component_refs_p_no_alias + + alias_stats.nonoverlapping_component_refs_p_may_alias); + fprintf (s, " nonoverlapping_component_refs_of_decl_p: " + HOST_WIDE_INT_PRINT_DEC" disambiguations, " + HOST_WIDE_INT_PRINT_DEC" queries\n", + alias_stats.nonoverlapping_component_refs_of_decl_p_no_alias, + alias_stats.nonoverlapping_component_refs_of_decl_p_no_alias + + alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias); + fprintf (s, " aliasing_component_refs_p: " HOST_WIDE_INT_PRINT_DEC" disambiguations, " HOST_WIDE_INT_PRINT_DEC" queries\n", alias_stats.aliasing_component_refs_p_no_alias, @@ -1047,7 +1063,10 @@ nonoverlapping_component_refs_of_decl_p if (TREE_CODE (ref1) == MEM_REF) { if (!integer_zerop (TREE_OPERAND (ref1, 1))) - return false; + { + ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias; + return false; + } ref1 = TREE_OPERAND (TREE_OPERAND (ref1, 0), 0); } @@ -1060,7 +1079,10 @@ nonoverlapping_component_refs_of_decl_p if (TREE_CODE (ref2) == MEM_REF) { if (!integer_zerop (TREE_OPERAND (ref2, 1))) - return false; + { + ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias; + return false; + } ref2 = TREE_OPERAND (TREE_OPERAND (ref2, 0), 0); } @@ -1080,7 +1102,10 @@ nonoverlapping_component_refs_of_decl_p do { if (component_refs1.is_empty ()) - return false; + { + ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias; + return false; + } ref1 = component_refs1.pop (); } while (!RECORD_OR_UNION_TYPE_P (TREE_TYPE (TREE_OPERAND (ref1, 0)))); @@ -1088,7 +1113,10 @@ nonoverlapping_component_refs_of_decl_p do { if (component_refs2.is_empty ()) - return false; + { + ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias; + return false; + } ref2 = component_refs2.pop (); } while (!RECORD_OR_UNION_TYPE_P (TREE_TYPE (TREE_OPERAND (ref2, 0)))); @@ -1096,7 +1124,10 @@ nonoverlapping_component_refs_of_decl_p /* Beware of BIT_FIELD_REF. */ if (TREE_CODE (ref1) != COMPONENT_REF || TREE_CODE (ref2) != COMPONENT_REF) - return false; + { + ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias; + return false; + } tree field1 = TREE_OPERAND (ref1, 1); tree field2 = TREE_OPERAND (ref2, 1); @@ -1109,7 +1140,10 @@ nonoverlapping_component_refs_of_decl_p /* We cannot disambiguate fields in a union or qualified union. */ if (type1 != type2 || TREE_CODE (type1) != RECORD_TYPE) - return false; + { + ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias; + return false; + } if (field1 != field2) { @@ -1117,15 +1151,23 @@ nonoverlapping_component_refs_of_decl_p same. */ if (DECL_BIT_FIELD_REPRESENTATIVE (field1) == field2 || DECL_BIT_FIELD_REPRESENTATIVE (field2) == field1) - return false; + { + ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias; + return false; + } /* Different fields of the same record type cannot overlap. ??? Bitfields can overlap at RTL level so punt on them. */ if (DECL_BIT_FIELD (field1) && DECL_BIT_FIELD (field2)) - return false; + { + ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias; + return false; + } + ++alias_stats.nonoverlapping_component_refs_of_decl_p_no_alias; return true; } } + ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias; return false; } @@ -1154,40 +1196,67 @@ nonoverlapping_component_refs_p (const_t { if (!flag_strict_aliasing || !x || !y - || TREE_CODE (x) != COMPONENT_REF - || TREE_CODE (y) != COMPONENT_REF) - return false; + || !handled_component_p (x) + || !handled_component_p (y)) + { + ++alias_stats.nonoverlapping_component_refs_p_may_alias; + return false; + } auto_vec<const_tree, 16> fieldsx; - while (TREE_CODE (x) == COMPONENT_REF) + while (handled_component_p (x)) { - tree field = TREE_OPERAND (x, 1); - tree type = DECL_FIELD_CONTEXT (field); - if (TREE_CODE (type) == RECORD_TYPE) - fieldsx.safe_push (field); + if (TREE_CODE (x) == COMPONENT_REF) + { + tree field = TREE_OPERAND (x, 1); + tree type = DECL_FIELD_CONTEXT (field); + if (TREE_CODE (type) == RECORD_TYPE) + fieldsx.safe_push (field); + } + else if (TREE_CODE (x) == VIEW_CONVERT_EXPR) + fieldsx.truncate (0); x = TREE_OPERAND (x, 0); } if (fieldsx.length () == 0) return false; auto_vec<const_tree, 16> fieldsy; - while (TREE_CODE (y) == COMPONENT_REF) + while (handled_component_p (y)) { - tree field = TREE_OPERAND (y, 1); - tree type = DECL_FIELD_CONTEXT (field); - if (TREE_CODE (type) == RECORD_TYPE) - fieldsy.safe_push (TREE_OPERAND (y, 1)); + if (TREE_CODE (y) == COMPONENT_REF) + { + tree field = TREE_OPERAND (y, 1); + tree type = DECL_FIELD_CONTEXT (field); + if (TREE_CODE (type) == RECORD_TYPE) + fieldsy.safe_push (TREE_OPERAND (y, 1)); + } + else if (TREE_CODE (y) == VIEW_CONVERT_EXPR) + fieldsx.truncate (0); y = TREE_OPERAND (y, 0); } if (fieldsy.length () == 0) - return false; + { + ++alias_stats.nonoverlapping_component_refs_p_may_alias; + return false; + } /* Most common case first. */ if (fieldsx.length () == 1 && fieldsy.length () == 1) - return ((DECL_FIELD_CONTEXT (fieldsx[0]) - == DECL_FIELD_CONTEXT (fieldsy[0])) - && fieldsx[0] != fieldsy[0] - && !(DECL_BIT_FIELD (fieldsx[0]) && DECL_BIT_FIELD (fieldsy[0]))); + { + if ((DECL_FIELD_CONTEXT (fieldsx[0]) + == DECL_FIELD_CONTEXT (fieldsy[0])) + && fieldsx[0] != fieldsy[0] + && !(DECL_BIT_FIELD (fieldsx[0]) && DECL_BIT_FIELD (fieldsy[0]))) + { + ++alias_stats.nonoverlapping_component_refs_p_no_alias; + return true; + } + else + { + ++alias_stats.nonoverlapping_component_refs_p_may_alias; + return false; + } + } if (fieldsx.length () == 2) { @@ -1222,11 +1291,18 @@ nonoverlapping_component_refs_p (const_t same. */ if (DECL_BIT_FIELD_REPRESENTATIVE (fieldx) == fieldy || DECL_BIT_FIELD_REPRESENTATIVE (fieldy) == fieldx) - return false; + { + ++alias_stats.nonoverlapping_component_refs_p_may_alias; + return false; + } /* Different fields of the same record type cannot overlap. ??? Bitfields can overlap at RTL level so punt on them. */ if (DECL_BIT_FIELD (fieldx) && DECL_BIT_FIELD (fieldy)) - return false; + { + ++alias_stats.nonoverlapping_component_refs_p_may_alias; + return false; + } + ++alias_stats.nonoverlapping_component_refs_p_no_alias; return true; } } @@ -1245,6 +1321,7 @@ nonoverlapping_component_refs_p (const_t } while (1); + ++alias_stats.nonoverlapping_component_refs_p_may_alias; return false; }
... and forgot to add stats for tramp3d :)
Alias oracle query stats:
refs_may_alias_p: 3021539 disambiguations, 3321136 queries
ref_maybe_used_by_call_p: 7118 disambiguations, 3047133 queries
call_may_clobber_ref_p: 817 disambiguations, 817 queries
nonoverlapping_component_refs_p: 22 disambiguations, 61735 queries
nonoverlapping_component_refs_of_decl_p: 19 disambiguations, 2192 queries
aliasing_component_refs_p: 2050 disambiguations, 19908 queries
TBAA oracle: 1419961 disambiguations 2916692 queries
555158 are in alias set 0
575103 queries asked about the same object
0 queries asked about the same alias set
0 access volatile
252874 are dependent in the DAG
113596 are aritificially in conflict with void *
PTA query stats:
pt_solution_includes: 671982 disambiguations, 952513 queries
pt_solutions_intersect: 97060 disambiguations, 437912 queries
Hi,
my patch had a typo - I forgot to update the second call of truncate.
This fixes it. Bootstrapped/regtested with all languages on x86_64-linux
and comitted.
This unbreaks non-lto Ada bootstrap failure.
* tree-ssa-alias.c (nonoverlapping_component_refs_p): Fix pasto
in my previous patch.
Index: tree-ssa-alias.c
===================================================================
--- tree-ssa-alias.c (revision 272353)
+++ tree-ssa-alias.c (working copy)
@@ -1230,7 +1230,7 @@ nonoverlapping_component_refs_p (const_t
fieldsy.safe_push (TREE_OPERAND (y, 1));
}
else if (TREE_CODE (y) == VIEW_CONVERT_EXPR)
- fieldsx.truncate (0);
+ fieldsy.truncate (0);
y = TREE_OPERAND (y, 0);
}
if (fieldsy.length () == 0)
On Fri, 14 Jun 2019, Jan Hubicka wrote: > > > > I think this is still error-prone since we look past > > VIEW_CONVERT_EXPRs in the path so we happily disambiguate, > > say, > > > > struct X { int e; int d; }; > > struct Y { int d; int e; }; > > > > VIEW_CONVERT<X>(p->b).d > > VIEW_CONVERT<Y>(q->b).e > > > > where in reality only the access paths from the base up to > > the first view-conversion are relevant for path-based > > analysis. > > > > So upon seeing a VIEW_CONVERT (or BIT_FIELD_REF which > > has the same issue) simply truncate the vector. > > > > As said, it's a pre-existing issue but you are extending > > things to possibly handle more stuff... > > > > Otherwise looks good. > OK, i added the check and also managed to construct testcase that we do > not optimize otherwise. > > I think same view convert bug ought to exist also in > aliasing_component_refs_p and IMO ao_ref_alias set should also look > for VCEs otherwise I can't make sense of > > /* First defer to TBAA if possible. */ > if (tbaa_p > && flag_strict_aliasing > && !alias_sets_conflict_p (ao_ref_alias_set (ref1), > ao_ref_alias_set (ref2))) > return false; get_alias_set already handles VCEs properly. Btw I've said BIT_FIELD_REF has the same issue but you didn't include that case below. Richard. > Honza > > * gcc.dg/tree-ssa/alias-access-path-2.c: New testcase. > > * tree-ssa-alias.c (alias_stats): Add > nonoverlapping_component_refs_p_may_alias, > nonoverlapping_component_refs_p_no_alias, > nonoverlapping_component_refs_of_decl_p_may_alias, > nonoverlapping_component_refs_of_decl_p_no_alias. > (dump_alias_stats): Dump them. > (nonoverlapping_component_refs_of_decl_p): Add stats. > (nonoverlapping_component_refs_p): Add stats; do not stop on first > ARRAY_REF. > > Index: testsuite/gcc.dg/tree-ssa/alias-access-path-2.c > =================================================================== > --- testsuite/gcc.dg/tree-ssa/alias-access-path-2.c (nonexistent) > +++ testsuite/gcc.dg/tree-ssa/alias-access-path-2.c (working copy) > @@ -0,0 +1,22 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdump-tree-fre3" } */ > +struct a { > + int val; > +}; > +struct b { > + struct a a[10],a2[10]; > +}; > +struct c { > + struct b b[10]; > +} *cptr; > + > +struct d {struct c c;} *dptr; > + > +int > +test (int i, int j, int k, int l) > +{ > + cptr->b[i].a[j].val=123; > + dptr->c.b[k].a2[l].val=2; > + return cptr->b[i].a[j].val; > +} > +/* { dg-final { scan-tree-dump-times "return 123" 1 "fre3"} } */ > Index: tree-ssa-alias.c > =================================================================== > --- tree-ssa-alias.c (revision 272283) > +++ tree-ssa-alias.c (working copy) > @@ -100,6 +100,10 @@ static struct { > unsigned HOST_WIDE_INT call_may_clobber_ref_p_no_alias; > unsigned HOST_WIDE_INT aliasing_component_refs_p_may_alias; > unsigned HOST_WIDE_INT aliasing_component_refs_p_no_alias; > + unsigned HOST_WIDE_INT nonoverlapping_component_refs_p_may_alias; > + unsigned HOST_WIDE_INT nonoverlapping_component_refs_p_no_alias; > + unsigned HOST_WIDE_INT nonoverlapping_component_refs_of_decl_p_may_alias; > + unsigned HOST_WIDE_INT nonoverlapping_component_refs_of_decl_p_no_alias; > } alias_stats; > > void > @@ -124,7 +128,19 @@ dump_alias_stats (FILE *s) > alias_stats.call_may_clobber_ref_p_no_alias, > alias_stats.call_may_clobber_ref_p_no_alias > + alias_stats.call_may_clobber_ref_p_may_alias); > - fprintf (s, " aliasing_component_ref_p: " > + fprintf (s, " nonoverlapping_component_refs_p: " > + HOST_WIDE_INT_PRINT_DEC" disambiguations, " > + HOST_WIDE_INT_PRINT_DEC" queries\n", > + alias_stats.nonoverlapping_component_refs_p_no_alias, > + alias_stats.nonoverlapping_component_refs_p_no_alias > + + alias_stats.nonoverlapping_component_refs_p_may_alias); > + fprintf (s, " nonoverlapping_component_refs_of_decl_p: " > + HOST_WIDE_INT_PRINT_DEC" disambiguations, " > + HOST_WIDE_INT_PRINT_DEC" queries\n", > + alias_stats.nonoverlapping_component_refs_of_decl_p_no_alias, > + alias_stats.nonoverlapping_component_refs_of_decl_p_no_alias > + + alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias); > + fprintf (s, " aliasing_component_refs_p: " > HOST_WIDE_INT_PRINT_DEC" disambiguations, " > HOST_WIDE_INT_PRINT_DEC" queries\n", > alias_stats.aliasing_component_refs_p_no_alias, > @@ -1047,7 +1063,10 @@ nonoverlapping_component_refs_of_decl_p > if (TREE_CODE (ref1) == MEM_REF) > { > if (!integer_zerop (TREE_OPERAND (ref1, 1))) > - return false; > + { > + ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias; > + return false; > + } > ref1 = TREE_OPERAND (TREE_OPERAND (ref1, 0), 0); > } > > @@ -1060,7 +1079,10 @@ nonoverlapping_component_refs_of_decl_p > if (TREE_CODE (ref2) == MEM_REF) > { > if (!integer_zerop (TREE_OPERAND (ref2, 1))) > - return false; > + { > + ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias; > + return false; > + } > ref2 = TREE_OPERAND (TREE_OPERAND (ref2, 0), 0); > } > > @@ -1080,7 +1102,10 @@ nonoverlapping_component_refs_of_decl_p > do > { > if (component_refs1.is_empty ()) > - return false; > + { > + ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias; > + return false; > + } > ref1 = component_refs1.pop (); > } > while (!RECORD_OR_UNION_TYPE_P (TREE_TYPE (TREE_OPERAND (ref1, 0)))); > @@ -1088,7 +1113,10 @@ nonoverlapping_component_refs_of_decl_p > do > { > if (component_refs2.is_empty ()) > - return false; > + { > + ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias; > + return false; > + } > ref2 = component_refs2.pop (); > } > while (!RECORD_OR_UNION_TYPE_P (TREE_TYPE (TREE_OPERAND (ref2, 0)))); > @@ -1096,7 +1124,10 @@ nonoverlapping_component_refs_of_decl_p > /* Beware of BIT_FIELD_REF. */ > if (TREE_CODE (ref1) != COMPONENT_REF > || TREE_CODE (ref2) != COMPONENT_REF) > - return false; > + { > + ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias; > + return false; > + } > > tree field1 = TREE_OPERAND (ref1, 1); > tree field2 = TREE_OPERAND (ref2, 1); > @@ -1109,7 +1140,10 @@ nonoverlapping_component_refs_of_decl_p > > /* We cannot disambiguate fields in a union or qualified union. */ > if (type1 != type2 || TREE_CODE (type1) != RECORD_TYPE) > - return false; > + { > + ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias; > + return false; > + } > > if (field1 != field2) > { > @@ -1117,15 +1151,23 @@ nonoverlapping_component_refs_of_decl_p > same. */ > if (DECL_BIT_FIELD_REPRESENTATIVE (field1) == field2 > || DECL_BIT_FIELD_REPRESENTATIVE (field2) == field1) > - return false; > + { > + ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias; > + return false; > + } > /* Different fields of the same record type cannot overlap. > ??? Bitfields can overlap at RTL level so punt on them. */ > if (DECL_BIT_FIELD (field1) && DECL_BIT_FIELD (field2)) > - return false; > + { > + ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias; > + return false; > + } > + ++alias_stats.nonoverlapping_component_refs_of_decl_p_no_alias; > return true; > } > } > > + ++alias_stats.nonoverlapping_component_refs_of_decl_p_may_alias; > return false; > } > > @@ -1154,40 +1196,67 @@ nonoverlapping_component_refs_p (const_t > { > if (!flag_strict_aliasing > || !x || !y > - || TREE_CODE (x) != COMPONENT_REF > - || TREE_CODE (y) != COMPONENT_REF) > - return false; > + || !handled_component_p (x) > + || !handled_component_p (y)) > + { > + ++alias_stats.nonoverlapping_component_refs_p_may_alias; > + return false; > + } > > auto_vec<const_tree, 16> fieldsx; > - while (TREE_CODE (x) == COMPONENT_REF) > + while (handled_component_p (x)) > { > - tree field = TREE_OPERAND (x, 1); > - tree type = DECL_FIELD_CONTEXT (field); > - if (TREE_CODE (type) == RECORD_TYPE) > - fieldsx.safe_push (field); > + if (TREE_CODE (x) == COMPONENT_REF) > + { > + tree field = TREE_OPERAND (x, 1); > + tree type = DECL_FIELD_CONTEXT (field); > + if (TREE_CODE (type) == RECORD_TYPE) > + fieldsx.safe_push (field); > + } > + else if (TREE_CODE (x) == VIEW_CONVERT_EXPR) > + fieldsx.truncate (0); > x = TREE_OPERAND (x, 0); > } > if (fieldsx.length () == 0) > return false; > auto_vec<const_tree, 16> fieldsy; > - while (TREE_CODE (y) == COMPONENT_REF) > + while (handled_component_p (y)) > { > - tree field = TREE_OPERAND (y, 1); > - tree type = DECL_FIELD_CONTEXT (field); > - if (TREE_CODE (type) == RECORD_TYPE) > - fieldsy.safe_push (TREE_OPERAND (y, 1)); > + if (TREE_CODE (y) == COMPONENT_REF) > + { > + tree field = TREE_OPERAND (y, 1); > + tree type = DECL_FIELD_CONTEXT (field); > + if (TREE_CODE (type) == RECORD_TYPE) > + fieldsy.safe_push (TREE_OPERAND (y, 1)); > + } > + else if (TREE_CODE (y) == VIEW_CONVERT_EXPR) > + fieldsx.truncate (0); > y = TREE_OPERAND (y, 0); > } > if (fieldsy.length () == 0) > - return false; > + { > + ++alias_stats.nonoverlapping_component_refs_p_may_alias; > + return false; > + } > > /* Most common case first. */ > if (fieldsx.length () == 1 > && fieldsy.length () == 1) > - return ((DECL_FIELD_CONTEXT (fieldsx[0]) > - == DECL_FIELD_CONTEXT (fieldsy[0])) > - && fieldsx[0] != fieldsy[0] > - && !(DECL_BIT_FIELD (fieldsx[0]) && DECL_BIT_FIELD (fieldsy[0]))); > + { > + if ((DECL_FIELD_CONTEXT (fieldsx[0]) > + == DECL_FIELD_CONTEXT (fieldsy[0])) > + && fieldsx[0] != fieldsy[0] > + && !(DECL_BIT_FIELD (fieldsx[0]) && DECL_BIT_FIELD (fieldsy[0]))) > + { > + ++alias_stats.nonoverlapping_component_refs_p_no_alias; > + return true; > + } > + else > + { > + ++alias_stats.nonoverlapping_component_refs_p_may_alias; > + return false; > + } > + } > > if (fieldsx.length () == 2) > { > @@ -1222,11 +1291,18 @@ nonoverlapping_component_refs_p (const_t > same. */ > if (DECL_BIT_FIELD_REPRESENTATIVE (fieldx) == fieldy > || DECL_BIT_FIELD_REPRESENTATIVE (fieldy) == fieldx) > - return false; > + { > + ++alias_stats.nonoverlapping_component_refs_p_may_alias; > + return false; > + } > /* Different fields of the same record type cannot overlap. > ??? Bitfields can overlap at RTL level so punt on them. */ > if (DECL_BIT_FIELD (fieldx) && DECL_BIT_FIELD (fieldy)) > - return false; > + { > + ++alias_stats.nonoverlapping_component_refs_p_may_alias; > + return false; > + } > + ++alias_stats.nonoverlapping_component_refs_p_no_alias; > return true; > } > } > @@ -1245,6 +1321,7 @@ nonoverlapping_component_refs_p (const_t > } > while (1); > > + ++alias_stats.nonoverlapping_component_refs_p_may_alias; > return false; > } > >
> > get_alias_set already handles VCEs properly. Btw I've said I see - i keep thinking of get_alias_set as a simple accessor to type's alias set which it is not. It may make sense to separate these two later. > BIT_FIELD_REF has the same issue but you didn't include that > case below. Sorry, missed that in your reply. I am testing the fix. What about aliasing_component_refs. When I have VCE in the the path, I suppose I do not want to consider the stuff after VCE as part of path and also while testing whether one path can be continuation from other I want to start from outermost VCE rather than ref_type, right? So I would go for adjusting the ref1 and ref2 accordingly in the first walk of handled components. alias set needs no updating since that is aready done earlier. Honza
On Mon, 17 Jun 2019, Jan Hubicka wrote: > > > > get_alias_set already handles VCEs properly. Btw I've said > > I see - i keep thinking of get_alias_set as a simple accessor to type's > alias set which it is not. It may make sense to separate these two > later. > > > BIT_FIELD_REF has the same issue but you didn't include that > > case below. > > Sorry, missed that in your reply. I am testing the fix. > > What about aliasing_component_refs. When I have VCE in the the path, > I suppose I do not want to consider the stuff after VCE as part of path > and also while testing whether one path can be continuation from other > I want to start from outermost VCE rather than ref_type, right? Yeah, when searching for base1/2 you want to initialize ref1/2p like while (handled_component_p (base1)) { if (TREE_CODE (base1) == VIEW_CONVERT_EXPR || TREE_CODE (base1) == BIT_FIELD_REF) ref1p = &TREE_OPERAND (base1, 0); base1 = TREE_OPERAND (base1, 0); } and then instead of refp = &ref1/2 do refp = ref1/2p for the searches. So not use the type of the innermost(!) VIEW_CONVERT_EXPR but its base (same for BIT_FIELD_REF). > > So I would go for adjusting the ref1 and ref2 accordingly in the first > walk of handled components. alias set needs no updating since that is > aready done earlier. Yes, adjusting ref1/2 is also possible. Richard. > Honza >
> On Mon, 17 Jun 2019, Jan Hubicka wrote: > > > > > > > get_alias_set already handles VCEs properly. Btw I've said > > > > I see - i keep thinking of get_alias_set as a simple accessor to type's > > alias set which it is not. It may make sense to separate these two > > later. > > > > > BIT_FIELD_REF has the same issue but you didn't include that > > > case below. > > > > Sorry, missed that in your reply. I am testing the fix. > > > > What about aliasing_component_refs. When I have VCE in the the path, > > I suppose I do not want to consider the stuff after VCE as part of path > > and also while testing whether one path can be continuation from other > > I want to start from outermost VCE rather than ref_type, right? > > Yeah, when searching for base1/2 you want to initialize ref1/2p > like > > while (handled_component_p (base1)) > { > if (TREE_CODE (base1) == VIEW_CONVERT_EXPR > || TREE_CODE (base1) == BIT_FIELD_REF) > ref1p = &TREE_OPERAND (base1, 0); > base1 = TREE_OPERAND (base1, 0); > } > > and then instead of refp = &ref1/2 do refp = ref1/2p for the searches. > > So not use the type of the innermost(!) VIEW_CONVERT_EXPR but its > base (same for BIT_FIELD_REF). Yep, that is what I had in mind. I will test the patch. Also will remove the ref pointers - we always just read them. Thanks, Honza
Hi
this is patch for BIT_FIELD_REFs on nonoverlapping_component_refs_p
I comitted after testing on x86_64-linux
Honza
Index: ChangeLog
===================================================================
--- ChangeLog (revision 272379)
+++ ChangeLog (working copy)
@@ -1,3 +1,8 @@
+2019-06-17 Jan Hubicka <hubicka@ucw.cz>
+
+ * tree-ssa-alias.c (nonoverlapping_component_refs_p): Also truncate
+ access path on BIT_FIELD_REFs.
+
2019-06-17 Martin Liska <mliska@suse.cz>
PR ipa/90874
Index: tree-ssa-alias.c
===================================================================
--- tree-ssa-alias.c (revision 272379)
+++ tree-ssa-alias.c (working copy)
@@ -1268,7 +1268,8 @@ nonoverlapping_component_refs_p (const_t
if (TREE_CODE (type) == RECORD_TYPE)
fieldsx.safe_push (field);
}
- else if (TREE_CODE (x) == VIEW_CONVERT_EXPR)
+ else if (TREE_CODE (x) == VIEW_CONVERT_EXPR
+ || TREE_CODE (x) == BIT_FIELD_REF)
fieldsx.truncate (0);
x = TREE_OPERAND (x, 0);
}
@@ -1284,7 +1285,8 @@ nonoverlapping_component_refs_p (const_t
if (TREE_CODE (type) == RECORD_TYPE)
fieldsy.safe_push (TREE_OPERAND (y, 1));
}
- else if (TREE_CODE (y) == VIEW_CONVERT_EXPR)
+ else if (TREE_CODE (y) == VIEW_CONVERT_EXPR
+ || TREE_CODE (y) == BIT_FIELD_REF)
fieldsy.truncate (0);
y = TREE_OPERAND (y, 0);
}
Hi,
this is patch for aliasing_component_refs_p and VCE.
I also turned the refp to ref, but there are no changes relative
to that.
Bootstrapped/regtested x86_64-linux, OK?
Honza
* tree-ssa-alias.c (aliasing_component_refs_p): Consider only
the access path from base to first VIEW_CONVERT_EXPR or
BIT_FIELD_REF.
Index: tree-ssa-alias.c
===================================================================
--- tree-ssa-alias.c (revision 272381)
+++ tree-ssa-alias.c (working copy)
@@ -874,7 +874,6 @@ aliasing_component_refs_p (tree ref1,
disambiguating q->i and p->a.j. */
tree base1, base2;
tree type1, type2;
- tree *refp;
int same_p1 = 0, same_p2 = 0;
bool maybe_match = false;
tree end_struct_ref1 = NULL, end_struct_ref2 = NULL;
@@ -903,6 +902,9 @@ aliasing_component_refs_p (tree ref1,
gcc_checking_assert (!end_struct_ref1);
end_struct_ref1 = base1;
}
+ if (TREE_CODE (base1) == VIEW_CONVERT_EXPR
+ || TREE_CODE (base1) == BIT_FIELD_REF)
+ ref1 = TREE_OPERAND (base1, 0);
base1 = TREE_OPERAND (base1, 0);
}
type1 = TREE_TYPE (base1);
@@ -918,6 +920,9 @@ aliasing_component_refs_p (tree ref1,
gcc_checking_assert (!end_struct_ref2);
end_struct_ref2 = base2;
}
+ if (TREE_CODE (base2) == VIEW_CONVERT_EXPR
+ || TREE_CODE (base2) == BIT_FIELD_REF)
+ ref2 = TREE_OPERAND (base2, 0);
base2 = TREE_OPERAND (base2, 0);
}
type2 = TREE_TYPE (base2);
@@ -934,23 +939,23 @@ aliasing_component_refs_p (tree ref1,
|| (end_struct_ref2
&& compare_type_sizes (TREE_TYPE (end_struct_ref2), type1) >= 0))
{
- refp = &ref2;
+ tree ref = ref2;
while (true)
{
/* We walk from inner type to the outer types. If type we see is
already too large to be part of type1, terminate the search. */
- int cmp = compare_type_sizes (type1, TREE_TYPE (*refp));
+ int cmp = compare_type_sizes (type1, TREE_TYPE (ref));
if (cmp < 0
&& (!end_struct_ref1
|| compare_type_sizes (TREE_TYPE (end_struct_ref1),
- TREE_TYPE (*refp)) < 0))
+ TREE_TYPE (ref)) < 0))
break;
/* If types may be of same size, see if we can decide about their
equality. */
if (cmp == 0)
{
- same_p2 = same_type_for_tbaa (TREE_TYPE (*refp), type1);
+ same_p2 = same_type_for_tbaa (TREE_TYPE (ref), type1);
if (same_p2 == 1)
break;
/* In case we can't decide whether types are same try to
@@ -960,9 +965,9 @@ aliasing_component_refs_p (tree ref1,
if (same_p2 == -1)
maybe_match = true;
}
- if (!handled_component_p (*refp))
+ if (!handled_component_p (ref))
break;
- refp = &TREE_OPERAND (*refp, 0);
+ ref = TREE_OPERAND (ref, 0);
}
if (same_p2 == 1)
{
@@ -977,13 +982,13 @@ aliasing_component_refs_p (tree ref1,
if (TREE_CODE (TREE_TYPE (base1)) == ARRAY_TYPE
&& (!TYPE_SIZE (TREE_TYPE (base1))
|| TREE_CODE (TYPE_SIZE (TREE_TYPE (base1))) != INTEGER_CST
- || (*refp == base2 && !ref2_is_decl)))
+ || (ref == base2 && !ref2_is_decl)))
{
++alias_stats.aliasing_component_refs_p_may_alias;
return true;
}
- get_ref_base_and_extent (*refp, &offadj, &sztmp, &msztmp, &reverse);
+ get_ref_base_and_extent (ref, &offadj, &sztmp, &msztmp, &reverse);
offset2 -= offadj;
get_ref_base_and_extent (base1, &offadj, &sztmp, &msztmp, &reverse);
offset1 -= offadj;
@@ -1005,28 +1010,28 @@ aliasing_component_refs_p (tree ref1,
|| (end_struct_ref1
&& compare_type_sizes (TREE_TYPE (end_struct_ref1), type1) <= 0))
{
- refp = &ref1;
+ tree ref = ref1;
while (true)
{
- int cmp = compare_type_sizes (type2, TREE_TYPE (*refp));
+ int cmp = compare_type_sizes (type2, TREE_TYPE (ref));
if (cmp < 0
&& (!end_struct_ref2
|| compare_type_sizes (TREE_TYPE (end_struct_ref2),
- TREE_TYPE (*refp)) < 0))
+ TREE_TYPE (ref)) < 0))
break;
/* If types may be of same size, see if we can decide about their
equality. */
if (cmp == 0)
{
- same_p1 = same_type_for_tbaa (TREE_TYPE (*refp), type2);
+ same_p1 = same_type_for_tbaa (TREE_TYPE (ref), type2);
if (same_p1 == 1)
break;
if (same_p1 == -1)
maybe_match = true;
}
- if (!handled_component_p (*refp))
+ if (!handled_component_p (ref))
break;
- refp = &TREE_OPERAND (*refp, 0);
+ ref = TREE_OPERAND (ref, 0);
}
if (same_p1 == 1)
{
@@ -1036,13 +1041,13 @@ aliasing_component_refs_p (tree ref1,
if (TREE_CODE (TREE_TYPE (base2)) == ARRAY_TYPE
&& (!TYPE_SIZE (TREE_TYPE (base2))
|| TREE_CODE (TYPE_SIZE (TREE_TYPE (base2))) != INTEGER_CST
- || (*refp == base1 && !ref2_is_decl)))
+ || (ref == base1 && !ref2_is_decl)))
{
++alias_stats.aliasing_component_refs_p_may_alias;
return true;
}
- get_ref_base_and_extent (*refp, &offadj, &sztmp, &msztmp, &reverse);
+ get_ref_base_and_extent (ref, &offadj, &sztmp, &msztmp, &reverse);
offset1 -= offadj;
get_ref_base_and_extent (base2, &offadj, &sztmp, &msztmp, &reverse);
offset2 -= offadj;
On Mon, 17 Jun 2019, Jan Hubicka wrote: > Hi, > this is patch for aliasing_component_refs_p and VCE. > I also turned the refp to ref, but there are no changes relative > to that. > Bootstrapped/regtested x86_64-linux, OK? OK. Bonus points for wrong-code testcases involving these... like the following which doesn't fail for some reason: struct S { int i; int j[2]; }; __GIMPLE(startwith("fre1")) int foo(struct S *x, struct S *y) { int _1; x->j[0] = 1; __VIEW_CONVERT <struct S> (__MEM <int[2]> ((int(*)[2])y + 4)).i = 0; _1 = x->j[0]; return _1; } int main() { struct S s; if (foo (&s, &s) != 0) __builtin_abort (); return 0; } Thanks, Richard. > Honza > > * tree-ssa-alias.c (aliasing_component_refs_p): Consider only > the access path from base to first VIEW_CONVERT_EXPR or > BIT_FIELD_REF. > Index: tree-ssa-alias.c > =================================================================== > --- tree-ssa-alias.c (revision 272381) > +++ tree-ssa-alias.c (working copy) > @@ -874,7 +874,6 @@ aliasing_component_refs_p (tree ref1, > disambiguating q->i and p->a.j. */ > tree base1, base2; > tree type1, type2; > - tree *refp; > int same_p1 = 0, same_p2 = 0; > bool maybe_match = false; > tree end_struct_ref1 = NULL, end_struct_ref2 = NULL; > @@ -903,6 +902,9 @@ aliasing_component_refs_p (tree ref1, > gcc_checking_assert (!end_struct_ref1); > end_struct_ref1 = base1; > } > + if (TREE_CODE (base1) == VIEW_CONVERT_EXPR > + || TREE_CODE (base1) == BIT_FIELD_REF) > + ref1 = TREE_OPERAND (base1, 0); > base1 = TREE_OPERAND (base1, 0); > } > type1 = TREE_TYPE (base1); > @@ -918,6 +920,9 @@ aliasing_component_refs_p (tree ref1, > gcc_checking_assert (!end_struct_ref2); > end_struct_ref2 = base2; > } > + if (TREE_CODE (base2) == VIEW_CONVERT_EXPR > + || TREE_CODE (base2) == BIT_FIELD_REF) > + ref2 = TREE_OPERAND (base2, 0); > base2 = TREE_OPERAND (base2, 0); > } > type2 = TREE_TYPE (base2); > @@ -934,23 +939,23 @@ aliasing_component_refs_p (tree ref1, > || (end_struct_ref2 > && compare_type_sizes (TREE_TYPE (end_struct_ref2), type1) >= 0)) > { > - refp = &ref2; > + tree ref = ref2; > while (true) > { > /* We walk from inner type to the outer types. If type we see is > already too large to be part of type1, terminate the search. */ > - int cmp = compare_type_sizes (type1, TREE_TYPE (*refp)); > + int cmp = compare_type_sizes (type1, TREE_TYPE (ref)); > > if (cmp < 0 > && (!end_struct_ref1 > || compare_type_sizes (TREE_TYPE (end_struct_ref1), > - TREE_TYPE (*refp)) < 0)) > + TREE_TYPE (ref)) < 0)) > break; > /* If types may be of same size, see if we can decide about their > equality. */ > if (cmp == 0) > { > - same_p2 = same_type_for_tbaa (TREE_TYPE (*refp), type1); > + same_p2 = same_type_for_tbaa (TREE_TYPE (ref), type1); > if (same_p2 == 1) > break; > /* In case we can't decide whether types are same try to > @@ -960,9 +965,9 @@ aliasing_component_refs_p (tree ref1, > if (same_p2 == -1) > maybe_match = true; > } > - if (!handled_component_p (*refp)) > + if (!handled_component_p (ref)) > break; > - refp = &TREE_OPERAND (*refp, 0); > + ref = TREE_OPERAND (ref, 0); > } > if (same_p2 == 1) > { > @@ -977,13 +982,13 @@ aliasing_component_refs_p (tree ref1, > if (TREE_CODE (TREE_TYPE (base1)) == ARRAY_TYPE > && (!TYPE_SIZE (TREE_TYPE (base1)) > || TREE_CODE (TYPE_SIZE (TREE_TYPE (base1))) != INTEGER_CST > - || (*refp == base2 && !ref2_is_decl))) > + || (ref == base2 && !ref2_is_decl))) > { > ++alias_stats.aliasing_component_refs_p_may_alias; > return true; > } > > - get_ref_base_and_extent (*refp, &offadj, &sztmp, &msztmp, &reverse); > + get_ref_base_and_extent (ref, &offadj, &sztmp, &msztmp, &reverse); > offset2 -= offadj; > get_ref_base_and_extent (base1, &offadj, &sztmp, &msztmp, &reverse); > offset1 -= offadj; > @@ -1005,28 +1010,28 @@ aliasing_component_refs_p (tree ref1, > || (end_struct_ref1 > && compare_type_sizes (TREE_TYPE (end_struct_ref1), type1) <= 0)) > { > - refp = &ref1; > + tree ref = ref1; > while (true) > { > - int cmp = compare_type_sizes (type2, TREE_TYPE (*refp)); > + int cmp = compare_type_sizes (type2, TREE_TYPE (ref)); > if (cmp < 0 > && (!end_struct_ref2 > || compare_type_sizes (TREE_TYPE (end_struct_ref2), > - TREE_TYPE (*refp)) < 0)) > + TREE_TYPE (ref)) < 0)) > break; > /* If types may be of same size, see if we can decide about their > equality. */ > if (cmp == 0) > { > - same_p1 = same_type_for_tbaa (TREE_TYPE (*refp), type2); > + same_p1 = same_type_for_tbaa (TREE_TYPE (ref), type2); > if (same_p1 == 1) > break; > if (same_p1 == -1) > maybe_match = true; > } > - if (!handled_component_p (*refp)) > + if (!handled_component_p (ref)) > break; > - refp = &TREE_OPERAND (*refp, 0); > + ref = TREE_OPERAND (ref, 0); > } > if (same_p1 == 1) > { > @@ -1036,13 +1041,13 @@ aliasing_component_refs_p (tree ref1, > if (TREE_CODE (TREE_TYPE (base2)) == ARRAY_TYPE > && (!TYPE_SIZE (TREE_TYPE (base2)) > || TREE_CODE (TYPE_SIZE (TREE_TYPE (base2))) != INTEGER_CST > - || (*refp == base1 && !ref2_is_decl))) > + || (ref == base1 && !ref2_is_decl))) > { > ++alias_stats.aliasing_component_refs_p_may_alias; > return true; > } > > - get_ref_base_and_extent (*refp, &offadj, &sztmp, &msztmp, &reverse); > + get_ref_base_and_extent (ref, &offadj, &sztmp, &msztmp, &reverse); > offset1 -= offadj; > get_ref_base_and_extent (base2, &offadj, &sztmp, &msztmp, &reverse); > offset2 -= offadj; >
> On Mon, 17 Jun 2019, Jan Hubicka wrote: > > > Hi, > > this is patch for aliasing_component_refs_p and VCE. > > I also turned the refp to ref, but there are no changes relative > > to that. > > Bootstrapped/regtested x86_64-linux, OK? > > OK. > > Bonus points for wrong-code testcases involving these... like > the following which doesn't fail for some reason: We punt on same_type_for_tbaa on arrays so the aliasing_component_refs_p thinks that int and int[2] possibly match and gives up :) Honza > > struct S { int i; int j[2]; }; > > __GIMPLE(startwith("fre1")) int foo(struct S *x, struct S *y) > { > int _1; > x->j[0] = 1; > __VIEW_CONVERT <struct S> (__MEM <int[2]> ((int(*)[2])y + 4)).i = 0; > _1 = x->j[0]; > return _1; > } > > int main() > { > struct S s; > if (foo (&s, &s) != 0) > __builtin_abort (); > return 0; > } > > Thanks, > Richard. > > > Honza > > > > * tree-ssa-alias.c (aliasing_component_refs_p): Consider only > > the access path from base to first VIEW_CONVERT_EXPR or > > BIT_FIELD_REF. > > Index: tree-ssa-alias.c > > =================================================================== > > --- tree-ssa-alias.c (revision 272381) > > +++ tree-ssa-alias.c (working copy) > > @@ -874,7 +874,6 @@ aliasing_component_refs_p (tree ref1, > > disambiguating q->i and p->a.j. */ > > tree base1, base2; > > tree type1, type2; > > - tree *refp; > > int same_p1 = 0, same_p2 = 0; > > bool maybe_match = false; > > tree end_struct_ref1 = NULL, end_struct_ref2 = NULL; > > @@ -903,6 +902,9 @@ aliasing_component_refs_p (tree ref1, > > gcc_checking_assert (!end_struct_ref1); > > end_struct_ref1 = base1; > > } > > + if (TREE_CODE (base1) == VIEW_CONVERT_EXPR > > + || TREE_CODE (base1) == BIT_FIELD_REF) > > + ref1 = TREE_OPERAND (base1, 0); > > base1 = TREE_OPERAND (base1, 0); > > } > > type1 = TREE_TYPE (base1); > > @@ -918,6 +920,9 @@ aliasing_component_refs_p (tree ref1, > > gcc_checking_assert (!end_struct_ref2); > > end_struct_ref2 = base2; > > } > > + if (TREE_CODE (base2) == VIEW_CONVERT_EXPR > > + || TREE_CODE (base2) == BIT_FIELD_REF) > > + ref2 = TREE_OPERAND (base2, 0); > > base2 = TREE_OPERAND (base2, 0); > > } > > type2 = TREE_TYPE (base2); > > @@ -934,23 +939,23 @@ aliasing_component_refs_p (tree ref1, > > || (end_struct_ref2 > > && compare_type_sizes (TREE_TYPE (end_struct_ref2), type1) >= 0)) > > { > > - refp = &ref2; > > + tree ref = ref2; > > while (true) > > { > > /* We walk from inner type to the outer types. If type we see is > > already too large to be part of type1, terminate the search. */ > > - int cmp = compare_type_sizes (type1, TREE_TYPE (*refp)); > > + int cmp = compare_type_sizes (type1, TREE_TYPE (ref)); > > > > if (cmp < 0 > > && (!end_struct_ref1 > > || compare_type_sizes (TREE_TYPE (end_struct_ref1), > > - TREE_TYPE (*refp)) < 0)) > > + TREE_TYPE (ref)) < 0)) > > break; > > /* If types may be of same size, see if we can decide about their > > equality. */ > > if (cmp == 0) > > { > > - same_p2 = same_type_for_tbaa (TREE_TYPE (*refp), type1); > > + same_p2 = same_type_for_tbaa (TREE_TYPE (ref), type1); > > if (same_p2 == 1) > > break; > > /* In case we can't decide whether types are same try to > > @@ -960,9 +965,9 @@ aliasing_component_refs_p (tree ref1, > > if (same_p2 == -1) > > maybe_match = true; > > } > > - if (!handled_component_p (*refp)) > > + if (!handled_component_p (ref)) > > break; > > - refp = &TREE_OPERAND (*refp, 0); > > + ref = TREE_OPERAND (ref, 0); > > } > > if (same_p2 == 1) > > { > > @@ -977,13 +982,13 @@ aliasing_component_refs_p (tree ref1, > > if (TREE_CODE (TREE_TYPE (base1)) == ARRAY_TYPE > > && (!TYPE_SIZE (TREE_TYPE (base1)) > > || TREE_CODE (TYPE_SIZE (TREE_TYPE (base1))) != INTEGER_CST > > - || (*refp == base2 && !ref2_is_decl))) > > + || (ref == base2 && !ref2_is_decl))) > > { > > ++alias_stats.aliasing_component_refs_p_may_alias; > > return true; > > } > > > > - get_ref_base_and_extent (*refp, &offadj, &sztmp, &msztmp, &reverse); > > + get_ref_base_and_extent (ref, &offadj, &sztmp, &msztmp, &reverse); > > offset2 -= offadj; > > get_ref_base_and_extent (base1, &offadj, &sztmp, &msztmp, &reverse); > > offset1 -= offadj; > > @@ -1005,28 +1010,28 @@ aliasing_component_refs_p (tree ref1, > > || (end_struct_ref1 > > && compare_type_sizes (TREE_TYPE (end_struct_ref1), type1) <= 0)) > > { > > - refp = &ref1; > > + tree ref = ref1; > > while (true) > > { > > - int cmp = compare_type_sizes (type2, TREE_TYPE (*refp)); > > + int cmp = compare_type_sizes (type2, TREE_TYPE (ref)); > > if (cmp < 0 > > && (!end_struct_ref2 > > || compare_type_sizes (TREE_TYPE (end_struct_ref2), > > - TREE_TYPE (*refp)) < 0)) > > + TREE_TYPE (ref)) < 0)) > > break; > > /* If types may be of same size, see if we can decide about their > > equality. */ > > if (cmp == 0) > > { > > - same_p1 = same_type_for_tbaa (TREE_TYPE (*refp), type2); > > + same_p1 = same_type_for_tbaa (TREE_TYPE (ref), type2); > > if (same_p1 == 1) > > break; > > if (same_p1 == -1) > > maybe_match = true; > > } > > - if (!handled_component_p (*refp)) > > + if (!handled_component_p (ref)) > > break; > > - refp = &TREE_OPERAND (*refp, 0); > > + ref = TREE_OPERAND (ref, 0); > > } > > if (same_p1 == 1) > > { > > @@ -1036,13 +1041,13 @@ aliasing_component_refs_p (tree ref1, > > if (TREE_CODE (TREE_TYPE (base2)) == ARRAY_TYPE > > && (!TYPE_SIZE (TREE_TYPE (base2)) > > || TREE_CODE (TYPE_SIZE (TREE_TYPE (base2))) != INTEGER_CST > > - || (*refp == base1 && !ref2_is_decl))) > > + || (ref == base1 && !ref2_is_decl))) > > { > > ++alias_stats.aliasing_component_refs_p_may_alias; > > return true; > > } > > > > - get_ref_base_and_extent (*refp, &offadj, &sztmp, &msztmp, &reverse); > > + get_ref_base_and_extent (ref, &offadj, &sztmp, &msztmp, &reverse); > > offset1 -= offadj; > > get_ref_base_and_extent (base2, &offadj, &sztmp, &msztmp, &reverse); > > offset2 -= offadj; > > > > -- > Richard Biener <rguenther@suse.de> > SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; > GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)
> > Ah, no, of course not. I guess the early out here should be a > "ignore this match" instead. Here is updated patch with a testcase I have re-tested on x86_64-linux and comitted. There are still few divergences left, I will debug them now. Again the testcase has extra wrapper in struct d to prevent oracle giving up earlier via overlaping_range_refs. Honza * tree-ssa-alias.c (nonoverlapping_component_refs_p): Do not give up on bitfields; continue searching for different refs appearing later. * gcc.dg/tree-ssa/alias-access-path-6.c: New testcase. Index: tree-ssa-alias.c =================================================================== --- tree-ssa-alias.c (revision 272510) +++ tree-ssa-alias.c (working copy) @@ -1350,19 +1350,16 @@ nonoverlapping_component_refs_p (const_t same. */ if (DECL_BIT_FIELD_REPRESENTATIVE (fieldx) == fieldy || DECL_BIT_FIELD_REPRESENTATIVE (fieldy) == fieldx) - { - ++alias_stats.nonoverlapping_component_refs_p_may_alias; - return false; - } + ; /* Different fields of the same record type cannot overlap. ??? Bitfields can overlap at RTL level so punt on them. */ - if (DECL_BIT_FIELD (fieldx) && DECL_BIT_FIELD (fieldy)) + else if (DECL_BIT_FIELD (fieldx) && DECL_BIT_FIELD (fieldy)) + ; + else { - ++alias_stats.nonoverlapping_component_refs_p_may_alias; - return false; + ++alias_stats.nonoverlapping_component_refs_p_no_alias; + return true; } - ++alias_stats.nonoverlapping_component_refs_p_no_alias; - return true; } } if (TYPE_UID (typex) < TYPE_UID (typey)) Index: testsuite/gcc.dg/tree-ssa/alias-access-path-6.c =================================================================== --- testsuite/gcc.dg/tree-ssa/alias-access-path-6.c (nonexistent) +++ testsuite/gcc.dg/tree-ssa/alias-access-path-6.c (working copy) @@ -0,0 +1,29 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized" } */ + +/* This tests that nonoveralpping_component_refs does not give up + on field delcs and continues looking to find mismatch between + a1 and a2. */ +struct a { + int a:3; + int b:3; +}; +struct b {struct a a1,a2;}; +struct c {struct b b[10];} *cptr; +struct d {struct c c;} *dptr; +int +test(int i,int j) +{ + cptr->b[i].a1.a=0; + dptr->c.b[j].a2.b=1; + return cptr->b[i].a1.a; +} +int +test2(int i,int j) +{ + cptr->b[i].a1.a=1; + dptr->c.b[j].a1.a=0; + return cptr->b[i].a1.a; +} +/* { dg-final { scan-tree-dump-times "return 0" 1 "optimized"} } */ +/* { dg-final { scan-tree-dump-not "return 1" "optimized"} } */
Index: tree-ssa-alias.c =================================================================== --- tree-ssa-alias.c (revision 272283) +++ tree-ssa-alias.c (working copy) @@ -100,6 +100,8 @@ static struct { unsigned HOST_WIDE_INT call_may_clobber_ref_p_no_alias; unsigned HOST_WIDE_INT aliasing_component_refs_p_may_alias; unsigned HOST_WIDE_INT aliasing_component_refs_p_no_alias; + unsigned HOST_WIDE_INT nonoverlapping_component_refs_p_may_alias; + unsigned HOST_WIDE_INT nonoverlapping_component_refs_p_no_alias; } alias_stats; void @@ -124,7 +126,13 @@ dump_alias_stats (FILE *s) alias_stats.call_may_clobber_ref_p_no_alias, alias_stats.call_may_clobber_ref_p_no_alias + alias_stats.call_may_clobber_ref_p_may_alias); - fprintf (s, " aliasing_component_ref_p: " + fprintf (s, " nonoverlapping_component_refs_p: " + HOST_WIDE_INT_PRINT_DEC" disambiguations, " + HOST_WIDE_INT_PRINT_DEC" queries\n", + alias_stats.nonoverlapping_component_refs_p_no_alias, + alias_stats.nonoverlapping_component_refs_p_no_alias + + alias_stats.nonoverlapping_component_refs_p_may_alias); + fprintf (s, " aliasing_component_refs_p: " HOST_WIDE_INT_PRINT_DEC" disambiguations, " HOST_WIDE_INT_PRINT_DEC" queries\n", alias_stats.aliasing_component_refs_p_no_alias, @@ -1029,106 +1037,6 @@ aliasing_component_refs_p (tree ref1, return false; } -/* Return true if we can determine that component references REF1 and REF2, - that are within a common DECL, cannot overlap. */ - -static bool -nonoverlapping_component_refs_of_decl_p (tree ref1, tree ref2) -{ - auto_vec<tree, 16> component_refs1; - auto_vec<tree, 16> component_refs2; - - /* Create the stack of handled components for REF1. */ - while (handled_component_p (ref1)) - { - component_refs1.safe_push (ref1); - ref1 = TREE_OPERAND (ref1, 0); - } - if (TREE_CODE (ref1) == MEM_REF) - { - if (!integer_zerop (TREE_OPERAND (ref1, 1))) - return false; - ref1 = TREE_OPERAND (TREE_OPERAND (ref1, 0), 0); - } - - /* Create the stack of handled components for REF2. */ - while (handled_component_p (ref2)) - { - component_refs2.safe_push (ref2); - ref2 = TREE_OPERAND (ref2, 0); - } - if (TREE_CODE (ref2) == MEM_REF) - { - if (!integer_zerop (TREE_OPERAND (ref2, 1))) - return false; - ref2 = TREE_OPERAND (TREE_OPERAND (ref2, 0), 0); - } - - /* Bases must be either same or uncomparable. */ - gcc_checking_assert (ref1 == ref2 - || (DECL_P (ref1) && DECL_P (ref2) - && compare_base_decls (ref1, ref2) != 0)); - - /* Pop the stacks in parallel and examine the COMPONENT_REFs of the same - rank. This is sufficient because we start from the same DECL and you - cannot reference several fields at a time with COMPONENT_REFs (unlike - with ARRAY_RANGE_REFs for arrays) so you always need the same number - of them to access a sub-component, unless you're in a union, in which - case the return value will precisely be false. */ - while (true) - { - do - { - if (component_refs1.is_empty ()) - return false; - ref1 = component_refs1.pop (); - } - while (!RECORD_OR_UNION_TYPE_P (TREE_TYPE (TREE_OPERAND (ref1, 0)))); - - do - { - if (component_refs2.is_empty ()) - return false; - ref2 = component_refs2.pop (); - } - while (!RECORD_OR_UNION_TYPE_P (TREE_TYPE (TREE_OPERAND (ref2, 0)))); - - /* Beware of BIT_FIELD_REF. */ - if (TREE_CODE (ref1) != COMPONENT_REF - || TREE_CODE (ref2) != COMPONENT_REF) - return false; - - tree field1 = TREE_OPERAND (ref1, 1); - tree field2 = TREE_OPERAND (ref2, 1); - - /* ??? We cannot simply use the type of operand #0 of the refs here - as the Fortran compiler smuggles type punning into COMPONENT_REFs - for common blocks instead of using unions like everyone else. */ - tree type1 = DECL_CONTEXT (field1); - tree type2 = DECL_CONTEXT (field2); - - /* We cannot disambiguate fields in a union or qualified union. */ - if (type1 != type2 || TREE_CODE (type1) != RECORD_TYPE) - return false; - - if (field1 != field2) - { - /* A field and its representative need to be considered the - same. */ - if (DECL_BIT_FIELD_REPRESENTATIVE (field1) == field2 - || DECL_BIT_FIELD_REPRESENTATIVE (field2) == field1) - return false; - /* Different fields of the same record type cannot overlap. - ??? Bitfields can overlap at RTL level so punt on them. */ - if (DECL_BIT_FIELD (field1) && DECL_BIT_FIELD (field2)) - return false; - return true; - } - } - - return false; -} - /* qsort compare function to sort FIELD_DECLs after their DECL_FIELD_CONTEXT TYPE_UID. */ @@ -1154,40 +1062,66 @@ nonoverlapping_component_refs_p (const_t { if (!flag_strict_aliasing || !x || !y - || TREE_CODE (x) != COMPONENT_REF - || TREE_CODE (y) != COMPONENT_REF) - return false; + || !handled_component_p (x) + || !handled_component_p (y)) + { + ++alias_stats.nonoverlapping_component_refs_p_may_alias; + return false; + } auto_vec<const_tree, 16> fieldsx; - while (TREE_CODE (x) == COMPONENT_REF) + while (handled_component_p (x)) { - tree field = TREE_OPERAND (x, 1); - tree type = DECL_FIELD_CONTEXT (field); - if (TREE_CODE (type) == RECORD_TYPE) - fieldsx.safe_push (field); + if (TREE_CODE (x) == COMPONENT_REF) + { + tree field = TREE_OPERAND (x, 1); + tree type = DECL_FIELD_CONTEXT (field); + if (TREE_CODE (type) == RECORD_TYPE) + fieldsx.safe_push (field); + } x = TREE_OPERAND (x, 0); } if (fieldsx.length () == 0) - return false; + { + ++alias_stats.nonoverlapping_component_refs_p_may_alias; + return false; + } auto_vec<const_tree, 16> fieldsy; - while (TREE_CODE (y) == COMPONENT_REF) + while (handled_component_p (y)) { - tree field = TREE_OPERAND (y, 1); - tree type = DECL_FIELD_CONTEXT (field); - if (TREE_CODE (type) == RECORD_TYPE) - fieldsy.safe_push (TREE_OPERAND (y, 1)); + if (TREE_CODE (y) == COMPONENT_REF) + { + tree field = TREE_OPERAND (y, 1); + tree type = DECL_FIELD_CONTEXT (field); + if (TREE_CODE (type) == RECORD_TYPE) + fieldsy.safe_push (TREE_OPERAND (y, 1)); + } y = TREE_OPERAND (y, 0); } if (fieldsy.length () == 0) - return false; + { + ++alias_stats.nonoverlapping_component_refs_p_may_alias; + return false; + } /* Most common case first. */ if (fieldsx.length () == 1 && fieldsy.length () == 1) - return ((DECL_FIELD_CONTEXT (fieldsx[0]) - == DECL_FIELD_CONTEXT (fieldsy[0])) - && fieldsx[0] != fieldsy[0] - && !(DECL_BIT_FIELD (fieldsx[0]) && DECL_BIT_FIELD (fieldsy[0]))); + { + if ((DECL_FIELD_CONTEXT (fieldsx[0]) + == DECL_FIELD_CONTEXT (fieldsy[0])) + && fieldsx[0] != fieldsy[0] + && !(DECL_BIT_FIELD (fieldsx[0]) && DECL_BIT_FIELD (fieldsy[0]))) + { + ++alias_stats.nonoverlapping_component_refs_p_no_alias; + return true; + } + else + { + ++alias_stats.nonoverlapping_component_refs_p_may_alias; + return false; + } + } if (fieldsx.length () == 2) { @@ -1215,19 +1149,26 @@ nonoverlapping_component_refs_p (const_t if (typex == typey) { /* We're left with accessing different fields of a structure, - no possible overlap. */ + no possible overlap. + If we can not disambiguate, do not give up and try to continue: + it is possible that there are multiple matching types on the + access patch and each of them may result in disambiguation. */ if (fieldx != fieldy) { /* A field and its representative need to be considered the same. */ if (DECL_BIT_FIELD_REPRESENTATIVE (fieldx) == fieldy || DECL_BIT_FIELD_REPRESENTATIVE (fieldy) == fieldx) - return false; + ; /* Different fields of the same record type cannot overlap. ??? Bitfields can overlap at RTL level so punt on them. */ - if (DECL_BIT_FIELD (fieldx) && DECL_BIT_FIELD (fieldy)) - return false; - return true; + else if (DECL_BIT_FIELD (fieldx) && DECL_BIT_FIELD (fieldy)) + ; + else + { + ++alias_stats.nonoverlapping_component_refs_p_no_alias; + return true; + } } } if (TYPE_UID (typex) < TYPE_UID (typey)) @@ -1245,10 +1186,11 @@ nonoverlapping_component_refs_p (const_t } while (1); + ++alias_stats.nonoverlapping_component_refs_p_may_alias; return false; } /* Return true if two memory references based on the variables BASE1 and BASE2 constrained to [OFFSET1, OFFSET1 + MAX_SIZE1) and [OFFSET2, OFFSET2 + MAX_SIZE2) may alias. REF1 and REF2 @@ -1271,11 +1212,41 @@ decl_refs_may_alias_p (tree ref1, tree b if (!ranges_maybe_overlap_p (offset1, max_size1, offset2, max_size2)) return false; + /* Access path oracle below needs to know both references. */ + if (!ref1 || !ref2) + return true; + + /* If the decl is accessed via a MEM_REF, reconstruct the base + we can use for TBAA. */ + tree dbase1 = ref1; + + while (handled_component_p (dbase1)) + dbase1 = TREE_OPERAND (dbase1, 0); + if (TREE_CODE (dbase1) == MEM_REF + || TREE_CODE (dbase1) == TARGET_MEM_REF) + { + tree ptrtype1 = TREE_TYPE (TREE_OPERAND (dbase1, 1)); + /* If first reference is view-converted, give up now. */ + if (same_type_for_tbaa (TREE_TYPE (dbase1), TREE_TYPE (ptrtype1)) != 1) + return true; + } + + tree dbase2 = ref2; + + while (handled_component_p (dbase2)) + dbase2 = TREE_OPERAND (dbase2, 0); + if (TREE_CODE (dbase2) == MEM_REF + || TREE_CODE (dbase2) == TARGET_MEM_REF) + { + tree ptrtype2 = TREE_TYPE (TREE_OPERAND (dbase2, 1)); + /* If second reference is view-converted, give up now. */ + if (same_type_for_tbaa (TREE_TYPE (dbase2), TREE_TYPE (ptrtype2)) != 1) + return true; + } + /* For components with variable position, the above test isn't sufficient, so we disambiguate component references manually. */ - if (ref1 && ref2 - && handled_component_p (ref1) && handled_component_p (ref2) - && nonoverlapping_component_refs_of_decl_p (ref1, ref2)) + if (nonoverlapping_component_refs_p (ref1, ref2)) return false; return true;
Hi, this patch removes nonoverlapping_component_refs_of_decl_p and replaces it by nonoverlapping_component_refs_p. Those functions are basically equivalent, however the first assumes that the outer type of memory access is type of decl which is not true in the gimple memory model. nonoverlapping_component_refs_p may wind up more expensive by doing quick sort. If the outer types match, we could use the same walk as in nonoverlapping_component_refs_of_decl_p (and I can easilly implement it in there saing some qsort for the indirect oracles too) but since nonoverlapping_component_refs_p already handles common case of shallow access paths and does not show in profiles I am not convinced it is worth the code duplication. I think the oracle is mostly redudnant with aliasing_component_refs - in fact I have troubles constructing testcases it would catch that the second would not except for expliting the -1 corner cases of same_types_for_tbaa, so I decided to leave this for later. The only case where this function is stronger is the case of match of types in the middle of access path. This require some non-const indexed array refs to exploit it. Perhaps we could integrate both tests and do nonoverlapping_component_refs_p only if one of many handled components walks we do earlier concludes that it has chance to suceed. Point of this patch is however to fix the code duplication and also add missing checks for view converts - I don't see how it would be safe to use the access paths here when it is not in the other two oracles. I implemented assert checking that whenever nonoverlapping_component_refs_of_decl_p so does nonoverlapping_component_refs_p and found two issues: 1) the first does not give up seeing handled component that is not COMPONENT_REF while other does. This prevents it to disambiguate for example foo.bar.array[1]; from foo.baz.array[1]; where bar and baz are fields of structure foo. This is valid 2) It gives up upon seeing bitfield while it may disambiguate later in the patch like. foo.bar.bitfield1; from foo.baz.bitfield2; Here we can not tell that bitfield1 is different from bitfield2 (at least we do not do so currently claiming that it is not different for RTL, but I think in that case we should not see bitfield in the access path) but we can still disambiguate based on bar/baz. Patch changes Alias oracle query stats: refs_may_alias_p: 3021539 disambiguations, 3321136 queries ref_maybe_used_by_call_p: 7118 disambiguations, 3047133 queries call_may_clobber_ref_p: 817 disambiguations, 817 queries aliasing_component_ref_p: 2050 disambiguations, 19908 queries TBAA oracle: 1419961 disambiguations 2916692 queries 555158 are in alias set 0 575103 queries asked about the same object 0 queries asked about the same alias set 0 access volatile 252874 are dependent in the DAG 113596 are aritificially in conflict with void * to Alias oracle query stats: refs_may_alias_p: 3021521 disambiguations, 3321121 queries ref_maybe_used_by_call_p: 7118 disambiguations, 3047115 queries call_may_clobber_ref_p: 817 disambiguations, 817 queries nonoverlapping_component_refs_p: 25 disambiguations, 83528 queries aliasing_component_refs_p: 2050 disambiguations, 19908 queries TBAA oracle: 1419961 disambiguations 2916690 queries 555158 are in alias set 0 575103 queries asked about the same object 0 queries asked about the same alias set 0 access volatile 252872 are dependent in the DAG 113596 are aritificially in conflict with void * So at least for tramp3d nonoverlapping_component_refs_p doesn't do any miracles. There are fewer disambiguations caused by the new view convert verification. It however also causes: FAIL: gcc.dg/vect/pr66142.c -flto -ffat-lto-objects scan-tree-dump-times vect "vectorized 1 loops in function" 1 FAIL: gcc.dg/vect/pr66142.c scan-tree-dump-times vect "vectorized 1 loops in function" 1 Test does: struct A { float x, y; }; struct B { struct A t, w; }; static inline float bar (const struct B *x) { struct A r; float b, c, d; r.x = x->t.x; r.y = x->t.y; which gets inlined to void foo (float *a, float *b, float *c) { int i; float z = 0.0f; float u = *a; #pragma omp simd for (i = 0; i < 32; i++) { float x = b[i]; float y = c[i]; struct B r; r.t.x = 1.0f; r.t.y = u; r.w.x = x; r.w.y = y; z += bar (&r); } *a = z; } SIMD code turns struct B r into array of size 32. The first difference is fre1. Here we give up on the oracle because of: @@ -488,15 +505,10 @@ D.2200[_20].t.y = u_13; D.2200[_20].w.x = x_22; D.2200[_20].w.y = y_24; - _30 = MEM <struct B[32]> [(const struct B *)&D.2200][_20].t.x; - _34 = MEM <struct B[32]> [(const struct B *)&D.2200][_20].t.y; - _35 = MEM <struct B[32]> [(const struct B *)&D.2200][_20].w.x; - _36 = _30 * _35; - _38 = y_24 * _34; - b_39 = _36 + _38; - _40 = _30 * _30; - _41 = b_39 + _40; - _42 = _34 * _34; + _38 = u_13 * y_24; + b_39 = x_22 + _38; + _41 = b_39 + 1.0e+0; + _42 = u_13 * u_13; It is truly silly to not optimize this, but there is a type mismatch between "struct B[32]" and "struct B" which we perceive as a view convert (same_type_for_tbaa_p return false which is bit ridiculout because precisely this case could be supported). (We would miss this if struct B was dynamically allocated previously too.) However in general we could run into this scenario since the type mismatch is a result of forwprop1 handling D.2200[_20].t.x = 1.0e+0; D.2200[_20].t.y = u_13; D.2200[_20].w.x = x_22; D.2200[_20].w.y = y_24; _29 = &D.2200[_20]; _30 = MEM[(const struct B *)_29].t.x; _34 = MEM[(const struct B *)_29].t.y; _35 = MEM[(const struct B *)_29].w.x; _36 = _30 * _35; _37 = MEM[(const struct B *)_29].w.y; So if there was outer struct, then the view convert would indeed happen. Any ideas how to solve this? I guess one option would be to delay such lossy forward subtitutions for one of later forwprops/PRE since for most of SSA optimizers the earlier sequence should be transparent enough and there is ahope that one can put constant in. Shall I XFAIL the testcase? Bootstrapped/regtested x86_64-linux, OK? Honza * tree-ssa-alias.c (alias_stats): Add nonoverlapping_component_refs_p_may_alias and nonoverlapping_component_refs_p_no_alias. (dump_alias_stats): Dump it. (nonoverlapping_component_refs_of_decl_p): Remove. (nonoverlapping_component_refs_p): Walk all handled components; do not give up on diambiguation on first failed match. (decl_refs_may_alias_p): Watch for view converts; use nonoverlapping_component_refs_of_decl_p.