[PATCH] D10386: [LAA] Merge memchecks for accesses separated by a constant offset

[silviu.baranga at arm.com]

sbaranga added a comment.

Hi Adam,

In http://reviews.llvm.org/D10386#201172, @anemet wrote:

> In http://reviews.llvm.org/D10386#199603, @sbaranga wrote:
>
> > Further digging into this has revealed that we should only get into the case where we get the regression if we have an unknown dependency - and we should retry without dependencies. In this case we don't have DepCands so we can't use the current algorithm to group pointers. Do you agree with this?
>
>
> No, I don't think that's quite correct.
>
> If I understand the situation correctly, we have two accesses of the same underlying object with non-const distance.  So we give up dependence analysis with "LAA: Retrying with memory checks".
>
> The problem is that we can have unknown distance between pointers of the same underlying object also if the stride is unknown (look for the return Dependence::Unknown before the ShouldRetryWithRuntimeCheck in isDependent).  In this case we don't give up on dependence analysis so your flag would still be true.
>
> I would think that you should be able to create a testcase with three pointers on the same objects two with known stride and one without.  In this case too we would merge all three but the one with unknown stride to cause the same issue you discovered.
>
> If this is all correct, I still think the best way is the veto-ing algorithm I described originally.


Ok, that makes sense. I already have the updated veto-ing algorithm, but I'd like to add it under a different review instead of rolling everything back. Is that ok with you?

> 

> 

> > I've fixed the algorithm to not group pointers in this case (this should be inline with previous comments).

> 

> 

> Please don't forget to add the testcase that prompted the new version.


I'll commit a test case for this.

Thanks,
Silviu


Repository:
  rL LLVM

http://reviews.llvm.org/D10386