Peter Bergner <berg...@linux.ibm.com> writes:
> Segher Boessenkool <seg...@kernel.crashing.org> writes:
>> So what should we do about this? There are arguments for *both*
>> behaviours, and apparently with LTO we do not know which flags are
>> explicit?
>
> Actually, from my testing, it seems the rs6000_isa_flags_explicit
> flags are set correctly in LTO!
>
>
>
> On 10/15/19 7:45 AM, Jiufu Guo wrote:
>> The difference between 'with LTO' and 'without LTO' is:
>> Wit LTO, if a function does not have target attribute in source code,
>> then using option attribute from command line(e.g. -mcpu=xxx) as
>> isa_flags; and if features (e.g. -mno-vsx, -mvsx) are specified on
>> command line, isa_flags_explicit is also set.
>> If a function has target attribute in source code explicitly,
>> then isa_flags_explicit is set to reflect feature is specified
>> explicitly besides explicit features from command line; and isa_flags is
>> set as the effective features.
>>
>> Without LTO, if a function does not have target attribute in source code,
>> then it's isa_flags_explicit and isa_flags are as NULL (target_options
>> == NULL).
>> If a function has target attribute in source code, it is same as 'with
>> LTO'.
>
> After reading this a few times and compiling a few unit tests cases in
> gdb/cc1/lto1, I agree with your explanation above and with your addition
> to my patch. However, how about a slight modification to your change
> with some updated comments? Updated patch below.
Great update.
>
> This also incorporates Segher and Andreas's changes to my test case.
> I have not looked at your extra test cases, so I have not added them
> to this patch, but I like the idea of adding test cases that exercise
> this code in LTO context.
>
> I am going on vacation tomorrow, so Jiufu, can you take over ownership
> of this combined patch, along with your extra test cases? I have not
> bootstrapped or regtested this, so that still needs doing. If you're
> busy, I can pick this up when I return to work on Monday.
Sure, I will add test cases and do bootstrap/regtest.
Jiufu
BR
>
> Peter
>
>
> gcc/
> PR target/70010
> * config/rs6000/rs6000.c (rs6000_can_inline_p): Prohibit inlining if
> the callee explicitly disables some isa_flags the caller is using.
>
> gcc.testsuite/
> PR target/70010
> * gcc.target/powerpc/pr70010.c: New test.
>
> Index: gcc/config/rs6000/rs6000.c
> ===================================================================
> --- gcc/config/rs6000/rs6000.c (revision 276975)
> +++ gcc/config/rs6000/rs6000.c (working copy)
> @@ -23964,25 +23964,31 @@ rs6000_can_inline_p (tree caller, tree c
> tree caller_tree = DECL_FUNCTION_SPECIFIC_TARGET (caller);
> tree callee_tree = DECL_FUNCTION_SPECIFIC_TARGET (callee);
>
> - /* If callee has no option attributes, then it is ok to inline. */
> + /* If the callee has no option attributes, then it is ok to inline. */
> if (!callee_tree)
> ret = true;
>
> - /* If caller has no option attributes, but callee does then it is not ok to
> - inline. */
> - else if (!caller_tree)
> - ret = false;
> -
> else
> {
> - struct cl_target_option *caller_opts = TREE_TARGET_OPTION
> (caller_tree);
> + HOST_WIDE_INT caller_isa;
> struct cl_target_option *callee_opts = TREE_TARGET_OPTION
> (callee_tree);
> + HOST_WIDE_INT callee_isa = callee_opts->x_rs6000_isa_flags;
> + HOST_WIDE_INT explicit_isa = callee_opts->x_rs6000_isa_flags_explicit;
> +
> + /* If the caller has option attributes, then use them.
> + Otherwise, use the command line options. */
> + if (caller_tree)
> + caller_isa = TREE_TARGET_OPTION (caller_tree)->x_rs6000_isa_flags;
> + else
> + caller_isa = rs6000_isa_flags;
>
> - /* Callee's options should a subset of the caller's, i.e. a vsx
> function
> - can inline an altivec function but a non-vsx function can't inline a
> - vsx function. */
> - if ((caller_opts->x_rs6000_isa_flags & callee_opts->x_rs6000_isa_flags)
> - == callee_opts->x_rs6000_isa_flags)
> + /* The callee's options must be a subset of the caller's options, i.e.
> + a vsx function may inline an altivec function, but a non-vsx function
> + must not inline a vsx function. However, for those options that the
> + callee has explicitly set, then we must enforce that the callee's
> + and caller's options match exactly; see PR70010. */
> + if (((caller_isa & callee_isa) == callee_isa)
> + && (caller_isa & explicit_isa) == (callee_isa & explicit_isa))
> ret = true;
> }
>
> Index: gcc/testsuite/gcc.target/powerpc/pr70010.c
> ===================================================================
> --- gcc/testsuite/gcc.target/powerpc/pr70010.c (nonexistent)
> +++ gcc/testsuite/gcc.target/powerpc/pr70010.c (working copy)
> @@ -0,0 +1,20 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target powerpc_vsx_ok } */
> +/* { dg-options "-O2 -finline-functions" } */
> +/* { dg-final { scan-assembler {\mbl vadd_no_vsx\M} } } */
> +
> +typedef int vec_t __attribute__((vector_size(16)));
> +
> +static vec_t
> +__attribute__((__target__("no-vsx")))
> +vadd_no_vsx (vec_t a, vec_t b)
> +{
> + return a + b;
> +}
> +
> +vec_t
> +__attribute__((__target__("vsx")))
> +call_vadd_no_vsx (vec_t x, vec_t y, vec_t z)
> +{
> + return vadd_no_vsx (x, y) - z;
> +}
