Fix pure/const propagation in modref

Message ID Yk7I6/fKXrv7Mpsd@kam.mff.cuni.cz
State Committed
Commit aabb9a261ef060cf24fd626713f1d7d9df81aa57
Headers
Series Fix pure/const propagation in modref |

Commit Message

Jan Hubicka April 7, 2022, 11:20 a.m. UTC
  Hi,
this patch fixes ipa-modref propagation of pure/const functions.  When we inline
function, the modref summary is updated to represent the function after
inlining and there we need to propagate nondeterministic and side-effects flag.

Bootstrapped/regtested x86_64-linux, will commit it shortly.

Honza

gcc/ChangeLog:

2022-04-07  Jan Hubicka  <hubicka@ucw.cz>

	PR ipa/105160
	* ipa-modref.cc (ipa_merge_modref_summary_after_inlining):

gcc/testsuite/ChangeLog:

2022-04-07  Jan Hubicka  <hubicka@ucw.cz>

	PR ipa/105160
	* gcc.dg/ipa/pr105160.c: New test.
  

Comments

Richard Biener April 7, 2022, 11:25 a.m. UTC | #1
On Thu, Apr 7, 2022 at 1:20 PM Jan Hubicka via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Hi,
> this patch fixes ipa-modref propagation of pure/const functions.  When we inline
> function, the modref summary is updated to represent the function after
> inlining and there we need to propagate nondeterministic and side-effects flag.
>
> Bootstrapped/regtested x86_64-linux, will commit it shortly.

I noticed we have ->writes_errno in the summary which we do not handle
in this function either?  That's also missing on the GCC 11 branch
(where I just picked r12-2851-g9851a1631f2915 and
r12-5538-ga70faf6e4df748, both still in testing).

Richard.

