Re: [patch testsuite]: Adjust gcc.dg/tree-ssa tests for LLP64 target

2011-08-07 Thread Kai Tietz
2011/8/7 Mike Stump :
> On Aug 6, 2011, at 6:04 AM, Kai Tietz wrote:
>> this adjusts some testcases for LLP64 target x86_64 mingw.
>
> Ok.  Please watch for any comments on stdarg...  I don't have any, but others 
> might.

Ok, will do.  Applied at rev 177543.

Thanks,
Kai


Re: [Patch, Fortran, OOP] PR 49638: [OOP] length parameter is ignored when overriding type bound character functions with constant length.

2011-08-07 Thread Thomas Koenig

Hi Janus,


Here is a preparational patch (basically a subset of the
previous one), which does not do any real changes yet, only some
preparation and cleanup:
* It moves check_typebound_override to interface.c and prefixes it
with gfc_ (I don't like moving and modifying it at the same time).
* It add the commutativity of multiplication in gfc_dep_compare_expr.
* It does some minor cleanup in dependency.c (making two routines
static and removing an unused argument).

Ok for trunk?

Cheers,
Janus


2011-08-06  Janus Weil

PR fortran/49638
* dependency.h (gfc_is_same_range,gfc_are_identical_variables): Remove
two prototypes.
* dependency.c (gfc_are_identical_variables): Made static and renamed.
(gfc_dep_compare_expr): Renamed 'gfc_are_identical_variables', handle
commutativity of multiplication.
(gfc_is_same_range): Made static and renamed, removed argument 'def'.
(check_section_vs_section): Renamed 'gfc_is_same_range'.
* gfortran.h (gfc_check_typebound_override): New prototype.
* interface.c (gfc_check_typebound_override): Moved here from ...
* resolv.c (check_typebound_override): ... here (and renamed).
(resolve_typebound_procedure): Renamed 'check_typebound_override'.


OK from my side (given Mikael's comments), provided you spell resolve.c 
with two e :-)


Regards

Thomas


Re: [Patch, Fortran, OOP] PR 49638: [OOP] length parameter is ignored when overriding type bound character functions with constant length.

2011-08-07 Thread Janus Weil
2011/8/7 Thomas Koenig :
> Hi Janus,
>
>> Here is a preparational patch (basically a subset of the
>> previous one), which does not do any real changes yet, only some
>> preparation and cleanup:
>> * It moves check_typebound_override to interface.c and prefixes it
>> with gfc_ (I don't like moving and modifying it at the same time).
>> * It add the commutativity of multiplication in gfc_dep_compare_expr.
>> * It does some minor cleanup in dependency.c (making two routines
>> static and removing an unused argument).
>>
>> Ok for trunk?
>>
>> Cheers,
>> Janus
>>
>>
>> 2011-08-06  Janus Weil
>>
>>        PR fortran/49638
>>        * dependency.h (gfc_is_same_range,gfc_are_identical_variables):
>> Remove
>>        two prototypes.
>>        * dependency.c (gfc_are_identical_variables): Made static and
>> renamed.
>>        (gfc_dep_compare_expr): Renamed 'gfc_are_identical_variables',
>> handle
>>        commutativity of multiplication.
>>        (gfc_is_same_range): Made static and renamed, removed argument
>> 'def'.
>>        (check_section_vs_section): Renamed 'gfc_is_same_range'.
>>        * gfortran.h (gfc_check_typebound_override): New prototype.
>>        * interface.c (gfc_check_typebound_override): Moved here from ...
>>        * resolv.c (check_typebound_override): ... here (and renamed).
>>        (resolve_typebound_procedure): Renamed 'check_typebound_override'.
>
> OK from my side (given Mikael's comments), provided you spell resolve.c with
> two e :-)

Thanks, guys. Committed as r177545 (including your comments).

Cheers,
Janus


Re: [Patch, Fortran, OOP] PR 49638: [OOP] length parameter is ignored when overriding type bound character functions with constant length.

2011-08-07 Thread Janus Weil
>> OK from my side (given Mikael's comments), provided you spell resolve.c with
>> two e :-)
>
> Thanks, guys. Committed as r177545 (including your comments).


Ok, with the 'trivial' parts out of the way, the remainder of my
previously proposed patch becomes rather compact (see attachment).

Thomas' ongoing criticism is that rejecting all nonzero return values
of 'gfc_dep_compare_expr' will reject too much (i.e. cases where we
can not prove that the expressions are equal, but they may still be).

My feeling is that only throwing a warning for a result of '-2' is too
weak, because the majority of cases will have that result (just think
'len=3' vs. 'len=n').

Instead I could offer to try to extend the return values of
'gfc_dep_compare_expr' to distinguish between the following cases
(which right now would both result in '-2'):

a) We can not determine the relationship between both expressions, but
we know they are different for certain input values. (This case would
include e.g. different expr_type)

b) We cannot make any statement.

Is this the way to go? Or are there any other proposals for resolving
this argument?

Cheers,
Janus
Index: gcc/fortran/interface.c
===
--- gcc/fortran/interface.c	(revision 177545)
+++ gcc/fortran/interface.c	(working copy)
@@ -3556,15 +3556,25 @@ gfc_check_typebound_override (gfc_symtree* proc, g
 	}
 
   /* FIXME:  Do more comprehensive checking (including, for instance, the
-	 rank and array-shape).  */
+	 array-shape).  */
   gcc_assert (proc_target->result && old_target->result);
-  if (!gfc_compare_types (&proc_target->result->ts,
-			  &old_target->result->ts))
+  if (!compare_type_rank (proc_target->result, old_target->result))
 	{
 	  gfc_error ("'%s' at %L and the overridden FUNCTION should have"
-		 " matching result types", proc->name, &where);
+		 " matching result types and ranks", proc->name, &where);
 	  return FAILURE;
 	}
+	
+  /* Check string length.  */
+  if (proc_target->result->ts.type == BT_CHARACTER
+	  && proc_target->result->ts.u.cl && old_target->result->ts.u.cl
+	  && gfc_dep_compare_expr (proc_target->result->ts.u.cl->length,
+   old_target->result->ts.u.cl->length) != 0)
+	{
+	  gfc_error ("Character length mismatch between '%s' at '%L' "
+		 "and overridden FUNCTION", proc->name, &where);
+	  return FAILURE;
+	}
 }
 
   /* If the overridden binding is PUBLIC, the overriding one must not be
Index: gcc/fortran/dependency.c
===
--- gcc/fortran/dependency.c	(revision 177545)
+++ gcc/fortran/dependency.c	(working copy)
@@ -123,8 +123,18 @@ are_identical_variables (gfc_expr *e1, gfc_expr *e
 {
   gfc_ref *r1, *r2;
 
-  if (e1->symtree->n.sym != e2->symtree->n.sym)
-return false;
+  if (e1->symtree->n.sym->attr.dummy && e2->symtree->n.sym->attr.dummy)
+{
+  /* Dummy arguments: Only check for equal names.  */
+  if (e1->symtree->n.sym->name != e2->symtree->n.sym->name)
+	return false;
+}
+  else
+{
+  /* Check for equal symbols.  */
+  if (e1->symtree->n.sym != e2->symtree->n.sym)
+	return false;
+}
 
   /* Volatile variables should never compare equal to themselves.  */
 


