[PATCH] D46928: implantation example for new AliasAnalysis functions (getAddressesDistance + ModRefSameBuffer)

[Dávid Bolvanský via Phabricator via llvm-commits]

xbolva00 added inline comments.


================
Comment at: include/llvm/Analysis/BasicAliasAnalysis.h:152
 
+  /// Track common Buffer queries to guard against recursion.
+  typedef std::pair<BuffUseResult, Optional<int64_t>> MemDistancePair;
----------------
..common buffer queries...


================
Comment at: include/llvm/Analysis/BasicAliasAnalysis.h:153
+  /// Track common Buffer queries to guard against recursion.
+  typedef std::pair<BuffUseResult, Optional<int64_t>> MemDistancePair;
+  typedef SmallDenseMap<LocPair, MemDistancePair, 8> CommonBufferCacheTy;
----------------
Convert to more C++ish code via "using MyType = ..."?


================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:625
+static const Function *getParent(const Value *V) {
+	if (const Instruction *inst = dyn_cast<Instruction>(V))
+		return inst->getParent()->getParent();
----------------
isa<...> is enough here, no?

You can use still getParent function with "V"


================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:803
+  // Figure out what objects these things are pointing to if we can.
+  if (O1 == nullptr)
+    O1 = GetUnderlyingObject(V1, DL, MaxLookupSearchDepth);
----------------
if (!ptr)...


================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:806
+
+  if (O2 == nullptr)
+    O2 = GetUnderlyingObject(V2, DL, MaxLookupSearchDepth);
----------------
Same as above


================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:878
+
+  if ((O1 == O2)) {
+    const GEPOperator *GV1 = dyn_cast<GEPOperator>(V1);
----------------
Why extra if ((...))?


================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:1767
+	// NoAlias / MustAlias. Otherwise, returns MayAlias.
+	for (unsigned i = 1, e = V1Srcs.size(); i != e; ++i) {
+		Value *V = V1Srcs[i];
----------------
Is this loop correct? Why start at 1? And why not "i < e"?

Anyway, range based iteration is nicer, you should chrck if possible here

for (Value *V : V1Srcs) { ... }


Repository:
  rL LLVM

https://reviews.llvm.org/D46928