[PATCH] Add a "probe-stack" attribute

John Kåre Alsaker john.mailinglists at gmail.com
Sat Aug 9 06:31:38 PDT 2014


A newer version was posted to the mailing list.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:18201
@@ -18200,3 +18200,3 @@
 
-  if (Subtarget->isTargetWin64()) {
+  if (Subtarget->isTargetWin64() || !Subtarget->isOSWindows()) {
     if (Subtarget->isTargetCygMing()) {
----------------
Philip Reames wrote:
> This condition "feels" wrong.  It looks like you're only avoiding this case for 32 bit Windows?  Why?  The old code didn't.  
> 
> 
The old code did also just avoid 32-bit Windows (the only other case was 64-bit Windows)

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:18227
@@ +18226,3 @@
+          .addReg(X86::RAX);
+      } else {
+        BuildMI(*BB, MI, DL, TII->get(X86::CALLpcrel32))
----------------
Philip Reames wrote:
> I'm not clear on what this change is doing.  In addition to the changed symbol, it looks like you're adding 64 vs 32 bit case?  Is this an unrelated bugfix?
This calls the x86-32 __probestack function the same way as x86-64 __chkstk/__probestack. I'll make this more explicit.

================
Comment at: test/CodeGen/X86/stack-probes.ll:1
@@ +1,2 @@
+; RUN: llc -mtriple=i386-pc-linux-gnu < %s -o - | FileCheck --check-prefix=X86-Linux %s
+; RUN: llc -mtriple=x86_64-pc-linux-gnu < %s -o - | FileCheck --check-prefix=X64-Linux %s
----------------
Philip Reames wrote:
> Why bother with a custom prefix if they are the same?  You can just use the default CHECK prefix.  
A custom prefix seems to be the pattern, and it makes sense if tests for other platforms will be added.

http://reviews.llvm.org/D4717






More information about the llvm-commits mailing list