[PATCH] D27034: [AliasAnalysis] Teach BasicAA about memcpy.

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added a comment.

I added some minor stylistic comments inline.



================
Comment at: llvm/trunk/lib/Analysis/BasicAliasAnalysis.cpp:783
+  // no-aliases the other.
+  if (auto *Inst = dyn_cast<MemCpyInst>(CS.getInstruction())) {
+    AliasResult SrcAA, DestAA;
----------------
This shadows the `Inst` variable in the outer scope, which can potentially be confusing.  How about `auto *MCI = dyn_cast<...>(...))`?


================
Comment at: llvm/trunk/lib/Analysis/BasicAliasAnalysis.cpp:786
+
+    if ((SrcAA = getBestAAResults().alias(MemoryLocation::getForSource(Inst),
+                                          Loc)) == MustAlias)
----------------
How about

```
auto SrcAA = getBestAAResults().alias(...)
if (SrcAA == MustAlias)
  ...
auto DestAA = getBestAAResults().alias(...)
if (DestAA == MustAlias)
  ...
```

That seems less cluttered.


================
Comment at: llvm/trunk/lib/Analysis/BasicAliasAnalysis.cpp:796
+    // It's also possible for Loc to alias both src and dest, or neither.
+    ModRefInfo rv = MRI_NoModRef;
+    if (SrcAA != NoAlias)
----------------
LLVM coding style forbids variables with lower case names.  `ModRefInfo MRI` would be more idiomatic.


Repository:
  rL LLVM

https://reviews.llvm.org/D27034