> Honza
>
> gcc/ChangeLog:
>
> 2022-04-07  Jan Hubicka  <hubicka@ucw.cz>
>
>         PR ipa/105160
>         * ipa-modref.cc (ipa_merge_modref_summary_after_inlining):
>
> gcc/testsuite/ChangeLog:
>
> 2022-04-07  Jan Hubicka  <hubicka@ucw.cz>
>
>         PR ipa/105160
>         * gcc.dg/ipa/pr105160.c: New test.
>
> diff --git a/gcc/ipa-modref.cc b/gcc/ipa-modref.cc
> index acfd7d80ff8..556816ab429 100644
> --- a/gcc/ipa-modref.cc
> +++ b/gcc/ipa-modref.cc
> @@ -5281,6 +5281,29 @@ ipa_merge_modref_summary_after_inlining (cgraph_edge *edge)
>        if (!ignore_stores)
>         to_info_lto->stores->collapse ();
>      }
> +  /* Merge side effects and non-determinism.
> +     PURE/CONST flags makes functions deterministic and if there is
> +     no LOOPING_CONST_OR_PURE they also have no side effects.  */
> +  if (!(flags & (ECF_CONST | ECF_NOVOPS | ECF_PURE))
> +      || (flags & ECF_LOOPING_CONST_OR_PURE))
> +    {
> +      if (to_info)
> +       {
> +         if (!callee_info || callee_info->side_effects)
> +           to_info->side_effects = true;
> +         if ((!callee_info || callee_info->nondeterministic)
> +             && !ignore_nondeterminism_p (edge->caller->decl, flags))
> +           to_info->nondeterministic = true;
> +       }
> +      if (to_info_lto)
> +       {
> +         if (!callee_info_lto || callee_info_lto->side_effects)
> +           to_info_lto->side_effects = true;
> +         if ((!callee_info_lto || callee_info_lto->nondeterministic)
> +             && !ignore_nondeterminism_p (edge->caller->decl, flags))
> +           to_info_lto->nondeterministic = true;
> +       }
> +     }
>    if (callee_info || callee_info_lto)
>      {
>        auto_vec <modref_parm_map, 32> parm_map;
> diff --git a/gcc/testsuite/gcc.dg/ipa/pr105160.c b/gcc/testsuite/gcc.dg/ipa/pr105160.c
> new file mode 100644
> index 00000000000..ea80545b102
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/ipa/pr105160.c
> @@ -0,0 +1,77 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O -fdump-ipa-modref" } */
> +#define sysreg_read(regname)           \
> +({                                     \
> +       unsigned long __sr_val;         \
> +       asm volatile("");               \
> +                                       \
> +       __sr_val;                       \
> +})
> +
> +#define sysreg_write(regname, __sw_val)        \
> +do {                                   \
> +       asm volatile("");                       \
> +} while (0)
> +
> +#define isb()                          \
> +do {                                   \
> +       asm volatile(                   \
> +       "isb"                           \
> +       :                               \
> +       :                               \
> +       : "memory");                    \
> +} while (0)
> +
> +static unsigned long sctlr_read(void)
> +{
> +       return sysreg_read(sctlr_el1);
> +}
> +
> +static void sctlr_write(unsigned long val)
> +{
> +       sysreg_write(sctlr_el1, val);
> +}
> +
> +static void sctlr_rmw(void)
> +{
> +       unsigned long val;
> +
> +       val = sctlr_read();
> +       val |= 1UL << 7;
> +       sctlr_write(val);
> +}
> +
> +void sctlr_read_multiple(void)
> +{
> +       sctlr_read();
> +       sctlr_read();
> +       sctlr_read();
> +       sctlr_read();
> +}
> +
> +void sctlr_write_multiple(void)
> +{
> +       sctlr_write(0);
> +       sctlr_write(0);
> +       sctlr_write(0);
> +       sctlr_write(0);
> +       sctlr_write(0);
> +}
> +
> +void sctlr_rmw_multiple(void)
> +{
> +       sctlr_rmw();
> +       sctlr_rmw();
> +       sctlr_rmw();
> +       sctlr_rmw();
> +}
> +
> +void function(void)
> +{
> +       sctlr_read_multiple();
> +       sctlr_write_multiple();
> +       sctlr_rmw_multiple();
> +
> +       isb();
> +}
> +/* { dg-final { scan-ipa-dump-not "Function found to be const" "modref"  } } */
  
Jan Hubicka April 7, 2022, 11:33 a.m. UTC | #2
> On Thu, Apr 7, 2022 at 1:20 PM Jan Hubicka via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > Hi,
> > this patch fixes ipa-modref propagation of pure/const functions.  When we inline
> > function, the modref summary is updated to represent the function after
> > inlining and there we need to propagate nondeterministic and side-effects flag.
> >
> > Bootstrapped/regtested x86_64-linux, will commit it shortly.
> 
> I noticed we have ->writes_errno in the summary which we do not handle
> in this function either?  That's also missing on the GCC 11 branch

writes_errno should be safe: it only can be set if function calls
something that has writes_errno in fnspec and that is propagated at IPA
time.  Similarly with calls_interposable.

> (where I just picked r12-2851-g9851a1631f2915 and
> r12-5538-ga70faf6e4df748, both still in testing).

Thanks!  I will look into gcc 11 backports as well after fixing the P1s.
Honza
> 
> Richard.
> 
> > Honza
> >
> > gcc/ChangeLog:
> >
> > 2022-04-07  Jan Hubicka  <hubicka@ucw.cz>
> >
> >         PR ipa/105160
> >         * ipa-modref.cc (ipa_merge_modref_summary_after_inlining):
> >
> > gcc/testsuite/ChangeLog:
> >
> > 2022-04-07  Jan Hubicka  <hubicka@ucw.cz>
> >
> >         PR ipa/105160
> >         * gcc.dg/ipa/pr105160.c: New test.
> >
> > diff --git a/gcc/ipa-modref.cc b/gcc/ipa-modref.cc
> > index acfd7d80ff8..556816ab429 100644
> > --- a/gcc/ipa-modref.cc
> > +++ b/gcc/ipa-modref.cc
> > @@ -5281,6 +5281,29 @@ ipa_merge_modref_summary_after_inlining (cgraph_edge *edge)
> >        if (!ignore_stores)
> >         to_info_lto->stores->collapse ();
> >      }
> > +  /* Merge side effects and non-determinism.
> > +     PURE/CONST flags makes functions deterministic and if there is
> > +     no LOOPING_CONST_OR_PURE they also have no side effects.  */
> > +  if (!(flags & (ECF_CONST | ECF_NOVOPS | ECF_PURE))
> > +      || (flags & ECF_LOOPING_CONST_OR_PURE))
> > +    {
> > +      if (to_info)
> > +       {
> > +         if (!callee_info || callee_info->side_effects)
> > +           to_info->side_effects = true;
> > +         if ((!callee_info || callee_info->nondeterministic)
> > +             && !ignore_nondeterminism_p (edge->caller->decl, flags))
> > +           to_info->nondeterministic = true;
> > +       }
> > +      if (to_info_lto)
> > +       {
> > +         if (!callee_info_lto || callee_info_lto->side_effects)
> > +           to_info_lto->side_effects = true;
> > +         if ((!callee_info_lto || callee_info_lto->nondeterministic)
> > +             && !ignore_nondeterminism_p (edge->caller->decl, flags))
> > +           to_info_lto->nondeterministic = true;
> > +       }
> > +     }
> >    if (callee_info || callee_info_lto)
> >      {
> >        auto_vec <modref_parm_map, 32> parm_map;
> > diff --git a/gcc/testsuite/gcc.dg/ipa/pr105160.c b/gcc/testsuite/gcc.dg/ipa/pr105160.c
> > new file mode 100644
> > index 00000000000..ea80545b102
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/ipa/pr105160.c
> > @@ -0,0 +1,77 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O -fdump-ipa-modref" } */
> > +#define sysreg_read(regname)           \
> > +({                                     \
> > +       unsigned long __sr_val;         \
> > +       asm volatile("");               \
> > +                                       \
> > +       __sr_val;                       \
> > +})
> > +
> > +#define sysreg_write(regname, __sw_val)        \
> > +do {                                   \
> > +       asm volatile("");                       \
> > +} while (0)
> > +
> > +#define isb()                          \
> > +do {                                   \
> > +       asm volatile(                   \
> > +       "isb"                           \
> > +       :                               \
> > +       :                               \
> > +       : "memory");                    \
> > +} while (0)
> > +
> > +static unsigned long sctlr_read(void)
> > +{
> > +       return sysreg_read(sctlr_el1);
> > +}
> > +
> > +static void sctlr_write(unsigned long val)
> > +{
> > +       sysreg_write(sctlr_el1, val);
> > +}
> > +
> > +static void sctlr_rmw(void)
> > +{
> > +       unsigned long val;
> > +
> > +       val = sctlr_read();
> > +       val |= 1UL << 7;
> > +       sctlr_write(val);
> > +}
> > +
> > +void sctlr_read_multiple(void)
> > +{
> > +       sctlr_read();
> > +       sctlr_read();
> > +       sctlr_read();
> > +       sctlr_read();
> > +}
> > +
> > +void sctlr_write_multiple(void)
> > +{
> > +       sctlr_write(0);
> > +       sctlr_write(0);
> > +       sctlr_write(0);
> > +       sctlr_write(0);
> > +       sctlr_write(0);
> > +}
> > +
> > +void sctlr_rmw_multiple(void)
> > +{
> > +       sctlr_rmw();
> > +       sctlr_rmw();
> > +       sctlr_rmw();
> > +       sctlr_rmw();
> > +}
> > +
> > +void function(void)
> > +{
> > +       sctlr_read_multiple();
> > +       sctlr_write_multiple();
> > +       sctlr_rmw_multiple();
> > +
> > +       isb();
> > +}
> > +/* { dg-final { scan-ipa-dump-not "Function found to be const" "modref"  } } */
  

Patch

diff --git a/gcc/ipa-modref.cc b/gcc/ipa-modref.cc
index acfd7d80ff8..556816ab429 100644
--- a/gcc/ipa-modref.cc
+++ b/gcc/ipa-modref.cc
@@ -5281,6 +5281,29 @@  ipa_merge_modref_summary_after_inlining (cgraph_edge *edge)
       if (!ignore_stores)
 	to_info_lto->stores->collapse ();
     }
