[PATCH] Fix hardcoded stack probe space
Philip Reames
listmail at philipreames.com
Fri Jan 2 14:34:31 PST 2015
Minor drive by comments. I'm explicitly deferring giving a LGTM to previous reviewers.
@majnemer -- I would suggest we let this land without addressing your LTO concerns. While I agree that's an issue, this is a strict improvement over what we have. If someone files a bug, adding the inliner logic wouldn't be too hard and I'd probably even get to it.
REPOSITORY
rL LLVM
================
Comment at: lib/Target/X86/X86FrameLowering.cpp:540
@@ -531,1 +539,3 @@
+ }
+
// If this is x86-64 and the Red Zone is not disabled, if we are a leaf
----------------
Why not use: Fn->hasFnAttribute and Fn->getFnAttribute
================
Comment at: lib/Target/X86/X86Subtarget.cpp:232
@@ +231,3 @@
+ // The page size is 4096 bytes for all targets.
+ static const size_t PageSize = 4096;
+
----------------
Your comment here is unclear. Specifically, many x86 targets support variable page sizes. I think what you want here is document the fact that we're *assuming* the page size for a page of stack space is *at least* 4k.
I'm not sure having this extracted as a named constant adds anything here.
================
Comment at: lib/Target/X86/X86Subtarget.h:301
@@ +300,3 @@
+ /// safely accessed without calling stack probing functions.
+ unsigned getStackProbeSize() const { return stackProbeSize; }
+
----------------
I might name this getDefaultStackProbeSize just to make it clear it's overridable.
http://reviews.llvm.org/D6684
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the llvm-commits
mailing list