Re: PATCH for MEM_IN_STRUCT_P confusion

[Mark Mitchell <mark at markmitchell dot com>]

>>>>> "Jeffrey" == Jeffrey A Law <law@hurl.cygnus.com> writes:

    Jeffrey> No, we should analyze the code and determine what the
    Jeffrey> correct action should be.  It's not that hard, and after
    Jeffrey> doing so I'm pretty sure your change is not safe.

I tried, but I failed.  I guess it was harder for me... :-(

    Jeffrey> Consider if loop_store_mems[i] contains a reference where
    Jeffrey> neither MEM_IN_STRUCT_P nor MEM_SCALAR_P is set.  If X as
    Jeffrey> the same address, but has MEM_IN_STRUCT_P set and uses a
    Jeffrey> wider mode, then we could replace the list entry with X.

    Jeffrey> Now the memory address in loop_store_mems looks like it
    Jeffrey> must be a structure, when previously it could have been
    Jeffrey> either a structure or a scalar.  Which doesn't sound safe
    Jeffrey> to me -- the set of memory references the list conflicts
    Jeffrey> with shrinks in that case.

Right; good point.

    Jeffrey> What I think the code was supposed to do was avoid
    Jeffrey> creating a new entry on the loop_store_mems list when the
    Jeffrey> memory conflicts created by X were a strict superset of
    Jeffrey> those created by the existing entry in loop_store_mems.

Right.

    Jeffrey> ie, If we know the struct/scalar status of both, and X is
    Jeffrey> wider, then anything that conflicted with
    Jeffrey> loop_store_mems[i] would also conflict with X, so we can
    Jeffrey> replace loop_store_mems[i] with X.

Makes sense.

    Jeffrey> Something like this ought to do the trick:

    Jeffrey>     if (rtx_equal_p (XEXP (loop_store_mems[i], 0), XEXP
    Jeffrey> (x, 0)) && ((MEM_IN_STRUCT_P (x) && MEM_IN_STRUCT_P (x)
    Jeffrey> == MEM_IN_STRUCT_P (loop_store_mems[i])) || (MEM_SCALAR_P
    Jeffrey> (x) && MEM_SCALAR_P (x) == MEM_SCALAR_P
    Jeffrey> (loop_store_mems[i]))))

    Jeffrey> Do you agree?

Yes.  Will you make the change?  BTW, you could just write:

  MEM_IN_STRUCT_P (x) && MEM_IN_STRUCT_P (loop_store_mems[i])

instead of the equality.  In fact, why not write:

  MEM_IN_STRUCT_P (x) <= MEM_IN_STRUCT_P (loop_store_mems[i])
  && MEM_SCALAR_P (x) <= MEM_SCALAR_P (loop_store_mems[i])

This nicely expresses the idea that X should be no more constrained
that loop_store_mems[i].

-- 
Mark Mitchell 			mark@markmitchell.com
Mark Mitchell Consulting	http://www.markmitchell.com