[PATCH] D74691: [Attributor] Detect possibly unbounded cycles in functions

[Stefanos Baziotis via Phabricator via llvm-commits]

baziotis added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2388-2394
+  // Check whether there is Irreducible control then the function contains
+  // non-loop cycles.
+  using RPOTraversal = ReversePostOrderTraversal<const Function *>;
+  RPOTraversal FuncRPOT(&F);
+  if (containsIrreducibleCFG<const BasicBlock *, const RPOTraversal,
+                             const LoopInfo>(FuncRPOT, *LI))
+    return true;
----------------
baziotis wrote:
> omarahmed wrote:
> > baziotis wrote:
> > > omarahmed wrote:
> > > > baziotis wrote:
> > > > > As @jdoerfert mentioned, you can replace this with [[ https://github.com/llvm/llvm-project/blob/master/llvm/lib/Analysis/MustExecute.cpp#L487 | mayContainIrreducibleControl ]]
> > > > I found that the function is in mustExcute.cpp only and not available in the header file mustExcute.h so how could i use it in this case i guess it should be added to the header :)
> > > Yep, true, it's `static`. Please add it to the header. :)
> > i tried to erase static as static means that it will only used in the cpp file also i have put the definition in the header file but that resulted in an error in linking so how could we do that here ? I guess also that all the functions i saw was only static in the cpp file or they are belong to a class in the .h or cpp file but no function that is implemented alone so I think that getting this 3 lines is more clean or what do you think ?
> I think it's better if we use the already made function. I'll try to do that myself tomorrow and see what problems come up.
> 
> [Irrelevant] Some more info about `static`: It means that you can use the function only in this //compilation unit//. Usually, a compilation unit is a `.cpp` file, but if you include a `.cpp` file in another, those 2 will make //one// compilation unit.
> As a side note, `static` has more to do with linking than with compiling. It basically means that the symbol (i.e. the function, which in assembly is just a label) is not exported. So, no code from another object file can jump to it.
> Here's a great video in case you're interested in this kind of stuff: https://www.youtube.com/watch?v=n4fI4eUTTKM
So, I had forgotten about `namespace llvm`. You need to put the declaration of the function inside `namespace llvm` in `MustExecute.h`.
Then, in `MustExecute.cpp`, you need to remove `static` but add `llvm::` in the name, i.e. `llvm::mayContainIrreducibleControl(...`.
In short, the function has to be declared inside the llvm namespace and when you define it, you have to specify that you want to define a function from this specific namespace.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74691