[llvm-commits] [PATCH] Make dependency DAG construction to use AA

[Andrew Trick]

On May 8, 2012, at 7:12 PM, Hal Finkel <hfinkel at anl.gov> wrote:

>> +  int64_t MinOffset = std::min(MMOa->getOffset(), MMOb->getOffset());
>> +  int64_t Overlapa = MMOa->getSize() + MMOa->getOffset() - MinOffset;
>> +  int64_t Overlapb = MMOb->getSize() + MMOb->getOffset() - MinOffset;
>> 
>> It took me a while to figure out what you're doing and why. We can
>> hide this behind an API eventually (I can do it post checkin), but
>> for now can you add comments with any cases you could think of?
>> Please also comment that it's taken from DAGCombiner. Apparently
>> DAGCombiner thinks it's a good idea to normalize offsets relative to
>> each other by subtracting MinOffset. I don't understand why this is a
>> good idea and can't prove to myself that it is safe. For example,
>> what if we have an IR-level offset from the same base pointer smaller
>> than the MachineOperand offset? We would seem to miss the overlap.
>> Maybe Dan can comment on this.
>> 
>> Hal also mentioned negative offsets, but it looks to me like that
>> would work assuming positive offsets work!
> 
> I was actually concerned about one positive offset and one negative.
> Based on your second e-mail, I'll assume that this is fine too.
> 
> For example:
> A: p + 2 (size = 4)
> B: p - 8 (size = 2)
> Here, MinOffset will be -6, and so Overlapa will be 4 + 2 - 8 = -2.
> That does not seem right, I thought that the size in AA::Location would
> need to be positive.


Size definitely needs to be positive.

I may not be reading your example right. I get:

OffsetA = 2; SizeA = 4
OffsetB = -8; SizeB = 2
MinOffset = min(2, -8) = -8
OverlapA = 4 + 2 + 8 = 14
OverlapB = 2 - 8 + 8 = 2

In general, the overlap with min offset is simply the size. The other overlap's offset is always signed greater than min offset, to the result of subtraction is positive. As Dan said, "assuming no overflow" :)

I think more comments could help.

-Andy
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120508/5af76432/attachment.html>