Hi!
On Mon, Oct 14, 2019 at 07:18:11PM -0500, Peter Bergner wrote:
> On 10/14/19 2:57 PM, Segher Boessenkool wrote:
> > On Mon, Oct 14, 2019 at 06:35:06PM +0200, Richard Biener wrote:
> >> The general case should be that if the caller ISA supports the callee one
> >> then inlining is OK. If this is not wanted in some cases then there are
> >> options like using a noinline attribute.
> >
> > I agree, and that is what we already do afaik.
>
> I agree on the making sure the caller's ISA supports the callee's ISA
> before allowing inlining...and I think that's what our code it "trying"
> to do. It just has some bugs.
It says that it wants the callee ISA to be a subset of the caller ISA,
and that is what it does. Which is exactly what we want. But as the
PR points out it is *also* useful to not inline callees that explicitly
disabled some ISA feature the caller has. Which we never promissed, but
perhaps we should.
> I don't think sprinkling noinline's around will work given LTO can
> inline random functions from one object file into a function in another
> object file and we have no idea what the options were used for both files.
> I think that would just force us to end up putting nolines on all fuctions
> that might be compiled with LTO.
I think LTO should handle noinline attributes just fine, it would be a
much bigger problem than this PR if it doesn't!
Missing date+name+address here; and missing PR ref.
> gcc/
> * config/rs6000/rs6000.c (rs6000_can_inline_p): Handle explicit
> options.
"Prohibit inlining if the callee explicitly disables some isa_flags the
caller has", or something like that? A few more words please.
> --- gcc/testsuite/gcc.target/powerpc/pr70010.c (nonexistent)
> +++ gcc/testsuite/gcc.target/powerpc/pr70010.c (working copy)
> @@ -0,0 +1,21 @@
> +/* { dg-do compile { target { powerpc*-*-* } } } */
Just
/* { dg-do compile } */
please: everything in gcc.target checks for powerpc*-*-* always (and
compile is the default, but it's nice documentation).
> +/* { dg-skip-if "" { powerpc*-*-darwin* } } */
You don't need this I think. If this test cannot work on Darwin for
some reason, the Darwin maintainers will take care of it. Unless you
*know* they want (or need) the skip here?
> +/* { dg-require-effective-target powerpc_vsx_ok } */
> +/* { dg-options "-O2 -finline" } */
-finline does nothing; it isn't even documented, only -fno-inline is.
> +/* { dg-final { scan-assembler "bl vadd_no_vsx" } } */
Please use
/* { dg-final { scan-assembler "\mbl vadd_no_vsx\M" } } */
so that it won't match "rldibl vadd_no_vsx", etc. Doesn't matter much
for this testcase, but it's a good habit (and using it makes me not
comment about it, also useful ;-) )
Okay for trunk with those tweaks. Thanks!
Segher
