[PATCH] D63046: [Attributor] Deduce "willreturn" function attribute

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:1115-1120
+    Visited.insert(BB);
+    for (auto *SuccBB : successors(BB)) {
+      if (Visited.count(SuccBB))
+        return true;
+    }
+  }
----------------
nicholas wrote:
> jdoerfert wrote:
> > nicholas wrote:
> > > FYI, with effort, this could be pushed to be more efficient. df_iterator maintains its own copy of Visited, and it does its own scan over the successors of the block, neither of which need to be done twice. Unfortunately the fix is not entirely simple.
> > Let's not optimize it preemptively. I'll run it through LLVM-TS and SPEC2006 before I commit it anyway, if there is a change in compile or runtime I'll report it back.
> I doubt human-written code will trigger slow compile times in this path, but I bet it could happen with machine generated code. Just leave a TODO noting how it could be more efficient.
Agreed, to all of it. We will most likely see various "compile time problems" once we add more and more functionality (and actually turn it on), so keeping a record of fixes is very useful. @uenoku Can you add such a comment.


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

https://reviews.llvm.org/D63046