https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113787
--- Comment #18 from rguenther at suse dot de <rguenther at suse dot de> --- On Wed, 14 Feb 2024, hubicka at ucw dot cz wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113787 > > --- Comment #17 from Jan Hubicka <hubicka at ucw dot cz> --- > > > I guess PTA gets around by tracking points-to set also for non-pointer > > > types and consequently it also gives up on any such addition. > > > > It does. But note it does _not_ for POINTER_PLUS where it treats > > the offset operand as non-pointer. > > > > > I think it is ipa-prop.c::unadjusted_ptr_and_unit_offset. It accepts > > > pointer_plus expression, but does not look through POINTER_PLUS. > > > We can restrict it further, but tracking base pointer is quite useful, > > > so it would be nice to not give up completely. > > > > It looks like that function might treat that > > > > ADDR_EXPR <TARGET_MEM_REF <0B, ...>> > > > > as integer_zerop base. It does > > > > if (TREE_CODE (op) == ADDR_EXPR) > > { > > poly_int64 extra_offset = 0; > > tree base = get_addr_base_and_unit_offset (TREE_OPERAND (op, 0), > > &offset); > > if (!base) > > { > > base = get_base_address (TREE_OPERAND (op, 0)); > > if (TREE_CODE (base) != MEM_REF) > > break; > > offset_known = false; > > } > > else > > { > > if (TREE_CODE (base) != MEM_REF) > > break; > > > > with a variable offset we fall to the TREE_CODE (base) != MEM_REF > > and will have offset_known == true. Not sure what it does with > > the result though (it's not the address of a decl). > > > > This function seems to oddly special-case != MEM_REF ... (maybe > > it wants to hande DECL_P () as finishing? > > Hmm the function was definitely not written with TARGET_MEM_REF in mind, > since it was originally used for IPA passes only. > We basically want to handle stuff like > &decl->foo > or > &(ptr->foo) > In the second case we want to continue the SSA walk to hopefully work > out the origin of PTR. > ipa-modref then looks if the base pointer is derived from function > parameter or points to local or readonly memory to produce its summary. > > > > Note get_addr_base_and_unit_offset will return NULL for > > a TARGET_MEM_REF <&decl, ..., offset> but TARGET_MEM_REF > > itself if the base isn't an ADDR_EXPR, irrespective of whether > > the offset within it is constant or not. > > Hmm, interesting. I would expect it to interpret the emantics of TMR > and return base. > > > > Not sure if the above is a problem, but it seems the only caller > > will just call points_to_local_or_readonly_memory_p on the > > ADDR_EXPR where refs_local_or_readonly_memory_p via > > points_to_local_or_readonly_memory_p will eventually do > > > > /* See if memory location is clearly invalid. */ > > if (integer_zerop (t)) > > return flag_delete_null_pointer_checks; > > > > and that might be a problem. As said, we rely on > > ADDR_EXPR <TARGET_MEM_REF <...> > to be an address computation > > that's not subject to strict interpretation to allow IVOPTs > > doing this kind of optimization w/o introducing some kind of > > INTEGER_LEA <...>. I know that's a bit awkward but we should > > make sure this is honored by IPA as well. > > > > I'd say > > > > diff --git a/gcc/ipa-fnsummary.cc b/gcc/ipa-fnsummary.cc > > index 74c9b4e1d1e..45a770cf940 100644 > > --- a/gcc/ipa-fnsummary.cc > > +++ b/gcc/ipa-fnsummary.cc > > @@ -2642,7 +2642,8 @@ points_to_local_or_readonly_memory_p (tree t) > > return true; > > return !ptr_deref_may_alias_global_p (t, false); > > } > > - if (TREE_CODE (t) == ADDR_EXPR) > > + if (TREE_CODE (t) == ADDR_EXPR > > + && TREE_CODE (TREE_OPERAND (t, 0)) != TARGET_MEM_REF) > > return refs_local_or_readonly_memory_p (TREE_OPERAND (t, 0)); > > return false; > > } > > > > might eventually work? Alternatively a bit less aggressive like > > the following. > > > > diff --git a/gcc/ipa-fnsummary.cc b/gcc/ipa-fnsummary.cc > > index 74c9b4e1d1e..7c79adf6440 100644 > > --- a/gcc/ipa-fnsummary.cc > > +++ b/gcc/ipa-fnsummary.cc > > @@ -2642,7 +2642,9 @@ points_to_local_or_readonly_memory_p (tree t) > > return true; > > return !ptr_deref_may_alias_global_p (t, false); > > } > > - if (TREE_CODE (t) == ADDR_EXPR) > > + if (TREE_CODE (t) == ADDR_EXPR > > + && (TREE_CODE (TREE_OPERAND (t, 0)) != TARGET_MEM_REF > > + || TREE_CODE (TREE_OPERAND (TREE_OPERAND (t, 0), 0)) != > > INTEGER_CST)) > > return refs_local_or_readonly_memory_p (TREE_OPERAND (t, 0)); > > return false; > > } > > Yes, those both looks reasonable to me, perhaps less agressive would be > better. Note I didn't check if it helps the testcase .. > > > > A "nicer" solution might be to add a informational operand > > to TARGET_MEM_REF, representing the base pointer to be used for > > alias/points-to purposes. But if that's not invariant it might > > keep some otherwise unnecessary definition stmts live. > > Yep, I see that forcing extra IV to track original semantics would be > trouble here. I think that after iv-opts we should be done with more > fancy propagation across loops. > > However, to avoid ipa-modref summary degradation, perhaps scheduling the > pass before ivopts would make sense... We also have that other bug where store-merging breaks the IPA summary which gets prevailed in the late modref, so moving it around doesn't solve all of the IL related issues ...
