[PATCH 0/5] remove tm.h from encoding.c

[PATCH 0/5] remove tm.h from encoding.c

[tbsaunde+gcc]

From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>

Hi,

Its not the nicest code in the world, and there's definitely room for cleanups,
however it seems like an improvement.  After this series the only usage of tm.h
in libobjc is thr.c which only uses tm.h so it can include gthr.h which uses
SUPPORTS_WEAK and possibly other target macros.

bootstrapped + regtested patches individually on x86_64-linux-gnu, also tested the
series as a whole doesn't regress check-objc on ppc64{,le}-linux-gnu and AIX,
ok?

Trev

Trevor Saunders (4):
  remove usage of ROUND_TYPE_SIZE from encoding.c
  stop using ROUND_TYPE_ALIGN in libobjc/
  remove usage of BIGGEST_FIELD_ALIGNMENT in encoding.c
  remove usage of ADJUST_FIELD_ALIGN in encoding.c

pault (1):
  2015-01-25  Paul Thomas  <pault@gcc.gnu.org>

 gcc/fortran/ChangeLog                              |  21 +-
 gcc/fortran/trans-array.c                          |  32 +++
 gcc/fortran/trans-expr.c                           |  70 ++++---
 gcc/fortran/trans-stmt.c                           |   9 +-
 gcc/testsuite/ChangeLog                            |  11 ++
 .../gfortran.dg/allocate_with_source_12.f03        |  38 ++++
 .../gfortran.dg/allocate_with_source_13.f03        | 220 +++++++++++++++++++++
 .../gfortran.dg/allocate_with_source_14.f03        | 214 ++++++++++++++++++++
 libobjc/encoding.c                                 |  48 +++--
 9 files changed, 613 insertions(+), 50 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/allocate_with_source_12.f03
 create mode 100644 gcc/testsuite/gfortran.dg/allocate_with_source_13.f03
 create mode 100644 gcc/testsuite/gfortran.dg/allocate_with_source_14.f03

-- 
2.6.2

[PATCH 1/5] 2015-01-25 Paul Thomas <pault@gcc.gnu.org>

[tbsaunde+gcc]

From: pault <pault@138bc75d-0d04-0410-961f-82ee72b054a4>

	PR fortran/67171
	* trans-array.c (structure_alloc_comps): On deallocation of
	class components, reset the vptr to the declared type vtable
	and reset the _len field of unlimited polymorphic components.
	*trans-expr.c (gfc_find_and_cut_at_last_class_ref): Bail out on
	allocatable component references to the right of part reference
	with non-zero rank and return NULL.
	(gfc_reset_vptr): Simplify this function by using the function
	gfc_get_vptr_from_expr. Return if the vptr is NULL_TREE.
	(gfc_reset_len): If gfc_find_and_cut_at_last_class_ref returns
	NULL return.
	* trans-stmt.c (gfc_trans_allocate): Rely on the use of
	gfc_trans_assignment if expr3 is a variable expression since
	this deals correctly with array sections.

2015-01-25  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/67171
	* gfortran.dg/allocate_with_source_12.f03: New test

	PR fortran/61819
	* gfortran.dg/allocate_with_source_13.f03: New test

	PR fortran/61830
	* gfortran.dg/allocate_with_source_14.f03: New test


git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@229303 138bc75d-0d04-0410-961f-82ee72b054a4
---
 gcc/fortran/ChangeLog                              |  21 +-
 gcc/fortran/trans-array.c                          |  32 +++
 gcc/fortran/trans-expr.c                           |  70 ++++---
 gcc/fortran/trans-stmt.c                           |   9 +-
 gcc/testsuite/ChangeLog                            |  11 ++
 .../gfortran.dg/allocate_with_source_12.f03        |  38 ++++
 .../gfortran.dg/allocate_with_source_13.f03        | 220 +++++++++++++++++++++
 .../gfortran.dg/allocate_with_source_14.f03        | 214 ++++++++++++++++++++
 8 files changed, 579 insertions(+), 36 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/allocate_with_source_12.f03
 create mode 100644 gcc/testsuite/gfortran.dg/allocate_with_source_13.f03
 create mode 100644 gcc/testsuite/gfortran.dg/allocate_with_source_14.f03

diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
index 1a351be..668013d 100644
--- a/gcc/fortran/ChangeLog
+++ b/gcc/fortran/ChangeLog
@@ -1,8 +1,25 @@
+2015-01-25  Paul Thomas  <pault@gcc.gnu.org>
+
+	PR fortran/67171
+	* trans-array.c (structure_alloc_comps): On deallocation of
+	class components, reset the vptr to the declared type vtable
+	and reset the _len field of unlimited polymorphic components.
+	*trans-expr.c (gfc_find_and_cut_at_last_class_ref): Bail out on
+	allocatable component references to the right of part reference
+	with non-zero rank and return NULL.
+	(gfc_reset_vptr): Simplify this function by using the function
+	gfc_get_vptr_from_expr. Return if the vptr is NULL_TREE.
+	(gfc_reset_len): If gfc_find_and_cut_at_last_class_ref returns
+	NULL return.
+	* trans-stmt.c (gfc_trans_allocate): Rely on the use of
+	gfc_trans_assignment if expr3 is a variable expression since
+	this deals correctly with array sections.
+
 2015-10-25  Andre Vehreschild  <vehre@gcc.gnu.org>
 
 	PR fortran/66927
-	PR fortran/67044	
-	* trans-array.c (build_array_ref): Modified call to 
+	PR fortran/67044
+	* trans-array.c (build_array_ref): Modified call to
 	gfc_get_class_array_ref to adhere to new interface.
 	(gfc_conv_expr_descriptor): For one-based arrays that
 	are filled by a loop starting at one the start index of the
diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index 45c18a5..b726998 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -8024,6 +8024,38 @@ structure_alloc_comps (gfc_symbol * der_type, tree decl,
 					 build_int_cst (TREE_TYPE (comp), 0));
 		}
 	      gfc_add_expr_to_block (&tmpblock, tmp);
