[PATCH] fortran: Detect duplicate unlimited polymorphic types [PR103662]

[PATCH] fortran: Detect duplicate unlimited polymorphic types [PR103662]

[Mikael Morin]

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

Hello,

this is a fix for PR103662, a TBAA issue with unlimited polymorphic types.

I attached a draft patch to the PR which was accumulating all unlimited
polymorphic symbols to a single namespace, avoiding duplicate symbols
and thus eliminating the problem.

After reviewing the code more in detail, I was afraid that some symbols
could still end up in the local namespace, and that the problem would
remain for them after all.

Despite not being able to generate a testcase where it happened, I
decided to produce a patch based on Jakub’s analysis in the PR audit
trail, as that way supports duplicates by design.

On top of Jakub’s patch, there are a couple more types registrations
just in case (they handle duplicates so that’s fine), and the type
comparison fix that he was too fortran-uncomfortable to do.

The testcase had to be fixed as we found out in the PR audit trail.

Regression tested on x86_64-pc-linux-gnu. OK for master?

Mikael

[-- Attachment #2: 0001-fortran-Detect-duplicate-unlimited-polymorphic-types.patch --]
[-- Type: text/x-patch, Size: 8109 bytes --]

From ff9de8b00e5eedf44af0ce75d268dce216bf645f Mon Sep 17 00:00:00 2001
From: Mikael Morin <mikael@gcc.gnu.org>
Date: Wed, 20 Apr 2022 12:04:38 +0200
Subject: [PATCH] fortran: Detect duplicate unlimited polymorphic types
 [PR103662]
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This fixes a type-based alias analysis issue with unlimited polymorphic
class descriptors (types behind class(*)) causing data initialisation to
be removed by optimization.

The fortran front-end may create multiple declarations for types, for
example if a type is redeclared in each program unit it is used in.
To avoid optimization seeing them as non-aliasing, a list of derived
types is created at resolution time, and used at translation to set
the same TYPE_CANONICAL type for each duplicate type declaration.

This mechanism didn’t work for unlimited polymorphic descriptors types,
as there is a short-circuit return skipping all the resolution handling
for them, including the type registration.

This change adds type registration (which handles duplicate registering)
at several short-circuit returns, and updates type comparison to handle
specifically unlimited polymorphic fake symbols, class descriptor types
and virtual table types.

The test, which exhibited mismatching dynamic types had to be fixed as
well.

	PR fortran/103662

gcc/fortran/ChangeLog:

	* interface.cc (gfc_compare_derived_types): Support comparing
	unlimited polymorphic fake symbols.  Recursively compare class
	descriptor types and virtual table types.
	* resolve.cc (resolve_fl_derived): Add type to the types list
	on unlimited polymorphic short-circuit return.
	(resolve_symbol): Ditto.

gcc/testsuite/ChangeLog:

	* gfortran.dg/unlimited_polymorphic_3.f03 (foo): Separate
	bind(c) and sequence checks to...
	(foo_bc, foo_sq): ... two different procedures.
	(main, foo*): Change type declarations so that type name,
	component name, and either bind(c) or sequence attribute match
	between the main type declarations and the procedure type
	declarations.
	(toplevel): Add optimization dump checks.

Co-Authored-By: Jakub Jelinek <jakub@redhat.com>
---
 gcc/fortran/interface.cc                      | 19 +++++--
 gcc/fortran/resolve.cc                        | 15 ++++-
 .../gfortran.dg/unlimited_polymorphic_3.f03   | 56 +++++++++++++------
 3 files changed, 66 insertions(+), 24 deletions(-)

diff --git a/gcc/fortran/interface.cc b/gcc/fortran/interface.cc
index 000a530cba4..7ed6e13711f 100644
--- a/gcc/fortran/interface.cc
+++ b/gcc/fortran/interface.cc
@@ -618,6 +618,14 @@ gfc_compare_derived_types (gfc_symbol *derived1, gfc_symbol *derived2)
   if (!derived1 || !derived2)
     gfc_internal_error ("gfc_compare_derived_types: invalid derived type");
 
+  if (derived1->attr.unlimited_polymorphic
+      && derived2->attr.unlimited_polymorphic)
+    return true;
+
+  if (derived1->attr.unlimited_polymorphic
+      != derived2->attr.unlimited_polymorphic)
+    return false;
+
   /* Compare UNION types specially.  */
   if (derived1->attr.flavor == FL_UNION || derived2->attr.flavor == FL_UNION)
     return compare_union_types (derived1, derived2);
@@ -630,10 +638,11 @@ gfc_compare_derived_types (gfc_symbol *derived1, gfc_symbol *derived2)
       && strcmp (derived1->module, derived2->module) == 0)
     return true;
 
