aemerson added inline comments.

================
Comment at: lib/CodeGen/BackendUtil.cpp:442
   Options.DebuggerTuning = CodeGenOpts.getDebuggerTuning();
+  Options.EnableStackProbe = CodeGenOpts.StackProbe;
 
----------------
ahatanak wrote:
> Is there a reason you can't use function attributes 
> "probe-stack"="___chkstk_darwin" and "stack-probe-size"=4096 instead of 
> setting a TargetOptions flag here? If you intend to use stack probing with 
> LTO, I think you need function attributes. Also, it looks like that would 
> simplify the changes made to X86 backend.
I don't think there's any reason not to. Is it worth specifying the probe size 
itself given that it'll be a known fixed value? It could be misleading to give 
a probe size which only has a single valid value for Darwin.


================
Comment at: lib/Driver/ToolChains/Clang.cpp:3955
+    CmdArgs.push_back("-mstack-probe");
+
   if (Args.hasArg(options::OPT_mstack_probe_size)) {
----------------
ahatanak wrote:
> Just to confirm, is stack probing going to be enabled unconditionally on 
> Darwin unless the user disables it with mno-stack-probe?
Yes that's the intention. However on D40863 there's discussion that it might be 
better to restrict to newer Darwin targets.


Repository:
  rC Clang

https://reviews.llvm.org/D40864



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to