Re: [PATCH] [JAVA] Double.parseDouble(null) throw NullPointerException

2011-08-07 Thread Jie Liu
Hi,

When I use gcj on an RTOS(RTEMS), Double.parseDouble(null) throw
NumberFormatException, but it should throw NullPointerException. So I
add the patch below:

Index: natVMDouble.cc
===
--- natVMDouble.cc  (revision 172224)
+++ natVMDouble.cc  (working copy)
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 

 #include 
@@ -162,6 +163,9 @@
 jdouble
 java::lang::VMDouble::parseDouble(jstring str)
 {
+  if(str == NULL)
+  throw new NullPointerException();
+
   int length = str->length();

   while (length > 0

The testsuite/Throw_2.java has been PASS after this patch. what do you
think about this patch?

Thanks,
Jie


Re: [PATCH PR43513, 1/3] Replace vla with array - Implementation.

2011-08-07 Thread Tom de Vries
On 07/30/2011 09:21 AM, Tom de Vries wrote:
> Hi,
> 
> On 07/28/2011 08:20 PM, Tom de Vries wrote:
>> On 07/28/2011 06:25 PM, Richard Guenther wrote:
>>> On Thu, 28 Jul 2011, Tom de Vries wrote:
>>>
 On 07/28/2011 12:22 PM, Richard Guenther wrote:
> On Wed, 27 Jul 2011, Tom de Vries wrote:
>
>> On 07/27/2011 05:27 PM, Richard Guenther wrote:
>>> On Wed, 27 Jul 2011, Tom de Vries wrote:
>>>
 On 07/27/2011 02:12 PM, Richard Guenther wrote:
> On Wed, 27 Jul 2011, Tom de Vries wrote:
>
>> On 07/27/2011 01:50 PM, Tom de Vries wrote:
>>> Hi Richard,
>>>
>>> I have a patch set for bug 43513 - The stack pointer is adjusted 
>>> twice.
>>>
>>> 01_pr43513.3.patch
>>> 02_pr43513.3.test.patch
>>> 03_pr43513.3.mudflap.patch
>>>
>>> The patch set has been bootstrapped and reg-tested on x86_64.
>>>
>>> I will sent out the patches individually.
>>>
>>
>> The patch replaces a vla __builtin_alloca that has a constant 
>> argument with an
>> array declaration.
>>
>> OK for trunk?
>
> I don't think it is safe to try to get at the VLA type the way you do.

 I don't understand in what way it's not safe. Do you mean I don't 
 manage to find
 the type always, or that I find the wrong type, or something else?
>>>
>>> I think you might get the wrong type,
>>
>> Ok, I'll review that code one more time.
>>
>>> you also do not transform code
>>> like
>>>
>>>   int *p = alloca(4);
>>>   *p = 3;
>>>
>>> as there is no array type involved here.
>>>
>>
>> I was trying to stay away from non-vla allocas.  A source declared 
>> alloca has
>> function livetime, so we could have a single alloca in a loop, called 10 
>> times,
>> with all 10 instances live at the same time. This patch does not detect 
>> such
>> cases, and thus stays away from non-vla allocas. A vla decl does not 
>> have such
>> problems, the lifetime ends when it goes out of scope.
>
> Yes indeed - that probably would require more detailed analysis.
>
> In fact I would simply do sth like
>
>   elem_type = build_nonstandard_integer_type (BITS_PER_UNIT, 1);
>   n_elem = size * 8 / BITS_PER_UNIT;
>   array_type = build_array_type_nelts (elem_type, n_elem);
>   var = create_tmp_var (array_type, NULL);
>   return fold_convert (TREE_TYPE (lhs), build_fold_addr_expr (var));
>

 I tried this code on the example, and it works, but the newly declared 
 type has
 an 8-bit alignment, while the vla base type has a 32 bit alignment.  
 This make
 the memory access in the example potentially unaligned, which 
 prohibits an
 ivopts optimization, so the resulting text size is 68 instead of the 
 64 achieved
 with my current patch.
>>>
>>> Ok, so then set DECL_ALIGN of the variable to something reasonable
>>> like MIN (size * 8, GET_MODE_PRECISION (word_mode)).  Basically the
>>> alignment that the targets alloca function would guarantee.
>>>
>>
>> I tried that, but that doesn't help. It's the alignment of the type that
>> matters, not of the decl.
>
> It shouldn't.  All accesses are performed with the original types and
> alignment comes from that (plus the underlying decl).
>

 I managed to get it all working by using build_aligned_type rather that 
 DECL_ALIGN.
>>>
>>> That's really odd, DECL_ALIGN should just work - nothing refers to the
>>> type of the decl in the IL.  Can you try also setting DECL_USER_ALIGN to 
>>> 1 maybe?
>>>
>>
>> This doesn't work either.
>>
>>   /* Declare array.  */
>>   elem_type = build_nonstandard_integer_type (BITS_PER_UNIT, 1);
>>   n_elem = size * 8 / BITS_PER_UNIT;
>>   align = MIN (size * 8, GET_MODE_PRECISION (word_mode));
>>   array_type = build_array_type_nelts (elem_type, n_elem);
>>   var = create_tmp_var (array_type, NULL);
>>   DECL_ALIGN (var) = align;
>>   DECL_USER_ALIGN (var) = 1;
>>
>> Maybe this clarifies it:
>>
>> Breakpoint 1, may_be_unaligned_p (ref=0xf7d9d410, step=0xf7d3d578) at
>> /home/vries/local/google/src/gcc-mainline/gcc/tree-ssa-loop-ivopts.c:1621
>> (gdb) call debug_generic_expr (ref)
>> MEM[(int[0:D.2579] *)&D.2595][0]
>> (gdb) call debug_generic_expr (step)
>> 4
>>
>> 1627   base = get_inner_reference (ref, &bitsize, &bitpos, &toffset, &mode,
>> (gdb) call debug_generic_expr (base)
>> D.2595
>>
>> 1629   base_type = TREE_TYPE (base);
>> (gdb) call debug_generic_expr (base_type)
>> [40]
>>
>> 1630   base_align = TYPE_ALIGN (base_type);
>> (gdb) p base_align
>> $1 = 8
>>
>> So the align is 8-bits, and we return true here:
>>
>> (gdb) n
>> 1632   if (mode != BLKmode)
>> (gdb) n

Re: [v3] doxygen markup fixes

2011-08-07 Thread Jonathan Wakely
On 7 August 2011 08:30, Benjamin Kosnik  wrote:
>
> This fixes markup that gives warnings or errors as part of html/pdf
> documentation generation. Most of this stuff is pretty simple, and is
> just making sure that the documented parameter names exactly match the
> signatures.
>
> tested x86/linux
>
> -benjamin

+/** @file bits/alloc_traits.h
+ *  This is an internal header file, included by other library headers.
+ *  Do not attempt to use it directly. @headername{scoped_allocator}
+ */
+

The correct header for allocator_traits is 


Re: [Patch, Fortran, OOP] PR 49638: [OOP] length parameter is ignored when overriding type bound character functions with constant length.

2011-08-07 Thread Thomas Koenig

Am 07.08.2011 12:56, schrieb Janus Weil:

+  /* Check string length.  */
+  if (proc_target->result->ts.type == BT_CHARACTER
+   &&  proc_target->result->ts.u.cl&&  old_target->result->ts.u.cl
+   &&  gfc_dep_compare_expr (proc_target->result->ts.u.cl->length,
+  old_target->result->ts.u.cl->length) != 0)
+   {
+ gfc_error ("Character length mismatch between '%s' at '%L' "
+"and overridden FUNCTION", proc->name,&where);
+ return FAILURE;
+   }
  }


Well, let's make this into (again, typing the patch directly into
e-mail)


  /* Check string length.  */
  if (proc_target->result->ts.type == BT_CHARACTER
  && proc_target->result->ts.u.cl && old_target->result->ts.u.cl
{
   int compval =
  gfc_dep_compare_expr (proc_target->result->ts.u.cl->length,
   old_target->result->ts.u.cl->length);

 switch (compval)
{
case -3:
case -1:
case 1:

  gfc_error ("Character length mismatch between '%s' at '%L' "
 "and overridden FUNCTION", proc->name, &where);
  return FAILURE;

case -2:
   gfc_warning ("Possible length mismatch between '%s' at '%L' "
"and overriden FUNCTION, proc->name, &where);
   break;

case 0:
break;

default:
gfc_internal_error ("Unexpected return of gfc_dep_compare_expr";
   break;
   }

and then work on extending gfc_dep_compare_expr to return -3 for more 
cases.  I can help with that.


Regards

Thomas


Re: [v3] doxygen markup fixes

2011-08-07 Thread Jonathan Wakely
On 7 August 2011 12:53, Jonathan Wakely  wrote:
> On 7 August 2011 08:30, Benjamin Kosnik  wrote:
>>
>> This fixes markup that gives warnings or errors as part of html/pdf
>> documentation generation. Most of this stuff is pretty simple, and is
>> just making sure that the documented parameter names exactly match the
>> signatures.
>>
>> tested x86/linux
>>
>> -benjamin
>
> +/** @file bits/alloc_traits.h
> + *  This is an internal header file, included by other library headers.
> + *  Do not attempt to use it directly. @headername{scoped_allocator}
> + */
> +
>
> The correct header for allocator_traits is 
>

Fixed like so.

2011-08-07  Jonathan Wakely  

* include/bits/alloc_traits.h: Fix doxygen @headername.

Index: include/bits/alloc_traits.h
===
--- include/bits/alloc_traits.h (revision 177545)
+++ include/bits/alloc_traits.h (working copy)
@@ -24,7 +24,7 @@

 /** @file bits/alloc_traits.h
  *  This is an internal header file, included by other library headers.
- *  Do not attempt to use it directly. @headername{scoped_allocator}
+ *  Do not attempt to use it directly. @headername{memory}
  */

 #ifndef _ALLOC_TRAITS_H


Re: [RFC PATCH, i386]: Allow zero_extended addresses (+ problems with reload and offsetable address, "o" constraint)

2011-08-07 Thread Uros Bizjak
On Fri, Aug 5, 2011 at 8:51 PM, Uros Bizjak  wrote:

> As I read this sentence, the RTX is forced into a temporary register,
> and reload tries to satisfy "o" constraint with plus ((reg ...)
> (const_int ...)), as said at the introduction of "o" constraint a
> couple of pages earlier. Unfortunately, this does not seem to be the
> case.
>
> Is there anything wrong with my approach, or is there something wrong in 
> reload?

To answer my own question, the problem was in *add3_doubleword
pattern, defined as:

(define_insn_and_split "*add3_doubleword"
  [(set (match_operand: 0 "nonimmediate_operand" "=r,o")
(plus:
  (match_operand: 1 "nonimmediate_operand" "%0,0")
  (match_operand: 2 "" "ro,r")))
   (clobber (reg:CC FLAGS_REG))]
  "ix86_binary_operator_ok (PLUS, mode, operands)"

When reload tried to satisfy alternative 1 (the "o" and matching "0")
with a non-offsettable (in this particular case, zero-extended)
address, it CSE'd operand 0 and operand 1 to a temporary TImode
register. Unfortunately a Timode move has its own constraints:

(define_insn "*movti_internal_rex64"
  [(set (match_operand:TI 0 "nonimmediate_operand" "=!r,o,x,x,xm")
(match_operand:TI 1 "general_operand" "riFo,riF,C,xm,x"))]
  "TARGET_64BIT && !(MEM_P (operands[0]) && MEM_P (operands[1]))"

where move from/to a general register to/from non-offsettable memory
is not valid.

Although, it would be nice for reload to subsequently fix CSE'd
non-offsetable memory by copying address to temporary reg (*as said in
the documentation*), we could simply require an XMM temporary for
TImode reloads to/from integer registers, and this fixes ICE for x32.

The testcase to play with (gcc -O2 -mx32):

--cut here--
void test (__int128 *array, int idx, int off)
{
  __int128 *dest = &array [idx];

  dest[0] += 1;
  dest[off] = 0;
}
--cut here--

So, following additional patch saves the day:

Index: i386/i386.c
===
--- i386/i386.c (revision 177536)
+++ i386/i386.c (working copy)
@@ -28233,6 +28248,15 @@ ix86_secondary_reload (bool in_p, rtx x, reg_class
   enum machine_mode mode,
   secondary_reload_info *sri ATTRIBUTE_UNUSED)
 {
+  /* Double-word spills from general registers to non-offsettable
+ memory references go through XMM register.  Following code
+ handles zero-extended addresses on x32 target.  */
+  if (TARGET_64BIT
+  && GET_MODE_SIZE (mode) > UNITS_PER_WORD
+  && rclass == GENERAL_REGS
+  && !offsettable_memref_p (x))
+return SSE_REGS;
+
   /* QImode spills from non-QI registers require
  intermediate register on 32bit targets.  */
   if (!TARGET_64BIT

Uros.


Re: [Patch, Fortran, OOP] PR 49638: [OOP] length parameter is ignored when overriding type bound character functions with constant length.

2011-08-07 Thread Janus Weil
2011/8/7 Thomas Koenig :
> Am 07.08.2011 12:56, schrieb Janus Weil:
>>
>> +      /* Check string length.  */
>> +      if (proc_target->result->ts.type == BT_CHARACTER
>> +       &&  proc_target->result->ts.u.cl&&  old_target->result->ts.u.cl
>> +       &&  gfc_dep_compare_expr (proc_target->result->ts.u.cl->length,
>> +                                  old_target->result->ts.u.cl->length) !=
>> 0)
>> +       {
>> +         gfc_error ("Character length mismatch between '%s' at '%L' "
>> +                    "and overridden FUNCTION", proc->name,&where);
>> +         return FAILURE;
>> +       }
>>      }
>
> Well, let's make this into (again, typing the patch directly into
> e-mail)
>
>  [...]
>
> and then work on extending gfc_dep_compare_expr to return -3 for more cases.
>  I can help with that.

Alright then. How about this: I'll commit the attached verision of the
patch (including your suggestions), and we start messing with the
return values afterwards? Patch is regtested on
x86_64-unknown-linux-gnu. I hope the test case is sufficient for a
start.

Cheers,
Janus


2011-08-07  Janus Weil  

PR fortran/49638
* dependency.c (are_identical_variables): For dummy arguments only
check for equal names, not equal symbols.
* interface.c (gfc_check_typebound_override): Add checking for rank
and character length.

2011-08-07  Janus Weil  

PR fortran/49638
* gfortran.dg/typebound_override_1.f90: New.
Index: gcc/fortran/interface.c
===
--- gcc/fortran/interface.c	(revision 177545)
+++ gcc/fortran/interface.c	(working copy)
@@ -3556,15 +3556,43 @@ gfc_check_typebound_override (gfc_symtree* proc, g
 	}
 
   /* FIXME:  Do more comprehensive checking (including, for instance, the
-	 rank and array-shape).  */
+	 array-shape).  */
   gcc_assert (proc_target->result && old_target->result);
-  if (!gfc_compare_types (&proc_target->result->ts,
-			  &old_target->result->ts))
+  if (!compare_type_rank (proc_target->result, old_target->result))
 	{
 	  gfc_error ("'%s' at %L and the overridden FUNCTION should have"
-		 " matching result types", proc->name, &where);
+		 " matching result types and ranks", proc->name, &where);
 	  return FAILURE;
 	}
+	
+  /* Check string length.  */
+  if (proc_target->result->ts.type == BT_CHARACTER
+	  && proc_target->result->ts.u.cl && old_target->result->ts.u.cl)
+	{
+	  int compval = gfc_dep_compare_expr (proc_target->result->ts.u.cl->length,
+	  old_target->result->ts.u.cl->length);
+	  switch (compval)
+	  {
+	case -1:
+	case 1:
+	  gfc_error ("Character length mismatch between '%s' at '%L' and "
+			 "overridden FUNCTION", proc->name, &where);
+	  return FAILURE;
+
+	case -2:
+	  gfc_warning ("Possible character length mismatch between '%s' at"
+			   " '%L' and overridden FUNCTION", proc->name, &where);
+	  break;
+
+	case 0:
+	  break;
+
+	default:
+	  gfc_internal_error ("gfc_check_typebound_override: Unexpected "
+  "result %i of gfc_dep_compare_expr", compval);
+	  break;
+	  }
+	}
 }
 
   /* If the overridden binding is PUBLIC, the overriding one must not be
Index: gcc/fortran/dependency.c
===
--- gcc/fortran/dependency.c	(revision 177545)
+++ gcc/fortran/dependency.c	(working copy)
@@ -123,8 +123,18 @@ are_identical_variables (gfc_expr *e1, gfc_expr *e
 {
   gfc_ref *r1, *r2;
 
-  if (e1->symtree->n.sym != e2->symtree->n.sym)
-return false;
+  if (e1->symtree->n.sym->attr.dummy && e2->symtree->n.sym->attr.dummy)
+{
+  /* Dummy arguments: Only check for equal names.  */
+  if (e1->symtree->n.sym->name != e2->symtree->n.sym->name)
+	return false;
+}
+  else
+{
+  /* Check for equal symbols.  */
+  if (e1->symtree->n.sym != e2->symtree->n.sym)
+	return false;
+}
 
   /* Volatile variables should never compare equal to themselves.  */
 
! { dg-do compile }
!
! PR 49638: [OOP] length parameter is ignored when overriding type bound character functions with constant length.
!
! Original test case contributed by Hans-Werner Boschmann 

module m

  implicit none

  type :: t1
   contains
 procedure, nopass :: a => a1
 procedure, nopass :: b => b1
 procedure, nopass :: c => c1
 procedure, nopass :: d => d1
 procedure, nopass :: e => e1
  end type

  type, extends(t1) :: t2
   contains
 procedure, nopass :: a => a2  ! { dg-error "Character length mismatch" }
 procedure, nopass :: b => b2  ! { dg-error "should have matching result types and ranks" }
 procedure, nopass :: c => c2  ! { dg-warning "Possible character length mismatch" }
 procedure, nopass :: d => d2  ! valid, check for commutativity (+,*)
 procedure, nopass :: e => e2  ! { dg-warning "Possible character length mismatch" }
  end type

contains

  function a1 ()
characte

Re: [PATCH] ARM fixed-point support [5.5/6]: argument & return padding for libcalls

2011-08-07 Thread Richard Sandiford
Hi Julian,

Julian Brown  writes:
> This patch allows padding to be specified per-target for libcalls. This
> hasn't been traditionally important, because libcalls haven't accepted
> quantities which might need padding, but that's no longer true with the
> new(-ish) fixed-point support helper functions.
>
> Tested (alongside other fixed-point support patches) with cross to ARM
> EABI in both big & little-endian mode (the target-specific part is to
> avoid a behaviour change for half-float types on ARM). OK to apply?

This patch caused several regressions on big-endian 64-bit MIPS targets,
which now try to shift single-precision floating-point arguments to
the top of an FPR.  The calls.c part...

> diff --git a/gcc/calls.c b/gcc/calls.c
> index 44a16ff..9d5d294 100644
> --- a/gcc/calls.c
> +++ b/gcc/calls.c
> @@ -3794,13 +3794,41 @@ emit_library_call_value_1 (int retval, rtx orgfun, 
> rtx value,
>rtx val = argvec[argnum].value;
>rtx reg = argvec[argnum].reg;
>int partial = argvec[argnum].partial;
> -
> +  int size = 0;
> +  
>/* Handle calls that pass values in multiple non-contiguous
>locations.  The PA64 has examples of this for library calls.  */
>if (reg != 0 && GET_CODE (reg) == PARALLEL)
>   emit_group_load (reg, val, NULL_TREE, GET_MODE_SIZE (mode));
>else if (reg != 0 && partial == 0)
> - emit_move_insn (reg, val);
> +{
> +   emit_move_insn (reg, val);
> +#ifdef BLOCK_REG_PADDING
> +   size = GET_MODE_SIZE (argvec[argnum].mode);
> +
> +   /* Copied from load_register_parameters.  */
> +
> +   /* Handle case where we have a value that needs shifting
> +  up to the msb.  eg. a QImode value and we're padding
> +  upward on a BYTES_BIG_ENDIAN machine.  */
> +   if (size < UNITS_PER_WORD
> +   && (argvec[argnum].locate.where_pad
> +   == (BYTES_BIG_ENDIAN ? upward : downward)))
> + {
> +   rtx x;
> +   int shift = (UNITS_PER_WORD - size) * BITS_PER_UNIT;
> +
> +   /* Assigning REG here rather than a temp makes CALL_FUSAGE
> +  report the whole reg as used.  Strictly speaking, the
> +  call only uses SIZE bytes at the msb end, but it doesn't
> +  seem worth generating rtl to say that.  */
> +   reg = gen_rtx_REG (word_mode, REGNO (reg));
> +   x = expand_shift (LSHIFT_EXPR, word_mode, reg, shift, reg, 1);
> +   if (x != reg)
> + emit_move_insn (reg, x);
> + }
> +#endif
> + }
>  
>NO_DEFER_POP;
>  }

...looks good in itself.  The problem on MIPS is the same one that
you mention in this "???" comment:

> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index 7d52b0e..cd32fe3 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -11375,6 +11375,15 @@ arm_pad_arg_upward (enum machine_mode mode, 
> const_tree type)
>if (type && BYTES_BIG_ENDIAN && INTEGRAL_TYPE_P (type))
>  return false;
>  
> +  /* Half-float values are only passed to libcalls, not regular functions.
> + They should be passed and returned as "short"s (see RTABI).  To achieve
> + that effect in big-endian mode, pad downwards so the value is passed in
> + the least-significant end of the register.  ??? This needs to be here
> + rather than in arm_pad_reg_upward due to peculiarity in the handling of
> + libcall arguments.  */
> +  if (BYTES_BIG_ENDIAN && mode == HFmode)
> +return false;
> +

in that the value of argvec[argnum].locate.where_pad is coming from
the wrong macro: FUNCTION_ARG_PADDING rather than BLOCK_REG_PADDING.
So while the code above is conditional on BLOCK_REG_PADDING, it's
actually using the value returned by FUNCTION_ARG_PADDING instead.

This represents an ABI change.  It looks like your arm.c patch is trying
to partially undo that change for ARM, but it still affects other targets
in a similar way.

The patch below borrows the padding code from the main call routines.
It fixes the MIPS problems for me (tested on mips64-linux-gnu),
but I'm not set up for big-endian ARM testing.  From what I can tell,
other targets' BLOCK_REG_PADDING definitions already handle null types.

Richard


Index: gcc/calls.c
===
*** gcc/calls.c 2011-08-07 11:11:23.0 +0100
--- gcc/calls.c 2011-08-07 11:11:27.0 +0100
*** emit_library_call_value_1 (int retval, r
*** 3576,3595 
argvec[count].partial
= targetm.calls.arg_partial_bytes (args_so_far, mode, NULL_TREE, 1);
  
!   locate_and_pad_parm (mode, NULL_TREE,
  #ifdef STACK_PARMS_IN_REG_PARM_AREA
!  1,
  #else
!  argvec[count].reg != 0,
  #endif
-  argvec[count].partial,
-  NULL_TREE, &args_size, &argvec[count].locate);
- 
-   gcc_assert (!argvec[count].locate.size.var);
- 

Re: [Patch, Fortran, OOP] PR 49638: [OOP] length parameter is ignored when overriding type bound character functions with constant length.

2011-08-07 Thread Janus Weil
2011/8/7 Janus Weil :
> 2011/8/7 Thomas Koenig :
>> Am 07.08.2011 12:56, schrieb Janus Weil:
>>>
>>> +      /* Check string length.  */
>>> +      if (proc_target->result->ts.type == BT_CHARACTER
>>> +       &&  proc_target->result->ts.u.cl&&  old_target->result->ts.u.cl
>>> +       &&  gfc_dep_compare_expr (proc_target->result->ts.u.cl->length,
>>> +                                  old_target->result->ts.u.cl->length) !=
>>> 0)
>>> +       {
>>> +         gfc_error ("Character length mismatch between '%s' at '%L' "
>>> +                    "and overridden FUNCTION", proc->name,&where);
>>> +         return FAILURE;
>>> +       }
>>>      }
>>
>> Well, let's make this into (again, typing the patch directly into
>> e-mail)
>>
>>  [...]
>>
>> and then work on extending gfc_dep_compare_expr to return -3 for more cases.
>>  I can help with that.
>
> Alright then. How about this: I'll commit the attached verision of the
> patch (including your suggestions), and we start messing with the
> return values afterwards? Patch is regtested on
> x86_64-unknown-linux-gnu. I hope the test case is sufficient for a
> start.
>
> Cheers,
> Janus
>
>
> 2011-08-07  Janus Weil  

Sorry, completely forgot to mention Thomas in the ChangeLog. Of course
this should be

2011-08-07  Janus Weil  
Thomas Koenig  

Cheers,
Janus



>
>        PR fortran/49638
>        * dependency.c (are_identical_variables): For dummy arguments only
>        check for equal names, not equal symbols.
>        * interface.c (gfc_check_typebound_override): Add checking for rank
>        and character length.
>
> 2011-08-07  Janus Weil  
>
>        PR fortran/49638
>        * gfortran.dg/typebound_override_1.f90: New.
>


Re: [Patch, Fortran, OOP] PR 49638: [OOP] length parameter is ignored when overriding type bound character functions with constant length.

2011-08-07 Thread Thomas Koenig

Am 07.08.2011 19:05, schrieb Janus Weil:

2011/8/7 Thomas Koenig:

Am 07.08.2011 12:56, schrieb Janus Weil:


+  /* Check string length.  */
+  if (proc_target->result->ts.type == BT_CHARACTER
+&&proc_target->result->ts.u.cl&&old_target->result->ts.u.cl
+&&gfc_dep_compare_expr (proc_target->result->ts.u.cl->length,
+  old_target->result->ts.u.cl->length) !=
0)
+   {
+ gfc_error ("Character length mismatch between '%s' at '%L' "
+"and overridden FUNCTION", proc->name,&where);
+ return FAILURE;
+   }
  }


Well, let's make this into (again, typing the patch directly into
e-mail)

  [...]