-  /* Compare type via the rules of the standard.  Both types must have
-     the SEQUENCE or BIND(C) attribute to be equal. STRUCTUREs are special
-     because they can be anonymous; therefore two structures with different
-     names may be equal.  */
+  /* Compare type via the rules of the standard.  Both types must have the
+     SEQUENCE or BIND(C) attribute to be equal.  We also compare types
+     recursively if they are class descriptors types or virtual tables types.
+     STRUCTUREs are special because they can be anonymous; therefore two
+     structures with different names may be equal.  */
 
   /* Compare names, but not for anonymous types such as UNION or MAP.  */
   if (!is_anonymous_dt (derived1) && !is_anonymous_dt (derived2)
@@ -646,6 +655,8 @@ gfc_compare_derived_types (gfc_symbol *derived1, gfc_symbol *derived2)
 
   if (!(derived1->attr.sequence && derived2->attr.sequence)
       && !(derived1->attr.is_bind_c && derived2->attr.is_bind_c)
+      && !(derived1->attr.is_class && derived2->attr.is_class)
+      && !(derived1->attr.vtype && derived2->attr.vtype)
       && !(derived1->attr.pdt_type && derived2->attr.pdt_type))
     return false;
 
diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index 21c8797c938..011fb5eb14d 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -15112,7 +15112,10 @@ resolve_fl_derived (gfc_symbol *sym)
   gfc_symbol *gen_dt = NULL;
 
   if (sym->attr.unlimited_polymorphic)
-    return true;
+    {
+      add_dt_to_dt_list (sym);
+      return true;
+    }
 
   if (!sym->attr.is_class)
     gfc_find_symbol (sym->name, sym->ns, 0, &gen_dt);
@@ -15150,7 +15153,10 @@ resolve_fl_derived (gfc_symbol *sym)
 
       /* Nothing more to do for unlimited polymorphic entities.  */
       if (data->ts.u.derived->attr.unlimited_polymorphic)
-	return true;
+	{
+	  add_dt_to_dt_list (sym);
+	  return true;
+	}
       else if (vptr->ts.u.derived == NULL)
 	{
 	  gfc_symbol *vtab = gfc_find_derived_vtab (data->ts.u.derived);
@@ -15467,7 +15473,10 @@ resolve_symbol (gfc_symbol *sym)
     return;
 
   if (sym->attr.unlimited_polymorphic)
-    return;
+    {
+      add_dt_to_dt_list (sym);
+      return;
+    }
 
   if (sym->attr.flavor == FL_UNKNOWN
       || (sym->attr.flavor == FL_PROCEDURE && !sym->attr.intrinsic
diff --git a/gcc/testsuite/gfortran.dg/unlimited_polymorphic_3.f03 b/gcc/testsuite/gfortran.dg/unlimited_polymorphic_3.f03
index 075d6d727e2..780d68cdd87 100644
--- a/gcc/testsuite/gfortran.dg/unlimited_polymorphic_3.f03
+++ b/gcc/testsuite/gfortran.dg/unlimited_polymorphic_3.f03
@@ -1,4 +1,5 @@
 ! { dg-do run }
+! { dg-additional-options "-fdump-tree-dse-details" }
 !
 ! Check that pointer assignments allowed by F2003:C717
 ! work and check null initialization of CLASS(*) pointers.
@@ -7,20 +8,31 @@
 !
 program main
   interface
-    subroutine foo(z)
+    subroutine foo_bc(z)
       class(*), pointer, intent(in) :: z
-    end subroutine foo
+    end subroutine foo_bc
+    subroutine foo_sq(z)
+      class(*), pointer, intent(in) :: z
+    end subroutine foo_sq
   end interface
+  type, bind(c) :: bc
+    integer :: i
+  end type bc
   type sq
     sequence
-    integer :: i
+    integer :: k
   end type sq
+  type(bc), target :: w
   type(sq), target :: x
   class(*), pointer :: y, z
-  x%i = 42
+  w%i = 23
+  y => w
+  z => y ! unlimited => unlimited allowed
+  call foo_bc(z)
+  x%k = 42
   y => x
   z => y ! unlimited => unlimited allowed
-  call foo (z)
+  call foo_sq(z)
   call bar
 contains
   subroutine bar
@@ -33,21 +45,31 @@ contains
 
 end program main
 
-
-subroutine foo(tgt)
+subroutine foo_bc(tgt)
   use iso_c_binding
   class(*), pointer, intent(in) :: tgt
-  type, bind(c) :: s
-    integer (c_int) :: k
-  end type s
-  type t
+  type, bind(c) :: bc
+    integer (c_int) :: i
+  end type bc
+  type(bc), pointer :: ptr1
+  ptr1 => tgt ! bind(c) => unlimited allowed
+  if (ptr1%i .ne. 23) STOP 2
+end subroutine foo_bc
+
+subroutine foo_sq(tgt)
+  class(*), pointer, intent(in) :: tgt
+  type sq
     sequence
     integer :: k
-  end type t
-  type(s), pointer :: ptr1
-  type(t), pointer :: ptr2
-  ptr1 => tgt ! bind(c) => unlimited allowed
-  if (ptr1%k .ne. 42) STOP 2
+  end type sq
+  type(sq), pointer :: ptr2
   ptr2 => tgt ! sequence type => unlimited allowed
   if (ptr2%k .ne. 42) STOP 3
-end subroutine foo
+end subroutine foo_sq
+
+! PR fortran/103662
+! We used to produce multiple independant types for the unlimited polymorphic
+! descriptors (types for class(*)) which caused stores to them to be seen as
+! useless.
+! { dg-final { scan-tree-dump-not "Deleted dead store: z._data = &w" "dse1" } }
+! { dg-final { scan-tree-dump-not "Deleted dead store: z._data = &x" "dse1" } }
-- 
2.35.1

[PATCH v2] fortran: Detect duplicate unlimited polymorphic types [PR103662]

[Mikael Morin]

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

Le 21/04/2022 à 23:14, Mikael Morin a écrit :
> Hello,
> 
> this is a fix for PR103662, a TBAA issue with unlimited polymorphic types.
> 
> I attached a draft patch to the PR which was accumulating all unlimited
> polymorphic symbols to a single namespace, avoiding duplicate symbols
> and thus eliminating the problem.
> 
> After reviewing the code more in detail, I was afraid that some symbols
> could still end up in the local namespace, and that the problem would
> remain for them after all.
> 
> Despite not being able to generate a testcase where it happened, I
> decided to produce a patch based on Jakub’s analysis in the PR audit
> trail, as that way supports duplicates by design.
> 
> On top of Jakub’s patch, there are a couple more types registrations
> just in case (they handle duplicates so that’s fine), and the type
> comparison fix that he was too fortran-uncomfortable to do.
> 
> The testcase had to be fixed as we found out in the PR audit trail.
> 
> Regression tested on x86_64-pc-linux-gnu. OK for master?
> 
> Mikael

I have read Jakub’s analysis again, and it says the type registration is 
useless for unlimited polymorphic fake symbols, as they are all 
translated as ptr_type_node.
So it can be dropped, which brings this v2 patch closer to Jakub’s original.

Regression tested again. OK?

[-- Attachment #2: 0001-fortran-Detect-duplicate-unlimited-polymorphic-types.patch --]
[-- Type: text/x-patch, Size: 7442 bytes --]

From e53ecc979ec2a798626eb94c60d18b015d6f52e5 Mon Sep 17 00:00:00 2001
From: Mikael Morin <mikael@gcc.gnu.org>
Date: Wed, 20 Apr 2022 12:04:38 +0200
Subject: [PATCH v2] fortran: Detect duplicate unlimited polymorphic types
 [PR103662]
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This fixes a type-based alias analysis issue with unlimited polymorphic
class descriptors (types behind class(*)) causing data initialisation to
be removed by optimization.

The fortran front-end may create multiple declarations for types, for
example if a type is redeclared in each program unit it is used in.
To avoid optimization seeing them as non-aliasing, a list of derived
types is created at resolution time, and used at translation to set
the same TYPE_CANONICAL type for each duplicate type declaration.

This mechanism didn’t work for unlimited polymorphic descriptors types,
as there is a short-circuit return skipping all the resolution handling
for them, including the type registration.

This change adds type registration at the short-circuit return, and
updates type comparison to handle specifically unlimited polymorphic
fake symbols, class descriptor types and virtual table types.

The test, which exhibited mismatching dynamic types had to be fixed as
well.

	PR fortran/103662

gcc/fortran/ChangeLog:

	* interface.cc (gfc_compare_derived_types): Support comparing
	unlimited polymorphic fake symbols.  Recursively compare class
	descriptor types and virtual table types.
	* resolve.cc (resolve_fl_derived): Add type to the types list
	on unlimited polymorphic short-circuit return.

gcc/testsuite/ChangeLog:

	* gfortran.dg/unlimited_polymorphic_3.f03 (foo): Separate
	bind(c) and sequence checks to...
	(foo_bc, foo_sq): ... two different procedures.
	(main, foo*): Change type declarations so that type name,
	component name, and either bind(c) or sequence attribute match
	between the main type declarations and the procedure type
	declarations.
	(toplevel): Add optimization dump checks.

Co-Authored-By: Jakub Jelinek <jakub@redhat.com>
---
 gcc/fortran/interface.cc                      | 19 +++++--
 gcc/fortran/resolve.cc                        |  5 +-
 .../gfortran.dg/unlimited_polymorphic_3.f03   | 56 +++++++++++++------
 3 files changed, 58 insertions(+), 22 deletions(-)

diff --git a/gcc/fortran/interface.cc b/gcc/fortran/interface.cc
index 000a530cba4..7ed6e13711f 100644
--- a/gcc/fortran/interface.cc
+++ b/gcc/fortran/interface.cc
@@ -618,6 +618,14 @@ gfc_compare_derived_types (gfc_symbol *derived1, gfc_symbol *derived2)
   if (!derived1 || !derived2)
     gfc_internal_error ("gfc_compare_derived_types: invalid derived type");
 
+  if (derived1->attr.unlimited_polymorphic
+      && derived2->attr.unlimited_polymorphic)
+    return true;
+
+  if (derived1->attr.unlimited_polymorphic
+      != derived2->attr.unlimited_polymorphic)
+    return false;
+
   /* Compare UNION types specially.  */
   if (derived1->attr.flavor == FL_UNION || derived2->attr.flavor == FL_UNION)
     return compare_union_types (derived1, derived2);
@@ -630,10 +638,11 @@ gfc_compare_derived_types (gfc_symbol *derived1, gfc_symbol *derived2)
       && strcmp (derived1->module, derived2->module) == 0)
     return true;
 
-  /* Compare type via the rules of the standard.  Both types must have
-     the SEQUENCE or BIND(C) attribute to be equal. STRUCTUREs are special
-     because they can be anonymous; therefore two structures with different
-     names may be equal.  */
+  /* Compare type via the rules of the standard.  Both types must have the
+     SEQUENCE or BIND(C) attribute to be equal.  We also compare types
+     recursively if they are class descriptors types or virtual tables types.
+     STRUCTUREs are special because they can be anonymous; therefore two
+     structures with different names may be equal.  */
 
   /* Compare names, but not for anonymous types such as UNION or MAP.  */
   if (!is_anonymous_dt (derived1) && !is_anonymous_dt (derived2)
@@ -646,6 +655,8 @@ gfc_compare_derived_types (gfc_symbol *derived1, gfc_symbol *derived2)
 
   if (!(derived1->attr.sequence && derived2->attr.sequence)
       && !(derived1->attr.is_bind_c && derived2->attr.is_bind_c)
+      && !(derived1->attr.is_class && derived2->attr.is_class)
+      && !(derived1->attr.vtype && derived2->attr.vtype)
       && !(derived1->attr.pdt_type && derived2->attr.pdt_type))
     return false;
 
diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index 21c8797c938..e7a4b79b40c 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -15150,7 +15150,10 @@ resolve_fl_derived (gfc_symbol *sym)
 
       /* Nothing more to do for unlimited polymorphic entities.  */
       if (data->ts.u.derived->attr.unlimited_polymorphic)
-	return true;
+	{
+	  add_dt_to_dt_list (sym);
+	  return true;
+	}
       else if (vptr->ts.u.derived == NULL)
 	{
 	  gfc_symbol *vtab = gfc_find_derived_vtab (data->ts.u.derived);
diff --git a/gcc/testsuite/gfortran.dg/unlimited_polymorphic_3.f03 b/gcc/testsuite/gfortran.dg/unlimited_polymorphic_3.f03
index 075d6d727e2..780d68cdd87 100644
--- a/gcc/testsuite/gfortran.dg/unlimited_polymorphic_3.f03
+++ b/gcc/testsuite/gfortran.dg/unlimited_polymorphic_3.f03
@@ -1,4 +1,5 @@
 ! { dg-do run }
+! { dg-additional-options "-fdump-tree-dse-details" }
 !
 ! Check that pointer assignments allowed by F2003:C717
 ! work and check null initialization of CLASS(*) pointers.
@@ -7,20 +8,31 @@
 !
 program main
   interface
-    subroutine foo(z)
+    subroutine foo_bc(z)
       class(*), pointer, intent(in) :: z
-    end subroutine foo
+    end subroutine foo_bc
+    subroutine foo_sq(z)
+      class(*), pointer, intent(in) :: z
+    end subroutine foo_sq
   end interface
+  type, bind(c) :: bc
+    integer :: i
+  end type bc
   type sq
     sequence
-    integer :: i
+    integer :: k
   end type sq
+  type(bc), target :: w
   type(sq), target :: x
   class(*), pointer :: y, z
-  x%i = 42
+  w%i = 23
+  y => w
+  z => y ! unlimited => unlimited allowed
+  call foo_bc(z)
+  x%k = 42
   y => x
   z => y ! unlimited => unlimited allowed
-  call foo (z)
+  call foo_sq(z)
   call bar
 contains
   subroutine bar
@@ -33,21 +45,31 @@ contains
 
 end program main
 
-
-subroutine foo(tgt)
+subroutine foo_bc(tgt)
   use iso_c_binding
   class(*), pointer, intent(in) :: tgt
-  type, bind(c) :: s
-    integer (c_int) :: k
-  end type s
-  type t
+  type, bind(c) :: bc
+    integer (c_int) :: i
+  end type bc
+  type(bc), pointer :: ptr1
+  ptr1 => tgt ! bind(c) => unlimited allowed
+  if (ptr1%i .ne. 23) STOP 2
+end subroutine foo_bc
+
+subroutine foo_sq(tgt)
+  class(*), pointer, intent(in) :: tgt
+  type sq
     sequence
     integer :: k
-  end type t
-  type(s), pointer :: ptr1
-  type(t), pointer :: ptr2
-  ptr1 => tgt ! bind(c) => unlimited allowed
-  if (ptr1%k .ne. 42) STOP 2
+  end type sq
+  type(sq), pointer :: ptr2
   ptr2 => tgt ! sequence type => unlimited allowed
   if (ptr2%k .ne. 42) STOP 3
-end subroutine foo
+end subroutine foo_sq
+
+! PR fortran/103662
+! We used to produce multiple independant types for the unlimited polymorphic
+! descriptors (types for class(*)) which caused stores to them to be seen as
+! useless.
+! { dg-final { scan-tree-dump-not "Deleted dead store: z._data = &w" "dse1" } }
+! { dg-final { scan-tree-dump-not "Deleted dead store: z._data = &x" "dse1" } }
-- 
2.35.1

Re: [PATCH v2] fortran: Detect duplicate unlimited polymorphic types [PR103662]

[Harald Anlauf]

Hi Mikael,

Am 22.04.22 um 12:53 schrieb Mikael Morin:
> Le 21/04/2022 à 23:14, Mikael Morin a écrit :
>> Hello,
>>
>> this is a fix for PR103662, a TBAA issue with unlimited polymorphic
>> types.
>>
>> I attached a draft patch to the PR which was accumulating all unlimited
>> polymorphic symbols to a single namespace, avoiding duplicate symbols
>> and thus eliminating the problem.
>>
>> After reviewing the code more in detail, I was afraid that some symbols
>> could still end up in the local namespace, and that the problem would
>> remain for them after all.
>>
>> Despite not being able to generate a testcase where it happened, I
>> decided to produce a patch based on Jakub’s analysis in the PR audit
>> trail, as that way supports duplicates by design.
>>
>> On top of Jakub’s patch, there are a couple more types registrations
>> just in case (they handle duplicates so that’s fine), and the type
>> comparison fix that he was too fortran-uncomfortable to do.
>>
>> The testcase had to be fixed as we found out in the PR audit trail.
>>
>> Regression tested on x86_64-pc-linux-gnu. OK for master?
>>
>> Mikael
>
> I have read Jakub’s analysis again, and it says the type registration is
> useless for unlimited polymorphic fake symbols, as they are all
> translated as ptr_type_node.
> So it can be dropped, which brings this v2 patch closer to Jakub’s
> original.
>
> Regression tested again. OK?

LGTM.

Thanks for the patch!

Harald

[pushed] testsuite: add additional option to force DSE execution [PR103662]

[Mikael Morin]

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

I have just pushed the attached fix for two UNRESOLVED checks at 
-O0 that I hadn’t seen.

[-- Attachment #2: 0001-testsuite-add-additional-option-to-force-DSE-executi.patch --]
[-- Type: text/x-patch, Size: 1382 bytes --]

From 6cc26f3037a18b9a958b4ac2a1363149a7fccd39 Mon Sep 17 00:00:00 2001
From: Mikael Morin <mikael@gcc.gnu.org>
Date: Mon, 25 Apr 2022 13:14:20 +0200
Subject: [pushed] =?UTF-8?q?testsuite:=C2=A0add=20additional=20option=20to?=
 =?UTF-8?q?=20force=20DSE=20execution=20[PR103662]?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This fixes a dump tree match check that is UNRESOLVED with the -O0
optimization option, as the optimization pass corresponding to the
dump file is not run at -O0, and the dump is not generated.

	PR fortran/103662

gcc/testsuite/ChangeLog:

	* gfortran.dg/unlimited_polymorphic_3.f03: Force execution of
	the DSE optimization pass.
---
 gcc/testsuite/gfortran.dg/unlimited_polymorphic_3.f03 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/gfortran.dg/unlimited_polymorphic_3.f03 b/gcc/testsuite/gfortran.dg/unlimited_polymorphic_3.f03
index 780d68cdd87..4104de6ac68 100644
--- a/gcc/testsuite/gfortran.dg/unlimited_polymorphic_3.f03
+++ b/gcc/testsuite/gfortran.dg/unlimited_polymorphic_3.f03
@@ -1,5 +1,5 @@
 ! { dg-do run }
-! { dg-additional-options "-fdump-tree-dse-details" }
+! { dg-additional-options "-ftree-dse -fdump-tree-dse-details" }
 !
 ! Check that pointer assignments allowed by F2003:C717
 ! work and check null initialization of CLASS(*) pointers.
-- 
2.35.1

Re: [pushed] testsuite: add additional option to force DSE execution [PR103662]

[Jakub Jelinek]

On Mon, Apr 25, 2022 at 01:38:25PM +0200, Mikael Morin wrote:
> I have just pushed the attached fix for two UNRESOLVED checks at -O0 that I
> hadn’t seen.

I don't like forcing of DSE in -O0 compilation, wouldn't it be better
to just not check the dse dump at -O0 like in the following patch?

Even better would be to check that the z._data = stores are both present
in *.optimized dump, but that doesn't really work at -O2 or above because
we inline the functions and optimize it completely away (both the stores
and corresponding reads).

The first hunk is needed so that __OPTIMIZE__ effective target works in
Fortran testsuite, otherwise one gets a pedantic error and __OPTIMIZE__
is considered not to match at all.

2022-04-25  Jakub Jelinek  <jakub@redhat.com>

	PR fortran/103662
	* lib/target-supports.exp (check_effective_target___OPTIMIZE__): Add
	a var definition to avoid pedwarn about empty translation unit.
	* gfortran.dg/unlimited_polymorphic_3.f03: Remove -ftree-dse from
	dg-additional-options, guard scan-tree-dump-not directives on
	__OPTIMIZE__ target.

--- gcc/testsuite/lib/target-supports.exp.jj	2022-04-22 13:36:56.150960826 +0200
+++ gcc/testsuite/lib/target-supports.exp	2022-04-25 14:06:21.620537483 +0200
@@ -11770,6 +11770,7 @@ proc check_effective_target___OPTIMIZE__
 	#ifndef __OPTIMIZE__
 	# error nein
 	#endif
+	int optimize;
     } [current_compiler_flags]]
 }
 
--- gcc/testsuite/gfortran.dg/unlimited_polymorphic_3.f03.jj	2022-04-25 13:54:38.320309119 +0200
+++ gcc/testsuite/gfortran.dg/unlimited_polymorphic_3.f03	2022-04-25 14:04:01.346486431 +0200
@@ -1,5 +1,5 @@
 ! { dg-do run }
-! { dg-additional-options "-ftree-dse -fdump-tree-dse-details" }
+! { dg-additional-options "-fdump-tree-dse-details" }
 !
 ! Check that pointer assignments allowed by F2003:C717
 ! work and check null initialization of CLASS(*) pointers.
@@ -71,5 +71,5 @@ end subroutine foo_sq
 ! We used to produce multiple independant types for the unlimited polymorphic
 ! descriptors (types for class(*)) which caused stores to them to be seen as
 ! useless.
-! { dg-final { scan-tree-dump-not "Deleted dead store: z._data = &w" "dse1" } }
-! { dg-final { scan-tree-dump-not "Deleted dead store: z._data = &x" "dse1" } }
+! { dg-final { scan-tree-dump-not "Deleted dead store: z._data = &w" "dse1" { target __OPTIMIZE__ } } }
+! { dg-final { scan-tree-dump-not "Deleted dead store: z._data = &x" "dse1" { target __OPTIMIZE__ } } }


	Jakub

Re: [pushed] testsuite: add additional option to force DSE execution [PR103662]

[Mikael Morin]

Le 25/04/2022 à 14:12, Jakub Jelinek a écrit :
> On Mon, Apr 25, 2022 at 01:38:25PM +0200, Mikael Morin wrote:
>> I have just pushed the attached fix for two UNRESOLVED checks at -O0 that I
>> hadn’t seen.
> 
> I don't like forcing of DSE in -O0 compilation, wouldn't it be better
> to just not check the dse dump at -O0 like in the following patch?
> 
Certainly better.  I actually looked for something alike already 
existing before trying to write a conditional in the dg-final code and 
finally deciding to go the easy way.

> Even better would be to check that the z._data = stores are both present
> in *.optimized dump, but that doesn't really work at -O2 or above because
> we inline the functions and optimize it completely away (both the stores
> and corresponding reads).
> 
Yes, and global optimization is already checked by test execution anyway.

> The first hunk is needed so that __OPTIMIZE__ effective target works in
> Fortran testsuite, otherwise one gets a pedantic error and __OPTIMIZE__
> is considered not to match at all.
> 
Maybe worth putting as code comment.

OK from my side, thanks.

Re: [pushed] testsuite: add additional option to force DSE execution [PR103662]

[Bernhard Reutner-Fischer]

On 25 April 2022 14:12:30 CEST, Jakub Jelinek via Fortran <fortran@gcc.gnu.org> wrote:
>On Mon, Apr 25, 2022 at 01:38:25PM +0200, Mikael Morin wrote:
>> I have just pushed the attached fix for two UNRESOLVED checks at -O0 that I
>> hadn’t seen.
>
>I don't like forcing of DSE in -O0 compilation, wouldn't it be better
>to just not check the dse dump at -O0 like in the following patch?

As Mikael already said, that's preferable indeed.

>
>Even better would be to check that the z._data = stores are both present
>in *.optimized dump, but that doesn't really work at -O2 or above because
>we inline the functions and optimize it completely away (both the stores
>and corresponding reads).
>
>The first hunk is needed so that __OPTIMIZE__ effective target works in
>Fortran testsuite, otherwise one gets a pedantic error and __OPTIMIZE__
>is considered not to match at all.
>
>2022-04-25  Jakub Jelinek  <jakub@redhat.com>
>
>	PR fortran/103662
>	* lib/target-supports.exp (check_effective_target___OPTIMIZE__): Add
>	a var definition to avoid pedwarn about empty translation unit.
>	* gfortran.dg/unlimited_polymorphic_3.f03: Remove -ftree-dse from
>	dg-additional-options, guard scan-tree-dump-not directives on
>	__OPTIMIZE__ target.
>
>--- gcc/testsuite/lib/target-supports.exp.jj	2022-04-22 13:36:56.150960826 +0200
>+++ gcc/testsuite/lib/target-supports.exp	2022-04-25 14:06:21.620537483 +0200
>@@ -11770,6 +11770,7 @@ proc check_effective_target___OPTIMIZE__
> 	#ifndef __OPTIMIZE__
> 	# error nein

Hmz. I _think_ that was /me and reading it again now, we should should not bluntly say nein but more something like
unwilling to ignore __OPTIMIZE__, dude

or anything more descriptive and universally understandably I assume.

> 	#endif
>+	int optimize;

a plain ; won't cut it. int dummy, maybe though. There is probably a lot more of these, aren't there?

thanks,

>     } [current_compiler_flags]]
> }
> 
>--- gcc/testsuite/gfortran.dg/unlimited_polymorphic_3.f03.jj	2022-04-25 13:54:38.320309119 +0200
>+++ gcc/testsuite/gfortran.dg/unlimited_polymorphic_3.f03	2022-04-25 14:04:01.346486431 +0200
>@@ -1,5 +1,5 @@
> ! { dg-do run }
>-! { dg-additional-options "-ftree-dse -fdump-tree-dse-details" }
>+! { dg-additional-options "-fdump-tree-dse-details" }
> !
> ! Check that pointer assignments allowed by F2003:C717
> ! work and check null initialization of CLASS(*) pointers.
>@@ -71,5 +71,5 @@ end subroutine foo_sq
> ! We used to produce multiple independant types for the unlimited polymorphic
> ! descriptors (types for class(*)) which caused stores to them to be seen as
> ! useless.
>-! { dg-final { scan-tree-dump-not "Deleted dead store: z._data = &w" "dse1" } }
>-! { dg-final { scan-tree-dump-not "Deleted dead store: z._data = &x" "dse1" } }
>+! { dg-final { scan-tree-dump-not "Deleted dead store: z._data = &w" "dse1" { target __OPTIMIZE__ } } }
>+! { dg-final { scan-tree-dump-not "Deleted dead store: z._data = &x" "dse1" { target __OPTIMIZE__ } } }
>
>
>	Jakub
>