[PATCH] Implement command line options for stack probe space

[David Majnemer]

REPOSITORY
  rL LLVM

================
Comment at: include/clang/Frontend/CodeGenOptions.def:144-145
@@ -141,1 +143,4 @@
+                                             ///< stack probe size.
+VALUE_CODEGENOPT(StackProbeSize    , 32, 0) ///< Overrides default stack 
+                                            ///< probe size, even if 0.
 CODEGENOPT(DebugColumnInfo, 1, 0) ///< Whether or not to use column information
----------------
Why not just make `StackProbeSize` default to 4096 and get rid of `IsStackProbeSizeOverridden`.  Then you can get rid of `IsStackProbeSizeOverridden`.

After all that, only add the "stack-probe-size" attribute to the function if the StackProbeSize is not equal to 4096.

================
Comment at: lib/CodeGen/TargetInfo.cpp:1637
@@ -1632,1 +1636,3 @@
 
+void AddStackProbeSizeTargetAttribute(const Decl *D,
+                                      llvm::GlobalValue *GV,
----------------
This should start with a lower case letter and be marked as `static`.

================
Comment at: lib/CodeGen/TargetInfo.cpp:1638-1640
@@ +1637,5 @@
+void AddStackProbeSizeTargetAttribute(const Decl *D,
+                                      llvm::GlobalValue *GV,
+                                      CodeGen::CodeGenModule &CGM)
+{
+  if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
----------------
This doesn't look correctly formatted for the coding style.

================
Comment at: lib/CodeGen/TargetInfo.cpp:1644
@@ +1643,3 @@
+      llvm::Function *Fn = cast<llvm::Function>(GV);
+      llvm::AttrBuilder B;
+
----------------
This looks unused.

================
Comment at: lib/CodeGen/TargetInfo.cpp:1646
@@ +1645,3 @@
+
+      Fn->addFnAttr("stack-probe-size", std::to_string(CGM.getCodeGenOpts().StackProbeSize));
+    }
----------------
I don't think we can use std::to_string.  IIRC, some platforms don't have it yet.

Please use llvm::utostr instead.  It is in "llvm/ADT/StringExtras.h"

================
Comment at: lib/CodeGen/TargetInfo.cpp:1654
@@ +1653,3 @@
+                                            CodeGen::CodeGenModule &CGM) const {
+  X86_32TargetCodeGenInfo::SetTargetAttributes(D, GV, CGM);
+
----------------
Please do the same for the X86_64 side so nothing mysterious happens if somebody adds an implementation of SetTargetAttributes.

================
Comment at: lib/Driver/Tools.cpp:3723
@@ +3722,3 @@
+  if (Args.hasArg(options::OPT_mstack_probe_size)) {
+    StringRef size = Args.getLastArgValue(options::OPT_mstack_probe_size);
+
----------------
Size should be capitalized.

================
Comment at: lib/Driver/Tools.cpp:3725
@@ +3724,3 @@
+
+    if(!size.empty())
+      CmdArgs.push_back(Args.MakeArgString("-mstack-probe-size=" + size));
----------------
Space between the if and the (

================
Comment at: lib/Frontend/CompilerInvocation.cpp:518-520
@@ +517,5 @@
+    Opts.IsStackProbeSizeOverridden = true;
+  }
+  else
+  {
+    Opts.IsStackProbeSizeOverridden = false;
----------------
Braces should be right next to the else.

http://reviews.llvm.org/D6685

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/