[PATCH] Fix PR50333

[Richard Guenther]

This fixes PR50333, another instance of a extract_ops_from_tree user
that doesn't handle ternaries.

Bootstrap and testing pending on x86_64-unknown-linux-gnu.

Richard.

2011-09-09  Richard Guenther  

PR middle-end/50333
* tree-data-ref.c (split_constant_offset): Do not try to handle
ternary ops.

* gcc.dg/torture/pr50333.c: New testcase.

Index: gcc/tree-data-ref.c
===
--- gcc/tree-data-ref.c (revision 178719)
+++ gcc/tree-data-ref.c (working copy)
@@ -686,7 +686,8 @@ split_constant_offset (tree exp, tree *v
   *off = ssize_int (0);
   STRIP_NOPS (exp);
 
-  if (tree_is_chrec (exp))
+  if (tree_is_chrec (exp)
+  || get_gimple_rhs_class (TREE_CODE (exp)) == GIMPLE_TERNARY_RHS)
 return;
 
   otype = TREE_TYPE (exp);
Index: gcc/testsuite/gcc.dg/torture/pr50333.c
===
--- gcc/testsuite/gcc.dg/torture/pr50333.c  (revision 0)
+++ gcc/testsuite/gcc.dg/torture/pr50333.c  (revision 0)
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+
+static inline void
+unext(unsigned int *_ui, unsigned _len, unsigned int _ui0)
+{
+unsigned j = 1;
+while (++j<_len)
+  ;
+_ui[j-1]=_ui0;
+}
+unsigned int
+ncwrs_urow(unsigned _n, unsigned _k, unsigned int *_u)
+{
+  unsigned k;
+  for(k=2; k<_n; k++)
+unext(_u+1,_k+1,1);
+}

Re: [PATCH, testsuite] Avoid architecture options conflict for case pr42894.c

[Richard Earnshaw]

Ok.

R.



On 25 Aug 2011, at 12:45, "Terry Guo"  wrote:

> Hello,
> 
> I think it is useful to run this case for newer arm targets. So the patch
> intends to skip the warning of architecture conflicts. Is it ok to commit to
> trunk?
> 
> BR,
> Terry
> 
> gcc/testsuite/ChangeLog:
> 
> 2011-08-25  Terry Guo  
> 
>* gcc.dg/tls/pr42894.c: Add dg-prune-output to skip
>architecture conflict.
> 
> 
> diff --git a/gcc/testsuite/gcc.dg/tls/pr42894.c
> b/gcc/testsuite/gcc.dg/tls/pr42894.c
> index c3bd76c..cda6719 100644
> --- a/gcc/testsuite/gcc.dg/tls/pr42894.c
> +++ b/gcc/testsuite/gcc.dg/tls/pr42894.c
> @@ -2,6 +2,7 @@
> /* { dg-do compile } */
> /* { dg-options "-march=armv5te -mthumb" { target arm*-*-* } } */
> /* { dg-require-effective-target tls } */
> +/* { dg-prune-output "switch .* conflicts with" } */
> 
> extern __thread int t;
> 
> 
> 
> 
> 
> 

Re: [Patch, testsuite] fix target/PR49614

[Dominique Dhumieres]

> A function not declared inline with an always_inline attribute is a bug.

If the test case is buggy, why is it not spotted on non ppc platforms?

Dominique

Re: [Patch, testsuite] fix target/PR49614

[Richard Guenther]

On Fri, Sep 9, 2011 at 11:03 AM, Dominique Dhumieres  wrote:
>> A function not declared inline with an always_inline attribute is a bug.
>
> If the test case is buggy, why is it not spotted on non ppc platforms?

because it's not run on anything besides ppc

> Dominique
>

Re: [RFC] Cleanup DW_CFA_GNU_args_size handling

[Eric Botcazou]

> Which version of binutils are you using, Eric?

Binutils CVS from this morning:

Target: i586-suse-linux
Configured 
with: /home/eric/svn/gcc/configure --build=i586-suse-linux 
--prefix=/home/eric/install/gcc 
--with-as=/home/eric/build/binutils/native32/gas/as-new 
--with-ld=/home/eric/build/binutils/native32/ld/ld-new 
--enable-languages=c,c++,ada --enable-checking=yes,rtl --enable-__cxa_atexit 
--disable-nls --disable-libmudflap --disable-initfini-array
Thread model: posix
gcc version 4.7.0 20110909 (experimental) [trunk revision 178719] (GCC)

eric@atlantis:~/build/gcc/native32> 
/home/eric/build/binutils/native32/gas/as-new --version
GNU assembler (GNU Binutils) 2.21.53.20110909
Copyright 2011 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or later.
This program has absolutely no warranty.
This assembler was configured for a target of `i586-suse-linux'.

eric@atlantis:~/build/gcc/native32> 
/home/eric/build/binutils/native32/ld/ld-new --version
GNU ld (GNU Binutils) 2.21.53.20110909
Copyright 2011 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or (at your option) a later version.
This program has absolutely no warranty.

Comparing stages 2 and 3
warning: gcc/cc1-checksum.o differs
warning: gcc/cc1plus-checksum.o differs
Bootstrap comparison failure!
gcc/ada/sprint.o differs
libiberty/pic/regex.o differs
libiberty/pic/cplus-dem.o differs
libiberty/pic/alloca.o differs
libiberty/pic/argv.o differs
libiberty/pic/crc32.o differs
libiberty/pic/fdmatch.o differs
libiberty/pic/floatformat.o differs
libiberty/pic/hashtab.o differs
libiberty/pic/pex-common.o differs
libiberty/pic/simple-object-coff.o differs
make[2]: *** [compare] Error 1

For libiberty/pic/argv.o, the difference lies within the .eh_frame section.

-- 
Eric Botcazou

Re: [Patch, testsuite] fix target/PR49614

[Andreas Schwab]

domi...@lps.ens.fr (Dominique Dhumieres) writes:

> If the test case is buggy, why is it not spotted on non ppc platforms?

Because it's not run on other platforms.

Andreas.

-- 
Andreas Schwab, sch...@redhat.com
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84  5EC7 45C6 250E 6F00 984E
"And now for something completely different."

[PATCH] Fix swap_tree_operands

[Richard Guenther]

swap_tree_operands does not handle swapping operands in
a_1 = x_2 + 1; very well as it keeps the use operand for
x_2 pointing to its old slot and thus 1 after it finished.
That's of course bad as no user of swap_tree_operands feels
the need to update the statement - swap_tree_operands is
after all exactly to avoid updating the statement (and
eventually disrupting the operand list).

Fixed, thus.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

Richard.

2011-09-09  Richard Guenther  

* tree-ssa-operands.c (swap_tree_operands): Always adjust
existing operand positions.

Index: gcc/tree-ssa-operands.c
===
--- gcc/tree-ssa-operands.c (revision 178719)
+++ gcc/tree-ssa-operands.c (working copy)
@@ -1149,7 +1149,8 @@ swap_tree_operands (gimple stmt, tree *e
 
   /* If the operand cache is active, attempt to preserve the relative
  positions of these two operands in their respective immediate use
- lists.  */
+ lists by adjusting their use pointer to point to the new
+ operand position.  */
   if (ssa_operands_active () && op0 != op1)
 {
   use_optype_p use0, use1, ptr;
@@ -1170,14 +1171,12 @@ swap_tree_operands (gimple stmt, tree *e
break;
  }
 
-  /* If both uses don't have operand entries, there isn't much we can do
- at this point.  Presumably we don't need to worry about it.  */
-  if (use0 && use1)
-{
- tree *tmp = USE_OP_PTR (use1)->use;
- USE_OP_PTR (use1)->use = USE_OP_PTR (use0)->use;
- USE_OP_PTR (use0)->use = tmp;
-   }
+  /* And adjust their location to point to the new position of the
+ operand.  */
+  if (use0)
+   USE_OP_PTR (use0)->use = exp1;
+  if (use1)
+   USE_OP_PTR (use1)->use = exp0;
 }
 
   /* Now swap the data.  */

[Patch, testsuite, arm] Skip the arch conflict to enable case neon-thumb2-move pass on more targets.

[Terry Guo]

Hello,

This patch enables the case pass on targets other than armv7-a by skipping
the architecture conflict message. Is it OK to trunk?

BR,
Terry

2011-09-09  Terry Guo  

  * gcc.target/arm/neon-thumb2-move.c: Skip the
architecture conflict to enable the case pass
on targets other than armv7-a.

diff --git a/gcc/testsuite/gcc.target/arm/neon-thumb2-move.c
b/gcc/testsuite/gcc.target/arm/neon-thumb2-move.c
index 430a4d5..9cf86dd 100644
--- a/gcc/testsuite/gcc.target/arm/neon-thumb2-move.c
+++ b/gcc/testsuite/gcc.target/arm/neon-thumb2-move.c
@@ -3,6 +3,7 @@
 /* { dg-require-effective-target arm_thumb2_ok } */
 /* { dg-options "-O2 -mthumb -march=armv7-a" } */
 /* { dg-add-options arm_neon } */
+/* { dg-prune-output "switch .* conflicts with" } */

 #include 
 #include 

[PATCH] Adjust fold_stmt_inplace interface

[Richard Guenther]

This adjusts fold_stmt_inplace to take a gimple_stmt_iterator argument
instead of a gimple.  This allows it to operate on statements that
are only in a gimple_seq but are not (yet) associated with a basic block.
fold_stmt_* shouldn't require a CFG (and yep, I'm going to need that
in a followup).

This exposes that fold_stmt_inplace used gsi_for_stmt which is an
O(n) operation to its callers, quite a few already having a stmt
iterator handy.

Bootstrap and regtest pending on x86_64-unknown-linux-gnu.

Richard.

2011-09-09  Richard Guenther  

* gimple.h (fold_stmt_inplace): Adjust to take a gimple_stmt_iterator
instead of a statement.
* gimple-fold.c (fold_stmt_inplace): Likewise.
* sese.c (graphite_copy_stmts_from_block): Adjust.
* tree-ssa-dom.c (propagate_rhs_into_lhs): Likewise.
* tree-ssa-forwprop.c (forward_propagate_into_comparison): Use
fold_stmt.
(forward_propagate_addr_into_variable_array_index): Likewise.
(forward_propagate_addr_expr_1): adjust.
(associate_plusminus): Likewise.
(ssa_forward_propagate_and_combine): Likewise.
* tree-ssa-mathopts.c (replace_reciprocal): Adjust.
(execute_cse_reciprocals): Likewise.
* tree-ssa.c (insert_debug_temp_for_var_def): Adjust.

Index: gcc/gimple.h
===
*** gcc/gimple.h(revision 178719)
--- gcc/gimple.h(working copy)
*** extern void dump_gimple_statistics (void
*** 5068,5074 
  void gimplify_and_update_call_from_tree (gimple_stmt_iterator *, tree);
  tree gimple_fold_builtin (gimple);
  bool fold_stmt (gimple_stmt_iterator *);
! bool fold_stmt_inplace (gimple);
  tree get_symbol_constant_value (tree);
  tree canonicalize_constructor_val (tree);
  extern tree maybe_fold_and_comparisons (enum tree_code, tree, tree, 
--- 5068,5074 
  void gimplify_and_update_call_from_tree (gimple_stmt_iterator *, tree);
  tree gimple_fold_builtin (gimple);
  bool fold_stmt (gimple_stmt_iterator *);
! bool fold_stmt_inplace (gimple_stmt_iterator *);
  tree get_symbol_constant_value (tree);
  tree canonicalize_constructor_val (tree);
  extern tree maybe_fold_and_comparisons (enum tree_code, tree, tree, 
Index: gcc/gimple-fold.c
===
*** gcc/gimple-fold.c   (revision 178719)
--- gcc/gimple-fold.c   (working copy)
*** fold_stmt (gimple_stmt_iterator *gsi)
*** 1286,1305 
return fold_stmt_1 (gsi, false);
  }
  
! /* Perform the minimal folding on statement STMT.  Only operations like
 *&x created by constant propagation are handled.  The statement cannot
 be replaced with a new one.  Return true if the statement was
 changed, false otherwise.
!The statement STMT should be in valid gimple form but may
 be in unfolded state as resulting from for example constant propagation
 which can produce *&x = 0.  */
  
  bool
! fold_stmt_inplace (gimple stmt)
  {
!   gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
!   bool changed = fold_stmt_1 (&gsi, true);
!   gcc_assert (gsi_stmt (gsi) == stmt);
return changed;
  }
  
--- 1286,1305 
return fold_stmt_1 (gsi, false);
  }
  
! /* Perform the minimal folding on statement *GSI.  Only operations like
 *&x created by constant propagation are handled.  The statement cannot
 be replaced with a new one.  Return true if the statement was
 changed, false otherwise.
!The statement *GSI should be in valid gimple form but may
 be in unfolded state as resulting from for example constant propagation
 which can produce *&x = 0.  */
  
  bool
! fold_stmt_inplace (gimple_stmt_iterator *gsi)
  {
!   gimple stmt = gsi_stmt (*gsi);
!   bool changed = fold_stmt_1 (gsi, true);
!   gcc_assert (gsi_stmt (*gsi) == stmt);
return changed;
  }
  
Index: gcc/sese.c
===
*** gcc/sese.c  (revision 178719)
--- gcc/sese.c  (working copy)
*** graphite_copy_stmts_from_block (basic_bl
*** 620,626 
  
if (rename_uses (copy, rename_map, &gsi_tgt, region, loop, iv_map,
   gloog_error))
!   fold_stmt_inplace (copy);
  
update_stmt (copy);
  }
--- 620,629 
  
if (rename_uses (copy, rename_map, &gsi_tgt, region, loop, iv_map,
   gloog_error))
!   {
! gcc_assert (gsi_stmt (gsi_tgt) == copy);
! fold_stmt_inplace (&gsi_tgt);
!   }
  
update_stmt (copy);
  }
Index: gcc/tree-ssa-dom.c
===
*** gcc/tree-ssa-dom.c  (revision 178719)
--- gcc/tree-ssa-dom.c  (working copy)
*** propagate_rhs_into_lhs (gimple stmt, tre
*** 2656,2662 
   GIMPLE_ASSIGN, and there is no way to effect such a
   transformation in-place.  We might want to consider
   us

Re: [Patch, testsuite, arm] Skip the arch conflict to enable case neon-thumb2-move pass on more targets.

[Richard Guenther]

On Fri, Sep 9, 2011 at 12:00 PM, Terry Guo  wrote:
> Hello,
>
> This patch enables the case pass on targets other than armv7-a by skipping
> the architecture conflict message. Is it OK to trunk?

I don't like these kind of patches.  Why do the testcases have -march=armv7-a
enabled in the first place?  If they really need it then they shouldn't be
run with another -march and thus instead there should be a
dg-requires-effective-target armv7-a.  If they don't really need it
they shouldn't
add it.

Richard.

> BR,
> Terry
>
> 2011-09-09  Terry Guo  
>
>          * gcc.target/arm/neon-thumb2-move.c: Skip the
>        architecture conflict to enable the case pass
>        on targets other than armv7-a.
>
> diff --git a/gcc/testsuite/gcc.target/arm/neon-thumb2-move.c
> b/gcc/testsuite/gcc.target/arm/neon-thumb2-move.c
> index 430a4d5..9cf86dd 100644
> --- a/gcc/testsuite/gcc.target/arm/neon-thumb2-move.c
> +++ b/gcc/testsuite/gcc.target/arm/neon-thumb2-move.c
> @@ -3,6 +3,7 @@
>  /* { dg-require-effective-target arm_thumb2_ok } */
>  /* { dg-options "-O2 -mthumb -march=armv7-a" } */
>  /* { dg-add-options arm_neon } */
> +/* { dg-prune-output "switch .* conflicts with" } */
>
>  #include 
>  #include 
>
>
>
>
>
>

[PATCH] Fold PRE inserted statements

[Richard Guenther]

This makes us fold (the last) inserted statement from PRE to avoid
MEM_REF vs. non-MEM_REF inconsistencies tripping up on un-aware
code such as data dependence analysis using operand_equal_p.  fold_stmt
makes sure to canonicalize either variant but PRE internally uses
MEM_REF for all accesses to be type agnostic.

Bootstrap and regtest pending on x86_64-unknown-linux-gnu.

Richard.

2011-09-09  Richard Guenther  

* tree-ssa-pre.c (create_expression_by_pieces): Fold the
last statement.

Index: gcc/tree-ssa-pre.c
===
--- gcc/tree-ssa-pre.c  (revision 178719)
+++ gcc/tree-ssa-pre.c  (working copy)
@@ -3182,6 +3182,10 @@ create_expression_by_pieces (basic_block
   /* All the symbols in NEWEXPR should be put into SSA form.  */
   mark_symbols_for_renaming (newstmt);
 
+  /* Fold the last statement.  */
+  gsi = gsi_last (*stmts);
+  fold_stmt_inplace (&gsi);
+
   /* Add a value number to the temporary.
  The value may already exist in either NEW_SETS, or AVAIL_OUT, because
  we are creating the expression by pieces, and this particular piece of

[PATCH] Fix PR50328

[Richard Guenther]

This fixes our inability to handle reductions with a constant
or a parameter.

Bootstrapped and tested on x86_64-unknown-linux-gnu, the testcase
requires all the previously posted patches from today to not ICE
and pass though.

Richard.

2011-09-09  Richard Guenther  

PR tree-optimization/50328
* tree-vect-loop.c (vect_is_simple_reduction_1): Allow one
constant or default-def operand.

* gcc.dg/vect/fast-math-vect-outer-7.c: New testcase.

Index: gcc/tree-vect-loop.c
===
*** gcc/tree-vect-loop.c(revision 178719)
--- gcc/tree-vect-loop.c(working copy)
*** vect_is_simple_reduction_1 (loop_vec_inf
*** 2149,2155 
op1 = gimple_assign_rhs1 (def_stmt);
op2 = gimple_assign_rhs2 (def_stmt);
  
!   if (TREE_CODE (op1) != SSA_NAME || TREE_CODE (op2) != SSA_NAME)
  {
if (vect_print_dump_info (REPORT_DETAILS))
report_vect_op (def_stmt, "reduction: uses not ssa_names: ");
--- 2149,2155 
op1 = gimple_assign_rhs1 (def_stmt);
op2 = gimple_assign_rhs2 (def_stmt);
  
!   if (TREE_CODE (op1) != SSA_NAME && TREE_CODE (op2) != SSA_NAME)
  {
if (vect_print_dump_info (REPORT_DETAILS))
report_vect_op (def_stmt, "reduction: uses not ssa_names: ");
*** vect_is_simple_reduction_1 (loop_vec_inf
*** 2255,2261 
  def2 = SSA_NAME_DEF_STMT (op2);
  
if (code != COND_EXPR
!   && (!def1 || !def2 || gimple_nop_p (def1) || gimple_nop_p (def2)))
  {
if (vect_print_dump_info (REPORT_DETAILS))
report_vect_op (def_stmt, "reduction: no defs for operands: ");
--- 2255,2261 
  def2 = SSA_NAME_DEF_STMT (op2);
  
if (code != COND_EXPR
!   && ((!def1 || gimple_nop_p (def1)) && (!def2 || gimple_nop_p (def2
  {
if (vect_print_dump_info (REPORT_DETAILS))
report_vect_op (def_stmt, "reduction: no defs for operands: ");
*** vect_is_simple_reduction_1 (loop_vec_inf
*** 2268,2273 
--- 2268,2274 
  
if (def2 && def2 == phi
&& (code == COND_EXPR
+ || !def1 || gimple_nop_p (def1)
|| (def1 && flow_bb_inside_loop_p (loop, gimple_bb (def1))
&& (is_gimple_assign (def1)
  || is_gimple_call (def1)
*** vect_is_simple_reduction_1 (loop_vec_inf
*** 2285,2290 
--- 2286,2292 
  
if (def1 && def1 == phi
&& (code == COND_EXPR
+ || !def2 || gimple_nop_p (def2)
|| (def2 && flow_bb_inside_loop_p (loop, gimple_bb (def2))
  && (is_gimple_assign (def2)
  || is_gimple_call (def2)
Index: gcc/testsuite/gcc.dg/vect/fast-math-vect-outer-7.c
===
*** gcc/testsuite/gcc.dg/vect/fast-math-vect-outer-7.c  (revision 0)
--- gcc/testsuite/gcc.dg/vect/fast-math-vect-outer-7.c  (revision 0)
***
*** 0 
--- 1,23 
+ /* { dg-do compile } */
+ /* { dg-require-effective-target vect_float } */
+ 
+ float dvec[256];
+ 
+ void test1 (float x)
+ {
+   long i, j;
+   for (i = 0; i < 256; ++i)
+ for (j = 0; j < 131072; ++j)
+   dvec[i] *= x;
+ }
+ 
+ void test2 (float x)
+ {
+   long i, j;
+   for (i = 0; i < 256; ++i)
+ for (j = 0; j < 131072; ++j)
+   dvec[i] *= 1.001f;
+ }
+ 
+ /* { dg-final { scan-tree-dump-times "OUTER LOOP VECTORIZED" 2 "vect" } } */
+ /* { dg-final { cleanup-tree-dump "vect" } } */

Re: RFA: MN10300: Fix splitting AND insns

[Nick Clifton]

Hi Jeff,


This is fine,


Thanks - committed.


after adding a function comment for mn10300_split_and_operand_count;


I added this comment:

/* This function is used to help split:

 (set (reg) (and (reg) (int)))

   into:

 (set (reg) (shift (reg) (int))
 (set (reg) (shift (reg) (int))

   where the shitfs will be shorter than the "and" insn.

   It returns the number of bits that should be shifted.  A positive
   values means that the low bits are to be cleared (and hence the
   shifts should be right followed by left) whereas a negative value
   means that the high bits are to be cleared (left followed by right).
   Zero is returned when it would not be economical to split the AND.  */


Cheers
  Nick

Re: [PATCH][ARM] -m{cpu,tune,arch}=native

[Richard Earnshaw]

On 06/09/11 14:35, Andrew Stubbs wrote:
> This update adds many more "magic numbers" for various ARM CPUs, and 
> also ensures that the implementer is ARM (as opposed to Marvell, etc.). 
> The list is far from comprehensive, but it should cover many (but by no 
> means all) of the cores in current use and it would not be hard to add 
> support for other implementers and CPU names in future.
> 
> It has been suggested that this patch should use auxv rather than 
> /proc/cpuinfo. Does anybody here have any insight/preferences?
> 
> Is the patch OK?
> 
> Andrew
> 
> 
> tune-native.patch
> 
> 
> 2011-08-27  Andrew Stubbs  
> 
>   gcc/
>   * config.host (arm*-*-linux*): Add driver-arm.o and x-arm.
>   * config/arm/arm.opt: Add 'native' processor_type and
>   arm_arch enum values.
>   * config/arm/arm.h (host_detect_local_cpu): New prototype.
>   (EXTRA_SPEC_FUNCTIONS): New define.
>   (MCPU_MTUNE_NATIVE_SPECS): New define.
>   (DRIVER_SELF_SPECS): New define.
>   * config/arm/driver-arm.c: New file.
>   * config/arm/x-arm: New file.
>   * doc/invoke.texi (ARM Options): Document -mcpu=native,
>   -mtune=native and -march=native.
> 

The part number field is meaningless outside of the context of a a
specific vendor -- only taken as a pair can they refer to a specific
part.  So why is the vendor field hard-coded rather than factored into
the table of parts.

Maybe it would be better to have a table of tables, with the top-level
table being indexed by vendor id.  Something like

struct vendor_cpu {
  const char *part_no;
  const char *arch_name;
  const char *cpu_name;
};

struct all_cpus {
  const char *vendor_no;
  const struct vendor_cpu *vendor_parts;
}

struct vendor_cpu vendor_arm[] = { ... }

Now your code will allow easy addition of third-party cores.

R.

> --- a/gcc/config.host
> +++ b/gcc/config.host
> @@ -100,6 +100,14 @@ case ${host} in
>  esac
>  
>  case ${host} in
> +  arm*-*-linux*)
> +case ${target} in
> +  arm*-*-*)
> + host_extra_gcc_objs="driver-arm.o"
> + host_xmake_file="${host_xmake_file} arm/x-arm"
> + ;;
> +esac
> +;;
>alpha*-*-linux* | alpha*-dec-osf*)
>  case ${target} in
>alpha*-*-linux* | alpha*-dec-osf*)
> --- a/gcc/config/arm/arm.h
> +++ b/gcc/config/arm/arm.h
> @@ -2223,4 +2223,21 @@ extern int making_const_table;
> instruction.  */
>  #define MAX_LDM_STM_OPS 4
>  
> +/* -mcpu=native handling only makes sense with compiler running on
> +   an ARM chip.  */
> +#if defined(__arm__)
> +extern const char *host_detect_local_cpu (int argc, const char **argv);
> +# define EXTRA_SPEC_FUNCTIONS
> \
> +  { "local_cpu_detect", host_detect_local_cpu },
> +
> +# define MCPU_MTUNE_NATIVE_SPECS \
> +   " %{march=native:% \
> +   " %{mcpu=native:% +   " %{mtune=native:% +#else
> +# define MCPU_MTUNE_NATIVE_SPECS ""
> +#endif
> +
> +#define DRIVER_SELF_SPECS MCPU_MTUNE_NATIVE_SPECS
> +
>  #endif /* ! GCC_ARM_H */
> --- a/gcc/config/arm/arm.opt
> +++ b/gcc/config/arm/arm.opt
> @@ -80,6 +80,11 @@ march=
>  Target RejectNegative Joined Enum(arm_arch) Var(arm_arch_option)
>  Specify the name of the target architecture
>  
> +; Other arm_arch values are loaded from arm-tables.opt
> +; but that is a generated file and this is an odd-one-out.
> +EnumValue
> +Enum(arm_arch) String(native) Value(-1) DriverOnly
> +
>  marm
>  Target Report RejectNegative InverseMask(THUMB)
>  Generate code in 32 bit ARM state.
> @@ -233,6 +238,11 @@ mtune=
>  Target RejectNegative Joined Enum(processor_type) Var(arm_tune_option) 
> Init(arm_none)
>  Tune code for the given processor
>  
> +; Other processor_type values are loaded from arm-tables.opt
> +; but that is a generated file and this is an odd-one-out.
> +EnumValue
> +Enum(processor_type) String(native) Value(-1) DriverOnly
> +
>  mwords-little-endian
>  Target Report RejectNegative Mask(LITTLE_WORDS)
>  Assume big endian bytes, little endian words.  This option is deprecated.
> --- /dev/null
> +++ b/gcc/config/arm/driver-arm.c
> @@ -0,0 +1,108 @@
> +/* Subroutines for the gcc driver.
> +   Copyright (C) 2011 Free Software Foundation, Inc.
> +
> +This file is part of GCC.
> +
> +GCC 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.
> +
> +GCC 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 GCC; see the file COPYING3.  If not see
> +.  */
> +
> +#include "config.h"
> +#include "system.h"
> +#include "coretypes.h"
> +#include "tm.h"

Re: Add unwind information to mips epilogues

[Bernd Schmidt]

On 09/08/11 16:08, Richard Sandiford wrote:
> I suppose I still don't get why this is OK but this:
> 
>> @@ -10324,12 +10350,26 @@ mips_expand_epilogue (bool sibcall_p)
>>if (!TARGET_MIPS16)
>>  target = stack_pointer_rtx;
>>  
>> -  emit_insn (gen_add3_insn (target, base, adjust));
>> +  insn = emit_insn (gen_add3_insn (target, base, adjust));
>> +  if (!frame_pointer_needed && target == stack_pointer_rtx)
>> +{
>> +  RTX_FRAME_RELATED_P (insn) = 1;
>> +  add_reg_note (insn, REG_CFA_DEF_CFA,
>> +plus_constant (stack_pointer_rtx, step2));
>> +}
> 
> triggered ICEs without the !frame_pointer_required.  I think I need
> to play around with it a bit before I understand enough to review.
> I'll try to find time this weekend.

I've rebuilt and tested it again (shrink-wrapping included, not sure if
it makes a difference). What seems to happen is that we have

(insn/f 60 59 61 2 (set (reg/f:SI 30 $fp) (reg/f:SI 29 $sp))

(note 61 60 8 2 NOTE_INSN_PROLOGUE_END)

(jump_insn 8 61 49 2 (set (pc)
(if_then_else (eq (reg/v:SI 4 $4 [orig:196 b ] [196])
(reg:SI 2 $2 [198]))
(label_ref:SI 29) (pc)))

[...]

(jump_insn 10 9 48 3 (set (pc)
(if_then_else (eq (reg/v:SI 4 $4 [orig:196 b ] [196])
(reg:SI 2 $2 [199]))
(label_ref:SI 29) (pc)))

(note 48 10 62 4 [bb 4] NOTE_INSN_BASIC_BLOCK)

(note 62 48 63 4 NOTE_INSN_EPILOGUE_BEG)

(insn/f 63 62 64 4 (set (reg/f:SI 29 $sp) (reg/f:SI 30 $fp))
 (REG_CFA_DEF_CFA (plus:SI (reg/f:SI 29 $sp) (const_int 8))

and reorg.c manages to hoist insn 63 into a delay slot for jump_insn 10.
Presumably it saw insn 60 somehow? In any case, we can now arrive at
label 29 with either FP or SP as CFA reg.

This is gcc.dg/pr49994-1.c.


Bernd

Re: New SPU failures (Re: [PATCH, PR 49923] Check for misaligned accesses before doing SRA)

[Martin Jambor]

Hi,

On Wed, Aug 10, 2011 at 04:29:40PM +0200, Richard Guenther wrote:
> On Wed, 10 Aug 2011, Ulrich Weigand wrote:
> 

...

> > 
> > FAIL: gcc.dg/ipa/ipa-sra-2.c scan-tree-dump eipa_sra "About to replace expr 
> > cow_.*D.->red with \\*ISRA"
> > FAIL: gcc.dg/ipa/ipa-sra-2.c scan-tree-dump eipa_sra "About to replace expr 
> > cow_.*D.->green with ISRA"
> > FAIL: gcc.dg/ipa/ipa-sra-2.c scan-tree-dump eipa_sra "About to replace expr 
> > calf_.*D.->red with \\*ISRA"
> > FAIL: gcc.dg/ipa/ipa-sra-2.c scan-tree-dump eipa_sra "About to replace expr 
> > calf_.*D.->green with ISRA"
> > FAIL: gcc.dg/ipa/ipa-sra-6.c scan-tree-dump-times eipa_sra "foo " 1

This has recvently been reported as PR 50052.

...

> > 
> > I must admin I continue to be confused about exactly what it is that
> > tree_non_mode_aligned_mem_p is supposed to be testing for.  We have:
> > 
> >   if (TREE_CODE (exp) == SSA_NAME
> >   || TREE_CODE (exp) == MEM_REF
> >   || mode == BLKmode
> >   || is_gimple_min_invariant (exp)
> >   || !STRICT_ALIGNMENT)
> > return false;
> > 
> > So if we passed in the plain MEM_REF, this would be considered no problem.
> > The COMPONENT_REF does not add any additional misalignment, so one would
> > hope that this also shouldn't be a problem.
> > 
> > However, just because there *is* a COMPONENT_REF around it, we suddenly
> > realize the fact that don't have points-to information for the MEM_REF
> > and therefore consider *it* (and consequently the whole COMPONENT_REF)
> > to be potentially misaligned ...
> 
> Yep.  That's because we are totally confused about alignment :/
> 
> Martin, what we need to do is get expands idea of what alignment
> it woudl assume for a handled_component_ref and compare that
> to what it would say if we re-materialize the mem as a MEM_REF.
> 
> Unfortunately there isn't a function that you can use to mimic
> expands behavior (that of the normal_inner_ref: case), the closest
> would be to look for TYPE_PACKED or TYPE_ALIGN in addition to
> what get_object_alignment gives us.
> 
> Thus something like
> 
>   align = get_object_alignment (exp);
>   if (!TYPE_PACKED (TREE_TYPE (exp))
>   && (TREE_CODE (exp) != COMPONENT_REF
>   || !DECL_PACKED (TREE_OPERAND (exp, 1
> align = MAX (TYPE_ALIGN (TREE_TYPE (exp)), align);
>   if (GET_MODE_ALIGNMENT (mode) > align)
> return true;
> 

This fixed the failing testcase ipa-sra-2.c but it creates a
misaligned access for the testcase below so I changed it to:

  align = get_object_alignment (exp);
  if (!TYPE_PACKED (TREE_TYPE (exp)))
{
  bool encountered_packed = false;
  tree t = exp;

  while (TREE_CODE (t) == COMPONENT_REF)
if (DECL_PACKED (TREE_OPERAND (t, 1)))
  {
encountered_packed = true;
break;
  }
else
  t = TREE_OPERAND (t, 0);

  if (!encountered_packed)
align = MAX (TYPE_ALIGN (TREE_TYPE (exp)), align);
}
  if (GET_MODE_ALIGNMENT (mode) > align)
return true;

I'm currently bootstrapping this on a sparc64 on the compile farm.  Is
the patch OK if it passes (it has passed bootstrap and testing on
x86_64-linux but that is no surprise)?

Thanks,

Martin


2011-09-08  Martin Jambor  

PR tree-optimization/50052
* tree-sra.c (tree_non_mode_aligned_mem_p): Look at TYPE_ALIGN if
there is no packed field decl in exp.

* gcc.dg/tree-ssa/pr49094-2.c: New test.

Index: src/gcc/testsuite/gcc.dg/tree-ssa/pr49094-2.c
===
--- /dev/null
+++ src/gcc/testsuite/gcc.dg/tree-ssa/pr49094-2.c
@@ -0,0 +1,44 @@
+/* { dg-do run } */
+/* { dg-options "-O" } */
+
+struct in_addr {
+  unsigned int s_addr;
+};
+
+struct ip {
+  struct in_addr ip_src,ip_dst;
+  unsigned char ip_p;
+  unsigned short ip_sum;
+};
+
+struct ip2 {
+  unsigned char blah;
+  unsigned short sheep;
+  struct ip ip;
+} __attribute__ ((aligned(1), packed));
+
+struct ip2 ip_fw_fwd_addr;
+
+int test_alignment( char *m )
+{
+  struct ip2 *ip = (struct ip2 *) m;
+  struct in_addr pkt_dst;
+  pkt_dst = ip->ip.ip_dst ;
+  if( pkt_dst.s_addr == 0 )
+return 1;
+  else
+return 0;
+}
+
+int __attribute__ ((noinline, noclone))
+intermediary (char *p)
+{
+  return test_alignment (p);
+}
+
+int
+main (int argc, char *argv[])
+{
+  ip_fw_fwd_addr.ip.ip_dst.s_addr = 1;
+  return intermediary ((void *) &ip_fw_fwd_addr);
+}
Index: src/gcc/tree-sra.c
===
--- src.orig/gcc/tree-sra.c
+++ src/gcc/tree-sra.c
@@ -1086,6 +1086,23 @@ tree_non_mode_aligned_mem_p (tree exp)
 return false;
 
   align = get_object_alignment (exp);
+  if (!TYPE_PACKED (TREE_TYPE (exp)))
+{
+  bool encountered_packed = false;
+  tree t = exp;
+
+  while (TREE_CODE (t) == COMPONENT_REF)
+if (DECL_PACKED (TREE_OPERAND (t, 1)))
+  {
+encountered_packed = true;
+break;
+  }
+

Re: [Patch, testsuite, arm] Skip the arch conflict to enable case neon-thumb2-move pass on more targets.

[Richard Earnshaw]

On 09/09/11 11:07, Richard Guenther wrote:
> On Fri, Sep 9, 2011 at 12:00 PM, Terry Guo  wrote:
>> Hello,
>>
>> This patch enables the case pass on targets other than armv7-a by skipping
>> the architecture conflict message. Is it OK to trunk?
> 
> I don't like these kind of patches.  Why do the testcases have -march=armv7-a
> enabled in the first place?  If they really need it then they shouldn't be
> run with another -march and thus instead there should be a
> dg-requires-effective-target armv7-a.  If they don't really need it
> they shouldn't
> add it.
> 

I'm not particularly keen on them either, but with the current structure
of the testsuite framework the only way to test some unusual option
combinations is to force them (we can't test every option set, or even
every CPU supported via multilib testing as it would take ~forever to
run through all the variants).  So the only alternative is to force some
tests to run with specific options.

It might be easier if there were a specific part of the testsuite which
was designated to be non-executable, non-multilib.  That part would then
be used without all the multilib paraphernalia that leads to these hacks
(and would only ever be run once as well, regardless of the number of
multilibs being tested).  But until such time, this patch seems like the
only way forward.

R.

> Richard.
> 
>> BR,
>> Terry
>>
>> 2011-09-09  Terry Guo  
>>
>>  * gcc.target/arm/neon-thumb2-move.c: Skip the
>>architecture conflict to enable the case pass
>>on targets other than armv7-a.
>>
>> diff --git a/gcc/testsuite/gcc.target/arm/neon-thumb2-move.c
>> b/gcc/testsuite/gcc.target/arm/neon-thumb2-move.c
>> index 430a4d5..9cf86dd 100644
>> --- a/gcc/testsuite/gcc.target/arm/neon-thumb2-move.c
>> +++ b/gcc/testsuite/gcc.target/arm/neon-thumb2-move.c
>> @@ -3,6 +3,7 @@
>>  /* { dg-require-effective-target arm_thumb2_ok } */
>>  /* { dg-options "-O2 -mthumb -march=armv7-a" } */
>>  /* { dg-add-options arm_neon } */
>> +/* { dg-prune-output "switch .* conflicts with" } */
>>
>>  #include 
>>  #include 
>>
>>
>>
>>
>>
>>
> 

[ARM] Loosen MODES_TIEABLE_P

[Richard Sandiford]

ARM's MODES_TIEABLE_P only allows classes of the same mode to be tied:

#define MODES_TIEABLE_P(MODE1, MODE2)  \
  (GET_MODE_CLASS (MODE1) == GET_MODE_CLASS (MODE2))

But for NEON, we'd like structure modes to be tied to their vector
elements.  In particular, a vector subreg of a structure register
should be considered "cheap", rather than as something that is likely
to need an intermediate.

The current definition made sense before my patch to redefine
CLASS_CANNOT_CHANGE_MODE:

http://gcc.gnu.org/ml/gcc-patches/2011-03/msg01631.html

so I think this is really a missing piece from that patch.  I haven't
measured any direct benefit from this patch alone, because no pass
propogates the sources of subreg moves, even if the modes are tieable.
But with a patch to do that too, I see significant improvements for
several vectorised loops.

Tested on arm-linux-gnueabi.  OK to install?

Richard


gcc/
* config/arm/arm-protos.h (arm_modes_tieable_p): Declare.
* config/arm/arm.h (MODES_TIEABLE_P): Use it.
* config/arm/arm.c (arm_modes_tieable_p): New function.  Allow
NEON vector and structure modes to be tied.

Index: gcc/config/arm/arm-protos.h
===
--- gcc/config/arm/arm-protos.h 2011-09-08 10:31:01.455663735 +0100
+++ gcc/config/arm/arm-protos.h 2011-09-09 13:46:32.797939324 +0100
@@ -46,6 +46,7 @@ extern void arm_output_fn_unwind (FILE *
 extern bool arm_vector_mode_supported_p (enum machine_mode);
 extern bool arm_small_register_classes_for_mode_p (enum machine_mode);
 extern int arm_hard_regno_mode_ok (unsigned int, enum machine_mode);
+extern bool arm_modes_tieable_p (enum machine_mode, enum machine_mode);
 extern int const_ok_for_arm (HOST_WIDE_INT);
 extern int const_ok_for_op (HOST_WIDE_INT, enum rtx_code);
 extern int arm_split_constant (RTX_CODE, enum machine_mode, rtx,
Index: gcc/config/arm/arm.h
===
--- gcc/config/arm/arm.h2011-09-08 10:31:01.455663735 +0100
+++ gcc/config/arm/arm.h2011-09-09 13:46:32.865939164 +0100
@@ -962,12 +962,7 @@ #define HARD_REGNO_NREGS(REGNO, MODE)  
 #define HARD_REGNO_MODE_OK(REGNO, MODE)
\
   arm_hard_regno_mode_ok ((REGNO), (MODE))
 
-/* Value is 1 if it is a good idea to tie two pseudo registers
-   when one has mode MODE1 and one has mode MODE2.
-   If HARD_REGNO_MODE_OK could produce different values for MODE1 and MODE2,
-   for any hard reg, then this must be 0 for correct output.  */
-#define MODES_TIEABLE_P(MODE1, MODE2)  \
-  (GET_MODE_CLASS (MODE1) == GET_MODE_CLASS (MODE2))
+#define MODES_TIEABLE_P(MODE1, MODE2) arm_modes_tieable_p (MODE1, MODE2)
 
 #define VALID_IWMMXT_REG_MODE(MODE) \
  (arm_vector_mode_supported_p (MODE) || (MODE) == DImode)
Index: gcc/config/arm/arm.c
===
--- gcc/config/arm/arm.c2011-09-08 10:31:01.455663735 +0100
+++ gcc/config/arm/arm.c2011-09-09 13:46:32.842939218 +0100
@@ -18236,6 +18236,29 @@ arm_hard_regno_mode_ok (unsigned int reg
  && regno <= LAST_FPA_REGNUM);
 }
 
+/* Implement MODES_TIEABLE_P.  */
+
+bool
+arm_modes_tieable_p (enum machine_mode mode1, enum machine_mode mode2)
+{
+  if (GET_MODE_CLASS (mode1) == GET_MODE_CLASS (mode2))
+return true;
+
+  /* We specifically want to allow elements of "structure" modes to
+ be tieable to the structure.  This more general condition allows
+ other rarer situations too.  */
+  if (TARGET_NEON
+  && (VALID_NEON_DREG_MODE (mode1)
+ || VALID_NEON_QREG_MODE (mode1)
+ || VALID_NEON_STRUCT_MODE (mode1))
+  && (VALID_NEON_DREG_MODE (mode2)
+ || VALID_NEON_QREG_MODE (mode2)
+ || VALID_NEON_STRUCT_MODE (mode2)))
+return true;
+
+  return false;
+}
+
 /* For efficiency and historical reasons LO_REGS, HI_REGS and CC_REGS are
not used in arm mode.  */
 

Re: New SPU failures (Re: [PATCH, PR 49923] Check for misaligned accesses before doing SRA)

[Richard Guenther]

On Fri, 9 Sep 2011, Martin Jambor wrote:

> Hi,
> 
> On Wed, Aug 10, 2011 at 04:29:40PM +0200, Richard Guenther wrote:
> > On Wed, 10 Aug 2011, Ulrich Weigand wrote:
> > 
> 
> ...
> 
> > > 
> > > FAIL: gcc.dg/ipa/ipa-sra-2.c scan-tree-dump eipa_sra "About to replace 
> > > expr cow_.*D.->red with \\*ISRA"
> > > FAIL: gcc.dg/ipa/ipa-sra-2.c scan-tree-dump eipa_sra "About to replace 
> > > expr cow_.*D.->green with ISRA"
> > > FAIL: gcc.dg/ipa/ipa-sra-2.c scan-tree-dump eipa_sra "About to replace 
> > > expr calf_.*D.->red with \\*ISRA"
> > > FAIL: gcc.dg/ipa/ipa-sra-2.c scan-tree-dump eipa_sra "About to replace 
> > > expr calf_.*D.->green with ISRA"
> > > FAIL: gcc.dg/ipa/ipa-sra-6.c scan-tree-dump-times eipa_sra "foo " 1
> 
> This has recvently been reported as PR 50052.
> 
> ...
> 
> > > 
> > > I must admin I continue to be confused about exactly what it is that
> > > tree_non_mode_aligned_mem_p is supposed to be testing for.  We have:
> > > 
> > >   if (TREE_CODE (exp) == SSA_NAME
> > >   || TREE_CODE (exp) == MEM_REF
> > >   || mode == BLKmode
> > >   || is_gimple_min_invariant (exp)
> > >   || !STRICT_ALIGNMENT)
> > > return false;
> > > 
> > > So if we passed in the plain MEM_REF, this would be considered no problem.
> > > The COMPONENT_REF does not add any additional misalignment, so one would
> > > hope that this also shouldn't be a problem.
> > > 
> > > However, just because there *is* a COMPONENT_REF around it, we suddenly
> > > realize the fact that don't have points-to information for the MEM_REF
> > > and therefore consider *it* (and consequently the whole COMPONENT_REF)
> > > to be potentially misaligned ...
> > 
> > Yep.  That's because we are totally confused about alignment :/
> > 
> > Martin, what we need to do is get expands idea of what alignment
> > it woudl assume for a handled_component_ref and compare that
> > to what it would say if we re-materialize the mem as a MEM_REF.
> > 
> > Unfortunately there isn't a function that you can use to mimic
> > expands behavior (that of the normal_inner_ref: case), the closest
> > would be to look for TYPE_PACKED or TYPE_ALIGN in addition to
> > what get_object_alignment gives us.
> > 
> > Thus something like
> > 
> >   align = get_object_alignment (exp);
> >   if (!TYPE_PACKED (TREE_TYPE (exp))
> >   && (TREE_CODE (exp) != COMPONENT_REF
> >   || !DECL_PACKED (TREE_OPERAND (exp, 1
> > align = MAX (TYPE_ALIGN (TREE_TYPE (exp)), align);
> >   if (GET_MODE_ALIGNMENT (mode) > align)
> > return true;
> > 
> 
> This fixed the failing testcase ipa-sra-2.c but it creates a
> misaligned access for the testcase below so I changed it to:
> 
>   align = get_object_alignment (exp);
>   if (!TYPE_PACKED (TREE_TYPE (exp)))
> {
>   bool encountered_packed = false;
>   tree t = exp;
> 
>   while (TREE_CODE (t) == COMPONENT_REF)

that should be handled_component_p (t) then to catch p->a.b[i].c
with packed a.

But the normal_inner_ref: case doesn't suggest that we need to dive
into handled-components at all ... but it uses DECL_MODE of
an outermost COMPONENT_REF instead of TYPE_MODE.  Maybe the easiest
would be to simply call get_inner_reference like the normal_inner_ref
case does to obtain the mode, perform the packed_p check and
decide based on that (supposed that the mode difference is what
makes the new testcase fail).  Also look for the predicates that
guard the extract_bit_field call ... (yeah, I know ...)

> if (DECL_PACKED (TREE_OPERAND (t, 1)))
>   {
> encountered_packed = true;
> break;
>   }
> else
>   t = TREE_OPERAND (t, 0);
> 
>   if (!encountered_packed)
> align = MAX (TYPE_ALIGN (TREE_TYPE (exp)), align);
> }
>   if (GET_MODE_ALIGNMENT (mode) > align)
> return true;
> 
> I'm currently bootstrapping this on a sparc64 on the compile farm.  Is
> the patch OK if it passes (it has passed bootstrap and testing on
> x86_64-linux but that is no surprise)?
> 
> Thanks,
> 
> Martin
> 
> 
> 2011-09-08  Martin Jambor  
> 
>   PR tree-optimization/50052
>   * tree-sra.c (tree_non_mode_aligned_mem_p): Look at TYPE_ALIGN if
>   there is no packed field decl in exp.
> 
>   * gcc.dg/tree-ssa/pr49094-2.c: New test.
> 
> Index: src/gcc/testsuite/gcc.dg/tree-ssa/pr49094-2.c
> ===
> --- /dev/null
> +++ src/gcc/testsuite/gcc.dg/tree-ssa/pr49094-2.c
> @@ -0,0 +1,44 @@
> +/* { dg-do run } */
> +/* { dg-options "-O" } */
> +
> +struct in_addr {
> +  unsigned int s_addr;
> +};
> +
> +struct ip {
> +  struct in_addr ip_src,ip_dst;
> +  unsigned char ip_p;
> +  unsigned short ip_sum;
> +};
> +
> +struct ip2 {
> +  unsigned char blah;
> +  unsigned short sheep;
> +  struct ip ip;
> +} __attribute__ ((aligned(1), packed));
> +
> +struct ip2 ip_fw_fwd_addr;
> +
> +int test_alignment( char *m )
> +{
> +  struct ip2 *ip = (struct ip2 *) m;
> +  struct in_addr pk

PING: [ARM] Model automodified addresses in the Cortex A8 and A9 schedulers -- NEON

[Richard Sandiford]

Ping for this patch:

http://gcc.gnu.org/ml/gcc-patches/2011-08/msg01488.html

which models address register writeback in the Cortex A8 and A9 NEON
schedulers.  (Ramana has already approved the core equivalent, thanks.)

Richard

Ping: Make SMS schedule register moves

[Richard Sandiford]

Ping for these four patches:

http://gcc.gnu.org/ml/gcc-patches/2011-08/msg02428.html

which make SMS schedule the register moves it creates.

Richard

Re: [Patch, testsuite, arm] Skip the arch conflict to enable case neon-thumb2-move pass on more targets.

[Richard Guenther]

On Fri, Sep 9, 2011 at 2:47 PM, Richard Earnshaw  wrote:
> On 09/09/11 11:07, Richard Guenther wrote:
>> On Fri, Sep 9, 2011 at 12:00 PM, Terry Guo  wrote:
>>> Hello,
>>>
>>> This patch enables the case pass on targets other than armv7-a by skipping
>>> the architecture conflict message. Is it OK to trunk?
>>
>> I don't like these kind of patches.  Why do the testcases have -march=armv7-a
>> enabled in the first place?  If they really need it then they shouldn't be
>> run with another -march and thus instead there should be a
>> dg-requires-effective-target armv7-a.  If they don't really need it
>> they shouldn't
>> add it.
>>
>
> I'm not particularly keen on them either, but with the current structure
> of the testsuite framework the only way to test some unusual option
> combinations is to force them (we can't test every option set, or even
> every CPU supported via multilib testing as it would take ~forever to
> run through all the variants).  So the only alternative is to force some
> tests to run with specific options.
>
> It might be easier if there were a specific part of the testsuite which
> was designated to be non-executable, non-multilib.  That part would then
> be used without all the multilib paraphernalia that leads to these hacks
> (and would only ever be run once as well, regardless of the number of
> multilibs being tested).  But until such time, this patch seems like the
> only way forward.

I suppose you want a torture that excercises different -march/-mtune
combinations then.

But can't you do the pruning somewhere in an .exp file then instead
of sprinkling it all over the tests itself?

Richard.

> R.
>
>> Richard.
>>
>>> BR,
>>> Terry
>>>
>>> 2011-09-09  Terry Guo  
>>>
>>>          * gcc.target/arm/neon-thumb2-move.c: Skip the
>>>        architecture conflict to enable the case pass
>>>        on targets other than armv7-a.
>>>
>>> diff --git a/gcc/testsuite/gcc.target/arm/neon-thumb2-move.c
>>> b/gcc/testsuite/gcc.target/arm/neon-thumb2-move.c
>>> index 430a4d5..9cf86dd 100644
>>> --- a/gcc/testsuite/gcc.target/arm/neon-thumb2-move.c
>>> +++ b/gcc/testsuite/gcc.target/arm/neon-thumb2-move.c
>>> @@ -3,6 +3,7 @@
>>>  /* { dg-require-effective-target arm_thumb2_ok } */
>>>  /* { dg-options "-O2 -mthumb -march=armv7-a" } */
>>>  /* { dg-add-options arm_neon } */
>>> +/* { dg-prune-output "switch .* conflicts with" } */
>>>
>>>  #include 
>>>  #include 
>>>
>>>
>>>
>>>
>>>
>>>
>>
>
>

Re: New SPU failures (Re: [PATCH, PR 49923] Check for misaligned accesses before doing SRA)

[Richard Guenther]

On Fri, 9 Sep 2011, Richard Guenther wrote:

> On Fri, 9 Sep 2011, Martin Jambor wrote:
> 
> > Hi,
> > 
> > On Wed, Aug 10, 2011 at 04:29:40PM +0200, Richard Guenther wrote:
> > > On Wed, 10 Aug 2011, Ulrich Weigand wrote:
> > > 
> > 
> > ...
> > 
> > > > 
> > > > FAIL: gcc.dg/ipa/ipa-sra-2.c scan-tree-dump eipa_sra "About to replace 
> > > > expr cow_.*D.->red with \\*ISRA"
> > > > FAIL: gcc.dg/ipa/ipa-sra-2.c scan-tree-dump eipa_sra "About to replace 
> > > > expr cow_.*D.->green with ISRA"
> > > > FAIL: gcc.dg/ipa/ipa-sra-2.c scan-tree-dump eipa_sra "About to replace 
> > > > expr calf_.*D.->red with \\*ISRA"
> > > > FAIL: gcc.dg/ipa/ipa-sra-2.c scan-tree-dump eipa_sra "About to replace 
> > > > expr calf_.*D.->green with ISRA"
> > > > FAIL: gcc.dg/ipa/ipa-sra-6.c scan-tree-dump-times eipa_sra "foo " 1
> > 
> > This has recvently been reported as PR 50052.
> > 
> > ...
> > 
> > > > 
> > > > I must admin I continue to be confused about exactly what it is that
> > > > tree_non_mode_aligned_mem_p is supposed to be testing for.  We have:
> > > > 
> > > >   if (TREE_CODE (exp) == SSA_NAME
> > > >   || TREE_CODE (exp) == MEM_REF
> > > >   || mode == BLKmode
> > > >   || is_gimple_min_invariant (exp)
> > > >   || !STRICT_ALIGNMENT)
> > > > return false;
> > > > 
> > > > So if we passed in the plain MEM_REF, this would be considered no 
> > > > problem.
> > > > The COMPONENT_REF does not add any additional misalignment, so one would
> > > > hope that this also shouldn't be a problem.
> > > > 
> > > > However, just because there *is* a COMPONENT_REF around it, we suddenly
> > > > realize the fact that don't have points-to information for the MEM_REF
> > > > and therefore consider *it* (and consequently the whole COMPONENT_REF)
> > > > to be potentially misaligned ...
> > > 
> > > Yep.  That's because we are totally confused about alignment :/
> > > 
> > > Martin, what we need to do is get expands idea of what alignment
> > > it woudl assume for a handled_component_ref and compare that
> > > to what it would say if we re-materialize the mem as a MEM_REF.
> > > 
> > > Unfortunately there isn't a function that you can use to mimic
> > > expands behavior (that of the normal_inner_ref: case), the closest
> > > would be to look for TYPE_PACKED or TYPE_ALIGN in addition to
> > > what get_object_alignment gives us.
> > > 
> > > Thus something like
> > > 
> > >   align = get_object_alignment (exp);
> > >   if (!TYPE_PACKED (TREE_TYPE (exp))
> > >   && (TREE_CODE (exp) != COMPONENT_REF
> > >   || !DECL_PACKED (TREE_OPERAND (exp, 1
> > > align = MAX (TYPE_ALIGN (TREE_TYPE (exp)), align);
> > >   if (GET_MODE_ALIGNMENT (mode) > align)
> > > return true;
> > > 
> > 
> > This fixed the failing testcase ipa-sra-2.c but it creates a
> > misaligned access for the testcase below so I changed it to:
> > 
> >   align = get_object_alignment (exp);
> >   if (!TYPE_PACKED (TREE_TYPE (exp)))
> > {
> >   bool encountered_packed = false;
> >   tree t = exp;
> > 
> >   while (TREE_CODE (t) == COMPONENT_REF)
> 
> that should be handled_component_p (t) then to catch p->a.b[i].c
> with packed a.
> 
> But the normal_inner_ref: case doesn't suggest that we need to dive
> into handled-components at all ... but it uses DECL_MODE of
> an outermost COMPONENT_REF instead of TYPE_MODE.  Maybe the easiest
> would be to simply call get_inner_reference like the normal_inner_ref
> case does to obtain the mode, perform the packed_p check and
> decide based on that (supposed that the mode difference is what
> makes the new testcase fail).  Also look for the predicates that
> guard the extract_bit_field call ... (yeah, I know ...)

Btw, it would be nice to try to produce a testcase for x86 by
using SSE vector types and make sure we do / do not expand to
unaligned moves (thus, both check for correctness and optimality).

Richard.

> > if (DECL_PACKED (TREE_OPERAND (t, 1)))
> >   {
> > encountered_packed = true;
> > break;
> >   }
> > else
> >   t = TREE_OPERAND (t, 0);
> > 
> >   if (!encountered_packed)
> > align = MAX (TYPE_ALIGN (TREE_TYPE (exp)), align);
> > }
> >   if (GET_MODE_ALIGNMENT (mode) > align)
> > return true;
> > 
> > I'm currently bootstrapping this on a sparc64 on the compile farm.  Is
> > the patch OK if it passes (it has passed bootstrap and testing on
> > x86_64-linux but that is no surprise)?
> > 
> > Thanks,
> > 
> > Martin
> > 
> > 
> > 2011-09-08  Martin Jambor  
> > 
> > PR tree-optimization/50052
> > * tree-sra.c (tree_non_mode_aligned_mem_p): Look at TYPE_ALIGN if
> > there is no packed field decl in exp.
> > 
> > * gcc.dg/tree-ssa/pr49094-2.c: New test.
> > 
> > Index: src/gcc/testsuite/gcc.dg/tree-ssa/pr49094-2.c
> > ===
> > --- /dev/null
> > +++ src/gcc/testsuite/gcc.dg/tree-ssa/pr49094-2.c
> > @@ -0,0 +1,4

Re: [Patch, testsuite, arm] Skip the arch conflict to enable case neon-thumb2-move pass on more targets.

[Richard Earnshaw]

On 9 Sep 2011, at 14:03, "Richard Guenther"  wrote:

> On Fri, Sep 9, 2011 at 2:47 PM, Richard Earnshaw  wrote:
>> On 09/09/11 11:07, Richard Guenther wrote:
>>> On Fri, Sep 9, 2011 at 12:00 PM, Terry Guo  wrote:
 Hello,
 
 This patch enables the case pass on targets other than armv7-a by skipping
 the architecture conflict message. Is it OK to trunk?
>>> 
>>> I don't like these kind of patches.  Why do the testcases have 
>>> -march=armv7-a
>>> enabled in the first place?  If they really need it then they shouldn't be
>>> run with another -march and thus instead there should be a
>>> dg-requires-effective-target armv7-a.  If they don't really need it
>>> they shouldn't
>>> add it.
>>> 
>> 
>> I'm not particularly keen on them either, but with the current structure
>> of the testsuite framework the only way to test some unusual option
>> combinations is to force them (we can't test every option set, or even
>> every CPU supported via multilib testing as it would take ~forever to
>> run through all the variants).  So the only alternative is to force some
>> tests to run with specific options.
>> 
>> It might be easier if there were a specific part of the testsuite which
>> was designated to be non-executable, non-multilib.  That part would then
>> be used without all the multilib paraphernalia that leads to these hacks
>> (and would only ever be run once as well, regardless of the number of
>> multilibs being tested).  But until such time, this patch seems like the
>> only way forward.
> 
> I suppose you want a torture that excercises different -march/-mtune
> combinations then.
> 
> But can't you do the pruning somewhere in an .exp file then instead
> of sprinkling it all over the tests itself?
> 

It seems not.  At present the multilib options are all added automatically by 
the dejagnu infrastructure.

R.

> Richard.
> 
>> R.
>> 
>>> Richard.
>>> 
 BR,
 Terry
 
 2011-09-09  Terry Guo  
 
  * gcc.target/arm/neon-thumb2-move.c: Skip the
architecture conflict to enable the case pass
on targets other than armv7-a.
 
 diff --git a/gcc/testsuite/gcc.target/arm/neon-thumb2-move.c
 b/gcc/testsuite/gcc.target/arm/neon-thumb2-move.c
 index 430a4d5..9cf86dd 100644
 --- a/gcc/testsuite/gcc.target/arm/neon-thumb2-move.c
 +++ b/gcc/testsuite/gcc.target/arm/neon-thumb2-move.c
 @@ -3,6 +3,7 @@
  /* { dg-require-effective-target arm_thumb2_ok } */
  /* { dg-options "-O2 -mthumb -march=armv7-a" } */
  /* { dg-add-options arm_neon } */
 +/* { dg-prune-output "switch .* conflicts with" } */
 
  #include 
  #include 
 
 
 
 
 
 
>>> 
>> 
>> 
> 

Re: [PATCH][Cilkplus] Patch to fix Template type inside cilk_for

[H.J. Lu]

On Thu, Sep 8, 2011 at 8:30 PM, Iyer, Balaji V  wrote:
> Hello Everyone,
>        This patch is for the Cilk Plus branch GCC C++ compiler. It will fix 
> the following cases of cilk_for (where 'T' is a template type)
>
> _Cilk_for ( T ii =  ;  ii   ;  
>   ii <+/->= )
>        
>

+   * cilk.c (check_incr): added "&& (TREE_OPERAND (incr, 0) !=
 ^  Should be 'A'.
+   DECL_NAME (var))" to if (TREE_OPERAND(incr, 0) != var) and to the outer
+   else if statements. Also removed gcc_assert (TREE_OPERAND (incr, 0) ==
+   var) from the body of both of these statements.

Can you find better description without quoting sources?

-  if (TREE_OPERAND (incr, 0) != var)
+  if ((TREE_OPERAND (incr, 0) != var) &&

 It should be on the next line.
+ (DECL_NAME (TREE_OPERAND (incr, 0)) != DECL_NAME (var)))

Please remove the extra ().  You have many extra () in your change.
All the trailing && and || should be on the next line.

+   * g++.dg/cilk-plus/template_cilk_for_plus_equal.cpp: New

Please add' the missing period and create a separate entry.

-- 
H.J.

Re: [Patch][Master] Finish function using absolute value not #define value

[H.J. Lu]

On Thu, Sep 8, 2011 at 9:38 PM, Iyer, Balaji V  wrote:
> Hello Everyone,
>        In several places, I found that finish_function was using an absolute 
> integer as input parameter instead of these #defines
>
> #define SF_DEFAULT           0  /* No flags.  */
> #define SF_PRE_PARSED        1  /* The function declaration has
>                                   already been parsed.  */
> #define SF_INCLASS_INLINE    2  /* The function is an inline, defined
>                                   in the class body.  */
>
> I have attached a patch that should fix this (for example, use SF_DEFAULT 
> instead of '0'). It should fix the bug #47791 
> (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47791)
>

A few comments:

1.  Remove [Master] in your email subject.
2.  Put ChangeLog entries like:

http://gcc.gnu.org/ml/gcc-patches/2011-09/msg00615.html

since you are not the only one who is changing trunk. Your
inlined ChangeLog won't apply when ChangeLogs are
updated by other people.


-- 
H.J.

Re: [PATCH][ARM] Generic tuning

[Andrew Stubbs]

On 08/09/11 16:32, Richard Earnshaw wrote:

OK.


Committed, thanks

Andrew

Re: [Patch][Master] Finish function using absolute value not #define value

[Tobias Burnus]

On 09/09/2011 03:55 PM, H.J. Lu wrote:

1.  Remove [Master] in your email subject.
2.  Put ChangeLog entries like:

> http://gcc.gnu.org/ml/gcc-patches/2011-09/msg00615.html

3. Do not attach patches with
Content-Type: application/octet-stream

(Email programs usually set the content-type automatically to text/* for 
extensions like .diff or .patch or .txt)


If you have application/octet-stream, others have to save the patch to a 
file in order to view it and and it also makes it much harder to reply 
with comments to the patch.


Tobias

PS: Some middle-end maintainers prefer if patches are not attached to an 
email but rather included in the email body as it makes it easier (with 
some emails clients) to comment on the patch. If you do so, please 
ensure that the tabs and the linebreaks of the patch remain intact.

RE: [Patch] Finish function using absolute value not #define value

[Iyer, Balaji V]

Hello Everyone,
Here are the fixes to the patches as mentioned by H. J. . I am not 
attaching the patch, just cut and pasting it.

Thanks,

Balaji V. Iyer.

Here is the cp/ChangeLog

2011-09-09  Balaji V. Iyer  

* decl2.c (finish_objects): Replaced finish_function (0) with
finish_function (SF_PRE_PARSED).
(finish_static_storage_duration_function): Likewise.
* decl.c (end_cleanup_fn): Likewise.
* method.c (synthesize_method): Likewise.
* optimize.c (maybe_clone_body): Likewise.
* pt.c (instantiate_decl): Likewise.
* parser.c (cp_parser_function_definition_after_declarator): Replaced
finish_function ((ctor_initializer_p ? 1 : 0) | (inline_p ? 2 : 0))
with finish_function ((ctor_initializer_p ? SF_PRE_PARSED : SF_DEFAULT)
| (inline_p ? SF_INCLASS_INLINE : SF_DEFAULT)).
* parser.c (cp_parser_lambda_body): Replaced finish_function (2) with
finish_function (SF_INCLASS_INLINE).
* semantics.c (maybe_add_lambda_conv_op): Likewise.

Here is the objcp/ChangeLog

2011-09-09  Balaji V. Iyer  

* objcp-decl.c (objcp_finish_function): Replaced finish_function (0)
with finish_function (SF_DEFAULT).


diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index eed4535..b92f13b 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -6540,7 +6540,7 @@ start_cleanup_fn (void)
 static void
 end_cleanup_fn (void)
 {
-  expand_or_defer_fn (finish_function (0));
+  expand_or_defer_fn (finish_function (SF_DEFAULT));
 
   pop_from_top_level ();
 }
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index f05b0f8..5b3bcc0 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -2775,7 +2775,7 @@ finish_objects (int method_type, int initp, tree body)
 
   /* Finish up.  */
   finish_compound_stmt (body);
-  fn = finish_function (0);
+  fn = finish_function (SF_DEFAULT);
 
   if (method_type == 'I')
 {
@@ -2917,7 +2917,7 @@ finish_static_storage_duration_function (tree body)
 {
   /* Close out the function.  */
   finish_compound_stmt (body);
-  expand_or_defer_fn (finish_function (0));
+  expand_or_defer_fn (finish_function (SF_DEFAULT));
 }
 
 /* Return the information about the indicated PRIORITY level.  If no
diff --git a/gcc/cp/method.c b/gcc/cp/method.c
index 5b24f8f..97213f1 100644
--- a/gcc/cp/method.c
+++ b/gcc/cp/method.c
@@ -767,7 +767,7 @@ synthesize_method (tree fndecl)
 }
 
   finish_function_body (stmt);
-  expand_or_defer_fn (finish_function (0));
+  expand_or_defer_fn (finish_function (SF_DEFAULT));
 
   input_location = save_input_location;
 
diff --git a/gcc/cp/optimize.c b/gcc/cp/optimize.c
index 6a06988..2e1c7b6 100644
--- a/gcc/cp/optimize.c
+++ b/gcc/cp/optimize.c
@@ -415,7 +415,7 @@ maybe_clone_body (tree fn)
   cp_function_chain->can_throw = !TREE_NOTHROW (fn);
 
   /* Now, expand this function into RTL, if appropriate.  */
-  finish_function (0);
+  finish_function (SF_DEFAULT);
   BLOCK_ABSTRACT_ORIGIN (DECL_INITIAL (clone)) = DECL_INITIAL (fn);
   if (alias)
{
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 7d766d1..3fc6c75 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -7904,7 +7904,7 @@ cp_parser_lambda_body (cp_parser* parser, tree 
lambda_expr)
 finish_lambda_scope ();
 
 /* Finish the function and generate code for it if necessary.  */
-expand_or_defer_fn (finish_function (/*inline*/2));
+expand_or_defer_fn (finish_function (SF_INCLASS_INLINE));
   }
 
   parser->local_variables_forbidden_p = local_variables_forbidden_p;
@@ -20091,8 +20091,8 @@ cp_parser_function_definition_after_declarator 
(cp_parser* parser,
   finish_lambda_scope ();
 
   /* Finish the function.  */
-  fn = finish_function ((ctor_initializer_p ? 1 : 0) |
-   (inline_p ? 2 : 0));
+  fn = finish_function ((ctor_initializer_p ? SF_PRE_PARSED : SF_DEFAULT) |
+   (inline_p ? SF_INCLASS_INLINE : SF_DEFAULT));
   /* Generate code for it, if necessary.  */
   expand_or_defer_fn (fn);
   /* Restore the saved values.  */
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 1f43ff1..8a79262 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -18286,7 +18286,7 @@ instantiate_decl (tree d, int defer_ok,
   local_specializations = saved_local_specializations;
 
   /* Finish the function.  */
-  d = finish_function (0);
+  d = finish_function (SF_DEFAULT);
   expand_or_defer_fn (d);
 }
 
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index f782df9..b416135 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -9043,7 +9043,7 @@ maybe_add_lambda_conv_op (tree type)
   finish_compound_stmt (compound_stmt);
   finish_function_body (body);
 
-  expand_or_defer_fn (finish_function (2));
+  expand_or_defer_fn (finish_function (SF_INCLASS_INLINE));
 
   /* Generate the body of the conversion op.  */
 
@@ -9057,7 +9057,7 @@ maybe_add_lambda_conv_op (tree type)
   finish_compound_stmt (compound_stmt);
   finish_function

Re: [PATCH] Fix PR50328

[H.J. Lu]

On Fri, Sep 9, 2011 at 3:15 AM, Richard Guenther  wrote:
>
> This fixes our inability to handle reductions with a constant
> or a parameter.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, the testcase
> requires all the previously posted patches from today to not ICE
> and pass though.
>
> Richard.
>
> 2011-09-09  Richard Guenther  
>
>        PR tree-optimization/50328
>        * tree-vect-loop.c (vect_is_simple_reduction_1): Allow one
>        constant or default-def operand.
>
>        * gcc.dg/vect/fast-math-vect-outer-7.c: New testcase.
>

This may have caused:

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


-- 
H.J.

Re: [Patch] Finish function using absolute value not #define value

[H.J. Lu]

On Fri, Sep 9, 2011 at 7:48 AM, Iyer, Balaji V  wrote:
> Hello Everyone,
>        Here are the fixes to the patches as mentioned by H. J. . I am not 
> attaching the patch, just cut and pasting it.

Cut/paste may not work if other people have to apply the patch for
you.

> Thanks,
>
> Balaji V. Iyer.
>
> Here is the cp/ChangeLog
>
> 2011-09-09  Balaji V. Iyer  
>
>        * decl2.c (finish_objects): Replaced finish_function (0) with
>        finish_function (SF_PRE_PARSED).
>        (finish_static_storage_duration_function): Likewise.
>        * decl.c (end_cleanup_fn): Likewise.
>        * method.c (synthesize_method): Likewise.
>        * optimize.c (maybe_clone_body): Likewise.
>        * pt.c (instantiate_decl): Likewise.
>        * parser.c (cp_parser_function_definition_after_declarator): Replaced
>        finish_function ((ctor_initializer_p ? 1 : 0) | (inline_p ? 2 : 0))
>        with finish_function ((ctor_initializer_p ? SF_PRE_PARSED : SF_DEFAULT)
>        | (inline_p ? SF_INCLASS_INLINE : SF_DEFAULT)).
>        * parser.c (cp_parser_lambda_body): Replaced finish_function (2) with
>        finish_function (SF_INCLASS_INLINE).
>        * semantics.c (maybe_add_lambda_conv_op): Likewise.
>
> Here is the objcp/ChangeLog
>
> 2011-09-09  Balaji V. Iyer  
>
>        * objcp-decl.c (objcp_finish_function): Replaced finish_function (0)
>        with finish_function (SF_DEFAULT).
>

Your ChangeLog can just say "Use SF_DEFAULT, SF_PRE_PARSED
and SF_INCLASS_INLINE."


-- 
H.J.

RE: [Patch] Finish function using absolute value not #define value

[Iyer, Balaji V]

Patch attached :).

-Balaji V. Iyer.


-Original Message-
From: H.J. Lu [mailto:hjl.to...@gmail.com] 
Sent: Friday, September 09, 2011 11:14 AM
To: Iyer, Balaji V
Cc: Tobias Burnus; gcc-patches@gcc.gnu.org
Subject: Re: [Patch] Finish function using absolute value not #define value

On Fri, Sep 9, 2011 at 7:48 AM, Iyer, Balaji V  wrote:
> Hello Everyone,
>        Here are the fixes to the patches as mentioned by H. J. . I am not 
> attaching the patch, just cut and pasting it.

Cut/paste may not work if other people have to apply the patch for you.

> Thanks,
>
> Balaji V. Iyer.
>
> Here is the cp/ChangeLog
>
> 2011-09-09  Balaji V. Iyer  
>
>        * decl2.c (finish_objects): Replaced finish_function (0) with
>        finish_function (SF_PRE_PARSED).
>        (finish_static_storage_duration_function): Likewise.
>        * decl.c (end_cleanup_fn): Likewise.
>        * method.c (synthesize_method): Likewise.
>        * optimize.c (maybe_clone_body): Likewise.
>        * pt.c (instantiate_decl): Likewise.
>        * parser.c (cp_parser_function_definition_after_declarator): 
> Replaced
>        finish_function ((ctor_initializer_p ? 1 : 0) | (inline_p ? 2 : 
> 0))
>        with finish_function ((ctor_initializer_p ? SF_PRE_PARSED : 
> SF_DEFAULT)
>        | (inline_p ? SF_INCLASS_INLINE : SF_DEFAULT)).
>        * parser.c (cp_parser_lambda_body): Replaced finish_function 
> (2) with
>        finish_function (SF_INCLASS_INLINE).
>        * semantics.c (maybe_add_lambda_conv_op): Likewise.
>
> Here is the objcp/ChangeLog
>
> 2011-09-09  Balaji V. Iyer  
>
>        * objcp-decl.c (objcp_finish_function): Replaced 
> finish_function (0)
>        with finish_function (SF_DEFAULT).
>

Your ChangeLog can just say "Use SF_DEFAULT, SF_PRE_PARSED and 
SF_INCLASS_INLINE."


--
H.J.
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index eed4535..b92f13b 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -6540,7 +6540,7 @@ start_cleanup_fn (void)
 static void
 end_cleanup_fn (void)
 {
-  expand_or_defer_fn (finish_function (0));
+  expand_or_defer_fn (finish_function (SF_DEFAULT));
 
   pop_from_top_level ();
 }
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index f05b0f8..5b3bcc0 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -2775,7 +2775,7 @@ finish_objects (int method_type, int initp, tree body)
 
   /* Finish up.  */
   finish_compound_stmt (body);
-  fn = finish_function (0);
+  fn = finish_function (SF_DEFAULT);
 
   if (method_type == 'I')
 {
@@ -2917,7 +2917,7 @@ finish_static_storage_duration_function (tree body)
 {
   /* Close out the function.  */
   finish_compound_stmt (body);
-  expand_or_defer_fn (finish_function (0));
+  expand_or_defer_fn (finish_function (SF_DEFAULT));
 }
 
 /* Return the information about the indicated PRIORITY level.  If no
diff --git a/gcc/cp/method.c b/gcc/cp/method.c
index 5b24f8f..97213f1 100644
--- a/gcc/cp/method.c
+++ b/gcc/cp/method.c
@@ -767,7 +767,7 @@ synthesize_method (tree fndecl)
 }
 
   finish_function_body (stmt);
-  expand_or_defer_fn (finish_function (0));
+  expand_or_defer_fn (finish_function (SF_DEFAULT));
 
   input_location = save_input_location;
 
diff --git a/gcc/cp/optimize.c b/gcc/cp/optimize.c
index 6a06988..2e1c7b6 100644
--- a/gcc/cp/optimize.c
+++ b/gcc/cp/optimize.c
@@ -415,7 +415,7 @@ maybe_clone_body (tree fn)
   cp_function_chain->can_throw = !TREE_NOTHROW (fn);
 
   /* Now, expand this function into RTL, if appropriate.  */
-  finish_function (0);
+  finish_function (SF_DEFAULT);
   BLOCK_ABSTRACT_ORIGIN (DECL_INITIAL (clone)) = DECL_INITIAL (fn);
   if (alias)
{
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 7d766d1..3fc6c75 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -7904,7 +7904,7 @@ cp_parser_lambda_body (cp_parser* parser, tree 
lambda_expr)
 finish_lambda_scope ();
 
 /* Finish the function and generate code for it if necessary.  */
-expand_or_defer_fn (finish_function (/*inline*/2));
+expand_or_defer_fn (finish_function (SF_INCLASS_INLINE));
   }
 
   parser->local_variables_forbidden_p = local_variables_forbidden_p;
@@ -20091,8 +20091,8 @@ cp_parser_function_definition_after_declarator 
(cp_parser* parser,
   finish_lambda_scope ();
 
   /* Finish the function.  */
-  fn = finish_function ((ctor_initializer_p ? 1 : 0) |
-   (inline_p ? 2 : 0));
+  fn = finish_function ((ctor_initializer_p ? SF_PRE_PARSED : SF_DEFAULT) |
+   (inline_p ? SF_INCLASS_INLINE : SF_DEFAULT));
   /* Generate code for it, if necessary.  */
   expand_or_defer_fn (fn);
   /* Restore the saved values.  */
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 1f43ff1..8a79262 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -18286,7 +18286,7 @@ instantiate_decl (tree d, int defer_ok,
   local_specializations = saved_local_specializations;
 
   /* Finish the function.  */
-  d = finish_function (0);
+  d = finish_function (SF_DEFAULT);
   expand_or

RE: [PATCH][Cilkplus] Patch to fix Template type inside cilk_for

[Iyer, Balaji V]

Here is a fixed patch with all the changes you have requested.

Thanks,

Balaji V. Iyer.

-Original Message-
From: H.J. Lu [mailto:hjl.to...@gmail.com] 
Sent: Friday, September 09, 2011 9:49 AM
To: Iyer, Balaji V
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH][Cilkplus] Patch to fix Template type inside cilk_for

On Thu, Sep 8, 2011 at 8:30 PM, Iyer, Balaji V  wrote:
> Hello Everyone,
>        This patch is for the Cilk Plus branch GCC C++ compiler. It will fix 
> the following cases of cilk_for (where 'T' is a template type)
>
> _Cilk_for ( T ii =  ;  ii   ;  
>   ii <+/->= )
>        
>

+   * cilk.c (check_incr): added "&& (TREE_OPERAND (incr, 0) !=
 ^  Should be 'A'.
+   DECL_NAME (var))" to if (TREE_OPERAND(incr, 0) != var) and to the outer
+   else if statements. Also removed gcc_assert (TREE_OPERAND (incr, 0) ==
+   var) from the body of both of these statements.

Can you find better description without quoting sources?

-  if (TREE_OPERAND (incr, 0) != var)
+  if ((TREE_OPERAND (incr, 0) != var) &&

 It should be on the next line.
+ (DECL_NAME (TREE_OPERAND (incr, 0)) != DECL_NAME (var)))

Please remove the extra ().  You have many extra () in your change.
All the trailing && and || should be on the next line.

+   * g++.dg/cilk-plus/template_cilk_for_plus_equal.cpp: New

Please add' the missing period and create a separate entry.

-- 
H.J.
diff --git a/gcc/ChangeLog.cilk b/gcc/ChangeLog.cilk
index 8880b0a..299febb 100644
--- a/gcc/ChangeLog.cilk
+++ b/gcc/ChangeLog.cilk
@@ -2,6 +2,9 @@
 
* gimplify.c (gimplify_call_expr): Removed if (SPAWN_CALL_P (*expr))
statement.
+   * tree.c (walk_tree_1): Added "case CILK_FOR_STMT:".
+   * tree.h (CILK_FOR_VAR): Changed TREE_OPERAND(..., 4) to
+   TREE_OPERAND(..., 5).
 
 2011-09-06  Balaji V. Iyer  
 
diff --git a/gcc/cp/ChangeLog.cilk b/gcc/cp/ChangeLog.cilk
index b49f3bf..4c54dc6 100644
--- a/gcc/cp/ChangeLog.cilk
+++ b/gcc/cp/ChangeLog.cilk
@@ -1,3 +1,10 @@
+2011-09-08  Balaji V. Iyer  
+
+   * cp-tree.h (FOR_SCOPE): Changed FOR_STMT_CHECK to FOR_STMT_CHECK2
+   * cilk.c (check_incr): Added a check for variable entity name match, 
not just
+   var. Removed the assert to check if operand 0 is the variable.
+   (cp_extract_for_fields): Likewise.
+
 2011-09-07  Balaji V. Iyer  
 
* parser.c (cp_parser_jump_statement): Removed "IN_CILK_FOR | " from
diff --git a/gcc/cp/cilk.c b/gcc/cp/cilk.c
index 139ec27..49af1d7 100644
--- a/gcc/cp/cilk.c
+++ b/gcc/cp/cilk.c
@@ -1024,17 +1024,18 @@ check_incr(tree var, tree arith_type, tree incr)
   if (TREE_CODE (incr) == MODIFY_EXPR)
 {
   modify = true;
-  if (TREE_OPERAND (incr, 0) != var)
+  if (TREE_OPERAND (incr, 0) != var
+ && DECL_NAME (TREE_OPERAND (incr, 0)) != DECL_NAME (var))
{
  error("Cilk for increment does not modify the loop variable.\n");
  return false;
}
   incr = TREE_OPERAND (incr, 1);
   incr_code = TREE_CODE (incr);
-  gcc_assert (TREE_OPERAND (incr, 0) == var);
 
 }
-  else if (TREE_OPERAND (incr, 0) != var)
+  else if (TREE_OPERAND (incr, 0) != var
+  && DECL_NAME (TREE_OPERAND (incr, 0)) != DECL_NAME (var))
 {
   error ("Cilk for increment does not modify the loop variable.");
   return false;
@@ -2589,7 +2590,6 @@ cp_extract_for_fields (struct cilk_for_desc *cfd, tree 
for_stmt)
 case MODIFY_EXPR:
   /* We don't get here unless the expression has the form
 (modify var (op var incr)) */
-  gcc_assert (TREE_OPERAND (incr, 0) == var);
   incr = TREE_OPERAND (incr, 1);
   /* again, should have checked form of increment earlier */
   if (TREE_CODE (incr) == PLUS_EXPR)
@@ -2597,9 +2597,13 @@ cp_extract_for_fields (struct cilk_for_desc *cfd, tree 
for_stmt)
  tree op0 = TREE_OPERAND (incr, 0);
  tree op1 = TREE_OPERAND (incr, 1);
 
- if (op0 == var)
+ /* if op0 is a pointer, then we should make sure the original 
+variable also works (e.g. if we declared as *i, then i++ is 
+acceptable) 
+  */
+ if (op0 == var || DECL_NAME (op0) == DECL_NAME (var))
incr = op1;
- else if (op1 == var)
+ else if (op1 == var || DECL_NAME (op1) == DECL_NAME (var))
incr = op0;
  else
gcc_unreachable ();
@@ -2637,8 +2641,17 @@ cp_extract_for_fields (struct cilk_for_desc *cfd, tree 
for_stmt)
  tree op0 = TREE_OPERAND (incr, 0);
  tree op1 = TREE_OPERAND (incr, 1);
 
- gcc_assert (op0 == var);
- incr = op1;
+ /* if op0 is a pointer, then we should make sure the original 
+variable also works (e.g. if we declared as *i, then i++ is 
+acceptable) 
+  */
+ if (op0 == var || DECL_NAME (op0) == DECL_NAME (var))
+   incr = op1;
+ else if (op1 == 

[PATCH][Cilkplus] Patch to add GCC Standard header

[Iyer, Balaji V]

Hello Everyone,
   This patch is for the Cilk plus branch. This patch will add the 
standard GCC header into pragma_simd.c file that I created.

Thanks,

Balaji V. Iyer.
diff --git a/gcc/ChangeLog.cilk b/gcc/ChangeLog.cilk
index 299febb..0eaa46e 100644
--- a/gcc/ChangeLog.cilk
+++ b/gcc/ChangeLog.cilk
@@ -1,3 +1,7 @@
+2011-09-09  Balaji V. Iyer  
+
+   * pragma_simd.c: Added the standard GCC header.
+
 2011-09-08  Balaji V. Iyer  
 
* gimplify.c (gimplify_call_expr): Removed if (SPAWN_CALL_P (*expr))
diff --git a/gcc/pragma_simd.c b/gcc/pragma_simd.c
index 6fc6f8e..eafdf99 100644
--- a/gcc/pragma_simd.c
+++ b/gcc/pragma_simd.c
@@ -1,8 +1,25 @@
-/* Routines to handle Pragma SIMD capabilities by the vectorizer
- * Copyright (C) 2011
- * Contributed by Balaji V. Iyer 
- *
- */
+/* This file is part of the Intel(R) Cilk(TM) Plus support
+   This file contains routines to handle PRAGMA SIMD assignments by the
+   vectorizer.
+   Copyright (C) 2011  Free Software Foundation, Inc.
+   Contributed by Balaji V. Iyer ,
+ Intel Corporation
+
+This file is part of GCC.
+
+GCC 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.
+
+GCC 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 GCC; see the file COPYING3.  If not see
+.  */
 
 #include "config.h"
 #include "system.h"

Re: [PATCH][Cilkplus] Patch to fix Template type inside cilk_for

[H.J. Lu]

On Fri, Sep 9, 2011 at 8:37 AM, Iyer, Balaji V  wrote:
> Here is a fixed patch with all the changes you have requested.

diff --git a/gcc/ChangeLog.cilk b/gcc/ChangeLog.cilk
index 8880b0a..299febb 100644
--- a/gcc/ChangeLog.cilk
+++ b/gcc/ChangeLog.cilk
@@ -2,6 +2,9 @@

* gimplify.c (gimplify_call_expr): Removed if (SPAWN_CALL_P (*expr))
statement.
+   * tree.c (walk_tree_1): Added "case CILK_FOR_STMT:".
+   * tree.h (CILK_FOR_VAR): Changed TREE_OPERAND(..., 4) to
+   TREE_OPERAND(..., 5).

Please use a separate ChangeLog entry.

+   * g++.dg/cilk-plus/template_cilk_for_plus_equal.cpp: New.

Likewise.

 2011-09-06  Balaji V. Iyer  

diff --git a/gcc/cp/ChangeLog.cilk b/gcc/cp/ChangeLog.cilk
index b49f3bf..4c54dc6 100644
--- a/gcc/cp/ChangeLog.cilk
+++ b/gcc/cp/ChangeLog.cilk
@@ -1,3 +1,10 @@
+2011-09-08  Balaji V. Iyer  
+
+   * cp-tree.h (FOR_SCOPE): Changed FOR_STMT_CHECK to FOR_STMT_CHECK2
+   * cilk.c (check_incr): Added a check for variable entity name match, 
not just
+   var. Removed the assert to check if operand 0 is the variable.
+   (cp_extract_for_fields): Likewise.
+

Please limit to 72 columns.


-- 
H.J.

Re: [PATCH][Cilkplus] Patch to add GCC Standard header

[H.J. Lu]

On Fri, Sep 9, 2011 at 8:47 AM, Iyer, Balaji V  wrote:
> Hello Everyone,
>    This patch is for the Cilk plus branch. This patch will add 
> the standard GCC header into pragma_simd.c file that I created.
>

I checked it in for you.


-- 
H.J.

[patch, arm] Fix PR target/50305 (arm_legitimize_reload_address problem)

[Ulrich Weigand]

Hello,

the problem in PR 50305 turned out to be caused by the ARM back-end
LEGITIMIZE_RELOAD_ADDRESS implementation.

One of the arguments to the inline asm ("+Qo" (perf_event_id)) has
the form
   (mem/c/i:DI (plus:SI (reg/f:SI 152)
(const_int 1200 [0x4b0])) [5 perf_event_id+0 S8 A64])
before reload, where reg 152 holds the section anchor:
(insn 23 21 29 3 (set (reg/f:SI 152)
(symbol_ref:SI ("*.LANCHOR0") [flags 0x182])) pr50305.c:36 176 
{*arm_movsi_insn}
 (expr_list:REG_EQUAL (symbol_ref:SI ("*.LANCHOR0") [flags 0x182])
(nil)))

The displacement is considered out of range for a DImode MEM, and therefore
reload attempts to reload the address.  The ARM LEGITIMIZE_RELOAD_ADDRESS
routine then attempts to optimize this by converting the address to:

(mem/c/i:DI (plus:SI (plus:SI (reg/f:SI 3 r3 [152])
  (const_int 1024 [0x400]))
  (const_int 176 [0xb0])) [5 perf_event_id+0 S8 A64])

and pushing reloads:

Reload 0: reload_in (SI) = (plus:SI (reg/f:SI 3 r3 [152])
(const_int 1024 [0x400]))
CORE_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 2)
reload_in_reg: (plus:SI (reg/f:SI 3 r3 [152])
(const_int 1024 [0x400]))
Reload 1: reload_in (SI) = (plus:SI (reg/f:SI 3 r3 [152])
(const_int 1024 [0x400]))
CORE_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 5)
reload_in_reg: (plus:SI (reg/f:SI 3 r3 [152])
(const_int 1024 [0x400]))
Reload 2: reload_in (SI) = (plus:SI (plus:SI (reg/f:SI 3 r3 [152])
(const_int 1024 
[0x400]))
(const_int 176 [0xb0]))
CORE_REGS, RELOAD_FOR_INPUT (opnum = 2), inc by 8
reload_in_reg: (plus:SI (plus:SI (reg/f:SI 3 r3 [152])
(const_int 1024 
[0x400]))
(const_int 176 [0xb0]))
Reload 3: reload_in (SI) = (plus:SI (plus:SI (reg/f:SI 3 r3 [152])
(const_int 1024 
[0x400]))
(const_int 176 [0xb0]))
CORE_REGS, RELOAD_FOR_INPUT (opnum = 5), inc by 8
reload_in_reg: (plus:SI (plus:SI (reg/f:SI 3 r3 [152])
(const_int 1024 
[0x400]))
(const_int 176 [0xb0]))

(Note that the duplicate reloads are because the "+" operand has been
implicitly converted to an input and an output operand.  Reloads 2/3
are there because reload is not sure that the result of 
LEGITIMIZE_RELOAD_ADDRESS
is offsetable, and therefore reloads the whole thing anyway.)

Now the problem is that some other arguments of the asm don't all fit into
registers, and therefore we get a second pass through find_reloads.  At this
point, the insn stream has already been modified, so LEGITIMIZE_RELOAD_ADDRESS
this time around sees the RTL it has itself generated at the first pass.
However, it is not able to recognize this, and therefore doesn't re-generate
the required reloads, so instead generic code attempts to handle the
nested plus, and creates somewhat unfortunate reloads:

Reload 0: reload_in (SI) = (plus:SI (reg/f:SI 3 r3 [152])
(const_int 176 [0xb0]))
CORE_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 2)
reload_in_reg: (plus:SI (reg/f:SI 3 r3 [152])
(const_int 176 [0xb0]))
Reload 1: reload_in (SI) = (plus:SI (reg/f:SI 3 r3 [152])
(const_int 176 [0xb0]))
CORE_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 5)
reload_in_reg: (plus:SI (reg/f:SI 3 r3 [152])
(const_int 176 [0xb0]))
Reload 2: reload_out (SI) = (reg:SI 151 [ tmp ])
GENERAL_REGS, RELOAD_FOR_INSN (opnum = 1)
reload_out_reg: (reg:SI 151 [ tmp ])
Reload 3: reload_in (SI) = (plus:SI (plus:SI (reg/f:SI 3 r3 [152])
(const_int 176 [0xb0]))
(const_int 1024 [0x400]))
CORE_REGS, RELOAD_FOR_INPUT (opnum = 2), inc by 8
reload_in_reg: (plus:SI (plus:SI (reg/f:SI 3 r3 [152])
(const_int 176 [0xb0]))
(const_int 1024 [0x400]))
Reload 4: reload_in (SI) = (plus:SI (plus:SI (reg/f:SI 3 r3 [152])
(const_int 176 [0xb0]))
(const_int 1024 [0x400]))
CORE_RE

[testsuite,committed]: Fix gcc.dg/torture/pr49030.c for int < 32 bits

[Georg-Johann Lay]

This test cases files with


gcc/testsuite/gcc.dg/torture/pr49030.c: In function 'sample_move_d32u24_sS':
gcc/testsuite/gcc.dg/torture/pr49030.c:10:2: warning: overflow in implicit
constant conversion [-Woverflow]

Fixed as obvious, tests pass now for int=16 (tested with avr).

http://gcc.gnu.org/viewcvs?view=revision&revision=178736

Johann

* gcc.dg/torture/pr49030.c: Run only if target int32plus.

 --- trunk/gcc/testsuite/gcc.dg/torture/pr49030.c   2011/09/09 16:12:50 
178735
+++ trunk/gcc/testsuite/gcc.dg/torture/pr49030.c2011/09/09 17:00:26 
178736
@@ -1,3 +1,5 @@
+/* { dg-require-effective-target int32plus } */
+
 void
 sample_move_d32u24_sS (char *dst, float *src, unsigned long nsamples,
   unsigned long dst_skip)

[Patch, Fortran, OOP] PR 47978: Invalid INTENT in overriding TBP not detected

[Janus Weil]

Hi all,

here is another small patch for an accepts-invalid OOP problem: When
overriding a type-bound procedure, we need to check that the intents
of the formal args agree (or more general: their 'characteristics', as
defined in chapter 12.3.2 of the F08 standard). For now I'm only
checking type+rank as well as the INTENT and OPTIONAL attributes, but
I added a FIXME for more comprehensive checking (which could be added
in a follow-up patch).

On the technical side of things, I'm adding a new function
'check_dummy_characteristics', which is called in two places:
 * gfc_compare_interfaces and
 * gfc_check_typebound_override.

A slight subtlety is given by the fact that for the PASS argument, the
type of the argument does not have to agree when overriding.

The improved checking also caught an invalid test case in the
testsuite (dynamic_dispatch_5), for another one the error message
changed slightly (typebound_proc_6).

Regtested on x86_64-unknown-linux-gnu. Ok for trunk?

Cheers,
Janus


2011-09-09  Janus Weil  

PR fortran/47978
* interface.c (check_dummy_characteristics): New function to check the
characteristics of dummy arguments.
(gfc_compare_interfaces,gfc_check_typebound_override): Call it here.


2011-09-09  Janus Weil  

PR fortran/47978
* gfortran.dg/dynamic_dispatch_5.f03: Fix invalid test case.
* gfortran.dg/typebound_proc_6.f03: Changed wording in error message.
* gfortran.dg/typebound_override_1.f90: New.
Index: gcc/testsuite/gfortran.dg/dynamic_dispatch_5.f03
===
--- gcc/testsuite/gfortran.dg/dynamic_dispatch_5.f03	(revision 178722)
+++ gcc/testsuite/gfortran.dg/dynamic_dispatch_5.f03	(working copy)
@@ -56,7 +56,7 @@ module s_base_mat_mod
 contains 
   subroutine s_scals(d,a,info) 
 implicit none 
-class(s_base_sparse_mat), intent(in) :: a
+class(s_base_sparse_mat), intent(inout) :: a
 real(spk_), intent(in)  :: d
 integer, intent(out):: info
 
@@ -73,7 +73,7 @@ contains
 
   subroutine s_scal(d,a,info) 
 implicit none 
-class(s_base_sparse_mat), intent(in) :: a
+class(s_base_sparse_mat), intent(inout) :: a
 real(spk_), intent(in)  :: d(:)
 integer, intent(out):: info
 
Index: gcc/testsuite/gfortran.dg/typebound_proc_6.f03
===
--- gcc/testsuite/gfortran.dg/typebound_proc_6.f03	(revision 178722)
+++ gcc/testsuite/gfortran.dg/typebound_proc_6.f03	(working copy)
@@ -89,7 +89,7 @@ MODULE testmod
 ! For corresponding dummy arguments.
 PROCEDURE, PASS :: corresp1 => proc_tmeint ! Ok.
 PROCEDURE, PASS :: corresp2 => proc_tmeintx ! { dg-error "should be named 'a'" }
-PROCEDURE, PASS :: corresp3 => proc_tmereal ! { dg-error "Types mismatch for dummy argument 'a'" }
+PROCEDURE, PASS :: corresp3 => proc_tmereal ! { dg-error "Type/rank mismatch in argument 'a'" }
 
   END TYPE t
 
Index: gcc/fortran/interface.c
===
--- gcc/fortran/interface.c	(revision 178722)
+++ gcc/fortran/interface.c	(working copy)
@@ -977,6 +977,45 @@ generic_correspondence (gfc_formal_arglist *f1, gf
 }
 
 
+/* Check if the characteristics of two dummy arguments match, cf. F08:12.3.2.  */
+
+static gfc_try
+check_dummy_characteristics (gfc_symbol *s1, gfc_symbol *s2,
+			 bool type_must_agree, char *errmsg, int err_len)
+{
+  /* Check type and rank.  */
+  if (type_must_agree && !compare_type_rank (s2, s1))
+{
+  if (errmsg != NULL)
+	snprintf (errmsg, err_len, "Type/rank mismatch in argument '%s'",
+		  s1->name);
+  return FAILURE;
+}
+
+  /* Check INTENT.  */
+  if (s1->attr.intent != s2->attr.intent)
+{
+  snprintf (errmsg, err_len, "INTENT mismatch in argument '%s'",
+		s1->name);
+  return FAILURE;
+}
+
+  /* Check OPTIONAL.  */
+  if (s1->attr.optional != s2->attr.optional)
+{
+  snprintf (errmsg, err_len, "OPTIONAL mismatch in argument '%s'",
+		s1->name);
+  return FAILURE;
+}
+
+  /* FIXME: Do more comprehensive testing of dummy characteristics,
+	e.g. array shape, string length and attributes like
+	ALLOCATABLE, POINTER, TARGET, etc.  */
+
+  return SUCCESS;
+}
+
+
 /* 'Compare' two formal interfaces associated with a pair of symbols.
We return nonzero if there exists an actual argument list that
would be ambiguous between the two interfaces, zero otherwise.
@@ -1059,31 +1098,22 @@ gfc_compare_interfaces (gfc_symbol *s1, gfc_symbol
 	return 0;
 	  }
 
-	/* Check type and rank.  */
-	if (!compare_type_rank (f2->sym, f1->sym))
+	if (intent_flag)
 	  {
+	/* Check all characteristics.  */
+	if (check_dummy_characteristics (f1->sym, f2->sym,
+	 true, errmsg, err_len) == FAILURE)
+	  return 0;
+	  }
+	else if (!compare_type_rank (f2->sym, f1->sym))
+	  {
+	/* Only check type and rank.

Re: [RFC] Cleanup DW_CFA_GNU_args_size handling

[Eric Botcazou]

> Binutils CVS from this morning:

OK, I see.  For some reasons, this build has:

#define HAVE_GAS_CFI_DIRECTIVE 0

and another build with binutils 2.20 succeeds, but it has:

#define HAVE_GAS_CFI_DIRECTIVE 1


So bootstrap is broken without CFI directives.

-- 
Eric Botcazou

RE: [PATCH][Cilkplus] Patch to fix Template type inside cilk_for

[Iyer, Balaji V]

Ok, fixed all the changes you mentioned. Here is the patch.

Thanks,

Balaji V. Iyer.

-Original Message-
From: H.J. Lu [mailto:hjl.to...@gmail.com] 
Sent: Friday, September 09, 2011 11:54 AM
To: Iyer, Balaji V
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH][Cilkplus] Patch to fix Template type inside cilk_for

On Fri, Sep 9, 2011 at 8:37 AM, Iyer, Balaji V  wrote:
> Here is a fixed patch with all the changes you have requested.

diff --git a/gcc/ChangeLog.cilk b/gcc/ChangeLog.cilk index 8880b0a..299febb 
100644
--- a/gcc/ChangeLog.cilk
+++ b/gcc/ChangeLog.cilk
@@ -2,6 +2,9 @@

* gimplify.c (gimplify_call_expr): Removed if (SPAWN_CALL_P (*expr))
statement.
+   * tree.c (walk_tree_1): Added "case CILK_FOR_STMT:".
+   * tree.h (CILK_FOR_VAR): Changed TREE_OPERAND(..., 4) to
+   TREE_OPERAND(..., 5).

Please use a separate ChangeLog entry.

+   * g++.dg/cilk-plus/template_cilk_for_plus_equal.cpp: New.

Likewise.

 2011-09-06  Balaji V. Iyer  

diff --git a/gcc/cp/ChangeLog.cilk b/gcc/cp/ChangeLog.cilk index 
b49f3bf..4c54dc6 100644
--- a/gcc/cp/ChangeLog.cilk
+++ b/gcc/cp/ChangeLog.cilk
@@ -1,3 +1,10 @@
+2011-09-08  Balaji V. Iyer  
+
+   * cp-tree.h (FOR_SCOPE): Changed FOR_STMT_CHECK to FOR_STMT_CHECK2
+   * cilk.c (check_incr): Added a check for variable entity name match, 
not just
+   var. Removed the assert to check if operand 0 is the variable.
+   (cp_extract_for_fields): Likewise.
+

Please limit to 72 columns.


--
H.J.
diff --git a/gcc/ChangeLog.cilk b/gcc/ChangeLog.cilk
index 8880b0a..aadf5da 100644
--- a/gcc/ChangeLog.cilk
+++ b/gcc/ChangeLog.cilk
@@ -1,7 +1,13 @@
+2011-09-09  Balaji V. Iyer  
+
+   * tree.c (walk_tree_1): Added "case CILK_FOR_STMT:".
+   * tree.h (CILK_FOR_VAR): Changed TREE_OPERAND(..., 4) to
+   TREE_OPERAND(..., 5).
+
 2011-09-08  Balaji V. Iyer  
 
-   * gimplify.c (gimplify_call_expr): Removed if (SPAWN_CALL_P (*expr))
-   statement.
+   * gimplify.c (gimplify_call_expr): Removed if 
+   (SPAWN_CALL_P (*expr)) statement.
 
 2011-09-06  Balaji V. Iyer  
 
diff --git a/gcc/cp/ChangeLog.cilk b/gcc/cp/ChangeLog.cilk
index b49f3bf..9e58fcd 100644
--- a/gcc/cp/ChangeLog.cilk
+++ b/gcc/cp/ChangeLog.cilk
@@ -1,3 +1,12 @@
+2011-09-08  Balaji V. Iyer  
+
+   * cp-tree.h (FOR_SCOPE): Changed FOR_STMT_CHECK to 
+   FOR_STMT_CHECK2
+   * cilk.c (check_incr): Added a check for variable entity name 
+   match, not just var. Removed the assert to check if operand 0 
+   is the variable.
+   (cp_extract_for_fields): Likewise.
+
 2011-09-07  Balaji V. Iyer  
 
* parser.c (cp_parser_jump_statement): Removed "IN_CILK_FOR | " from
diff --git a/gcc/cp/cilk.c b/gcc/cp/cilk.c
index 139ec27..49af1d7 100644
--- a/gcc/cp/cilk.c
+++ b/gcc/cp/cilk.c
@@ -1024,17 +1024,18 @@ check_incr(tree var, tree arith_type, tree incr)
   if (TREE_CODE (incr) == MODIFY_EXPR)
 {
   modify = true;
-  if (TREE_OPERAND (incr, 0) != var)
+  if (TREE_OPERAND (incr, 0) != var
+ && DECL_NAME (TREE_OPERAND (incr, 0)) != DECL_NAME (var))
{
  error("Cilk for increment does not modify the loop variable.\n");
  return false;
}
   incr = TREE_OPERAND (incr, 1);
   incr_code = TREE_CODE (incr);
-  gcc_assert (TREE_OPERAND (incr, 0) == var);
 
 }
-  else if (TREE_OPERAND (incr, 0) != var)
+  else if (TREE_OPERAND (incr, 0) != var
+  && DECL_NAME (TREE_OPERAND (incr, 0)) != DECL_NAME (var))
 {
   error ("Cilk for increment does not modify the loop variable.");
   return false;
@@ -2589,7 +2590,6 @@ cp_extract_for_fields (struct cilk_for_desc *cfd, tree 
for_stmt)
 case MODIFY_EXPR:
   /* We don't get here unless the expression has the form
 (modify var (op var incr)) */
-  gcc_assert (TREE_OPERAND (incr, 0) == var);
   incr = TREE_OPERAND (incr, 1);
   /* again, should have checked form of increment earlier */
   if (TREE_CODE (incr) == PLUS_EXPR)
@@ -2597,9 +2597,13 @@ cp_extract_for_fields (struct cilk_for_desc *cfd, tree 
for_stmt)
  tree op0 = TREE_OPERAND (incr, 0);
  tree op1 = TREE_OPERAND (incr, 1);
 
- if (op0 == var)
+ /* if op0 is a pointer, then we should make sure the original 
+variable also works (e.g. if we declared as *i, then i++ is 
+acceptable) 
+  */
+ if (op0 == var || DECL_NAME (op0) == DECL_NAME (var))
incr = op1;
- else if (op1 == var)
+ else if (op1 == var || DECL_NAME (op1) == DECL_NAME (var))
incr = op0;
  else
gcc_unreachable ();
@@ -2637,8 +2641,17 @@ cp_extract_for_fields (struct cilk_for_desc *cfd, tree 
for_stmt)
  tree op0 = TREE_OPERAND (incr, 0);
  tree op1 = TREE_OPERAND (incr, 1);
 
- gcc_assert (op0 == var);
- incr = op1;
+ /* if op0 is a pointer, then we should make sure the original 

Re: [RFC] Cleanup DW_CFA_GNU_args_size handling

[H.J. Lu]

On Fri, Sep 9, 2011 at 11:15 AM, Eric Botcazou  wrote:
>> Binutils CVS from this morning:
>
> OK, I see.  For some reasons, this build has:
>
> #define HAVE_GAS_CFI_DIRECTIVE 0
>
> and another build with binutils 2.20 succeeds, but it has:
>
> #define HAVE_GAS_CFI_DIRECTIVE 1
>

I have no problems with binutils in CVS today.  That
may be another --with-as/--with-ld issue similar to

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

-- 
H.J.

Re: [PATCH][Cilkplus] Patch to fix Template type inside cilk_for

[Jakub Jelinek]

On Fri, Sep 09, 2011 at 11:56:29AM -0700, Iyer, Balaji V wrote:
> diff --git a/gcc/ChangeLog.cilk b/gcc/ChangeLog.cilk index 8880b0a..299febb 
> 100644
> --- a/gcc/ChangeLog.cilk
> +++ b/gcc/ChangeLog.cilk
> @@ -2,6 +2,9 @@
> 
>   * gimplify.c (gimplify_call_expr): Removed if (SPAWN_CALL_P (*expr))
>   statement.
> + * tree.c (walk_tree_1): Added "case CILK_FOR_STMT:".
> + * tree.h (CILK_FOR_VAR): Changed TREE_OPERAND(..., 4) to
> + TREE_OPERAND(..., 5).

The above is still not correct ChangeLog, you are quoting the actual code
way too much.   E.g. the second could be:
* tree.c (walk_tree_1): Handle CILK_FOR_STMT.
third maybe:
* tree.c (CILK_FOR_VAR): Use 5 instead of 4 as last TREE_OPERAND
argument.
The first one should describe what kind of code you've actually removed,
Don't handle this or that.
etc.

> +2011-09-08  Balaji V. Iyer  
> +
> + * cp-tree.h (FOR_SCOPE): Changed FOR_STMT_CHECK to FOR_STMT_CHECK2

All ChangeLog entries end with a dot.

> + * cilk.c (check_incr): Added a check for variable entity name match, 
> not just
> + var. Removed the assert to check if operand 0 is the variable.

Two spaces after . in between sentences.

Jakub

Re: [PATCH][Cilkplus] Patch to fix Template type inside cilk_for

[H.J. Lu]

On Fri, Sep 9, 2011 at 11:56 AM, Iyer, Balaji V  wrote:
> Ok, fixed all the changes you mentioned. Here is the patch.
>
> Thanks,
>

Please provide a patch against the current branch since your patch
won't apply.

-   * gimplify.c (gimplify_call_expr): Removed if (SPAWN_CALL_P (*expr))
-   statement.
+   * gimplify.c (gimplify_call_expr): Removed if
+   (SPAWN_CALL_P (*expr)) statement.

Please use a separate patch to change existing ChangeLog entries.

+case CILK_FOR_STMT:
+  {
+   WALK_SUBTREE (CILK_FOR_INIT (*tp));
+   WALK_SUBTREE (FOR_COND (*tp));
+   WALK_SUBTREE (FOR_EXPR (*tp));
+   WALK_SUBTREE (FOR_BODY (*tp));
+   WALK_SUBTREE (CILK_FOR_GRAIN (*tp));
+   WALK_SUBTREE (CILK_FOR_VAR (*tp));
+  }
+  break;
+

Please remove extra {}.


-- 
H.J.

Fwd: [cxx-mem-model] Don't over process __sync_mem parameters.

[Andrew MacLeod]

 This patch implements a reduced type massaging for the new 
'__sync_mem' built-in functions, as well as verifying the number of 
parameters are correct.


The gory details from the wiki TODO :

   The parameters are massaged in
   c-family/c-common.c::resolve_overloaded_builtin,
   sync_resolve_return, and sync_resolve_params.

   Previous to this, all the __sync were of the format T __Sync (T *,
   T, T, ...) so these 3 routines figured out what type T was and cast
   the return and all parameters to T and then to the type of the
   builtins's argument, usually BT_I{1,2,4,8,16}, which is an unsigned
   value. ie if it the builtin type was 1 byte, BT_I1, we'd see
   something like (BT_I1)(char)actual_param where BT_I1 maps to
   type_for_size(1), which I think is unsigned char.

   The new __sync_mem routines are similar, but the memory model
   parameter is a BT_INT. This means that currently the memory model
   parameter on a call for 16 bytes values would be cast to an
   __int128_t, and then back to an int. Which is quite ugly and silly.

   The right thing would be to change these routines to look at all the
   arguments and only do these casts when the underlying type of the
   builtin argument is the same size as the real base (T) (picked up
   from the pointer in parameter 1). Since the memory_model argument is
   a BT_INT, we'll only get the cast on the memory model parameter when
   it's size matches T (either long or int).. and then its a harmless cast.

   Extra parameters can be thrown on the end as well and no errors are
   reported, this dates back to the variadic versions the IA64 stuff
   required. the new routines should complain if there are extra
   parameters.

   The reason for all this casting is to prevent a bunch of compiler
   warnings when passing in pointers for compare and swap and to avoid
   signed/unsigned conversions issues which may cause miscompilations.

   And now there is a bug uncovered. If type T is a bool for instance,
   the memory_model parameter will be cast to type T, which means it
   will end up being either a 0 or a 1, which is very very bad.  This
   caused a failure in one of the follow on patches when
   __sync_mem_exchange(&bool, bool, memmodel) is called.

A new testcase for checking the number of parameters on a sample call is 
also added.  Bootstraps on x86_64-unknown-linux-gnu and causes no new 
regression.


OK for branch?
Andrew




* c-family/c-common.c (sync_resolve_params, sync_resolve_return): Only
tweak parameters that are the same type size.
(resolve_overloaded_builtin): Use new param for __sync_mem builtins.

* testsuite/gcc.dg/sync-mem-param.c: New testcase to check correct
number of parameters on a sample __sync_mem builtin.


Index: c-family/c-common.c
===
*** c-family/c-common.c (revision 178734)
--- c-family/c-common.c (working copy)
*** sync_resolve_size (tree function, VEC(tr
*** 9015,9021 
 was encountered; true on success.  */
  
  static bool
! sync_resolve_params (tree orig_function, tree function, VEC(tree, gc) *params)
  {
function_args_iterator iter;
tree ptype;
--- 9015,9022 
 was encountered; true on success.  */
  
  static bool
! sync_resolve_params (tree orig_function, tree function, VEC(tree, gc) *params,
!bool orig_format)
  {
function_args_iterator iter;
tree ptype;
*** sync_resolve_params (tree orig_function,
*** 9047,9063 
  return false;
}
  
!   /* ??? Ideally for the first conversion we'd use convert_for_assignment
!so that we get warnings for anything that doesn't match the pointer
!type.  This isn't portable across the C and C++ front ends atm.  */
!   val = VEC_index (tree, params, parmnum);
!   val = convert (ptype, val);
!   val = convert (arg_type, val);
!   VEC_replace (tree, params, parmnum, val);
  
function_args_iter_next (&iter);
  }
  
/* The definition of these primitives is variadic, with the remaining
   being "an optional list of variables protected by the memory barrier".
   No clue what that's supposed to mean, precisely, but we consider all
--- 9048,9077 
  return false;
}
  
!   /* Only convert parameters if the size is appropriate with new format
!sync routines.  */
!   if (orig_format ||
!   tree_int_cst_equal (TYPE_SIZE (ptype), TYPE_SIZE (arg_type)))
!   {
! /* Ideally for the first conversion we'd use convert_for_assignment
!so that we get warnings for anything that doesn't match the pointer
!type.  This isn't portable across the C and C++ front ends atm.  */
! val = VEC_index (tree, params, parmnum);
! val = convert (ptype, val);
! val = convert (arg_type, val);
! VEC_replace (tree, params, parmnum, val);
!   }
 

[lra] Fix ppc64 bootstrap failure

[Vladimir Makarov]

The following patch fixes compiler crash on stage3 of bootstrap.  The 
patch was successfully bootstrapped on x86/x86-64 and ppc64.


2011-09-09  Vladimir Makarov 

* lra-constraints.c (equivalence_change_p): Rename to
debug_loc_equivalence_change_p.  Process subreg of reg whose
equivalence mode is unknown.


Index: lra-constraints.c
===
--- lra-constraints.c   (revision 178707)
+++ lra-constraints.c   (working copy)
@@ -3113,17 +3113,30 @@ contains_pseudo_p (rtx x, bool spilled_p
   return false;
 }
 
-/* Process all regs in *LOC and change them on equivalent
-   substitution.  Return true if any change was done.  */
+/* Process all regs in debug location *LOC and change them on
+   equivalent substitution.  Return true if any change was done.  */
 static bool
-equivalence_change_p (rtx *loc)
+debug_loc_equivalence_change_p (rtx *loc)
 {
-  rtx subst, x = *loc;
+  rtx subst, reg, x = *loc;
   bool result = false;
   enum rtx_code code = GET_CODE (x);
   const char *fmt;
   int i, j;
 
+  if (code == SUBREG)
+{
+  reg = SUBREG_REG (x);
+  if ((subst = get_equiv_substitution (reg)) != reg
+ && GET_MODE (subst) == VOIDmode)
+   {
+ /* We cannot reload debug location.  Simplify subreg here
+while we know the inner mode.  */
+ *loc = simplify_gen_subreg (GET_MODE (x), subst,
+ GET_MODE (reg), SUBREG_BYTE (x));
+ return true;
+   }
+}
   if (code == REG && (subst = get_equiv_substitution (x)) != x)
 {
   *loc = subst;
@@ -3135,10 +3148,11 @@ equivalence_change_p (rtx *loc)
   for (i = GET_RTX_LENGTH (code) - 1; i >= 0; i--)
 {
   if (fmt[i] == 'e')
-   result = equivalence_change_p (&XEXP (x, i)) || result;
+   result = debug_loc_equivalence_change_p (&XEXP (x, i)) || result;
   else if (fmt[i] == 'E')
for (j = XVECLEN (x, i) - 1; j >= 0; j--)
- result = equivalence_change_p (&XVECEXP (x, i, j)) || result;
+ result
+   = debug_loc_equivalence_change_p (&XVECEXP (x, i, j)) || result;
 }
   return result;
 }
@@ -3237,7 +3251,7 @@ lra_constraints (bool first_p)
  /* We need to check equivalence in debug insn and change
 pseudo to the equivalent value if necessary.  */
  curr_id = lra_get_insn_recog_data (curr_insn);
- if (equivalence_change_p (curr_id->operand_loc[0]))
+ if (debug_loc_equivalence_change_p (curr_id->operand_loc[0]))
changed_p = true;
}
   else if (INSN_P (curr_insn))

[cxx-mem-model] Some consolidation.

[Andrew MacLeod]

I wasn't liking the duplication between the old __sync routines and the 
new __sync_mem routines. I thought it was going to add confusion to port 
maintainers at the very least.


This patch consolidates everything except the various fetch_op routines, 
which I will do in a different patch since they are much more complicated,


The basic form now is that the new routine expansion code follow this 
pattern:


__sync_mem_ROUTINE:
 - Check for existence of a pattern for __sync_mem_ROUTINE, and try to 
use that.

 - Absorb the code from the expansion routine for __sync_ROUTINE.
 - See if that pattern exists and can be used, issuing any 
additional fences that may be needed.
 - fall back to the compare and swap loop, or execute the insn with 
fences as appropriate.


And now the original __sync routines will simply call the expanders for 
the new routines passing in the memory model they actually intend.


__sync_ROUTINE:
   call expand __sync_mem_ROUTINE (..., DOCUMENTED_MEMORY_MODEL)

ie, so  __sync_lock_test_and_set is defined to be acquire mode, so now 
any use of that older built-in will actually end up being expanded by 
this call:

   expand_sync_mem_exchange (target, mem, val, MEMMODEL_ACQUIRE);
rather than the original call to expand_sync_test_and_set.

Any slight variances in behaviour which need to be absorbed are 
documented, such as __sync_lock_test_and_set may only use values of 0 
and 1 on some limited architectures.   With that functionality 
consolidated, I have removed the __sync_mem_flag_{test_and_set,clear} 
routines since their functionality is now completed by invoking 
__sync_mem_exchange (byte_addr, 1) and __sync_mem_store (byte_addr, 0).



Its all much cleaner, and now it much easier to maintain a target. All 
you need to do is take the original patterns for the old __sync 
routines, rename them and add any memory model support you want. no 
confusing duplication of functionality and all that.


the various fetch_ops will be consolidated in a followup next week.

Bootstraps on x86_64-unknown-linux-gnu and causes no new regressions.

Andrew


* expr.h: Remove prototypes.
* sync-builtins.def (BUILT_IN_SYNC_MEM_FLAG_TEST_AND_SET,
BUILT_IN_SYNC_MEM_FLAG_CLEAR): Remove.
* optabs.h (enum direct_optab_index): Remove DOI_sync_mem_flag_*.
* optabs.c (expand_sync_lock_test_and_set): Remove.
(expand_sync_mem_exchange): Incorporate sync_lock_test_and_set here.
(expand_sync_mem_store): If storing const0_rtx, try using
sync_lock_release.
* builtins.c (expand_builtin_sync_lock_test_and_set): Expand into
sync_mem_exchange instead.
(expand_builtin_sync_lock_release): Expand into sync_mem_store of 0.
(expand_builtin_sync_mem_flag_test_and_set): Remove.
(expand_builtin_sync_mem_flag_clear): Remove.
(expand_builtin): Remove cases for __SYNC_MEM_FLAG_*.
* doc/extend.texi: Update documentation to match.

* testsuite/gcc.dg/sync-mem-invalid.c: Remove __sync_mem_flag_clear
tests.
* testsuite/gcc.dg/sync-mem-flag.c: Remove.


Index: expr.h
===
*** expr.h  (revision 178710)
--- expr.h  (working copy)
*** rtx expand_val_compare_and_swap (rtx, rt
*** 216,222 
  rtx expand_bool_compare_and_swap (rtx, rtx, rtx, rtx);
  rtx expand_sync_operation (rtx, rtx, enum rtx_code);
  rtx expand_sync_fetch_operation (rtx, rtx, enum rtx_code, bool, rtx);
- rtx expand_sync_lock_test_and_set (rtx, rtx, rtx);
  
  rtx expand_sync_mem_exchange (rtx, rtx, rtx, enum memmodel);
  rtx expand_sync_mem_compare_exchange (rtx, rtx, rtx, rtx, enum memmodel, 
--- 216,221 
*** rtx expand_sync_mem_load (rtx, rtx, enum
*** 225,232 
  void expand_sync_mem_store (rtx, rtx, enum memmodel);
  rtx expand_sync_mem_fetch_op (rtx, rtx, rtx, enum rtx_code, enum memmodel, 
  bool);
- rtx expand_sync_mem_flag_test_and_set (rtx, rtx, enum memmodel);
- void expand_sync_mem_flag_clear (rtx, enum memmodel);
  void expand_sync_mem_thread_fence (enum memmodel);
  void expand_sync_mem_signal_fence (enum memmodel);
  
--- 224,229 
Index: sync-builtins.def
===
*** sync-builtins.def   (revision 178710)
--- sync-builtins.def   (working copy)
*** DEF_SYNC_BUILTIN (BUILT_IN_SYNC_MEM_FETC
*** 533,546 
  "__sync_mem_fetch_or_16",
  BT_FN_I16_VPTR_I16_INT, ATTR_NOTHROW_LEAF_LIST)
  
- DEF_SYNC_BUILTIN (BUILT_IN_SYNC_MEM_FLAG_TEST_AND_SET,
- "__sync_mem_flag_test_and_set",
- BT_FN_BOOL_VPTR_INT, ATTR_NOTHROW_LEAF_LIST)
- 
- DEF_SYNC_BUILTIN (BUILT_IN_SYNC_MEM_FLAG_CLEAR,
- "__sync_mem_flag_clear",
- BT_FN_VOID_VPTR_INT, ATTR_NOTHROW_LEAF_LIST)
- 
  DEF_SYNC_BUILTIN (BUILT_IN_SYNC_MEM_THREAD_FENCE,
  

[cxx-mem-model] C++ wrappers

[Andrew MacLeod]

Same as before, only a slight change.  __sync_mem_flag_test_and_set() 
and __sync_mem_flag_clear() have been removed in a different 
consolidation patch, so this changes those calls to 
__sync_mem_exchange() and __sync_mem_store() which are subsuming all the 
behaviour of those routines.


Straight up translation which now calls the new routines with a memory 
model parameter instead of the old __sync routines with various barriers.


bootstrapped on x86_64-unknown-linux-gnu and no new regressions.

OK for branch?

* libstdc++-v3/include/bits/atomic_2.h (__atomic2): Use new
__sync_mem routines.


Index: include/bits/atomic_2.h
===
*** include/bits/atomic_2.h (revision 178710)
--- include/bits/atomic_2.h (working copy)
*** namespace __atomic2
*** 60,78 
  bool
  test_and_set(memory_order __m = memory_order_seq_cst) noexcept
  {
!   // Redundant synchronize if built-in for lock is a full barrier.
!   if (__m != memory_order_acquire && __m != memory_order_acq_rel)
!   __sync_synchronize();
!   return __sync_lock_test_and_set(&_M_i, 1);
  }
  
  bool
  test_and_set(memory_order __m = memory_order_seq_cst) volatile noexcept
  {
!   // Redundant synchronize if built-in for lock is a full barrier.
!   if (__m != memory_order_acquire && __m != memory_order_acq_rel)
!   __sync_synchronize();
!   return __sync_lock_test_and_set(&_M_i, 1);
  }
  
  void
--- 60,72 
  bool
  test_and_set(memory_order __m = memory_order_seq_cst) noexcept
  {
!   return __sync_mem_exchange (&_M_i, 1, __m);
  }
  
  bool
  test_and_set(memory_order __m = memory_order_seq_cst) volatile noexcept
  {
!   return __sync_mem_exchange (&_M_i, 1, __m);
  }
  
  void
*** namespace __atomic2
*** 82,90 
__glibcxx_assert(__m != memory_order_acquire);
__glibcxx_assert(__m != memory_order_acq_rel);
  
!   __sync_lock_release(&_M_i);
!   if (__m != memory_order_acquire && __m != memory_order_acq_rel)
!   __sync_synchronize();
  }
  
  void
--- 76,82 
__glibcxx_assert(__m != memory_order_acquire);
__glibcxx_assert(__m != memory_order_acq_rel);
  
!   __sync_mem_store (&_M_i, 0, __m);
  }
  
  void
*** namespace __atomic2
*** 94,102 
__glibcxx_assert(__m != memory_order_acquire);
__glibcxx_assert(__m != memory_order_acq_rel);
  
!   __sync_lock_release(&_M_i);
!   if (__m != memory_order_acquire && __m != memory_order_acq_rel)
!   __sync_synchronize();
  }
};
  
--- 86,92 
__glibcxx_assert(__m != memory_order_acquire);
__glibcxx_assert(__m != memory_order_acq_rel);
  
!   __sync_mem_store (&_M_i, 0, __m);
  }
};
  
*** namespace __atomic2
*** 180,238 
  
__int_type
operator++() noexcept
!   { return __sync_add_and_fetch(&_M_i, 1); }
  
__int_type
operator++() volatile noexcept
!   { return __sync_add_and_fetch(&_M_i, 1); }
  
__int_type
operator--() noexcept
!   { return __sync_sub_and_fetch(&_M_i, 1); }
  
__int_type
operator--() volatile noexcept
!   { return __sync_sub_and_fetch(&_M_i, 1); }
  
__int_type
operator+=(__int_type __i) noexcept
!   { return __sync_add_and_fetch(&_M_i, __i); }
  
__int_type
operator+=(__int_type __i) volatile noexcept
!   { return __sync_add_and_fetch(&_M_i, __i); }
  
__int_type
operator-=(__int_type __i) noexcept
!   { return __sync_sub_and_fetch(&_M_i, __i); }
  
__int_type
operator-=(__int_type __i) volatile noexcept
!   { return __sync_sub_and_fetch(&_M_i, __i); }
  
__int_type
operator&=(__int_type __i) noexcept
!   { return __sync_and_and_fetch(&_M_i, __i); }
  
__int_type
operator&=(__int_type __i) volatile noexcept
!   { return __sync_and_and_fetch(&_M_i, __i); }
  
__int_type
operator|=(__int_type __i) noexcept
!   { return __sync_or_and_fetch(&_M_i, __i); }
  
__int_type
operator|=(__int_type __i) volatile noexcept
!   { return __sync_or_and_fetch(&_M_i, __i); }
  
__int_type
operator^=(__int_type __i) noexcept
!   { return __sync_xor_and_fetch(&_M_i, __i); }
  
__int_type
operator^=(__int_type __i) volatile noexcept
!   { return __sync_xor_and_fetch(&_M_i, __i); }
  
bool
is_lock_free() const noexcept
--- 170,228 
  
__int_type
operator++() noexcept
!   { return __sync_mem_add_fetch(&_M_i, 1, memory_order_seq_cst); }
  
__int_type
operator++() volatile noexcept
!   { return __sync_mem_add__fetch(&_M_i, 1, memory_order_seq_cst); }
  
__int_type
operator--() noexcept
!

[pph] Fix method lookups (part 1) (issue4997042)

[Diego Novillo]

The main problem fixed here is that name lookups for class methods
uses a binary search that assumes that the methods in
CLASSTYPE_METHOD_VEC are sorted by pointer value.

Since the reader typically allocates trees in a different pattern than
the writer, it is common for the symbols in the vector to have
different pointer values, so the order used by the writer is different
than the reader.

This was causing us to fail name lookups when generating the pph image
for x6dynarray5.h and x6dynarray6.h.

To fix this, I am making finish_struct_methods an extern function and
calling it after reading a type that has a CLASSTYPE_METHOD_VEC.

This exposed another failure that was simple enough to roll in
together with this patch.  We should not emit preloaded symbols when
writing the names in the global namespace.  This was causing a
DECL_CHAIN cycle.  I added a new filter PPHF_NO_PREFS to skip the
preloaded symbols when needed.

There is another failure exposed by this patch that I am going to fix
separately.  It goes something like this:

File decl.h:
---
class X
{
void foo(int);
};
---

File impl.h:
---
#include "decl.h"
void X::foo(int x) { return x + 1; }
---
  
We generate pph images for decl.h and impl.h.  The problem begins when
we are generating impl.pph.  When the time comes to write impl.pph's
symbol table we go to write X::foo() and find it inside the pickle
cache of decl.pph (since we are including it).  So, instead of
pickling it again, we write an external reference to the pickled
representation in decl.pph.

The problem here is that the pickled representation in decl.pph
contains the DECLARATION for X::foo().  When we parsed impl.h, we
overwrote the DECL node to convert it into the IMPLEMENTATION of
X::foo().

So, when reading impl.pph from a translation unit, we never read the
implementation for X::foo, which confuses the compiler and produces an
ICE.

I will implement a way of detecting and replaying these in-place
modifications in my next patch.

Tested on x86_64.  Committed to pph.


Diego.

cp/ChangeLog.pph

* class.c (finish_struct_methods): Make extern.
* cp-tree.h (finish_struct_methdods): Declare.
* pph-streamer-in.c (pph_in_tcc_type): Call it.
* pph-streamer.h (PPHF_NO_PREFS): Define.
* pph-streamer-out.c (pph_tree_matches): Use it to filter trees
present in the preloaded cache.
(pph_out_scope_chain): Call pph_out_binding_level_1 with
PPH_NO_PREFS in the filter.

testsuite/ChangeLog.pph

* g++.dg/pph/x6dynarray3.cc: Change failure from ICE to asm diff.
* g++.dg/pph/x6dynarray4.cc: Change expected failure.
* g++.dg/pph/x6dynarray5.h: Mark fixed.
* g++.dg/pph/x6dynarray6.h: Mark fixed.
* g++.dg/pph/x7dynarray5.cc: Change expected failure.

diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index 4e3a58f..0a63c78 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -127,7 +127,6 @@ static void handle_using_decl (tree, tree);
 static tree dfs_modify_vtables (tree, void *);
 static tree modify_all_vtables (tree, tree);
 static void determine_primary_bases (tree);
-static void finish_struct_methods (tree);
 static void maybe_warn_about_overly_private_class (tree);
 static int method_name_cmp (const void *, const void *);
 static int resort_method_name_cmp (const void *, const void *);
@@ -1793,7 +1792,7 @@ resort_type_method_vec (void* obj,
and type conversion operators) so that we can find them faster in
search.  */
 
-static void
+void
 finish_struct_methods (tree t)
 {
   tree fn_fields;
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 3da77a7..6cd52aa 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -4117,6 +4117,8 @@ extern int current_class_depth;
 /* An array of all local classes present in this translation unit, in
declaration order.  */
 extern GTY(()) VEC(tree,gc) *local_classes;
+
+void finish_struct_methods (tree);
 
 /* Here's where we control how name mangling takes place.  */
 
diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c
index 2fcb436..b111850 100644
--- a/gcc/cp/pph-streamer-in.c
+++ b/gcc/cp/pph-streamer-in.c
@@ -1750,6 +1750,15 @@ pph_in_tcc_type (pph_stream *stream, tree type)
 default:
   break;
 }
+
+  /* If TYPE has a METHOD_VEC, we need to resort it.  Name lookup in
+ classes relies on the specific ordering of the class method
+ pointers.  Since we generally instantiate them in a different
+ order than the original compile, the pointer values will be
+ different.  This will cause name lookups to fail, unless we
+ resort the vector.  */
+  if (TYPE_LANG_SPECIFIC (type) && CLASSTYPE_METHOD_VEC (type))
+finish_stru

[pph] Do not read pph files more than once (issue4983055)

[Diego Novillo]

This was not causing any failures, but it is pretty wasteful to read
the same PPH more than once.

We cannot just skip them, however.  We need to read the line table to
properly modify the line table for the current translation unit.

Tested on x86_64.  Committed to branch.


Diego.


* pph-streamer-in.c (pph_image_already_read): New.
(pph_read_file_1): Call pph_image_already_read.  If it returns
true, return after reading the line table.
(pph_add_read_image): New.
(pph_read_file): Change return value to void.  Update all callers.
Call pph_add_read_image.
* pph-streamer-out.c (pph_add_include): Remove second argument.
Update all callers.
Do not add INCLUDE to pph_read_images.
* pph-streamer.h (pph_add_include): Update prototype.

diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c
index b111850..ea44460 100644
--- a/gcc/cp/pph-streamer-in.c
+++ b/gcc/cp/pph-streamer-in.c
@@ -1462,7 +1462,6 @@ pph_in_include (pph_stream *stream)
 {
   int old_loc_offset;
   const char *include_name;
-  pph_stream *include;
   source_location prev_start_loc = pph_in_source_location (stream);
 
   /* Simulate highest_location to be as it would be at this point in a non-pph
@@ -1475,8 +1474,7 @@ pph_in_include (pph_stream *stream)
   old_loc_offset = pph_loc_offset;
 
   include_name = pph_in_string (stream);
-  include = pph_read_file (include_name);
-  pph_add_include (include, false);
+  pph_read_file (include_name);
 
   pph_loc_offset = old_loc_offset;
 }
@@ -1583,6 +1581,23 @@ pph_in_line_table_and_includes (pph_stream *stream)
 }
 
 
+/* If FILENAME has already been read, return the stream associated with it.  */
+
+static pph_stream *
+pph_image_already_read (const char *filename)
+{
+  pph_stream *include;
+  unsigned i;
+
+  /* FIXME pph, implement a hash map to avoid this linear search.  */
+  FOR_EACH_VEC_ELT (pph_stream_ptr, pph_read_images, i, include)
+if (strcmp (include->name, filename) == 0)
+  return include;
+
+  return NULL;
+}
+
+
 /* Helper for pph_read_file.  Read contents of PPH file in STREAM.  */
 
 static void
@@ -1605,6 +1620,11 @@ pph_read_file_1 (pph_stream *stream)
  read.  */
   cpp_token_replay_loc = pph_in_line_table_and_includes (stream);
 
+  /* If we have read STREAM before, we do not need to re-read the rest
+ of its body.  We only needed to read its line table.  */
+  if (pph_image_already_read (stream->name))
+return;
+
   /* Read all the identifiers and pre-processor symbols in the global
  namespace.  */
   pph_in_identifiers (stream, &idents_used);
@@ -1650,13 +1670,22 @@ pph_read_file_1 (pph_stream *stream)
  STREAM will need to be read again the next time we want to read
  the image we are now generating.  */
   if (pph_out_file && !pph_reading_includes)
-pph_add_include (stream, true);
+pph_add_include (stream);
+}
+
+
+/* Add STREAM to the list of read images.  */
+
+static void
+pph_add_read_image (pph_stream *stream)
+{
+  VEC_safe_push (pph_stream_ptr, heap, pph_read_images, stream);
 }
 
 
 /* Read PPH file FILENAME.  Return the in-memory pph_stream instance.  */
 
-pph_stream *
+void
 pph_read_file (const char *filename)
 {
   pph_stream *stream;
@@ -1667,7 +1696,7 @@ pph_read_file (const char *filename)
   else
 error ("Cannot open PPH file for reading: %s: %m", filename);
 
-  return stream;
+  pph_add_read_image (stream);
 }
 
 
diff --git a/gcc/cp/pph-streamer-out.c b/gcc/cp/pph-streamer-out.c
index 264294c..1a32814 100644
--- a/gcc/cp/pph-streamer-out.c
+++ b/gcc/cp/pph-streamer-out.c
@@ -1845,18 +1845,13 @@ pph_add_decl_to_symtab (tree decl, enum 
pph_symtab_action action,
 }
 
 
-/* Add INCLUDE to the list of files included by  the main pph_out_stream
-   if IS_MAIN_STREAM_INCLUDE, as well as to the global list of all read
-   includes.  */
+/* Add INCLUDE to the list of files included by pph_out_stream.  */
 
 void
-pph_add_include (pph_stream *include, bool is_main_stream_include)
+pph_add_include (pph_stream *include)
 {
-  if (is_main_stream_include)
-VEC_safe_push (pph_stream_ptr, heap,
-  pph_out_stream->encoder.w.includes, include);
-
-  VEC_safe_push (pph_stream_ptr, heap, pph_read_images, include);
+  VEC_safe_push (pph_stream_ptr, heap, pph_out_stream->encoder.w.includes,
+include);
 }
 
 
diff --git a/gcc/cp/pph-streamer.h b/gcc/cp/pph-streamer.h
index 7205d64..7f98764 100644
--- a/gcc/cp/pph-streamer.h
+++ b/gcc/cp/pph-streamer.h
@@ -255,7 +255,7 @@ void pph_flush_buffers (pph_stream *);
 void pph_init_write (pph_stream *);
 void pph_write_tree (struct output_block *, tree, bool);
 void pph_add_decl_to_symtab (tree, enum pph_symtab_action, bool, bool);
-void pph_add_include (pph_stream *, bool);
+void pph_add_include (pph_stream *);
 void pph_writer_init (void);
 void pph_writer_finish (void);
 void pph_write_location (struct output_block *, location_t);
@@ -269,7 +269,7 @@ struct bindi

RE: [PATCH][Cilkplus] Patch to fix Template type inside cilk_for

[Iyer, Balaji V]

Hello H.J.,
I think I have fixed all the changes you and Jakub have requested in 
the patch.

Thanks,

Balaji V. Iyer.

-Original Message-
From: H.J. Lu [mailto:hjl.to...@gmail.com] 
Sent: Friday, September 09, 2011 3:10 PM
To: Iyer, Balaji V
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH][Cilkplus] Patch to fix Template type inside cilk_for

On Fri, Sep 9, 2011 at 11:56 AM, Iyer, Balaji V  wrote:
> Ok, fixed all the changes you mentioned. Here is the patch.
>
> Thanks,
>

Please provide a patch against the current branch since your patch won't apply.

-   * gimplify.c (gimplify_call_expr): Removed if (SPAWN_CALL_P (*expr))
-   statement.
+   * gimplify.c (gimplify_call_expr): Removed if
+   (SPAWN_CALL_P (*expr)) statement.

Please use a separate patch to change existing ChangeLog entries.

+case CILK_FOR_STMT:
+  {
+   WALK_SUBTREE (CILK_FOR_INIT (*tp));
+   WALK_SUBTREE (FOR_COND (*tp));
+   WALK_SUBTREE (FOR_EXPR (*tp));
+   WALK_SUBTREE (FOR_BODY (*tp));
+   WALK_SUBTREE (CILK_FOR_GRAIN (*tp));
+   WALK_SUBTREE (CILK_FOR_VAR (*tp));
+  }
+  break;
+

Please remove extra {}.


--
H.J.
diff --git a/gcc/ChangeLog.cilk b/gcc/ChangeLog.cilk
index c59f38f..364704a 100644
--- a/gcc/ChangeLog.cilk
+++ b/gcc/ChangeLog.cilk
@@ -1,6 +1,9 @@
 2011-09-09  Balaji V. Iyer  
 
* pragma_simd.c: Added the standard GCC header.
+   * tree.c (walk_tree_1): Handled the "cilk_for" case.
+   * tree.h (CILK_FOR_VAR): Use 5 instead of 4 as TREE_OPERAND
+   argument.
 
 2011-09-08  Balaji V. Iyer  
 
diff --git a/gcc/cp/ChangeLog.cilk b/gcc/cp/ChangeLog.cilk
index b49f3bf..0c9e1bd 100644
--- a/gcc/cp/ChangeLog.cilk
+++ b/gcc/cp/ChangeLog.cilk
@@ -1,3 +1,12 @@
+2011-09-08  Balaji V. Iyer  
+
+   * cp-tree.h (FOR_SCOPE): Changed FOR_STMT_CHECK to
+   FOR_STMT_CHECK2.
+   * cilk.c (check_incr): Added a check for variable entity name
+   match, not just var.  Removed the assert to check if 1st
+   operand is the variable.
+   (cp_extract_for_fields): Likewise.
+
 2011-09-07  Balaji V. Iyer  
 
* parser.c (cp_parser_jump_statement): Removed "IN_CILK_FOR | " from
diff --git a/gcc/cp/cilk.c b/gcc/cp/cilk.c
index 139ec27..375e8b6 100644
--- a/gcc/cp/cilk.c
+++ b/gcc/cp/cilk.c
@@ -1024,17 +1024,18 @@ check_incr(tree var, tree arith_type, tree incr)
   if (TREE_CODE (incr) == MODIFY_EXPR)
 {
   modify = true;
-  if (TREE_OPERAND (incr, 0) != var)
+  if (TREE_OPERAND (incr, 0) != var
+ && DECL_NAME (TREE_OPERAND (incr, 0)) != DECL_NAME (var))
{
  error("Cilk for increment does not modify the loop variable.\n");
  return false;
}
   incr = TREE_OPERAND (incr, 1);
   incr_code = TREE_CODE (incr);
-  gcc_assert (TREE_OPERAND (incr, 0) == var);
 
 }
-  else if (TREE_OPERAND (incr, 0) != var)
+  else if (TREE_OPERAND (incr, 0) != var
+  && DECL_NAME (TREE_OPERAND (incr, 0)) != DECL_NAME (var))
 {
   error ("Cilk for increment does not modify the loop variable.");
   return false;
@@ -2589,7 +2590,6 @@ cp_extract_for_fields (struct cilk_for_desc *cfd, tree 
for_stmt)
 case MODIFY_EXPR:
   /* We don't get here unless the expression has the form
 (modify var (op var incr)) */
-  gcc_assert (TREE_OPERAND (incr, 0) == var);
   incr = TREE_OPERAND (incr, 1);
   /* again, should have checked form of increment earlier */
   if (TREE_CODE (incr) == PLUS_EXPR)
@@ -2597,9 +2597,9 @@ cp_extract_for_fields (struct cilk_for_desc *cfd, tree 
for_stmt)
  tree op0 = TREE_OPERAND (incr, 0);
  tree op1 = TREE_OPERAND (incr, 1);
 
- if (op0 == var)
+ if (op0 == var || DECL_NAME (op0) == DECL_NAME (var))
incr = op1;
- else if (op1 == var)
+ else if (op1 == var || DECL_NAME (op1) == DECL_NAME (var))
incr = op0;
  else
gcc_unreachable ();
@@ -2637,8 +2637,13 @@ cp_extract_for_fields (struct cilk_for_desc *cfd, tree 
for_stmt)
  tree op0 = TREE_OPERAND (incr, 0);
  tree op1 = TREE_OPERAND (incr, 1);
 
- gcc_assert (op0 == var);
- incr = op1;
+ if (op0 == var || DECL_NAME (op0) == DECL_NAME (var))
+   incr = op1;
+ else if (op1 == var || DECL_NAME (op1) == DECL_NAME(var))
+   incr = op0;
+ else
+   gcc_unreachable();
+ 
  /* Store the amount to be subtracted.
 Negating it could overflow. */
  negate_incr = true;
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index f33b7f4..a924b73 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -3874,7 +3874,7 @@ more_aggr_init_expr_args_p (const 
aggr_init_expr_arg_iterator *iter)
condition, update expression, and body of the for statement,
respectively.  */
 /* bviyer: we need it in C, so I have defined them in tree.h */
-#define FOR_SCOPE(NODE)TR

Re: [PATCH][Cilkplus] Patch to fix Template type inside cilk_for

[H.J. Lu]

On Fri, Sep 9, 2011 at 5:02 PM, Iyer, Balaji V  wrote:
> Hello H.J.,
>        I think I have fixed all the changes you and Jakub have requested in 
> the patch.
>
> Thanks,
>

I checked it in for you.


-- 
H.J.

Go patch committed: Fix package name as composite literal struct key

[Ian Lance Taylor]

This patch to the Go frontend fixes using a package name as a key in a
struct composite literal.  Bootstrapped and ran Go testsuite on
x86_64-unknown-linux-gnu.  Committed to mainline.

Ian

diff -r c266c495ca06 go/expressions.cc
--- a/go/expressions.cc	Mon Aug 29 15:04:11 2011 -0700
+++ b/go/expressions.cc	Fri Sep 09 21:08:58 2011 -0700
@@ -1436,7 +1436,6 @@
 Expression*
 Expression::make_unknown_reference(Named_object* no, source_location location)
 {
-  go_assert(no->resolve()->is_unknown());
   return new Unknown_expression(no, location);
 }
 
diff -r c266c495ca06 go/parse.cc
--- a/go/parse.cc	Mon Aug 29 15:04:11 2011 -0700
+++ b/go/parse.cc	Fri Sep 09 21:08:58 2011 -0700
@@ -2445,7 +2445,9 @@
 // LiteralValue  = "{" [ ElementList [ "," ] ] "}" .
 // ElementList   = Element { "," Element } .
 // Element   = [ Key ":" ] Value .
-// Key   = Expression .
+// Key   = FieldName | ElementIndex .
+// FieldName = identifier .
+// ElementIndex  = Expression .
 // Value = Expression | LiteralValue .
 
 // We have already seen the type if there is one, and we are now
@@ -2478,7 +2480,33 @@
 
   const Token* token = this->peek_token();
 
-  if (!token->is_op(OPERATOR_LCURLY))
+  if (token->is_identifier())
+	{
+	  std::string identifier = token->identifier();
+	  bool is_exported = token->is_identifier_exported();
+	  source_location location = token->location();
+
+	  if (this->advance_token()->is_op(OPERATOR_COLON))
+	{
+	  // This may be a field name.  We don't know for sure--it
+	  // could also be an expression for an array index.  We
+	  // don't want to parse it as an expression because may
+	  // trigger various errors, e.g., if this identifier
+	  // happens to be the name of a package.
+	  Gogo* gogo = this->gogo_;
+	  val = this->id_to_expression(gogo->pack_hidden_name(identifier,
+  is_exported),
+	   location);
+	}
+	  else
+	{
+	  this->unget_token(Token::make_identifier_token(identifier,
+			 is_exported,
+			 location));
+	  val = this->expression(PRECEDENCE_NORMAL, false, true, NULL);
+	}
+	}
+  else if (!token->is_op(OPERATOR_LCURLY))
 	val = this->expression(PRECEDENCE_NORMAL, false, true, NULL);
   else
 	{
@@ -2922,6 +2950,12 @@
   return Expression::make_func_reference(named_object, NULL, location);
 case Named_object::NAMED_OBJECT_UNKNOWN:
   return Expression::make_unknown_reference(named_object, location);
+case Named_object::NAMED_OBJECT_PACKAGE:
+case Named_object::NAMED_OBJECT_TYPE:
+case Named_object::NAMED_OBJECT_TYPE_DECLARATION:
+  // These cases can arise for a field name in a composite
+  // literal.
+  return Expression::make_unknown_reference(named_object, location);
 default:
   error_at(this->location(), "unexpected type of identifier");
   return Expression::make_error(location);