Set RTX_UNCHANGING is readonly_field_p when clearing

Richard Kenner kenner@vlsi1.ultra.nyu.edu
Mon Apr 21 22:01:00 GMT 2003


    Well, we know one of them; you just introduced it with your patch.
    Let's start by fixing that one.  

No, it's certainly not new with my patch.  There is existing code
in case INDIRECT_REF in expand_expr which does the same thing.  And I don't
think it even added any new occurrences.

    It doesn't seem hard to fix; you just have to avoid setting
    RTX_UNCHANGING_P on any field of a structure that isn't completely
    read-only.

No, that won't work: you're forgetting about nesting.

Consider:

	struct foo {const int f;};
	struct bar {int f1; struct foo f2};

	struct foo *p;
	struct bar x;

By what we currently do, and what you propose above, dereferences associated
with P should have RTX_UNCHANGING_P for both loads and stores, right?

But P might be pointing to x.f2!

Are references to X supposed to have RTX_UNCHANGING_P or not?

If you assign X from a pointer to struct bar using a BLKmode move
and *don't* have RTX_UNCHANGING_P set, *P won't be marked as conflicting
with it, since the *P references will be /u and those of X won't.
So the assignments must have /u.

You can construct the case in the opposite direction to show that loads
from X must *not* have /u.

But we went through this years ago when the code I mentioned above
was first added.


Suppose I have a struct foo with two readonly-fields.  So you agree
that dereferences to a pointer to that structure should be readonly, right?
Now suppose I have a struct bar with no readonly fields except that one
field is struct foo.

    If it's supposed to mean what the documentation says, than we should
    make sure that it does actually mean that.

I agree it would be nice to fix the bugs, but nobody knows how to even
find them, let alone fix them.

    On the other hand, maybe what the documentation says isn't a very good
    definition; in which case, we should change it.

This is a similar situation: nobody has been able to come up with a better
one either.

This is not a new problem and this patch does not change the situation
here at all.

    I don't think that's a very good proposal.  I would propose instead
    that we do it the other way around: that we use the knowledge of
    whether something is 'readonly' to refine the alias information by
    creating a separate alias set for each field that's set only once.

But that's the same problem: if there's field that's an aggregate and
initializing it involves first clearing it and then setting an inner
field (let's say all are readonly) is that "set only once" or not?  It's
the *exact* same problem we have now.



More information about the Gcc-patches mailing list