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.
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.
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;
+}
