[PATCH] D9401: llvm.noalias - The AA implementaton

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D9401#833714, @jeroen.dobbelaere wrote:

> Hi Hal, I found an issue with this part when analyzing two pointers that are 'derived from each other', but are used in different loops.
>  Following code would go wrong:
>
>   int* restrict p=malloc(sizeof(int)*20);
>   int i;
>   for (i=0; i<20; ++i) {p[i]=i; }
>   for (i=0; i<20; ++i) { if (p[i] != i) should_never_happen(); }
>   
>
> Do you think that the proposed fix is ok ?


I don't yet understand what's wrong. Can you provide a full test case? Locally, the results on this look fine.

FYI: There is currently some problem because the current patchset miscompiles MultiSource/Benchmarks/tramp3d-v4.



================
Comment at: lib/Analysis/ScopedNoAliasAA.cpp:454
+  DEBUG(dbgs() << "SNA: B does not derive from the compatible set!\n");
+
+  // Note: This can be removed when legacy-pass-manager support is removed;
----------------
jeroen.dobbelaere wrote:
> We need an extra check here to see if APtr is directly depending on BPtr.  Something like following code should do the trick:
> 
>   {
>     // Check if A depends directly on B
>     SmallVector<Value *, 8> AObjs;
>     GetUnderlyingObjects(const_cast<Value*>(APtr), AObjs, DL, nullptr, 0, nullptr);
>     DEBUG(dbgs() << "SNA: underlying objects:\n");
>   #ifndef NDEBUG
>     for (auto *A : AObjs)
>       DEBUG(dbgs() << "\t" << *A << "\n");
>     DEBUG(dbgs() << "\n"); 
>   #endif
>     if (CSB) {
>       for (Value* Arg : CSB.args())
>         if (llvm::is_contained(AObjs, Arg)) {
>           DEBUG(dbgs() << "SNA : A depends directly on B:"; Arg->dump());
> 
>           return false;
>         }
>     } else if (llvm::is_contained(AObjs, BPtr)) {
>       DEBUG(dbgs() << "SNA : A depends directly on B:"; BPtr->dump());
>       return false;
>     }
>   }
I don't see why this should be needed. If A is derived from B, and A is also derived from some some noalias intrinsic N, then either B is derived from N or N is derived from B. The former case is one for which we explicitly check (it checks whether B is derived from the same intrinsic, or any other compatible one) and the latter case correctly returns noalias.



https://reviews.llvm.org/D9401