[PATCH] D104342: [IR] convert warn-stack-size from module attr to fn attr

Thu Jun 17 13:56:14 PDT 2021

dblaikie added a comment.

In D104342#2825357 <https://reviews.llvm.org/D104342#2825357>, @nickdesaulniers wrote:

> In D104342#2820811 <https://reviews.llvm.org/D104342#2820811>, @dblaikie wrote:
>
>> what're the rules about module level attributes like this? For instance: Does this affect/hinder inlining (if the attributes don't match between the caller and callee)? Other situations that might matter?
>
> I think you meant s/module level/function level/?  That's a good question, one I had to think about a little bit.  Here's my thoughts on the behavior this should exhibit, please let me know if you agree.
>
> When using `-Wframe-larger-than=<threshold>` per TU, the developer wants to be alerted if any stack frame exceeds a threshold. The Linux kernel's use case is that the kernel's stack is limited to (usually) two pages (`ulimit -s`; typically 8KiB, but different architectures do support non-4KiB page sizes), so functions using more than 1KiB of stack are usually indicative of large objects being stack allocated that should have been heap allocated.
>
> Currently, in C (and C with GNU extensions), there is no way to describe to the compiler function-grain specific values for `-Wframe-larger-than=`; rather than fine grain per function control, we only have coarse grain TU control.
>
> So in the general case (non-LTO), we can only perform inline substitution at call sites visible from callers' definitions. Because there's no GNU C function attribute to change the current value of `-Wframe-larger-than=`, it's not possible for that value to differ between caller and callee.  But with LTO; shit gets weird.
>
> Suddenly now with LTO, we have cross TU (cross Module) visibility into call sites, so we can inline across TU/Module boundaries.  Thus we can have an IR intermediary object with a call site where the caller's value of `-Wframe-larger-than=` differs from the callees!  So the question is what should happen in such a case?
>
> The extremely conservative approach which we have done in the past for certain mismatched function attributes is to simply not perform inline substitution, if we have no other options.  This adds overhead to the inliner to check the XOR of attribute lists of the caller and callee for each call site.
>
> But I *think* (and am open to sugguestions) that we should:
>
> 1. permit inline substitution
> 2. the caller's value of `"warn-stack-size"=` IR Fn Attr wins
>
> I think this is ok because: if caller is defined in TU1 with `-Wframe-larger-than=` distinct from callee defined in TU2 with a different value of `-Wframe-larger-than=`, then we don't care what callee's value was. callee may even be DCE'd if it's inlined into a lone call site.  I'd expect in such cases that callee's value was larger than caller's, in which case callee should be attributed `no_inline` for LTO if the tighter threshold for caller now warns.  If callee's value was smaller than callers and we performed inline substitution, I think that's also perfectly fine, caller should not become "more strict."
>
> Generally in the Linux kernel, we see a common value of `-Wframe-larger-than=` throughout most of the TUs, with only a few generally having a larger value to relax constraints a little. (Also, those relaxations are questionable, given the intent of `-Wframe-larger-than=` use in the kernel in the first place).
>
> Let me add such a test to encode that intention; though I don't know yet what's involved/possible to implement. Let's see.

Sure, that all sounds pretty reasonable to me - mostly I was curious what the existing/default behavior is (if we do nothing other than what's already in this patch, how does the inliner handle different values/mismatched presence of warn-stack-size attributes, for instance) - to check that whatever it does seems reasonable/acceptable.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104342