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

2022-12-15 Thread Jan Hubicka via Gcc-patches
> On Wed, Dec 14, 2022 at 4:20 PM Jan Hubicka via Gcc-patches
>  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  
> > >
> > >   * ipa-sra.cc (create_parameter_descriptors): Consider the first
> > >   parameter of a method safe to dereference.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > > 2022-12-14  Martin Jambor  
> > >
> > >   * 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


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

2022-12-15 Thread Richard Biener via Gcc-patches
On Wed, Dec 14, 2022 at 4:20 PM Jan Hubicka via Gcc-patches
 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  
> >
> >   * ipa-sra.cc (create_parameter_descriptors): Consider the first
> >   parameter of a method safe to dereference.
> >
> > gcc/testsuite/ChangeLog:
> >
> > 2022-12-14  Martin Jambor  
> >
> >   * 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

2022-12-14 Thread Jan Hubicka via Gcc-patches
> 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  
> 
>   * ipa-sra.cc (create_parameter_descriptors): Consider the first
>   parameter of a method safe to dereference.
> 
> gcc/testsuite/ChangeLog:
> 
> 2022-12-14  Martin Jambor  
> 
>   * g++.dg/ipa/ipa-sra-6.C: New test.

OK,
thanks a lot!
Honza


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

2022-12-14 Thread 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  

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

gcc/testsuite/ChangeLog:

2022-12-14  Martin Jambor  

* 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 000..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 = 
+  something ();
+  bar (gp, cond);
+}
+
+void
+hoo(void)
+{
+  gp = _instance;
+}
+
+/* { dg-final { scan-ipa-dump "Will split parameter" "sra" } } */
-- 
2.38.1