[PATCH] D84589: [CodeMoverUtils] Add optional data dependence checks using Alias Analysis

Whitney Tsang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Jul 25 12:44:08 PDT 2020


Whitney added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/CodeMoverUtils.cpp:230
 
+bool isDependenceSafe(Instruction &I, DependenceInfo &DI,
+                      SmallPtrSet<Instruction *, 10> InstsToCheck) {
----------------
Please add a description for each `isDependenceSafe()`. And declare them as `static`. 


================
Comment at: llvm/lib/Transforms/Utils/CodeMoverUtils.cpp:231
+bool isDependenceSafe(Instruction &I, DependenceInfo &DI,
+                      SmallPtrSet<Instruction *, 10> InstsToCheck) {
+  return llvm::none_of(InstsToCheck, [&DI, &I](Instruction *CurInst) {
----------------
`SmallPtrSet<Instruction *, 10>` -> `SmallPtrSetImpl<Instruction *>`

Same for the other `isDependenceSafe()`.


================
Comment at: llvm/lib/Transforms/Utils/CodeMoverUtils.cpp:258
+
+    if (!AA.isNoAlias(MemLoc, DestMemLoc))
+      return true;
----------------
check this at line 251.
```
if (AA.isNoAlias(MemLoc, DestMemLoc))
  return false;
```


================
Comment at: llvm/unittests/Transforms/Utils/CodeMoverUtilsTest.cpp:46
+  AAResults AA(TLI);
+  DataLayout DL("e-i64:64-f80:128-n8:16:32:64-S128");
+  BasicAAResult BAA(DL, *F, TLI, AC, &DT);
----------------
Does `DataLayout DL("");` work?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84589





More information about the llvm-commits mailing list