[PATCH] D23339: Don't import variadic functions

[Easwaran Raman via llvm-commits]

eraman added a comment.

In https://reviews.llvm.org/D23339#510714, @Prazek wrote:

> In https://reviews.llvm.org/D23339#510712, @Prazek wrote:
>
> > In https://reviews.llvm.org/D23339#510690, @eraman wrote:
> >
> > > In https://reviews.llvm.org/D23339#510677, @mehdi_amini wrote:
> > >
> > > > What is the benefit of knowing that a function is variadic compared to just knowing that is can't be inlined?
> > > >  We should just have a bit that is "can't be inlined"
> > >
> > >
> > > InlineCost.cpp has an isInlineViable function. It doesn't check for vararg now, so that should be first fixed and then can be used to set a bit in the summary.
> >
> >
> > You mean to use isInlineViable to set this bit? It definitely checks more things, but I actually didn't get any of the cases that the functions checks in the SPEC int.
> >  My concern is that this is probably very costly to call.
>
>
> I looked on this function, and it seems that inlineCost only calls it to determinate if function marked as "always_inline" should be inlined. I should add if Variadic there, but I don't think we should use this function to determinate the bit.




In https://reviews.llvm.org/D23339#510714, @Prazek wrote:

> In https://reviews.llvm.org/D23339#510712, @Prazek wrote:
>
> > In https://reviews.llvm.org/D23339#510690, @eraman wrote:
> >
> > > In https://reviews.llvm.org/D23339#510677, @mehdi_amini wrote:
> > >
> > > > What is the benefit of knowing that a function is variadic compared to just knowing that is can't be inlined?
> > > >  We should just have a bit that is "can't be inlined"
> > >
> > >
> > > InlineCost.cpp has an isInlineViable function. It doesn't check for vararg now, so that should be first fixed and then can be used to set a bit in the summary.
> >
> >
> > You mean to use isInlineViable to set this bit? It definitely checks more things, but I actually didn't get any of the cases that the functions checks in the SPEC int.
> >  My concern is that this is probably very costly to call.
>
>
> I looked on this function, and it seems that inlineCost only calls it to determinate if function marked as "always_inline" should be inlined. I should add if Variadic there, but I don't think we should use this function to determinate the bit.


Yes, I missed the part in getInlineCost where this is called only with the always_inline attribute. All (or most) of the checks in this method is also checked in the regular inliner (SimpleInliner) but only in the reachable path. But it's probably fine not to check for them (and let the inliner reject them later). Even if you're checking for just var_args, this code be added to InlineCost.cpp.


https://reviews.llvm.org/D23339