Fix PR rtl-optimization/55630

2012-12-16 Thread Eric Botcazou
This is an ICE for g++.dg/pr48660.C on HP-PA/Linux introduced by:
  http://gcc.gnu.org/ml/gcc-patches/2012-10/msg00745.html

When I wrote it, I convinced myself that the HP-PA wasn't using the BLKmode 
registers path any more and wrapped everything in PARALLELs for return values, 
so it tested the change on MIPS instead.  Now it turns out that, for sub-word 
structures, the HP-PA still uses BLKmode registers and the C++ testcase shows 
that the code doesn't correctly handle assignments from incoming return values 
to outgoing return values.  Not clear why this didn't show up on MIPS...

Tested on x86-64, HP-PA and MIPS/Linux, applied on the mainline.


2012-12-16  Eric Botcazou  ebotca...@adacore.com

PR rtl-optimization/55630
* expr.c (expand_assignment): Do not call copy_blkmode_to_reg to move
BLKmode return values to the return register.


-- 
Eric BotcazouIndex: expr.c
===
--- expr.c	(revision 194517)
+++ expr.c	(working copy)
@@ -4920,7 +4920,12 @@ expand_assignment (tree to, tree from, b
   rtx temp;
 
   push_temp_slots ();
-  if (REG_P (to_rtx)  TYPE_MODE (TREE_TYPE (from)) == BLKmode)
+
+  /* If the source is itself a return value, it still is in a pseudo at
+	 this point so we can move it back to the return register directly.  */
+  if (REG_P (to_rtx)
+	   TYPE_MODE (TREE_TYPE (from)) == BLKmode
+	   TREE_CODE (from) != CALL_EXPR)
 	temp = copy_blkmode_to_reg (GET_MODE (to_rtx), from);
   else
 	temp = expand_expr (from, NULL_RTX, GET_MODE (to_rtx), EXPAND_NORMAL);


[SPARC] Fix PR target/55673

2012-12-16 Thread Eric Botcazou
As reported by Tomash, the handling of the before and after cases is reversed 
in sparc_emit_membar_for_model.  Moreover, there is a pasto in sync.md that 
disables the atomic_store expander.

Tested on SPARC/Linux, applied on the mainline and 4.7 branch.


2012-12-16  Eric Botcazou  ebotca...@adacore.com
Tomash Brechko  tomash.brec...@gmail.com

PR target/55673
* config/sparc/sparc.c (sparc_emit_membar_for_model): Fix reversed
handling of before and after cases.
* config/sparc/sync.md (atomic_store): Fix pasto.


-- 
Eric BotcazouIndex: config/sparc/sync.md
===
--- config/sparc/sync.md	(revision 194517)
+++ config/sparc/sync.md	(working copy)
@@ -35,8 +35,7 @@ (define_expand mem_thread_fence
 
 (define_expand membar
   [(set (match_dup 1)
-	(unspec:BLK [(match_dup 1)
-		 (match_operand:SI 0 const_int_operand)]
+	(unspec:BLK [(match_dup 1) (match_operand:SI 0 const_int_operand)]
 		UNSPEC_MEMBAR))]
   TARGET_V8 || TARGET_V9
 {
@@ -66,7 +65,7 @@ (define_insn *membar_storestore
   stbar
   [(set_attr type multi)])
 
-;; For V8, LDSTUB has the effect of membar #StoreLoad
+;; For V8, LDSTUB has the effect of membar #StoreLoad.
 (define_insn *membar_storeload
   [(set (match_operand:BLK 0  )
 	(unspec:BLK [(match_dup 0) (const_int 2)] UNSPEC_MEMBAR))]
@@ -123,8 +122,8 @@ (define_insn atomic_loaddi_1
   [(set_attr type load,fpload)])
 
 (define_expand atomic_storemode
-  [(match_operand:I 0 register_operand )
-   (match_operand:I 1 memory_operand )
+  [(match_operand:I 0 memory_operand )
+   (match_operand:I 1 register_operand )
(match_operand:SI 2 const_int_operand )]
   
 {
Index: config/sparc/sparc.c
===
--- config/sparc/sparc.c	(revision 194517)
+++ config/sparc/sparc.c	(working copy)
@@ -11190,26 +11190,26 @@ sparc_emit_membar_for_model (enum memmod
 
   if (before_after  1)
 {
-  if (model == MEMMODEL_ACQUIRE
-  || model == MEMMODEL_ACQ_REL
-  || model == MEMMODEL_SEQ_CST)
+  if (model == MEMMODEL_RELEASE
+	  || model == MEMMODEL_ACQ_REL
+	  || model == MEMMODEL_SEQ_CST)
 	{
 	  if (load_store  1)
-	mm |= LoadLoad | LoadStore;
+	mm |= LoadLoad | StoreLoad;
 	  if (load_store  2)
-	mm |= StoreLoad | StoreStore;
+	mm |= LoadStore | StoreStore;
 	}
 }
   if (before_after  2)
 {
-  if (model == MEMMODEL_RELEASE
+  if (model == MEMMODEL_ACQUIRE
 	  || model == MEMMODEL_ACQ_REL
 	  || model == MEMMODEL_SEQ_CST)
 	{
 	  if (load_store  1)
-	mm |= LoadLoad | StoreLoad;
+	mm |= LoadLoad | LoadStore;
 	  if (load_store  2)
-	mm |= LoadStore | StoreStore;
+	mm |= StoreLoad | StoreStore;
 	}
 }
 

Re: [fortran, patch] Allow displaying backtraces from user code

2012-12-16 Thread Tobias Burnus

Janus Weil wrote:

So, in principle I'm fine with all your BACKTRACE_* variants (except
for _splurge, maybe;)

Or, why not just (plain and simple) BACKTRACE?

The name is the same as backtrace() in glibc, but otherwise, sure why
not. _show/_print might be preferable in the sense that they convey
that stuff will be directly printed on the screen, rather than, say,
the procedure returning an array of strings with the stack trace info.

Agreed. Let's go with BACKTRACE_SHOW.


I have to admit that I prefer show_backtrace to backtrace_show, which 
sounds a bit clumsy. I also don't think that finding show_backtrace is 
more difficult than finding backtrace_show. backtrace is in the index, 
looking at the documentation, one can still search for backtrace and 
search engines should find backtrace in either way. (A name which 
comes just into my mind is: backtrace_now(); I am not claiming that it 
is better than any of the others.)


Enough of bikeshadding: I think having the possibility to simply print a 
backtrace is very useful! Hence, I leave the name to Janus and Janne.


Tobias


Re: [Patch, Fortran] PR 55072: [4.6/4.7/4.8 Regression] Missing internal_pack leads to wrong code with derived type

2012-12-16 Thread Janus Weil
Hi Tobias,

 here is a patch for a pretty bad wrong-code regression, which affects
 all maintained releases of gfortran. For discussion see bugzilla.

 2012-12-15  Janus Weilja...@gcc.gnu.org
  PR fortran/55072
  * trans-array.c (gfc_conv_array_parameter): No packing was done for
  full arrays of derived type.

 @@ -6995,20 +6995,14 @@ gfc_conv_array_parameter (gfc_se * se, gfc_expr *
   this_array_result = false;
   /* Passing address of the array if it is not pointer or
 assumed-shape.  */
 -  if (full_array_var  g77  !this_array_result)
 +  if (full_array_var  g77  !this_array_result
 +   sym-ts.type != BT_DERIVED  sym-ts.type != BT_CLASS)

 Without experimenting more carefully, I have the gut feeling that there are
 still cases where we might end up with invalid code and there is missed
 optimization.

 Regarding the latter: If the variable is simply contiguous, there is no need
 to pack it. Hence, for

 type(t), allocatable :: a(:)
 ...
 call bar(a)

 there is no need to pack the array.

Hm, ok. Do we do any packing in this case? Anyway, this sort of
missed-optimization issue can be treated in a follow-up patch, I
guess. (Mikael also noted a similar missed-optimization case in
comment #13 of the PR.)

What I'm aiming for here is primarily a patch for the wrong-code
regression which is suitable for all three maintained branches.


 The problem with the original test case
 is that one has a non-CONTIGUOUS pointer:

 p = tgt(::2,::2)
 call bar(p)

 But that has in principle nothing to do with BT_DERIVED.

Yes, the reason for the patch to handle BT_DERIVED in particular, is
that the original commit which introduced the regression (i.e.
r156749) messed up things for BT_DERIVED, which is what I'm reverting.


 In particular, I
 would like to see an additional test case for the first example case with
 ptr having the CONTIGUOUS attribute - and a check that then no packing
 call is invoked.

I just checked this: The patched trunk seems to handle this properly
and does not do any packing.

However, I think there might be another problem:

implicit none
type t
  integer :: i
end type t
type(t), target :: tgt(4,4)
type(t), pointer, contiguous :: p(:,:)
p = tgt(::2,::2)! accepts invalid?
end

The pointer assignment of a contiguous pointer to a non-contiguous
target should probably be rejected, right? Another follow-up problem
...


 For the second test case (comment 2, from GiBUU): Here, the problem is that
 full_array_var is wrongly true:

   call s1(OutPart(1,:))

 I wonder whether some call to gfc_is_simply_contiguous could solve the
 problem for both issues.

No, here I disagree. The problem with this one was not related to the
call of s1, but of s2, where indeed a full array is passed!



 (For non-whole arrays one still have to ensure that one passes the correct
 element: call(a) should pass a-data and not a and call bar(a(:,2))
 should neither pass a-data nor a but a-data + offset.)

 Regarding BT_CLASS: BT_CLASS - BT_TYPE (with same declared type) should
 already be handled via gfc_conv_subref_array_arg, which takes care of the
 actual type. Thus, the patched function should only be reachable for
 BT_CLASS - BT_CLASS. Here, packing is required for non-simply contiguous
 actual arguments; but after the packing, a class container has to be
 re-added. I think one should add a test case for this; testing declared type
 == actual type and declared type != actual type - and either one for both
 declared type being the same and for the dummy having the declared type of
 the ancestor of the declared type of the actual argument. And all cases for
 both simply contiguous arrays and (simply - or better actually)
 noncontiguous arrays.

I'm ignoring all this for now. All I want to fix at this point is the
wrong-code regression!


 Regarding the wrong code: I fear that some code involving non-BT_DERIVED
 could lead to wrong code, e.g. a(:)%x. I don't have an example for that

If you find an example where stuff goes wrong (as a regression of my
patch), I'll take care of it.


 but I fear that code which lead to the original issue (e.g. full_array_var
 is true although it shouldn't) is not solved via the patch.

I actually don't think this is the case!


 Sorry for listing more my concerns that giving a proper review.

Thanks for your comments, anyway.

Cheers,
Janus


Re: [Patch,Fortran] reset dynamic type with MOVE_ALLOC (was: Re: [Patch, Fortran] Small patch for calls to gfc_deallocate_scalar_with_status)

2012-12-16 Thread Paul Richard Thomas
Dear Tobias,

This does what it is advertised to do - OK for trunk

Thanks for the patch.

Paul

PS Thomas has beaten it to me on the other two patches; they look OK
for trunk to me too.

On 9 December 2012 20:04, Tobias Burnus bur...@net-b.de wrote:
 Janus Weil wrote:

 The expr to al-expr change is to pass a BT_CLASS instead of a
 BT_DERIVED. And the NULL to gfc_lval_expr_from_sym change allows to
  access
 var-_vtab-_final for a BT_CLASS deferred variable.

 It seems that both of them will not have any effect right now, but are
 useful only as preparation for FINAL, right?


 I think that's true. I wanted to claim that it also fixes the following, but
 it doesn't:

  class(t), allocatable :: a, b, c
  allocate (t2 :: a)
  call move_alloc (from=a, to=b)

 a should not only be deallocated but same_type_as(a,c) should be true,
 i.e. one has to reset the a-_vtab pointer to the declared type.

 A follow-up patch which fixes this is attached.

 Build and regtested on x86-64-gnu-linux.
 OK for the trunk?

 Tobias



-- 
The knack of flying is learning how to throw yourself at the ground and miss.
   --Hitchhikers Guide to the Galaxy


Re: [fortran, patch] Allow displaying backtraces from user code

2012-12-16 Thread Janus Weil
 So, in principle I'm fine with all your BACKTRACE_* variants (except
 for _splurge, maybe;)
 
 Or, why not just (plain and simple) BACKTRACE?

 The name is the same as backtrace() in glibc, but otherwise, sure why
 not. _show/_print might be preferable in the sense that they convey
 that stuff will be directly printed on the screen, rather than, say,
 the procedure returning an array of strings with the stack trace info.

 Agreed. Let's go with BACKTRACE_SHOW.

 I have to admit that I prefer show_backtrace to backtrace_show, which sounds
 a bit clumsy. I also don't think that finding show_backtrace is more
 difficult than finding backtrace_show. backtrace is in the index, looking
 at the documentation, one can still search for backtrace and search
 engines should find backtrace in either way. (A name which comes just into
 my mind is: backtrace_now(); I am not claiming that it is better than any
 of the others.)

Look, I really don't care if we call it APOCALYPSE_NOW or
SERGEANT_FUZZY_BOOTS, as long as it prints a proper backtrace!

If we can not agree on any name here, we could just open a doodle poll
to decide this by democratic vote ...?!?

Cheers,
Janus


Re: Patch to enable unlimited polymorphism to gfortran

2012-12-16 Thread Tobias Burnus
Thanks for the patch, here is a first but incomplete review. I think the 
patch looks mostly okay.



Running your test cases through crayftn, I found:

  if (SAME_TYPE_AS (obj1, u1) .neqv. .FALSE.) call abort
  ^
ftn-1698 crayftn: ERROR $MAIN, File = unlimited_polymorphic_1.f03, Line 
= 67, Column = 27
Type INTEGER(kind=4) is not allowed for the B argument of intrinsic 
SAME_TYPE_AS.


The message itself is correct. However, Paul mentioned off-list that 
obj1 and u1 are both extensible types and, hence, neither is an 
integer(kind=4). In any case, if either of them were an integer(4) it 
would be invalid:


From Fortran 2008's 13.7.142 SAME_TYPE_AS (A, B)
Arguments.
A shall be an object of extensible declared type or unlimited 
polymorphic. If it is a pointer, it shall not have an undefined 
association status.
B shall be an object of extensible declared type or unlimited 
polymorphic. If it is a pointer, it shall

not have an undefined association status.

Ditto for 13.7.60 EXTENDS TYPE OF (A, MOLD).

At least for -std=f2003/f2008 we have to reject nonextensible, 
non-unlimited-polymorphic arguments. - Even if a supporting intrinsic 
types were a nice addition.



Paul Richard Thomas wrote:

+
+   if (UNLIMITED_POLY (c-expr))
+   {
+ gfc_error (F08: C4106 array constructor value at %L shall not be 
+ unlimited polymorphic, c-expr-where);
+ t = FAILURE;


You have a   at both then end and the beginning of the line. 
Additionally, I wonder how we want to inform the user about those 
constrains. So far, we always had the message only without reference to 
the standard. Having one, is not bad, the question is only where. I 
would favor to have the information at the end, given that gfortran 
currently prints Fortran 2008: if a something is only valid in Fortran 
2008 and later. Additionally, I personally don't like F08 that much 
and prefer Fortran 2008 (or at least F2008). Thus, I'd use something 
like:


+ gfc_error (Array constructor value at %L shall not be unlimited
+ polymorphic [Fortran 2008, C4106], c-expr-where);


(Except for the double  , that's bikeshadding; hence, I leave it to 
you. The F08: C... also occures at other places in the patch)




+sprintf (dt_name, %s, $tar);

(Off-topic question for mere curiosity: Why $tar?)


+ gfc_find_intrinsic_vtab (gfc_typespec *ts)

+   if (ts-type == BT_CHARACTER  ts-deferred)
+ {
+   gfc_error (TODO: Deferred character length variable at %C cannot 
+yet be associated with unlimited polymorphic entities);


The same issue also applies to assumed-length strings. At least the 
following program prints:

   0 
   0 
I think we have to fill a PR which lists all of the known deficits of 
the current implementation. Besides the string issue, that's also the 
renaming of gfc_find_intrinsic_vtab into gfc_find_derived_vtab.


Here's the test case:

call foo(Hello)
call foo(World!)
contains
subroutine foo(str)
  character(len=*), target :: str
  class(*), pointer :: up
  up = str
  call bar(up)
end subroutine foo
subroutine bar(x)
  class(*) :: x

  select type(x)
type is (character(len=*))
  print *, len(x), ''//x//''
  end select
end subroutine bar
end




+ #define UNLIMITED_POLY(sym) (sym != NULL  sym-ts.type == BT_CLASS  
CLASS_DATA (sym) \
+CLASS_DATA (sym)-ts.u.derived \
+CLASS_DATA 
(sym)-ts.u.derived-attr.unlimited_polymorphic)


The lines are way too long: 90 and 86 characters.



+   sprintf (name, __tmp_%s_%d, gfc_basic_typename (ts-type),
+  ts-type == BT_CHARACTER ? charlen : ts-kind);


How do you distinguish between character(kind=1) and character(kind=4)? 
The same issue exists for a like-wise code in resolve_select_type.



+  /* Unlimited polymorphic pointers should have their vptr nullified.  */
+  if (UNLIMITED_POLY (sym)  CLASS_DATA (sym)-attr.pointer)
+gfc_defer_symbol_init (sym);


Why? If the pointer has never been pointer-associated, one shouldn't 
access it. Thus, the code is not need. If it is needed, I fear that code 
will also break when one later deallocates/nullifies the pointer. I 
think _vptr should only be set when also nonpolymorphic CLASS get their 
_vptr set. For pointers, that's presumably only ALLOCATE and 
pointer-association.



Tobias


[PATCH] Fix Infinite loop in pointer_set_insert

2012-12-16 Thread John David Anglin
The attached patch fixes a regression introduced on the trunk to
fix PR middle-end/52640.  The fix was previously applied to the 4.6
and 4.7 branches.

Tested on hppa-unknown-linux-gnu with full bootstrap.

OK for trunk?

Dave
-- 
J. David Anglin  dave.ang...@nrc-cnrc.gc.ca
National Research Council of Canada  (613) 990-0752 (FAX: 952-6602)

2012-12-16  John David Anglin  dave.ang...@nrc-cnrc.gc.ca

PR middle-end/55709
Forward port from 4.7 branch:
2012-04-10  John David Anglin  dave.ang...@nrc-cnrc.gc.ca

PR middle-end/52894
* varasm.c (process_pending_assemble_externals): Set
pending_assemble_externals_processed true.
(assemble_external): Call assemble_external_real if the pending
assemble externals have been processed.

Index: varasm.c
===
--- varasm.c(revision 194441)
+++ varasm.c(working copy)
@@ -2088,6 +2088,11 @@
it all the way to final.  See PR 17982 for further discussion.  */
 static GTY(()) tree pending_assemble_externals;
 
+/* Some targets delay some output to final using TARGET_ASM_FILE_END.
+   As a result, assemble_external can be called after the list of externals
+   is processed and the pointer set destroyed.  */
+static bool pending_assemble_externals_processed;
+
 #ifdef ASM_OUTPUT_EXTERNAL
 /* Avoid O(external_decls**2) lookups in the pending_assemble_externals
TREE_LIST in assemble_external.  */
@@ -2144,6 +2149,7 @@
 assemble_external_real (TREE_VALUE (list));
 
   pending_assemble_externals = 0;
+  pending_assemble_externals_processed = true;
   pointer_set_destroy (pending_assemble_externals_set);
 #endif
 }
@@ -2196,6 +2202,12 @@
 weak_decls = tree_cons (NULL, decl, weak_decls);
 
 #ifdef ASM_OUTPUT_EXTERNAL
+  if (pending_assemble_externals_processed)
+{
+  assemble_external_real (decl);
+  return;
+}
+
   if (! pointer_set_insert (pending_assemble_externals_set, decl))
 pending_assemble_externals = tree_cons (NULL, decl,
pending_assemble_externals);


Re: [PATCH] Fix Infinite loop in pointer_set_insert

2012-12-16 Thread Richard Biener
On Sun, Dec 16, 2012 at 5:33 PM, John David Anglin
d...@hiauly1.hia.nrc.ca wrote:
 The attached patch fixes a regression introduced on the trunk to
 fix PR middle-end/52640.  The fix was previously applied to the 4.6
 and 4.7 branches.

 Tested on hppa-unknown-linux-gnu with full bootstrap.

 OK for trunk?

Ok.

Thanks,
Richard.

 Dave
 --
 J. David Anglin  dave.ang...@nrc-cnrc.gc.ca
 National Research Council of Canada  (613) 990-0752 (FAX: 
 952-6602)

 2012-12-16  John David Anglin  dave.ang...@nrc-cnrc.gc.ca

 PR middle-end/55709
 Forward port from 4.7 branch:
 2012-04-10  John David Anglin  dave.ang...@nrc-cnrc.gc.ca

 PR middle-end/52894
 * varasm.c (process_pending_assemble_externals): Set
 pending_assemble_externals_processed true.
 (assemble_external): Call assemble_external_real if the pending
 assemble externals have been processed.

 Index: varasm.c
 ===
 --- varasm.c(revision 194441)
 +++ varasm.c(working copy)
 @@ -2088,6 +2088,11 @@
 it all the way to final.  See PR 17982 for further discussion.  */
  static GTY(()) tree pending_assemble_externals;

 +/* Some targets delay some output to final using TARGET_ASM_FILE_END.
 +   As a result, assemble_external can be called after the list of externals
 +   is processed and the pointer set destroyed.  */
 +static bool pending_assemble_externals_processed;
 +
  #ifdef ASM_OUTPUT_EXTERNAL
  /* Avoid O(external_decls**2) lookups in the pending_assemble_externals
 TREE_LIST in assemble_external.  */
 @@ -2144,6 +2149,7 @@
  assemble_external_real (TREE_VALUE (list));

pending_assemble_externals = 0;
 +  pending_assemble_externals_processed = true;
pointer_set_destroy (pending_assemble_externals_set);
  #endif
  }
 @@ -2196,6 +2202,12 @@
  weak_decls = tree_cons (NULL, decl, weak_decls);

  #ifdef ASM_OUTPUT_EXTERNAL
 +  if (pending_assemble_externals_processed)
 +{
 +  assemble_external_real (decl);
 +  return;
 +}
 +
if (! pointer_set_insert (pending_assemble_externals_set, decl))
  pending_assemble_externals = tree_cons (NULL, decl,
 pending_assemble_externals);


Re: Patch to enable unlimited polymorphism to gfortran

2012-12-16 Thread Paul Richard Thomas
Dear Tobias,

Up front, thanks for this initial feedback.  Dominique informed me on
#gfortran that the patch works as advertised.

...snip...
 Running your test cases through crayftn, I found:

   if (SAME_TYPE_AS (obj1, u1) .neqv. .FALSE.) call abort
   ^
 ftn-1698 crayftn: ERROR $MAIN, File = unlimited_polymorphic_1.f03, Line =
 67, Column = 27
 Type INTEGER(kind=4) is not allowed for the B argument of intrinsic
 SAME_TYPE_AS.
...snip...
 At least for -std=f2003/f2008 we have to reject nonextensible,
 non-unlimited-polymorphic arguments. - Even if a supporting intrinsic types
 were a nice addition.

gfortran was already giving an error at the right place - see
same_type_as_1.f03  I have improved the message to contain the name of
the offending type, like the Cray compiler.  I believe the Cray
compiler has a bug at this point.
.
..snip...

 You have a   at both then end and the beginning of the line. Additionally,

Indeed - thanks

...snip...

 F2008). Thus, I'd use something like:

 + gfc_error (Array constructor value at %L shall not be unlimited
 + polymorphic [Fortran 2008, C4106], c-expr-where);



That's fine by me. I have changed all the messages, as suggested.
However, F03/08 is so widespread in comments that I have left well
alone.


 (Off-topic question for mere curiosity: Why $tar?)

Well '$' is non-alphanumeric but is just an 's' with a bar through it :-)


 The same issue also applies to assumed-length strings. At least the
 following program prints:
0 
0 

Adding an error to pick up assumed length is proving to be problematic
because the TYPE IS selector is assumed length.  I'll think about it
some  more.

 I think we have to fill a PR which lists all of the known deficits of the
 current implementation. Besides the string issue, that's also the renaming
 of gfc_find_intrinsic_vtab into gfc_find_derived_vtab.

I did not do this immediately because the latter takes the derived
type, whilst the former uses the typespec. When the two are rolled
into one function, the typespec will be used.

I agree that a catch-all PR is needed for class(*) limitations.

...snip...



 + #define UNLIMITED_POLY(sym) (sym != NULL  sym-ts.type == BT_CLASS 
 CLASS_DATA (sym) \
 +CLASS_DATA (sym)-ts.u.derived \
 +CLASS_DATA
 (sym)-ts.u.derived-attr.unlimited_polymorphic)


 The lines are way too long: 90 and 86 characters.

Blush - yes, you are right. Done.



 +   sprintf (name, __tmp_%s_%d, gfc_basic_typename (ts-type),
 +  ts-type == BT_CHARACTER ? charlen : ts-kind);


 How do you distinguish between character(kind=1) and character(kind=4)? The
 same issue exists for a like-wise code in resolve_select_type.

I have introduced that distinction seemingly without anything breaking!
TYPE IS (CHARACTER(*, kind = 4) works fine.



 +  /* Unlimited polymorphic pointers should have their vptr nullified.  */
 +  if (UNLIMITED_POLY (sym)  CLASS_DATA (sym)-attr.pointer)
 +gfc_defer_symbol_init (sym);


 Why? If the pointer has never been pointer-associated, one shouldn't access

It's so that SAME_TYPE_AS and EXTENDS_TYPE_OF do the right thing with
unassociated pointers.  I am not sure that I understand your concern
if the code is needed.

Cheers

Paul


Re: Patch to enable unlimited polymorphism to gfortran

2012-12-16 Thread Tobias Burnus

Paul Richard Thomas wrote:

+  /* Unlimited polymorphic pointers should have their vptr nullified.  */
+  if (UNLIMITED_POLY (sym)  CLASS_DATA (sym)-attr.pointer)
+gfc_defer_symbol_init (sym);


Why? If the pointer has never been pointer-associated, one shouldn't access

It's so that SAME_TYPE_AS and EXTENDS_TYPE_OF do the right thing with
unassociated pointers.  I am not sure that I understand your concern
if the code is needed.


It's not really a concern. I just see it as missed code-size/performance 
optimization as I believe that it is not needed.


Adding
  ptr-_vptr = NULL
directly after declaration of a local pointer variable is pointless. 
Such a variable has an undefined pointer association status. And for 
same_type_as and extends_type_of the argument may not be such a pointer. 
Only unassociated pointers are allowed. In order to create such a 
pointer, one either needs an initialization like in

  class(*), pointer :: ptr = null()
or has to explicitly call ptr=null() or nullify(ptr). That's not 
different to other pointers and in particular not different to 
CLASS(name) types.


In other words, the following program is invalid as ptr has an 
undefined pointer association status:


type(t), target :: x
class(*), pointer :: ptr
print *, same_type_as (ptr, x)  !  invalid: foo is an undefined pointer

while the following code is valid (and causes an ICE):

type(t), target :: x
class(*), pointer :: ptr = NULL()  ! pointer initialization
print *, same_type_as (ptr, x)

as is

type(t), target :: x
class(*), pointer :: ptr
! ptr = x  ! optionally
ptr = NULL()
print *, same_type_as (ptr, x)

 * * *

By the way, the following code also causes an ICE. I think it is valid 
since Fortran 2008:


type t
end type t
type(t), target :: x
class(*), pointer :: ptr = x
print *, same_type_as (ptr, x)
end

Namely:

R442   component-initialization is   [...]  or   = initial-data-target
C460 (R442) If initial-data-target appears, component-name shall be 
data-pointer-initialization compatible with it.
If initial-data-target appears for a data pointer component, that 
component in any object of the type is initially associated with the 
target or becomes associated with the target as specied in 16.5.2.3.


Tobias


Re: [PATCH] Fix Infinite loop in pointer_set_insert

2012-12-16 Thread Dominique Dhumieres
 The attached patch fixes a regression introduced on the trunk to ...

This breaks bootstrap on x86_64-apple-darwin10:

../../work/gcc/varasm.c:2094:13: error: 'pending_assemble_externals_processed' 
defined but not used [-Werror=unused-variable]
 static bool pending_assemble_externals_processed;

I fixed it by moving

+/* Some targets delay some output to final using TARGET_ASM_FILE_END.
+   As a result, assemble_external can be called after the list of externals
+   is processed and the pointer set destroyed.  */
+static bool pending_assemble_externals_processed;
+

after

 #ifdef ASM_OUTPUT_EXTERNAL

instead of having it before.

TIA

Dominique


[patch] std::thread must rethrow __forced_unwind exceptions

2012-12-16 Thread Jonathan Wakely
* src/c++11/thread.cc (execute_native_thread_routine): Do not swallow
__forced_unwind exceptions.
* testsuite/30_threads/thread/native_handle/cancel.cc: New.

Tested x86_64-linux, committed to trunk, will commit to 4.7 and 4.6
branches asap.
commit 832f7b1b233d68ca635c2356615fd74691a33ee0
Author: Jonathan Wakely jwakely@gmail.com
Date:   Sun Dec 16 03:02:39 2012 +

* src/c++11/thread.cc (execute_native_thread_routine): Do not swallow
__forced_unwind exceptions.
* testsuite/30_threads/thread/native_handle/cancel.cc: New.

diff --git a/libstdc++-v3/src/c++11/thread.cc b/libstdc++-v3/src/c++11/thread.cc
index 084be42..fa86a1b 100644
--- a/libstdc++-v3/src/c++11/thread.cc
+++ b/libstdc++-v3/src/c++11/thread.cc
@@ -26,6 +26,7 @@
 #include thread
 #include system_error
 #include cerrno
+#include cxxabi_forced.h
 
 #if defined(_GLIBCXX_HAS_GTHREADS)  defined(_GLIBCXX_USE_C99_STDINT_TR1)
 
@@ -80,6 +81,10 @@ namespace std _GLIBCXX_VISIBILITY(default)
{
  __t-_M_run();
}
+  __catch(const __cxxabiv1::__forced_unwind)
+   {
+ __throw_exception_again;
+   }
   __catch(...)
{
  std::terminate();
diff --git a/libstdc++-v3/testsuite/30_threads/thread/native_handle/cancel.cc 
b/libstdc++-v3/testsuite/30_threads/thread/native_handle/cancel.cc
new file mode 100644
index 000..96b5791
--- /dev/null
+++ b/libstdc++-v3/testsuite/30_threads/thread/native_handle/cancel.cc
@@ -0,0 +1,44 @@
+// { dg-do run { target *-*-freebsd* *-*-netbsd* *-*-linux* *-*-solaris* 
*-*-cygwin *-*-darwin* powerpc-ibm-aix* } }
+// { dg-options  -std=gnu++0x -pthread { target *-*-freebsd* *-*-netbsd* 
*-*-linux* powerpc-ibm-aix* } }
+// { dg-options  -std=gnu++0x -pthreads { target *-*-solaris* } }
+// { dg-options  -std=gnu++0x  { target *-*-cygwin *-*-darwin* } }
+// { dg-require-cstdint  }
+// { dg-require-gthreads  }
+
+// Copyright (C) 2012 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// http://www.gnu.org/licenses/.
+
+#include pthread.h
+#include thread
+#include atomic
+
+void f(std::atomicbool started)
+{
+  started = true;
+  while (true)
+std::this_thread::sleep_for(std::chrono::milliseconds(100));
+}
+
+int main()
+{
+  std::atomicbool started{ false };
+  std::thread t(f, std::ref(started));
+  while (!started)
+std::this_thread::sleep_for(std::chrono::milliseconds(100));
+  pthread_cancel(t.native_handle());
+  t.join();
+}


[PATCH][Revisedx2] PR55679: skip invalid tests from r194458 on darwin

2012-12-16 Thread Jack Howarth
   The attached revised patch disables three asan tests introduced in r194458 
which are invalid
on darwin. The g++.dg/asan/interception-test-1.C test should be dg-skip-if'd  
on darwin since 
mac function interposition is used instead of interception. The 
c-c++-common/asan/swapcontext-test-1.c
test is invalid as ucontext support was deprecated in darwin10 and removed 
completely
in darwin11. The current implementation of check_effective_target_swapcontext in
lib/target-supports.exp doesn't catch the deprecation of ucontext support on 
darwin
because it doesn't try to parse the ucontext.h include file. The attached patch
changes the check_effective_target_swapcontext to use check_no_compiler_messages
with compilation of ucontext.h. It also skips the 
c-c++-common/asan/rlimit-mmap-test-1.c
on darwin since that target lacks RLIMIT_AS in setrlimit. Tested without 
regression on both 
x86_64-apple-darwin10 and x86_64-unknown-linux-gnu. Okay for gcc trunk?
 Jack
ps This patch leaves the failures in c-c++-common/asan/global-overflow-1.c and
c-c++-common/asan/stack-overflow-1.c as the only unresolved parts of PR55679 on 
darwin.
gcc/testsuite/

2012-12-16  Jack Howarth howa...@bromo.med.uc.edu

PR sanitizer/55679
* g++.dg/asan/interception-test-1.C: Skip on darwin.
* c-c++-common/asan/rlimit-mmap-test-1.c: Likewise.
* lib/target-supports.exp (check_effective_target_swapcontext): Use
check_no_compiler_messages to test support in ucontext.h.

Index: gcc/testsuite/lib/target-supports.exp
===
--- gcc/testsuite/lib/target-supports.exp   (revision 194538)
+++ gcc/testsuite/lib/target-supports.exp   (working copy)
@@ -736,7 +736,14 @@ proc check_effective_target_setrlimit {}
 
 # Return 1 if the target supports swapcontext, 0 otherwise.
 proc check_effective_target_swapcontext {} {
-return [check_function_available swapcontext]
+return [check_no_compiler_messages swapcontext executable {
+   #include ucontext.h
+   int main (void)
+   {
+ ucontext_t orig_context,child_context;
+ if (swapcontext(child_context, orig_context)  0) { }
+   }
+}]
 }
 
 # Return 1 if compilation with -pthread is error-free for trivial
Index: gcc/testsuite/g++.dg/asan/interception-test-1.C
===
--- gcc/testsuite/g++.dg/asan/interception-test-1.C (revision 194538)
+++ gcc/testsuite/g++.dg/asan/interception-test-1.C (working copy)
@@ -3,6 +3,7 @@
 // { dg-do run }
 // { dg-options -fno-builtin-malloc -fno-builtin-free }
 // { dg-shouldfail asan }
+// { dg-skip-if Darwin uses mac function interposition { *-*-darwin* } }
 
 #include stdlib.h
 #include stdio.h
Index: gcc/testsuite/c-c++-common/asan/rlimit-mmap-test-1.c
===
--- gcc/testsuite/c-c++-common/asan/rlimit-mmap-test-1.c(revision 
194538)
+++ gcc/testsuite/c-c++-common/asan/rlimit-mmap-test-1.c(working copy)
@@ -2,6 +2,7 @@
 
 /* { dg-do run { target setrlimit } } */
 /* { dg-skip-if  { *-*-* } { * } { -O0 } } */
+// { dg-skip-if Darwin lacks RLIMIT_AS in setrlimit { *-*-darwin* } }
 /* { dg-shouldfail asan } */
 
 #include stdlib.h


Re: [PATCH][Revisedx2] PR55679: skip invalid tests from r194458 on darwin

2012-12-16 Thread Dominique Dhumieres
Hi jack,

 +// { dg-skip-if Darwin lacks RLIMIT_AS in setrlimit { *-*-darwin* } }

The comment is not fully accurate. See pr55679 comment #17 for some analysis
and another candidate patch for rlimit-mmap-test-1.c (it will detect any fix
by Apple).

Cheers,

Dominique


Re: Patch to enable unlimited polymorphism to gfortran

2012-12-16 Thread Tobias Burnus

Paul Richard Thomas wrote:

The problem is with the way in which extends_type_of is organised.  It
takes the _vptr directly.  Unless it is null for undefined pointers, a
segfault is triggered.


So what? If I have:
  integer, pointer :: ptr
  ptr = 5
will also lead to a segfault (or a bus error); I do not see the 
difference. Here, the integer pointer ptr is not defj ned while for

  class(*), pointer :: poly_ptr
both the value (poly_ptr-_data) and the pointer to the virtual table 
(poly_ptr-_vptr) is uninitialized. I really do not see a difference. In 
any case, the Fortran standard explicitly doesn't allow code like

   class(*), pointer :: ptr, ptr2
   print *, same_type_as(ptr, ptr2)
as if either of the arguments is an undefined pointer. That's different 
to allocatables. Those are automatically in the nonallocatable state and 
hence

   class(*), allocatable :: ptr, ptr2
   print *, same_type_as(ptr, ptr2)
is valid.



Side remark: I think the following code is always true as you use 
attr.pointer instead of attr.class_pointer:


 +  if (UNLIMITED_POLY (sym)  CLASS_DATA (sym)-attr.pointer)
 +gfc_defer_symbol_init (sym);

Still, I believe that the automatic definition of sym-_vptr should only 
be done for allocatables (i.e. nonpointers). As CLASS(...), allocatable 
is already handled, the three lines should go for good.



I guess that I could achieve the same thing with the default initialization.


I am not sure whether I correctly understand this remark. I am think 
poly_ptr shouldn't be automatically be set, only via an explict pointer 
assignment, explict initialization, explict default initialization and 
explict nullify(). Hence, I believe adding code which does so 
automatically - and not when the user explicitly ask for it - is a 
missed optimization (useless gfortran code plus in extra instructions 
(code size/performance) for the generated code).



Initialization of class(*) pointers appears to be stuffed, as you
point out.  I'll try to figure it out tomorrow night.


Thanks! Please also ensure that

integer, target :: tgt
type t
   class(*), pointer :: poly1 = null()
   class(*), pointer :: poly2 = tgt
! class(*), pointer :: poly3 = poly2  ! Invalid*
end type

is supported. (* I believe this line is invalid: the data-target needs to be something which is 
link-time resolvable and poly3 = poly2 isn't as poly3 has to point to the 
target of poly2 not to poly2.)


Tobias



[PATCH] Another fix for PR54838

2012-12-16 Thread Marek Polacek
Here's a fix for the C++ TC part of this PR.  The issue was
that we had a loop with header and two latches, and via delete_basic_block
we deleted both latches, but we weren't removing the loop properly.
This ICEd later on because in merge_latch_edges  there's an assert:
gcc_assert (latches.length ()  0);

[Unfortunately this still not fixes profiledbootstrap with ada
mentioned in the PR (it seems to be a different issue, namely something
in cleanup_cfg in .into_cfglayout: we have a loop consisting of one
BB--its outcoming edge is at the same time its incoming edge--and we
remove this edge, but this loop is not removed).  The interim
workaround is to filter-out -fprofile-generate switch on targparm.o
in gcc/ada/gcc-interface/Make-lang.in.  I hope I will fix this one
during this week.]

Fixed thusly, bootstrapped/regtested on x86_64-linux, ok for trunk?

2012-12-15  Marek Polacek  pola...@redhat.com

PR middle-end/54838
* cfghooks.c (delete_basic_block): Remove loop if all the
latches are gone.
* cfgcleanup.c (delete_unreachable_blocks): Call
mark_dfs_back_edges.

* g++.dg/pr54838.C: New test.

--- gcc/cfghooks.c.mp   2012-12-15 04:03:18.388050403 +0100
+++ gcc/cfghooks.c  2012-12-15 04:03:35.313102810 +0100
@@ -541,11 +541,31 @@ delete_basic_block (basic_block bb)
   if (current_loops != NULL)
 {
   struct loop *loop = bb-loop_father;
+  bool is_last_latch = false;
+
+  /* We can get into situation where we have multiple latches.  We
+have to cancel the loop, if we remove all the latches--then
+the header won't have any incoming back edges.  */
+  if (loop-header
+  bb-loop_father != current_loops-tree_root)
+   {
+ edge e;
+ edge_iterator ei;
+ unsigned int n_back_edges = 0;
+ /* Check whether there's only one latch edge...  */
+ FOR_EACH_EDGE (e, ei, loop-header-preds)
+   if (e-flags  EDGE_DFS_BACK)
+ n_back_edges++;
+ /* ... and whether we're deleting its latch.  */
+ is_last_latch = n_back_edges == 1
+  find_edge (bb, loop-header);
+   }
 
   /* If we remove the header or the latch of a loop, mark the loop for
 removal by setting its header and latch to NULL.  */
   if (loop-latch == bb
- || loop-header == bb)
+ || loop-header == bb
+ || is_last_latch)
{
  loop-header = NULL;
  loop-latch = NULL;
--- gcc/testsuite/g++.dg/pr54838.C.mp   2012-12-15 03:52:52.436371725 +0100
+++ gcc/testsuite/g++.dg/pr54838.C  2012-12-15 03:52:36.962330626 +0100
@@ -0,0 +1,117 @@
+// PR middle-end/54838
+// { dg-do compile }
+// { dg-options -O2 -ftracer -fno-tree-dce -fno-tree-sra }
+
+  struct bidirectional_iterator_tag
+  {};
+  struct random_access_iterator_tag:bidirectional_iterator_tag
+  {};
+  template  typename _Category, typename, typename _Distance, typename  
struct iterator
+  {
+typedef _Distance difference_type;
+  };
+  template  typename _Iterator  struct iterator_traits
+  {
+typedef typename _Iterator::difference_type difference_type;
+  };
+  template  typename _Tp  struct iterator_traits _Tp * 
+  {
+typedef random_access_iterator_tag iterator_category;
+typedef _Tp value_type;
+typedef int difference_type;
+typedef _Tp reference;
+  };
+template  typename _Iterator  class reverse_iterator:
+  public
+iterator
+
+typename
+iterator_traits
+
+_Iterator
+::iterator_category,
+typename
+iterator_traits
+
+_Iterator
+::value_type,
+typename
+iterator_traits
+
+_Iterator
+::difference_type, typename iterator_traits  _Iterator ::reference 
+  {
+_Iterator current;
+  public:
+typedef _Iterator iterator_type;
+reverse_iterator (const reverse_iterator  __x):current (__x.current)
+{}
+iterator_type base ()
+{
+  return current;
+}
+reverse_iterator operator++ ()
+{
+  --current;
+}
+  };
+  template
+
+typename
+_Iterator
+
+bool
+operator
+==
+(reverse_iterator  _Iterator  __x, reverse_iterator  _Iterator  __y)
+  {
+return __x.base () == __y.base ();
+  }
+
+  template
+
+typename
+_Iterator
+
+typename
+reverse_iterator
+
+_Iterator
+::difference_type
+operator
+- (reverse_iterator  _Iterator , reverse_iterator  _Iterator )
+  {}
+  template
+
+typename
+_RandomAccessIterator
+
+_RandomAccessIterator
+__find
+(_RandomAccessIterator
+ __first, _RandomAccessIterator __last)
+  {
+typename
+  iterator_traits
+  
+  _RandomAccessIterator
+  ::difference_type __trip_count (__last - __first);
+for (; __trip_count; --__trip_count)
+++__first;
+   return __last;
+  }
+typedef reverse_iterator  int*  _ForwardIterator1;
+_ForwardIterator1
+search
+

Re: [cxx-conversion] Convert tree-sra.c'candidates to hash_table

2012-12-16 Thread Lawrence Crowl
On 12/13/12, Martin Jambor mjam...@suse.cz wrote:
 On Thu, Dec 13, 2012 at 11:05:49AM -0800, Lawrence Crowl wrote:
  On 12/12/12, Jakub Jelinek ja...@redhat.com wrote:
   On Tue, Dec 11, 2012 at 02:44:41PM -0800, Lawrence Crowl wrote:
+/* Hash a tree in a uid_decl_map.  */
+
+inline hashval_t
+uid_decl_hasher::hash (const value_type *item)
+{
+  return item-decl_minimal.uid;
  
   Ugh, why aren't you using DECL_UID here?  The fact that DECL_UID is
   right now defined to (DECL_MINIMAL_CHECK (NODE)-decl_minimal.uid)
   might change, we really don't want to update thousands of locations
   should we need to change that.
 
  That is what the code looked like in its original location.  I have
  resisted making changes not essential to the purpose of the patch.
  However, if you want me to change it to use DECL_UID, I will.

 Yes, I think it ought to be changed.
 Thanks for the conversion,

It turns out that making the change causes the check to fail.
Therefore, I will leave the code alone and let someone more familiar
with the code investigate.

-- 
Lawrence Crowl