[PATCH] D108048: [DependenceAnalysis] Conservatively exit on type mismatch

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: llvm/include/llvm/Analysis/DependenceAnalysis.h:285
     /// FullDependence) with as much information as can be gleaned.
-    /// The flag PossiblyLoopIndependent should be set by the caller
-    /// if it appears that control flow can reach from Src to Dst
-    /// without traversing a loop back edge.
+    /// If PossiblyLoopIndependant is true will also test for intra-iteration
+    /// dependencies.
----------------
Maybe we should rename `PossiblyLoopIndependent` to something else. I don't see how "LoopIndependent" makes a statement about non-loop dependences.


================
Comment at: llvm/lib/Analysis/DependenceAnalysis.cpp:3558
 
+  // No common loops AND don't care about loop independent dependencies
+  if (!PossiblyLoopIndependent && CommonLevels == 0)
----------------



================
Comment at: llvm/lib/Analysis/DependenceAnalysis.cpp:3571-3574
+  // DependenceAnalysis isn't able to deal with different
+  // access widths. If access widths are the same, but alignment is
+  // smaller than the store size, accesses could overlap.
+  // Reject these cases.
----------------
`underlyingObjectsAlias` should return `PartialAlias` alias in such cases. That is, I think it still may return `MustAlias` if the address is the same but the size is different. In that case it should be sufficient to a compare just the sizes, but not the alignment


================
Comment at: llvm/lib/Analysis/DependenceAnalysis.cpp:3597-3598
+      !dyn_cast<GEPOperator>(SrcPtr) || !dyn_cast<GEPOperator>(DstPtr);
+  TypeSize SrcWriteSize = DL.getTypeStoreSize(GetMemOperandType(Src));
+  TypeSize DstWriteSize = DL.getTypeStoreSize(GetMemOperandType(Dst));
+
----------------
You could consider `llvm::MemoryLocation` for this. It abstracts over for LoadInst/StoreInst and also determines that access size.


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

https://reviews.llvm.org/D108048