[PATCH][Tree-optimization/PR89730]grant always_inline when -flive-patching=inline-only-static

[PATCH][Tree-optimization/PR89730]grant always_inline when -flive-patching=inline-only-static

[Qing Zhao]

[-- Attachment #1: Type: text/plain, Size: 734 bytes --]

Hi,

there is a bug in the current support for -flive-patching=inline-only-static:

it rejects inlining of external always_inline routine, therefore triggers a compilation time error. 

we should always inline “always_inline” routines.

please review the following simple patch, it has been bootstrapped and regression tested on aarch64.

Okay for committing? 

thanks.

Qing.

gcc/ChangeLog:

2019-03-20  qing zhao  <qing.zhao@oracle.com>

	PR tree-optimization/89730
       * ipa-inline.c (can_inline_edge_p): Grant always_inline even when
       -flive-patching=inline-only-static.	

gcc/testsuite/ChangeLog:

2019-03-20  qing zhao  <qing.zhao@oracle.com>

	* gcc.dg/live-patching-4.c: New test.


[-- Attachment #2: PR89730.patch --]
[-- Type: application/octet-stream, Size: 1602 bytes --]

From 68f21c507c811f3eb81acce9f3853e6ea3b1624a Mon Sep 17 00:00:00 2001
From: qing zhao <qing.zhao@oracle.com>
Date: Tue, 19 Mar 2019 11:39:00 -0700
Subject: [PATCH] fixing PR89730

---
 gcc/ipa-inline.c                       |  1 +
 gcc/testsuite/gcc.dg/live-patching-4.c | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/live-patching-4.c

diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c
index 360c3de..3d04d97 100644
--- a/gcc/ipa-inline.c
+++ b/gcc/ipa-inline.c
@@ -386,6 +386,7 @@ can_inline_edge_p (struct cgraph_edge *e, bool report,
       inlinable = false;
     }
   else if (callee->externally_visible
+	   && !DECL_DISREGARD_INLINE_LIMITS (callee->decl)
 	   && flag_live_patching == LIVE_PATCHING_INLINE_ONLY_STATIC)
     {
       e->inline_failed = CIF_EXTERN_LIVE_ONLY_STATIC;
diff --git a/gcc/testsuite/gcc.dg/live-patching-4.c b/gcc/testsuite/gcc.dg/live-patching-4.c
new file mode 100644
index 0000000..ffea8f4
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/live-patching-4.c
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -flive-patching=inline-only-static -fdump-tree-einline-optimized" } */
+
+extern int sum, n, m;
+
+extern inline __attribute__((always_inline)) int foo (int a);
+inline __attribute__((always_inline)) int foo (int a)
+{
+  return a + n;
+}
+
+static int bar (int b)
+{
+  return b * m;
+}
+
+int main()
+{
+  sum = foo (m) + bar (n); 
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump "Inlining foo/0 into main/2"  "einline" } } */
-- 
1.9.1

Re: [PATCH][Tree-optimization/PR89730]grant always_inline when -flive-patching=inline-only-static

[Richard Biener]

On Wed, Mar 20, 2019 at 4:16 PM Qing Zhao <qing.zhao@oracle.com> wrote:
>
> Hi,
>
> there is a bug in the current support for -flive-patching=inline-only-static:
>
> it rejects inlining of external always_inline routine, therefore triggers a compilation time error.
>
> we should always inline “always_inline” routines.
>
> please review the following simple patch, it has been bootstrapped and regression tested on aarch64.
>
> Okay for committing?

To me this seems like can_inline_edge_p was the wrong spot to disable inlining
for -flive-patching=inline-only-static and instead it should have been in
can_inline_edge_by_limits_p which already has proper allowance for
always-inline.

So can you move the check there, like after

      /* gcc.dg/pr43564.c.  Apply user-forced inline even at -O0.  */
      else if (always_inline)
        ;

?

> thanks.
>
> Qing.
>
> gcc/ChangeLog:
>
> 2019-03-20  qing zhao  <qing.zhao@oracle.com>
>
>         PR tree-optimization/89730
>        * ipa-inline.c (can_inline_edge_p): Grant always_inline even when
>        -flive-patching=inline-only-static.
>
> gcc/testsuite/ChangeLog:
>
> 2019-03-20  qing zhao  <qing.zhao@oracle.com>
>
>         * gcc.dg/live-patching-4.c: New test.
>

Re: [PATCH][Tree-optimization/PR89730]grant always_inline when -flive-patching=inline-only-static

[Qing Zhao]

Hi, Richard,

thanks for the suggestion.

I tried it yesterday, but it did not work. 

the reason is:

inside “can_inline_edge_by_limits_p”,  the “allowance for always_inline” is guarded by the following condition:

else if “caller_tree != callee_tree”

this condition is ONLY true when when callee has different optimization level than the caller. 

So, I think that the correct spot for the checking of live-patching=inline-only-static should still be inside “can_inline_edge_p”. 

so my previous patch for this bug should be fine.

what’s your opinion?

thanks.

Qing

> On Mar 25, 2019, at 7:23 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> 
> On Wed, Mar 20, 2019 at 4:16 PM Qing Zhao <qing.zhao@oracle.com> wrote:
>> 
>> Hi,
>> 
>> there is a bug in the current support for -flive-patching=inline-only-static:
>> 
>> it rejects inlining of external always_inline routine, therefore triggers a compilation time error.
>> 
>> we should always inline “always_inline” routines.
>> 
>> please review the following simple patch, it has been bootstrapped and regression tested on aarch64.
>> 
>> Okay for committing?
> 
> To me this seems like can_inline_edge_p was the wrong spot to disable inlining
> for -flive-patching=inline-only-static and instead it should have been in
> can_inline_edge_by_limits_p which already has proper allowance for
> always-inline.
> 
> So can you move the check there, like after
> 
>      /* gcc.dg/pr43564.c.  Apply user-forced inline even at -O0.  */
>      else if (always_inline)
>        ;
> 
> ?
> 
>> thanks.
>> 
>> Qing.
>> 
>> gcc/ChangeLog:
>> 
>> 2019-03-20  qing zhao  <qing.zhao@oracle.com>
>> 
>>        PR tree-optimization/89730
>>       * ipa-inline.c (can_inline_edge_p): Grant always_inline even when
>>       -flive-patching=inline-only-static.
>> 
>> gcc/testsuite/ChangeLog:
>> 
>> 2019-03-20  qing zhao  <qing.zhao@oracle.com>
>> 
>>        * gcc.dg/live-patching-4.c: New test.
>> 

Re: [PATCH][Tree-optimization/PR89730]grant always_inline when -flive-patching=inline-only-static

[Richard Biener]

On Tue, Mar 26, 2019 at 5:25 PM Qing Zhao <qing.zhao@oracle.com> wrote:
>
> Hi, Richard,
>
> thanks for the suggestion.
>
> I tried it yesterday, but it did not work.
>
> the reason is:
>
> inside “can_inline_edge_by_limits_p”,  the “allowance for always_inline” is guarded by the following condition:
>
> else if “caller_tree != callee_tree”
>
> this condition is ONLY true when when callee has different optimization level than the caller.
>
> So, I think that the correct spot for the checking of live-patching=inline-only-static should still be inside “can_inline_edge_p”.
>
> so my previous patch for this bug should be fine.
>
> what’s your opinion?

You could still move it after the following?

  /* Check if caller growth allows the inlining.  */
  if (!DECL_DISREGARD_INLINE_LIMITS (callee->decl)
      && !disregard_limits
      && !lookup_attribute ("flatten",
                 DECL_ATTRIBUTES (caller->decl))
      && !caller_growth_limits (e))
    inlinable = false;

that said, looking at DECL_DISREGARD_INLINE_LIMITS in
can_inline_edge_p looks wrong.

Richard.

> thanks.
>
> Qing
>
> > On Mar 25, 2019, at 7:23 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> >
> > On Wed, Mar 20, 2019 at 4:16 PM Qing Zhao <qing.zhao@oracle.com> wrote:
> >>
> >> Hi,
> >>
> >> there is a bug in the current support for -flive-patching=inline-only-static:
> >>
> >> it rejects inlining of external always_inline routine, therefore triggers a compilation time error.
> >>
> >> we should always inline “always_inline” routines.
> >>
> >> please review the following simple patch, it has been bootstrapped and regression tested on aarch64.
> >>
> >> Okay for committing?
> >
> > To me this seems like can_inline_edge_p was the wrong spot to disable inlining
> > for -flive-patching=inline-only-static and instead it should have been in
> > can_inline_edge_by_limits_p which already has proper allowance for
> > always-inline.
> >
> > So can you move the check there, like after
> >
> >      /* gcc.dg/pr43564.c.  Apply user-forced inline even at -O0.  */
> >      else if (always_inline)
> >        ;
> >
> > ?
> >
> >> thanks.
> >>
> >> Qing.
> >>
> >> gcc/ChangeLog:
> >>
> >> 2019-03-20  qing zhao  <qing.zhao@oracle.com>
> >>
> >>        PR tree-optimization/89730
> >>       * ipa-inline.c (can_inline_edge_p): Grant always_inline even when
> >>       -flive-patching=inline-only-static.
> >>
> >> gcc/testsuite/ChangeLog:
> >>
> >> 2019-03-20  qing zhao  <qing.zhao@oracle.com>
> >>
> >>        * gcc.dg/live-patching-4.c: New test.
> >>
>

Re: [PATCH][Tree-optimization/PR89730]grant always_inline when -flive-patching=inline-only-static

[Qing Zhao]

[-- Attachment #1: Type: text/plain, Size: 798 bytes --]

Thanks.

I updated ipa-inline.c as you suggested, and tested the gcc with all live-patching-*.c testing cases, all were fine.
bootstrapped on aarch64, and now the regression testing is running.

the new patch is as following:

Okay for trunk?

thanks.

Qing

gcc/ChangeLog:

2019-03-28  qing zhao  <qing.zhao@oracle.com>

	PR tree-optimization/89730
	* ipa-inline.c (can_inline_edge_p): Delete the checking for
	-flive-patching=inline-only-static.
	(can_inline_edge_by_limits_p): Add the checking for 
	-flive-patching=inline-only-static and grant always_inline
	even when -flive-patching=inline-only-static is specified. 


gcc/testsuite/ChangeLog:

2019-03-28  qing zhao  <qing.zhao@oracle.com>

	PR tree-optimization/89730
	* gcc.dg/live-patching-4.c: New test.


[-- Attachment #2: fixing-PR89730.patch --]
[-- Type: application/octet-stream, Size: 2382 bytes --]

From f6e41f39fbbdd56916f853ea4c3a7c8f991ebf48 Mon Sep 17 00:00:00 2001
From: qing zhao <qing.zhao@oracle.com>
Date: Thu, 28 Mar 2019 08:40:55 -0700
Subject: [PATCH] Fixing PR89730

---
 gcc/ipa-inline.c                       | 13 +++++++------
 gcc/testsuite/gcc.dg/live-patching-4.c | 23 +++++++++++++++++++++++
 2 files changed, 30 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/live-patching-4.c

diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c
index 360c3de..f37cd9d 100644
--- a/gcc/ipa-inline.c
+++ b/gcc/ipa-inline.c
@@ -385,12 +385,6 @@ can_inline_edge_p (struct cgraph_edge *e, bool report,
       e->inline_failed = CIF_ATTRIBUTE_MISMATCH;
       inlinable = false;
     }
-  else if (callee->externally_visible
-	   && flag_live_patching == LIVE_PATCHING_INLINE_ONLY_STATIC)
-    {
-      e->inline_failed = CIF_EXTERN_LIVE_ONLY_STATIC;
-      inlinable = false;
-    }
   if (!inlinable && report)
     report_inline_failed_reason (e);
   return inlinable;
@@ -433,6 +427,13 @@ can_inline_edge_by_limits_p (struct cgraph_edge *e, bool report,
      		 DECL_ATTRIBUTES (caller->decl))
       && !caller_growth_limits (e))
     inlinable = false;
+  else if (callee->externally_visible
+	   && !DECL_DISREGARD_INLINE_LIMITS (callee->decl)
+	   && flag_live_patching == LIVE_PATCHING_INLINE_ONLY_STATIC)
+    {
+      e->inline_failed = CIF_EXTERN_LIVE_ONLY_STATIC;
+      inlinable = false;
+    }
   /* Don't inline a function with a higher optimization level than the
      caller.  FIXME: this is really just tip of iceberg of handling
      optimization attribute.  */
diff --git a/gcc/testsuite/gcc.dg/live-patching-4.c b/gcc/testsuite/gcc.dg/live-patching-4.c
new file mode 100644
index 0000000..ffea8f4
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/live-patching-4.c
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -flive-patching=inline-only-static -fdump-tree-einline-optimized" } */
+
+extern int sum, n, m;
+
+extern inline __attribute__((always_inline)) int foo (int a);
+inline __attribute__((always_inline)) int foo (int a)
+{
+  return a + n;
+}
+
+static int bar (int b)
+{
+  return b * m;
+}
+
+int main()
+{
+  sum = foo (m) + bar (n); 
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump "Inlining foo/0 into main/2"  "einline" } } */
-- 
1.9.1

Re: [PATCH][Tree-optimization/PR89730]grant always_inline when -flive-patching=inline-only-static

[Qing Zhao]

> On Mar 28, 2019, at 11:30 AM, Qing Zhao <qing.zhao@oracle.com> wrote:
> 
> Thanks.
> 
> I updated ipa-inline.c as you suggested, and tested the gcc with all live-patching-*.c testing cases, all were fine.
> bootstrapped on aarch64, and now the regression testing is running.

the regression test passed without any issue.

qing
> 
> the new patch is as following:
> 
> Okay for trunk?
> 
> thanks.
> 
> Qing
> 
> gcc/ChangeLog:
> 
> 2019-03-28  qing zhao  <qing.zhao@oracle.com>
> 
> 	PR tree-optimization/89730
> 	* ipa-inline.c (can_inline_edge_p): Delete the checking for
> 	-flive-patching=inline-only-static.
> 	(can_inline_edge_by_limits_p): Add the checking for 
> 	-flive-patching=inline-only-static and grant always_inline
> 	even when -flive-patching=inline-only-static is specified. 
> 
> 
> gcc/testsuite/ChangeLog:
> 
> 2019-03-28  qing zhao  <qing.zhao@oracle.com>
> 
> 	PR tree-optimization/89730
> 	* gcc.dg/live-patching-4.c: New test.
> 
> <fixing-PR89730.patch>

Re: [PATCH][Tree-optimization/PR89730]grant always_inline when -flive-patching=inline-only-static

[Richard Biener]

On Thu, Mar 28, 2019 at 6:53 PM Qing Zhao <qing.zhao@oracle.com> wrote:
>
>
> > On Mar 28, 2019, at 11:30 AM, Qing Zhao <qing.zhao@oracle.com> wrote:
> >
> > Thanks.
> >
> > I updated ipa-inline.c as you suggested, and tested the gcc with all live-patching-*.c testing cases, all were fine.
> > bootstrapped on aarch64, and now the regression testing is running.
>
> the regression test passed without any issue.

OK.

Thanks,
Richard.

> qing
> >
> > the new patch is as following:
> >
> > Okay for trunk?
> >
> > thanks.
> >
> > Qing
> >
> > gcc/ChangeLog:
> >
> > 2019-03-28  qing zhao  <qing.zhao@oracle.com>
> >
> >       PR tree-optimization/89730
> >       * ipa-inline.c (can_inline_edge_p): Delete the checking for
> >       -flive-patching=inline-only-static.
> >       (can_inline_edge_by_limits_p): Add the checking for
> >       -flive-patching=inline-only-static and grant always_inline
> >       even when -flive-patching=inline-only-static is specified.
> >
> >
> > gcc/testsuite/ChangeLog:
> >
> > 2019-03-28  qing zhao  <qing.zhao@oracle.com>
> >
> >       PR tree-optimization/89730
> >       * gcc.dg/live-patching-4.c: New test.
> >
> > <fixing-PR89730.patch>
>