[PATCH] D20263: X86: Avoid using _chkstk when lowering WIN_ALLOCA instructions

[Reid Kleckner via llvm-commits]

rnk added a comment.

In http://reviews.llvm.org/D20263#431442, @DavidKreitzer wrote:

> BTW, any thoughts on how to fix the above bug? This is another thing that I think ought to be handled by clang.


We did notice this, it's https://llvm.org/bugs/show_bug.cgi?id=26776. We can leverage the fact that argument evaluation order is currently unsequenced, and evaluate the argument containing the alloca call first.

However, if C++17 defines argument evaluation order, then it will become impossible to satisfy the copy elision requirement and the evaluation order requirement in this old and busted 32-bit ABI. =/

---

I think we can transform WIN_ALLOCA to ADJCALLSTACK today in certain limited cases, and if we can, it would definitely be a win. I doubt we can tolerate nested ADJCALLSTACK frames, though, and inalloca typically happens precisely when there is a nested call sequence. Obviously, stack frame reservation conflicts with nested call stack adjustments, but we should already be protected from that by the `hasVarSizedObjects` flag, which is set earlier in FunctionLoweringInfo.

Do you intend to pursue store-to-push conversion for calls where an argument is still address-taken by the time we reach codegen? Consider this kind of code:

  struct S { S(const S &); ~S(); int s; };
  S makeS(); // cannot inline away
  void takeS(int, S, int);
  void passS() { takeS(1, makeS(), 3); }

MSVC can make this into "push 1, sub 4, call makeS, push 3", but by the time we reach LLVM codegen, we have no ability to reason about what memory makeS writes. I'm happy if LLVM can turn this into "sub, call makeS, store 1, store 3, call takeS, add". This is what we used to get before push conversion, and I think we can achieve it with our current approach. If we want to do full push conversion, then we need to revisit the IR.


http://reviews.llvm.org/D20263