[PATCH] D94633: [FunctionAttrs] Infer willreturn for functions without loops

Thu Jan 21 11:35:56 PST 2021

nikic added inline comments.

================
Comment at: lib/Transforms/IPO/FunctionAttrs.cpp:1441
+  SmallVector<std::pair<const BasicBlock *, const BasicBlock *>> Backedges;
+  FindFunctionBackedges(F, Backedges);
+  if (!Backedges.empty())
----------------
nikic wrote:
> nikic wrote:
> > fhahn wrote:
> > >  We do not really need to find all back edges, wouldn't it be sufficient to just traverse the CFG and exit once we found a cycle?
> > I don't think FindFunctionBackedges is expensive enough to warrant reimplementing it here. Note that just keeping a visited set doesn't cut it, as that would detect any join point. You need to perform a DFS search, and doing that non-recursively is ugly enough that I'd rather not repeat it.
> I ran some tests to check the compile-time impact. For this patch as-is: https://llvm-compile-time-tracker.com/compare.php?from=17fb21f875f4aaf6ad2cf9499cb75d76588167f2&to=335a834be8fe9a571fdbeea1262989c704ba91cf&stat=instructions There is a 0.1% regression. However, if I comment out the `setWillReturn` call, I get: http://llvm-compile-time-tracker.com/compare.php?from=17fb21f875f4aaf6ad2cf9499cb75d76588167f2&to=1b9a75cff1543e5ca29dea0dd56b111312901d98&stat=instructions So it seems that the impact here is due to second-order effects. The `FindFunctionBackedges` call itself doesn't appear to be problematic.
I've looked a bit closer into what causes the compile-time regression, and apparently it is the `setWillReturn()` call itself. Adding an attribute is quite expensive (we create AttributeBuilders for the original attributes and the new attribute, merge them, reconstruct an attribute set from that, look up the interned set...) Something that can be optimized, but otherwise not related to this patch.

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

https://reviews.llvm.org/D94633