+  /* Merge side effects and non-determinism.
+     PURE/CONST flags makes functions deterministic and if there is
+     no LOOPING_CONST_OR_PURE they also have no side effects.  */
+  if (!(flags & (ECF_CONST | ECF_NOVOPS | ECF_PURE))
+      || (flags & ECF_LOOPING_CONST_OR_PURE))
+    {
+      if (to_info)
+	{
+	  if (!callee_info || callee_info->side_effects)
+	    to_info->side_effects = true;
+	  if ((!callee_info || callee_info->nondeterministic)
+	      && !ignore_nondeterminism_p (edge->caller->decl, flags))
+	    to_info->nondeterministic = true;
+	}
+      if (to_info_lto)
+	{
+	  if (!callee_info_lto || callee_info_lto->side_effects)
+	    to_info_lto->side_effects = true;
+	  if ((!callee_info_lto || callee_info_lto->nondeterministic)
+	      && !ignore_nondeterminism_p (edge->caller->decl, flags))
+	    to_info_lto->nondeterministic = true;
+	}
+     }
   if (callee_info || callee_info_lto)
     {
       auto_vec <modref_parm_map, 32> parm_map;
diff --git a/gcc/testsuite/gcc.dg/ipa/pr105160.c b/gcc/testsuite/gcc.dg/ipa/pr105160.c
new file mode 100644
index 00000000000..ea80545b102
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr105160.c
@@ -0,0 +1,77 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-ipa-modref" } */
+#define sysreg_read(regname)		\
+({					\
+	unsigned long __sr_val;		\
+	asm volatile("");		\
+					\
+	__sr_val;			\
+})
+
+#define sysreg_write(regname, __sw_val)	\
+do {					\
+	asm volatile("");			\
+} while (0)
+
+#define isb()				\
+do {					\
+	asm volatile(			\
+	"isb"				\
+	:				\
+	:				\
+	: "memory");			\
+} while (0)
+
+static unsigned long sctlr_read(void)
+{
+	return sysreg_read(sctlr_el1);
+}
+
+static void sctlr_write(unsigned long val)
+{
+	sysreg_write(sctlr_el1, val);
+}
+
+static void sctlr_rmw(void)
+{
+	unsigned long val;
+
+	val = sctlr_read();
+	val |= 1UL << 7;
+	sctlr_write(val);
+}
+
+void sctlr_read_multiple(void)
+{
+	sctlr_read();
+	sctlr_read();
+	sctlr_read();
+	sctlr_read();
+}
+
+void sctlr_write_multiple(void)
+{
+	sctlr_write(0);
+	sctlr_write(0);
+	sctlr_write(0);
+	sctlr_write(0);
+	sctlr_write(0);
+}
+
+void sctlr_rmw_multiple(void)
+{
+	sctlr_rmw();
+	sctlr_rmw();
+	sctlr_rmw();
+	sctlr_rmw();
+}
+
+void function(void)
+{
+	sctlr_read_multiple();
+	sctlr_write_multiple();
+	sctlr_rmw_multiple();
+
+	isb();
+}
+/* { dg-final { scan-ipa-dump-not "Function found to be const" "modref"  } } */