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

Hans Wennborg via llvm-commits llvm-commits at lists.llvm.org
Tue May 17 11:12:27 PDT 2016


hans added a comment.

Any more comments, or are you all OK with this going in?


================
Comment at: lib/Target/X86/X86WinAllocaExpander.cpp:181
@@ +180,3 @@
+                 MI.getOpcode() == X86::ADJCALLSTACKUP64) {
+        Offset -= MI.getOperand(0).getImm();
+      } else if (MI.getOpcode() == X86::ADJCALLSTACKDOWN32 ||
----------------
mkuper wrote:
> This looks a bit scary to me. It can cause an offset to be temporary negative, and negative offsets are special-cased now, right? (Well, we'll probably never hit "-1" due to alignment anyway, but...)
> 
> I think I can live with this, though, especially since, as Dave's example shows, allocations within this region are probably broken as is, so it's not like this would make anything worse. But if we keep it as is, then I think we need at least a FIXME.
Thinking more about this, I don't think a negative offset would break anything. The interesting code is on line 107, which I think will do the right thing (if we ever ended up in this situation).

Negative-offsets are not special-cased (a negative WinAlloca amount means "unknown" though.)


http://reviews.llvm.org/D20263





More information about the llvm-commits mailing list