[PATCH] D43108: Support for the mno-stack-arg-probe flag

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

I see in the PR that matches a MinGW flag, but I'm curious to the motivation here. In what scenario would the user want to use this, i.e. how do they know it's safe to drop the probes?



================
Comment at: lib/CodeGen/TargetInfo.cpp:2358
 
-static void addStackProbeSizeTargetAttribute(const Decl *D,
-                                             llvm::GlobalValue *GV,
-                                             CodeGen::CodeGenModule &CGM) {
+static void addStackProbeParamsTargetAttribute(const Decl *D,
+                                               llvm::GlobalValue *GV,
----------------
I'd suggest perhaps "addStackProbeTargetAttributes" as a name instead, since I'm not sure what Params is for.


================
Comment at: lib/CodeGen/TargetInfo.cpp:2361
+                                               CodeGen::CodeGenModule &CGM) {
   if (D && isa<FunctionDecl>(D)) {
+    llvm::Function *Fn = cast<llvm::Function>(GV);
----------------
This could be written as

```
if (llvm::Function *Fn = dyn_cast_or_null<llvm::Function>(GV)) {
```


================
Comment at: lib/Driver/ToolChains/Clang.cpp:4038
+  if (Args.hasArg(options::OPT_mno_stack_arg_probe))
+    CmdArgs.push_back("-mno-stack-arg-probe");
+
----------------
I think you can just do

```
Args.AddLastArg(CmdArgs, options::OPT_mno_stack_arg_probe)
```

which avoids the if-statement and having to spell out the flag.


Repository:
  rC Clang

https://reviews.llvm.org/D43108