This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Incorrect bitfield aliasing with Tree SSA
On 6/17/07, Richard Kenner <kenner@vlsi1.ultra.nyu.edu> wrote:
> The "alias set of the field" would be that of its type, for example
> 'int'. Your patch will cause any indirect memory access through a
> pointer to 'int' to be deemed conflicting with any access to the
> field, which is precisely what DECL_NONADDRESSABLE_P is meant to
> avoid.
You are not answering my question of how the alias set of the field is
ever used in RTL if the purpose of the flag is *not* to use it (but
use its parent).
I read all the way to the end of this thread in the hope that things would be
clarified by the time I got back online. It's a little clearer, but there
was still no direct answer to that question and I think it's important that
there be.
The *whole point* here is that the "alias set of the field" is *not* used
in that case: it's not supposed to be.
Which is simply wrong.
I use the term in quotes because I don't really like to think of fields as
*having* an alias set. *Expressions*, more precisely the subset of those
that reference memory, are what have alias sets.
No. Types have alias sets. That's why it's type based aliasing.
Field has a type, and thus, the field should have an alias. set
That's why using DECL_NONADDRESSABLE_P is so wrong. It has nothing to
do with the subset relationship of the types, and yet you are using to
change the type based alias sets in a way that breaks the middle end,
when multiple clear and clean solutions are available that won't break
things.
get_alias_set does it
precisely right and the relevant function is component_uses_parent_alias_set.
component_uses_parent_alias_set is a hack around the fact that you
want a pointer to the first field of the structure to alias the field,
and you want stores to the entire structure to alias the field, but
you want nothing else to alias the field.
In fact, looking at the code, someone gave up when this failed in some
cases, and just made these accesses always use alias set 0, likely
because it kept removing the stores/loads!
THis "field uses parent component alias set" is not the correct way to
achieve the goal of using DECL_NONADDRESSABLE_P, as these bugs prove.
The correct way, is either
1. Record the type as a subset of the structure, as the types actually
say is the relationship, and use DECL_NON_ADDRESSABLE_P on a
per-access basis
2. Create a new alias set for the type, and record it as a subset of
the structure, as Adam suggested.
These are the only two sane options, IMHO
What we have now, where we try to use the alias set of the parent
component, is not right and generates wrong answers, as you've seen.
All i've seen so far besides adam's patches are attempts to hack
around this solution by trying to fool the points-to analyzer's type
pruning into letting this happen.
I've reiterated this position multiple times. I simply disagree that
what we have now is going to be able to be made to work without more
hacks, and I will not approve patches that I do not believe will lead
our compiler in a good direction.
HTH,
Dan