Re: [PATCH] Don't include DECL_UIDs in tree-sra fancy names for -fdump-final-insns= (PR c++/70594)

2016-04-13 Thread Richard Biener
On Tue, 12 Apr 2016, Jakub Jelinek wrote:

> On Tue, Apr 12, 2016 at 08:19:57PM +0200, Richard Biener wrote:
> > >So you mean add another hash table that maps DECL_UIDs to these SRA
> > >counters?  Because if we dump there any number of say FIELD_DECL, it
> > >would
> > >be desirable to use the same number on any further fancy names with
> > >that
> > >FIELD_DECL.
> > >Do it unconditionally, or just for flag_dump_final_insns?
> > 
> > I'd have just added sth to the dumps so you can manually connect the XXX in 
> > the name with the UID.
> 
> But the names appear in all the gimple, ipa? and rtl dumps after that, so
> if it e.g. just printed a SRA uid -> DECL uid mapping in tree-sra*-details
> dump, then one would not have it easily accessible when looking all the
> other dumps.

Sure - but does it really matter?  Usually decls also have names.

> If it is just guarded with flag_dump_final_insns != NULL, then we have the
> previous behavior almost all the time, just for -fcompare-debug effectively
> force -nouid flag into all the dumps for the fancy names.  But it is easy
> to just remove that flag from the command line to have more details, IMHO
> that is better than just looking up some mapping dump somewhere.

I hate to change behavior of passes based on unrelated flags.  
Over-engineering things just in case you need to debug sth is also bad.

So if you think it's not acceptable to drop the relation between
the artificial number used by SRA and the original UID then go
with a hash-map unconditionally.  You still have to dump and lookup
the actual relation though - there's no way around this unless you
hack dump-final-insns to filter DECL names (maybe just make it
strip all DECL_ARTIFICIAL names completely?)

Richard.

-- 
Richard Biener 
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)


Re: [C++ Patch] PR 70635 ([4.9/5/6 Regression] ICE on (and rejects) valid code...)

2016-04-13 Thread Paolo Carlini

Hi,

On 12/04/2016 15:50, Jason Merrill wrote:

On 04/12/2016 09:25 AM, Paolo Carlini wrote:

in this regression we have an infinite recursion affecting the
same_type_p call at parser.c:25125 which I added in the patch for
c++/38313. The issue is that for, eg, the testcase at issue, we are
passing a TYPENAME_TYPE to same_type_p. I think we can simply handle the
problem by checking first that we have a CLASS_TYPE_P as TREE_TYPE
(type_decl). Tested x86_64-linux.


I don't see how this can be considered valid code; the typenames are 
indeed mutually recursive, with no real underlying type anywhere.  
clang and EDG both reject the testcase if the template is 
instantiated. Please add an instantiation to the testcase.  And we 
should fix the recursion issue in structural_comptypes.
Ok. But then, if I understand correctly, our user-visible behavior 
should not change much: accept the testcase (modulo the -fpermissive 
detail) without an instantiation and reject it when the template is 
actually instantiated. In other terms, exactly what EDG and clang also 
do. Thus same_type_p should be fixed to simply return false in such 
recursive cases without ICEing and nothing more, right? I'm not sure I 
will be able to do that in time for 6.0 but I'm going to try...


Thanks,
Paolo.


Re: [PATCH] Clear TREE_STATIC on vector ctors if needed (PR middle-end/70633)

2016-04-13 Thread Richard Biener
On Tue, 12 Apr 2016, Jakub Jelinek wrote:

> Hi!
> 
> As the testcase shows, gimplification of the VECTOR_TYPE CONSTRUCTOR
> elements can turn previously valid initializer_constant_valid_p
> into a temporary (thus no longer initializer_constant_valid_p).
> 
> The following patch just clears TREE_STATIC on the ctor if that happens,
> so that we expand it properly.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> Or should we instead check initializer_constant_valid_p first and not
> gimplify the initializer in that case if the ctor is TREE_STATIC?
> I'm just afraid that the optimization passes would be upset to see
> more complex elements in the initializers, and it is something people don't
> really write in real-world anyway.

I think GIMPLE doesn't really expect TREE_STATIC constructors in the IL
but if expansion is better in this case we can keep it as with your
patch (nothing in GIMPLE optimizations should make a previously
valid TREE_STATIC constructor invalid ...).

In an ideal world all "static" constructors would be forced to
CONST_DECLs (and we would maintain an IPA constant pool in the
cgraph code).

But I wonder why expansion doesn't simply "re-compute" TREE_STATIC
here.  That is, do we rely on TREE_STATIC constructors for correctness
purposes as set by frontends?  Or is this a "optimization" that is now
(historically) done very too much early?  For example 
recompute_constructor_flags doesn't re-compute TREE_STATIC - what
does set it in the first place?  verify_constructor_flags also doesn't
verify it.

Looks like the C/C++ FEs do that.

Patch is ok.

Thanks,
Richard.

> 2016-04-12  Jakub Jelinek  
> 
>   PR middle-end/70633
>   * gimplify.c (gimplify_init_constructor): Clear TREE_STATIC if
>   gimplification turns some element into non-constant.
> 
>   * gcc.c-torture/compile/pr70633.c: New test.
> 
> --- gcc/gimplify.c.jj 2016-04-09 13:21:09.0 +0200
> +++ gcc/gimplify.c2016-04-12 16:50:17.271533881 +0200
> @@ -4164,7 +4164,7 @@ gimplify_init_constructor (tree *expr_p,
> }
>  
>   /* Vector types use CONSTRUCTOR all the way through gimple
> -   compilation as a general initializer.  */
> +compilation as a general initializer.  */
>   FOR_EACH_VEC_SAFE_ELT (elts, ix, ce)
> {
>   enum gimplify_status tret;
> @@ -4172,6 +4172,10 @@ gimplify_init_constructor (tree *expr_p,
> fb_rvalue);
>   if (tret == GS_ERROR)
> ret = GS_ERROR;
> + else if (TREE_STATIC (ctor)
> +  && !initializer_constant_valid_p (ce->value,
> +TREE_TYPE (ce->value)))
> +   TREE_STATIC (ctor) = 0;
> }
>   if (!is_gimple_reg (TREE_OPERAND (*expr_p, 0)))
> TREE_OPERAND (*expr_p, 1) = get_formal_tmp_var (ctor, pre_p);
> --- gcc/testsuite/gcc.c-torture/compile/pr70633.c.jj  2016-04-12 
> 17:24:40.494491310 +0200
> +++ gcc/testsuite/gcc.c-torture/compile/pr70633.c 2016-04-12 
> 17:24:23.0 +0200
> @@ -0,0 +1,12 @@
> +/* PR middle-end/70633 */
> +
> +typedef long V __attribute__((vector_size (4 * sizeof (long;
> +
> +void foo (V *);
> +
> +void
> +bar (void)
> +{ 
> +  V b = { (long) bar, 0, 0, 0 };
> +  foo (&b);
> +}
> 
>   Jakub
> 
> 

-- 
Richard Biener 
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)


Re: [PATCH] Clear TREE_STATIC on vector ctors if needed (PR middle-end/70633)

2016-04-13 Thread Jakub Jelinek
On Wed, Apr 13, 2016 at 09:50:29AM +0200, Richard Biener wrote:
> But I wonder why expansion doesn't simply "re-compute" TREE_STATIC
> here.  That is, do we rely on TREE_STATIC constructors for correctness
> purposes as set by frontends?

Clearly we do rely on it.  If we did not rely on it, would the flag be
useful for anything?

Jakub


Re: [PATCH] Fix debug ICE on aarch64 due to bad rtl simplification (PR debug/70628)

2016-04-13 Thread Andreas Schwab
Jakub Jelinek  writes:

> So here is what I'll throw at testing:
>
> 2016-04-12  Jakub Jelinek  
>
>   PR debug/70628
>   * rtl.h (convert_memory_address_addr_space_1): New prototype.
>   * explow.c (convert_memory_address_addr_space_1): No longer static,
>   add NO_EMIT argument and don't call convert_modes if true, pass
>   it down recursively, remove break after return.
>   (convert_memory_address_addr_space): Adjust caller.
>   * simplify-rtx.c (simplify_unary_operation_1): Call
>   convert_memory_address_addr_space_1 instead of convert_memory_address,
>   if it returns NULL, don't simplify.

I have successfully bootstrapped it on ILP32 and verified that it fixes
the ICE.

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."


Re: [PATCH] Remove dead code in omp-low.c

2016-04-13 Thread Thomas Schwinge
Hi!

Ping Andi's patch; I just found the same thing:

On Sun, 14 Dec 2014 11:34:18 -0800, Andi Kleen  wrote:
> Since the accelerator changes omp_expand_local is not used anymore.

Specifically,

and thereabouts.  As far as I can tell, Tom's reason to not "remove
omp_expand_local, since [he's] still using that in [his] oacc kernels
directive patch series" no longer applies.

> Remove the dead function and another function used by it.
> 
> gcc/:
> 
> 2014-12-14  Andi Kleen  
> 
>   * omp-low.c (build_omp_regions_root): Remove dead function.
>   (omp_expand_local): Dito.
>   * omp-low.h: Dito.
> ---
>  gcc/omp-low.c | 30 --
>  gcc/omp-low.h |  1 -
>  2 files changed, 31 deletions(-)
> 
> diff --git a/gcc/omp-low.c b/gcc/omp-low.c
> index a2e4737..908d280 100644
> --- a/gcc/omp-low.c
> +++ b/gcc/omp-low.c
> @@ -8753,36 +8753,6 @@ build_omp_regions_1 (basic_block bb, struct omp_region 
> *parent,
>  build_omp_regions_1 (son, parent, single_tree);
>  }
>  
> -/* Builds the tree of OMP regions rooted at ROOT, storing it to
> -   root_omp_region.  */
> -
> -static void
> -build_omp_regions_root (basic_block root)
> -{
> -  gcc_assert (root_omp_region == NULL);
> -  build_omp_regions_1 (root, NULL, true);
> -  gcc_assert (root_omp_region != NULL);
> -}
> -
> -/* Expands omp construct (and its subconstructs) starting in HEAD.  */
> -
> -void
> -omp_expand_local (basic_block head)
> -{
> -  build_omp_regions_root (head);
> -  if (dump_file && (dump_flags & TDF_DETAILS))
> -{
> -  fprintf (dump_file, "\nOMP region tree\n\n");
> -  dump_omp_region (dump_file, root_omp_region, 0);
> -  fprintf (dump_file, "\n");
> -}
> -
> -  remove_exit_barriers (root_omp_region);
> -  expand_omp (root_omp_region);
> -
> -  free_omp_regions ();
> -}
> -
>  /* Scan the CFG and build a tree of OMP regions.  Return the root of
> the OMP region tree.  */
>  
> diff --git a/gcc/omp-low.h b/gcc/omp-low.h
> index ac587d0..718e707 100644
> --- a/gcc/omp-low.h
> +++ b/gcc/omp-low.h
> @@ -23,7 +23,6 @@ along with GCC; see the file COPYING3.  If not see
>  struct omp_region;
>  
>  extern tree find_omp_clause (tree, enum omp_clause_code);
> -extern void omp_expand_local (basic_block);
>  extern void free_omp_regions (void);
>  extern tree omp_reduction_init (tree, tree);
>  extern bool make_gimple_omp_edges (basic_block, struct omp_region **, int *);


Grüße
 Thomas


signature.asc
Description: PGP signature


[testsuite] gcc-dg: handle all return values when shouldfail is set

2016-04-13 Thread Christophe Lyon
Hi,

While investigating stability problems when running GCC validations,
I fixed DejaGnu to properly handle cases where it cannot parse the
testcase output:
http://lists.gnu.org/archive/html/dejagnu/2016-04/msg8.html

This means that such cases now return "unresolved" which confuses
${tool}_load in gcc-dg.exp, as it currently only handles "pass" and "fail".

The attached small patch fixes this.

This is probably for stage 1 only I guess.

OK?

Christophe.
2016-04-13  Christophe Lyon  

* lib/gcc-dg.exp (${tool}_load): Add default return value handler.
diff --git a/gcc/testsuite/lib/gcc-dg.exp b/gcc/testsuite/lib/gcc-dg.exp
index 9e4ecce..6cdce0d 100644
--- a/gcc/testsuite/lib/gcc-dg.exp
+++ b/gcc/testsuite/lib/gcc-dg.exp
@@ -380,6 +380,7 @@ if { [info procs ${tool}_load] != [list] \
switch [lindex $result 0] {
"pass" { set status "fail" }
"fail" { set status "pass" }
+   default { set status [lindex $result 0] }
}
set result [list $status [lindex $result 1]]
}


[PATCH] Some tree-vect-data-refs.c cleanup

2016-04-13 Thread Robin Dapp
Hi,

the attached patch is a start at cleaning up tree-vect-data-refs.c.
I refactored some stuff I found strange and/or difficult to understand.
Probably more to come.

The misalignment calculation in vect_compute_data_ref_alignment seems
quite coarse, is there any work planned for the future?

No regressions on s390x and amd64.

Regards
 Robin

--

gcc/ChangeLog:

2016-04-13  Robin Dapp  

* tree-vectorizer.h
(dr_misalignment): Introduce named DR_MISALIGNMENT constants.
(aligned_access_p): Use constants.
(known_alignment_for_access_p): Likewise.
* tree-vect-data-refs.c
(vect_compute_data_ref_alignment):
Improve output messages and cleanup.
(vect_duplicate_ssa_name_ptr_info): Use new constants.
(vect_enhance_data_refs_alignment): Likewise.
(vect_supportable_dr_alignment): Likewise.
(vect_update_misalignment_for_peel): Likewise.

gcc/testsuite/ChangeLog:

2016-04-13  Robin Dapp  

* gcc.dg/vect/vect-outer-3a-big-array.c: Use new output message.
* gcc.dg/vect/vect-outer-3a.c: Likewise.
diff --git a/gcc/testsuite/gcc.dg/vect/vect-outer-3a-big-array.c b/gcc/testsuite/gcc.dg/vect/vect-outer-3a-big-array.c
index 1316e80..d9bfcdf 100644
--- a/gcc/testsuite/gcc.dg/vect/vect-outer-3a-big-array.c
+++ b/gcc/testsuite/gcc.dg/vect/vect-outer-3a-big-array.c
@@ -49,4 +49,4 @@ int main (void)
 }
 
 /* { dg-final { scan-tree-dump-times "OUTER LOOP VECTORIZED" 1 "vect" { xfail { vect_no_align && { ! vect_hw_misalign } } } } } */
-/* { dg-final { scan-tree-dump-times "step doesn't divide the vector-size" 1 "vect" } } */
+/* { dg-final { scan-tree-dump-times "step is not a multiple of the vector size" 1 "vect" } } */
diff --git a/gcc/testsuite/gcc.dg/vect/vect-outer-3a.c b/gcc/testsuite/gcc.dg/vect/vect-outer-3a.c
index 6a7ab91..7030a64 100644
--- a/gcc/testsuite/gcc.dg/vect/vect-outer-3a.c
+++ b/gcc/testsuite/gcc.dg/vect/vect-outer-3a.c
@@ -49,4 +49,4 @@ int main (void)
 }
 
 /* { dg-final { scan-tree-dump-times "OUTER LOOP VECTORIZED" 1 "vect" { xfail { vect_no_align && { ! vect_hw_misalign } } } } } */
-/* { dg-final { scan-tree-dump-times "step doesn't divide the vector-size" 1 "vect" } } */
+/* { dg-final { scan-tree-dump-times "step is not a multiple of the vector size" 1 "vect" } } */
diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c
index f8695b1..3c9cef4 100644
--- a/gcc/tree-vect-data-refs.c
+++ b/gcc/tree-vect-data-refs.c
@@ -689,10 +689,9 @@ vect_compute_data_ref_alignment (struct data_reference *dr)
   struct loop *loop = NULL;
   tree ref = DR_REF (dr);
   tree vectype;
-  tree base, base_addr;
-  tree misalign = NULL_TREE;
-  tree aligned_to;
-  unsigned HOST_WIDE_INT alignment;
+  tree base_addr;
+  tree misalign;
+  tree dr_alignment;
 
   if (dump_enabled_p ())
 dump_printf_loc (MSG_NOTE, vect_location,
@@ -702,11 +701,13 @@ vect_compute_data_ref_alignment (struct data_reference *dr)
 loop = LOOP_VINFO_LOOP (loop_vinfo);
 
   /* Initialize misalignment to unknown.  */
-  SET_DR_MISALIGNMENT (dr, -1);
+  SET_DR_MISALIGNMENT (dr, DR_MISALIGNMENT_UNKNOWN);
 
-  if (tree_fits_shwi_p (DR_STEP (dr)))
+  misalign = NULL_TREE;
+  tree step = DR_STEP (dr);
+  if (tree_fits_shwi_p (step))
 misalign = DR_INIT (dr);
-  aligned_to = DR_ALIGNED_TO (dr);
+  dr_alignment = DR_ALIGNED_TO (dr);
   base_addr = DR_BASE_ADDRESS (dr);
   vectype = STMT_VINFO_VECTYPE (stmt_info);
 
@@ -714,27 +715,25 @@ vect_compute_data_ref_alignment (struct data_reference *dr)
  vectorized (LOOP), we use the base and misalignment information
  relative to the outer-loop (LOOP).  This is ok only if the misalignment
  stays the same throughout the execution of the inner-loop, which is why
- we have to check that the stride of the dataref in the inner-loop evenly
- divides by the vector size.  */
+ we have to check that the stride of the dataref in the inner-loop is a
+ multiple of the vector size.  */
   if (loop && nested_in_vect_loop_p (loop, stmt))
 {
-  tree step = DR_STEP (dr);
-
   if (tree_fits_shwi_p (step)
 	  && tree_to_shwi (step) % GET_MODE_SIZE (TYPE_MODE (vectype)) == 0)
 {
   if (dump_enabled_p ())
 dump_printf_loc (MSG_NOTE, vect_location,
- "inner step divides the vector-size.\n");
+ "inner step is a multiple of the vector size.\n");
 	  misalign = STMT_VINFO_DR_INIT (stmt_info);
-	  aligned_to = STMT_VINFO_DR_ALIGNED_TO (stmt_info);
+	  dr_alignment = STMT_VINFO_DR_ALIGNED_TO (stmt_info);
 	  base_addr = STMT_VINFO_DR_BASE_ADDRESS (stmt_info);
 }
   else
 	{
 	  if (dump_enabled_p ())
 	dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
-	 "inner step doesn't divide the vector-size.\n");
+	 "inner step is not a multiple of the vector size.\n");
 	  misalign = NULL_TREE;
 	}
 }
@@ -742,10 +741,9 @@ vect_co

Re: [PATCH] Revert r234572 (aka PR testsuite/70577)

2016-04-13 Thread Jan Hubicka
> On April 12, 2016 2:12:14 PM GMT+02:00, Jan Hubicka  wrote:
> >> Hello.
> >> 
> >> As release managers agreed on IRC, following patch reverts r234572
> >> which introduced new PR testsuite/70577.
> >> 
> >> I've been running bootstrap & regression tests on x86_64-linux-gnu.
> >> Ready to be installed after it finishes?
> >
> >OK, thanks for the revert. 
> >I looked into the prefetch testcase and I think it only needs updating
> >becasue
> >all arrays are trailing. I will try to debug the galgel issue.
> 
> I think we do not want prefetching for the trailing array of size 5.  This is 
> what the testcase tests.  I think we do want prefetching for a trailing array 
> of size 1000.  Thus I think the previous behavior of estimated niter was 
> better.

Just becasue array has size 100 we can't assume that the loop will most
likely 100 times.  This is what happens in the go solver that motivated me
for this patch: there are 10 nested loops where each iterates at most 10 times,
but most of time they iterate once or twice (10^10 is simply too much).
Setting estimated niter to 10 makes us to trade setup cost for iteration cost
way too much.

I see that the heuristics worked here bit by accident.  We can assume
trailing arrays to have size 0 or 1 of they aer really such and we can
work out how the slot is allocated and rely on object size, which we
don't at the moment (I think).

In order to enable loop peeling by default next stage1, I extended niter to
also collect likely upper bound. I use it in this path and also in some other
cases. For example I predict

int a[3];
for (i=0;cond;i++)
  if (cond2)
 a[i]=1;

to likely iterest at most 3 times which makes us to peel the loop 3 times but
keep the rolled loop as fallback.  That patch also sets upper bounds for those
trailing arrays of odd sizes.

The likely upper bounds can then often be used in places we use estimates and
upper bounds (such as to trottle down unrolling/vectorizing/ivopts) and to
drive peeling.

Honza
> 
> Richard.
> 
> >Honza
> 


Re: [PATCH] Clear TREE_STATIC on vector ctors if needed (PR middle-end/70633)

2016-04-13 Thread Richard Biener
On Wed, 13 Apr 2016, Jakub Jelinek wrote:

> On Wed, Apr 13, 2016 at 09:50:29AM +0200, Richard Biener wrote:
> > But I wonder why expansion doesn't simply "re-compute" TREE_STATIC
> > here.  That is, do we rely on TREE_STATIC constructors for correctness
> > purposes as set by frontends?
> 
> Clearly we do rely on it.  If we did not rely on it, would the flag be
> useful for anything?

Sure, if it is merely a hint that an optimization is possible.  If so
we can compute it where we perform that optimization (if that's in
a single place).

Richard.


[PATCH] "Fix" PR70623

2016-04-13 Thread Richard Biener

The following happens to fix the testcase in PR70623 while in reality
I was only cleaning up code, adding comments when I figured that
the worklist handling is a) obfuscated and b) visits nodes
unnecessarily.  b) may be the reason for this bug as the cruical
difference appears in the second iteration where we have

-ANTIC_OUT[13] := { _9 (0006) }
-ANTIC_IN[13] := { _9 (0006) }
-S[13] := { _9 (0006) }
-ANTIC_OUT[6] := { _9 (0006) }
-[changed] ANTIC_IN[6] := { _9 (0006), 
{mem_ref<0B>,addr_expr<&nm>}@.MEM_7(D) (0007) }
-S[6] := { _9 (0006) }
-ANTIC_OUT[14] := { _9 (0006) }
-ANTIC_IN[14] := { _9 (0006) }
-S[14] := { _9 (0006) }

thus ANTIC_IN for 6 changes because we visit it at an unfortunate
point.  The change in iteration order with the patch is

@@ -23,18 +23,12 @@
 18
 20
 11
-19
 10
-12
-15
 7
 5
 4
 16
 8
-13
-6
-14
 3
 21
 2
@@ -45,5480 +39,4 @@
 12
 15
 7
-5
... (the patched variant has finished now)

we _do_ visit 6 again, at line 36, so it's cruical we don't pick up
intermittend state from antic iteration 0 and thus skip computing antic in
for 6 during iteration 1.

I'm not 100% sure this isn't really an issue with the iteration order
and constraints on how the DF problem can converge.  But at least the
patch also improves compile-time.

Bootstrap and regtest running on x86_64_unknown-linux-gnu.

Richard.

2016-04-13  Richard Biener  

PR tree-optimization/70623
* tree-ssa-pre.c (changed_blocks): Make global ...
(compute_antic): ... local here.  Move and fix worklist
handling here.  Do not clear EDGE_DFS_BACK or call mark_dfs_back_edges.
(compute_antic_aux): Add dumping for MAX assumed succs.  Remove
worklist handling, dump when ANTIC_IN changed.
(compute_partial_antic_aux): Remove worklist handling.
(init_pre): Do not compute post dominators.  Add a comment about
the CFG order chosen.
(fini_pre): Do not free post dominators.

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

Index: gcc/tree-ssa-pre.c
===
*** gcc/tree-ssa-pre.c  (revision 234894)
--- gcc/tree-ssa-pre.c  (working copy)
*** prune_clobbered_mems (bitmap_set_t set,
*** 2062,2072 
  
  static sbitmap has_abnormal_preds;
  
- /* List of blocks that may have changed during ANTIC computation and
-thus need to be iterated over.  */
- 
- static sbitmap changed_blocks;
- 
  /* Compute the ANTIC set for BLOCK.
  
 If succs(BLOCK) > 1 then
--- 2078,2083 
*** compute_antic_aux (basic_block block, bo
*** 2125,2130 
--- 2136,2151 
first = e->dest;
  else if (BB_VISITED (e->dest))
worklist.quick_push (e->dest);
+ else
+   {
+ /* Unvisited successors get their ANTIC_IN replaced by the
+maximal set to arrive at a maximum ANTIC_IN solution.
+We can ignore them in the intersection operation and thus
+need not explicitely represent that maximum solution.  */
+ if (dump_file && (dump_flags & TDF_DETAILS))
+   fprintf (dump_file, "ANTIC_IN is MAX on %d->%d\n",
+e->src->index, e->dest->index);
+   }
}
  
