This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] rs6000: -flto forgets 'no-vsx' function attributes (PR target/70010)


Peter Bergner <bergner@linux.ibm.com> writes:
....
>
> I think we just need to fix the bug in the current logic when checking
> whether the caller's ISA flags supports the callee's ISA flags. ...and
> for that, I think we just need to add a test that enforces that the
> caller's ISA flags match exactly the callee's flags, for those flags
> that were explicitly set in the callee.  The patch below seems to fix
> the issue (regtesting now).  Does this look like what we want?
>
> Peter
>

And another issue: Behavior is still inconsistent between "-mno-vsx
-flto" and "-mno-vsx" for user code. Previous patch makes it consistent
between "-mvsx -flto" and "-mvsx". 

 cat novsx.c
vector int c, a, b;
static inline void __attribute__ ((__always_inline__, target ("no-vsx")))
foo ()
{
  c = a + b;
}

int
main ()
{
  foo ();
  c = a + b;
}

As I expected, 'foo' would be able to inline into 'main'. -flto works as
expect, but without -flto, it failed.

 $GCC_BUILD/gcc/xgcc -B$GCC_BUILD/gcc novsx.c -O2 -mno-vsx -flto ## ---> inlined

 $GCC_BUILD/gcc/xgcc -B$GCC_BUILD/gcc novsx.c -O2 -mno-vsx
/home/guojiufu/temp/novsx.c: In function 'main':
/home/guojiufu/temp/novsx.c:6:1: error: inlining failed in call to 'always_inline' 'foo': target specific option mismatch
    6 | foo () /* { dg-error "inlining failed in call to .* target specific option mismatch" } */
      | ^~~
/home/guojiufu/temp/novsx.c:14:3: note: called from here
   14 |   foo (); /* { dg-message "called from here" } */
      |   ^~~~~~


To handle this, we could fix the code a little more.

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index d2a9f26..1f26c58 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -23971,7 +23971,18 @@ rs6000_can_inline_p (tree caller, tree callee)
   /* If caller has no option attributes, but callee does then it is not ok to
      inline.  */
   else if (!caller_tree)
-    ret = false;
+    {
+      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;
+
+      /* Propogate global flags to caller.  */
+      HOST_WIDE_INT caller_isa = rs6000_isa_flags;
+
+      if (((caller_isa & callee_isa) == callee_isa)
+         && (caller_isa & explicit_isa) == (callee_isa & explicit_isa))
+       ret = true;
+    }
   else
     {
       struct cl_target_option *caller_opts = TREE_TARGET_OPTION (caller_tree);

And combine with your patch as below:

gcc/
	* config/rs6000/rs6000.c (rs6000_can_inline_p): Handle explicit
	options.

gcc.testsuite/
	* gcc.target/powerpc/pr70010.c: New test.
	* gcc.target/powerpc/pr70010-1.c: New test.
	* gcc.target/powerpc/pr70010-2.c: New test.
	* gcc.target/powerpc/pr70010-3.c: New test.

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index d1434a9..68a9224 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -23968,21 +23968,25 @@ rs6000_can_inline_p (tree caller, tree callee)
   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);
       struct cl_target_option *callee_opts = TREE_TARGET_OPTION (callee_tree);
-
-      /* 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)
+      HOST_WIDE_INT callee_isa = callee_opts->x_rs6000_isa_flags;
+      HOST_WIDE_INT explicit_isa = callee_opts->x_rs6000_isa_flags_explicit;
+
+      /* If caller has no option attributes (without -flto), propogate global
+	 flags to caller, else use itself's option attributes.  */
+      HOST_WIDE_INT caller_isa
+	= caller_tree ? TREE_TARGET_OPTION (caller_tree)->x_rs6000_isa_flags
+		      : 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;
     }
 
diff --git a/gcc/testsuite/gcc.target/powerpc/pr70010-1.c b/gcc/testsuite/gcc.target/powerpc/pr70010-1.c
new file mode 100644
index 0000000..78870db
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr70010-1.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -flto -mvsx" } */
+
+vector int c, a, b;
+
+static inline void __attribute__ ((__always_inline__, target ("no-vsx")))
+foo () /* { dg-error "inlining failed in call to .* target specific option mismatch" } */
+{
+  c = a + b;
+}
+
+int
+main ()
+{
+  foo (); /* { dg-message "called from here" } */
+  c = a + b;
+}
diff --git a/gcc/testsuite/gcc.target/powerpc/pr70010-2.c b/gcc/testsuite/gcc.target/powerpc/pr70010-2.c
new file mode 100644
index 0000000..4c09b21
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr70010-2.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -flto -mno-vsx" } */
+
+vector int c, a, b;
+
+static inline void __attribute__ ((__always_inline__, target ("no-vsx")))
+foo ()
+{
+  c = a + b;
+}
+
+int
+main ()
+{
+  foo ();
+  c = a + b;
+}
diff --git a/gcc/testsuite/gcc.target/powerpc/pr70010-3.c b/gcc/testsuite/gcc.target/powerpc/pr70010-3.c
new file mode 100644
index 0000000..bca3187
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr70010-3.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -mno-vsx" } */
+
+vector int c, a, b;
+
+static inline void __attribute__ ((__always_inline__, target ("no-vsx")))
+foo ()
+{
+  c = a + b;
+}
+
+int
+main ()
+{
+  foo ();
+  c = a + b;
+}
diff --git a/gcc/testsuite/gcc.target/powerpc/pr70010.c b/gcc/testsuite/gcc.target/powerpc/pr70010.c
new file mode 100644
index 0000000..2e6582c
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr70010.c
@@ -0,0 +1,21 @@
+/* { dg-do compile { target { powerpc*-*-* } } } */
+/* { dg-skip-if "" { powerpc*-*-darwin* } } */
+/* { dg-require-effective-target powerpc_vsx_ok } */
+/* { dg-options "-O2 -finline" } */
+/* { dg-final { scan-assembler "bl vadd_no_vsx" } } */
+
+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;
+}


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]