[PATCH] D20003: X86CallFrameOpt: a first step towards optimizing inalloca calls (PR27076)

[Hans Wennborg via llvm-commits]

hans added a comment.

Thank you both for the review comments!

In http://reviews.llvm.org/D20003#423533, @DavidKreitzer wrote:

> I think X86CallFrameOptimization is the wrong place to be trying to eliminate the _chkstk calls for inalloca. The ability to do the store-to-push optimization has no bearing on whether the chkstk call is needed. Your comment about the pushes naturally probing the stack is true, but it is also true of the original stores.
>
> I think David had the right idea in r262370. But I assume the problem he ran into is that clang is nesting these inalloca calls, so it isn't easy to tell how much stack space is ultimately going to be allocated. Consider a case like this:


The connection I saw between removing the _chkstk calls and this transformation is that the pushes will touch the stack in a safe order (starting at %esp, which should be safe, and progressing downwards), whereas the original stores might be in an order that starts by touching an address beyond the allocated stack.

What worries me about the approach in r262370 is that it doesn't take other stack objects into account (and I think that's why it failed). What if our function has a 3 KB fixed array which hasn't been touched yet, can we then remove a 2 KB _chkstk? And IIUC (but I could be wrong here), we can't rely on checking the size of the stack frame at that stage, because it could increase due to register spills.

I figured tying this to the push-conversion would resolve these concerns in a neat way.

I agree with your point, and Michael's, that it's not clear yet if we could make this work for more complicated calls in a reasonable way. I figured this would be a good patch to start with, but maybe I need to experiment a bit further to see if it will work out.


http://reviews.llvm.org/D20003