[PATCH] ipa-sra: Consider the first parameter of methods safe to dereference

[PATCH] ipa-sra: Consider the first parameter of methods safe to dereference

[Martin Jambor]

Hi,

Honza requested this after reviewing the patch that taught IPA-SRA
that REFERENCE_TYPEs are always non-NULL that the pass also handles
the first parameters of methods, this pointers, in the same way.  So
this patch does that.

The patch is undergoing bootstrap and testing on an x86_64-linux right
now.  OK if it passes?

Thanks,

Martin


gcc/ChangeLog:

2022-12-14  Martin Jambor  <mjambor@suse.cz>

	* ipa-sra.cc (create_parameter_descriptors): Consider the first
	parameter of a method safe to dereference.

gcc/testsuite/ChangeLog:

2022-12-14  Martin Jambor  <mjambor@suse.cz>

	* g++.dg/ipa/ipa-sra-6.C: New test.
---
 gcc/ipa-sra.cc                       |  7 +++-
 gcc/testsuite/g++.dg/ipa/ipa-sra-6.C | 62 ++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/ipa/ipa-sra-6.C

diff --git a/gcc/ipa-sra.cc b/gcc/ipa-sra.cc
index bcabdedfc6c..6fe336eeb19 100644
--- a/gcc/ipa-sra.cc
+++ b/gcc/ipa-sra.cc
@@ -1206,7 +1206,12 @@ create_parameter_descriptors (cgraph_node *node,
       if (POINTER_TYPE_P (type))
 	{
 	  desc->by_ref = true;
-	  desc->safe_ref = (TREE_CODE (type) == REFERENCE_TYPE);
+	  if (TREE_CODE (type) == REFERENCE_TYPE
+	      || (num == 0
+		  && TREE_CODE (TREE_TYPE (node->decl)) == METHOD_TYPE))
+	    desc->safe_ref = true;
+	  else
+	    desc->safe_ref = false;
 	  type = TREE_TYPE (type);
 
 	  if (TREE_CODE (type) == FUNCTION_TYPE
diff --git a/gcc/testsuite/g++.dg/ipa/ipa-sra-6.C b/gcc/testsuite/g++.dg/ipa/ipa-sra-6.C
new file mode 100644
index 00000000000..d6b7822533f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/ipa-sra-6.C
@@ -0,0 +1,62 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-ipa-sra"  } */
+
+namespace {
+
+class C
+{
+
+  int mi;
+
+public:
+  C (int i)
+    : mi(i)
+  {}
+
+  void foo (int c);
+};
+
+volatile int vi;
+
+
+void __attribute__((noinline))
+C::foo (int cond)
+{
+  int i;
+  if (cond)
+    i = mi;
+  else
+    i = 0;
+  vi = i;
+}
+
+static C c_instance(1);
+}
+
+void __attribute__((noinline))
+bar (C *p, int cond)
+{
+  p->foo (cond);
+}
+
+
+class C *gp;
+
+void something(void);
+
+void
+baz (int cond)
+{
+  C c(vi);
+  gp = &c;
+  something ();
+  bar (gp, cond);
+}
+
+void
+hoo(void)
+{
+  gp = &c_instance;
+}
+
+/* { dg-final { scan-ipa-dump "Will split parameter" "sra" } } */
-- 
2.38.1

Re: [PATCH] ipa-sra: Consider the first parameter of methods safe to dereference

[Jan Hubicka]

> Hi,
> 
> Honza requested this after reviewing the patch that taught IPA-SRA
> that REFERENCE_TYPEs are always non-NULL that the pass also handles
> the first parameters of methods, this pointers, in the same way.  So
> this patch does that.
> 
> The patch is undergoing bootstrap and testing on an x86_64-linux right
> now.  OK if it passes?
> 
> Thanks,
> 
> Martin
> 
> 
> gcc/ChangeLog:
> 
> 2022-12-14  Martin Jambor  <mjambor@suse.cz>
> 
> 	* ipa-sra.cc (create_parameter_descriptors): Consider the first
> 	parameter of a method safe to dereference.
> 
> gcc/testsuite/ChangeLog:
> 
> 2022-12-14  Martin Jambor  <mjambor@suse.cz>
> 
> 	* g++.dg/ipa/ipa-sra-6.C: New test.

OK,
thanks a lot!
Honza

Re: [PATCH] ipa-sra: Consider the first parameter of methods safe to dereference

[Richard Biener]

On Wed, Dec 14, 2022 at 4:20 PM Jan Hubicka via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> > Hi,
> >
> > Honza requested this after reviewing the patch that taught IPA-SRA
> > that REFERENCE_TYPEs are always non-NULL that the pass also handles
> > the first parameters of methods, this pointers, in the same way.  So
> > this patch does that.
> >
> > The patch is undergoing bootstrap and testing on an x86_64-linux right
> > now.  OK if it passes?
> >
> > Thanks,
> >
> > Martin
> >
> >
> > gcc/ChangeLog:
> >
> > 2022-12-14  Martin Jambor  <mjambor@suse.cz>
> >
> >       * ipa-sra.cc (create_parameter_descriptors): Consider the first
> >       parameter of a method safe to dereference.
> >
> > gcc/testsuite/ChangeLog:
> >
> > 2022-12-14  Martin Jambor  <mjambor@suse.cz>
> >
> >       * g++.dg/ipa/ipa-sra-6.C: New test.
>
> OK,

Are you sure that's safe?  The docs for METHOD_TYPE doesn't say 'this'
is the first parameter nor does it say methods cannot be invoked for a
nullptr object.

Do frontends other than C++ create METHOD_TYPE functions?  Grep
shows uses in ada and objective C at least.

> thanks a lot!
> Honza

Re: [PATCH] ipa-sra: Consider the first parameter of methods safe to dereference

[Jan Hubicka]

> On Wed, Dec 14, 2022 at 4:20 PM Jan Hubicka via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > > Hi,
> > >
> > > Honza requested this after reviewing the patch that taught IPA-SRA
> > > that REFERENCE_TYPEs are always non-NULL that the pass also handles
> > > the first parameters of methods, this pointers, in the same way.  So
> > > this patch does that.
> > >
> > > The patch is undergoing bootstrap and testing on an x86_64-linux right
> > > now.  OK if it passes?
> > >
> > > Thanks,
> > >
> > > Martin
> > >
> > >
> > > gcc/ChangeLog:
> > >
> > > 2022-12-14  Martin Jambor  <mjambor@suse.cz>
> > >
> > >       * ipa-sra.cc (create_parameter_descriptors): Consider the first
> > >       parameter of a method safe to dereference.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > > 2022-12-14  Martin Jambor  <mjambor@suse.cz>
> > >
> > >       * g++.dg/ipa/ipa-sra-6.C: New test.
> >
> > OK,
> 
> Are you sure that's safe?  The docs for METHOD_TYPE doesn't say 'this'
> is the first parameter nor does it say methods cannot be invoked for a
> nullptr object.
> 
> Do frontends other than C++ create METHOD_TYPE functions?  Grep
> shows uses in ada and objective C at least.

I think for Objective-C++ calling method is also considerd a
dereference.  We make similar assumptions in devirtualization code for
some time.  We could also check that the type is METHOD_TYPE satisfies
odr_type_p if we want to look specifically for C++ methods.

Honza
> 
> > thanks a lot!
> > Honza