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

Michael Kuperstein via llvm-commits llvm-commits at lists.llvm.org
Tue May 17 12:40:18 PDT 2016


mkuper accepted this revision.
mkuper added a comment.
This revision is now accepted and ready to land.

LGTM


================
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 ||
----------------
hans wrote:
> 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.)
Ok, that sounds reasonable.


http://reviews.llvm.org/D20263





More information about the llvm-commits mailing list