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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
