[PATCH] D49144: [FunctionAttrs] Infer the speculatable attribute

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added inline comments.


================
Comment at: lib/Transforms/IPO/FunctionAttrs.cpp:1207
+      if (SCCNodes.count(Callee) > 0)
+        return false;
+
----------------
efriedma wrote:
> I don't think we can completely ignore calls to functions within the current SCC: we also have to ensure the call itself is never UB due to violating some attribute on the call (like nonnull; see D49041).
Two thoughts:

 1. How we handle calls might not matter at all. Given that the calls can't have any branching, if there's a call in the closed SCC that we would otherwise mark as speculatable, doesn't it need to be infinite recursion (and that, we can't speculate)?

 2. Maybe D49041 is too aggressive? I think that we'd like violating nonnull on an argument to yield a poison value, not hard UB. This would be consistent with how to handle other flags (nsw, etc.)?

In any case, independent of (2), we can't speculate infinite recursion calls, so we should probably ignore the SCC regardless?


Repository:
  rL LLVM

https://reviews.llvm.org/D49144