+
+	      /* Finally, reset the vptr to the declared type vtable and, if
+		 necessary reset the _len field.
+
+		 First recover the reference to the component and obtain
+		 the vptr.  */
+	      comp = fold_build3_loc (input_location, COMPONENT_REF, ctype,
+				     decl, cdecl, NULL_TREE);
+	      tmp = gfc_class_vptr_get (comp);
+
+	      if (UNLIMITED_POLY (c))
+		{
+		  /* Both vptr and _len field should be nulled.  */
+		  gfc_add_modify (&tmpblock, tmp,
+				  build_int_cst (TREE_TYPE (tmp), 0));
+		  tmp = gfc_class_len_get (comp);
+		  gfc_add_modify (&tmpblock, tmp,
+				  build_int_cst (TREE_TYPE (tmp), 0));
+		}
+	      else
+		{
+		  /* Build the vtable address and set the vptr with it.  */
+		  tree vtab;
+		  gfc_symbol *vtable;
+		  vtable = gfc_find_derived_vtab (c->ts.u.derived);
+		  vtab = vtable->backend_decl;
+		  if (vtab == NULL_TREE)
+		    vtab = gfc_get_symbol_decl (vtable);
+		  vtab = gfc_build_addr_expr (NULL, vtab);
+		  vtab = fold_convert (TREE_TYPE (tmp), vtab);
+		  gfc_add_modify (&tmpblock, tmp, vtab);
+		}
 	    }
 
 	  if (cmp_has_alloc_comps
diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c
index 9585de6..f8ed0df 100644
--- a/gcc/fortran/trans-expr.c
+++ b/gcc/fortran/trans-expr.c
@@ -271,15 +271,29 @@ gfc_expr *
 gfc_find_and_cut_at_last_class_ref (gfc_expr *e)
 {
   gfc_expr *base_expr;
-  gfc_ref *ref, *class_ref, *tail;
+  gfc_ref *ref, *class_ref, *tail, *array_ref;
 
   /* Find the last class reference.  */
   class_ref = NULL;
+  array_ref = NULL;
   for (ref = e->ref; ref; ref = ref->next)
     {
+      if (ref->type == REF_ARRAY
+	  && ref->u.ar.type != AR_ELEMENT)
+	array_ref = ref;
+
       if (ref->type == REF_COMPONENT
 	  && ref->u.c.component->ts.type == BT_CLASS)
+	{
+	  /* Component to the right of a part reference with nonzero rank
+	     must not have the ALLOCATABLE attribute.  If attempts are
+	     made to reference such a component reference, an error results
+	     followed by anICE.  */
+	  if (array_ref
+	      && CLASS_DATA (ref->u.c.component)->attr.allocatable)
+	    return NULL;
 	class_ref = ref;
+	}
 
       if (ref->next == NULL)
 	break;
@@ -320,47 +334,37 @@ gfc_find_and_cut_at_last_class_ref (gfc_expr *e)
 void
 gfc_reset_vptr (stmtblock_t *block, gfc_expr *e)
 {
-  gfc_expr *rhs, *lhs = gfc_copy_expr (e);
   gfc_symbol *vtab;
-  tree tmp;
-  gfc_ref *ref;
+  tree vptr;
+  tree vtable;
+  gfc_se se;
 
-  /* If we have a class array, we need go back to the class
-     container.  */
-  if (lhs->ref && lhs->ref->next && !lhs->ref->next->next
-      && lhs->ref->next->type == REF_ARRAY
-      && lhs->ref->next->u.ar.type == AR_FULL
-      && lhs->ref->type == REF_COMPONENT
-      && strcmp (lhs->ref->u.c.component->name, "_data") == 0)
-    {
-      gfc_free_ref_list (lhs->ref);
-      lhs->ref = NULL;
-    }
+  /* Evaluate the expression and obtain the vptr from it.  */
+  gfc_init_se (&se, NULL);
+  if (e->rank)
+    gfc_conv_expr_descriptor (&se, e);
   else
-    for (ref = lhs->ref; ref; ref = ref->next)
-      if (ref->next && ref->next->next && !ref->next->next->next
-	  && ref->next->next->type == REF_ARRAY
-	  && ref->next->next->u.ar.type == AR_FULL
-	  && ref->next->type == REF_COMPONENT
-	  && strcmp (ref->next->u.c.component->name, "_data") == 0)
-	{
-	  gfc_free_ref_list (ref->next);
-	  ref->next = NULL;
-	}
+    gfc_conv_expr (&se, e);
+  gfc_add_block_to_block (block, &se.pre);
+  vptr = gfc_get_vptr_from_expr (se.expr);
 
-  gfc_add_vptr_component (lhs);
+  /* If a vptr is not found, we can do nothing more.  */
+  if (vptr == NULL_TREE)
+    return;
 
   if (UNLIMITED_POLY (e))
-    rhs = gfc_get_null_expr (NULL);
+    gfc_add_modify (block, vptr, build_int_cst (TREE_TYPE (vptr), 0));
   else
     {
+      /* Return the vptr to the address of the declared type.  */
       vtab = gfc_find_derived_vtab (e->ts.u.derived);
-      rhs = gfc_lval_expr_from_sym (vtab);
+      vtable = vtab->backend_decl;
+      if (vtable == NULL_TREE)
+	vtable = gfc_get_symbol_decl (vtab);
+      vtable = gfc_build_addr_expr (NULL, vtable);
+      vtable = fold_convert (TREE_TYPE (vptr), vtable);
+      gfc_add_modify (block, vptr, vtable);
     }
-  tmp = gfc_trans_pointer_assignment (lhs, rhs);
-  gfc_add_expr_to_block (block, tmp);
-  gfc_free_expr (lhs);
-  gfc_free_expr (rhs);
 }
 
 
@@ -372,6 +376,8 @@ gfc_reset_len (stmtblock_t *block, gfc_expr *expr)
   gfc_expr *e;
   gfc_se se_len;
   e = gfc_find_and_cut_at_last_class_ref (expr);
+  if (e == NULL)
+    return;
   gfc_add_len_component (e);
   gfc_init_se (&se_len, NULL);
   gfc_conv_expr (&se_len, e);
diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c
index 1bd131e..85558f0 100644
--- a/gcc/fortran/trans-stmt.c
+++ b/gcc/fortran/trans-stmt.c
@@ -5379,8 +5379,13 @@ gfc_trans_allocate (gfc_code * code)
 	     will benefit of every enhancements gfc_trans_assignment ()
 	     gets.
 	     No need to check whether e3_is is E3_UNSET, because that is
-	     done by expr3 != NULL_TREE.  */
-	  if (e3_is != E3_MOLD && expr3 != NULL_TREE
+	     done by expr3 != NULL_TREE.
+	     Exclude variables since the following block does not handle
+	     array sections. In any case, there is no harm in sending
+	     variables to gfc_trans_assignment because there is no
+	     evaluation of variables.  */
+	  if (code->expr3->expr_type != EXPR_VARIABLE
+	      && e3_is != E3_MOLD && expr3 != NULL_TREE
 	      && DECL_P (expr3) && DECL_ARTIFICIAL (expr3))
 	    {
 	      /* Build a temporary symtree and symbol.  Do not add it to
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 8024273..8ecfd09 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,14 @@
+2015-01-25  Paul Thomas  <pault@gcc.gnu.org>
+
+	PR fortran/67171
+	* gfortran.dg/allocate_with_source_12.f03: New test
+
+	PR fortran/61819
+	* gfortran.dg/allocate_with_source_13.f03: New test
+
+	PR fortran/61830
+	* gfortran.dg/allocate_with_source_14.f03: New test
+
 2015-10-25  John David Anglin  <danglin@gcc.gnu.org>
 
 	* g++.dg/Wno-frame-address.C: Skip on hppa*-*-*.
diff --git a/gcc/testsuite/gfortran.dg/allocate_with_source_12.f03 b/gcc/testsuite/gfortran.dg/allocate_with_source_12.f03
new file mode 100644
index 0000000..76deb61
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/allocate_with_source_12.f03
@@ -0,0 +1,38 @@
+! { dg-do run }
+!
+! Checks the fix for PR67171, where the second ALLOCATE with and array section
+! SOURCE produced a zero index based temporary, which threw the assignment.
+!
+! Contributed by Anton Shterenlikht  <mexas@bristol.ac.uk>
+!
+program z
+  implicit none
+  integer, parameter :: DIM1_SIZE = 10
+  real, allocatable :: d(:,:), tmp(:,:)
+  integer :: i, errstat
+
+  allocate (d(DIM1_SIZE, 2), source = 0.0, stat=errstat )
+
+  d(:,1) = [( real (i), i=1,DIM1_SIZE)]
+  d(:,2) = [( real(2*i), i=1,DIM1_SIZE)]
+!  write (*,*) d(1, :)
+
+  call move_alloc (from = d, to = tmp)
+!  write (*,*) tmp( 1, :)
+
+  allocate (d(DIM1_SIZE / 2, 2), source = tmp(1 : DIM1_SIZE / 2, :) , stat=errstat)
+  if (any (d .ne. tmp(1:DIM1_SIZE/2,:))) call abort
+  deallocate (d)
+
+  allocate (d(DIM1_SIZE / 2, 2), source = foo (tmp(1 : DIM1_SIZE / 2, :)) , stat=errstat)
+  if (any (d .ne. tmp(1 : DIM1_SIZE / 2, :))) call abort
+
+  deallocate (tmp , d)
+
+contains
+  function foo (arg) result (res)
+    real :: arg(:,:)
+    real :: res(size (arg, 1), size (arg, 2))
+    res = arg
+  end function
+end program z
diff --git a/gcc/testsuite/gfortran.dg/allocate_with_source_13.f03 b/gcc/testsuite/gfortran.dg/allocate_with_source_13.f03
new file mode 100644
index 0000000..27b5c17
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/allocate_with_source_13.f03
@@ -0,0 +1,220 @@
+! { dg-do compile }
+!
+! Tests the fix for PR61819.
+!
+! Contributed by Salvatore Filippone  <sfilippone@uniroma2.it>
+!
+module foo_base_mod
+  integer, parameter :: foo_ipk_ = kind(1)
+  integer, parameter :: foo_dpk_ = kind(1.d0)
+  type foo_d_base_vect_type
+    real(foo_dpk_), allocatable :: v(:)
+  contains
+    procedure :: free     => d_base_free
+    procedure :: get_vect => d_base_get_vect
+    procedure :: allocate => d_base_allocate
+  end type foo_d_base_vect_type
+
+
+  type foo_d_vect_type
+    class(foo_d_base_vect_type), allocatable :: v
+  contains
+    procedure :: free     => d_vect_free
+    procedure :: get_vect => d_vect_get_vect
+  end type foo_d_vect_type
+
+  type foo_desc_type
+    integer(foo_ipk_) :: nl=-1
+  end type foo_desc_type
+
+
+contains
+
+  subroutine foo_init(ictxt)
+    integer :: ictxt
+  end subroutine foo_init
+
+
+  subroutine foo_exit(ictxt)
+    integer :: ictxt
+  end subroutine foo_exit
+
+  subroutine foo_info(ictxt,iam,np)
+    integer(foo_ipk_) :: ictxt,iam,np
+    iam = 0
+    np = 1
+  end subroutine foo_info
+
+  subroutine  foo_cdall(ictxt,map,info,nl)
+    integer(foo_ipk_) :: ictxt, info
+    type(foo_desc_type) :: map
+    integer(foo_ipk_), optional  :: nl
+
+    if (present(nl)) then
+      map%nl = nl
+    else
+      map%nl = 1
+    end if
+  end subroutine foo_cdall
+
+  subroutine  foo_cdasb(map,info)
+    integer(foo_ipk_) :: info
+    type(foo_desc_type) :: map
+    if (map%nl < 0) map%nl=1
+  end subroutine foo_cdasb
+
+
+  subroutine d_base_allocate(this,n)
+    class(foo_d_base_vect_type), intent(out) :: this
+
+    allocate(this%v(max(1,n)))
+
+  end subroutine d_base_allocate
+
+  subroutine d_base_free(this)
+    class(foo_d_base_vect_type), intent(inout) :: this
+    if (allocated(this%v)) &
+         & deallocate(this%v)
+  end subroutine d_base_free
+
+  function d_base_get_vect(this) result(res)
+    class(foo_d_base_vect_type), intent(inout) :: this
+    real(foo_dpk_), allocatable :: res(:)
+
+    if (allocated(this%v)) then
+      res = this%v
+    else
+      allocate(res(1))
+    end if
+  end function d_base_get_vect
+
+  subroutine d_vect_free(this)
+    class(foo_d_vect_type) :: this
+    if (allocated(this%v)) then
+      call this%v%free()
+      deallocate(this%v)
+    end if
+  end subroutine d_vect_free
+
+  function d_vect_get_vect(this) result(res)
+    class(foo_d_vect_type) :: this
+    real(foo_dpk_), allocatable :: res(:)
+
+    if (allocated(this%v)) then
+      res = this%v%get_vect()
+    else
+      allocate(res(1))
+    end if
+  end function d_vect_get_vect
+
+  subroutine foo_geall(v,map,info)
+    type(foo_d_vect_type), intent(out) :: v
+    type(foo_Desc_type) :: map
+    integer(foo_ipk_) :: info
+
+    allocate(foo_d_base_vect_type :: v%v,stat=info)
+    if (info == 0) call v%v%allocate(map%nl)
+  end subroutine foo_geall
+
+end module foo_base_mod
+
+
+module foo_scalar_field_mod
+  use foo_base_mod
+  implicit none
+
+  type scalar_field
+    type(foo_d_vect_type)        :: f
+    type(foo_desc_type), pointer :: map => null()
+  contains
+    procedure :: free
+  end type
+
+  integer(foo_ipk_), parameter :: nx=4,ny=nx, nz=nx
+  type(foo_desc_type), allocatable, save, target :: map
+  integer(foo_ipk_) ,save :: NumMy_xy_planes
+  integer(foo_ipk_) ,parameter :: NumGlobalElements = nx*ny*nz
+  integer(foo_ipk_) ,parameter :: NumGlobal_xy_planes = nz, Num_xy_points_per_plane = nx*ny
+
+contains
+  subroutine initialize_map(ictxt,NumMyElements,info)
+    integer(foo_ipk_) :: ictxt, NumMyElements, info
+    info = 0
+    if (allocated(map)) deallocate(map,stat=info)
+    if (info == 0) allocate(map,stat=info)
+    if (info == 0) call foo_cdall(ictxt,map,info,nl=NumMyElements)
+    if (info == 0) call foo_cdasb(map,info)
+  end subroutine initialize_map
+
+  function new_scalar_field(comm) result(this)
+    type(scalar_field)                          :: this
+    integer(foo_ipk_)              ,intent(in) :: comm
+    real(foo_dpk_) ,allocatable   :: f_v(:)
+    integer(foo_ipk_) :: i,j,k,NumMyElements, iam, np, info,ip
+    integer(foo_ipk_), allocatable :: idxs(:)
+    call foo_info(comm,iam,np)
+    NumMy_xy_planes = NumGlobal_xy_planes/np
+    NumMyElements = NumMy_xy_planes*Num_xy_points_per_plane
+    if (.not. allocated(map)) call initialize_map(comm,NumMyElements,info)
+    this%map => map
+    call foo_geall(this%f,this%map,info)
+  end function
+
+  subroutine free(this)
+    class(scalar_field), intent(inout) :: this
+    integer(foo_ipk_) ::info
+    write(0,*) 'Freeing scalar_this%f'
+    call this%f%free()
+  end subroutine free
+
+end module foo_scalar_field_mod
+
+module foo_vector_field_mod
+  use foo_base_mod
+  use foo_scalar_field_mod, only : scalar_field,new_scalar_field
+  implicit none
+  type vector_field
+    type(scalar_field) :: u(1)
+  contains
+    procedure :: free
+  end type
+contains
+  function new_vector_field(comm_in) result(this)
+    type(vector_field) :: this
+    integer(foo_ipk_), intent(in) :: comm_in
+    this%u = [new_scalar_field(comm_in)] ! Removing this line eliminates the memory leak
+  end function
+
+  subroutine free(this)
+    class(vector_field), intent(inout) :: this
+    integer :: i
+    associate(vf=>this%u)
+      do i=1, size(vf)
+        write(0,*) 'Freeing vector_this%u(',i,')'
+        call vf(i)%free()
+      end do
+    end associate
+  end subroutine free
+
+end module foo_vector_field_mod
+
+program main
+  use foo_base_mod
+  use foo_vector_field_mod,only: vector_field,new_vector_field
+  use foo_scalar_field_mod,only: map
+  implicit none
+  type(vector_field) :: u
+  type(foo_d_vect_type) :: v
+  real(foo_dpk_), allocatable :: av(:)
+  integer(foo_ipk_) :: ictxt, iam, np, i,info
+  call foo_init(ictxt)
+  call foo_info(ictxt,iam,np)
+  u = new_vector_field(ictxt)
+  call u%free()
+  do i=1,10
+    u = new_vector_field(ictxt)
+    call u%free()
+  end do
+  call u%free()
+  call foo_exit(ictxt)
+end program
diff --git a/gcc/testsuite/gfortran.dg/allocate_with_source_14.f03 b/gcc/testsuite/gfortran.dg/allocate_with_source_14.f03
new file mode 100644
index 0000000..36c1245
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/allocate_with_source_14.f03
@@ -0,0 +1,214 @@
+! { dg-do compile }
+! { dg-options "-fdump-tree-original" }
+!
+! Tests the fix for PR61830.
+!
+! Contributed by Salvatore Filippone  <sfilippone@uniroma2.it>
+!
+module foo_base_mod
+  integer, parameter :: foo_dpk_ = kind(1.d0)
+  type foo_d_base_vect_type
+    real(foo_dpk_), allocatable :: v(:)
+  contains
+    procedure :: free     => d_base_free
+    procedure :: get_vect => d_base_get_vect
+    procedure :: allocate => d_base_allocate
+  end type foo_d_base_vect_type
+
+
+  type foo_d_vect_type
+    class(foo_d_base_vect_type), allocatable :: v
+  contains
+    procedure :: free     => d_vect_free
+    procedure :: get_vect => d_vect_get_vect
+  end type foo_d_vect_type
+
+  type foo_desc_type
+    integer :: nl=-1
+  end type foo_desc_type
+
+contains
+
+  subroutine  foo_cdall(map,nl)
+    type(foo_desc_type) :: map
+    integer, optional  :: nl
+
+    if (present(nl)) then
+      map%nl = nl
+    else
+      map%nl = 1
+    end if
+  end subroutine foo_cdall
+
+
+  subroutine  foo_cdasb(map,info)
+    integer :: info
+    type(foo_desc_type) :: map
+    if (map%nl < 0) map%nl=1
+  end subroutine foo_cdasb
+
+
+
+  subroutine d_base_allocate(this,n)
+    class(foo_d_base_vect_type), intent(out) :: this
+
+    allocate(this%v(max(1,n)))
+
+  end subroutine d_base_allocate
+
+  subroutine d_base_free(this)
+    class(foo_d_base_vect_type), intent(inout) :: this
+    if (allocated(this%v))  then
+      write(0,*) 'Scalar deallocation'
+      deallocate(this%v)
+    end if
+  end subroutine d_base_free
+
+  function d_base_get_vect(this) result(res)
+    class(foo_d_base_vect_type), intent(inout) :: this
+    real(foo_dpk_), allocatable :: res(:)
+
+    if (allocated(this%v)) then
+      res = this%v
+    else
+      allocate(res(1))
+    end if
+  end function d_base_get_vect
+
+  subroutine d_vect_free(this)
+    class(foo_d_vect_type) :: this
+    if (allocated(this%v)) then
+      call this%v%free()
+      write(0,*) 'Deallocate class() component'
+      deallocate(this%v)
+    end if
+  end subroutine d_vect_free
+
+  function d_vect_get_vect(this) result(res)
+    class(foo_d_vect_type) :: this
+    real(foo_dpk_), allocatable :: res(:)
+
+    if (allocated(this%v)) then
+      res = this%v%get_vect()
+    else
+      allocate(res(1))
+    end if
+  end function d_vect_get_vect
+
+  subroutine foo_geall(v,map,info)
+    type(foo_d_vect_type), intent(out) :: v
+    type(foo_Desc_type) :: map
+    integer :: info
+
+    allocate(foo_d_base_vect_type :: v%v,stat=info)
+    if (info == 0) call v%v%allocate(map%nl)
+  end subroutine foo_geall
+
+end module foo_base_mod
+
+
+module foo_scalar_field_mod
+  use foo_base_mod
+  implicit none
+
+  type scalar_field
+    type(foo_d_vect_type)        :: f
+    type(foo_desc_type), pointer :: map => null()
+  contains
+    procedure :: free
+  end type
+
+  integer, parameter :: nx=4,ny=nx, nz=nx
+  type(foo_desc_type), allocatable, save, target :: map
+  integer ,save :: NumMy_xy_planes
+  integer ,parameter :: NumGlobalElements = nx*ny*nz
+  integer ,parameter :: NumGlobal_xy_planes = nz, &
+       & Num_xy_points_per_plane = nx*ny
+
+contains
+  subroutine initialize_map(NumMyElements)
+    integer :: NumMyElements, info
+    info = 0
+    if (allocated(map)) deallocate(map,stat=info)
+    if (info == 0) allocate(map,stat=info)
+    if (info == 0) call foo_cdall(map,nl=NumMyElements)
+    if (info == 0) call foo_cdasb(map,info)
+  end subroutine initialize_map
+
+  function new_scalar_field() result(this)
+    type(scalar_field)                          :: this
+    real(foo_dpk_) ,allocatable   :: f_v(:)
+    integer :: i,j,k,NumMyElements, iam, np, info,ip
+    integer, allocatable :: idxs(:)
+
+    NumMy_xy_planes = NumGlobal_xy_planes
+    NumMyElements = NumMy_xy_planes*Num_xy_points_per_plane
+    if (.not. allocated(map)) call initialize_map(NumMyElements)
+    this%map => map
+    call foo_geall(this%f,this%map,info)
+  end function
+
+  subroutine free(this)
+    class(scalar_field), intent(inout) :: this
+    integer ::info
+    call this%f%free()
+  end subroutine free
+
+end module foo_scalar_field_mod
+
+module foo_vector_field_mod
+  use foo_base_mod
+  use foo_scalar_field_mod
+  implicit none
+  type vector_field
+    type(scalar_field) :: u(1)
+  end type vector_field
+contains
+  function new_vector_field() result(this)
+    type(vector_field) :: this
+    integer :: i
+    do i=1, size(this%u)
+      associate(sf=>this%u(i))
+        sf = new_scalar_field()
+      end associate
+    end do
+  end function
+
+  subroutine free_v_field(this)
+    class(vector_field), intent(inout) :: this
+    integer :: i
+    associate(vf=>this%u)
+      do i=1, size(vf)
+        call vf(i)%free()
+      end do
+    end associate
+  end subroutine free_v_field
+
+end module foo_vector_field_mod
+
+program main
+  use foo_base_mod
+  use foo_vector_field_mod
+  use foo_scalar_field_mod
+  implicit none
+  type(vector_field) :: u
+  type(foo_d_vect_type) :: v
+  real(foo_dpk_), allocatable :: av(:)
+  integer  :: iam, np, i,info
+
+  u = new_vector_field()
+  call foo_geall(v,map,info)
+  call free_v_field(u)
+  do i=1,10
+    u = new_vector_field()
+    call free_v_field(u)
+    av = v%get_vect()
+  end do
+! This gets rid of the "memory leak"
+  if (associated (u%u(1)%map)) deallocate (u%u(1)%map)
+  call free_v_field(u)
+  call v%free()
+  deallocate(av)
+end program
+! { dg-final { scan-tree-dump-times "__builtin_malloc" 23 "original" } }
+! { dg-final { scan-tree-dump-times "__builtin_free" 29 "original" } }
-- 
2.6.2

Re: [PATCH 1/5] 2015-01-25 Paul Thomas <pault@gcc.gnu.org>

[Bernd Schmidt]

On 10/30/2015 12:48 PM, tbsaunde+gcc@tbsaunde.org wrote:
> From: pault <pault@138bc75d-0d04-0410-961f-82ee72b054a4>
>
> 	PR fortran/67171
> 	* trans-array.c (structure_alloc_comps): On deallocation of
> 	class components, reset the vptr to the declared type vtable
> 	and reset the _len field of unlimited polymorphic components.
> 	*trans-expr.c (gfc_find_and_cut_at_last_class_ref): Bail out on
> 	allocatable component references to the right of part reference
> 	with non-zero rank and return NULL.
> 	(gfc_reset_vptr): Simplify this function by using the function
> 	gfc_get_vptr_from_expr. Return if the vptr is NULL_TREE.
> 	(gfc_reset_len): If gfc_find_and_cut_at_last_class_ref returns
> 	NULL return.
> 	* trans-stmt.c (gfc_trans_allocate): Rely on the use of
> 	gfc_trans_assignment if expr3 is a variable expression since
> 	this deals correctly with array sections.

There's no explanation of this patch or how it relates to the others in 
this series. Did you send the wrong patch?


Bernd

Re: [PATCH 1/5] 2015-01-25 Paul Thomas <pault@gcc.gnu.org>

[Trevor Saunders]

On Fri, Oct 30, 2015 at 01:10:32PM +0100, Bernd Schmidt wrote:
> On 10/30/2015 12:48 PM, tbsaunde+gcc@tbsaunde.org wrote:
> >From: pault <pault@138bc75d-0d04-0410-961f-82ee72b054a4>
> >
> >	PR fortran/67171
> >	* trans-array.c (structure_alloc_comps): On deallocation of
> >	class components, reset the vptr to the declared type vtable
> >	and reset the _len field of unlimited polymorphic components.
> >	*trans-expr.c (gfc_find_and_cut_at_last_class_ref): Bail out on
> >	allocatable component references to the right of part reference
> >	with non-zero rank and return NULL.
> >	(gfc_reset_vptr): Simplify this function by using the function
> >	gfc_get_vptr_from_expr. Return if the vptr is NULL_TREE.
> >	(gfc_reset_len): If gfc_find_and_cut_at_last_class_ref returns
> >	NULL return.
> >	* trans-stmt.c (gfc_trans_allocate): Rely on the use of
> >	gfc_trans_assignment if expr3 is a variable expression since
> >	this deals correctly with array sections.
> 
> There's no explanation of this patch or how it relates to the others in this
> series. Did you send the wrong patch?

yes, sorry about that.

Trev

> 
> 
> Bernd
> 

[PATCH 2/5] remove usage of ROUND_TYPE_SIZE from encoding.c

[tbsaunde+gcc]

From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>

gcc got rid of this target macro in 2003, so it seems safe to assume the
alternate path works fine on all targets.

libobjc/ChangeLog:

2015-10-30  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>

	PR libobjc/24775
	* encoding.c (objc_layout_finish_structure): Remove usage of
	ROUND_TYPE_SIZE.
---
 libobjc/encoding.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/libobjc/encoding.c b/libobjc/encoding.c
index abb6145..7de768f 100644
--- a/libobjc/encoding.c
+++ b/libobjc/encoding.c
@@ -1245,14 +1245,8 @@ void objc_layout_finish_structure (struct objc_struct_layout *layout,
       layout->record_align = MAX (1, layout->record_align);
 #endif
 
-#ifdef ROUND_TYPE_SIZE
-      layout->record_size = ROUND_TYPE_SIZE (layout->original_type,
-                                             layout->record_size,
-                                             layout->record_align);
-#else
       /* Round the size up to be a multiple of the required alignment */
       layout->record_size = ROUND (layout->record_size, layout->record_align);
-#endif
 
       layout->type = NULL;
     }
-- 
2.6.2

Re: [PATCH 2/5] remove usage of ROUND_TYPE_SIZE from encoding.c

[Andrew Pinski]

On Fri, Oct 30, 2015 at 7:48 PM,  <tbsaunde+gcc@tbsaunde.org> wrote:
> From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
>
> gcc got rid of this target macro in 2003, so it seems safe to assume the
> alternate path works fine on all targets.

This is ok.


>
> libobjc/ChangeLog:
>
> 2015-10-30  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>
>
>         PR libobjc/24775
>         * encoding.c (objc_layout_finish_structure): Remove usage of
>         ROUND_TYPE_SIZE.
> ---
>  libobjc/encoding.c | 6 ------
>  1 file changed, 6 deletions(-)
>
> diff --git a/libobjc/encoding.c b/libobjc/encoding.c
> index abb6145..7de768f 100644
> --- a/libobjc/encoding.c
> +++ b/libobjc/encoding.c
> @@ -1245,14 +1245,8 @@ void objc_layout_finish_structure (struct objc_struct_layout *layout,
>        layout->record_align = MAX (1, layout->record_align);
>  #endif
>
> -#ifdef ROUND_TYPE_SIZE
> -      layout->record_size = ROUND_TYPE_SIZE (layout->original_type,
> -                                             layout->record_size,
> -                                             layout->record_align);
> -#else
>        /* Round the size up to be a multiple of the required alignment */
>        layout->record_size = ROUND (layout->record_size, layout->record_align);
> -#endif
>
>        layout->type = NULL;
>      }
> --
> 2.6.2
>

[PATCH 3/5] stop using ROUND_TYPE_ALIGN in libobjc/

[tbsaunde+gcc]

From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>

Given the layering violation that using ROUND_TYPE_ALIGN in target libs
is, and the hacks needed to make it work just coppying the relevant code
into encoding.c seems to make sense as an incremental improvement.  The
epiphany version of this macro called a function that doesn't exist in
target libs, so libobjc must not build on that target and not coppying
that macro definition doesn't make anything worse.  We already
explicitly prefered the default version to the macro for sparc so we
don't need to copy that version either.  On ppc linux64 and freebsd64
after constant folding values of other target macros used for libobjc
the macro turned out to be the same as the default version so we can
just use the default for them.  Which means the only version of the
macro we actually need to copy are for ppc-darwin and AIX.

libobjc/ChangeLog:

2015-10-30  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>

	PR libobjc/24775
	* encoding.c (objc_layout_finish_structure): Remove usage of
	ROUND_TYPE_ALIGN.
---
 libobjc/encoding.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/libobjc/encoding.c b/libobjc/encoding.c
index 7de768f..867372d 100644
--- a/libobjc/encoding.c
+++ b/libobjc/encoding.c
@@ -1237,10 +1237,22 @@ void objc_layout_finish_structure (struct objc_struct_layout *layout,
       /* Work out the alignment of the record as one expression and store
          in the record type.  Round it up to a multiple of the record's
          alignment. */
-#if defined (ROUND_TYPE_ALIGN) && ! defined (__sparc__)
-      layout->record_align = ROUND_TYPE_ALIGN (layout->original_type-1,
-                                               1,
-                                               layout->record_align);
+#if _AIX
+      char type = layout->original_type[-1];
+      if (type == '{' || type == '(')
+	layout->record_align =
+	  rs6000_special_round_type_align (layout->original_type-1, 1,
+					   layout->record_align);
+      else
+	layout->record_align = MAX (1, layout->record_align);
+#elif __POWERPC__ && __APPLE__
+      char type = layout->original_type[-1];
+      if (type == '{' || type == '(')
+	layout->record_align
+	  = darwin_rs6000_special_round_type_align (layout->original_type - 1,
+						    1, layout->record_align);
+      else
+	layout->record_align = MAX (1, layout->record_align);
 #else
       layout->record_align = MAX (1, layout->record_align);
 #endif
-- 
2.6.2

Re: [PATCH 3/5] stop using ROUND_TYPE_ALIGN in libobjc/

[Andrew Pinski]

On Fri, Oct 30, 2015 at 7:48 PM,  <tbsaunde+gcc@tbsaunde.org> wrote:
> From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
>
> Given the layering violation that using ROUND_TYPE_ALIGN in target libs
> is, and the hacks needed to make it work just coppying the relevant code
> into encoding.c seems to make sense as an incremental improvement.  The
> epiphany version of this macro called a function that doesn't exist in
> target libs, so libobjc must not build on that target and not coppying
> that macro definition doesn't make anything worse.  We already
> explicitly prefered the default version to the macro for sparc so we
> don't need to copy that version either.  On ppc linux64 and freebsd64
> after constant folding values of other target macros used for libobjc
> the macro turned out to be the same as the default version so we can
> just use the default for them.  Which means the only version of the
> macro we actually need to copy are for ppc-darwin and AIX.
>
> libobjc/ChangeLog:
>
> 2015-10-30  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>
>
>         PR libobjc/24775
>         * encoding.c (objc_layout_finish_structure): Remove usage of
>         ROUND_TYPE_ALIGN.
> ---
>  libobjc/encoding.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/libobjc/encoding.c b/libobjc/encoding.c
> index 7de768f..867372d 100644
> --- a/libobjc/encoding.c
> +++ b/libobjc/encoding.c
> @@ -1237,10 +1237,22 @@ void objc_layout_finish_structure (struct objc_struct_layout *layout,
>        /* Work out the alignment of the record as one expression and store
>           in the record type.  Round it up to a multiple of the record's
>           alignment. */
> -#if defined (ROUND_TYPE_ALIGN) && ! defined (__sparc__)
> -      layout->record_align = ROUND_TYPE_ALIGN (layout->original_type-1,
> -                                               1,
> -                                               layout->record_align);
> +#if _AIX
> +      char type = layout->original_type[-1];
> +      if (type == '{' || type == '(')
> +       layout->record_align =
> +         rs6000_special_round_type_align (layout->original_type-1, 1,
> +                                          layout->record_align);
> +      else
> +       layout->record_align = MAX (1, layout->record_align);
> +#elif __POWERPC__ && __APPLE__
> +      char type = layout->original_type[-1];
> +      if (type == '{' || type == '(')
> +       layout->record_align
> +         = darwin_rs6000_special_round_type_align (layout->original_type - 1,
> +                                                   1, layout->record_align);
> +      else
> +       layout->record_align = MAX (1, layout->record_align);
>  #else
>        layout->record_align = MAX (1, layout->record_align);
>  #endif

I rather not have a special defines in the source but rather a header
that gets included for these special cases instead.

Thanks,
Andrew


> --
> 2.6.2
>

[PATCH 4/5] remove usage of BIGGEST_FIELD_ALIGNMENT in encoding.c

[tbsaunde+gcc]

From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>

Similar to ROUND_TYPE_ALIGN it seems to make sense to copy the
information in the target macros to libobjc as an incremental step.  Its
worth noting a large portion of the definitions of this macro only exist
to work around ADJUST_FIELD_ALIGN being used in target libs, so once all
target macros are gone from target libs we should be able to remove most
of the definitions of BIGGEST_FIELD_ALIGNMENT in gcc/, at which point
there won't be a significant amount of dupplication.

libobjc/ChangeLog:

2015-10-30  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>

	PR libobjc/24775
	* encoding.c (objc_layout_structure_next_member): Remove usage
	of BIGGEST_FIELD_ALIGNMENT.
---
 libobjc/encoding.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/libobjc/encoding.c b/libobjc/encoding.c
index 867372d..7438d64 100644
--- a/libobjc/encoding.c
+++ b/libobjc/encoding.c
@@ -1158,8 +1158,18 @@ objc_layout_structure_next_member (struct objc_struct_layout *layout)
     }
 
   /* The following won't work for vectors.  */
-#ifdef BIGGEST_FIELD_ALIGNMENT
-  desired_align = MIN (desired_align, BIGGEST_FIELD_ALIGNMENT);
+#if defined __x86_64__ || defined __i386__
+#if defined __CYGWIN__ || defined __MINGW32__
+  desired_align = MIN (desired_align, 64);
+#elif defined __x86_64__
+  desired_align = MIN (desired_align, 128);
+#else
+  desired_align = MIN (desired_align, 32);
+#endif
+#elif defined __tilepro__ || defined __frv__ || defined __arm__
+  desired_align = MIN (desired_align, 64);
+#elif defined __tilegx__
+  desired_align = MIN (desired_align, 128);
 #endif
 #ifdef ADJUST_FIELD_ALIGN
   desired_align = ADJUST_FIELD_ALIGN (type, desired_align);
-- 
2.6.2

Re: [PATCH 4/5] remove usage of BIGGEST_FIELD_ALIGNMENT in encoding.c

[Andrew Pinski]

On Fri, Oct 30, 2015 at 7:48 PM,  <tbsaunde+gcc@tbsaunde.org> wrote:
> From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
>
> Similar to ROUND_TYPE_ALIGN it seems to make sense to copy the
> information in the target macros to libobjc as an incremental step.  Its
> worth noting a large portion of the definitions of this macro only exist
> to work around ADJUST_FIELD_ALIGN being used in target libs, so once all
> target macros are gone from target libs we should be able to remove most
> of the definitions of BIGGEST_FIELD_ALIGNMENT in gcc/, at which point
> there won't be a significant amount of dupplication.
>
> libobjc/ChangeLog:
>
> 2015-10-30  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>
>
>         PR libobjc/24775
>         * encoding.c (objc_layout_structure_next_member): Remove usage
>         of BIGGEST_FIELD_ALIGNMENT.
> ---
>  libobjc/encoding.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/libobjc/encoding.c b/libobjc/encoding.c
> index 867372d..7438d64 100644
> --- a/libobjc/encoding.c
> +++ b/libobjc/encoding.c
> @@ -1158,8 +1158,18 @@ objc_layout_structure_next_member (struct objc_struct_layout *layout)
>      }
>
>    /* The following won't work for vectors.  */
> -#ifdef BIGGEST_FIELD_ALIGNMENT
> -  desired_align = MIN (desired_align, BIGGEST_FIELD_ALIGNMENT);
> +#if defined __x86_64__ || defined __i386__
> +#if defined __CYGWIN__ || defined __MINGW32__
> +  desired_align = MIN (desired_align, 64);
> +#elif defined __x86_64__
> +  desired_align = MIN (desired_align, 128);
> +#else
> +  desired_align = MIN (desired_align, 32);
> +#endif
> +#elif defined __tilepro__ || defined __frv__ || defined __arm__
> +  desired_align = MIN (desired_align, 64);
> +#elif defined __tilegx__
> +  desired_align = MIN (desired_align, 128);
>  #endif
>  #ifdef ADJUST_FIELD_ALIGN
>    desired_align = ADJUST_FIELD_ALIGN (type, desired_align);

Just like the other patch, I would rather have a header files for each
of the target instead so we separate out the target specific code from
the target independent code.
And so when porting to a new arch, it is easier to figure out all of
the macros that need to be defined rather than having to audit the
code.

Thanks,
Andrew

> --
> 2.6.2
>

Re: [PATCH 4/5] remove usage of BIGGEST_FIELD_ALIGNMENT in encoding.c

[Trevor Saunders]

On Sat, Oct 31, 2015 at 01:20:45PM +0800, Andrew Pinski wrote:
> On Fri, Oct 30, 2015 at 7:48 PM,  <tbsaunde+gcc@tbsaunde.org> wrote:
> > From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
> >
> > Similar to ROUND_TYPE_ALIGN it seems to make sense to copy the
> > information in the target macros to libobjc as an incremental step.  Its
> > worth noting a large portion of the definitions of this macro only exist
> > to work around ADJUST_FIELD_ALIGN being used in target libs, so once all
> > target macros are gone from target libs we should be able to remove most
> > of the definitions of BIGGEST_FIELD_ALIGNMENT in gcc/, at which point
> > there won't be a significant amount of dupplication.
> >
> > libobjc/ChangeLog:
> >
> > 2015-10-30  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>
> >
> >         PR libobjc/24775
> >         * encoding.c (objc_layout_structure_next_member): Remove usage
> >         of BIGGEST_FIELD_ALIGNMENT.
> > ---
> >  libobjc/encoding.c | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/libobjc/encoding.c b/libobjc/encoding.c
> > index 867372d..7438d64 100644
> > --- a/libobjc/encoding.c
> > +++ b/libobjc/encoding.c
> > @@ -1158,8 +1158,18 @@ objc_layout_structure_next_member (struct objc_struct_layout *layout)
> >      }
> >
> >    /* The following won't work for vectors.  */
> > -#ifdef BIGGEST_FIELD_ALIGNMENT
> > -  desired_align = MIN (desired_align, BIGGEST_FIELD_ALIGNMENT);
> > +#if defined __x86_64__ || defined __i386__
> > +#if defined __CYGWIN__ || defined __MINGW32__
> > +  desired_align = MIN (desired_align, 64);
> > +#elif defined __x86_64__
> > +  desired_align = MIN (desired_align, 128);
> > +#else
> > +  desired_align = MIN (desired_align, 32);
> > +#endif
> > +#elif defined __tilepro__ || defined __frv__ || defined __arm__
> > +  desired_align = MIN (desired_align, 64);
> > +#elif defined __tilegx__
> > +  desired_align = MIN (desired_align, 128);
> >  #endif
> >  #ifdef ADJUST_FIELD_ALIGN
> >    desired_align = ADJUST_FIELD_ALIGN (type, desired_align);
> 
> Just like the other patch, I would rather have a header files for each
> of the target instead so we separate out the target specific code from
> the target independent code.
> And so when porting to a new arch, it is easier to figure out all of
> the macros that need to be defined rather than having to audit the
> code.

Well ok that's an argument we can at least discuss :)  Avoiding tempting
people to dupplicate the same target specific code in multiple places is
certainly of value, and making porting easier is also useful.  On the
other hand I doubt that many people care to port libojbc in particular
to a new platform, and as I pointed out in another mail there's another
libobjc out there from the gnustep people which I would guess people
might be more likely to port?  It also assumes someone would port the
library by looking for macros to change rather than trying to build the
library and then test that it works correctly, and if not debug the
issues they find.
As for avoiding dupplication stuff can easily be moved around by someone
who actually wants to work on libobjc and is trying to add new things
that would actually otherwise require the dupplication.  Also moving the
ifdef parts out into there own function, and probably even their own
.c file isn't that much work.

Supposing you wanted to move this code into macros in a config/ you'd
also have to clean up the target dependent stuff at the top of the file
that this code uses, and that sounds like more work than I'm really
interested in doing right now.

thanks

Trev

> 
> Thanks,
> Andrew
> 
> > --
> > 2.6.2
> >

[PATCH 5/5] remove usage of ADJUST_FIELD_ALIGN in encoding.c

[tbsaunde+gcc]

From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>

Not many targets define this macro in ways that do something in libojc,
so it seems to make sense to just inline the few definitions that do do
something.

libobjc/ChangeLog:

2015-10-30  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>

	PR libobjc/24775
	* encoding.c (objc_layout_structure_next_member): Remove usage
	of ADJUST_FIELD_ALIGN.
---
 libobjc/encoding.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/libobjc/encoding.c b/libobjc/encoding.c
index 7438d64..0fbfd39 100644
--- a/libobjc/encoding.c
+++ b/libobjc/encoding.c
@@ -1171,8 +1171,12 @@ objc_layout_structure_next_member (struct objc_struct_layout *layout)
 #elif defined __tilegx__
   desired_align = MIN (desired_align, 128);
 #endif
-#ifdef ADJUST_FIELD_ALIGN
-  desired_align = ADJUST_FIELD_ALIGN (type, desired_align);
+#if defined __arc__ || defined _AIX
+  if (TYPE_MODE (strip_array_types (TREE_TYPE (type))) == DFmode)
+    desired_align = MIN (desired_align, 32);
+#elif __POWERPC__ && __APPLE__
+  if (desired_align != 128)
+    desired_align = MIN (desired_align, 32);
 #endif
 
   /* Record must have at least as much alignment as any field.
-- 
2.6.2

Re: [PATCH 5/5] remove usage of ADJUST_FIELD_ALIGN in encoding.c

[Bernd Schmidt]

On 10/30/2015 12:48 PM, tbsaunde+gcc@tbsaunde.org wrote:
> -#ifdef ADJUST_FIELD_ALIGN
> -  desired_align = ADJUST_FIELD_ALIGN (type, desired_align);
> +#if defined __arc__ || defined _AIX
> +  if (TYPE_MODE (strip_array_types (TREE_TYPE (type))) == DFmode)
> +    desired_align = MIN (desired_align, 32);
> +#elif __POWERPC__ && __APPLE__
> +  if (desired_align != 128)
> +    desired_align = MIN (desired_align, 32);
>   #endif

No way. We never use this kind of test in target-independent code.


Bernd

Re: [PATCH 5/5] remove usage of ADJUST_FIELD_ALIGN in encoding.c

[Richard Biener]

On Fri, Oct 30, 2015 at 1:06 PM, Bernd Schmidt <bschmidt@redhat.com> wrote:
> On 10/30/2015 12:48 PM, tbsaunde+gcc@tbsaunde.org wrote:
>>
>> -#ifdef ADJUST_FIELD_ALIGN
>> -  desired_align = ADJUST_FIELD_ALIGN (type, desired_align);
>> +#if defined __arc__ || defined _AIX
>> +  if (TYPE_MODE (strip_array_types (TREE_TYPE (type))) == DFmode)
>> +    desired_align = MIN (desired_align, 32);
>> +#elif __POWERPC__ && __APPLE__
>> +  if (desired_align != 128)
>> +    desired_align = MIN (desired_align, 32);
>>   #endif
>
>
> No way. We never use this kind of test in target-independent code.

it's not target independent code.  Are you suggesting to add a config/
to libobjc?  IMHO for a not really mantained frontend / target lib that's
an excessive requirement.

For any such replacements as in the patch I suggest to at least keep a comment
before it indicating the origin of the inlined vairants (in this case refer to
ADJUST_FIELD_ALIGN).

In general I'm happy with this kind of patches (maybe not the
BIGGEST_FIELD_ALIGN
one which could be made a CPP macro when -fbuilding-libgcc)

Richard.

>
> Bernd

Re: [PATCH 5/5] remove usage of ADJUST_FIELD_ALIGN in encoding.c

[Bernd Schmidt]

>
> it's not target independent code.  Are you suggesting to add a config/
> to libobjc?  IMHO for a not really mantained frontend / target lib that's
> an excessive requirement.

If necessary, then yes that would be a better solution.

Even just keeping the abstraction of the macro and putting definitions 
of it inside #ifdef at the top of the file would be an improvement over 
the submitted patch, but IMO still not really compatible with our standards.


Bernd

Re: [PATCH 5/5] remove usage of ADJUST_FIELD_ALIGN in encoding.c

[Richard Biener]

On Fri, Oct 30, 2015 at 1:28 PM, Bernd Schmidt <bschmidt@redhat.com> wrote:
>>
>> it's not target independent code.  Are you suggesting to add a config/
>> to libobjc?  IMHO for a not really mantained frontend / target lib that's
>> an excessive requirement.
>
>
> If necessary, then yes that would be a better solution.
>
> Even just keeping the abstraction of the macro and putting definitions of it
> inside #ifdef at the top of the file would be an improvement over the
> submitted patch, but IMO still not really compatible with our standards.

I agree, that would make the source of the copy more obvious.

Still libobjc is beyond what I consider compatible with our standards (just
look at the hoops it jumps through to make the target macros work!)

Richard.

>
> Bernd
>

Re: [PATCH 5/5] remove usage of ADJUST_FIELD_ALIGN in encoding.c

[Bernd Schmidt]

On 10/30/2015 01:47 PM, Richard Biener wrote:
> On Fri, Oct 30, 2015 at 1:28 PM, Bernd Schmidt <bschmidt@redhat.com> wrote:
>>>
>>> it's not target independent code.  Are you suggesting to add a config/
>>> to libobjc?  IMHO for a not really mantained frontend / target lib that's
>>> an excessive requirement.
>>
>>
>> If necessary, then yes that would be a better solution.
>>
>> Even just keeping the abstraction of the macro and putting definitions of it
>> inside #ifdef at the top of the file would be an improvement over the
>> submitted patch, but IMO still not really compatible with our standards.
>
> I agree, that would make the source of the copy more obvious.

If we go down that path, there's another minimum requirement - adjust 
the docs to say that the macro must be defined in two places. That's my 
main worry, someone creating a new port, completely oblivious that 
they'd have to modify library code for it to work.


Bernd

Re: [PATCH 5/5] remove usage of ADJUST_FIELD_ALIGN in encoding.c

[Andrew Pinski]

On Fri, Oct 30, 2015 at 9:11 PM, Bernd Schmidt <bschmidt@redhat.com> wrote:
> On 10/30/2015 01:47 PM, Richard Biener wrote:
>>
>> On Fri, Oct 30, 2015 at 1:28 PM, Bernd Schmidt <bschmidt@redhat.com>
>> wrote:
>>>>
>>>>
>>>> it's not target independent code.  Are you suggesting to add a config/
>>>> to libobjc?  IMHO for a not really mantained frontend / target lib
>>>> that's
>>>> an excessive requirement.
>>>
>>>
>>>
>>> If necessary, then yes that would be a better solution.
>>>
>>> Even just keeping the abstraction of the macro and putting definitions of
>>> it
>>> inside #ifdef at the top of the file would be an improvement over the
>>> submitted patch, but IMO still not really compatible with our standards.
>>
>>
>> I agree, that would make the source of the copy more obvious.
>
>
> If we go down that path, there's another minimum requirement - adjust the
> docs to say that the macro must be defined in two places. That's my main
> worry, someone creating a new port, completely oblivious that they'd have to
> modify library code for it to work.

That happens already with respect to libffi anyways.  libffi knows the
inter-working of how ABIs work and has many target specific assembly
code.
If you look at libsanitizer, the code is even worse with all the
macros that need to be changed and I rather not go down that route
where we have processor specific #ifdefs in the code.
Yes libobjc is mostly just works but going the route of libsanitizer
is just wrong and makes a really bad precedent of target libraries.
Even libgomp has some target specific headers and is very short of the
number of headers/defines you need to change to it working (most of
the time none).

Thanks,
Andrew

>
>
> Bernd

Re: [PATCH 5/5] remove usage of ADJUST_FIELD_ALIGN in encoding.c

[Trevor Saunders]

On Sat, Oct 31, 2015 at 03:02:07PM +0800, Andrew Pinski wrote:
> On Fri, Oct 30, 2015 at 9:11 PM, Bernd Schmidt <bschmidt@redhat.com> wrote:
> > On 10/30/2015 01:47 PM, Richard Biener wrote:
> >>
> >> On Fri, Oct 30, 2015 at 1:28 PM, Bernd Schmidt <bschmidt@redhat.com>
> >> wrote:
> >>>>
> >>>>
> >>>> it's not target independent code.  Are you suggesting to add a config/
> >>>> to libobjc?  IMHO for a not really mantained frontend / target lib
> >>>> that's
> >>>> an excessive requirement.
> >>>
> >>>
> >>>
> >>> If necessary, then yes that would be a better solution.
> >>>
> >>> Even just keeping the abstraction of the macro and putting definitions of
> >>> it
> >>> inside #ifdef at the top of the file would be an improvement over the
> >>> submitted patch, but IMO still not really compatible with our standards.
> >>
> >>
> >> I agree, that would make the source of the copy more obvious.
> >
> >
> > If we go down that path, there's another minimum requirement - adjust the
> > docs to say that the macro must be defined in two places. That's my main
> > worry, someone creating a new port, completely oblivious that they'd have to
> > modify library code for it to work.
> 
> That happens already with respect to libffi anyways.  libffi knows the
> inter-working of how ABIs work and has many target specific assembly
> code.

I think basically any library of sufficient size will end up having
target specific code somewhere, and will require some amount of effort
to port to new arches, and if it pokes at low level details it'll
probably take even more.

> If you look at libsanitizer, the code is even worse with all the
> macros that need to be changed and I rather not go down that route
> where we have processor specific #ifdefs in the code.

I've never worked with libsanitizer, but #ifdef really isn't different
from macros and a giant shell case on $target accept that has more
indirection.  libgcc uses a config/ and it is still some rather bad
code, and don't get me started on the stuff people have stuffed into
macros in gcc/config/.

> Yes libobjc is mostly just works but going the route of libsanitizer
> is just wrong and makes a really bad precedent of target libraries.

That seems to be a conclusion without any real justification.  It may
well be true libsanitizer could be organized better, but just saying its
"wrong" isn't very convincing ;)

thanks

Trev


> Even libgomp has some target specific headers and is very short of the
> number of headers/defines you need to change to it working (most of
> the time none).
> 
> Thanks,
> Andrew
> 
> >
> >
> > Bernd

Re: [PATCH 5/5] remove usage of ADJUST_FIELD_ALIGN in encoding.c

[Trevor Saunders]

On Fri, Oct 30, 2015 at 01:16:16PM +0100, Richard Biener wrote:
> On Fri, Oct 30, 2015 at 1:06 PM, Bernd Schmidt <bschmidt@redhat.com> wrote:
> > On 10/30/2015 12:48 PM, tbsaunde+gcc@tbsaunde.org wrote:
> >>
> >> -#ifdef ADJUST_FIELD_ALIGN
> >> -  desired_align = ADJUST_FIELD_ALIGN (type, desired_align);
> >> +#if defined __arc__ || defined _AIX
> >> +  if (TYPE_MODE (strip_array_types (TREE_TYPE (type))) == DFmode)
> >> +    desired_align = MIN (desired_align, 32);
> >> +#elif __POWERPC__ && __APPLE__
> >> +  if (desired_align != 128)
> >> +    desired_align = MIN (desired_align, 32);
> >>   #endif
> >
> >
> > No way. We never use this kind of test in target-independent code.
> 
> it's not target independent code.  Are you suggesting to add a config/
> to libobjc?  IMHO for a not really mantained frontend / target lib that's
> an excessive requirement.

Given the amount of target dependant stuff involved adding a config/
actually seems worse to me.  You are accomplishing the exact same thing,
but you need a whole lot more machinary to do it, and its hard to
understand what happens for any given platform.  Sure, if there was a
whole lot more target code doing something else might make sense, but
there isn't and I'm certainly not planning on adding more.  SO I think
its best to leave it this way and if someone wants to do substantial
work on libobjc in the future they can worry about that then.

btw the claim its never done just doesn't stand up either, look at the
__SPARC__ check this series removes, or the __MINGW__ check in gthr.h, or
even all the crap at the top of encoding.c that makes using these target
macros possible (it wouldn't actually suprise me if cleaning that up
ment doing this was a net reduction in target dependent code in
encoding.c).

If you want to be kind of sad I discovered
https://github.com/gnustep/libobjc2 while looking at this, so it seems
like many of the possible users of libobjc may even be not using it.

> For any such replacements as in the patch I suggest to at least keep a comment
> before it indicating the origin of the inlined vairants (in this case refer to
> ADJUST_FIELD_ALIGN).

That seems fairly reasonable, I'd kind of worry about them getting out
of date, but i guess it at least gives a place to start looking for an
explanation.

> In general I'm happy with this kind of patches (maybe not the
> BIGGEST_FIELD_ALIGN
> one which could be made a CPP macro when -fbuilding-libgcc)

I considered that, but the only targets that define
BIGGEST_FIELD_ALIGNMENT  for purposes other than IN_TARGET_LIBS hacks
were v850, vax, tilegx, and tilepro so considering
BIGGEST_FIELD_ALIGNMENT kind of dupplicates ADJUST_FIELD_ALIGN my
conclusion was it would make more sense to not do
that.  I'm thinking it makes sense to instead just merge
BIGGEST_FIELD_ALIGNMENT into ADJUST_FIELD_ALIGN, but adding a predefined
macro would make that harder.

Trev

> 
> Richard.
> 
> >
> > Bernd

Re: [PATCH 0/5] remove tm.h from encoding.c

[Mike Stump]

On Oct 30, 2015, at 4:48 AM, tbsaunde+gcc@tbsaunde.org wrote:
> Its not the nicest code in the world, and there's definitely room for cleanups,
> however it seems like an improvement.  After this series the only usage of tm.h
> in libobjc is thr.c which only uses tm.h so it can include gthr.h which uses
> SUPPORTS_WEAK and possibly other target macros.
> 
> bootstrapped + regtested patches individually on x86_64-linux-gnu, also tested the
> series as a whole doesn't regress check-objc on ppc64{,le}-linux-gnu and AIX,
> ok?

You might have to wake Andrew up…  I glanced over the testing and the patch for libobjc, and they look ok to me, but, it might be nice to watch for darwin fallout if it goes in with no darwin testing.