and then work on extending gfc_dep_compare_expr to return -3 for more cases.
  I can help with that.


Alright then. How about this: I'll commit the attached verision of the
patch (including your suggestions), and we start messing with the
return values afterwards? Patch is regtested on
x86_64-unknown-linux-gnu. I hope the test case is sufficient for a
start.


This is OK.  Thanks for the patch (and for bearing with me ;-)

When extending the values of gfc_dep_compare_expr, we will need to go
through all its uses (making sure we change == -2 to <= -2).

Regards

Thomas


PATCH: PR middle-end/49721: convert_memory_address_addr_space may generate invalid new insns

2011-08-07 Thread H.J. Lu
Hi,

We transform

ptr_extend:DI (plus:SI (FOO:SI) (const_int Y)))

to

(plus:DI (ptr_extend:DI (FOO:SI)) (const_int Y))

since this is how Pmode != ptr_mode is supported even if the resulting
address may overflow/underflow.   It is also true for x32 which has
zero_extend instead of ptr_extend.  I have tried different approaches
to avoid transforming

(zero_extend:DI (plus:SI (FOO:SI) (const_int Y)))

to

(plus:DI (zero_extend:DI (FOO:SI)) (const_int Y))

without success.  This patch relaxes the condition to check
POINTERS_EXTEND_UNSIGNED != 0 instead if POINTERS_EXTEND_UNSIGNED < 0
to cover both ptr_extend and zero_extend. We can investigate a better
approach for ptr_extend and zero_extend later.  For now, I believe it
is the saftest way to support ptr_extend and zero_extend.

