[PATCH] D74691: [Attributor] add some pattern to containsCycle

[Stefanos Baziotis via Phabricator via llvm-commits]

baziotis added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2371
 // TODO: Replace with more efficent code
 static bool containsCycle(Function &F) {
+  bool noAnalysis = false;
----------------
I think you still need to pass `A`. Are you sure this compiles?


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2372
 static bool containsCycle(Function &F) {
-  SmallPtrSet<BasicBlock *, 32> Visited;
+  bool noAnalysis = false;
 
----------------
Caps on the name. Be sure to check the nearby code and use clang-format (it seems you have done so, just reminding).


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2380
+    noAnalysis = true;
+  }
+
----------------
No braces.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2385
+    const std::vector<BasicBlock *> &SCCBBs = *I;
+    // when noAnalysis available then check only if the SCC has loop or not
+    if (noAnalysis) {
----------------
Caps starting the sentence.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2387
+    if (noAnalysis) {
+      if (I.hasLoop()) return true;
+      continue;
----------------
`return` in a newline


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2391
+    // when current SCC has a single block with no loop continue to the next SCC
+    if (!I.hasLoop()) continue;
+
----------------
You can cache this as you use it 2 times.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2404-2407
+    if (L->getNumBlocks() > SCCBBs.size()) return true;
+    if (L->getNumBlocks() == SCCBBs.size()) {
+      if (!SE->getSmallConstantMaxTripCount(L)) return true;
+      continue;
----------------
You want to put those in a loop. You may need to get parent loop multiple times. Something like that.
```
while (L) {
  if (L is bigger)
    return true;
  if (they're equal) {
    if (there's no max trip count)
      return true;
    else
      continue;
  }
  L = getParent();
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74691