/* Of multiple successors we have to have visited one already
*** compute_antic_aux (basic_block block, bo
*** 2167,2180 
clean (ANTIC_IN (block));
  
if (!bitmap_set_equal (old, ANTIC_IN (block)))
! {
!   changed = true;
!   bitmap_set_bit (changed_blocks, block->index);
!   FOR_EACH_EDGE (e, ei, block->preds)
!   bitmap_set_bit (changed_blocks, e->src->index);
! }
!   else
! bitmap_clear_bit (changed_blocks, block->index);
  
   maybe_dump_sets:
if (dump_file && (dump_flags & TDF_DETAILS))
--- 2188,2194 
clean (ANTIC_IN (block));
  
if (!bitmap_set_equal (old, ANTIC_IN (block)))
! changed = true;
  
   maybe_dump_sets:
if (dump_file && (dump_flags & TDF_DETAILS))
*** compute_antic_aux (basic_block block, bo
*** 2182,2187 
--- 2196,2203 
if (ANTIC_OUT)
print_bitmap_set (dump_file, ANTIC_OUT, "ANTIC_OUT", block->index);
  
+   if (changed)
+   fprintf (dump_file, "[changed] ");
print_bitmap_set (dump_file, ANTIC_IN (block), "ANTIC_IN",
block->index);
  
*** compute_partial_antic_aux (basic_block b
*** 2313,2326 
dependent_clean (PA_IN (block), ANTIC_IN (block));
  
if (!bitmap_set_equal (old_PA_IN, PA_IN (block)))
! {
!   changed = true;
!   bitmap_set_bit (changed_blocks, block->index);
!   FOR_EACH_EDGE (e, ei, block->preds)
!   bitmap_set_bit (changed_blocks, e->src->index);
! }
!   else
! bitmap_clear_bit (changed_blocks, block->index);
  
   maybe_dump_sets:
if (dump_file && (dump_flags & TDF_DETAILS))
--- 2329,2335 
dependent_clean (PA_IN (block), ANTIC_IN (block));
  
  

Re: [Patch, testsuite] Require int32plus and scheduling support for some tests

2016-04-13 Thread Senthil Kumar Selvaraj
Ping!

Regards
Senthil

Senthil Kumar Selvaraj writes:

> Hi,
>
>   This patch add dg-require-effective-target directives to a few tests
>   that were failing unnecessarily for the AVR target.
>
>   One of them invokes the compiler with -fschedule-insns2 - I've
>   required scheduling support for that testcase. For all other tests,
>   I've required int32plus - they either use bit shifts wider than
>   16 bits (AVR's int size), or use int constants that are too big
>   for a 16 bit int.
>
>   If ok, could someone commit please? I don't have commit access.
>
> Regards
> Senthil
>
> 2016-04-04  Senthil Kumar Selvaraj  
>
> * gcc.c-torture/compile/pr69102.c: Require scheduling support.
> * gcc.c-torture/compile/pr37669.c: Require >=32 bit integers.
> * gcc.c-torture/execute/bitfld-6.c: Likewise.
> * gcc.c-torture/execute/bitfld-7.c: Likewise.
> * gcc.c-torture/execute/pr38151.c: Likewise.
> * gcc.c-torture/execute/pr66556.c: Likewise.
> * gcc.c-torture/execute/pr67781.c: Likewise.
> * gcc.c-torture/execute/pr68648.c: Likewise.
>
>
> diff --git gcc/testsuite/gcc.c-torture/compile/pr37669.c 
> gcc/testsuite/gcc.c-torture/compile/pr37669.c
> index c78243b..a2eafc7 100644
> --- gcc/testsuite/gcc.c-torture/compile/pr37669.c
> +++ gcc/testsuite/gcc.c-torture/compile/pr37669.c
> @@ -1,5 +1,6 @@
>  /* This testcase used to fail because a miscompiled 
> execute_fold_all_builtins. */
>  /* { dg-options "-fgnu89-inline" } */
> +/* { dg-require-effective-target int32plus } */
>  
>  typedef __SIZE_TYPE__ size_t;
>  extern __inline __attribute__ ((__always_inline__)) int __attribute__
> diff --git gcc/testsuite/gcc.c-torture/compile/pr69102.c 
> gcc/testsuite/gcc.c-torture/compile/pr69102.c
> index b1328ca..1f0cdc6 100644
> --- gcc/testsuite/gcc.c-torture/compile/pr69102.c
> +++ gcc/testsuite/gcc.c-torture/compile/pr69102.c
> @@ -1,4 +1,5 @@
>  /* { dg-options "-Og -fPIC -fschedule-insns2 -fselective-scheduling2 
> -fno-tree-fre --param=max-sched-extend-regions-iters=10" } */
> +/* { dg-require-effective-target scheduling } */
>  void bar (unsigned int);
>  
>  void
> diff --git gcc/testsuite/gcc.c-torture/execute/bitfld-6.c 
> gcc/testsuite/gcc.c-torture/execute/bitfld-6.c
> index 50927dc..b8c5cbd 100644
> --- gcc/testsuite/gcc.c-torture/execute/bitfld-6.c
> +++ gcc/testsuite/gcc.c-torture/execute/bitfld-6.c
> @@ -1,3 +1,4 @@
> +/* { dg-require-effective-target int32plus } */
>  union U
>  {
>const int a;
> diff --git gcc/testsuite/gcc.c-torture/execute/bitfld-7.c 
> gcc/testsuite/gcc.c-torture/execute/bitfld-7.c
> index e9a61df..350e7a3 100644
> --- gcc/testsuite/gcc.c-torture/execute/bitfld-7.c
> +++ gcc/testsuite/gcc.c-torture/execute/bitfld-7.c
> @@ -1,3 +1,4 @@
> +/* { dg-require-effective-target int32plus } */
>  union U
>  {
>const int a;
> diff --git gcc/testsuite/gcc.c-torture/execute/pr38151.c 
> gcc/testsuite/gcc.c-torture/execute/pr38151.c
> index 5ee058d..86c8f77 100644
> --- gcc/testsuite/gcc.c-torture/execute/pr38151.c
> +++ gcc/testsuite/gcc.c-torture/execute/pr38151.c
> @@ -1,4 +1,5 @@
>  /* { dg-options "-Wno-psabi" } */
> +/* { dg-require-effective-target int32plus } */
>  void abort (void);
>  
>  struct S2848
> diff --git gcc/testsuite/gcc.c-torture/execute/pr66556.c 
> gcc/testsuite/gcc.c-torture/execute/pr66556.c
> index f7acf1c..d1259c4 100644
> --- gcc/testsuite/gcc.c-torture/execute/pr66556.c
> +++ gcc/testsuite/gcc.c-torture/execute/pr66556.c
> @@ -1,4 +1,5 @@
>  /* { dg-do run } */
> +/* { dg-require-effective-target int32plus } */
>  
>  extern void abort (void);
>  
> diff --git gcc/testsuite/gcc.c-torture/execute/pr67781.c 
> gcc/testsuite/gcc.c-torture/execute/pr67781.c
> index bf50aa2..71ccd6a 100644
> --- gcc/testsuite/gcc.c-torture/execute/pr67781.c
> +++ gcc/testsuite/gcc.c-torture/execute/pr67781.c
> @@ -1,3 +1,4 @@
> +/* { dg-require-effective-target int32plus } */
>  #ifdef __UINT32_TYPE__
>  typedef __UINT32_TYPE__ uint32_t;
>  #else
> diff --git gcc/testsuite/gcc.c-torture/execute/pr68648.c 
> gcc/testsuite/gcc.c-torture/execute/pr68648.c
> index fc66806..db55bd0 100644
> --- gcc/testsuite/gcc.c-torture/execute/pr68648.c
> +++ gcc/testsuite/gcc.c-torture/execute/pr68648.c
> @@ -1,3 +1,4 @@
> +/* { dg-require-effective-target int32plus } */
>  int __attribute__ ((noinline))
>  foo (void)
>  {



Re: Fix for PR70498 in Libiberty Demangler

2016-04-13 Thread Marcel Böhme
Hi Bernd,

> -static long
> +static int
>  d_compact_number (struct d_info *di)
>  {
> -  long num;
> +  int num;
>if (d_peek_char (di) == '_')
>  num = 0;
>else if (d_peek_char (di) == 'n')
> @@ -2957,7 +2957,7 @@ d_compact_number (struct d_info *di)
>else
>  num = d_number (di) + 1;
> 
> -  if (! d_check_char (di, '_'))
> +  if (num < 0 || ! d_check_char (di, '_'))
>  return -1;
>return num;
>  }

> Shouldn't we check for overflows before performing the +1 addition (i.e. 0 <= 
> num < INT_MAX)? Ideally we'd also have a way to signal from d_number if we 
> had an overflow while parsing that number.
Without an overflow signal, d_number will already be prone to return a negative 
number for supposedly non-negative numbers (those not preceded with ’n’). In 
that case an overflow check would be unnecessary in d_compact_number which is 
supposed to always return a positive number or a negative one (-1). If you 
decide in favour of an overflow signal, it must be handled by the call-sites. 
Not sure what the “default behaviour” should be then. Otherwise, we can simply 
assume that the call sites for d_number can handle negative numbers.

Kindly advice.

> 
> There's also this, in d_expression_1:
> 
> index = d_compact_number (di) + 1;
> if (index == 0)
>   return NULL;
> 
> which probably ought to have the same kind of check (I'll note that at this 
> point we've accumulated two "+1"s, I'll assume that's what we want).
Yes. There should be an overflow check here. 

Thanks!
- Marcel

[PATCH, i386] Fix operands order in kunpck* insns and corresponding expands

2016-04-13 Thread Ilya Enkovich
Hi,

Current kunpck[hi|si|di] patterns emit operands in a wrong order. This
is compensated by a wrong operands order in vec_pack_trunc_[qi|hi|si]
expands and therefore we get correct code for vectorized loops.  Code
using kunpck* intrinsics would be wrong though.  This patch fixes
operands order and adds runtime tests for _mm512_kunpack* intrinsics.

Bootstrapped and regtested on x86_64-pc-linux-gnu.  OK for trunk?

Thanks,
Ilya
--
gcc/

2016-04-13  Ilya Enkovich  

* config/i386/i386.md (kunpckhi): Swap operands.
(kunpcksi): Likewise.
(kunpckdi): Likewise.
* config/i386/sse.md (vec_pack_trunc_qi): Likewise.
(vec_pack_trunc_): Likewise.

gcc/testsuite/

2016-04-13  Ilya Enkovich  

* gcc.target/i386/avx512bw-kunpckdq-2.c: New test.
* gcc.target/i386/avx512bw-kunpckwd-2.c: New test.
* gcc.target/i386/avx512f-kunpckbw-2.c: New test.


diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 09da69e..56a3050 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -8907,7 +8907,7 @@
(const_int 8))
  (zero_extend:HI (match_operand:QI 2 "register_operand" "k"]
   "TARGET_AVX512F"
-  "kunpckbw\t{%1, %2, %0|%0, %2, %1}"
+  "kunpckbw\t{%2, %1, %0|%0, %1, %2}"
   [(set_attr "mode" "HI")
(set_attr "type" "msklog")
(set_attr "prefix" "vex")])
@@ -8920,7 +8920,7 @@
(const_int 16))
  (zero_extend:SI (match_operand:HI 2 "register_operand" "k"]
   "TARGET_AVX512BW"
-  "kunpckwd\t{%1, %2, %0|%0, %2, %1}"
+  "kunpckwd\t{%2, %1, %0|%0, %1, %2}"
   [(set_attr "mode" "SI")])
 
 (define_insn "kunpckdi"
@@ -8931,7 +8931,7 @@
(const_int 32))
  (zero_extend:DI (match_operand:SI 2 "register_operand" "k"]
   "TARGET_AVX512BW"
-  "kunpckdq\t{%1, %2, %0|%0, %2, %1}"
+  "kunpckdq\t{%2, %1, %0|%0, %1, %2}"
   [(set_attr "mode" "DI")])
 
 ;; See comment for addsi_1_zext why we do use nonimmediate_operand
diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md
index 5132955..b64457e 100644
--- a/gcc/config/i386/sse.md
+++ b/gcc/config/i386/sse.md
@@ -11747,16 +11747,16 @@
 
 (define_expand "vec_pack_trunc_qi"
   [(set (match_operand:HI 0 ("register_operand"))
-(ior:HI (ashift:HI (zero_extend:HI (match_operand:QI 1 
("register_operand")))
+(ior:HI (ashift:HI (zero_extend:HI (match_operand:QI 2 
("register_operand")))
(const_int 8))
-(zero_extend:HI (match_operand:QI 2 ("register_operand")]
+(zero_extend:HI (match_operand:QI 1 ("register_operand")]
   "TARGET_AVX512F")
 
 (define_expand "vec_pack_trunc_"
   [(set (match_operand: 0 ("register_operand"))
-(ior: (ashift: 
(zero_extend: (match_operand:SWI24 1 ("register_operand")))
+(ior: (ashift: 
(zero_extend: (match_operand:SWI24 2 ("register_operand")))
(match_dup 3))
-(zero_extend: (match_operand:SWI24 2 
("register_operand")]
+(zero_extend: (match_operand:SWI24 1 
("register_operand")]
   "TARGET_AVX512BW"
 {
   operands[3] = GEN_INT (GET_MODE_BITSIZE (mode));
diff --git a/gcc/testsuite/gcc.target/i386/avx512bw-kunpckdq-2.c 
b/gcc/testsuite/gcc.target/i386/avx512bw-kunpckdq-2.c
new file mode 100644
index 000..4fe503e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/avx512bw-kunpckdq-2.c
@@ -0,0 +1,24 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -mavx512bw" } */
+/* { dg-require-effective-target avx512bw } */
+
+#define AVX512BW
+
+#include "avx512f-helper.h"
+
+static __mmask64 __attribute__((noinline,noclone))
+unpack (__mmask64 arg1, __mmask64 arg2)
+{
+  __mmask64 res;
+
+  res = _mm512_kunpackd (arg1, arg2);
+
+  return res;
+}
+
+void
+TEST (void)
+{
+  if (unpack (0x07UL, 0x70UL) != 0x070070UL)
+__builtin_abort ();
+}
diff --git a/gcc/testsuite/gcc.target/i386/avx512bw-kunpckwd-2.c 
b/gcc/testsuite/gcc.target/i386/avx512bw-kunpckwd-2.c
new file mode 100644
index 000..5d7f895
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/avx512bw-kunpckwd-2.c
@@ -0,0 +1,24 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -mavx512bw" } */
+/* { dg-require-effective-target avx512bw } */
+
+#define AVX512BW
+
+#include "avx512f-helper.h"
+
+static __mmask32 __attribute__((noinline,noclone))
+unpack (__mmask32 arg1, __mmask32 arg2)
+{
+  __mmask32 res;
+
+  res = _mm512_kunpackw (arg1, arg2);
+
+  return res;
+}
+
+void
+TEST (void)
+{
+  if (unpack (0x07, 0x70) != 0x070070)
+__builtin_abort ();
+}
diff --git a/gcc/testsuite/gcc.target/i386/avx512f-kunpckbw-2.c 
b/gcc/testsuite/gcc.target/i386/avx512f-kunpckbw-2.c
new file mode 100644
index 000..86580f2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/avx512f-kunpckbw-2.c
@@ -0,0 +1,24 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -mavx512f" } */
+/* { dg-require-effective-target avx512f } */
+
+#define AVX512F
+
+#include "avx512f-helper.h"
+
+static __mmask16 __attribute__((noinline,noclo

Re: [PATCH, i386] Fix operands order in kunpck* insns and corresponding expands

2016-04-13 Thread Kirill Yukhin
Hi Ilya,
On 13 Apr 16:29, Ilya Enkovich wrote:
> Hi,
> 
> Current kunpck[hi|si|di] patterns emit operands in a wrong order. This
> is compensated by a wrong operands order in vec_pack_trunc_[qi|hi|si]
> expands and therefore we get correct code for vectorized loops.  Code
> using kunpck* intrinsics would be wrong though.  This patch fixes
> operands order and adds runtime tests for _mm512_kunpack* intrinsics.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu.  OK for trunk?
Patch is OK.
> 
> Thanks,
> Ilya

--
Thanks, K


Re: [PATCH] Fix debug ICE on aarch64 due to bad rtl simplification (PR debug/70628)

2016-04-13 Thread Bernd Schmidt

On 04/12/2016 10:34 PM, Jakub Jelinek wrote:

+ return temp ? gen_rtx_fmt_ee (GET_CODE (x), to_mode,
+   temp, XEXP (x, 1))
+ : temp;


Wrap multi-line expressions in parens. No need for a full retest, just 
make sure the file still builds.



Bernd



Unreviewed^2 patches

2016-04-13 Thread Rainer Orth
The following two patches have remained unreviewed for a week or
(despite a reminder) two:

[testsuite, sparcv9] Fix gcc.dg/ifcvt-4.c on 64-bit SPARC (PR 
rtl-optimization/68749)
https://gcc.gnu.org/ml/gcc-patches/2016-03/msg01631.html

Compile libcilkrts with -funwind-tables (PR target/60290)
https://gcc.gnu.org/ml/gcc-patches/2016-04/msg00292.html

Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University


C PATCH to overhaul warning about dangling else (PR c/70436)

2016-04-13 Thread Marek Polacek
I said I'd take care of the C FE part of this PR, so here it is.

As this PR shows, our dangling else warning regressed in such a way that we
weren't warning about cases such as

if (A)
  for (;;)
if (B)
  bar ();
else
  baz ();

The problem was that we were setting FIRST_IF only when an "if" was immediately
followed by an RID_IF token.

I revamped the warning so that it follows what the C++ FE does (i.e.  passing
IF_P bools here and there) and it seems to work quite well.  I didn't mean to
tackle the OMP bits but I bet it would be just a matter of passing IF_P
somewhere.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2016-04-13  Marek Polacek  

PR c/70436
* c-parser.c
(c_parser_statement_after_labels): Add IF_P argument and adjust
callers.
(c_parser_statement): Likewise.
(c_parser_c99_block_statement): Likewise.
(c_parser_while_statement): Likewise.
(c_parser_for_statement): Likewise.
(c_parser_if_body): Don't set IF_P here.
(c_parser_if_statement): Add IF_P argument.  Set IF_P here.  Warn
about dangling else here.
* c-tree.h (c_finish_if_stmt): Adjust declaration.
* c-typeck.c (c_finish_if_stmt): Remove NESTED_IF parameter.  Don't
warn about dangling else here.

* testsuite/gcc.dg/Wparentheses-12.c: New test.
* testsuite/gcc.dg/Wparentheses-13.c: New test.

diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index 6460684..cafcb99 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -1301,13 +1301,14 @@ static void c_parser_initval (c_parser *, struct c_expr 
*,
 static tree c_parser_compound_statement (c_parser *);
 static void c_parser_compound_statement_nostart (c_parser *);
 static void c_parser_label (c_parser *);
-static void c_parser_statement (c_parser *);
-static void c_parser_statement_after_labels (c_parser *, vec * = NULL);
-static void c_parser_if_statement (c_parser *, vec *);
+static void c_parser_statement (c_parser *, bool *);
+static void c_parser_statement_after_labels (c_parser *, bool *,
+vec * = NULL);
+static void c_parser_if_statement (c_parser *, bool *, vec *);
 static void c_parser_switch_statement (c_parser *);
-static void c_parser_while_statement (c_parser *, bool);
+static void c_parser_while_statement (c_parser *, bool, bool *);
 static void c_parser_do_statement (c_parser *, bool);
-static void c_parser_for_statement (c_parser *, bool);
+static void c_parser_for_statement (c_parser *, bool, bool *);
 static tree c_parser_asm_statement (c_parser *);
 static tree c_parser_asm_operands (c_parser *);
 static tree c_parser_asm_goto_operands (c_parser *);
@@ -4853,7 +4854,7 @@ c_parser_compound_statement_nostart (c_parser *parser)
  last_label = false;
  last_stmt = true;
  mark_valid_location_for_stdc_pragma (false);
- c_parser_statement_after_labels (parser);
+ c_parser_statement_after_labels (parser, NULL);
}
 
   parser->error = false;
@@ -5098,22 +5099,25 @@ c_parser_label (c_parser *parser)
 */
 
 static void
-c_parser_statement (c_parser *parser)
+c_parser_statement (c_parser *parser, bool *if_p)
 {
   c_parser_all_labels (parser);
-  c_parser_statement_after_labels (parser);
+  c_parser_statement_after_labels (parser, if_p, NULL);
 }
 
 /* Parse a statement, other than a labeled statement.  CHAIN is a vector
of if-else-if conditions.  */
 
 static void
-c_parser_statement_after_labels (c_parser *parser, vec *chain)
+c_parser_statement_after_labels (c_parser *parser, bool *if_p,
+vec *chain)
 {
   location_t loc = c_parser_peek_token (parser)->location;
   tree stmt = NULL_TREE;
   bool in_if_block = parser->in_if_block;
   parser->in_if_block = false;
+  if (if_p != NULL)
+*if_p = false;
   switch (c_parser_peek_token (parser)->type)
 {
 case CPP_OPEN_BRACE:
@@ -5123,19 +5127,19 @@ c_parser_statement_after_labels (c_parser *parser, 
vec *chain)
   switch (c_parser_peek_token (parser)->keyword)
{
case RID_IF:
- c_parser_if_statement (parser, chain);
+ c_parser_if_statement (parser, if_p, chain);
  break;
case RID_SWITCH:
  c_parser_switch_statement (parser);
  break;
case RID_WHILE:
- c_parser_while_statement (parser, false);
+ c_parser_while_statement (parser, false, if_p);
  break;
case RID_DO:
  c_parser_do_statement (parser, false);
  break;
case RID_FOR:
- c_parser_for_statement (parser, false);
+ c_parser_for_statement (parser, false, if_p);
  break;
case RID_CILK_FOR:
  if (!flag_cilkplus)
@@ -5324,11 +5328,11 @@ c_parser_paren_condition (c_parser *parser)
 /* Parse a statement which is a block in C99.  */
 
 static tree
-c_parser_c99_block_statement (c_parser *parser)
+c_parser_c99_block_statement (c_parser *parser, b

C/C++ PATCH to add -Wdangling-else option

2016-04-13 Thread Marek Polacek
This patch is meant to be applied on top of the "Wparentheses overhaul" patch.

I really think that warning about the dangling else problem isn't appropriate
as a part of the -Wparentheses warning, which I think should only deal with
stuff like precedence of operators, i.e. things where ()'s are missing and not
{}'s.

This new warning is, however, a subset of -Wparentheses.

Bootstrapped/regtested on x86_64-linux, ok for trunk or should I stash it
for the next stage1?

2016-04-13  Marek Polacek  

* c.opt (Wdangling-else): New option.

* c-parser.c (c_parser_if_statement): Replace OPT_Wparentheses with
OPT_Wdangling_else.

* parser.c (cp_parser_selection_statement): Replace OPT_Wparentheses
with OPT_Wdangling_else.

* doc/invoke.texi: Document -Wdangling-else.

* c-c++-common/Wdangling-else-1.c: New test.
* c-c++-common/Wdangling-else-2.c: New test.
* c-c++-common/Wdangling-else-3.c: New test.

diff --git gcc/c-family/c.opt gcc/c-family/c.opt
index 4f86876..5e7bcb8 100644
--- gcc/c-family/c.opt
+++ gcc/c-family/c.opt
@@ -370,6 +370,10 @@ Wctor-dtor-privacy
 C++ ObjC++ Var(warn_ctor_dtor_privacy) Warning
 Warn when all constructors and destructors are private.
 
+Wdangling-else
+C ObjC C++ ObjC++ Var(warn_dangling_else) Warning LangEnabledBy(C ObjC C++ 
ObjC++,Wparentheses)
+Warn about dangling else.
+
 Wdate-time
 C ObjC C++ ObjC++ CPP(warn_date_time) CppReason(CPP_W_DATE_TIME) 
Var(cpp_warn_date_time) Init(0) Warning
 Warn about __TIME__, __DATE__ and __TIMESTAMP__ usage.
diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index cafcb99..1f95446 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -5486,7 +5486,7 @@ c_parser_if_statement (c_parser *parser, bool *if_p, 
vec *chain)
   /* Diagnose an ambiguous else if if-then-else is nested inside
 if-then.  */
   if (nested_if)
-   warning_at (loc, OPT_Wparentheses,
+   warning_at (loc, OPT_Wdangling_else,
"suggest explicit braces to avoid ambiguous %");
 
   if (warn_duplicated_cond)
diff --git gcc/cp/parser.c gcc/cp/parser.c
index 00e211e..968706f 100644
--- gcc/cp/parser.c
+++ gcc/cp/parser.c
@@ -10951,7 +10951,7 @@ cp_parser_selection_statement (cp_parser* parser, bool 
*if_p,
   statement which does have an else clause.  We warn
   about the potential ambiguity.  */
if (nested_if)
- warning_at (EXPR_LOCATION (statement), OPT_Wparentheses,
+ warning_at (EXPR_LOCATION (statement), OPT_Wdangling_else,
  "suggest explicit braces to avoid ambiguous"
  " %");
if (warn_duplicated_cond)
diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
index e9763d4..6253d78 100644
--- gcc/doc/invoke.texi
+++ gcc/doc/invoke.texi
@@ -257,7 +257,8 @@ Objective-C and Objective-C++ Dialects}.
 -Wc90-c99-compat -Wc99-c11-compat @gol
 -Wc++-compat -Wc++11-compat -Wc++14-compat -Wcast-align  -Wcast-qual  @gol
 -Wchar-subscripts -Wclobbered  -Wcomment -Wconditionally-supported  @gol
--Wconversion -Wcoverage-mismatch -Wno-cpp -Wdate-time -Wdelete-incomplete @gol
+-Wconversion -Wcoverage-mismatch -Wno-cpp -Wdangling-else -Wdate-time @gol
+-Wdelete-incomplete @gol
 -Wno-deprecated -Wno-deprecated-declarations -Wno-designated-init @gol
 -Wdisabled-optimization @gol
 -Wno-discarded-qualifiers -Wno-discarded-array-qualifiers @gol
@@ -3974,46 +3975,6 @@ Also warn if a comparison like @code{x<=y<=z} appears; 
this is
 equivalent to @code{(x<=y ? 1 : 0) <= z}, which is a different
 interpretation from that of ordinary mathematical notation.
 
-Also warn about constructions where there may be confusion to which
-@code{if} statement an @code{else} branch belongs.  Here is an example of
-such a case:
-
-@smallexample
-@group
-@{
-  if (a)
-if (b)
-  foo ();
-  else
-bar ();
-@}
-@end group
-@end smallexample
-
-In C/C++, every @code{else} branch belongs to the innermost possible
-@code{if} statement, which in this example is @code{if (b)}.  This is
-often not what the programmer expected, as illustrated in the above
-example by indentation the programmer chose.  When there is the
-potential for this confusion, GCC issues a warning when this flag
-is specified.  To eliminate the warning, add explicit braces around
-the innermost @code{if} statement so there is no way the @code{else}
-can belong to the enclosing @code{if}.  The resulting code
-looks like this:
-
-@smallexample
-@group
-@{
-  if (a)
-@{
-  if (b)
-foo ();
-  else
-bar ();
-@}
-@}
-@end group
-@end smallexample
-
 Also warn for dangerous uses of the GNU extension to
 @code{?:} with omitted middle operand. When the condition
 in the @code{?}: operator is a boolean expression, the omitted value is
@@ -5146,6 +5107,51 @@ compiler doesn't give this warning for types defined in 
the main .C
 file, as those are unlikely to have multipl

Re: C PATCH to overhaul warning about dangling else (PR c/70436)

2016-04-13 Thread Jakub Jelinek
On Wed, Apr 13, 2016 at 04:14:03PM +0200, Marek Polacek wrote:
> I said I'd take care of the C FE part of this PR, so here it is.
> 
> As this PR shows, our dangling else warning regressed in such a way that we
> weren't warning about cases such as
> 
> if (A)
>   for (;;)
> if (B)
>   bar ();
> else
>   baz ();
> 
> The problem was that we were setting FIRST_IF only when an "if" was 
> immediately
> followed by an RID_IF token.
> 
> I revamped the warning so that it follows what the C++ FE does (i.e.  passing
> IF_P bools here and there) and it seems to work quite well.  I didn't mean to
> tackle the OMP bits but I bet it would be just a matter of passing IF_P
> somewhere.

I'll look at the OMP bits both for C and C++ once/if this is committed.

Jakub


Re: C PATCH to overhaul warning about dangling else (PR c/70436)

2016-04-13 Thread Marek Polacek
Looks like Joseph will be on vacation for the next few days so if we
want to get this in sooner, someone else will need to review & approve
the patch.


Re: [PATCH] Don't include DECL_UIDs in tree-sra fancy names for -fdump-final-insns= (PR c++/70594)

2016-04-13 Thread Jakub Jelinek
On Wed, Apr 13, 2016 at 09:25:56AM +0200, Richard Biener wrote:
> So if you think it's not acceptable to drop the relation between
> the artificial number used by SRA and the original UID then go
> with a hash-map unconditionally.  You still have to dump and lookup
> the actual relation though - there's no way around this unless you
> hack dump-final-insns to filter DECL names (maybe just make it
> strip all DECL_ARTIFICIAL names completely?)

Here is an alternative patch, keep the fancy names as before,
but in TDF_NOUID dumps sanitize them.  Most of the DECL_NAMELESS
decls other than ones with the SRA fancy names should not contain
anything that would be tweaked for the printout, and doing it this
way has the advantage that one can always just use flags without TDF_NOUID
to see the real fancy names.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2016-04-13  Jakub Jelinek  

PR c++/70594
* tree-sra.c (create_access_replacement,
get_replaced_param_substitute): Set DECL_NAMELESS on repl if it
gets fancy name.
* tree-pretty-print.c (dump_fancy_name): New function.
(dump_decl_name, dump_generic_node): Use it.

--- gcc/tree-sra.c.jj   2016-04-12 19:12:34.0 +0200
+++ gcc/tree-sra.c  2016-04-13 13:52:28.329925560 +0200
@@ -2132,6 +2132,7 @@ create_access_replacement (struct access
   bool fail = false;
 
   DECL_NAME (repl) = get_identifier (pretty_name);
+  DECL_NAMELESS (repl) = 1;
   obstack_free (&name_obstack, pretty_name);
 
   /* Get rid of any SSA_NAMEs embedded in debug_expr,
@@ -4704,6 +4705,7 @@ get_replaced_param_substitute (struct ip
 
   repl = create_tmp_reg (TREE_TYPE (adj->base), "ISR");
   DECL_NAME (repl) = get_identifier (pretty_name);
+  DECL_NAMELESS (repl) = 1;
   obstack_free (&name_obstack, pretty_name);
 
   adj->new_ssa_base = repl;
--- gcc/tree-pretty-print.c.jj  2016-03-17 16:54:44.0 +0100
+++ gcc/tree-pretty-print.c 2016-04-13 14:10:59.667085487 +0200
@@ -161,6 +161,85 @@ print_generic_expr (FILE *file, tree t,
   pp_flush (tree_pp);
 }
 
+/* Dump NAME, an IDENTIFIER_POINTER, sanitized so that D sequences
+   in it are replaced with D, as long as they are at the start or
+   preceded by $ and at the end or followed by $.  See make_fancy_name
+   in tree-sra.c.  */
+
+static void
+dump_fancy_name (pretty_printer *pp, tree name)
+{
+  int cnt = 0;
+  int length = IDENTIFIER_LENGTH (name);
+  const char *n = IDENTIFIER_POINTER (name);
+  do
+{
+  n = strchr (n, 'D');
+  if (n == NULL)
+   break;
+  if (ISDIGIT (n[1])
+ && (n == IDENTIFIER_POINTER (name) || n[-1] == '$'))
+   {
+ int l = 2;
+ while (ISDIGIT (n[l]))
+   l++;
+ if (n[l] == '\0' || n[l] == '$')
+   {
+ cnt++;
+ length += 5 - l;
+   }
+ n += l;
+   }
+  else
+   n++;
+}
+  while (1);
+  if (cnt == 0)
+{
+  pp_tree_identifier (pp, name);
+  return;
+}
+
+  char *str = XNEWVEC (char, length + 1);
+  char *p = str;
+  const char *q;
+  q = n = IDENTIFIER_POINTER (name);
+  do
+{
+  q = strchr (q, 'D');
+  if (q == NULL)
+   break;
+  if (ISDIGIT (q[1])
+ && (q == IDENTIFIER_POINTER (name) || q[-1] == '$'))
+   {
+ int l = 2;
+ while (ISDIGIT (q[l]))
+   l++;
+ if (q[l] == '\0' || q[l] == '$')
+   {
+ memcpy (p, n, q - n);
+ memcpy (p + (q - n), "D", 5);
+ p += (q - n) + 5;
+ n = q + l;
+   }
+ q += l;
+   }
+  else
+   q++;
+}
+  while (1);
+  memcpy (p, n, IDENTIFIER_LENGTH (name) - (n - IDENTIFIER_POINTER (name)));
+  str[length] = '\0';
+  if (pp_translate_identifiers (pp))
+{
+  const char *text = identifier_to_locale (str);
+  pp_append_text (pp, text, text + strlen (text));
+}
+  else
+pp_append_text (pp, str, str + length);
+  XDELETEVEC (str);
+}
+
 /* Dump the name of a _DECL node and its DECL_UID if TDF_UID is set
in FLAGS.  */
 
@@ -171,6 +250,10 @@ dump_decl_name (pretty_printer *pp, tree
 {
   if ((flags & TDF_ASMNAME) && DECL_ASSEMBLER_NAME_SET_P (node))
pp_tree_identifier (pp, DECL_ASSEMBLER_NAME (node));
+  /* For DECL_NAMELESS names look for embedded uids in the
+names and sanitize them for TDF_NOUID.  */
+  else if ((flags & TDF_NOUID) && DECL_NAMELESS (node))
+   dump_fancy_name (pp, DECL_NAME (node));
   else
pp_tree_identifier (pp, DECL_NAME (node));
 }
@@ -2593,8 +2676,15 @@ dump_generic_node (pretty_printer *pp, t
 
 case SSA_NAME:
   if (SSA_NAME_IDENTIFIER (node))
-   dump_generic_node (pp, SSA_NAME_IDENTIFIER (node),
-  spc, flags, false);
+   {
+ if ((flags & TDF_NOUID)
+ && SSA_NAME_VAR (node)
+ && DECL_NAMELESS (SSA_NAME_V

C++ PATCH for c++/70615 (ICE with pointer to member)

2016-04-13 Thread Jason Merrill
cp_gimplify_r doesn't always get to see the contents of a CONSTRUCTOR, 
so it can miss lowering PTRMEM_CSTs there.  cp_genericize_r does look 
into CONSTRUCTORs, and lowering them earlier is fine.


Tested x86_64-pc-linux-gnu, applying to trunk.
commit fff4c8804414258331785f08435259ecfdb7311c
Author: Jason Merrill 
Date:   Tue Apr 12 17:00:42 2016 -0400

	PR c++/70615
	* cp-gimplify.c (cp_genericize_r): Expand PTRMEM_CST here.
	(cp_gimplify_expr): Not here.

diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index 13f7b7c..30ac660 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -576,11 +576,6 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
 
   switch (code)
 {
-case PTRMEM_CST:
-  *expr_p = cplus_expand_constant (*expr_p);
-  ret = GS_OK;
-  break;
-
 case AGGR_INIT_EXPR:
   simplify_aggr_init_expr (expr_p);
   ret = GS_OK;
@@ -1388,6 +1383,13 @@ cp_genericize_r (tree *stmt_p, int *walk_subtrees, void *data)
 	   || TREE_CODE (stmt) == OMP_SIMD
 	   || TREE_CODE (stmt) == OMP_DISTRIBUTE)
 genericize_omp_for_stmt (stmt_p, walk_subtrees, data);
+  else if (TREE_CODE (stmt) == PTRMEM_CST)
+{
+  /* By the time we get here we're handing off to the back end, so we don't
+	 need or want to preserve PTRMEM_CST anymore.  */
+  *stmt_p = cplus_expand_constant (stmt);
+  *walk_subtrees = 0;
+}
   else if ((flag_sanitize
 	& (SANITIZE_NULL | SANITIZE_ALIGNMENT | SANITIZE_VPTR))
 	   && !wtd->no_sanitize_p)
diff --git a/gcc/testsuite/g++.dg/opt/ptrmem7.C b/gcc/testsuite/g++.dg/opt/ptrmem7.C
new file mode 100644
index 000..7d9e9b1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/opt/ptrmem7.C
@@ -0,0 +1,31 @@
+// PR c++/70615
+// { dg-options -O }
+
+struct C
+{
+  virtual void f () {}
+};
+
+struct B
+{
+  virtual ~B () {}
+};
+
+class D : public B, public C
+{
+public:
+  D () {}
+};
+
+typedef void (C::*FP) ();
+typedef void (D::*D_f) ();
+
+int
+main ()
+{
+  D *d = new D ();
+  C *c = d;
+  const FP fptr = (FP) & D::f;
+  (d->*(D_f) fptr) ();
+  return 0;
+}


[C++ PATCH] Avoid code generation differences dependent on DECL_UID gaps (PR c++/70594)

2016-04-13 Thread Jakub Jelinek
Hi!

pop_labels adds LABEL_DECLs into BLOCK_VARS by traversing a hash table
indexed by DECL_UID.  This is something that is undesirable, if -g vs. -g0
has different DECL_UID values (but still the same order of them), which can
happen even without the deletable constexpr tables, then the exact values
of the uids affect the order of BLOCK_VARS, which e.g. affects the inliner
and other places.

Fixed in similar way how other spots handle this case, by pushing the
slots into a vector, sorting the vector and then traversing the vector
instead of the hash table.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2016-04-13  Jakub Jelinek  

PR c++/70594
* decl.c (pop_labels_1): Removed.
(note_label, sort_labels): New functions.
(pop_labels): During named_labels traversal, just push the slot
pointers into a vector, then qsort it by DECL_UID and only then
call pop_label and chain it into BLOCK_VARS.

--- gcc/cp/decl.c.jj2016-04-01 17:21:30.0 +0200
+++ gcc/cp/decl.c   2016-04-13 11:59:04.604105861 +0200
@@ -368,33 +368,61 @@ pop_label (tree label, tree old_value)
   SET_IDENTIFIER_LABEL_VALUE (DECL_NAME (label), old_value);
 }
 
-/* At the end of a function, all labels declared within the function
-   go out of scope.  BLOCK is the top-level block for the
-   function.  */
+/* Push all named labels into a vector, so that we can sort it on DECL_UID
+   to avoid code generation differences.  */
 
 int
-pop_labels_1 (named_label_entry **slot, tree block)
+note_label (named_label_entry **slot, vec &labels)
 {
-  struct named_label_entry *ent = *slot;
-
-  pop_label (ent->label_decl, NULL_TREE);
-
-  /* Put the labels into the "variables" of the top-level block,
- so debugger can see them.  */
-  DECL_CHAIN (ent->label_decl) = BLOCK_VARS (block);
-  BLOCK_VARS (block) = ent->label_decl;
+  labels.quick_push (slot);
+  return 1;
+}
 
-  named_labels->clear_slot (slot);
+/* Helper function to sort named label entries in a vector by DECL_UID.  */
 
-  return 1;
+static int
+sort_labels (const void *a, const void *b)
+{
+  named_label_entry **slot1 = *(named_label_entry **const *) a;
+  named_label_entry **slot2 = *(named_label_entry **const *) b;
+  if (DECL_UID ((*slot1)->label_decl) < DECL_UID ((*slot2)->label_decl))
+return -1;
+  if (DECL_UID ((*slot1)->label_decl) > DECL_UID ((*slot2)->label_decl))
+return 1;
+  return 0;
 }
 
+/* At the end of a function, all labels declared within the function
+   go out of scope.  BLOCK is the top-level block for the
+   function.  */
+
 static void
 pop_labels (tree block)
 {
   if (named_labels)
 {
-  named_labels->traverse (block);
+  auto_vec labels;
+  named_label_entry **slot;
+  unsigned int i;
+
+  /* Push all the labels into a vector and sort them by DECL_UID,
+so that gaps between DECL_UIDs don't affect code generation.  */
+  labels.reserve_exact (named_labels->elements ());
+  named_labels->traverse &, note_label> (labels);
+  labels.qsort (sort_labels);
+  FOR_EACH_VEC_ELT (labels, i, slot)
+   {
+ struct named_label_entry *ent = *slot;
+
+ pop_label (ent->label_decl, NULL_TREE);
+
+ /* Put the labels into the "variables" of the top-level block,
+so debugger can see them.  */
+ DECL_CHAIN (ent->label_decl) = BLOCK_VARS (block);
+ BLOCK_VARS (block) = ent->label_decl;
+
+ named_labels->clear_slot (slot);
+   }
   named_labels = NULL;
 }
 }

Jakub


Re: [C++ Patch] PR 70635 ([4.9/5/6 Regression] ICE on (and rejects) valid code...)

2016-04-13 Thread Jason Merrill

On 04/13/2016 03:36 AM, Paolo Carlini wrote:

Hi,

On 12/04/2016 15:50, Jason Merrill wrote:

On 04/12/2016 09:25 AM, Paolo Carlini wrote:

in this regression we have an infinite recursion affecting the
same_type_p call at parser.c:25125 which I added in the patch for
c++/38313. The issue is that for, eg, the testcase at issue, we are
passing a TYPENAME_TYPE to same_type_p. I think we can simply handle the
problem by checking first that we have a CLASS_TYPE_P as TREE_TYPE
(type_decl). Tested x86_64-linux.


I don't see how this can be considered valid code; the typenames are
indeed mutually recursive, with no real underlying type anywhere.
clang and EDG both reject the testcase if the template is
instantiated. Please add an instantiation to the testcase.  And we
should fix the recursion issue in structural_comptypes.


...but your patch is OK; even if it doesn't fix the real problem, it 
isn't wrong.  Only the testcase should be adjusted.



Ok. But then, if I understand correctly, our user-visible behavior
should not change much: accept the testcase (modulo the -fpermissive
detail) without an instantiation and reject it when the template is
actually instantiated. In other terms, exactly what EDG and clang also
do.


The testcase without an instantiation is ill-formed, no diagnostic 
required, so we shouldn't test for accepting it, we should only test for 
rejecting the variant with an instantiation.



Thus same_type_p should be fixed to simply return false in such
recursive cases without ICEing and nothing more, right? I'm not sure I
will be able to do that in time for 6.0 but I'm going to try...


resolve_typename_type should give up in such cases.

Jason



Re: [C++ PATCH] Avoid code generation differences dependent on DECL_UID gaps (PR c++/70594)

2016-04-13 Thread Jason Merrill

OK.

Jason


Re: [PATCH, rs6000] Add support for int versions of vec_adde

2016-04-13 Thread Bill Seurer
Here is an updated patch:


This patch adds support for the signed and unsigned int versions of the
vec_adde altivec builtins from the Power Architecture 64-Bit ELF V2 ABI
OpenPOWER ABI for Linux Supplement (16 July 2015 Version 1.1). There are
many of the builtins that are missing and this is the first of a series
of patches to add them.

There aren't instructions for the int versions of vec_adde so the
output code is built from other built-ins that do have instructions
which in this case is just two vec_adds.

The new test cases are executable tests which verify that the generated
code produces expected values. C macros were used so that the same
test case could be used for both the signed and unsigned versions. An
extra executable test case is also included to ensure that the modified
support for the __int128 versions of vec_adde is not broken. The same
test case could not be used for both int and __int128 because of some
differences in loading and storing the vectors.

Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
regressions. Is this ok for trunk?

[gcc]

2016-04-06 Bill Seurer 

* config/rs6000/rs6000-builtin.def (vec_adde): Change vec_adde to a
special case builtin.
* config/rs6000/rs6000-c.c (altivec_overloaded_builtins): Remove
ALTIVEC_BUILTIN_VEC_ADDE.
* config/rs6000/rs6000-c.c (altivec_resolve_overloaded_builtin): Add
support for ALTIVEC_BUILTIN_VEC_ADDE.
* config/rs6000/rs6000.c (altivec_init_builtins): Add definition
for __builtin_vec_adde.

[gcc/testsuite]

2016-04-06 Bill Seurer 

* gcc.target/powerpc/vec-adde.c: New test.
* gcc.target/powerpc/vec-adde-int128.c: New test.


Index: gcc/config/rs6000/rs6000-builtin.def
===
--- gcc/config/rs6000/rs6000-builtin.def(revision 234936)
+++ gcc/config/rs6000/rs6000-builtin.def(working copy)
@@ -951,7 +951,6 @@ BU_ALTIVEC_X (VEC_EXT_V4SF, "vec_ext_v4sf", CO
before we get to the point about classifying the builtin type.  */
 
 /* 3 argument Altivec overloaded builtins.  */
-BU_ALTIVEC_OVERLOAD_3 (ADDE,  "adde")
 BU_ALTIVEC_OVERLOAD_3 (ADDEC, "addec")
 BU_ALTIVEC_OVERLOAD_3 (MADD,   "madd")
 BU_ALTIVEC_OVERLOAD_3 (MADDS,  "madds")
@@ -1137,6 +1136,7 @@ BU_ALTIVEC_OVERLOAD_P (VCMPGT_P,   "vcmpgt_p")
 BU_ALTIVEC_OVERLOAD_P (VCMPGE_P,   "vcmpge_p")
 
 /* Overloaded Altivec builtins that are handled as special cases.  */
+BU_ALTIVEC_OVERLOAD_X (ADDE,  "adde")
 BU_ALTIVEC_OVERLOAD_X (CTF,   "ctf")
 BU_ALTIVEC_OVERLOAD_X (CTS,   "cts")
 BU_ALTIVEC_OVERLOAD_X (CTU,   "ctu")
Index: gcc/config/rs6000/rs6000-c.c
===
--- gcc/config/rs6000/rs6000-c.c(revision 234936)
+++ gcc/config/rs6000/rs6000-c.c(working copy)
@@ -842,11 +842,6 @@ const struct altivec_builtin_types altivec_overloa
 RS6000_BTI_unsigned_V1TI, 0 },
   { ALTIVEC_BUILTIN_VEC_ADDC, P8V_BUILTIN_VADDCUQ,
 RS6000_BTI_V1TI, RS6000_BTI_V1TI, RS6000_BTI_V1TI, 0 },
-  { ALTIVEC_BUILTIN_VEC_ADDE, P8V_BUILTIN_VADDEUQM,
-RS6000_BTI_unsigned_V1TI, RS6000_BTI_unsigned_V1TI,
-RS6000_BTI_unsigned_V1TI, RS6000_BTI_unsigned_V1TI },
-  { ALTIVEC_BUILTIN_VEC_ADDE, P8V_BUILTIN_VADDEUQM,
-RS6000_BTI_V1TI, RS6000_BTI_V1TI, RS6000_BTI_V1TI, RS6000_BTI_V1TI },
   { ALTIVEC_BUILTIN_VEC_ADDEC, P8V_BUILTIN_VADDECUQ,
 RS6000_BTI_unsigned_V1TI, RS6000_BTI_unsigned_V1TI,
 RS6000_BTI_unsigned_V1TI, RS6000_BTI_unsigned_V1TI },
@@ -4515,6 +4510,62 @@ assignment for unaligned loads and stores");
 warning (OPT_Wdeprecated, "vec_lvsr is deprecated for little endian; use \
 assignment for unaligned loads and stores");
 
+  if (fcode == ALTIVEC_BUILTIN_VEC_ADDE)
+{
+  /* vec_adde needs to be special cased because there is no instruction
+ for the {un}signed int version.  */
+  if (nargs != 3)
+   {
+ error ("vec_adde only accepts 3 arguments");
+ return error_mark_node;
+   }
+
+  tree arg0 = (*arglist)[0];
+  tree arg0_type = TREE_TYPE (arg0);
+  tree arg1 = (*arglist)[1];
+  tree arg1_type = TREE_TYPE (arg1);
+  tree arg2 = (*arglist)[2];
+  tree arg2_type = TREE_TYPE (arg2);
+
+  /* All 3 arguments must be vectors of (signed or unsigned) (int or
+ __int128) and the types must match.  */
+  if ((arg0_type != arg1_type) || (arg1_type != arg2_type))
+   goto bad; 
+  if (TREE_CODE (arg0_type) != VECTOR_TYPE)
+   goto bad; 
+
+  switch (TYPE_MODE (TREE_TYPE (arg0_type)))
+   {
+ /* For {un}signed ints, 
+vec_adde (va, vb, carryv) == vec_add (vec_add (va, vb), vec_and 
(carryv, 0x1)).  */
+ case SImode:
+   {
+ vec *params = make_tree_vector();
+ vec_safe_push (params, arg0);
+ vec_safe_push (params, arg1);
+ tree call = altivec_resolve_overloaded_builtin
+(loc, rs6000_builtin_decls[ALTIVEC_B

Re: C PATCH to overhaul warning about dangling else (PR c/70436)

2016-04-13 Thread Bernd Schmidt

On 04/13/2016 04:14 PM, Marek Polacek wrote:

I revamped the warning so that it follows what the C++ FE does (i.e.  passing
IF_P bools here and there) and it seems to work quite well.  I didn't mean to
tackle the OMP bits but I bet it would be just a matter of passing IF_P
somewhere.

Bootstrapped/regtested on x86_64-linux, ok for trunk?


Ok if you adjust function comments to mention the new argument where 
applicable.



Bernd



C++ PATCH for c++/70634 (ICE with self-referential initializer)

2016-04-13 Thread Jason Merrill
The compiler was getting into infinite recursion here because when 
looking at whether 'i>0' was value-dependent, we looked at the 
initializer for i, which contains sizeof(i), and we asked whether i is 
value-dependent again.


The bug is that we shouldn't care whether the operand of sizeof is 
value-dependent; this patch splits up the instantiation-dependence 
predicate for evaluated and unevaluated expressions.  I also needed to 
swap the order of checking potential_constant_expression and 
instantiation_dependent_expression_p, so I factored out all the places 
that check those two into a new function that tests both.


Tested x86_64-pc-linux-gnu, applying to trunk.
commit 0897b3e9b5be7fc3adbe96d871876fdb8452d2a9
Author: Jason Merrill 
Date:   Tue Apr 12 23:41:04 2016 -0400

	PR c++/70634

	* pt.c (instantiation_dependent_uneval_expression_p): Split out
	from instantiation_dependent_expression_p.
	(value_dependent_expression_p): Use it for unevaluated operands.
	(instantiation_dependent_r): Don't check value-dependence.
	(instantiation_dependent_expression_p): Check
	value-dependence of the expression as a whole.
	* cp-tree.h: Declare instantiation_dependent_uneval_expression_p.
	* semantics.c (finish_decltype_type): Use it.

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index ecf2a5d..a3cd834 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6127,6 +6127,7 @@ extern bool any_type_dependent_elements_p   (const_tree);
 extern bool type_dependent_expression_p_push	(tree);
 extern bool value_dependent_expression_p	(tree);
 extern bool instantiation_dependent_expression_p (tree);
+extern bool instantiation_dependent_uneval_expression_p (tree);
 extern bool any_value_dependent_elements_p  (const_tree);
 extern bool dependent_omp_for_p			(tree, tree, tree, tree);
 extern tree resolve_typename_type		(tree, bool);
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 2871f33..b75ac24 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -22720,7 +22720,7 @@ value_dependent_expression_p (tree expression)
 return true;
   else if (TYPE_P (expression))
 	return dependent_type_p (expression);
-  return instantiation_dependent_expression_p (expression);
+  return instantiation_dependent_uneval_expression_p (expression);
 
 case AT_ENCODE_EXPR:
   /* An 'encode' expression is value-dependent if the operand is
@@ -22730,7 +22730,7 @@ value_dependent_expression_p (tree expression)
 
 case NOEXCEPT_EXPR:
   expression = TREE_OPERAND (expression, 0);
-  return instantiation_dependent_expression_p (expression);
+  return instantiation_dependent_uneval_expression_p (expression);
 
 case SCOPE_REF:
   /* All instantiation-dependent expressions should also be considered
@@ -23101,13 +23101,6 @@ instantiation_dependent_r (tree *tp, int *walk_subtrees,
 case TREE_VEC:
   return NULL_TREE;
 
-case VAR_DECL:
-case CONST_DECL:
-  /* A constant with a dependent initializer is dependent.  */
-  if (value_dependent_expression_p (*tp))
-	return *tp;
-  break;
-
 case TEMPLATE_PARM_INDEX:
   return *tp;
 
@@ -23133,12 +23126,6 @@ instantiation_dependent_r (tree *tp, int *walk_subtrees,
 	break;
   }
 
-case TRAIT_EXPR:
-  if (value_dependent_expression_p (*tp))
-	return *tp;
-  *walk_subtrees = false;
-  return NULL_TREE;
-
 case COMPONENT_REF:
   if (identifier_p (TREE_OPERAND (*tp, 1)))
 	/* In a template, finish_class_member_access_expr creates a
@@ -23189,10 +23176,15 @@ instantiation_dependent_r (tree *tp, int *walk_subtrees,
 
"An expression is instantiation-dependent if it is type-dependent
or value-dependent, or it has a subexpression that is type-dependent
-   or value-dependent."  */
+   or value-dependent."
+
+   Except don't actually check value-dependence for unevaluated expressions,
+   because in sizeof(i) we don't care about the value of i.  Checking
+   type-dependence will in turn check value-dependence of array bounds/template
+   arguments as needed.  */
 
 bool
-instantiation_dependent_expression_p (tree expression)
+instantiation_dependent_uneval_expression_p (tree expression)
 {
   tree result;
 
@@ -23207,6 +23199,15 @@ instantiation_dependent_expression_p (tree expression)
   return result != NULL_TREE;
 }
 
+/* As above, but also check value-dependence of the expression as a whole.  */
+
+bool
+instantiation_dependent_expression_p (tree expression)
+{
+  return (instantiation_dependent_uneval_expression_p (expression)
+	  || value_dependent_expression_p (expression));
+}
+
 /* Like type_dependent_expression_p, but it also works while not processing
a template definition, i.e. during substitution or mangling.  */
 
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 1574e60..0487adf 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -8740,7 +8740,7 @@ finish_decltype_type (tree expr, bool id_expression_or_member_access_p,
   /* 

Re: [C++ Patch] PR 70635 ([4.9/5/6 Regression] ICE on (and rejects) valid code...)

2016-04-13 Thread Paolo Carlini

Hi again,

On 13/04/2016 16:42, Jason Merrill wrote:

On 04/13/2016 03:36 AM, Paolo Carlini wrote:

Hi,

On 12/04/2016 15:50, Jason Merrill wrote:

On 04/12/2016 09:25 AM, Paolo Carlini wrote:

in this regression we have an infinite recursion affecting the
same_type_p call at parser.c:25125 which I added in the patch for
c++/38313. The issue is that for, eg, the testcase at issue, we are
passing a TYPENAME_TYPE to same_type_p. I think we can simply 
handle the

problem by checking first that we have a CLASS_TYPE_P as TREE_TYPE
(type_decl). Tested x86_64-linux.


I don't see how this can be considered valid code; the typenames are
indeed mutually recursive, with no real underlying type anywhere.
clang and EDG both reject the testcase if the template is
instantiated. Please add an instantiation to the testcase. And we
should fix the recursion issue in structural_comptypes.


...but your patch is OK; even if it doesn't fix the real problem, it 
isn't wrong.  Only the testcase should be adjusted.



Ok. But then, if I understand correctly, our user-visible behavior
should not change much: accept the testcase (modulo the -fpermissive
detail) without an instantiation and reject it when the template is
actually instantiated. In other terms, exactly what EDG and clang also
do.


The testcase without an instantiation is ill-formed, no diagnostic 
required, so we shouldn't test for accepting it, we should only test 
for rejecting the variant with an instantiation.
Agreed, now I really get your point: in the future certainly it would 
not be wrong to somehow provide diagnostic for a testcase without the 
instantiation too. A testcase with the instantiation is robust.


As it happens, anyway, the below exceedingly trivial patch avoids the 
ICE in resolve_typename_type and appears to pass testing (is already 
past g++.dg/tm/tm.exp...). Should I apply it instead together with the 
amended testcase? Weird that nobody noticed the pair of typos before!?!


Thanks,
Paolo.

//
Index: pt.c
===
--- pt.c(revision 234938)
+++ pt.c(working copy)
@@ -23598,9 +23598,9 @@ resolve_typename_type (tree type, bool only_curren
 {
   /* Ill-formed programs can cause infinite recursion here, so we
 must catch that.  */
-  TYPENAME_IS_RESOLVING_P (type) = 1;
+  TYPENAME_IS_RESOLVING_P (result) = 1;
   result = resolve_typename_type (result, only_current_p);
-  TYPENAME_IS_RESOLVING_P (type) = 0;
+  TYPENAME_IS_RESOLVING_P (result) = 0;
 }
   
   /* Qualify the resulting type.  */


Re: C PATCH to overhaul warning about dangling else (PR c/70436)

2016-04-13 Thread Jakub Jelinek
On Wed, Apr 13, 2016 at 04:14:03PM +0200, Marek Polacek wrote:
>   PR c/70436
>   * c-parser.c
>   (c_parser_statement_after_labels): Add IF_P argument and adjust
>   callers.

Can you put 

> @@ -5533,7 +5548,7 @@ c_parser_switch_statement (c_parser *parser)
>c_start_case (switch_loc, switch_cond_loc, expr, explicit_cast_p);
>save_break = c_break_label;
>c_break_label = NULL_TREE;
> -  body = c_parser_c99_block_statement (parser);
> +  body = c_parser_c99_block_statement (parser, NULL/*if??*/);
>c_finish_case (body, ce.original_type);
>if (c_break_label)
>  {

Is:
void bar (int);

void
foo (int a, int b, int c)
{
  if (a)
switch (b)
  case 0:
if (c)
  bar (1);
  else
bar (2);
}
something -Wdangling-else should warn about?  Note, g++ does not warn
about it, and clang nor clang++ doesn't either.
As it is something that unlikely happens in user code, I guess it is ok
to not warn about this for GCC 6, and we can deal with that later on.

Jakub


Re: C/C++ PATCH to add -Wdangling-else option

2016-04-13 Thread Jakub Jelinek
On Wed, Apr 13, 2016 at 04:14:44PM +0200, Marek Polacek wrote:
> This patch is meant to be applied on top of the "Wparentheses overhaul" patch.
> 
> I really think that warning about the dangling else problem isn't appropriate
> as a part of the -Wparentheses warning, which I think should only deal with
> stuff like precedence of operators, i.e. things where ()'s are missing and not
> {}'s.
> 
> This new warning is, however, a subset of -Wparentheses.
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk or should I stash it
> for the next stage1?
> 
> 2016-04-13  Marek Polacek  
> 
>   * c.opt (Wdangling-else): New option.
> 
>   * c-parser.c (c_parser_if_statement): Replace OPT_Wparentheses with
>   OPT_Wdangling_else.
> 
>   * parser.c (cp_parser_selection_statement): Replace OPT_Wparentheses
>   with OPT_Wdangling_else.
> 
>   * doc/invoke.texi: Document -Wdangling-else.
> 
>   * c-c++-common/Wdangling-else-1.c: New test.
>   * c-c++-common/Wdangling-else-2.c: New test.
>   * c-c++-common/Wdangling-else-3.c: New test.

LGTM, though I think it would be useful to include in invoke.texi also a
small example with for to make it clear the option also handles that.

Jakub


Re: C/C++ PATCH to add -Wdangling-else option

2016-04-13 Thread Bernd Schmidt

On 04/13/2016 04:14 PM, Marek Polacek wrote:

This patch is meant to be applied on top of the "Wparentheses overhaul" patch.

I really think that warning about the dangling else problem isn't appropriate
as a part of the -Wparentheses warning, which I think should only deal with
stuff like precedence of operators, i.e. things where ()'s are missing and not
{}'s.

This new warning is, however, a subset of -Wparentheses.

Bootstrapped/regtested on x86_64-linux, ok for trunk or should I stash it
for the next stage1?


I think it's not appropriate for now. I'm ambivalent about the concept; 
my (vague) recollection is that putting it under -Wparentheses was 
Kenner's idea, and it's been there so long that I'm not sure there's 
really a point to changing this. In a sense it is a very similar problem 
as operator precedence.



Bernd


Re: C/C++ PATCH to add -Wdangling-else option

2016-04-13 Thread Jakub Jelinek
On Wed, Apr 13, 2016 at 05:13:28PM +0200, Bernd Schmidt wrote:
> On 04/13/2016 04:14 PM, Marek Polacek wrote:
> >This patch is meant to be applied on top of the "Wparentheses overhaul" 
> >patch.
> >
> >I really think that warning about the dangling else problem isn't appropriate
> >as a part of the -Wparentheses warning, which I think should only deal with
> >stuff like precedence of operators, i.e. things where ()'s are missing and 
> >not
> >{}'s.
> >
> >This new warning is, however, a subset of -Wparentheses.
> >
> >Bootstrapped/regtested on x86_64-linux, ok for trunk or should I stash it
> >for the next stage1?
> 
> I think it's not appropriate for now. I'm ambivalent about the concept; my
> (vague) recollection is that putting it under -Wparentheses was Kenner's
> idea, and it's been there so long that I'm not sure there's really a point
> to changing this. In a sense it is a very similar problem as operator
> precedence.

Well, even with the change it is still included with -Wparentheses, just
it is a suboption with more specific name that can be enabled/disabled
independently from -Wparentheses if needed.
Though, of course, it can wait for GCC 7.

Jakub


[committed, Fortran] PR67039 and PR58000 minor documentation changes

2016-04-13 Thread Dominique d'Humières
I have committed the following patches to trunk after Jerry’s approval on 
bugzilla. Is it OK to back port them to the gcc-5 branch?

Dominique

Index: gcc/fortran/ChangeLog
===
--- gcc/fortran/ChangeLog   (revision 234935)
+++ gcc/fortran/ChangeLog   (working copy)
@@ -1,3 +1,9 @@
+2016-04-13  Dominique d'Humieres  
+
+   PR fortran/58000
+   * gfortran.texi: Document OPEN( ... NAME=) as not implemented
+   in GNU Fortran
+
 2016-04-09  Jerry DeLisle  
 
PR fortran/68566
Index: gcc/fortran/gfortran.texi
===
--- gcc/fortran/gfortran.texi   (revision 234935)
+++ gcc/fortran/gfortran.texi   (working copy)
@@ -2148,6 +2148,7 @@
 @c * Omitted arguments in procedure call::
 * Alternate complex function syntax::
 * Volatile COMMON blocks::
+* OPEN( ... NAME=)::
 @end menu
 
 
@@ -2355,7 +2356,20 @@
 
 
 
+@node OPEN( ... NAME=)
+@subsection @code{OPEN( ... NAME=)}
+@cindex @code{NAM}
+
+Some Fortran compilers, including @command{g77}, let the user declare
+@code{OPEN( ... NAME=)}. This is
+invalid standard Fortran syntax and is not supported by
+@command{gfortran}.  @code{OPEN( ... NAME=)} should be replaced
+with @code{OPEN( ... FILE=)}.
+
+
+
 @c -
+@c -
 @c Mixed-Language Programming
 @c ——
Index: gcc/fortran/ChangeLog
===
--- gcc/fortran/ChangeLog   (revision 234942)
+++ gcc/fortran/ChangeLog   (working copy)
@@ -1,5 +1,11 @@
+2016-04-13  Dominique d'Humieres  
+
+   PR fortran/67039
+   * intrinsic.texi: Correct the documentation of pseudorandom
+   number intrinsics.
+
 2016-04-13  Dominique d'Humieres  
 
PR fortran/58000
* gfortran.texi: Document OPEN( ... NAME=) as not implemented
in GNU Fortran
Index: gcc/fortran/intrinsic.texi
===
--- gcc/fortran/intrinsic.texi  (revision 234942)
+++ gcc/fortran/intrinsic.texi  (working copy)
@@ -12628,10 +12628,12 @@
 See @code{RAND} and @code{IRAND} for examples.
 
 @item @emph{Notes}:
-The Fortran 2003 standard specifies the intrinsic @code{RANDOM_SEED} to
-initialize the pseudo-random numbers generator and @code{RANDOM_NUMBER}
-to generate pseudo-random numbers. Please note that in
-GNU Fortran, these two sets of intrinsics (@code{RAND},
+The Fortran standard specifies the intrinsic subroutines
+@code{RANDOM_SEED} to initialize the pseudo-random number
+generator and @code{RANDOM_NUMBER} to generate pseudo-random numbers.
+These subroutines should be used in new codes.
+
+Please note that in GNU Fortran, these two sets of intrinsics (@code{RAND},
 @code{IRAND} and @code{SRAND} on the one hand, @code{RANDOM_NUMBER} and
 @code{RANDOM_SEED} on the other hand) access two independent
 pseudo-random number generators.

 




Re: [C++ Patch] PR 70635 ([4.9/5/6 Regression] ICE on (and rejects) valid code...)

2016-04-13 Thread Jason Merrill

On 04/13/2016 11:05 AM, Paolo Carlini wrote:

As it happens, anyway, the below exceedingly trivial patch avoids the
ICE in resolve_typename_type and appears to pass testing (is already
past g++.dg/tm/tm.exp...). Should I apply it instead together with the
amended testcase? Weird that nobody noticed the pair of typos before!?!


OK.

Jason



PATCH for c++/70639 (ICE-on-valid with -Wmisleading-indentation and switch)

2016-04-13 Thread Marek Polacek
This is an ICE-on-valid-though-weirdo (TM) problem.  We were trying to warn
about misleading indentation for a switch statement, but guard_tinfo_to_string
doesn't know what to do with RID_SWITCH and so a crash ensues.  Rather than
teaching it about RID_SWITCH I think this warning can't usefully warn about
switch statements at all, similarly to do-while constructs.

David, what do you think?

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2016-04-13  Marek Polacek  

PR c++/70639
* c-indentation.c (should_warn_for_misleading_indentation): Bail out
for switch statements, too.

* c-c++-common/Wmisleading-indentation-4.c: New test.

diff --git gcc/c-family/c-indentation.c gcc/c-family/c-indentation.c
index 1da3f68..8c33686 100644
--- gcc/c-family/c-indentation.c
+++ gcc/c-family/c-indentation.c
@@ -239,10 +239,11 @@ should_warn_for_misleading_indentation (const 
token_indent_info &guard_tinfo,
   if (line_table->seen_line_directive)
 return false;
 
-  /* We can't usefully warn about do-while statements since the bodies of these
- statements are always explicitly delimited at both ends, so control flow 
is
- quite obvious.  */
-  if (guard_tinfo.keyword == RID_DO)
+  /* We can't usefully warn about do-while and switch statements since the
+ bodies of these statements are always explicitly delimited at both ends,
+ so control flow is quite obvious.  */
+  if (guard_tinfo.keyword == RID_DO
+  || guard_tinfo.keyword == RID_SWITCH)
 return false;
 
   /* If the token following the body is a close brace or an "else"
diff --git gcc/testsuite/c-c++-common/Wmisleading-indentation-4.c 
gcc/testsuite/c-c++-common/Wmisleading-indentation-4.c
index e69de29..d15a479 100644
--- gcc/testsuite/c-c++-common/Wmisleading-indentation-4.c
+++ gcc/testsuite/c-c++-common/Wmisleading-indentation-4.c
@@ -0,0 +1,11 @@
+/* PR c++/70639 */
+/* { dg-do compile } */
+/* { dg-options "-Wmisleading-indentation" } */
+
+void bar (int);
+void
+foo (int x)
+{
+  switch (x);
+   bar (x);
+}

Marek


Re: RFC: C++ PATCH to adjust empty class parameter passing ABI

2016-04-13 Thread Jakub Jelinek
On Tue, Apr 12, 2016 at 04:27:48PM -0400, Jason Merrill wrote:
> Unfortunately, a drawback of doing this in the front end is that it's
> difficult to warn only about affected cases; the front end doesn't know
> what's actually going to be emitted, and has to warn conservatively, leading
> to false positives particularly for inline functions.
> 
> The third patch is a sketch of an attempt to address this by delaying the
> warning until expand time.  We can't use front end information at this point
> because it has been thrown away by pass_ipa_free_lang_data, so I'm resorting
> to pre-generating the warning and stashing it away. This is several kinds of
> kludge, including hashing on a call location, but it greatly improves the
> accuracy of the warning.  I'm hoping that someone will have a better idea
> about how to approach this.

Is the intent that we emit different warnings between e.g.
-fkeep-inline-functions vs. without that, or -O0 vs. -O2?
Do you want to warn just on the out of line definitions (visible outside of
the containing TU), or also calls to those?

For e.g. warning on calls to those functions it could be similar to the
warning attribute - where we warn if a call to function with that attribute
is expanded - if it is inlined or optimized away etc., we don't warn.

For warnings on the definitions, perhaps stick it into the IL (could be
internal fn, we have e.g. IFN_ANNOTATE that can hold some info, though right
now it is removed quite early).  We'd need to remove it when inlining from
the IL, and decide what to do if such functions are e.g. cloned (SRA,
IPA-CP, ...).

Jakub


Re: C PATCH to overhaul warning about dangling else (PR c/70436)

2016-04-13 Thread Marek Polacek
On Wed, Apr 13, 2016 at 05:09:58PM +0200, Jakub Jelinek wrote:
> On Wed, Apr 13, 2016 at 04:14:03PM +0200, Marek Polacek wrote:
> > PR c/70436
> > * c-parser.c
> > (c_parser_statement_after_labels): Add IF_P argument and adjust
> > callers.
> 
> Can you put 
 
Were you going to say to put them on the same line?  ;)
Done.

> > @@ -5533,7 +5548,7 @@ c_parser_switch_statement (c_parser *parser)
> >c_start_case (switch_loc, switch_cond_loc, expr, explicit_cast_p);
> >save_break = c_break_label;
> >c_break_label = NULL_TREE;
> > -  body = c_parser_c99_block_statement (parser);
> > +  body = c_parser_c99_block_statement (parser, NULL/*if??*/);
> >c_finish_case (body, ce.original_type);
> >if (c_break_label)
> >  {
> 
> Is:
> void bar (int);
> 
> void
> foo (int a, int b, int c)
> {
>   if (a)
> switch (b)
>   case 0:
> if (c)
>   bar (1);
>   else
> bar (2);
> }
> something -Wdangling-else should warn about?  Note, g++ does not warn
> about it, and clang nor clang++ doesn't either.
> As it is something that unlikely happens in user code, I guess it is ok
> to not warn about this for GCC 6, and we can deal with that later on.

Seems quite artificial in the sense that I wouldn't expect many switch
statements to be lacking {}'s, but also I don't think that the warning
would be inappropriate here.  Since I already regtested my patch, I'll
go ahead with that, but sure, could be something to address in the future.

Marek


Re: C PATCH to overhaul warning about dangling else (PR c/70436)

2016-04-13 Thread Jakub Jelinek
On Wed, Apr 13, 2016 at 05:43:25PM +0200, Marek Polacek wrote:
> On Wed, Apr 13, 2016 at 05:09:58PM +0200, Jakub Jelinek wrote:
> > On Wed, Apr 13, 2016 at 04:14:03PM +0200, Marek Polacek wrote:
> > >   PR c/70436
> > >   * c-parser.c
> > >   (c_parser_statement_after_labels): Add IF_P argument and adjust
> > >   callers.
> > 
> > Can you put 
>  
> Were you going to say to put them on the same line?  ;)
> Done.

Sure, thank you for reading my mind.  Started thinking about the switch
case in the middle of writing that sentence and forgot about it.

Jakub


Re: [PATCH] c++/70594 debug info differences

2016-04-13 Thread Nathan Sidwell
This patch builds from Jason's proto-patch in comment #26. As Jason discovered, 
that led to link problems with gt_ggc_mx (tree_node *&) when building cc1. 
Making 'hash_map' GC-able caused gengtype to put the 
GC-walkers for tree into constexpr.c. No idea why. That's also what led me to 
add the user GTY stuff in the patch I posted earlier. For some reason manual GC 
fns kept the tree GC-walker in a sane place. I wonder if the same problem is why 
Patrick's original patch wrapped the hash_map pointer in a local structure?


Rather than peer into gengtype's mind, I figured on changing to use a regular 
tree->tree mapper, which didn't disturb gengtype. I employ a TREE_LIST to hold 
the bits that fundef_copy held (body, parms, result) on (PURPOSE, VALUE, TYPE). 
A little unclean, but not the first time non-types are on such a TYPE, IIRC. 
While there I noticed that the getter only needed to use a hash getter, rather 
than get_or_insert.


The thrust of the patch makes the fundef copies and constexpr call tables 
GCable, not GC-deletable. Thus their contents are not affected by GC 
occurrences. Finally, a new hook called at end of parsing to delete the 
constexpr call & fundef copies tables, so they don't remain after parsing.  We 
don't do anything about stopping them getting too big.


Patch survives boot & test, and fixes the testcase in 70594/#4 (without Jakub's 
patch to not emit UIDs in the gimple dump).


ok?

nathan

2016-04-13  Jason Merrill  
	Nathan Sidwell  

	PR c++/70594
	* constexpr.c (constexpr_call_table): Preserve in GC.
	(struct fundef_copy, struct fundef_copies_table_t):	Delete.
	(fundef_copies_table): Preserve in GC. Change to pointer to
	tree->tree hash.
	(maybe_initialize_fundef_copies_table): Adjust.
	(get_fundef_copy): Return a TREE_LIST.  Use non-inserting search.
	(save_fundef_copy): Adjust for a TREE_LIST.
	(cxx_eval_call_expression): Adjust for a fundef_copy TREE_LIST.
	(fini_constexpr): New.
	* cp-tree.h (fini_constexpr): Declare.
	* decl2.c (c_parse_final_cleanups): Call fini_constexpr.

Index: cp/constexpr.c
===
--- cp/constexpr.c	(revision 234934)
+++ cp/constexpr.c	(working copy)
@@ -915,7 +915,7 @@ struct constexpr_ctx {
 /* A table of all constexpr calls that have been evaluated by the
compiler in this translation unit.  */
 
-static GTY ((deletable)) hash_table *constexpr_call_table;
+static GTY (()) hash_table *constexpr_call_table;
 
 static tree cxx_eval_constant_expression (const constexpr_ctx *, tree,
 	  bool, bool *, bool *, tree * = NULL);
@@ -965,17 +965,6 @@ maybe_initialize_constexpr_call_table (v
 constexpr_call_table = hash_table::create_ggc (101);
 }
 
-/* The representation of a single node in the per-function freelist maintained
-   by FUNDEF_COPIES_TABLE.  */
-
-struct fundef_copy
-{
-  tree body;
-  tree parms;
-  tree res;
-  fundef_copy *prev;
-};
-
 /* During constexpr CALL_EXPR evaluation, to avoid issues with sharing when
a function happens to get called recursively, we unshare the callee
function's body and evaluate this unshared copy instead of evaluating the
@@ -983,45 +972,42 @@ struct fundef_copy
 
FUNDEF_COPIES_TABLE is a per-function freelist of these unshared function
copies.  The underlying data structure of FUNDEF_COPIES_TABLE is a hash_map
-   that's keyed off of the original FUNCTION_DECL and whose value is the chain
-   of this function's unused copies awaiting reuse.  */
+   that's keyed off of the original FUNCTION_DECL and whose value is a
+   TREE_LIST of this function's unused copies awaiting reuse.
 
-struct fundef_copies_table_t
-{
-  hash_map *map;
-};
+   This is not GC-deletable to avoid GC affecting UID generation.  */
 
-static GTY((deletable)) fundef_copies_table_t fundef_copies_table;
+static GTY(()) hash_map *fundef_copies_table;
 
 /* Initialize FUNDEF_COPIES_TABLE if it's not initialized.  */
 
 static void
 maybe_initialize_fundef_copies_table ()
 {
-  if (fundef_copies_table.map == NULL)
-fundef_copies_table.map = hash_map::create_ggc (101);
+  if (fundef_copies_table == NULL)
+fundef_copies_table = hash_map::create_ggc (101);
 }
 
 /* Reuse a copy or create a new unshared copy of the function FUN.
Return this copy.  */
 
-static fundef_copy *
+static tree
 get_fundef_copy (tree fun)
 {
   maybe_initialize_fundef_copies_table ();
 
-  fundef_copy *copy;
-  fundef_copy **slot = &fundef_copies_table.map->get_or_insert (fun, NULL);
-  if (*slot == NULL)
-{
-  copy = ggc_alloc ();
-  copy->body = copy_fn (fun, copy->parms, copy->res);
-  copy->prev = NULL;
+  tree copy;
+  tree *slot = fundef_copies_table->get (fun);
+  if (slot == NULL)
+{
+  copy = build_tree_list (NULL, NULL);
+  /* PURPOSE is body, VALUE is parms, TYPE is result.  */
+  TREE_PURPOSE (copy) = copy_fn (fun, TREE_VALUE (copy), TREE_TYPE (copy));
 }
   else
 {
   copy = *slot;
-  *slot = (*slot)->prev;

Re: RFC: C++ PATCH to adjust empty class parameter passing ABI

2016-04-13 Thread Jason Merrill

On 04/13/2016 11:32 AM, Jakub Jelinek wrote:

On Tue, Apr 12, 2016 at 04:27:48PM -0400, Jason Merrill wrote:

Unfortunately, a drawback of doing this in the front end is that it's
difficult to warn only about affected cases; the front end doesn't know
what's actually going to be emitted, and has to warn conservatively, leading
to false positives particularly for inline functions.

The third patch is a sketch of an attempt to address this by delaying the
warning until expand time.  We can't use front end information at this point
because it has been thrown away by pass_ipa_free_lang_data, so I'm resorting
to pre-generating the warning and stashing it away. This is several kinds of
kludge, including hashing on a call location, but it greatly improves the
accuracy of the warning.  I'm hoping that someone will have a better idea
about how to approach this.


Is the intent that we emit different warnings between e.g.
-fkeep-inline-functions vs. without that, or -O0 vs. -O2?
Do you want to warn just on the out of line definitions (visible outside of
the containing TU), or also calls to those?


That's what I'm thinking, since only actually emitted definitions/calls 
have ABI.



For e.g. warning on calls to those functions it could be similar to the
warning attribute - where we warn if a call to function with that attribute
is expanded - if it is inlined or optimized away etc., we don't warn.


Ah, that sounds promising, thanks.

Jason



Re: C PATCH to overhaul warning about dangling else (PR c/70436)

2016-04-13 Thread Marek Polacek
On Wed, Apr 13, 2016 at 05:00:00PM +0200, Bernd Schmidt wrote:
> On 04/13/2016 04:14 PM, Marek Polacek wrote:
> >I revamped the warning so that it follows what the C++ FE does (i.e.  passing
> >IF_P bools here and there) and it seems to work quite well.  I didn't mean to
> >tackle the OMP bits but I bet it would be just a matter of passing IF_P
> >somewhere.
> >
> >Bootstrapped/regtested on x86_64-linux, ok for trunk?
> 
> Ok if you adjust function comments to mention the new argument where
> applicable.

I kept telling myself to not forget to add the commentary and I did all the
same.  Fixed in the below.  Thanks for reviewing!

2016-04-13  Marek Polacek  

PR c/70436
* c-parser.c (c_parser_statement_after_labels): Add IF_P argument and
adjust callers.
(c_parser_statement): Likewise.
(c_parser_c99_block_statement): Likewise.
(c_parser_while_statement): Likewise.
(c_parser_for_statement): Likewise.
(c_parser_if_body): Don't set IF_P here.
(c_parser_if_statement): Add IF_P argument.  Set IF_P here.  Warn
about dangling else here.
* c-tree.h (c_finish_if_stmt): Adjust declaration.
* c-typeck.c (c_finish_if_stmt): Remove NESTED_IF parameter.  Don't
warn about dangling else here.

* testsuite/gcc.dg/Wparentheses-12.c: New test.
* testsuite/gcc.dg/Wparentheses-13.c: New test.

diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index 6460684..d37c691 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -1301,13 +1301,14 @@ static void c_parser_initval (c_parser *, struct c_expr 
*,
 static tree c_parser_compound_statement (c_parser *);
 static void c_parser_compound_statement_nostart (c_parser *);
 static void c_parser_label (c_parser *);
-static void c_parser_statement (c_parser *);
-static void c_parser_statement_after_labels (c_parser *, vec * = NULL);
-static void c_parser_if_statement (c_parser *, vec *);
+static void c_parser_statement (c_parser *, bool *);
+static void c_parser_statement_after_labels (c_parser *, bool *,
+vec * = NULL);
+static void c_parser_if_statement (c_parser *, bool *, vec *);
 static void c_parser_switch_statement (c_parser *);
-static void c_parser_while_statement (c_parser *, bool);
+static void c_parser_while_statement (c_parser *, bool, bool *);
 static void c_parser_do_statement (c_parser *, bool);
-static void c_parser_for_statement (c_parser *, bool);
+static void c_parser_for_statement (c_parser *, bool, bool *);
 static tree c_parser_asm_statement (c_parser *);
 static tree c_parser_asm_operands (c_parser *);
 static tree c_parser_asm_goto_operands (c_parser *);
@@ -4853,7 +4854,7 @@ c_parser_compound_statement_nostart (c_parser *parser)
  last_label = false;
  last_stmt = true;
  mark_valid_location_for_stdc_pragma (false);
- c_parser_statement_after_labels (parser);
+ c_parser_statement_after_labels (parser, NULL);
}
 
   parser->error = false;
@@ -5095,25 +5096,35 @@ c_parser_label (c_parser *parser)
statement:
  transaction-statement
  transaction-cancel-statement
-*/
+
+   IF_P is used to track whether there's a (possibly labeled) if statement
+   which is not enclosed in braces and has an else clause.  This is used to
+   implement -Wparentheses.  */
 
 static void
-c_parser_statement (c_parser *parser)
+c_parser_statement (c_parser *parser, bool *if_p)
 {
   c_parser_all_labels (parser);
-  c_parser_statement_after_labels (parser);
+  c_parser_statement_after_labels (parser, if_p, NULL);
 }
 
 /* Parse a statement, other than a labeled statement.  CHAIN is a vector
-   of if-else-if conditions.  */
+   of if-else-if conditions.
+
+   IF_P is used to track whether there's a (possibly labeled) if statement
+   which is not enclosed in braces and has an else clause.  This is used to
+   implement -Wparentheses.  */
 
 static void
-c_parser_statement_after_labels (c_parser *parser, vec *chain)
+c_parser_statement_after_labels (c_parser *parser, bool *if_p,
+vec *chain)
 {
   location_t loc = c_parser_peek_token (parser)->location;
   tree stmt = NULL_TREE;
   bool in_if_block = parser->in_if_block;
   parser->in_if_block = false;
+  if (if_p != NULL)
+*if_p = false;
   switch (c_parser_peek_token (parser)->type)
 {
 case CPP_OPEN_BRACE:
@@ -5123,19 +5134,19 @@ c_parser_statement_after_labels (c_parser *parser, 
vec *chain)
   switch (c_parser_peek_token (parser)->keyword)
{
case RID_IF:
- c_parser_if_statement (parser, chain);
+ c_parser_if_statement (parser, if_p, chain);
  break;
case RID_SWITCH:
  c_parser_switch_statement (parser);
  break;
case RID_WHILE:
- c_parser_while_statement (parser, false);
+ c_parser_while_statement (parser, false, if_p);
  break;
case RID_DO:
  c_parser_do_statemen

Re: [PATCH, rs6000] Add support for int versions of vec_adde

2016-04-13 Thread Bill Seurer

Segher pointed out a few formatting issues via IRC which I have fixed.

On 04/13/16 09:47, Bill Seurer wrote:

Here is an updated patch:


This patch adds support for the signed and unsigned int versions of the
vec_adde altivec builtins from the Power Architecture 64-Bit ELF V2 ABI
OpenPOWER ABI for Linux Supplement (16 July 2015 Version 1.1). There are
many of the builtins that are missing and this is the first of a series
of patches to add them.

There aren't instructions for the int versions of vec_adde so the
output code is built from other built-ins that do have instructions
which in this case is just two vec_adds.

The new test cases are executable tests which verify that the generated
code produces expected values. C macros were used so that the same
test case could be used for both the signed and unsigned versions. An
extra executable test case is also included to ensure that the modified
support for the __int128 versions of vec_adde is not broken. The same
test case could not be used for both int and __int128 because of some
differences in loading and storing the vectors.

Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
regressions. Is this ok for trunk?

[gcc]

2016-04-06 Bill Seurer 

* config/rs6000/rs6000-builtin.def (vec_adde): Change vec_adde to a
special case builtin.
* config/rs6000/rs6000-c.c (altivec_overloaded_builtins): Remove
ALTIVEC_BUILTIN_VEC_ADDE.
* config/rs6000/rs6000-c.c (altivec_resolve_overloaded_builtin): Add
support for ALTIVEC_BUILTIN_VEC_ADDE.
* config/rs6000/rs6000.c (altivec_init_builtins): Add definition
for __builtin_vec_adde.

[gcc/testsuite]

2016-04-06 Bill Seurer 

* gcc.target/powerpc/vec-adde.c: New test.
* gcc.target/powerpc/vec-adde-int128.c: New test.


Index: gcc/config/rs6000/rs6000-builtin.def
===
--- gcc/config/rs6000/rs6000-builtin.def(revision 234936)
+++ gcc/config/rs6000/rs6000-builtin.def(working copy)
@@ -951,7 +951,6 @@ BU_ALTIVEC_X (VEC_EXT_V4SF, "vec_ext_v4sf", CO
 before we get to the point about classifying the builtin type.  */

  /* 3 argument Altivec overloaded builtins.  */
-BU_ALTIVEC_OVERLOAD_3 (ADDE,  "adde")
  BU_ALTIVEC_OVERLOAD_3 (ADDEC,"addec")
  BU_ALTIVEC_OVERLOAD_3 (MADD,   "madd")
  BU_ALTIVEC_OVERLOAD_3 (MADDS,  "madds")
@@ -1137,6 +1136,7 @@ BU_ALTIVEC_OVERLOAD_P (VCMPGT_P,   "vcmpgt_p")
  BU_ALTIVEC_OVERLOAD_P (VCMPGE_P,   "vcmpge_p")

  /* Overloaded Altivec builtins that are handled as special cases.  */
+BU_ALTIVEC_OVERLOAD_X (ADDE,  "adde")
  BU_ALTIVEC_OVERLOAD_X (CTF,  "ctf")
  BU_ALTIVEC_OVERLOAD_X (CTS,  "cts")
  BU_ALTIVEC_OVERLOAD_X (CTU,  "ctu")
Index: gcc/config/rs6000/rs6000-c.c
===
--- gcc/config/rs6000/rs6000-c.c(revision 234936)
+++ gcc/config/rs6000/rs6000-c.c(working copy)
@@ -842,11 +842,6 @@ const struct altivec_builtin_types altivec_overloa
  RS6000_BTI_unsigned_V1TI, 0 },
{ ALTIVEC_BUILTIN_VEC_ADDC, P8V_BUILTIN_VADDCUQ,
  RS6000_BTI_V1TI, RS6000_BTI_V1TI, RS6000_BTI_V1TI, 0 },
-  { ALTIVEC_BUILTIN_VEC_ADDE, P8V_BUILTIN_VADDEUQM,
-RS6000_BTI_unsigned_V1TI, RS6000_BTI_unsigned_V1TI,
-RS6000_BTI_unsigned_V1TI, RS6000_BTI_unsigned_V1TI },
-  { ALTIVEC_BUILTIN_VEC_ADDE, P8V_BUILTIN_VADDEUQM,
-RS6000_BTI_V1TI, RS6000_BTI_V1TI, RS6000_BTI_V1TI, RS6000_BTI_V1TI },
{ ALTIVEC_BUILTIN_VEC_ADDEC, P8V_BUILTIN_VADDECUQ,
  RS6000_BTI_unsigned_V1TI, RS6000_BTI_unsigned_V1TI,
  RS6000_BTI_unsigned_V1TI, RS6000_BTI_unsigned_V1TI },
@@ -4515,6 +4510,62 @@ assignment for unaligned loads and stores");
  warning (OPT_Wdeprecated, "vec_lvsr is deprecated for little endian; use \
  assignment for unaligned loads and stores");

+  if (fcode == ALTIVEC_BUILTIN_VEC_ADDE)
+{
+  /* vec_adde needs to be special cased because there is no instruction
+ for the {un}signed int version.  */
+  if (nargs != 3)
+   {
+ error ("vec_adde only accepts 3 arguments");
+ return error_mark_node;
+   }
+
+  tree arg0 = (*arglist)[0];
+  tree arg0_type = TREE_TYPE (arg0);
+  tree arg1 = (*arglist)[1];
+  tree arg1_type = TREE_TYPE (arg1);
+  tree arg2 = (*arglist)[2];
+  tree arg2_type = TREE_TYPE (arg2);
+
+  /* All 3 arguments must be vectors of (signed or unsigned) (int or
+ __int128) and the types must match.  */
+  if ((arg0_type != arg1_type) || (arg1_type != arg2_type))
+   goto bad;
+  if (TREE_CODE (arg0_type) != VECTOR_TYPE)
+   goto bad;
+
+  switch (TYPE_MODE (TREE_TYPE (arg0_type)))
+   {
+ /* For {un}signed ints,
+vec_adde (va, vb, carryv) == vec_add (vec_add (va, vb), vec_and 
(carryv, 0x1)).  */
+ case SImode:
+   {
+ vec *params = make_tree_vector();
+ vec_safe_push (params, arg0);
+ vec_safe_push (params, a

Re: Splitting up gcc/omp-low.c?

2016-04-13 Thread Thomas Schwinge
Hi!

On Fri, 08 Apr 2016 11:36:03 +0200, I wrote:
> On Thu, 10 Dec 2015 09:08:35 +0100, Jakub Jelinek  wrote:
> > On Wed, Dec 09, 2015 at 06:23:22PM +0100, Bernd Schmidt wrote:
> > > On 12/09/2015 05:24 PM, Thomas Schwinge wrote:
> > > >how about we split up gcc/omp-low.c into several
> > > >files?  Would it make sense (I have not yet looked in detail) to do so
> > > >along the borders of the several passes defined therein?

> > > I suspect a split along the ompexp/omplow boundary would be quite easy to
> > > achieve.

That was indeed the first one that I tackled, omp-expand.c (spelled out
"expand" instead of "exp" to avoid confusion as "exp" might also be short
for "expression"; OK?), and a omp-offload.c also fell out of that (with
more content to be moved into there, I suspect).

We could split up omp-offload.c even further, but I don't know if that's
really feasible.  Currently in there: offload tables stuff, OpenACC loops
stuff and pass_oacc_device_lower, pass_omp_target_link; separated by ^L
in this one file.

> And possibly some kind of omp-simd.c, and omp-checking.c, and so
> on, if feasible.  (I have not yet looked in detail.)

Not yet looked into these.

Stuff that does not relate to OMP lowering, I did not move stuff out of
omp-low.c (into a new omp.c, or omp-misc.c, for example) so far, but
instead just left all that in omp-low.c.  We'll see how far we get.

One thing I noticed is that there sometimes is more than one suitable
place to put stuff: omp-low.c and omp-expand.c categorize by compiler
passes, and omp-offload.c -- at least in part -- is about the orthogonal
"offloading" category.  For example, see the OMPTODO "struct oacc_loop
and enum oacc_loop_flags" in gcc/omp-offload.h.  We'll see how that goes.

> > > >I'd suggest to do this shortly before GCC 6 is released, [...]

Here is a first variant of such a patch.  I will continue to maintain
this, and intend to send (incremental?) patches on top of that one, but
intend to eventually commit all changes as one big commit, to avoid too
much "source code churn" (as much as that's possible...).

Some more comments, to help review:

The attached 0001-Split-up-gcc-omp-low.c.patch.xz is a Git "--color
--word-diff --ignore-space-change" patch, purely meant for manual review;
I'm intentionally ;-) not attaching a "patch-applyable" patch at this
point, to minimize the risk of other people starting to work on this in
parallel with my ongoing changes, which no doubt would result in a ton of
patch merge conflicts.  Yet, I'm of course very open to any kind of
suggestions; please submit these as a "verbal patch".  I will of course
submit a patch in any other format that you'd like for review.

This already survived "light" C/C++/Fortran
--enable-offload-targets=nvptx-none,x86_64-intelmicemul-linux-gnu,hsa
testing (no HSA execution testing, though), and also survived a "big"
bootstrap build.

As I don't know how this is usually done: is it appropriate to remove
"Contributed by Diego Novillo" from omp-low.c (he does get mentioned for
his OpenMP work in gcc/doc/contrib.texi; a ton of other people have been
contributing a ton of other stuff since omp-low.c has been created), or
does this line stay in omp-low.c, or do I even duplicate it into the new
files?

I tried not to re-order stuff when moving.  But: we may actually want to
reorder stuff, to put it into a more sensible order.  Any suggestions?

All lines with "//OMP" tags in them will eventually be removed; these are
just to help review (hence the --word-diff patch), and to solicit
comments, in the case of "//OMPTODO".  Some of the OMPTODOs are for
myself (clean up #include directives), but for the others, I'd like to
hear any comments that you have.

I guess you can just ignore any "//OMPCUT" tags (and I'll remove them at
one point, and clean up the whitespace).  (In the new files) these mean
that in the file where the surrounding stuff is from, there has been
other stuff that either remained in the original file (omp-low.c), or has
been moved to other files.

In omp-low.c and omp-low.h, a "//OMPLOWH" tag means that this line has
been moved to omp-low.h, and likewise: "//OMPEXP" to omp-expand.c,
"//OMPEXPH" to omp-expand.h, "//OMPOFF" to omp-offload.c, and "//OMPOFFH"
to omp-offload.h.

I had to export a small number of functions (see the prototypes not moved
but added to the header files).

Because it's also used in omp-expand.c, I moved the one-line static
inline is_reference function from omp-low.c to omp-low.h, and renamed it
to omp_is_reference because of the very generic name.  Similar functions
stay in omp-low.c however, so they're no longer defined next to each
other.  OK, or does this need a different solution?


Grüße
 Thomas




0001-Split-up-gcc-omp-low.c.patch.xz
Description: application/xz


Re: PATCH for c++/70639 (ICE-on-valid with -Wmisleading-indentation and switch)

2016-04-13 Thread David Malcolm
On Wed, 2016-04-13 at 17:31 +0200, Marek Polacek wrote:
> This is an ICE-on-valid-though-weirdo (TM) problem.  We were trying
> to warn
> about misleading indentation for a switch statement, but
> guard_tinfo_to_string
> doesn't know what to do with RID_SWITCH and so a crash ensues. 
>  Rather than
> teaching it about RID_SWITCH I think this warning can't usefully warn
> about
> switch statements at all, similarly to do-while constructs.
> 
> David, what do you think?

My first thought was that warn_for_misleading_indentation is only meant
to be called by the frontend on if/else/while/for constructs, and I
wondered how it was getting called on a "switch" statement.  Answering
my own question, I see that cp_parser_selection_statement handles both
"if" and "switch" in the C++ FE, and builds the guard_tinfo, and passes
it to cp_parser_implicitly_scoped_statement.  Looks like I missed this
possibility.

So another way to fix this would be to update
 cp_parser_implicitly_scoped_statement to wrap this:

  token_indent_info next_tinfo
= get_token_indent_info (cp_lexer_peek_token (parser->lexer));
  warn_for_misleading_indentation (guard_tinfo, body_tinfo, next_tinfo);

so it only happens if we have an "if" token, not for a "switch".

Your approach encapsulates the logic for rejecting this situation
within should_warn_for_misleading_indentation, rather than at the
callers, which is arguably better for modularity (similar to how we
already call it for "do", which is then always rejected).

So your patch looks good to me (I don't have formal approval rights for
this though).

> Bootstrapped/regtested on x86_64-linux, ok for trunk?
> 
> 2016-04-13  Marek Polacek  
> 
>   PR c++/70639
>   * c-indentation.c (should_warn_for_misleading_indentation):
> Bail out
>   for switch statements, too.
> 
>   * c-c++-common/Wmisleading-indentation-4.c: New test.
> 
> diff --git gcc/c-family/c-indentation.c gcc/c-family/c-indentation.c
> index 1da3f68..8c33686 100644
> --- gcc/c-family/c-indentation.c
> +++ gcc/c-family/c-indentation.c
> @@ -239,10 +239,11 @@ should_warn_for_misleading_indentation (const
> token_indent_info &guard_tinfo,
>if (line_table->seen_line_directive)
>  return false;
>  
> -  /* We can't usefully warn about do-while statements since the
> bodies of these
> - statements are always explicitly delimited at both ends, so
> control flow is
> - quite obvious.  */
> -  if (guard_tinfo.keyword == RID_DO)
> +  /* We can't usefully warn about do-while and switch statements
> since the
> + bodies of these statements are always explicitly delimited at
> both ends,
> + so control flow is quite obvious.  */
> +  if (guard_tinfo.keyword == RID_DO
> +  || guard_tinfo.keyword == RID_SWITCH)
>  return false;
>  
>/* If the token following the body is a close brace or an "else"
> diff --git gcc/testsuite/c-c++-common/Wmisleading-indentation-4.c
> gcc/testsuite/c-c++-common/Wmisleading-indentation-4.c
> index e69de29..d15a479 100644
> --- gcc/testsuite/c-c++-common/Wmisleading-indentation-4.c
> +++ gcc/testsuite/c-c++-common/Wmisleading-indentation-4.c
> @@ -0,0 +1,11 @@
> +/* PR c++/70639 */
> +/* { dg-do compile } */
> +/* { dg-options "-Wmisleading-indentation" } */
> +
> +void bar (int);
> +void
> +foo (int x)
> +{
> +  switch (x);
> + bar (x);
> +}
> 
>   Marek


[patch] fix an openacc reduction test for PR68242

2016-04-13 Thread Cesar Philippidis
As stated in PR68242, the OpenACC libgomp reduction test reduction-2.c
has one test case involving a multiplication reduction which overflows
and that results in a failure on i686-linux host targets. This patch
adjusts the reduction size to prevent the overflow from happening. I've
also adjusted reduction-1.c because it can also overflow.

This patch has been tested on i686-linux and x86_64-linux. I'll apply
this patch to trunk as obvious shortly.

Cesar
2016-04-13  Cesar Philippidis  

	libgomp/
	PR testsuite/68242
	* testsuite/libgomp.oacc-c-c++-common/reduction-1.c: Adjust test.
	* testsuite/libgomp.oacc-c-c++-common/reduction-2.c: Likewise.

diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-1.c b/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-1.c
index 10eb278..e8a8911 100644
--- a/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-1.c
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-1.c
@@ -15,7 +15,7 @@ const int vl = 32;
 static void
 test_reductions (void)
 {
-  const int n = 100;
+  const int n = 10;
   int i;
   int array[n];
 
diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-2.c b/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-2.c
index 7cb9497..d19b1c8 100644
--- a/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-2.c
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-2.c
@@ -15,7 +15,7 @@ const int vl = 32;
 static void
 test_reductions (void)
 {
-  const int n = 100;
+  const int n = 10;
   int i;
   float array[n];
 


Re: PATCH for c++/70639 (ICE-on-valid with -Wmisleading-indentation and switch)

2016-04-13 Thread Bernd Schmidt

On 04/13/2016 06:11 PM, David Malcolm wrote:

Your approach encapsulates the logic for rejecting this situation
within should_warn_for_misleading_indentation, rather than at the
callers, which is arguably better for modularity (similar to how we
already call it for "do", which is then always rejected).

So your patch looks good to me (I don't have formal approval rights for
this though).


I'll ack it, I also thought it looked reasonable.


Bernd



Re: [PATCH, rs6000] Add support for int versions of vec_adde

2016-04-13 Thread David Edelsohn
On Wed, Apr 13, 2016 at 10:47 AM, Bill Seurer  wrote:
> Here is an updated patch:
>
>
> This patch adds support for the signed and unsigned int versions of the
> vec_adde altivec builtins from the Power Architecture 64-Bit ELF V2 ABI
> OpenPOWER ABI for Linux Supplement (16 July 2015 Version 1.1). There are
> many of the builtins that are missing and this is the first of a series
> of patches to add them.
>
> There aren't instructions for the int versions of vec_adde so the
> output code is built from other built-ins that do have instructions
> which in this case is just two vec_adds.
>
> The new test cases are executable tests which verify that the generated
> code produces expected values. C macros were used so that the same
> test case could be used for both the signed and unsigned versions. An
> extra executable test case is also included to ensure that the modified
> support for the __int128 versions of vec_adde is not broken. The same
> test case could not be used for both int and __int128 because of some
> differences in loading and storing the vectors.
>
> Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
> regressions. Is this ok for trunk?
>
> [gcc]
>
> 2016-04-06 Bill Seurer 
>
> * config/rs6000/rs6000-builtin.def (vec_adde): Change vec_adde to a
> special case builtin.
> * config/rs6000/rs6000-c.c (altivec_overloaded_builtins): Remove
> ALTIVEC_BUILTIN_VEC_ADDE.
> * config/rs6000/rs6000-c.c (altivec_resolve_overloaded_builtin): Add
> support for ALTIVEC_BUILTIN_VEC_ADDE.
> * config/rs6000/rs6000.c (altivec_init_builtins): Add definition
> for __builtin_vec_adde.
>
> [gcc/testsuite]
>
> 2016-04-06 Bill Seurer 
>
> * gcc.target/powerpc/vec-adde.c: New test.
> * gcc.target/powerpc/vec-adde-int128.c: New test.

The revised patch is okay for GCC 7, with the changes requested by Segher.

Thanks, David


Re: [PATCH] avoid false positives in trailing operator rule in check_GNU_style.sh

2016-04-13 Thread Jeff Law

On 04/04/2016 11:59 AM, Martin Sebor wrote:

My recent change to check_GNU_style.sh (commit 1bbf98) introduces
false positives for declarations of functions with a pointer return
type.  For example, in the following, the asterisk in the "void *"
return type is incorrectly diagnosed as a trailing operator:

   -void
   -foo (void);
   +void *
   +foo (int);

The patch below avoids those false positive for file-scope and
namespace-scope functions.

Martin

index fbf6cb2..a7478f8 100755
--- a/contrib/check_GNU_style.sh
+++ b/contrib/check_GNU_style.sh
@@ -240,6 +240,7 @@ g 'There should be no space before closing
parenthesis.' \
  g 'Braces should be on a separate line.' \
  '(\)|else)[[:blank:]]*{'

-# Does this apply to definition of aggregate objects?
-g 'Trailing operator.' \
+# Does this apply to definitions of aggregate objects?
+ag 'Trailing operator.' \
+  '^[1-9][0-9]*:\+[[:space:]]' \
'(([^a-zA-Z_]\*)|([-%<=&|^?])|([^*]/)|([^:][+]))$'

OK.
jeff


[PATCH] Fix ICE caused by nothrow pass (PR c++/70641)

2016-04-13 Thread Jakub Jelinek
Hi!

If the nothrow pass (which is GIMPLE, not IPA pass) determines that the
current function can't throw externally, it marks it nothrow.
But it fails to adjust self-recursive calls, which might change because of
that from possibly throwing to not being able to throw, so we need to
cleanup the EH region, purge dead eh edges and adjust CFG if needed.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2016-04-13  Jakub Jelinek  

PR c++/70641
* ipa-pure-const.c (pass_nothrow::execute): Call maybe_clean_eh_stmt
on all recursive call stmts.  Return TODO_cleanup_cfg if any dead
eh edges have been purged.

* g++.dg/opt/pr70641.C: New test.

--- gcc/ipa-pure-const.c.jj 2016-04-13 15:53:01.051799840 +0200
+++ gcc/ipa-pure-const.c2016-04-13 16:03:10.715556547 +0200
@@ -1956,10 +1956,28 @@ pass_nothrow::execute (function *)
 }
 
   node->set_nothrow_flag (true);
+
+  bool cfg_changed = false;
+  if (self_recursive_p (node))
+FOR_EACH_BB_FN (this_block, cfun)
+  for (gimple_stmt_iterator gsi = gsi_start_bb (this_block);
+   !gsi_end_p (gsi);
+   gsi_next (&gsi))
+   if (is_gimple_call (gsi_stmt (gsi)))
+ {
+   gimple *g = gsi_stmt (gsi);
+   tree callee_t = gimple_call_fndecl (g);
+   if (callee_t
+   && recursive_call_p (current_function_decl, callee_t)
+   && maybe_clean_eh_stmt (g)
+   && gimple_purge_dead_eh_edges (gimple_bb (g)))
+ cfg_changed = true;
+ }
+
   if (dump_file)
 fprintf (dump_file, "Function found to be nothrow: %s\n",
 current_function_name ());
-  return 0;
+  return cfg_changed ? TODO_cleanup_cfg : 0;
 }
 
 } // anon namespace
--- gcc/testsuite/g++.dg/opt/pr70641.C.jj   2016-04-13 16:01:16.381102468 
+0200
+++ gcc/testsuite/g++.dg/opt/pr70641.C  2016-04-13 16:01:16.381102468 +0200
@@ -0,0 +1,10 @@
+// PR c++/70641
+// { dg-do compile }
+// { dg-options "-O2" }
+
+void
+foo ()
+{
+  try { foo (); }
+  catch (...) { __builtin_abort (); }
+}

Jakub


Re: Unreviewed patch

2016-04-13 Thread Jeff Law

On 04/06/2016 04:58 AM, Rainer Orth wrote:

The following patch has remainded unreviewed for a week:

[testsuite, sparcv9] Fix gcc.dg/ifcvt-4.c on 64-bit SPARC (PR 
rtl-optimization/68749)
https://gcc.gnu.org/ml/gcc-patches/2016-03/msg01631.html

Although it's testsuite-only, I'm quite reluctant to make potential
semantic changes to a testcase without approval from the subject-matter
expert.

It's OK for the trunk.

jeff


Re: Splitting up gcc/omp-low.c?

2016-04-13 Thread Bernd Schmidt

On 04/13/2016 06:01 PM, Thomas Schwinge wrote:


The attached 0001-Split-up-gcc-omp-low.c.patch.xz is a Git "--color
--word-diff --ignore-space-change" patch, purely meant for manual review;
I'm intentionally ;-) not attaching a "patch-applyable" patch at this
point, to minimize the risk of other people starting to work on this in
parallel with my ongoing changes, which no doubt would result in a ton of
patch merge conflicts.  Yet, I'm of course very open to any kind of
suggestions; please submit these as a "verbal patch".  I will of course
submit a patch in any other format that you'd like for review.


I have no idea how to read this patch. I can't even properly show it 
with "less" because it seems to contain color escape sequences. The 
word-diff format (I assume that's what it is) is also unfamiliar and not 
immediately readable.


Best way to present this might be to do
diff -du old-omp-low.c .


Bernd


Re: [PATCH] Make distclean clean up more (PR70173)

2016-04-13 Thread Jeff Law

On 04/11/2016 09:57 AM, Segher Boessenkool wrote:

Currently, distclean does not remove the libcc1, gnattools, and gotools
directories, and not the stage_final file either.  Fix that.

This finishes PR70173.

Tested on powerpc64-linux, --enable-languages=all,ada,go,obj-c++ ,
followed by "make distclean".  Is this okay for trunk?


Segher


2016-04-11  Segher Boessenkool  

PR bootstrap/70173
* Makefile.tpl (local-distclean): Delete the libcc1, gnattools,
and gotools directories.  Delete the stage_final file.
* Makefile.in: Regenerate.

Not really a regression, but I think this is an OK fix even at this stage.


jeff



Re: Splitting up gcc/omp-low.c?

2016-04-13 Thread Bernd Schmidt

On 04/13/2016 07:56 PM, Thomas Schwinge wrote:


Best way to present this might be to do
diff -du old-omp-low.c .


OK, I found Git "-C5%" produce something very similar to that;
0001-Split-up-gcc-omp-low.c-plain.patch.xz attached.


That looks much better. However, the //OMPWHATEVER comments are not 
really all that helpful. I think the next step would be to clear out all 
this stuff including the OMPCUT markers, and also to start with an 
initial patch that includes everything that actually modifies code 
rather than moving it (omp_is_reference seems to be the major thing here).



Bernd



C++ PATCH for c++/70627 (ICE with opaque enum)

2016-04-13 Thread Jason Merrill
Here the redeclaration of H changed the underlying type from a typedef 
to a non-typedef, leading to confusion in the back end.  We shouldn't 
change the underlying type from redeclarations.


Tested x86_64-pc-linux-gnu, applying to trunk.
commit bfabb2dff84a409cf856cda7959ff4395f073d8b
Author: Jason Merrill 
Date:   Wed Apr 13 11:43:59 2016 -0400

	PR c++/70627

	* decl.c (start_enum): Don't change an existing ENUM_UNDERLYING_TYPE.

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 8867b43..0326900 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -13257,7 +13257,10 @@ start_enum (tree name, tree enumtype, tree underlying_type,
 
   if (underlying_type)
 {
-  if (CP_INTEGRAL_TYPE_P (underlying_type))
+  if (ENUM_UNDERLYING_TYPE (enumtype))
+	/* We already checked that it matches, don't change it to a different
+	   typedef variant.  */;
+  else if (CP_INTEGRAL_TYPE_P (underlying_type))
 {
 	  copy_type_enum (enumtype, underlying_type);
   ENUM_UNDERLYING_TYPE (enumtype) = underlying_type;
diff --git a/gcc/testsuite/g++.dg/cpp0x/enum_base3.C b/gcc/testsuite/g++.dg/cpp0x/enum_base3.C
new file mode 100644
index 000..3cb2d6d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/enum_base3.C
@@ -0,0 +1,29 @@
+// PR c++/70627
+// { dg-do compile { target c++11 } }
+
+struct D;
+struct A
+{
+  D *operator->();
+};
+struct B
+{
+  template  void foo (T &&...) {}
+};
+typedef unsigned char G;
+enum class H : G;
+struct C
+{
+};
+struct D : C
+{
+  B foo () const { B a; a.foo (d); }
+  H d;
+};
+struct F : C
+{
+  void foo ();
+  A f;
+};
+enum class H : unsigned char;
+void F::foo () { B b = f->foo (); }


Re: [PATCH] 69517 - [5/6 regression] SEGV on a VLA with excess initializer elements

2016-04-13 Thread Martin Sebor

On 04/12/2016 12:17 PM, Jason Merrill wrote:

On 04/10/2016 07:14 PM, Martin Sebor wrote:

The call to build_vla_check() in check_initializer() is to check
only explicitly initialized VLAs.  The latter function may need
to complete the VLA element type and reshape the initializer so
the call cannot be made earlier.  The function returns the runtime
initializer code so the call cannot be made after it returns.


But this call isn't checking the initializer; we're looking at the
initializer in build_vec_init now.


Let me try to explain.  Strings aside, there are three cases each
of which the patch handles in a different function:

1) The No Initializer case is handled by calling build_vla_check
directly from cp_finish_decl.

2) The Empty initializer case is handled from check_initializer
in the hunk you had pasted upthread.  (It isn't handled in
build_vec_init because length_check is false for empty
initializers, and because it's too late to check VLA bounds
there anyway -- see below.)

3) In the Non-empty Initializer case, excess initializer elements
are checked for in build_vec_init.  VLA bounds are checked from
check_initializer same as in (2).

As I mentioned last Friday, emitting the check for the VLA bounds
in build_vec_init appears to be too late because by then the code
to allocate the stack has already been emitted.  Maybe there's
a way to do it, I don't know.  Controlling what piece of code is
emitted when is something I don't know much about yet.


The call to build_vla_check() in cp_finish_decl() is guarded with
!init and made to check VLAs that are not explicitly initialized.
This call could perhaps be moved into check_initializer() though
it doesn't seem like it logically belongs there (since there is
no initializer).


check_initializer handles implicit (default-) as well as explicit
initialization.


If it were moved there, it seems to me that it
would require more extensive changes to the function than making
it in cp_finish_decl().


I don't see that; you ought to be able to move the check_initializer
copy down out of the if/else structure and use that for both cases.


You're right, it was simpler than I thought.  I was being overly
conservative in an effort to avoid changing more code than is
absolutely necessary.

Attached is an updated patch with this simplification. It avoids
case (1) above.  It also adds an argument to build_vla_check to
avoid building the size check when it's called from build_vec_init.

I also modified the alternate patch accordingly.  It's attached
for comparison. I still find it preferable to the first patch.
It's simpler because it doesn't require the special handling for
strings and avoids parameterizing build_vla_check so as not to
build the duplicate check in build_vec_init.

I don't see the benefit in doing the checking in build_vec_init
and split_constant_init_1 when it can all be done just in
check_initializer.  I'm sure you have your reasons for going that
route so I'd appreciate if you could let me know why you think
that's better.

Thanks
Martin
PR c++/69517 - [5/6 regression] SEGV on a VLA with excess initializer elements
PR c++/70019 - VLA size overflow not detected

gcc/testsuite/ChangeLog:
2016-04-10  Martin Sebor  

	PR c++/69517
	PR c++/70019
	* c-c++-common/ubsan/vla-1.c (main): Catch exceptions.
	* g++.dg/cpp1y/vla11.C: New test.
	* g++.dg/cpp1y/vla12.C: New test.
	* g++.dg/cpp1y/vla13.C: New test.
	* g++.dg/cpp1y/vla14.C: New test.
	* g++.dg/cpp1y/vla3.C: Restore deleted test.
	* gcc/testsuite/g++.dg/init/array24.C: Fully brace VLA initializer.
	* g++.dg/ubsan/vla-1.C: Disable exceptions.

gcc/cp/ChangeLog:
2016-04-10  Martin Sebor  

	PR c++/69517
	PR c++/70019
	* cp-tree.h (throw_bad_array_length, build_vla_check): Declare new
	functions.
	* decl.c (check_initializer): Call them.
	(reshape_init_r): Reject incompletely braced intializer-lists
	for VLAs.
	* init.c (throw_bad_array_length, build_vla_check)
	(build_vla_size_check, build_vla_init_check): Define new functions.
	(build_vec_init): Use variably_modified_type_p() to detect a VLA.
	Call throw_bad_array_length() and build_vla_check().
	* typeck2.c (split_nonconstant_init_1): Use variably_modified_type_p()
	to detect a VLA.
	(store_init_value): Same.
	(split_nonconstant_init): Handle excess elements in string literal
	initializers for VLAs.

gcc/doc/ChangeLog:

2016-04-10  Martin Sebor  

	PR c++/69517
	PR c++/70019
	* extend.texi (Variable Length): Document C++ specifics.

libstdc++-v3/ChangeLog:

2016-04-10  Martin Sebor  

	PR c++/69517
	* testsuite/25_algorithms/rotate/moveable2.cc: Make sure VLA
	upper bound is positive.

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index b7b770f..2936ac9 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5978,6 +5978,7 @@ extern tree build_value_init_noctor		(tree, tsubst_flags_t);
 extern tree get_nsdmi(tree, bool);
 extern tree build_offset_ref			(tree, tree, bool,
 		 tsubst_flags_t);
+extern tree th

[patch] removing aged ifdef 0 from boehm-gc/os_dep.c

2016-04-13 Thread gcc
This is my first effort at contributing to gcc, so I thought I would try
with some of the easy stuff listed here:
https://gcc.gnu.org/projects/beginner.html

Attached is a patch removing a block which has been #if 0'd out since
2006.  I tested the build afterward.

I look forward to your feedback.

Glen StarkFrom 479aade4f9120f200bf77f027d9f4fce34b56b5c Mon Sep 17 00:00:00 2001
From: Glen Stark 
Date: Sat, 9 Apr 2016 22:05:03 +0200
Subject: [PATCH] Removed stale # if 0 statement

---
 boehm-gc/os_dep.c | 33 -
 1 file changed, 33 deletions(-)

diff --git a/boehm-gc/os_dep.c b/boehm-gc/os_dep.c
index 2b22c9d..caff82e 100644
--- a/boehm-gc/os_dep.c
+++ b/boehm-gc/os_dep.c
@@ -1566,38 +1566,6 @@ void GC_register_data_segments()
 #   define SBRK_ARG_T int
 # endif
 
-
-# if 0 && defined(RS6000)  /* We now use mmap */
-/* The compiler seems to generate speculative reads one past the end of	*/
-/* an allocated object.  Hence we need to make sure that the page 	*/
-/* following the last heap page is also mapped.*/
-ptr_t GC_unix_get_mem(bytes)
-word bytes;
-{
-caddr_t cur_brk = (caddr_t)sbrk(0);
-caddr_t result;
-SBRK_ARG_T lsbs = (word)cur_brk & (GC_page_size-1);
-static caddr_t my_brk_val = 0;
-
-if ((SBRK_ARG_T)bytes < 0) return(0); /* too big */
-if (lsbs != 0) {
-if((caddr_t)(sbrk(GC_page_size - lsbs)) == (caddr_t)(-1)) return(0);
-}
-if (cur_brk == my_brk_val) {
-	/* Use the extra block we allocated last time. */
-result = (ptr_t)sbrk((SBRK_ARG_T)bytes);
-if (result == (caddr_t)(-1)) return(0);
-result -= GC_page_size;
-} else {
-result = (ptr_t)sbrk(GC_page_size + (SBRK_ARG_T)bytes);
-if (result == (caddr_t)(-1)) return(0);
-}
-my_brk_val = result + bytes + GC_page_size;	/* Always page aligned */
-return((ptr_t)result);
-}
-
-#else  /* Not RS6000 */
-
 #if defined(USE_MMAP) || defined(USE_MUNMAP)
 
 #ifdef USE_MMAP_FIXED
@@ -1694,7 +1662,6 @@ word bytes;
 }
 
 #endif /* Not USE_MMAP */
-#endif /* Not RS6000 */
 
 # endif /* UN*X */
 
-- 
2.5.5


libgo patch committed: Update to 1.6.1 release

2016-04-13 Thread Ian Lance Taylor
This patch updates libgo to the 1.6.1 release.  This is a minor
release that just fixes a couple of bugs with potential security
implications (see
https://github.com/golang/go/issues?utf8=%E2%9C%93&q=is%3Aopen+is%3Aissue+milestone%3AGo1.6.1
for more details).  Bootstrapped and ran Go testsuite on
x86_64-pc-linux-gnu.  Committed to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 234923)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-8e7b5e777333fa4cd070d96e94ea82e3e1132739
+ff29ea8e4e69eb94958aef4388da09a61b2b52b6
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/MERGE
===
--- libgo/MERGE (revision 234304)
+++ libgo/MERGE (working copy)
@@ -1,4 +1,4 @@
-7bc40ffb05d8813bf9b41a331b45d37216f9e747
+f5cf5673590a68c55b2330df9dfcdd6fac75b893
 
 The first line of this file holds the git revision number of the
 last merge done from the master library sources.
Index: libgo/VERSION
===
--- libgo/VERSION   (revision 234304)
+++ libgo/VERSION   (working copy)
@@ -1 +1 @@
-go1.6
\ No newline at end of file
+go1.6.1
\ No newline at end of file
Index: libgo/go/crypto/dsa/dsa.go
===
--- libgo/go/crypto/dsa/dsa.go  (revision 234304)
+++ libgo/go/crypto/dsa/dsa.go  (working copy)
@@ -249,6 +249,10 @@ func Sign(rand io.Reader, priv *PrivateK
 func Verify(pub *PublicKey, hash []byte, r, s *big.Int) bool {
// FIPS 186-3, section 4.7
 
+   if pub.P.Sign() == 0 {
+   return false
+   }
+
if r.Sign() < 1 || r.Cmp(pub.Q) >= 0 {
return false
}
Index: libgo/go/crypto/ecdsa/ecdsa.go
===
--- libgo/go/crypto/ecdsa/ecdsa.go  (revision 234304)
+++ libgo/go/crypto/ecdsa/ecdsa.go  (working copy)
@@ -23,6 +23,7 @@ import (
"crypto/elliptic"
"crypto/sha512"
"encoding/asn1"
+   "errors"
"io"
"math/big"
 )
@@ -140,6 +141,8 @@ func fermatInverse(k, N *big.Int) *big.I
return new(big.Int).Exp(k, nMinus2, N)
 }
 
+var errZeroParam = errors.New("zero parameter")
+
 // Sign signs an arbitrary length hash (which should be the result of hashing a
 // larger message) using the private key, priv. It returns the signature as a
 // pair of integers. The security of the private key depends on the entropy of
@@ -180,7 +183,9 @@ func Sign(rand io.Reader, priv *PrivateK
// See [NSA] 3.4.1
c := priv.PublicKey.Curve
N := c.Params().N
-
+   if N.Sign() == 0 {
+   return nil, nil, errZeroParam
+   }
var k, kInv *big.Int
for {
for {
@@ -193,7 +198,7 @@ func Sign(rand io.Reader, priv *PrivateK
if in, ok := priv.Curve.(invertible); ok {
kInv = in.Inverse(k)
} else {
-   kInv = fermatInverse(k, N)
+   kInv = fermatInverse(k, N) // N != 0
}
 
r, _ = priv.Curve.ScalarBaseMult(k.Bytes())
@@ -207,7 +212,7 @@ func Sign(rand io.Reader, priv *PrivateK
s = new(big.Int).Mul(priv.D, r)
s.Add(s, e)
s.Mul(s, kInv)
-   s.Mod(s, N)
+   s.Mod(s, N) // N != 0
if s.Sign() != 0 {
break
}
Index: libgo/go/crypto/rsa/rsa.go
===
--- libgo/go/crypto/rsa/rsa.go  (revision 234304)
+++ libgo/go/crypto/rsa/rsa.go  (working copy)
@@ -465,6 +465,9 @@ func decrypt(random io.Reader, priv *Pri
err = ErrDecryption
return
}
+   if priv.N.Sign() == 0 {
+   return nil, ErrDecryption
+   }
 
var ir *big.Int
if random != nil {
@@ -490,7 +493,7 @@ func decrypt(random io.Reader, priv *Pri
}
}
bigE := big.NewInt(int64(priv.E))
-   rpowe := new(big.Int).Exp(r, bigE, priv.N)
+   rpowe := new(big.Int).Exp(r, bigE, priv.N) // N != 0
cCopy := new(big.Int).Set(c)
cCopy.Mul(cCopy, rpowe)
cCopy.Mod(cCopy, priv.N)
Index: libgo/go/go/build/deps_test.go
===
--- libgo/go/go/build/deps_test.go  (revision 234304)
+++ libgo/go/go/build/deps_test.go  (working copy)
@@ -132,10 +132,10 @@ var pkgDeps = map[string][]string{
// End of linear dependency definitions.
 
// Operating system access.
-   "syscall": 

Re: RFC: C++ PATCH to adjust empty class parameter passing ABI

2016-04-13 Thread Jason Merrill

On 04/13/2016 11:54 AM, Jason Merrill wrote:

On 04/13/2016 11:32 AM, Jakub Jelinek wrote:

On Tue, Apr 12, 2016 at 04:27:48PM -0400, Jason Merrill wrote:

Unfortunately, a drawback of doing this in the front end is that it's
difficult to warn only about affected cases; the front end doesn't know
what's actually going to be emitted, and has to warn conservatively,
leading
to false positives particularly for inline functions.

The third patch is a sketch of an attempt to address this by delaying
the
warning until expand time.  We can't use front end information at
this point
because it has been thrown away by pass_ipa_free_lang_data, so I'm
resorting
to pre-generating the warning and stashing it away. This is several
kinds of
kludge, including hashing on a call location, but it greatly improves
the
accuracy of the warning.  I'm hoping that someone will have a better
idea
about how to approach this.


Is the intent that we emit different warnings between e.g.
-fkeep-inline-functions vs. without that, or -O0 vs. -O2?
Do you want to warn just on the out of line definitions (visible
outside of
the containing TU), or also calls to those?


That's what I'm thinking, since only actually emitted definitions/calls
have ABI.


For e.g. warning on calls to those functions it could be similar to the
warning attribute - where we warn if a call to function with that
attribute
is expanded - if it is inlined or optimized away etc., we don't warn.


Ah, that sounds promising, thanks.


Here's a patch along those lines, that passes testing.  Does it look OK?

commit 761983a023b5217ef831a43f423779940c788ecf
Author: Jason Merrill 
Date:   Tue Apr 12 13:16:50 2016 -0400

gcc/
	* cfgexpand.c (pass_expand::execute): Handle attribute abi_warning.
	* expr.c (expand_expr_real_1): Likewise.
gcc/cp/
	* call.c (empty_class_msg, mark_for_abi_warning): New.
	(build_call_a): Use them.
	* decl.c (store_parm_decls): Use mark_for_abi_warning.
	* error.c (pp_format_to_string): New.

diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 1341c14..4e5ccbb 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -73,6 +73,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "builtins.h"
 #include "tree-chkp.h"
 #include "rtl-chkp.h"
+#include "langhooks.h"
 
 /* Some systems use __main in a way incompatible with its use in gcc, in these
cases use the macros NAME__MAIN to give a quoted symbol and SYMBOL__MAIN to
@@ -6221,6 +6222,15 @@ pass_expand::execute (function *fun)
 		 (int) PARAM_VALUE (PARAM_SSP_BUFFER_SIZE));
 }
 
+  if (warn_abi)
+if (tree attr = lookup_attribute ("abi_warning",
+  DECL_ATTRIBUTES (current_function_decl)))
+  warning_at (DECL_SOURCE_LOCATION (current_function_decl),
+		  OPT_Wabi, "definition of %qs: %s",
+		  identifier_to_locale (lang_hooks.decl_printable_name
+	(current_function_decl, 1)),
+		  TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr;
+
   /* Set up parameters and prepare for return, for the function.  */
   expand_function_start (current_function_decl);
 
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 9cee3b7..97d790f 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -401,7 +401,10 @@ build_call_a (tree function, int n, tree *argarray)
 	else if (!warned && empty_arg)
 	  {
 	location_t loc = EXPR_LOC_OR_LOC (empty_arg, input_location);
-	warn_empty_class_abi (empty_arg, loc);
+	if (decl && !varargs_function_p (decl))
+	  mark_for_abi_warning (decl, empty_arg);
+	else
+	  warn_empty_class_abi (empty_arg, loc);
 	warned = true;
 	  }
   }
@@ -7019,8 +7022,19 @@ empty_class_arg (tree val)
   return build2 (COMPOUND_EXPR, etype, val, empty);
 }
 
-/* Warn about the change in empty class parameter passing ABI.  Returns true
-   if we warned.  */
+/* Generate a message warning about the change in empty class parameter passing ABI.  */
+
+static tree
+empty_class_msg (tree type)
+{
+  if (!TYPE_P (type))
+type = TREE_TYPE (type);
+
+  return pp_format_to_string ("empty class %qT parameter passing ABI "
+			  "changes in -fabi-version=10 (GCC 6)", type);
+}
+
+/* Warn immediately about the change in empty class parameter ABI.  */
 
 void
 warn_empty_class_abi (tree arg, location_t loc)
@@ -7028,19 +7042,24 @@ warn_empty_class_abi (tree arg, location_t loc)
   if (!warn_abi || !abi_version_crosses (10))
 return;
 
-  tree type;
-  if (TYPE_P (arg))
-type = arg;
-  else
-{
-  if (TREE_TYPE (arg) == empty_struct_type
-	  && TREE_CODE (arg) == COMPOUND_EXPR)
-	arg = TREE_OPERAND (arg, 0);
-  type = TREE_TYPE (arg);
-}
+  warning_at (loc, OPT_Wabi, "%E", empty_class_msg (arg));
+}
 
-  warning_at (loc, OPT_Wabi, "empty class %qT parameter passing ABI "
-	  "changes in -fabi-version=10 (GCC 6)", type);
+/* Tack a warning about the change in empty class parameter ABI onto FN, so
+   that we get a warning if a definition or call is emitted.  */
+
+void
+mark_for_abi_warning (tree 

Re: [PATCH] Fix ICE caused by nothrow pass (PR c++/70641)

2016-04-13 Thread Richard Biener
On April 13, 2016 7:34:36 PM GMT+02:00, Jakub Jelinek  wrote:
>Hi!
>
>If the nothrow pass (which is GIMPLE, not IPA pass) determines that the
>current function can't throw externally, it marks it nothrow.
>But it fails to adjust self-recursive calls, which might change because
>of
>that from possibly throwing to not being able to throw, so we need to
>cleanup the EH region, purge dead eh edges and adjust CFG if needed.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
>2016-04-13  Jakub Jelinek  
>
>   PR c++/70641
>   * ipa-pure-const.c (pass_nothrow::execute): Call maybe_clean_eh_stmt
>   on all recursive call stmts.  Return TODO_cleanup_cfg if any dead
>   eh edges have been purged.
>
>   * g++.dg/opt/pr70641.C: New test.
>
>--- gcc/ipa-pure-const.c.jj2016-04-13 15:53:01.051799840 +0200
>+++ gcc/ipa-pure-const.c   2016-04-13 16:03:10.715556547 +0200
>@@ -1956,10 +1956,28 @@ pass_nothrow::execute (function *)
> }
> 
>   node->set_nothrow_flag (true);
>+
>+  bool cfg_changed = false;
>+  if (self_recursive_p (node))
>+FOR_EACH_BB_FN (this_block, cfun)
>+  for (gimple_stmt_iterator gsi = gsi_start_bb (this_block);
>+   !gsi_end_p (gsi);
>+  gsi_next (&gsi))

I think it's enough to look at last_stmt.

OK with that change.

Richard.

>+  if (is_gimple_call (gsi_stmt (gsi)))
>+{
>+  gimple *g = gsi_stmt (gsi);
>+  tree callee_t = gimple_call_fndecl (g);
>+  if (callee_t
>+  && recursive_call_p (current_function_decl, callee_t)
>+  && maybe_clean_eh_stmt (g)
>+  && gimple_purge_dead_eh_edges (gimple_bb (g)))
>+cfg_changed = true;
>+}
>+
>   if (dump_file)
> fprintf (dump_file, "Function found to be nothrow: %s\n",
>current_function_name ());
>-  return 0;
>+  return cfg_changed ? TODO_cleanup_cfg : 0;
> }
> 
> } // anon namespace
>--- gcc/testsuite/g++.dg/opt/pr70641.C.jj  2016-04-13 16:01:16.381102468
>+0200
>+++ gcc/testsuite/g++.dg/opt/pr70641.C 2016-04-13 16:01:16.381102468
>+0200
>@@ -0,0 +1,10 @@
>+// PR c++/70641
>+// { dg-do compile }
>+// { dg-options "-O2" }
>+
>+void
>+foo ()
>+{
>+  try { foo (); }
>+  catch (...) { __builtin_abort (); }
>+}
>
>   Jakub




Re: RFC: C++ PATCH to adjust empty class parameter passing ABI

2016-04-13 Thread Jakub Jelinek
On Wed, Apr 13, 2016 at 03:11:34PM -0400, Jason Merrill wrote:
> commit 761983a023b5217ef831a43f423779940c788ecf
> Author: Jason Merrill 
> Date:   Tue Apr 12 13:16:50 2016 -0400
> 
> gcc/
>   * cfgexpand.c (pass_expand::execute): Handle attribute abi_warning.
>   * expr.c (expand_expr_real_1): Likewise.
> gcc/cp/
>   * call.c (empty_class_msg, mark_for_abi_warning): New.
>   (build_call_a): Use them.
>   * decl.c (store_parm_decls): Use mark_for_abi_warning.
>   * error.c (pp_format_to_string): New.

I think you should put a space into the attribute name instead of _
to make it clear that it is not an attribute users can use directly in their
code through __attribute__.

Otherwise it looks reasonable to me.

Jakub


Re: [PATCH] Don't include DECL_UIDs in tree-sra fancy names for -fdump-final-insns= (PR c++/70594)

2016-04-13 Thread Richard Biener
On April 13, 2016 4:32:25 PM GMT+02:00, Jakub Jelinek  wrote:
>On Wed, Apr 13, 2016 at 09:25:56AM +0200, Richard Biener wrote:
>> So if you think it's not acceptable to drop the relation between
>> the artificial number used by SRA and the original UID then go
>> with a hash-map unconditionally.  You still have to dump and lookup
>> the actual relation though - there's no way around this unless you
>> hack dump-final-insns to filter DECL names (maybe just make it
>> strip all DECL_ARTIFICIAL names completely?)
>
>Here is an alternative patch, keep the fancy names as before,
>but in TDF_NOUID dumps sanitize them.  Most of the DECL_NAMELESS
>decls other than ones with the SRA fancy names should not contain
>anything that would be tweaked for the printout, and doing it this
>way has the advantage that one can always just use flags without
>TDF_NOUID
>to see the real fancy names.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Richard.

>2016-04-13  Jakub Jelinek  
>
>   PR c++/70594
>   * tree-sra.c (create_access_replacement,
>   get_replaced_param_substitute): Set DECL_NAMELESS on repl if it
>   gets fancy name.
>   * tree-pretty-print.c (dump_fancy_name): New function.
>   (dump_decl_name, dump_generic_node): Use it.
>
>--- gcc/tree-sra.c.jj  2016-04-12 19:12:34.0 +0200
>+++ gcc/tree-sra.c 2016-04-13 13:52:28.329925560 +0200
>@@ -2132,6 +2132,7 @@ create_access_replacement (struct access
>   bool fail = false;
> 
>   DECL_NAME (repl) = get_identifier (pretty_name);
>+  DECL_NAMELESS (repl) = 1;
>   obstack_free (&name_obstack, pretty_name);
> 
>   /* Get rid of any SSA_NAMEs embedded in debug_expr,
>@@ -4704,6 +4705,7 @@ get_replaced_param_substitute (struct ip
> 
>   repl = create_tmp_reg (TREE_TYPE (adj->base), "ISR");
>   DECL_NAME (repl) = get_identifier (pretty_name);
>+  DECL_NAMELESS (repl) = 1;
>   obstack_free (&name_obstack, pretty_name);
> 
>   adj->new_ssa_base = repl;
>--- gcc/tree-pretty-print.c.jj 2016-03-17 16:54:44.0 +0100
>+++ gcc/tree-pretty-print.c2016-04-13 14:10:59.667085487 +0200
>@@ -161,6 +161,85 @@ print_generic_expr (FILE *file, tree t,
>   pp_flush (tree_pp);
> }
> 
>+/* Dump NAME, an IDENTIFIER_POINTER, sanitized so that D
>sequences
>+   in it are replaced with D, as long as they are at the start or
>+   preceded by $ and at the end or followed by $.  See make_fancy_name
>+   in tree-sra.c.  */
>+
>+static void
>+dump_fancy_name (pretty_printer *pp, tree name)
>+{
>+  int cnt = 0;
>+  int length = IDENTIFIER_LENGTH (name);
>+  const char *n = IDENTIFIER_POINTER (name);
>+  do
>+{
>+  n = strchr (n, 'D');
>+  if (n == NULL)
>+  break;
>+  if (ISDIGIT (n[1])
>+&& (n == IDENTIFIER_POINTER (name) || n[-1] == '$'))
>+  {
>+int l = 2;
>+while (ISDIGIT (n[l]))
>+  l++;
>+if (n[l] == '\0' || n[l] == '$')
>+  {
>+cnt++;
>+length += 5 - l;
>+  }
>+n += l;
>+  }
>+  else
>+  n++;
>+}
>+  while (1);
>+  if (cnt == 0)
>+{
>+  pp_tree_identifier (pp, name);
>+  return;
>+}
>+
>+  char *str = XNEWVEC (char, length + 1);
>+  char *p = str;
>+  const char *q;
>+  q = n = IDENTIFIER_POINTER (name);
>+  do
>+{
>+  q = strchr (q, 'D');
>+  if (q == NULL)
>+  break;
>+  if (ISDIGIT (q[1])
>+&& (q == IDENTIFIER_POINTER (name) || q[-1] == '$'))
>+  {
>+int l = 2;
>+while (ISDIGIT (q[l]))
>+  l++;
>+if (q[l] == '\0' || q[l] == '$')
>+  {
>+memcpy (p, n, q - n);
>+memcpy (p + (q - n), "D", 5);
>+p += (q - n) + 5;
>+n = q + l;
>+  }
>+q += l;
>+  }
>+  else
>+  q++;
>+}
>+  while (1);
>+  memcpy (p, n, IDENTIFIER_LENGTH (name) - (n - IDENTIFIER_POINTER
>(name)));
>+  str[length] = '\0';
>+  if (pp_translate_identifiers (pp))
>+{
>+  const char *text = identifier_to_locale (str);
>+  pp_append_text (pp, text, text + strlen (text));
>+}
>+  else
>+pp_append_text (pp, str, str + length);
>+  XDELETEVEC (str);
>+}
>+
> /* Dump the name of a _DECL node and its DECL_UID if TDF_UID is set
>in FLAGS.  */
> 
>@@ -171,6 +250,10 @@ dump_decl_name (pretty_printer *pp, tree
> {
>   if ((flags & TDF_ASMNAME) && DECL_ASSEMBLER_NAME_SET_P (node))
>   pp_tree_identifier (pp, DECL_ASSEMBLER_NAME (node));
>+  /* For DECL_NAMELESS names look for embedded uids in the
>+   names and sanitize them for TDF_NOUID.  */
>+  else if ((flags & TDF_NOUID) && DECL_NAMELESS (node))
>+  dump_fancy_name (pp, DECL_NAME (node));
>   else
>   pp_tree_identifier (pp, DECL_NAME (node));
> }
>@@ -2593,8 +2676,15 @@ dump_generic_node (pretty_printer *pp, t
> 
> case SSA_NAME:
>   if (SSA_NAME_IDENTIFIER (node))
>-  dump_generic_node (pp, SSA_NAME_IDENTIF

Re: [PATCH] 69517 - [5/6 regression] SEGV on a VLA with excess initializer elements

2016-04-13 Thread Jason Merrill

On 04/13/2016 02:36 PM, Martin Sebor wrote:

I don't see the benefit in doing the checking in build_vec_init
and split_constant_init_1 when it can all be done just in
check_initializer.  I'm sure you have your reasons for going that
route so I'd appreciate if you could let me know why you think
that's better.


In order to share the code for checking VLAs and array new, but I take 
your point that they aren't similar enough for that.  Your alternate 
patch is OK.


Jason



Re: [PATCH] c++/70594 debug info differences

2016-04-13 Thread Jason Merrill

On 04/13/2016 11:54 AM, Nathan Sidwell wrote:

This patch builds from Jason's proto-patch in comment #26. As Jason
discovered, that led to link problems with gt_ggc_mx (tree_node *&) when
building cc1. Making 'hash_map' GC-able caused
gengtype to put the GC-walkers for tree into constexpr.c. No idea why.
That's also what led me to add the user GTY stuff in the patch I posted
earlier. For some reason manual GC fns kept the tree GC-walker in a sane
place. I wonder if the same problem is why Patrick's original patch
wrapped the hash_map pointer in a local structure?

Rather than peer into gengtype's mind, I figured on changing to use a
regular tree->tree mapper, which didn't disturb gengtype. I employ a
TREE_LIST to hold the bits that fundef_copy held (body, parms, result)
on (PURPOSE, VALUE, TYPE). A little unclean, but not the first time
non-types are on such a TYPE, IIRC. While there I noticed that the
getter only needed to use a hash getter, rather than get_or_insert.

The thrust of the patch makes the fundef copies and constexpr call
tables GCable, not GC-deletable. Thus their contents are not affected by
GC occurrences. Finally, a new hook called at end of parsing to delete
the constexpr call & fundef copies tables, so they don't remain after
parsing.  We don't do anything about stopping them getting too big.

Patch survives boot & test, and fixes the testcase in 70594/#4 (without
Jakub's patch to not emit UIDs in the gimple dump).

ok?


The fini_constexpr stuff is OK immediately.

The rest of the patch is OK if Jakub thinks it should go in, but if his 
approach addresses the problem, we might as well continue to GC the tables.


Jason



Re: RFC: C++ PATCH to adjust empty class parameter passing ABI

2016-04-13 Thread Jason Merrill

On 04/13/2016 03:18 PM, Jakub Jelinek wrote:

On Wed, Apr 13, 2016 at 03:11:34PM -0400, Jason Merrill wrote:

commit 761983a023b5217ef831a43f423779940c788ecf
Author: Jason Merrill 
Date:   Tue Apr 12 13:16:50 2016 -0400

 gcc/
* cfgexpand.c (pass_expand::execute): Handle attribute abi_warning.
* expr.c (expand_expr_real_1): Likewise.
 gcc/cp/
* call.c (empty_class_msg, mark_for_abi_warning): New.
(build_call_a): Use them.
* decl.c (store_parm_decls): Use mark_for_abi_warning.
* error.c (pp_format_to_string): New.


I think you should put a space into the attribute name instead of _
to make it clear that it is not an attribute users can use directly in their
code through __attribute__.

Otherwise it looks reasonable to me.


Thanks, applied with that change.

Jason



libgcc: Fix typos in comments for ARM FP emulation routines

2016-04-13 Thread Martin Galvan
A lifetime ago I contributed a patch that added CFI directives to ieee754-df.S,
among other files. For unrelated reasons I looked at that file again and saw
that some of the comments have extra '@' characters interwined; this is probably
the result of splitting lines because they were too long. This patch simply
removes those extra chars, as well as fixing a couple other cosmetic issues.

libgcc/ChangeLog:
2016-04-13  Martin Galvan  

* config/arm/ieee754-df.S: Fix typos in comments.

Index: libgcc/config/arm/ieee754-df.S
===
--- libgcc/config/arm/ieee754-df.S  (revision 234960)
+++ libgcc/config/arm/ieee754-df.S  (working copy)
@@ -160,8 +160,8 @@
teq r4, r5
beq LSYM(Lad_d)
 
-@ CFI note: we're lucky that the branches to Lad_* that appear after this 
function
-@ have a CFI state that's exactly the same as the one we're in at this
+@ CFI note: we're lucky that the branches to Lad_* that appear after this
+@ function have a CFI state that's exactly the same as the one we're in at this
 @ point. Otherwise the CFI would change to a different state after the branch,
 @ which would be disastrous for backtracing.
 LSYM(Lad_x):
@@ -1158,8 +1158,8 @@
 1: str ip, [sp, #-4]!
.cfi_adjust_cfa_offset 4@ CFA is now sp + previousOffset + 4.
@ We're not adding CFI for ip as it's pushed into the stack
-   @ only because @ it may be popped off later as a return value
-   @ (i.e. we're not preserving @ it anyways).
+   @ only because it may be popped off later as a return value
+   @ (i.e. we're not preserving it anyways).
 
@ Trap any INF/NAN first.
mov ip, xh, lsl #1
@@ -1169,14 +1169,14 @@
COND(mvn,s,ne)  ip, ip, asr #21
beq 3f
.cfi_remember_state
-   @ Save the current CFI state. This is done because the branch
-   @ is conditional, @ and if we don't take it we'll issue a
-   @ .cfi_adjust_cfa_offset and return.  @ If we do take it,
-   @ however, the .cfi_adjust_cfa_offset from the non-branch @ code
-   @ will affect the branch code as well. To avoid this we'll
-   @ restore @ the current state before executing the branch code.
+   @ Save the current CFI state.  This is done because the branch
+   @ is conditional, and if we don't take it we'll issue a
+   @ .cfi_adjust_cfa_offset and return.  If we do take it,
+   @ however, the .cfi_adjust_cfa_offset from the non-branch code
+   @ will affect the branch code as well.  To avoid this we'll
+   @ restore the current state before executing the branch code.
 
-   @ Test for equality.  @ Note that 0.0 is equal to -0.0.
+   @ Test for equality.  Note that 0.0 is equal to -0.0.
 2: add sp, sp, #4
.cfi_adjust_cfa_offset -4   @ CFA is now sp + previousOffset.
 


Re: RFC: C++ PATCH to adjust empty class parameter passing ABI

2016-04-13 Thread Jonathan Wakely

On 12/04/16 16:27 -0400, Jason Merrill wrote:

A revision of the patch previously posted at

 https://gcc.gnu.org/ml/gcc-patches/2016-03/msg00841.html

To recap quickly, the C++ compiler has used a different calling 
convention for passing empty classes, because C++ says they have size 
1, while the GCC C extension gives them size 0.  But this difference 
doesn't mean that they need to be passed differently; in either case 
there's no actual data involved.  And the Clang folks recently pointed 
out that if you squint properly, it seems that the x86_64 psABI says 
that an empty class shouldn't be passed either in registers or memory. 
H.J. had a back-end fix, but it seemed to me that a front-end fix 
would be simpler and safer.


The first patch corrects errors in what the C++ front end considers an 
empty classes; unnamed bit-fields aren't really members.  I'm checking 
this in now.


The second patch is the actual ABI change under -fabi-version=10, 
including a warning.  Jonathan, please check this in with your library 
changes.


I've committed the library changes as the attached patch.

commit dd715d3cb76a536aa4496e9efdf3f932aeb605ff
Author: Jonathan Wakely 
Date:   Wed Apr 13 20:47:23 2016 +0100

Adjust for new empty class parameter passing ABI.

	* include/bits/c++config (_GLIBCXX_BEGIN_NAMESPACE_EMPTY_TYPES,
	_GLIBCXX_END_NAMESPACE_EMPTY_TYPES, _GLIBCXX_ABI_TAG_EMPTY): Define.
	* include/bits/hashtable.h (_Hashtable::_M_emplace): Change signatures
	of functions taking empty structs by value. Add a template parameter
	to overloads without hints. Rename overloads with hints to
	_M_emplace_hint.
	(_Hashtable::_M_erase(true_type, const_iterator),
	_Hashtable::_M_erase(false_type, const_iterator)): Change signatures
	by reordering parameters.
	* include/bits/hashtable_policy.h (_Insert::insert): Adjust to call
	_M_emplace_hint instead of _M_emplace.
	* include/bits/shared_ptr.h (shared_ptr(_Tp1*, _Deleter, _Alloc),
	shared_ptr(nullptr_t, _Deleter, _Alloc)): Use _GLIBCXX_ABI_TAG_EMPTY.
	* include/bits/shared_ptr_base.h (_Sp_counted_deleter, __shared_count,
	__shared_ptr): Likewise.
	* include/bits/stl_algo.h (replace_if): Likewise.
	* include/bits/stl_pair.h (piecewise_construct_t,
	piecewise_construct): Use _GLIBCXX_BEGIN_NAMESPACE_EMPTY_TYPES.
	* include/bits/uses_allocator.h (allocator_arg_t, allocator_arg,
	__uses_alloc0): Likewise.
	* include/ext/pb_ds/assoc_container.hpp (basic_hash_table): Likewise.
	* testsuite/20_util/scoped_allocator/69293_neg.cc: Adjust dg-error.
	* testsuite/20_util/shared_ptr/cons/43820_neg.cc: Likewise.
	* testsuite/20_util/shared_ptr/cons/void_neg.cc: Likewise.
	* testsuite/20_util/uses_allocator/69293_neg.cc: Likewise.
	* testsuite/20_util/uses_allocator/cons_neg.cc: Likewise.
	* testsuite/ext/profile/mutex_extensions_neg.cc: Likewise.

diff --git a/libstdc++-v3/include/bits/c++config b/libstdc++-v3/include/bits/c++config
index 57024e4..bde003c 100644
--- a/libstdc++-v3/include/bits/c++config
+++ b/libstdc++-v3/include/bits/c++config
@@ -84,13 +84,32 @@
 # define _GLIBCXX_DEPRECATED
 #endif
 
+#if __cplusplus
+
 // Macros for ABI tag attributes.
 #ifndef _GLIBCXX_ABI_TAG_CXX11
 # define _GLIBCXX_ABI_TAG_CXX11 __attribute ((__abi_tag__ ("cxx11")))
 #endif
 
-
-#if __cplusplus
+#if __GXX_ABI_VERSION >= 1010
+namespace std
+{
+  inline namespace _V2 { }
+}
+# define _GLIBCXX_BEGIN_NAMESPACE_EMPTY_TYPES \
+  _GLIBCXX_END_NAMESPACE_VERSION \
+  namespace _V2 { \
+  _GLIBCXX_BEGIN_NAMESPACE_VERSION
+# define _GLIBCXX_END_NAMESPACE_EMPTY_TYPES \
+  _GLIBCXX_END_NAMESPACE_VERSION \
+  } \
+  _GLIBCXX_BEGIN_NAMESPACE_VERSION
+# define _GLIBCXX_ABI_TAG_EMPTY __attribute ((__abi_tag__ ("cxxempty")))
+#else
+# define _GLIBCXX_BEGIN_NAMESPACE_EMPTY_TYPES
+# define _GLIBCXX_END_NAMESPACE_EMPTY_TYPES
+# define _GLIBCXX_ABI_TAG_EMPTY
+#endif
 
 // Macro for constexpr, to support in mixed 03/0x mode.
 #ifndef _GLIBCXX_CONSTEXPR
diff --git a/libstdc++-v3/include/bits/hashtable.h b/libstdc++-v3/include/bits/hashtable.h
index 5748920..22b7187 100644
--- a/libstdc++-v3/include/bits/hashtable.h
+++ b/libstdc++-v3/include/bits/hashtable.h
@@ -663,24 +663,26 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   _M_insert_multi_node(__node_type* __hint,
 			   __hash_code __code, __node_type* __n);
 
-  template
-	std::pair
-	_M_emplace(std::true_type, _Args&&... __args);
+  template
+	typename enable_if<_Uniq, std::pair>::type
+	_M_emplace(__bool_constant<_Uniq>, _Args&&... __args);
 
-  template
-	iterator
-	_M_emplace(std::false_type __uk, _Args&&... __args)
-	{ return _M_emplace(cend(), __uk, std::forward<_Args>(__args)...); }
+  template
+	typename enable_if::type
+	_M_emplace(__bool_constant<_Uniq> __uk, _Args&&... __args)
+	{
+	  return _M_emplace_hint(cend(), __uk, std::forward<_Args>(__args)...);
+	}
 
   // Emplace with hint, useless when keys are unique.
   te

Re: RFC: C++ PATCH to adjust empty class parameter passing ABI

2016-04-13 Thread H.J. Lu
On Wed, Apr 13, 2016 at 1:12 PM, Jason Merrill  wrote:
> On 04/13/2016 03:18 PM, Jakub Jelinek wrote:
>>
>> On Wed, Apr 13, 2016 at 03:11:34PM -0400, Jason Merrill wrote:
>>>
>>> commit 761983a023b5217ef831a43f423779940c788ecf
>>> Author: Jason Merrill 
>>> Date:   Tue Apr 12 13:16:50 2016 -0400
>>>
>>>  gcc/
>>> * cfgexpand.c (pass_expand::execute): Handle attribute
>>> abi_warning.
>>> * expr.c (expand_expr_real_1): Likewise.
>>>  gcc/cp/
>>> * call.c (empty_class_msg, mark_for_abi_warning): New.
>>> (build_call_a): Use them.
>>> * decl.c (store_parm_decls): Use mark_for_abi_warning.
>>> * error.c (pp_format_to_string): New.
>>
>>
>> I think you should put a space into the attribute name instead of _
>> to make it clear that it is not an attribute users can use directly in
>> their
>> code through __attribute__.
>>
>> Otherwise it looks reasonable to me.
>
>
> Thanks, applied with that change.
>
> Jason
>

Here are tests from PR 60336 and PR 68355.  They pass
on Linux/x86-64 with -m32/-mx32/-m64.  OK for trunk?

Thanks.

-- 
H.J.
From d6420be9ce4f5e9e5c6a97ca66fa07309bfc11f7 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" 
Date: Sun, 15 Nov 2015 13:19:05 -0800
Subject: [PATCH] Add more tests for empty C++ class passing

	PR c++/60336
	PR middle-end/67239
	PR target/68355
	* g++.dg/pr60336-1.C: Likewise.
	* g++.dg/pr60336-2.C: Likewise.
	* g++.dg/pr60336-3.C: Likewise.
	* g++.dg/pr60336-4.C: Likewise.
	* g++.dg/pr60336-5.C: Likewise.
	* g++.dg/pr60336-6.C: Likewise.
	* g++.dg/pr60336-7.C: Likewise.
	* g++.dg/pr60336-8.C: Likewise.
	* g++.dg/pr60336-9.C: Likewise.
	* g++.dg/pr60336-10.C: Likewise.
	* g++.dg/pr60336-11.C: Likewise.
	* g++.dg/pr60336-12.C: Likewise.
	* g++.dg/pr68355.C: Likewise.
---
 gcc/testsuite/g++.dg/pr60336-1.C  | 17 
 gcc/testsuite/g++.dg/pr60336-10.C | 50 ++
 gcc/testsuite/g++.dg/pr60336-11.C | 56 ++
 gcc/testsuite/g++.dg/pr60336-12.C | 57 +++
 gcc/testsuite/g++.dg/pr60336-2.C  | 48 +
 gcc/testsuite/g++.dg/pr60336-3.C  | 15 +++
 gcc/testsuite/g++.dg/pr60336-4.C  | 48 +
 gcc/testsuite/g++.dg/pr60336-5.C  | 17 
 gcc/testsuite/g++.dg/pr60336-6.C  | 17 
 gcc/testsuite/g++.dg/pr60336-7.C  | 17 
 gcc/testsuite/g++.dg/pr60336-8.C  | 15 +++
 gcc/testsuite/g++.dg/pr60336-9.C  | 28 +++
 gcc/testsuite/g++.dg/pr68355.C| 24 +
 13 files changed, 409 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/pr60336-1.C
 create mode 100644 gcc/testsuite/g++.dg/pr60336-10.C
 create mode 100644 gcc/testsuite/g++.dg/pr60336-11.C
 create mode 100644 gcc/testsuite/g++.dg/pr60336-12.C
 create mode 100644 gcc/testsuite/g++.dg/pr60336-2.C
 create mode 100644 gcc/testsuite/g++.dg/pr60336-3.C
 create mode 100644 gcc/testsuite/g++.dg/pr60336-4.C
 create mode 100644 gcc/testsuite/g++.dg/pr60336-5.C
 create mode 100644 gcc/testsuite/g++.dg/pr60336-6.C
 create mode 100644 gcc/testsuite/g++.dg/pr60336-7.C
 create mode 100644 gcc/testsuite/g++.dg/pr60336-8.C
 create mode 100644 gcc/testsuite/g++.dg/pr60336-9.C
 create mode 100644 gcc/testsuite/g++.dg/pr68355.C

diff --git a/gcc/testsuite/g++.dg/pr60336-1.C b/gcc/testsuite/g++.dg/pr60336-1.C
new file mode 100644
index 000..946f8accd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr60336-1.C
@@ -0,0 +1,17 @@
+// { dg-do compile { target i?86-*-* x86_64-*-* } }
+// { dg-options "-O2 -std=c++11 -fno-pic" }
+// { dg-require-effective-target fpic }
+
+struct dummy { };
+struct true_type { struct dummy i; };
+
+extern true_type y;
+extern void xxx (true_type c);
+
+void
+yyy (void)
+{
+  xxx (y);
+}
+
+// { dg-final { scan-assembler "jmp\[\t \]+\[^\$\]*?_Z3xxx9true_type" { target i?86-*-* x86_64-*-* } } }
diff --git a/gcc/testsuite/g++.dg/pr60336-10.C b/gcc/testsuite/g++.dg/pr60336-10.C
new file mode 100644
index 000..58de85b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr60336-10.C
@@ -0,0 +1,50 @@
+// { dg-do run { target i?86-*-* x86_64-*-* } }
+// { dg-options "-O2" }
+
+#include 
+
+struct dummy0 { };
+struct dummy1 { };
+struct dummy : dummy0, dummy1 { };
+
+void
+test (struct dummy a, int m, ...)
+{
+  va_list va_arglist;
+  int i;
+  int count = 0;
+
+  if (m == 0)
+count++;
+  va_start (va_arglist, m);
+  i = va_arg (va_arglist, int);
+  if (i == 1)
+count++;
+  i = va_arg (va_arglist, int);
+  if (i == 2)
+count++;
+  i = va_arg (va_arglist, int);
+  if (i == 3)
+count++;
+  i = va_arg (va_arglist, int);
+  if (i == 4)
+count++;
+  i = va_arg (va_arglist, int);
+  if (i == 5)
+count++;
+  i = va_arg (va_arglist, int);
+  if (i == 6)
+count++;
+  va_end (va_arglist);
+  if (count != 7)
+__builtin_abort ();
+}
+
+struct dummy a0;
+
+int
+main ()
+{
+  test (a0, 0, 1, 2, 3, 4, 5, 6);
+  return 0;
+}
diff --git a/gcc/testsuite/g

[patch] fix an openacc reference-type reduction bug in PR70643

2016-04-13 Thread Cesar Philippidis
Thomas noticed that one of the openacc fortran tests involving
reductions inside fortran modules in gomp-4_0-branch had regressed after
I backported the recent reduction changes I made to trunk. That failure
occurred because lower_oacc_reductions wasn't considering that reduction
initializers can be non-integral constants. This patch fixes that
problem and adds a new runtime test case.

Is this patch ok for trunk? I tested it on x86_64-linux.

Cesar
2016-04-13  Cesar Philippidis  

	gcc/
	* omp-low.c (lower_oacc_reductions): Check for TREE_CONSTANT
	when building a mem ref for the incoming reduction variable.

	libgomp/
	* testsuite/libgomp.oacc-fortran/pr70643.f90: New test.


diff --git a/gcc/omp-low.c b/gcc/omp-low.c
index 7335abc..aa3721e 100644
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -5691,7 +5691,7 @@ lower_oacc_reductions (location_t loc, tree clauses, tree level, bool inner,
 	v3 = build_simple_mem_ref (v3);
 	outgoing = build_simple_mem_ref (outgoing);
 
-	if (TREE_CODE (incoming) != INTEGER_CST)
+	if (!TREE_CONSTANT (incoming))
 	  incoming = build_simple_mem_ref (incoming);
 	  }
 	else
diff --git a/libgomp/testsuite/libgomp.oacc-fortran/pr70643.f90 b/libgomp/testsuite/libgomp.oacc-fortran/pr70643.f90
new file mode 100644
index 000..7c2e5ee
--- /dev/null
+++ b/libgomp/testsuite/libgomp.oacc-fortran/pr70643.f90
@@ -0,0 +1,51 @@
+MODULE reduction_test
+
+CONTAINS
+
+SUBROUTINE reduction_kernel(x_min,x_max,y_min,y_max,arr,sum)
+
+  IMPLICIT NONE
+
+  INTEGER  :: x_min,x_max,y_min,y_max
+  REAL(KIND=8), DIMENSION(x_min-2:x_max+2,y_min-2:y_max+2) :: arr
+  REAL(KIND=8) :: sum
+
+  INTEGER  :: j,k
+
+!$ACC DATA PRESENT(arr) COPY(sum)
+!$ACC PARALLEL LOOP REDUCTION(+ : sum)
+  DO k=y_min,y_max
+DO j=x_min,x_max
+  sum=sum+arr(j,k)
+ENDDO
+  ENDDO
+!$ACC END PARALLEL LOOP
+!$ACC END DATA
+END SUBROUTINE reduction_kernel
+
+END MODULE reduction_test
+
+program main
+use reduction_test
+
+integer :: x_min,x_max,y_min,y_max
+real(kind=8), dimension(1:10,1:10) :: arr
+real(kind=8) :: sum
+
+x_min = 1
+x_max = 2
+y_min = 1
+y_max = 2
+
+arr(:,:) = 1.0
+
+sum = 0.0
+
+!$acc data copy(arr)
+
+call reduction_kernel(x_min,x_max,y_min,y_max,arr,sum)
+
+!$acc end data
+
+if (sum .ne. 4.0) call abort
+end program


Re: RFC: C++ PATCH to adjust empty class parameter passing ABI

2016-04-13 Thread H.J. Lu
On Wed, Apr 13, 2016 at 1:12 PM, Jason Merrill  wrote:
> On 04/13/2016 03:18 PM, Jakub Jelinek wrote:
>>
>> On Wed, Apr 13, 2016 at 03:11:34PM -0400, Jason Merrill wrote:
>>>
>>> commit 761983a023b5217ef831a43f423779940c788ecf
>>> Author: Jason Merrill 
>>> Date:   Tue Apr 12 13:16:50 2016 -0400
>>>
>>>  gcc/
>>> * cfgexpand.c (pass_expand::execute): Handle attribute
>>> abi_warning.
>>> * expr.c (expand_expr_real_1): Likewise.
>>>  gcc/cp/
>>> * call.c (empty_class_msg, mark_for_abi_warning): New.
>>> (build_call_a): Use them.
>>> * decl.c (store_parm_decls): Use mark_for_abi_warning.
>>> * error.c (pp_format_to_string): New.
>>
>>
>> I think you should put a space into the attribute name instead of _
>> to make it clear that it is not an attribute users can use directly in
>> their
>> code through __attribute__.
>>
>> Otherwise it looks reasonable to me.
>
>
> Thanks, applied with that change.

I opened:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70649

-- 
H.J.


Re: [PATCH] 69517 - [5/6 regression] SEGV on a VLA with excess initializer elements

2016-04-13 Thread H.J. Lu
On Wed, Apr 13, 2016 at 11:36 AM, Martin Sebor  wrote:
> On 04/12/2016 12:17 PM, Jason Merrill wrote:
>>
>> On 04/10/2016 07:14 PM, Martin Sebor wrote:
>>>
>>> The call to build_vla_check() in check_initializer() is to check
>>> only explicitly initialized VLAs.  The latter function may need
>>> to complete the VLA element type and reshape the initializer so
>>> the call cannot be made earlier.  The function returns the runtime
>>> initializer code so the call cannot be made after it returns.
>>
>>
>> But this call isn't checking the initializer; we're looking at the
>> initializer in build_vec_init now.
>
>
> Let me try to explain.  Strings aside, there are three cases each
> of which the patch handles in a different function:
>
> 1) The No Initializer case is handled by calling build_vla_check
> directly from cp_finish_decl.
>
> 2) The Empty initializer case is handled from check_initializer
> in the hunk you had pasted upthread.  (It isn't handled in
> build_vec_init because length_check is false for empty
> initializers, and because it's too late to check VLA bounds
> there anyway -- see below.)
>
> 3) In the Non-empty Initializer case, excess initializer elements
> are checked for in build_vec_init.  VLA bounds are checked from
> check_initializer same as in (2).
>
> As I mentioned last Friday, emitting the check for the VLA bounds
> in build_vec_init appears to be too late because by then the code
> to allocate the stack has already been emitted.  Maybe there's
> a way to do it, I don't know.  Controlling what piece of code is
> emitted when is something I don't know much about yet.
>
>>> The call to build_vla_check() in cp_finish_decl() is guarded with
>>> !init and made to check VLAs that are not explicitly initialized.
>>> This call could perhaps be moved into check_initializer() though
>>> it doesn't seem like it logically belongs there (since there is
>>> no initializer).
>>
>>
>> check_initializer handles implicit (default-) as well as explicit
>> initialization.
>>
>>> If it were moved there, it seems to me that it
>>> would require more extensive changes to the function than making
>>> it in cp_finish_decl().
>>
>>
>> I don't see that; you ought to be able to move the check_initializer
>> copy down out of the if/else structure and use that for both cases.
>
>
> You're right, it was simpler than I thought.  I was being overly
> conservative in an effort to avoid changing more code than is
> absolutely necessary.
>
> Attached is an updated patch with this simplification. It avoids
> case (1) above.  It also adds an argument to build_vla_check to
> avoid building the size check when it's called from build_vec_init.
>
> I also modified the alternate patch accordingly.  It's attached
> for comparison. I still find it preferable to the first patch.
> It's simpler because it doesn't require the special handling for
> strings and avoids parameterizing build_vla_check so as not to
> build the duplicate check in build_vec_init.
>
> I don't see the benefit in doing the checking in build_vec_init
> and split_constant_init_1 when it can all be done just in
> check_initializer.  I'm sure you have your reasons for going that
> route so I'd appreciate if you could let me know why you think
> that's better.

It caused:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70652


-- 
H.J.


Re: PATCH] Fix PR 31531: A microoptimization of isnegative of signed integer

2016-04-13 Thread Hurugalawadi, Naveen
Hi Richard,

>> What's the motivation of splitting this into a equal type 
>> and a non-equal type case?  Simply only allow nop-conversions here
>> (tree_nop_conversion_p) and unconditionally emit

Done.

Thanks for the review and Comments. Implemented the modifications
as per you review comments.
Please review the modified patch and let me know if its okay?

Thanks,
Naveendiff --git a/gcc/match.pd b/gcc/match.pd
index 75aa601..928a529 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -1896,10 +1896,13 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 (for cmp (simple_comparison)
  scmp (swapped_simple_comparison)
  (simplify
-  (cmp (bit_not@2 @0) CONSTANT_CLASS_P@1)
+  (cmp (convert?@3 (bit_not@2 @0)) CONSTANT_CLASS_P@1)
   (if (single_use (@2)
-   && (TREE_CODE (@1) == INTEGER_CST || TREE_CODE (@1) == VECTOR_CST))
-   (scmp @0 (bit_not @1)
+   && (TREE_CODE (@1) == INTEGER_CST || TREE_CODE (@1) == VECTOR_CST)
+   && (TYPE_PRECISION (TREE_TYPE (@3)) == TYPE_PRECISION (TREE_TYPE (@2)))
+   && tree_nop_conversion_p (type, TREE_TYPE (@0)))
+   (with { tree newtype = TREE_TYPE (@1); }
+(scmp (view_convert:newtype @0) (bit_not @1))
 
 (for cmp (simple_comparison)
  /* Fold (double)float1 CMP (double)float2 into float1 CMP float2.  */


Re: [patch] fix an openacc reference-type reduction bug in PR70643

2016-04-13 Thread Jakub Jelinek
On Wed, Apr 13, 2016 at 03:56:22PM -0700, Cesar Philippidis wrote:
> Thomas noticed that one of the openacc fortran tests involving
> reductions inside fortran modules in gomp-4_0-branch had regressed after
> I backported the recent reduction changes I made to trunk. That failure
> occurred because lower_oacc_reductions wasn't considering that reduction
> initializers can be non-integral constants. This patch fixes that
> problem and adds a new runtime test case.
> 
> Is this patch ok for trunk? I tested it on x86_64-linux.
> 
> Cesar

> 2016-04-13  Cesar Philippidis  
> 
>   gcc/
>   * omp-low.c (lower_oacc_reductions): Check for TREE_CONSTANT
>   when building a mem ref for the incoming reduction variable.
> 
>   libgomp/
>   * testsuite/libgomp.oacc-fortran/pr70643.f90: New test.

Ok.

Jakub


Re: [PATCH] Fix PR31096

2016-04-13 Thread Hurugalawadi, Naveen
Hi,

>> I think we should handle at least INTEGER_CST and SSA_NAME 
>> with VRP, and it seems natural to add a VRP check 

The check should be added in the tree_single_nonzero_warnv_p
for SSA_NAME case for tree_expr_nonzero_p.
However, for tree_expr_nonnegative_p, its been handled in a 
different way. Should we combine this check with the existing one?

+   (if (!tree_expr_nonnegative_p (@1))
+(cmp @2 @0))

>> Ideally, you would call tree_expr_nonpositive_p, except that that
>> function doesn't exist yet. So for now, I guess we

Would the tree_expr_nonpositive_p function be helpful for other cases
as well, I would try to add it if its useful.

Please find attached the modified patch as per the suggestions and
let me know if its fine?

Thanks,
Naveendiff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 0f4bf7e..5922dbd 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -9177,7 +9177,7 @@ tree_expr_nonzero_warnv_p (tree t, bool *strict_overflow_p)
 /* Return true when T is an address and is known to be nonzero.
Handle warnings about undefined signed overflow.  */
 
-static bool
+bool
 tree_expr_nonzero_p (tree t)
 {
   bool ret, strict_overflow_p;
diff --git a/gcc/fold-const.h b/gcc/fold-const.h
index 02f4270..8579622 100644
--- a/gcc/fold-const.h
+++ b/gcc/fold-const.h
@@ -167,6 +167,7 @@ extern tree size_diffop_loc (location_t, tree, tree);
 #define non_lvalue(T) non_lvalue_loc (UNKNOWN_LOCATION, T)
 extern tree non_lvalue_loc (location_t, tree);
 
+extern bool tree_expr_nonzero_p (tree);
 extern bool tree_expr_nonnegative_p (tree);
 extern bool tree_expr_nonnegative_warnv_p (tree, bool *, int = 0);
 extern tree make_range (tree, int *, tree *, tree *, bool *);
diff --git a/gcc/match.pd b/gcc/match.pd
index 75aa601..6655a3c 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -31,6 +31,7 @@ along with GCC; see the file COPYING3.  If not see
zerop
CONSTANT_CLASS_P
tree_expr_nonnegative_p
+   tree_expr_nonzero_p
integer_valued_real_p
integer_pow2p
HONOR_NANS)
@@ -894,7 +895,27 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
   && tree_nop_conversion_p (type, TREE_TYPE (@1)))
   (convert (bit_and (bit_not @1) @0
 
+/* Fold A * 10 == B * 10 into A == B.  */
+(for cmp (eq ne)
+ (simplify
+  (cmp (mult:c @0 @1) (mult:c @2 @1))
+  (if (INTEGRAL_TYPE_P (TREE_TYPE (@1))
+   && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))
+   && tree_expr_nonzero_p (@1))
+   (cmp @0 @2
 
+/* Fold A * 10 < B * 10 into A < B.  */
+(for cmp (lt gt le ge)
+ (simplify
+  (cmp (mult:c @0 @1) (mult:c @2 @1))
+  (if (INTEGRAL_TYPE_P (TREE_TYPE (@1))
+   && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))
+   && tree_expr_nonzero_p (@1))
+   (if (tree_expr_nonnegative_p (@1))
+(cmp @0 @2)
+   (if (TREE_CODE (@1) == INTEGER_CST
+	&& wi::lt_p (@1, 0, TYPE_SIGN (TREE_TYPE (@1
+(cmp @2 @0))
 
 /* ((X inner_op C0) outer_op C1)
With X being a tree where value_range has reasoned certain bits to always be
diff --git a/gcc/testsuite/gcc.dg/pr31096.c b/gcc/testsuite/gcc.dg/pr31096.c
new file mode 100644
index 000..72446bc
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr31096.c
@@ -0,0 +1,41 @@
+/* PR middle-end/31096 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" }  */
+
+int
+f (int a, int b)
+{
+  return a * 67 == b * 67;
+}
+
+int
+f1 (int a, int b)
+{
+  return a * -42 !=  b * -42;
+}
+
+int
+f2 (int a, int b)
+{
+  return a * 10 >= b * 10;
+}
+
+int
+f3 (int a, int b)
+{
+  return a * -4 <  b * -4;
+}
+
+int
+f4 (unsigned int a, unsigned int b)
+{
+  return a * 10 == b * 10;
+}
+
+int
+f5 (unsigned int a, unsigned int b)
+{
+  return a * -42 <  b * -42;
+}
+
+/* { dg-final { scan-tree-dump-times "\\(D\\) \\*" 4 "optimized" } } */