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

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added inline comments.


================
Comment at: llvm/lib/CodeGen/PrologEpilogInserter.cpp:282
+        .getValueAsString()
+        .getAsInteger(0, Threshold);
   if (StackSize > Threshold) {
----------------
nickdesaulniers wrote:
> dblaikie wrote:
> > I guess the 0 value here is the default value if the value can't be parsed as an integer? Is that desirable? I guess maybe we should ignore it (use UINT_MAX here instead, maybe) and fail in the verifier.
> > 
> > But I guess if we fail in the verifier, then it doesn't really matter/shouldn't be tested what the behavior is here when presented with invalid IR.
> > 
> > (but this is a divergence from the module flag handling, which looks like it does silently ignore non-numeric values, by using UINT_MAX)
> IIUC, the first parameter to `getAsInteger` is the `Radix`, not the default value on failure to parse. But it does return `true` on error, so I should check that here.
> 
> I also should add a verifier check for this new function attribute.  While the "string key equals string value" attributes are quite flexible, it would be good to have some rigidity in requiring the string value to be parseable as an unsigned int.
Oh, I should use base 10 as the radix, otherwise it will try to parse hex and binary literals.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104342