[PATCH] D126533: [LAA] Relax pointer dependency with runtime pointer checks

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:1698
+    if (APtr != BPtr && (SrcAffine || SinkAffine))
+      FoundNonConstantDistanceDependence = true;
+
----------------
The variable  seems misnamed now, for the case here it seems like the distance is uncomputable, right?


================
Comment at: llvm/test/Analysis/LoopAccessAnalysis/pointer-phis.ll:210
+; CHECK-NEXT:  Memory dependences are safe with run-time checks
+;
 entry:
----------------
Why drop the check lines for the runtime checks?



================
Comment at: llvm/test/Analysis/LoopAccessAnalysis/pointer-phis.ll:252
+; CHECK-NEXT:      Memory dependences are safe with run-time checks
+; CHECK-NEXT:      Dependences:
+;
----------------
Why drop the check lines for the runtime checks?


================
Comment at: llvm/test/Transforms/LoopVectorize/X86/illegal-parallel-loop-uniform-write.ll:146
 
 ; Here we can see the vectorizer does the mem dep checks and decides it is
 ; unsafe to vectorize.
----------------
this comment seems out of date now?


================
Comment at: llvm/test/Transforms/LoopVectorize/global_alias.ll:781
+; CHECK: vector.memcheck
+; CHECK: add nsw <4 x i32>
 ; CHECK: ret
----------------
This seems like a regression, as here we have a case where the index depends on a load in the loop, right? Same for the change below.


================
Comment at: llvm/test/Transforms/LoopVectorize/memory-dep-remarks.ll:274
-
-define void @test_unknown_dep(i64 %n, i32* nocapture %A) !dbg !214 {
-entry:
----------------
why remove the test instead of updating the check?


================
Comment at: llvm/test/Transforms/LoopVectorize/memory-dep-remarks.ll:367
-; YAML-NEXT: Name:            MissedDetails
-; YAML-NEXT: Function:        test_backwardVectorizableButPreventsForwarding
-; YAML-NEXT: Args:
----------------
Can this be kept?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126533/new/

https://reviews.llvm.org/D126533