Any comments?

Thanks.


H.J.
---
gcc/

2011-08-06  H.J. Lu  

PR middle-end/49721
* explow.c (convert_memory_address_addr_space): Also permute the
conversion and addition of constant for zero-extend.

gcc/testsuite/

2011-08-06  H.J. Lu  

PR middle-end/49721
* gfortran.dg/pr49721.f: New.

diff --git a/gcc/explow.c b/gcc/explow.c
index f8262db..ed2f621 100644
--- a/gcc/explow.c
+++ b/gcc/explow.c
@@ -384,18 +384,23 @@ convert_memory_address_addr_space (enum machine_mode 
to_mode ATTRIBUTE_UNUSED,
 
 case PLUS:
 case MULT:
-  /* For addition we can safely permute the conversion and addition
-operation if one operand is a constant and converting the constant
-does not change it or if one operand is a constant and we are
-using a ptr_extend instruction  (POINTERS_EXTEND_UNSIGNED < 0).
+  /* FIXME: For addition, we used to permute the conversion and
+   * addition operation only if one operand is a constant and
+converting the constant does not change it or if one operand
+is a constant and we are using a ptr_extend instruction
+(POINTERS_EXTEND_UNSIGNED < 0) even if the resulting address
+may overflow/underflow.  We relax the condition to include
+zero-extend (POINTERS_EXTEND_UNSIGNED > 0) since the other
+parts of the compiler depend on it.  See PR 49721.
+
 We can always safely permute them if we are making the address
 narrower.  */
   if (GET_MODE_SIZE (to_mode) < GET_MODE_SIZE (from_mode)
  || (GET_CODE (x) == PLUS
  && CONST_INT_P (XEXP (x, 1))
- && (XEXP (x, 1) == convert_memory_address_addr_space
-  (to_mode, XEXP (x, 1), as)
- || POINTERS_EXTEND_UNSIGNED < 0)))
+ && (POINTERS_EXTEND_UNSIGNED != 0
+ || XEXP (x, 1) == convert_memory_address_addr_space
+   (to_mode, XEXP (x, 1), as
return gen_rtx_fmt_ee (GET_CODE (x), to_mode,
   convert_memory_address_addr_space
 (to_mode, XEXP (x, 0), as),
diff --git a/gcc/testsuite/gfortran.dg/pr49721.f 
b/gcc/testsuite/gfortran.dg/pr49721.f
new file mode 100644
index 000..39e2ed7
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr49721.f
@@ -0,0 +1,35 @@
+! PR middle-end/49721
+! { dg-do compile }
+! { dg-options "-O3 -funroll-loops" }
+
+  subroutine midbloc6(c,a2,a2i,q)
+  parameter (ndim2=6)
+  parameter (ndim=3)
+  dimension ri(ndim2),cr(ndim2,ndim2),xj(ndim2,ndim2),q(*)
+ @,sai(ndim2,ndim2),cm(ndim2,ndim2),w(ndim2,ndim2)
+  dimension vr(ndim2,ndim2),vi(ndim2,ndim2),s1(ndim2,ndim2),p(ndim)
+  dimension xq(6),qb(2),qc(2),ifl(6),iplane(3)
+  save
+  call eig66(cr,rr,ri,vr,vi)
+  xq(i)=asin(ri(i))/x2pi
+  i9=6
+  qb(1)=q(1)/x2pi
+do 180 i=1,2
+  do 170 j=1,6
+  120   if(xq(j)) 130,190,140
+  130   if(qb(i)-0.5d0) 160,150,150
+  140   if(qb(i)-0.5d0) 150,150,160
+  150   continue
+tst=abs(abs(qb(i))-abs(xq(j)))
+  160   continue
+  170 continue
+  iplane(i)=k
+  180   continue
+  190   continue
+  n1=iplane(3)
+  if(i9.eq.6) then
+z=vr(1,n1)*vi(2,n1)-vr(2,n1)*vi(1,n1)+vr(3,n1)*vi(4,n1)-vr(4,n1)
+  endif
+  sai(6,i)=vi(i,n1)/z
+  call dacond6(a2,zero)
+  end


Re: Support -mcpu=native on Solaris/SPARC

2011-08-07 Thread Eric Botcazou
> 2011-07-27  Rainer Orth  
>
>   gcc:
>   * config/sparc/driver-sparc.c: New file.
>   * config/sparc/x-sparc: New file.
>   * config.host: Use driver-sparc.o, sparc/x-sparc on
>   sparc*-*-solaris2*.
>   * config/sparc/sparc.opt (native): New value for enum
>   processor_type.
>   * config/sparc/sparc-opts.h (PROCESSOR_NATIVE): Declare.
>   * config/sparc/sparc.c (sparc_option_override): Abort if
>   PROCESSOR_NATIVE gets here.
>   * config/sparc/sol2.h [__sparc__] (host_detect_local_cpu): Declare.
>   (EXTRA_SPEC_FUNCTIONS, MCPU_MTUNE_NATIVE_SPECS,
>   DRIVER_SELF_SPECS): Define.
>   * configure.ac (EXTRA_GCC_LIBS): Check for libkstat.
>   Substitute result.
>   * configure: Regenerate.
>   * Makefile.in (EXTRA_GCC_LIBS): Set.
>   (xgcc$(exeext)): Add $(EXTRA_GCC_LIBS).
>   (cpp$(exeext)): Likewise.

OK if you document the new value in doc/invoke.texi, thanks in advance.

-- 
Eric Botcazou


PATCH: Add -mavx2 and properly check numbers of mask bits

2011-08-07 Thread H.J. Lu
Hi,

opth-gen.awk has

print "#define " mask name " (1 << " masknum[vname]++ ")"

and int has 32bits. We should check

if (masknum[var] > 32)

instead of

if (masknum[var] > 31)

Now, I got

#define OPTION_MASK_ISA_X32 (1 << 30)
#define OPTION_MASK_ISA_XOP (1 << 31)

in options.h.  OK for trunk?

Thanks.


H.J.
---
2011-08-07  H.J. Lu  

* opth-gen.awk: Properly check numbers of mask bits.

* config/i386/i386.opt: Add mavx2.

diff --git a/gcc/config/i386/i386.opt b/gcc/config/i386/i386.opt
index f197dd8..3cfc812 100644
--- a/gcc/config/i386/i386.opt
+++ b/gcc/config/i386/i386.opt
@@ -457,6 +457,10 @@ mavx
 Target Report Mask(ISA_AVX) Var(ix86_isa_flags) Save
 Support MMX, SSE, SSE2, SSE3, SSSE3, SSE4.1, SSE4.2 and AVX built-in functions 
and code generation
 
+mavx2
+Target Report Mask(ISA_AVX2) Var(ix86_isa_flags) Save
+Support MMX, SSE, SSE2, SSE3, SSSE3, SSE4.1, SSE4.2, AVX and AVX2 built-in 
functions and code generation
+
 mfma
 Target Report Mask(ISA_FMA) Var(ix86_isa_flags) Save
 Support MMX, SSE, SSE2, SSE3, SSSE3, SSE4.1, SSE4.2, AVX and FMA built-in 
functions and code generation
diff --git a/gcc/opth-gen.awk b/gcc/opth-gen.awk
index 876e0f6..5d156f5 100644
--- a/gcc/opth-gen.awk
+++ b/gcc/opth-gen.awk
@@ -311,7 +311,7 @@ for (i = 0; i < n_extra_masks; i++) {
 }
 
 for (var in masknum) {
-   if (masknum[var] > 31) {
+   if (masknum[var] > 32) {
if (var == "")
print "#error too many target masks"
else


Re: [Patch, Fortran, OOP] PR 49638: [OOP] length parameter is ignored when overriding type bound character functions with constant length.

2011-08-07 Thread Janus Weil
>> Alright then. How about this: I'll commit the attached verision of the
>> patch (including your suggestions), and we start messing with the
>> return values afterwards? Patch is regtested on
>> x86_64-unknown-linux-gnu. I hope the test case is sufficient for a
>> start.
>
> This is OK.  Thanks for the patch (and for bearing with me ;-)

Well, thanks for the thorough discussion :)

Committed as r177550.


> When extending the values of gfc_dep_compare_expr, we will need to go
> through all its uses (making sure we change == -2 to <= -2).

Right. I will try to take care of that during the coming week (unless
you beat me to it).

Cheers,
Janus


Re: [PATCH] Fix PR49518

2011-08-07 Thread H.J. Lu
On Tue, Jul 5, 2011 at 2:35 AM, Richard Guenther  wrote:
> On Tue, 5 Jul 2011, Ira Rosen wrote:
>
>> Richard Guenther  wrote on 04/07/2011 03:30:59 PM:
>> > >
>> > > Richard Guenther  wrote on 04/07/2011 02:38:50 PM:
>> > >
>> > > > Handling of negative steps broke one of the many asserts in
>> > > > the vectorizer.  The following patch drops one that I can't
>> > > > make sense of.  I think all asserts need comments - especially
>> > > > this one would, as I can't see why using vf is correct to
>> > > > test against and not nelements (and why <= vf and not < vf).
>> > >
>> > > There is an explanation 10 rows above the assert. It doesn't make sense
>> to
>> > > peel more than vf iterations (and not nelements, since for the case of
>> > > multiple types it may help to align more data-refs - see the comment in
>> the
>> > > code). IIRC <= is for the case of aligned access, but I am not sure
>> about
>> > > that, so maybe you are right.
>> > >
>> > > I don't see how it is related to negative steps.
>> > >
>> > > I think that the real reason for this failure is that the loads are
>> > > actually irrelevant (hence, vf=4 that doesn't take char loads into
>> > > account), but we don't check that when we analyze data-refs. So, in my
>> > > opinion, the proper fix will add such check.
>> >
>> > The following also works for me:
>> >
>> > Index: tree-vect-data-refs.c
>> > ===
>> > --- tree-vect-data-refs.c       (revision 175802)
>> > +++ tree-vect-data-refs.c       (working copy)
>> > @@ -1495,6 +1495,9 @@ vect_enhance_data_refs_alignment (loop_v
>> >        stmt = DR_STMT (dr);
>> >        stmt_info = vinfo_for_stmt (stmt);
>> >
>> > +      if (!STMT_VINFO_RELEVANT (stmt_info))
>> > +       continue;
>> > +
>> >        /* For interleaving, only the alignment of the first access
>> >           matters.  */
>> >        if (STMT_VINFO_STRIDED_ACCESS (stmt_info)
>> >
>> > does that look better or do you propose to clean the datarefs
>> > vector from those references?
>>
>> Well, this is certainly enough to fix the PR.
>> I am not sure if we can just remove these data-refs from the dependence
>> checks. After that all the alignment and access checks are at least
>> redundant.
>
> Ok.  The following is what I have tested for this PR and also for
> PR49628.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, ok for trunk?
>
> Thanks,
> Richard.
>
> 2011-07-04  Richard Guenther  
>
>        PR tree-optimization/49518
>        PR tree-optimization/49628
>        * tree-vect-data-refs.c (vect_enhance_data_refs_alignment): Skip
>        irrelevant and invariant data-references.
>        (vect_analyze_data_ref_access): For invariant loads clear the
>        group association.
>
>        * g++.dg/torture/pr49628.C: New testcase.
>        * gcc.dg/torture/pr49518.c: Likewise.
>

This caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50017

-- 
H.J.


Re: PATCH: Add -mavx2 and properly check numbers of mask bits

2011-08-07 Thread Hans-Peter Nilsson
On Sun, 7 Aug 2011, H.J. Lu wrote:
> Hi,
>
> opth-gen.awk has
>
> print "#define " mask name " (1 << " masknum[vname]++ ")"
>
> and int has 32bits. We should check
>
> if (masknum[var] > 32)
>
> instead of
>
> if (masknum[var] > 31)

IIUC the (int32_t) sign-bit is supposed to be reserved and this
is just a case of a missing commentarey and documentation to
that effect. (Use of .opt Var being preferred.)  Otherwise, you
can't e.g. negate that option using the -MASK trick: see the
docs.

brgds, H-P


[google] Backport r174965 from trunk to google/gcc-4_6 (issue4852046)

2011-08-07 Thread Guozhi Wei
Hi

I want to backport r174965 from trunk to google/gcc-4_6, which fixed vect-72.c
failure in target arm, as described in
http://gcc.gnu.org/ml/gcc-patches/2011-06/msg00927.html

Tested with buildit and regression test on arm qemu.

OK for google/gcc-4_6 ?

thanks
Carrot


2011-08-08  Guozhi Wei  

Backport r174965 from trunk.

2011-06-12  Ira Rosen  

* tree-vect-data-refs.c (vect_peeling_hash_get_most_frequent):
Take number of iterations to peel into account for equally 
frequent
misalignment values.


Index: tree-vect-data-refs.c
===
--- tree-vect-data-refs.c   (revision 177320)
+++ tree-vect-data-refs.c   (working copy)
@@ -1250,7 +1250,9 @@ vect_peeling_hash_get_most_frequent (voi
   vect_peel_info elem = (vect_peel_info) *slot;
   vect_peel_extended_info max = (vect_peel_extended_info) data;
 
-  if (elem->count > max->peel_info.count)
+  if (elem->count > max->peel_info.count
+  || (elem->count == max->peel_info.count
+  && max->peel_info.npeel > elem->npeel))
 {
   max->peel_info.npeel = elem->npeel;
   max->peel_info.count = elem->count;


--
This patch is available for review at http://codereview.appspot.com/4852046


[wwwdocs] Announce new ARM/embedded-4_6-branch branch

2011-08-07 Thread Terry Guo
Hi,

This new branch intends to provide fixes and enhancements for GCC 4.6 when
used with ARM embedded cores.

The attached patch adds documentation for this branch.

Is it OK to commit? Thanks.

BR,
Terry

svn-arm-embedded-4_6-branch.patch
Description: Binary data


RE: [PATCH, ARM] Generate conditional compares in Thumb2 state

2011-08-07 Thread Jiangning Liu
In attached new patch, arm_arch_thumb2 is changed to TARGET_THUMB2. I tried
that with my patch command line option -mcpu=armv7-a9 doesn't generate IT
instruction any longer, unless option "-mthumb" is being added.

All of my tests assume command line option -mthumb, while cortex-M0,
cortex-M3 cortex-M4 are covered by options -mcpu=cortex-m0, -march=armv7-m,
and -march=armv7e-m respectively.

As for the extra problem exposed by this specific case, may we treat it as a
separate fix to decouple it with this one, and I can give follow up later
on? I think it is a general problem not only for the particular pattern
it/op/it/op. But I'm not sure how far we can go to optimize this kind of
problems introduced by IT block. For this specific case, I see "if
conversion" already generates conditional move before combination pass. So
basically the peephole rules may probably work for most of the general
scenarios. My initial thought is go over the rules introducing IT block and
try to figure out all of the combination that two of this kinds of rules can
be in sequential order. Maybe we only need to handle the most common case
like this one. Since I do see a bunch of rules have something to do with
problem, I'd like to look into all of them to give a most reasonable
solution in a separate fix. 

Does it make sense?

Thanks,
-Jiangning

> -Original Message-
> From: Ramana Radhakrishnan [mailto:ramana.radhakrish...@linaro.org]
> Sent: Friday, August 05, 2011 9:20 AM
> To: Jiangning Liu
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH, ARM] Generate conditional compares in Thumb2 state
> 
> On 3 August 2011 08:48, Jiangning Liu  wrote:
> > This patch is to generate more conditional compare instructions in
> Thumb2
> > state. Given an example like below,
> >
> > int f(int i, int j)
> > {
> >  if ( (i == '+') || (j == '-') ) {
> >    return i;
> >  } else {
> >    return j;
> >  }
> > }
> >
> > Without the patch, compiler generates the following codes,
> >
> >        sub     r2, r0, #43
> >        rsbs    r3, r2, #0
> >        adc     r3, r3, r2
> >        cmp     r1, #45
> >        it      eq
> >        orreq   r3, r3, #1
> >        cmp     r3, #0
> >        it      eq
> >        moveq   r0, r1
> >        bx      lr
> >
> > With the patch, compiler can generate conditional jump like below,
> >
> >        cmp     r0, #43
> >        it      ne
> >        cmpne   r1, #45
> >        it      ne
> >        movne   r0, r1
> >        bx      lr
> 
> 
> Nice improvement but there could be a single it block to handle both
> and thus you
> could make this even better with
> 
> cmp r0, #43
> itt ne
> cmpne r1 ,#45
> movne r0, r1
> 
> The way to do this would be to try and split this post-reload
> unfortunately into the cmp instruction and the conditional compare
> with the appropriate instruction length - Then the backend has a
> chance of merging some of this into a single instruction.
> Unfortunately that won't be very straight-forward but that's a
> direction we probably ought to proceed with in this case.
> 
> In a number of places:
> 
> > +   if (arm_arch_thumb2)
> 
> Ah instead of this please use if (TARGET_THUMB2) - arm_arch_thumb2 is
> true based on the architecture levels and not necessarily if the user
> wants to generate Thumb code. I don't want an unnecessary IT
> instruction being emitted in the ASM block in ARM state for v7-a and
> above.
> 
> > Tested against arm-none-eabi target and no regression found.
> 
> Presumably for ARM and Thumb2 state ?
> 
> 
> cheers
> Ramana


fix_cond_cmp_2.patch
Description: Binary data