Re: Add a mem_alias_size helper class

[Richard Sandiford <richard dot sandiford at arm dot com>]

Eric Botcazou <ebotcazou@adacore.com> writes:
>> alias.c encodes memory sizes as follows:
>> 
>> size > 0: the exact size is known
>> size == 0: the size isn't known
>> size < 0: the exact size of the reference itself is known,
>>   but the address has been aligned via AND.  In this case
>>   "-size" includes the size of the reference and the worst-case
>>   number of bytes traversed by the AND.
>> 
>> This patch wraps this up in a helper class and associated
>> functions.  The new routines fix what seems to be a hole
>> in the old logic: if the size of a reference A was unknown,
>> offset_overlap_p would assume that it could conflict with any
>> other reference B, even if we could prove that B comes before A.
>> 
>> The fallback CONSTANT_P (x) && CONSTANT_P (y) case looked incorrect.
>> Either "c" is trustworthy as a distance between the two constants,
>> in which case the alignment handling should work as well there as
>> elsewhere, or "c" isn't trustworthy, in which case offset_overlap_p
>> is unsafe.  I think the latter's true; AFAICT we have no evidence
>> that "c" really is the distance between the two references, so using
>> it in the check doesn't make sense.
>> 
>> At this point we've excluded cases for which:
>> 
>> (a) the base addresses are the same
>> (b) x and y are SYMBOL_REFs, or SYMBOL_REF-based constants
>>     wrapped in a CONST
>> (c) x and y are both constant integers
>> 
>> No useful cases should be left.  As things stood, we would
>> assume that:
>> 
>>   (mem:SI (const_int X))
>> 
>> could overlap:
>> 
>>   (mem:SI (symbol_ref Y))
>> 
>> but not:
>> 
>>   (mem:SI (const (plus (symbol_ref Y) (const_int 4))))
>
> Frankly this seems to be an example of counter-productive C++ization: the 
> class doesn't provide any useful abstraction and the code gets obfuscated by 
> all the wrapper methods.  Moreover it's mixed with real changes so very hard 
> to review.  Can't you just fix what needs to be fixed first?

Sorry, I should have said, but this wasn't C++-ification for its own sake.
It was part of the changes to make modes and MEM_OFFSETs be runtime
invariants of the form a+bX for runtime X.  Making that change to modes
and MEM_OFFSETs meant that these alias sizes also become runtime invariants.
The classification above is then:

  size may be greater than 0: the exact size is known
  size must be equal to 0: the size is unknown
  size may be less than 0: -size is the maximum size including alignment

with the assumption that a and b cannot be such that a+bX>0 for some X
and <0 for other X.  So we were faced the prospect of having to change
every existing ==0, >0 and <0 test anyway.  The question was whether
to change them to "may be greater than 0?" etc. or change them to
something more mnemonic like "exact size known?".  The latter seemed
better.

That part in itself could be done using inline functions.  What the
class gives is that it also enforces statically that the restrictions
on a and b above hold, i.e. that a+bX is always ordered wrt 0.

Similarly, abs(a+bX) cannot be represented as a'+b'X for all a and b,
so abs() is not unconditionally computable on these runtime invariants.
The use of abs() on the encoded sizes would therefore also need either
a wrapper like "max_size" or a cut-&-paste change to cope with the fact
that abs() is always computable on these sizes.  Using the more mnemonic
"max_size" seemed better than preserving the use of "abs"; to me, abs
implies we have a negative displacement from an end point, whereas
really the sign bit is being used as a boolean to indicate inexactness.
Again, "max_size" could just be an inline function, but the class
enforces statically that abs() is computable.

We separated the patch out because it made the actual switch to support
runtime invariants almost mechanical.

Thanks,
Richard