[PATCH] D81765: Don't hoist very large static allocas to the entry block during inlining

[JF Bastien via Phabricator via llvm-commits]

jfb added a subscriber: MadCoder.
jfb added inline comments.


================
Comment at: llvm/include/llvm/Analysis/InlineCost.h:54
+/// size is above this amount in bytes.
+const unsigned MaxStaticAllocaSizeToMove = 65536;
 } // namespace InlineConstants
----------------
@MadCoder does this size limit make sense to you, or would you recommend something smaller?


================
Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1939
+        if (!Size)
+          return false;
+        return Size.getValue() / 8 > InlineConstants::MaxStaticAllocaSizeToMove;
----------------
This will also prevent allocations of unknown size from inlining, right? Is that a pessimization? I guess the problem you have is that you don't know whether you'll then propagate a huge size into the alloca, and then hoist it outside a block?

Would it make sense to both:

* Refuse inlining large alloca of known size
* Refuse moving large alloca of known size outside blocks
* Refuse moving variable-sized alloca (unknown size) outside blocks

?

That way you can still inline functions with alloca of unknown size, you can still constant propagate into them, but you would hoist the alloca if it's big.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81765