tnfchris added inline comments.

================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3460
+  } else if (EffectiveTriple.isArm() || EffectiveTriple.isThumb()) {
+    CmdArgs.push_back("-mstack-probe-size=1024");
+  }
----------------
efriedma wrote:
> tnfchris wrote:
> > efriedma wrote:
> > > Why 1024?
> > 1024 was experimentally determined by Arm and is part of the ABI for stack 
> > clash (which has not yet been published).  It was determined by examining a 
> > large number of programs and looking at the function stack usages.  1024 
> > covers 80-90% of programs such that we can minimize the number of probes 
> > required in the average cases. 
> There are actually multiple numbers involved here, no?  One is the spacing of 
> probes, i.e. if allocating a large amount of stack, how many times you need 
> to probe; this is basically the page size of the target. the other is how 
> much unprobed space a function is allowed to allocate before calling another 
> function. Referring to the the AArch64 patch, -mstack-probe-size is the 
> former, the hardcoded "1024" is the latter.
I hadn't looked at the patch in detail yet, I thought this was the probing 
offset.  But you're right, what I thought of was `StackClashCallerGuard`,  if 
`stack-probe-size` indeed the guard size itself, then yeah this would be wrong. 
 It seems incorrect to allow it smaller than the page size.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154911/new/

https://reviews.llvm.org/D154911

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

Reply via email to