Re: [PATCH] Fix PR84512

2018-03-21 Thread Richard Biener
On Tue, 20 Mar 2018, Rainer Orth wrote:

> Hi Tom,
> 
> > On 03/19/2018 10:11 AM, Richard Biener wrote:
> >> On Fri, 16 Mar 2018, Tom de Vries wrote:
> >>
> >>> On 03/16/2018 12:55 PM, Richard Biener wrote:
>  On Fri, 16 Mar 2018, Tom de Vries wrote:
> 
> > On 02/27/2018 01:42 PM, Richard Biener wrote:
> >> Index: gcc/testsuite/gcc.dg/tree-ssa/pr84512.c
> >> ===
> >> --- gcc/testsuite/gcc.dg/tree-ssa/pr84512.c(nonexistent)
> >> +++ gcc/testsuite/gcc.dg/tree-ssa/pr84512.c(working copy)
> >> @@ -0,0 +1,15 @@
> >> +/* { dg-do compile } */
> >> +/* { dg-options "-O3 -fdump-tree-optimized" } */
> >> +
> >> +int foo()
> >> +{
> >> +  int a[10];
> >> +  for(int i = 0; i < 10; ++i)
> >> +a[i] = i*i;
> >> +  int res = 0;
> >> +  for(int i = 0; i < 10; ++i)
> >> +res += a[i];
> >> +  return res;
> >> +}
> >> +
> >> +/* { dg-final { scan-tree-dump "return 285;" "optimized" } } */
> >
> > This fails for nvptx, because it doesn't have the required vector
> > operations.
> > To fix the fail, I've added requiring effective target vect_int_mult.
> 
>  On targets that do not vectorize you should see the scalar loops unrolled
>  instead.  Or do you have only one loop vectorized?
> >>>
> >>> Sort of. Loop vectorization has no effect, and the scalar loops are 
> >>> completely
> >>> unrolled. But then slp vectorization vectorizes the stores.
> >>>
> >>> So at optimized we have:
> >>> ...
> >>>MEM[(int *)&a] = { 0, 1 };
> >>>MEM[(int *)&a + 8B] = { 4, 9 };
> >>>MEM[(int *)&a + 16B] = { 16, 25 };
> >>>MEM[(int *)&a + 24B] = { 36, 49 };
> >>>MEM[(int *)&a + 32B] = { 64, 81 };
> >>>_6 = a[0];
> >>>_28 = a[1];
> >>>res_29 = _6 + _28;
> >>>_35 = a[2];
> >>>res_36 = res_29 + _35;
> >>>_42 = a[3];
> >>>res_43 = res_36 + _42;
> >>>_49 = a[4];
> >>>res_50 = res_43 + _49;
> >>>_56 = a[5];
> >>>res_57 = res_50 + _56;
> >>>_63 = a[6];
> >>>res_64 = res_57 + _63;
> >>>_70 = a[7];
> >>>res_71 = res_64 + _70;
> >>>_77 = a[8];
> >>>res_78 = res_71 + _77;
> >>>_2 = a[9];
> >>>res_11 = _2 + res_78;
> >>>a ={v} {CLOBBER};
> >>>return res_11;
> >>> ...
> >>>
> >>> The stores and loads are eliminated by dse1 in the rtl phase, and in the 
> >>> end
> >>> we have:
> >>> ...
> >>> .visible .func (.param.u32 %value_out) foo
> >>> {
> >>>  .reg.u32 %value;
> >>>  .local .align 16 .b8 %frame_ar[48];
> >>>  .reg.u64 %frame;
> >>>  cvta.local.u64 %frame, %frame_ar;
> >>>  mov.u32 %value, 285;
> >>>  st.param.u32[%value_out], %value;
> >>>  ret;
> >>> }
> >>> ...
> >>>
>  That's precisely
>  what the PR was about...  which means it isn't fixed for nvptx :/
> >>>
> >>> Indeed the assembly is not optimal, and would be optimal if we'd have 
> >>> optimal
> >>> code at optimized.
> >>>
> >>> FWIW, using this patch we generate optimal code at optimized:
> >>> ...
> >>> diff --git a/gcc/passes.def b/gcc/passes.def
> >>> index 3ebcfc30349..6b64f600c4a 100644
> >>> --- a/gcc/passes.def
> >>> +++ b/gcc/passes.def
> >>> @@ -325,6 +325,7 @@ along with GCC; see the file COPYING3.  If not see
> >>> NEXT_PASS (pass_tracer);
> >>> NEXT_PASS (pass_thread_jumps);
> >>> NEXT_PASS (pass_dominator, false /* may_peel_loop_headers_p */);
> >>> +  NEXT_PASS (pass_fre);
> >>> NEXT_PASS (pass_strlen);
> >>> NEXT_PASS (pass_thread_jumps);
> >>> NEXT_PASS (pass_vrp, false /* warn_array_bounds_p */);
> >>> ...
> >>>
> >>> and we get:
> >>> ...
> >>> .visible .func (.param.u32 %value_out) foo
> >>> {
> >>>  .reg.u32 %value;
> >>>  mov.u32 %value, 285;
> >>>  st.param.u32[%value_out], %value;
> >>>  ret;
> >>> }
> >>> ...
> >>>
> >>> I could file a missing optimization PR for nvptx, but I'm not sure where 
> >>> this
> >>> should be fixed.
> >>
> >> Ah, yeah... the usual issue then.
> >>
> >> Can you please XFAIL the test on nvptx instead of requiring vect_int_mult?
> >>
> >
> > Done.
> >
> > Committed at attached.
> 
> this caused the test to FAIL on 64-bit (only) sparc-sun-solaris2.11:
> 
> FAIL: gcc.dg/tree-ssa/pr84512.c scan-tree-dump optimized "return 285;"
> 
> where it was UNSUPPORTED before.

So it failed before Toms original patch.  Please add sparc-solaris
to the list of XFAILed targets.

> The dump has
> 
> ;; Function foo (foo, funcdef_no=0, decl_uid=1557, cgraph_uid=0, 
> symbol_order=0)
> 
> foo ()
> {
>   int res;
>   int a[10];
>   int _2;
>   int _6;
>   int _28;
>   int _35;
>   int _42;
>   int _49;
>   int _56;
>   int _63;
>   int _70;
>   int _77;
> 
>[local count: 97603132]:
>   MEM[(int *)&a] = { 0, 1 };
>   MEM[(int *)&a + 8B] = { 4, 9 };
>   MEM[(int *)&a + 16B] = { 16, 25 };
>   MEM[(int *)&a + 24B] = { 

Re: [PATCH] Fix up handling of bool BIT_NOT_EXPRs in store-merging (PR tree-optimization/84982)

2018-03-21 Thread Richard Biener
On Tue, 20 Mar 2018, Jakub Jelinek wrote:

> Hi!
> 
> Boolean !x is often expanded as ^ 1, but store merging it actually merges
> as ^ 255 (for 8-bit bool), which is incorrect.
> 
> The following patch fixes it to do that ^ 1 instead.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux and checked on the
> testcase with -> powerpc64-linux cross, ok for trunk?
> 
> 2018-03-20  Jakub Jelinek  
> 
>   PR tree-optimization/84982
>   * gimple-ssa-store-merging.c (invert_op): Handle boolean inversion
>   by flipping the least significant bit rather than all bits from
>   bitpos to bitpos + bitsize - 1.
> 
>   * c-c++-common/pr84982.c: New test.
> 
> --- gcc/gimple-ssa-store-merging.c.jj 2018-03-20 13:53:31.701938584 +0100
> +++ gcc/gimple-ssa-store-merging.c2018-03-20 14:39:50.925657339 +0100
> @@ -3248,16 +3248,22 @@ invert_op (split_store *split_store, int
>unsigned int i;
>store_immediate_info *info;
>unsigned int cnt = 0;
> +  bool any_bools = false;
>FOR_EACH_VEC_ELT (split_store->orig_stores, i, info)
>  {
>bool bit_not_p = idx < 2 ? info->ops[idx].bit_not_p : info->bit_not_p;
>if (bit_not_p)
> - ++cnt;
> + {
> +   ++cnt;
> +   tree lhs = gimple_assign_lhs (info->stmt);
> +   if (TREE_CODE (TREE_TYPE (lhs)) == BOOLEAN_TYPE && info->bitsize > 1)

So I'm slightly uncomfortable with keying this just on BOOLEAN_TYPE.
Do you think anything would go wrong with simply using

 if (INTEGRAL_TYPE_P (TREE_TYPE (lhs))
 && TYPE_PRECISION (TREE_TYPE (lhs)) < info->bitsize)

?

It would then be any_padding rather than any_bool.

> + any_bools = true;
> + }
>  }
>mask = NULL_TREE;
>if (cnt == 0)
>  return NOP_EXPR;
> -  if (cnt == split_store->orig_stores.length ())
> +  if (cnt == split_store->orig_stores.length () && !any_bools)
>  return BIT_NOT_EXPR;
>  
>unsigned HOST_WIDE_INT try_bitpos = split_store->bytepos * BITS_PER_UNIT;
> @@ -3275,13 +3281,34 @@ invert_op (split_store *split_store, int
>set in the mask.  */
>unsigned HOST_WIDE_INT bitsize = info->bitsize;
>unsigned int pos_in_buffer = 0;
> +  bool is_bool = false;
> +  if (any_bools && bitsize > 1)
> + {
> +   tree lhs = gimple_assign_lhs (info->stmt);
> +   if (TREE_CODE (TREE_TYPE (lhs)) == BOOLEAN_TYPE)
> + is_bool = true;
> + }
>if (info->bitpos < try_bitpos)
>   {
> gcc_assert (info->bitpos + bitsize > try_bitpos);
> bitsize -= (try_bitpos - info->bitpos);
> +   if (is_bool && !BYTES_BIG_ENDIAN)
> + continue;
>   }
>else
>   pos_in_buffer = info->bitpos - try_bitpos;
> +  if (is_bool && bitsize)
> + {
> +   /* If this is a bool inversion, invert just the LSB
> +  rather than all bits of it.  */
> +   if (BYTES_BIG_ENDIAN)
> + {
> +   pos_in_buffer += bitsize - 1;
> +   if (pos_in_buffer >= split_store->size)
> + continue;
> + }
> +   bitsize = 1;
> + }
>if (pos_in_buffer + bitsize > split_store->size)
>   bitsize = split_store->size - pos_in_buffer;
>unsigned char *p = buf + (pos_in_buffer / BITS_PER_UNIT);
> --- gcc/testsuite/c-c++-common/pr84982.c.jj   2018-03-20 14:49:00.259744750 
> +0100
> +++ gcc/testsuite/c-c++-common/pr84982.c  2018-03-20 12:27:34.111363552 
> +0100
> @@ -0,0 +1,38 @@
> +/* PR tree-optimization/84982 */
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +
> +#ifndef __cplusplus
> +#define bool _Bool
> +#define true 1
> +#define false 0
> +#endif
> +
> +struct S { bool a, b, c, d; };
> +
> +__attribute__((noipa)) void
> +bar (bool *x)
> +{
> +  if (x[0] || !x[1] || !x[2] || x[3])
> +__builtin_abort ();
> +}
> +
> +__attribute__((noipa)) void
> +foo (struct S *x)
> +{
> +  bool a[4];
> +  a[0] = !x->a;
> +  a[1] = !x->b;
> +  a[2] = x->c;
> +  a[3] = !x->d;
> +  bar (a);
> +} 
> +
> +int
> +main ()
> +{
> +  struct S s;
> +  s.a = true; s.b = false; s.c = true; s.d = true;
> +  foo (&s);
> +  return 0;
> +}
> 
>   Jakub
> 
> 

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


Re: Seeking Release Manager approval for: [PATCH] jit: fix link on OS X and Solaris (PR jit/64089 and PR jit/84288)

2018-03-21 Thread Richard Biener
On Tue, 20 Mar 2018, David Malcolm wrote:

> On Wed, 2018-03-21 at 00:39 +0100, Rainer Orth wrote:
> > Hi Malcolm,
> > 
> > > > I've now tested the patch (together with the one from PR
> > > > jit/84288
> > > > for
> > > > several remaining issues).  I've needed another snippet for
> > > > Solaris/SPARC which links libkstat into xgcc and needs it in
> > > > libgccjit.so, too.  Bootstrapped without regressions on
> > > > i386-pc-solaris2.11 and sparc-sun-solaris2.11.
> > > 
> > > FWIW I've successfully tested this on x86_64-pc-linux-gnu
> > > (regenerating
> > > the gcc/configure), and, as jit maintainer, it looks good to me
> > > [1],
> > > though it may still need RM approval given stage 4.
> > 
> > thanks for trying this.
> > 
> > > [1] ...though I have a slight preference for listing
> > > $(EXTRA_GCC_LIBS) on the same line as $(EXTRA_GCC_OBJS) in
> > > jit/Make-
> > > lang.in, so that these two items needed to embed the driver code
> > > into
> > > the libgccjit shared library are visually grouped together.
> > 
> > I've selected the location of $(EXTRA_GCC_LIBS) in the link line to
> > match what gcc/Makefile.in does for xgcc etc.
> 
> Indeed, I don't want to bikeshed it - I care much more about whether it
> works ;)

I'm fine with this patch.

Richard.


poly_span_traits fixes (PR 84811)

2018-03-21 Thread Richard Sandiford
This patch fixes incorrect results for HOST_WIDE_INT positions
at opposite extremes when used with HOST_WIDE_INT sizes.  It also
fixes UB when comparing such positions with unsigned HOST_WIDE_INT
sizes (although the results in that case were wrapv-correct).

Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

Richard


2018-03-21  Richard Sandiford  

gcc/
PR tree-optimization/84811
* poly-int.h (poly_span_traits): Remove the T3 parameter and
promote HOST_WIDE_INT T2 - T1 results to unsigned HOST_WIDE_INT.
(maybe_in_range_p, known_in_range_p, ranges_known_overlap_p):
(known_subrange_p): Update accordingly.  Cast each value involved
in the size comparison, rather than casting the result of the
subtraction.

gcc/testsuite/
PR tree-optimization/84811
* gcc.dg/torture/pr84811.c: New test.

Index: gcc/poly-int.h
===
--- gcc/poly-int.h  2018-01-14 08:42:44.497155977 +
+++ gcc/poly-int.h  2018-03-21 08:28:14.656720617 +
@@ -2399,30 +2399,34 @@ print_dec (const poly_int_pod &val
 poly_coeff_traits::signedness ? SIGNED : UNSIGNED);
 }
 
-/* Helper for correctly comparing Pos - Start with Size in cases where
-   known_ge (Pos, Start), Pos and Start are potentially signed, and Size is
-   potentially unsigned.  Applying the cast function to the result of
-   Pos - Start gives the value that should be compared with the size.
-
-   Try to avoid doing any unnecessary arithmetic or copying.  */
-template
+/* Helper for calculating the distance between two points P1 and P2,
+   in cases where known_le (P1, P2).  T1 and T2 are the types of the
+   two positions, in either order.  The coefficients of P2 - P1 have
+   type unsigned HOST_WIDE_INT if the coefficients of both T1 and T2
+   have C++ primitive type, otherwise P2 - P1 has its usual
+   wide-int-based type.
+
+   The actual subtraction should look something like this:
+
+ typedef poly_span_traits span_traits;
+ span_traits::cast (P2) - span_traits::cast (P1)
+
+   Applying the cast before the subtraction avoids undefined overflow
+   for signed T1 and T2.
+
+   The implementation of the cast tries to avoid unnecessary arithmetic
+   or copying.  */
+template
 struct poly_span_traits
 {
-  /* Assume no cast is needed.  We'll get a warning about signed vs.
- unsigned comparisons if the assumption is wrong.  */
   template
   static const T &cast (const T &x) { return x; }
 };
 
-/* The only case a change in type is needed is this one, in which the
-   subtraction would give a HOST_WIDE_INT-based result if done on poly_ints
-   and adding a zero size would give an unsigned HOST_WIDE_INT-based
-   result.  Since we know known_ge (Pos, Start), it is safe to treat
-   Pos - Start as an unsigned HOST_WIDE_INT.  */
-template
-struct poly_span_traits
+template
+struct poly_span_traits
 {
   template
   static typename if_nonpoly::type
@@ -2451,7 +2455,8 @@ known_size_p (const T &a)
 inline bool
 maybe_in_range_p (const T1 &val, const T2 &pos, const T3 &size)
 {
-  typedef poly_span_traits span;
+  typedef poly_span_traits start_span;
+  typedef poly_span_traits size_span;
   if (known_lt (val, pos))
 return false;
   if (!known_size_p (size))
@@ -2462,7 +2467,8 @@ maybe_in_range_p (const T1 &val, const T
 /* In this case we don't know whether VAL >= POS is true at compile
time, so we can't prove that VAL >= POS + SIZE.  */
 return true;
-  return maybe_lt (span::cast (val - pos), size);
+  return maybe_lt (start_span::cast (val) - start_span::cast (pos),
+  size_span::cast (size));
 }
 
 /* Return true if range [POS, POS + SIZE) is known to include VAL.
@@ -2473,10 +2479,12 @@ maybe_in_range_p (const T1 &val, const T
 inline bool
 known_in_range_p (const T1 &val, const T2 &pos, const T3 &size)
 {
-  typedef poly_span_traits span;
+  typedef poly_span_traits start_span;
+  typedef poly_span_traits size_span;
   return (known_size_p (size)
  && known_ge (val, pos)
- && known_lt (span::cast (val - pos), size));
+ && known_lt (start_span::cast (val) - start_span::cast (pos),
+  size_span::cast (size)));
 }
 
 /* Return true if the two ranges [POS1, POS1 + SIZE1) and [POS2, POS2 + SIZE2)
@@ -2504,8 +2512,9 @@ ranges_maybe_overlap_p (const T1 &pos1,
 ranges_known_overlap_p (const T1 &pos1, const T2 &size1,
const T3 &pos2, const T4 &size2)
 {
-  typedef poly_span_traits span1;
-  typedef poly_span_traits span2;
+  typedef poly_span_traits start_span;
+  typedef poly_span_traits size1_span;
+  typedef poly_span_traits size2_span;
   /* known_gt (POS1 + SIZE1, POS2) [infinite precision]
  --> known_gt (SIZE1, POS2 - POS1) [infinite precision]
  --> known_gt (SIZE1, POS2 - lower_bound (POS1, POS2)) [infinite precision]
@@ -2520,8 +2529,12 @@ r

Fix ICE after sorry for big stack arguments (PR 84964)

2018-03-21 Thread Richard Sandiford
After the sorry we'd skip storing the argument, but that just creates an
inconsistency later when we try to deallocate the arguments.  This used to
"work" because pending_stack_adjust and stack_pointer_delta were int rather
than HWI, so 1<<33 got truncated to 0.

It's not easy to back out at this point because there's so much global
state floating around.  One option I tried was to put the sorry after:

  unadjusted_args_size
= compute_argument_block_size (reg_parm_stack_space,
   &adjusted_args_size,
   fndecl, fntype,
   (pass == 0 ? 0
: preferred_stack_boundary));

and then zero the argument sizes and num_actual.  That avoids the
ICE too, but who knows what other problems it creates.

In the end it seemed easier just to stop.

Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

Richard


2018-03-21  Richard Sandiford  

gcc/
PR error-recovery/84964
* diagnostic-core.h (fatal_sorry): Declare.
* diagnostic.c (fatal_sorry): New function.
* calls.c (expand_call): Use it instead of sorry.

gcc/testsuite/
PR error-recovery/84964
* g++.dg/diagnostic/pr84964.C: New test.

Index: gcc/diagnostic-core.h
===
--- gcc/diagnostic-core.h   2018-03-01 08:20:43.590534306 +
+++ gcc/diagnostic-core.h   2018-03-21 08:31:34.635677798 +
@@ -87,6 +87,8 @@ extern bool permerror (location_t, const
 extern bool permerror (rich_location *, const char *,
   ...) ATTRIBUTE_GCC_DIAG(2,3);
 extern void sorry (const char *, ...) ATTRIBUTE_GCC_DIAG(1,2);
+extern void fatal_sorry (const char *, ...) ATTRIBUTE_GCC_DIAG(1,2)
+ATTRIBUTE_NORETURN;
 extern void inform (location_t, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3);
 extern void inform (rich_location *, const char *, ...) 
ATTRIBUTE_GCC_DIAG(2,3);
 extern void inform_n (location_t, unsigned HOST_WIDE_INT, const char *,
Index: gcc/diagnostic.c
===
--- gcc/diagnostic.c2018-03-01 08:20:43.589534337 +
+++ gcc/diagnostic.c2018-03-21 08:31:34.635677798 +
@@ -1407,6 +1407,20 @@ sorry (const char *gmsgid, ...)
   va_end (ap);
 }
 
+/* Same, but stop compilation immediately.  */
+
+void
+fatal_sorry (const char *gmsgid, ...)
+{
+  va_list ap;
+  va_start (ap, gmsgid);
+  rich_location richloc (line_table, input_location);
+  diagnostic_impl (&richloc, -1, gmsgid, &ap, DK_SORRY);
+  va_end (ap);
+
+  exit (FATAL_EXIT_CODE);
+}
+
 /* Return true if an error or a "sorry" has been seen.  Various
processing is disabled after errors.  */
 bool
Index: gcc/calls.c
===
--- gcc/calls.c 2018-03-17 08:30:20.937100705 +
+++ gcc/calls.c 2018-03-21 08:31:34.635677798 +
@@ -4052,13 +4052,10 @@ expand_call (tree exp, rtx target, int i
  rtx_insn *before_arg = get_last_insn ();
 
  /* We don't allow passing huge (> 2^30 B) arguments
-by value.  It would cause an overflow later on.  */
+by value.  It would cause an overflow later on.  */
  if (constant_lower_bound (adjusted_args_size.constant)
  >= (1 << (HOST_BITS_PER_INT - 2)))
-   {
- sorry ("passing too large argument on stack");
- continue;
-   }
+   fatal_sorry ("passing too large argument on stack");
 
  if (store_one_arg (&args[i], argblock, flags,
 adjusted_args_size.var != 0,
Index: gcc/testsuite/g++.dg/diagnostic/pr84964.C
===
--- /dev/null   2018-03-17 08:19:33.716019995 +
+++ gcc/testsuite/g++.dg/diagnostic/pr84964.C   2018-03-21 08:31:34.635677798 
+
@@ -0,0 +1,6 @@
+/* { dg-do compile { target lp64 } } */
+
+struct a {
+  short b : 1ULL << 33; /* { dg-warning "width of 'a::b' exceeds its type" } */
+};
+void c(...) { c(a()); } /* { dg-message "sorry, unimplemented: passing too 
large argument on stack" } */


[PATCH] PR 84615 Regressions due to type mismatch with character functions

2018-03-21 Thread Janne Blomqvist
Since the kind of the hidden character length variable is not part of
the character variable definition, we must ensure that character
lengths are always of the same kind in interfaces, regardless of how
they were declared in the source. This patch ensures this when calling
a procedure.

Regtested on x86_64-pc-linux-gnu and i686-pc-linux-gnu, Ok for trunk?

gcc/fortran/ChangeLog:

2018-03-21  Janne Blomqvist  

PR fortra/84615
* trans-expr.c (gfc_conv_procedure_call): Convert charlen to
gfc_charlen_type_node when calling procedure.

gcc/testsuite/ChangeLog:

2018-03-21  Janne Blomqvist  

PR fortran/84615
* gfortran.dg/char_result_17.f90: New test.
---
 gcc/fortran/trans-expr.c |  8 ++--
 gcc/testsuite/gfortran.dg/char_result_17.f90 | 20 
 2 files changed, 26 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/char_result_17.f90

diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c
index 54bda1d..8bf5504 100644
--- a/gcc/fortran/trans-expr.c
+++ b/gcc/fortran/trans-expr.c
@@ -5973,9 +5973,13 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
  gfc_add_block_to_block (&se->pre, &parmse.pre);
  gfc_add_block_to_block (&se->post, &parmse.post);
  tmp = parmse.expr;
+ /* TODO: It would be better to have the charlens as
+gfc_charlen_type_node already when the interface is
+created instead of converting it here (see PR 84615).  */
  tmp = fold_build2_loc (input_location, MAX_EXPR,
-TREE_TYPE (tmp), tmp,
-build_zero_cst (TREE_TYPE (tmp)));
+gfc_charlen_type_node,
+fold_convert (gfc_charlen_type_node, tmp),
+build_zero_cst (gfc_charlen_type_node));
  cl.backend_decl = tmp;
}
 
diff --git a/gcc/testsuite/gfortran.dg/char_result_17.f90 
b/gcc/testsuite/gfortran.dg/char_result_17.f90
new file mode 100644
index 000..05ab72d
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/char_result_17.f90
@@ -0,0 +1,20 @@
+! { dg-do run }
+! PR fortran/84615
+! Charlen should always be the ABI defined character length type
+! regardless of which kind it is declared as in the source.
+program TestStringTools
+  character(len=52)   :: txt
+  character(len=1), dimension(52) :: chararr = &
+   (/(char(i+64),char(i+96), i = 1,26)/)
+  txt = chararray2string(chararr)
+  if (txt .ne. "AaBbCcDdEeFfGgHhIiJjKkLlMmNnOoPpQqRrSsTtUuVvWwXxYyZz") &
+   STOP 1
+contains
+  function chararray2string(chararray) result(text)
+character(len=1), dimension(:) :: chararray! input
+character(len=int(size(chararray, 1), kind=8)) :: text  ! output
+do i = 1,size(chararray,1)
+   text(i:i) = chararray (i)
+end do
+  end function chararray2string
+end program TestStringTools
-- 
2.7.4



Re: [C++ PATCH] Fix FIX_TRUNC_EXPR instantiation (PR c++/84942)

2018-03-21 Thread Jakub Jelinek
On Tue, Mar 20, 2018 at 05:00:34PM -0400, Jason Merrill wrote:
> On Mon, Mar 19, 2018 at 3:50 PM, Jakub Jelinek  wrote:
> > +int a(__attribute__((b((int)__builtin_inf() * 1ULL / auto;
> 
> This seems like another situation like 84610 and 84642 that Alex sent
> a patch for, of 'auto' in an attribute wrongly being treated as an
> implicit template parameter.  As I suggested in response to his patch,
> we ought to disable auto_is_implicit_function_parm_p within an
> attribute-specifier.

Ok, for this specific testcase I'll wait for Alex.  That doesn't mean
the FIX_TRUNC_EXPR handling isn't completely wrong though.
So, shall we fix it anyway, or remove or just gcc_unreachable ()?
It was added in PR51150 (it couldn't work even at that point,
cp_build_unary_op would use wrong return type even back then), but the
testcase never invokes this anymore, uses CAST_EXPR, STATIC_CAST_EXPR etc.
instead.

Jakub


[wwwdocs] Couple of items for gcc-8/changes.html

2018-03-21 Thread Eric Botcazou
Plus a wording change for the sake of consistency.  OK to commit?

-- 
Eric BotcazouIndex: htdocs/gcc-8/changes.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-8/changes.html,v
retrieving revision 1.47
diff -u -r1.47 changes.html
--- htdocs/gcc-8/changes.html	20 Mar 2018 08:22:03 -	1.47
+++ htdocs/gcc-8/changes.html	21 Mar 2018 08:42:39 -
@@ -30,8 +30,7 @@
   Support for the obsolete SDB/coff debug info format has been
   removed.  The option -gcoff no longer
   does anything.
-  The Cilk+ extensions to the C and C++ languages were removed from
-  GCC.
+  The Cilk+ extensions to the C and C++ languages have been removed.
   
 The MPX extensions to the C and C++ languages have been deprecated and
 will be removed in a future release.
@@ -103,6 +102,11 @@
 thus mitigate the attack vector that relies on jumping over
 a stack guard page as provided by the operating system.
   
+  
+A new pragma GCC unroll has been implemented in the C
+family of languages, as well as Fortran and Ada, so as to make it
+possible for the user to have a finer-grained control over the loop
+unrolling optimization than with optimization switches.
 
 
 
@@ -111,7 +115,11 @@
 
 Ada
 
-  
+  For its internal exception handling used on the host for error
+  recovery in the front-end, the compiler now relies on the native
+  exception handling mechanism of the host platform, which should
+  be more efficient than the former mechanism.
+  
 
 
 BRIG (HSAIL)


Re: [PATCH][ARM][PR82989] Fix unexpected use of NEON instructions for shifts

2018-03-21 Thread Christophe Lyon
On 20 March 2018 at 11:58, Sudakshina Das  wrote:
> Hi
>
> On 20/03/18 10:03, Richard Earnshaw (lists) wrote:
>>
>> On 14/03/18 10:11, Sudakshina Das wrote:
>>>
>>> Hi
>>>
>>> This patch fixes PR82989 so that we avoid NEON instructions when
>>> -mneon-for-64bits is not enabled. This is more of a short term fix for
>>> the real deeper problem of making and early decision of choosing or
>>> rejecting NEON instructions. There is now a new ticket PR84467 to deal
>>> with the longer term solution.
>>> (Please refer to the discussion in the bug report for more details).
>>>
>>> Testing: Bootstrapped and regtested on arm-none-linux-gnueabihf and
>>> added a new test case based on the test given on the bug report.
>>>
>>> Ok for trunk and backports for gcc-7 and gcc-6 branches?
>>>
>>
>> OK for trunk.  Please leave it a couple of days before backporting to
>> ensure that the testcase doesn't tickle any multilib issues.
>>
>> R.
>
>
> Thanks. Committed to trunk as r258677. Will wait a week for backporting.
>
> Sudi
>

Hi Sudi,

I've noticed that:
FAIL:gcc.target/arm/pr82989.c scan-assembler-times lsl\\tr[0-9]+,
r[0-9]+, r[0-9] 2
FAIL:gcc.target/arm/pr82989.c scan-assembler-times lsr\\tr[0-9]+,
r[0-9]+, r[0-9] 2
on target armeb-none-linux-gnueabihf
--with-mode thumb
--with-cpu cortex-a9
--with-fpu neon-fp16

The tests pass when using --with-mode arm

Can you check?

Thanks

Christophe

>
>>
>>> Sudi
>>>
>>>
>>> *** gcc/ChangeLog ***
>>>
>>> 2018-03-14  Sudakshina Das  
>>>
>>>  * config/arm/neon.md (ashldi3_neon): Update ?s for constraints
>>>  to favor GPR over NEON registers.
>>>  (di3_neon): Likewise.
>>>
>>> *** gcc/testsuite/ChangeLog ***
>>>
>>> 2018-03-14  Sudakshina Das  
>>>
>>>  * gcc.target/arm/pr82989.c: New test.
>>>
>>> pr82989.diff
>>>
>>>
>>> diff --git a/gcc/config/arm/neon.md b/gcc/config/arm/neon.md
>>> index 6a6f5d7..1646b21 100644
>>> --- a/gcc/config/arm/neon.md
>>> +++ b/gcc/config/arm/neon.md
>>> @@ -1180,12 +1180,12 @@
>>>   )
>>> (define_insn_and_split "ashldi3_neon"
>>> -  [(set (match_operand:DI 0 "s_register_operand"   "= w,
>>> w,?&r,?r,?&r, ?w,w")
>>> -   (ashift:DI (match_operand:DI 1 "s_register_operand" " 0w, w, 0r,
>>> 0,  r, 0w,w")
>>> -  (match_operand:SI 2 "general_operand""rUm, i,  r,
>>> i,  i,rUm,i")))
>>> -   (clobber (match_scratch:SI 3"= X,
>>> X,?&r, X,  X,  X,X"))
>>> -   (clobber (match_scratch:SI 4"= X,
>>> X,?&r, X,  X,  X,X"))
>>> -   (clobber (match_scratch:DI 5"=&w,
>>> X,  X, X,  X, &w,X"))
>>> +  [(set (match_operand:DI 0 "s_register_operand"   "= w, w, &r,
>>> r, &r, ?w,?w")
>>> +   (ashift:DI (match_operand:DI 1 "s_register_operand" " 0w, w, 0r,
>>> 0,  r, 0w, w")
>>> +  (match_operand:SI 2 "general_operand""rUm, i,  r,
>>> i,  i,rUm, i")))
>>> +   (clobber (match_scratch:SI 3"= X,
>>> X, &r, X,  X,  X, X"))
>>> +   (clobber (match_scratch:SI 4"= X,
>>> X, &r, X,  X,  X, X"))
>>> +   (clobber (match_scratch:DI 5"=&w,
>>> X,  X, X,  X, &w, X"))
>>>  (clobber (reg:CC_C CC_REGNUM))]
>>> "TARGET_NEON"
>>> "#"
>>> @@ -1276,7 +1276,7 @@
>>>   ;; ashrdi3_neon
>>>   ;; lshrdi3_neon
>>>   (define_insn_and_split "di3_neon"
>>> -  [(set (match_operand:DI 0 "s_register_operand""= w,
>>> w,?&r,?r,?&r,?w,?w")
>>> +  [(set (match_operand:DI 0 "s_register_operand""= w, w, &r,
>>> r, &r,?w,?w")
>>> (RSHIFTS:DI (match_operand:DI 1 "s_register_operand" " 0w, w, 0r,
>>> 0,  r,0w, w")
>>> (match_operand:SI 2 "reg_or_int_operand" "  r, i,  r,
>>> i,  i, r, i")))
>>>  (clobber (match_scratch:SI 3
>>> "=2r, X, &r, X,  X,2r, X"))
>>> diff --git a/gcc/testsuite/gcc.target/arm/pr82989.c
>>> b/gcc/testsuite/gcc.target/arm/pr82989.c
>>> new file mode 100644
>>> index 000..1295ee6
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arm/pr82989.c
>>> @@ -0,0 +1,38 @@
>>> +/* PR target/82989 */
>>> +/* { dg-do compile } */
>>> +/* { dg-require-effective-target arm_neon_ok } */
>>> +/* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } {
>>> "-mcpu=*" } { "-mcpu=cortex-a8" } } */
>>> +/* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } {
>>> "-mfpu=*" } { "-mfpu=neon" } } */
>>> +/* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } {
>>> "-mfloat-abi=*" } { "-mfloat-abi=hard" } } */
>>> +/* { dg-options "-O2 -mcpu=cortex-a8 -mfpu=neon -mfloat-abi=hard" } */
>>> +/* { dg-add-options arm_neon } */
>>> +
>>> +typedef unsigned long long uint64_t;
>>> +
>>> +void f_shr_imm (uint64_t *a )
>>> +{
>>> +  *a += *a >> 32;
>>> +}
>>> +/* { dg-final { scan-assembler-not "vshr*" } } */
>>> +
>>> +void f_shr_reg (uint64_t *a, uint64_t b)
>>> +{
>>> +  *a += *a >> b;
>>> +}
>>> +/* { dg-final { s

Re: [PATCH] Fix PR84512

2018-03-21 Thread Eric Botcazou
> So it failed before Toms original patch.  Please add sparc-solaris
> to the list of XFAILed targets.

SPARC/Linux is affected too so sparc*-*-* instead.

-- 
Eric Botcazou


[PATCH, gotools]: Add -test.timeout to to runtime gotools tests

2018-03-21 Thread Uros Bizjak
Hello!

Attached patch adds -test.timeout argument to runtime gotools test.
Without this argument, default 4 minute timeout instead of
$(GOTOOLS_TEST_TIMEOUT) is used which is way to short for slow
machines.

Patch was tested on alphaev68-linux-gnu.

Uros.
diff --git a/gotools/Makefile.am b/gotools/Makefile.am
index 2019497..fb5db63 100644
--- a/gotools/Makefile.am
+++ b/gotools/Makefile.am
@@ -256,14 +256,14 @@ check-runtime: go$(EXEEXT) $(noinst_PROGRAMS) check-head 
check-gccgo check-gcc
GOARCH=`$(abs_builddir)/go$(EXEEXT) env GOARCH`; \
GOOS=`$(abs_builddir)/go$(EXEEXT) env GOOS`; \
files=`$(SHELL) $(libgosrcdir)/../match.sh --goarch=$${GOARCH} 
--goos=$${GOOS} --srcdir=$(libgosrcdir)/runtime 
--extrafiles="$(libgodir)/runtime_sysinfo.go $(libgodir)/sigtab.go" 
--tag=libffi`; \
-   echo "$(ECHO_ENV) GC='$(abs_builddir)/check-gccgo 
-fgo-compiling-runtime' GOARCH=$${GOARCH} GOOS=$${GOOS} $(SHELL) 
$(libgosrcdir)/../testsuite/gotest --goarch=$${GOARCH} --goos=$${GOOS} 
--basedir=$(libgosrcdir)/.. --srcdir=$(libgosrcdir)/runtime --pkgpath=runtime 
--pkgfiles='$${files}' $(GOTESTFLAGS) -test.v" > runtime-testlog
+   echo "$(ECHO_ENV) GC='$(abs_builddir)/check-gccgo 
-fgo-compiling-runtime' GOARCH=$${GOARCH} GOOS=$${GOOS} $(SHELL) 
$(libgosrcdir)/../testsuite/gotest --goarch=$${GOARCH} --goos=$${GOOS} 
--basedir=$(libgosrcdir)/.. --srcdir=$(libgosrcdir)/runtime --pkgpath=runtime 
--pkgfiles='$${files}' $(GOTESTFLAGS) -test.timeout=$(GOTOOLS_TEST_TIMEOUT)s 
-test.v" > runtime-testlog
$(CHECK_ENV) \
GC="$${GCCGO} -fgo-compiling-runtime"; \
export GC; \
GOARCH=`$(abs_builddir)/go$(EXEEXT) env GOARCH`; \
GOOS=`$(abs_builddir)/go$(EXEEXT) env GOOS`; \
files=`$(SHELL) $(libgosrcdir)/../match.sh --goarch=$${GOARCH} 
--goos=$${GOOS} --srcdir=$(libgosrcdir)/runtime 
--extrafiles="$(libgodir)/runtime_sysinfo.go $(libgodir)/sigtab.go" 
--tag=libffi`; \
-   $(SHELL) $(libgosrcdir)/../testsuite/gotest --goarch=$${GOARCH} 
--goos=$${GOOS} --basedir=$(libgosrcdir)/.. --srcdir=$(libgosrcdir)/runtime 
--pkgpath=runtime --pkgfiles="$${files}" $(GOTESTFLAGS) -test.v >> 
runtime-testlog 2>&1 || echo "--- $${fl}: go test runtime (0.00s)" >> 
runtime-testlog
+   $(SHELL) $(libgosrcdir)/../testsuite/gotest --goarch=$${GOARCH} 
--goos=$${GOOS} --basedir=$(libgosrcdir)/.. --srcdir=$(libgosrcdir)/runtime 
--pkgpath=runtime --pkgfiles="$${files}" $(GOTESTFLAGS) 
-test.timeout=$(GOTOOLS_TEST_TIMEOUT)s -test.v >> runtime-testlog 2>&1 || echo 
"--- $${fl}: go test runtime (0.00s)" >> runtime-testlog
grep '^--- ' runtime-testlog | sed -e 's/^--- \(.*\) ([^)]*)$$/\1/' | 
sort -k 2
 
 # check-cgo-test runs `go test misc/cgo/test` in our environment.
diff --git a/gotools/Makefile.in b/gotools/Makefile.in
index 4d42d5e..13b13ee 100644
--- a/gotools/Makefile.in
+++ b/gotools/Makefile.in
@@ -835,14 +835,14 @@ mostlyclean-local:
 @NATIVE_TRUE@  GOARCH=`$(abs_builddir)/go$(EXEEXT) env GOARCH`; \
 @NATIVE_TRUE@  GOOS=`$(abs_builddir)/go$(EXEEXT) env GOOS`; \
 @NATIVE_TRUE@  files=`$(SHELL) $(libgosrcdir)/../match.sh --goarch=$${GOARCH} 
--goos=$${GOOS} --srcdir=$(libgosrcdir)/runtime 
--extrafiles="$(libgodir)/runtime_sysinfo.go $(libgodir)/sigtab.go" 
--tag=libffi`; \
-@NATIVE_TRUE@  echo "$(ECHO_ENV) GC='$(abs_builddir)/check-gccgo 
-fgo-compiling-runtime' GOARCH=$${GOARCH} GOOS=$${GOOS} $(SHELL) 
$(libgosrcdir)/../testsuite/gotest --goarch=$${GOARCH} --goos=$${GOOS} 
--basedir=$(libgosrcdir)/.. --srcdir=$(libgosrcdir)/runtime --pkgpath=runtime 
--pkgfiles='$${files}' $(GOTESTFLAGS) -test.v" > runtime-testlog
+@NATIVE_TRUE@  echo "$(ECHO_ENV) GC='$(abs_builddir)/check-gccgo 
-fgo-compiling-runtime' GOARCH=$${GOARCH} GOOS=$${GOOS} $(SHELL) 
$(libgosrcdir)/../testsuite/gotest --goarch=$${GOARCH} --goos=$${GOOS} 
--basedir=$(libgosrcdir)/.. --srcdir=$(libgosrcdir)/runtime --pkgpath=runtime 
--pkgfiles='$${files}' $(GOTESTFLAGS) -test.timeout=$(GOTOOLS_TEST_TIMEOUT)s 
-test.v" > runtime-testlog
 @NATIVE_TRUE@  $(CHECK_ENV) \
 @NATIVE_TRUE@  GC="$${GCCGO} -fgo-compiling-runtime"; \
 @NATIVE_TRUE@  export GC; \
 @NATIVE_TRUE@  GOARCH=`$(abs_builddir)/go$(EXEEXT) env GOARCH`; \
 @NATIVE_TRUE@  GOOS=`$(abs_builddir)/go$(EXEEXT) env GOOS`; \
 @NATIVE_TRUE@  files=`$(SHELL) $(libgosrcdir)/../match.sh --goarch=$${GOARCH} 
--goos=$${GOOS} --srcdir=$(libgosrcdir)/runtime 
--extrafiles="$(libgodir)/runtime_sysinfo.go $(libgodir)/sigtab.go" 
--tag=libffi`; \
-@NATIVE_TRUE@  $(SHELL) $(libgosrcdir)/../testsuite/gotest --goarch=$${GOARCH} 
--goos=$${GOOS} --basedir=$(libgosrcdir)/.. --srcdir=$(libgosrcdir)/runtime 
--pkgpath=runtime --pkgfiles="$${files}" $(GOTESTFLAGS) -test.v >> 
runtime-testlog 2>&1 || echo "--- $${fl}: go test runtime (0.00s)" >> 
runtime-testlog
+@NATIVE_TRUE@  $(SHELL) $(libgosrcdir)/../testsuite/gotest --goarch=$${GOARCH} 
--goos=$${GOOS} --basedir=$(libgosrcdir)/.. --srcdir=$(libgosrcdir)/runtime 
--pkgpath=runtime --pkgfiles="$${files}" $(GO

Re: [PATCH][arm] PR target/82518: Return false in ARRAY_MODE_SUPPORTED_P for BYTES_BIG_ENDIAN

2018-03-21 Thread Christophe Lyon
On 20 March 2018 at 18:11, Kyrill  Tkachov  wrote:
> Hi all,
>
> This PR shows that we get the load/store_lanes logic wrong for arm
> big-endian.
> It is tricky to get right. Aarch64 does it by adding the appropriate
> lane-swapping
> operations during expansion.
>
> I'd like to do the same on arm eventually, but we'd need to port and
> validate the VTBL-generating
> code and add it to all the right places and I'm not comfortable enough doing
> it for GCC 8, but I am keen
> in getting the wrong-code fixed.
> As I say in the PR, vectorisation on armeb is already severely restricted
> (we disable many patterns on BYTES_BIG_ENDIAN)
> and the load/store_lanes patterns really were not working properly at all,
> so disabling them is not
> a radical approach.
>
> The way to do that is to return false in ARRAY_MODE_SUPPORTED_P for
> BYTES_BIG_ENDIAN.
>
> Bootstrapped and tested on arm-none-linux-gnueabihf.
> Also tested on armeb-none-eabi.
>
> Committing to trunk.
> Thanks,
> Kyrill
>
> 2018-03-20  Kyrylo Tkachov  
>
> PR target/82518
> * config/arm/arm.c (arm_array_mode_supported_p): Return false for
> BYTES_BIG_ENDIAN.
>
> 2018-03-20  Kyrylo Tkachov  
>
> PR target/82518
> * lib/target-supports.exp (check_effective_target_vect_load_lanes):
> Disable for armeb targets.
> * gcc.target/arm/pr82518.c: New test.

Hi Kyrill,

I think you need:
Index: pr82518.c
===
--- pr82518.c   (revision 258705)
+++ pr82518.c   (working copy)
@@ -1,5 +1,5 @@
 /* { dg-do run } */
-/* { dg-require-effective-target arm_neon_ok } */
+/* { dg-require-effective-target arm_neon_hw } */
 /* { dg-additional-options "-O3 -fno-inline -std=gnu99" } */
 /* { dg-add-options arm_neon } */

(The test fails to run for me when using qemu --cpu arm926)

OK?

Christophe


Re: [PATCH][arm] PR target/82518: Return false in ARRAY_MODE_SUPPORTED_P for BYTES_BIG_ENDIAN

2018-03-21 Thread Kyrill Tkachov


On 21/03/18 09:07, Christophe Lyon wrote:

On 20 March 2018 at 18:11, Kyrill  Tkachov  wrote:

Hi all,

This PR shows that we get the load/store_lanes logic wrong for arm
big-endian.
It is tricky to get right. Aarch64 does it by adding the appropriate
lane-swapping
operations during expansion.

I'd like to do the same on arm eventually, but we'd need to port and
validate the VTBL-generating
code and add it to all the right places and I'm not comfortable enough doing
it for GCC 8, but I am keen
in getting the wrong-code fixed.
As I say in the PR, vectorisation on armeb is already severely restricted
(we disable many patterns on BYTES_BIG_ENDIAN)
and the load/store_lanes patterns really were not working properly at all,
so disabling them is not
a radical approach.

The way to do that is to return false in ARRAY_MODE_SUPPORTED_P for
BYTES_BIG_ENDIAN.

Bootstrapped and tested on arm-none-linux-gnueabihf.
Also tested on armeb-none-eabi.

Committing to trunk.
Thanks,
Kyrill

2018-03-20  Kyrylo Tkachov  

 PR target/82518
 * config/arm/arm.c (arm_array_mode_supported_p): Return false for
 BYTES_BIG_ENDIAN.

2018-03-20  Kyrylo Tkachov  

 PR target/82518
 * lib/target-supports.exp (check_effective_target_vect_load_lanes):
 Disable for armeb targets.
 * gcc.target/arm/pr82518.c: New test.

Hi Kyrill,

I think you need:
Index: pr82518.c
===
--- pr82518.c   (revision 258705)
+++ pr82518.c   (working copy)
@@ -1,5 +1,5 @@
  /* { dg-do run } */
-/* { dg-require-effective-target arm_neon_ok } */
+/* { dg-require-effective-target arm_neon_hw } */
  /* { dg-additional-options "-O3 -fno-inline -std=gnu99" } */
  /* { dg-add-options arm_neon } */

(The test fails to run for me when using qemu --cpu arm926)

OK?


Yes, you're right. Thanks Christophe.

Kyrill


Christophe




Re: [PR middle-end/70359] uncoalesce IVs outside of loops

2018-03-21 Thread Richard Biener
On Tue, Mar 20, 2018 at 7:15 PM, Bin.Cheng  wrote:
> On Tue, Mar 20, 2018 at 5:56 PM, Richard Biener
>  wrote:
>> On March 20, 2018 6:11:53 PM GMT+01:00, "Bin.Cheng"  
>> wrote:
>>>On Mon, Mar 19, 2018 at 5:08 PM, Aldy Hernandez 
>>>wrote:
 Hi Richard.

 As discussed in the PR, the problem here is that we have two
>>>different
 iterations of an IV live outside of a loop.  This inhibits us from
>>>using
 autoinc/dec addressing on ARM, and causes extra lea's on x86.

 An abbreviated example is this:

 loop:
   # p_9 = PHI 
   p_20 = p_9 + 18446744073709551615;
 goto loop
   p_24 = p_9 + 18446744073709551614;
   MEM[(char *)p_20 + -1B] = 45;

 Here we have both the previous IV (p_9) and the current IV (p_20)
>>>used
 outside of the loop.  On Arm this keeps us from using auto-dec
>>>addressing,
 because one use is -2 and the other one is -1.

 With the attached patch we attempt to rewrite out-of-loop uses of the
>>>IV in
 terms of the current/last IV (p_20 in the case above).  With it, we
>>>end up
 with:

   p_24 = p_20 + 18446744073709551615;
   *p_24 = 45;

 ...which helps both x86 and Arm.

 As you have suggested in comment 38 on the PR, I handle specially
 out-of-loop IV uses of the form IV+CST and propagate those
>>>accordingly
 (along with the MEM_REF above).  Otherwise, in less specific cases,
>>>we un-do
 the IV increment, and use that value in all out-of-loop uses.  For
>>>instance,
 in the attached testcase, we rewrite:

   george (p_9);

 into

   _26 = p_20 + 1;
   ...
   george (_26);

 The attached testcase tests the IV+CST specific case, as well as the
>>>more
 generic case with george().

 Although the original PR was for ARM, this behavior can be noticed on
>>>x86,
 so I tested on x86 with a full bootstrap + tests.  I also ran the
>>>specific
 test on an x86 cross ARM build and made sure we had 2 auto-dec with
>>>the
 test.  For the original test (slightly different than the testcase in
>>>this
 patch), with this patch we are at 104 bytes versus 116 without it.
>>>There is
 still the issue of a division optimization which would further reduce
>>>the
 code size.  I will discuss this separately as it is independent from
>>>this
 patch.

 Oh yeah, we could make this more generic, and maybe handle any
>>>multiple of
 the constant, or perhaps *= and /=.  Perhaps something for next
>>>stage1...

 OK for trunk?
>>>Just FYI, this looks similar to what I did in
>>>https://gcc.gnu.org/ml/gcc-patches/2013-11/msg00535.html
>>>That change was non-trivial and didn't give obvious improvement back
>>>in time.  But I still wonder if this
>>>can be done at rewriting iv_use in a light-overhead way.
>>
>> Certainly, but the issue is we wreck it again at forwprop time as ivopts 
>> runs too early.
> So both values of p_9/p_20 are used after loop.
>
> loop:
>   # p_9 = PHI 
>   p_20 = p_9 + 18446744073709551615;
> goto loop
>   p_24 = p_20 + 18446744073709551615;
>   MEM[(char *)p_20 + -1B] = 45;
>
> It looks like a fwprop issue that propagating p_20 with p_9 which
> results in below code:
>
> loop:
>   # p_9 = PHI 
>   p_20 = p_9 + 18446744073709551615;
> goto loop
>   p_24 = p_9 + 18446744073709551614;
>   MEM[(char *)p_20 + -1B] = 45;
>
> It creates intersecting/longer live ranges while doesn't eliminate
> copy or definition for p_9.

Yes.  It's actually general folding patterns that combine the two adds.
It may be profitable to do this even if intersecting live ranges because
you gain some scheduling freedom and reduce dependences.

So it's not easy to avoid in general.

> Ah, IIRC, RTL address forward propagation also has this issue.

I guess so.

Richard.

> Thanks,
> bin
>>
>> Richard.
>>>
>>>Thanks,
>>>bin
 Aldy
>>


Re: [PATCH PR84969]Don't reorder builtin memsets if they set different rhs values

2018-03-21 Thread Richard Biener
On Tue, Mar 20, 2018 at 7:18 PM, Bin Cheng  wrote:
> Hi,
> As noted in PR84969, fuse_memset_builtins breaks dependence between different 
> memsets.
> Specifically, it reorders different builtin memset partitions though it 
> doesn't merge
> them in the end.  This simple patch fixes this wrong code issue by checking 
> if any two
> builtin memsets set the same rhs value or not.  Note we don't need to bother 
> if two
> memsets intersect with each other or not.
>
> Of course, this would miss opportunity merging S1/S3 in below case:
>   memset(p+12, 0, 12);   //<-S1
>   memset(p+17, 1, 10);
>   memset(p, 0, 12);  //<-S3
> In my opinion, this should be resolved in a more general way maximizing 
> parallelism
> as well as merging opportunities when sorting partitions into topological 
> order from
> dependence graph, which isn't GCC8 task.
>
> Bootstrap and test on x86_64 and AArch64 ongoing.  Okay if no failures?

OK.

Richard.

> Thanks,
> bin
>
> 2018-03-20  Bin Cheng  
>
> PR tree-optimization/84969
> * tree-loop-distribution.c (fuse_memset_builtins): Don't reorder
> builtin memset partitions if they set differnt rhs values.
>
> gcc/testsuite
> 2018-03-20  Bin Cheng  
>
> PR tree-optimization/84969
> * gcc.dg/tree-ssa/pr84969.c: New test.
>


[C++ Patch] PR 84972 ("[6/7/8 Regression] ICE: tree check: expected class 'type', have 'exceptional' (error_mark)...")

2018-03-21 Thread Paolo Carlini

Hi,

as I told Marek in the audit trail, I'm working on a more complete patch 
consistently changing maybe_deduce_size_from_array_init for various 
error recovery issues (see the trail of c++/84632 too), but since this 
specific regression ultimately started with a previous change of mine 
and it's very simple I decided to send it now, for 8.1.0. Essentially, 
the idea is setting TREE_TYPE (decl) to error_mark_node - the normal 
error recovery mechanism in this function via cp_complete_array_type - 
also when the early check_array_designated_initializer fails.


Tested x86_64-linux, Paolo.

//

/cp
2018-03-21  Paolo Carlini  

PR c++/84972
* decl.c (maybe_deduce_size_from_array_init): Set TREE_TYPE to
error_mark_node when check_array_designated_initializer fails.

/testsuite
2018-03-21  Paolo Carlini  

PR c++/84972
* g++.dg/ext/desig10.C: New.
Index: cp/decl.c
===
--- cp/decl.c   (revision 258706)
+++ cp/decl.c   (working copy)
@@ -5467,7 +5467,9 @@ maybe_deduce_size_from_array_init (tree decl, tree
  failure = 1;
}
 
-  if (!failure)
+  if (failure)
+   TREE_TYPE (decl) = error_mark_node;
+  else
{
  failure = cp_complete_array_type (&TREE_TYPE (decl), initializer,
do_default);
Index: testsuite/g++.dg/ext/desig10.C
===
--- testsuite/g++.dg/ext/desig10.C  (nonexistent)
+++ testsuite/g++.dg/ext/desig10.C  (working copy)
@@ -0,0 +1,4 @@
+// PR c++/84972
+// { dg-additional-options "-w" }
+
+char(a[])({.a = 0});  // { dg-error "designated initializer" }


Re: [PATCH] Fix compile-time hog in MPX boundary checking (PR target/84988).

2018-03-21 Thread Richard Biener
On Tue, Mar 20, 2018 at 10:44 PM, Richard Sandiford
 wrote:
> Jeff Law  writes:
>> On 03/20/2018 01:36 PM, Martin Liška wrote:
>>> Hi.
>>>
>>> This is a work-around to not iterate all members of array that can be huge.
>>> As MPX will be removed in GCC 9.x, I hope it's acceptable. I don't want
>>> to come
>>> up with a new param for it.
>>>
>>> Survives tests&bootstrap on x86_64-linux-gnu.
>>>
>>> Martin
>>>
>>> gcc/ChangeLog:
>>>
>>> 2018-03-20  Martin Liska  
>>>
>>> PR target/84988
>>> * tree-chkp.c (CHKP_ARRAY_MAX_CHECK_STEPS): Define a new macro.
>>> (chkp_find_bound_slots_1): Limit number of iterations.
>> Or just CLOSE/WONTFIX :-)
>>
>> I've got no objections here -- we want to  minimize the effort put into
>> CHKP given its going to be deprecated.
>
> The problem is that this affects normal configs, not just ones with
> MPX enabled.

Indeed.  It get's called via

#0  chkp_find_bound_slots_1 (type=0x769ee9d8, have_bound=0x2ed3868, offs=0)
at /space/rguenther/src/svn/early-lto-debug/gcc/tree-chkp.c:1708
#1  0x01379a13 in chkp_find_bound_slots (type=0x769ee9d8,
res=0x2ed3868)
at /space/rguenther/src/svn/early-lto-debug/gcc/tree-chkp.c:1754
#2  0x01377054 in chkp_type_bounds_count (type=0x769ee9d8)
at /space/rguenther/src/svn/early-lto-debug/gcc/tree-chkp.c:1009
#3  0x016c664f in ix86_function_arg_advance (cum_v=...,
mode=E_BLKmode, type=0x769ee9d8, named=true)
at /space/rguenther/src/svn/early-lto-debug/gcc/config/i386/i386.c:8621
8616{
8617  /* Track if there are outgoing arguments on stack.  */
8618  if (cum->caller)
8619cfun->machine->outgoing_args_on_stack = true;
8620
8621  cum->bnds_in_bt = chkp_type_bounds_count (type);
8622}
8623}
8624
8625/* Define where to put the arguments to a function.

but I think we know POINTER_BOUNDS_TYPE_P etc. never return
true if -fcheck-pointer-* or -mmpx is not enabled, right?  So we can
guard the above call appropriately and save some compile-time
for all of us?

Richard.

> Thanks,
> Richard


Re: [PATCH] Fix compile-time hog in MPX boundary checking (PR target/84988).

2018-03-21 Thread Martin Liška

On 03/21/2018 10:36 AM, Richard Biener wrote:

On Tue, Mar 20, 2018 at 10:44 PM, Richard Sandiford
 wrote:

Jeff Law  writes:

On 03/20/2018 01:36 PM, Martin Liška wrote:

Hi.

This is a work-around to not iterate all members of array that can be huge.
As MPX will be removed in GCC 9.x, I hope it's acceptable. I don't want
to come
up with a new param for it.

Survives tests&bootstrap on x86_64-linux-gnu.

Martin

gcc/ChangeLog:

2018-03-20  Martin Liska  

 PR target/84988
 * tree-chkp.c (CHKP_ARRAY_MAX_CHECK_STEPS): Define a new macro.
 (chkp_find_bound_slots_1): Limit number of iterations.

Or just CLOSE/WONTFIX :-)

I've got no objections here -- we want to  minimize the effort put into
CHKP given its going to be deprecated.


The problem is that this affects normal configs, not just ones with
MPX enabled.


Indeed.  It get's called via

#0  chkp_find_bound_slots_1 (type=0x769ee9d8, have_bound=0x2ed3868, offs=0)
 at /space/rguenther/src/svn/early-lto-debug/gcc/tree-chkp.c:1708
#1  0x01379a13 in chkp_find_bound_slots (type=0x769ee9d8,
 res=0x2ed3868)
 at /space/rguenther/src/svn/early-lto-debug/gcc/tree-chkp.c:1754
#2  0x01377054 in chkp_type_bounds_count (type=0x769ee9d8)
 at /space/rguenther/src/svn/early-lto-debug/gcc/tree-chkp.c:1009
#3  0x016c664f in ix86_function_arg_advance (cum_v=...,
 mode=E_BLKmode, type=0x769ee9d8, named=true)
 at /space/rguenther/src/svn/early-lto-debug/gcc/config/i386/i386.c:8621
8616{
8617  /* Track if there are outgoing arguments on stack.  */
8618  if (cum->caller)
8619cfun->machine->outgoing_args_on_stack = true;
8620
8621  cum->bnds_in_bt = chkp_type_bounds_count (type);
8622}
8623}
8624
8625/* Define where to put the arguments to a function.

but I think we know POINTER_BOUNDS_TYPE_P etc. never return
true if -fcheck-pointer-* or -mmpx is not enabled, right?  So we can
guard the above call appropriately and save some compile-time
for all of us?


Good observation, let me enhance the patch for the PR.

Martin



Richard.


Thanks,
Richard




Re: [PATCH, PR84660] Fix combine bug with SHIFT_COUNT_TRUNCATED.

2018-03-21 Thread Richard Biener
On Tue, Mar 20, 2018 at 11:10 PM, Jim Wilson  wrote:
> This fixes a wrong-code issue on RISC-V, but in theory could be a problem for
> any SHIFT_COUNT_TRUNCATED target.
>
> The testcase computes 46 or 47 (0x2e or 0x2f), then ANDs the value with 0xf,
> and then SHIFTs the value.  On a SHIFT_COUNT_TRUNCATED target, the AND can get
> optimized away because for a 32-bit shift we can use the 46/47 directly and
> still get the correct result.  Combine then tries to convert the 32-bit shift
> into a 64-bit shift, and then we have a problem, as the AND 0xf is no longer
> redundant.  So we must prevent combine from converting a 32-bit shift to a
> 64-bit shift on a SHIFT_COUNT_TRUNCATED target when there are non-zero bits
> in the shift count that matter for the larger shift mode.
>
> Combine already has code to handle the case where shifts are being narrowed
> and this accidentally changes the shift amount.  This patch adds additional
> code to handle the case where shifts are being widened.
>
> This was tested with a cross riscv64-linux toolchain build and make check.
> The new testcase fails without the patch, and works with the patch.  This was
> also tested with an x86_64-linux build and make check.  There were no
> regressions.
>
> OK?

IMHO the real issue is that SHIFT_COUNT_TRUNCATED is used for
optimizing away the and early.  We then rely on the compiler not
assuming anything on the value.  Like if you'd do that on GIMPLE
VRP would come along and second-guess you because the shift
value may not be too large.  This combine issue sounds very similar.

So I'm suggesting again to instead provide patterns that
match (shft A (and B cst))

Richard.

> Jim
>
> gcc/
> PR rtl-optimization/84660
> * combine.c (force_int_to_mode) : If SHIFT_COUNT_TRUNCATED,
> call nonzero_bits and compare against xmode precision.
>
> gcc/testsuite/
> PR rtl-optimization/84660
> * gcc.dg/pr84660.c: New.
> ---
>  gcc/combine.c  | 10 --
>  gcc/testsuite/gcc.dg/pr84660.c | 17 +
>  2 files changed, 25 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/pr84660.c
>
> diff --git a/gcc/combine.c b/gcc/combine.c
> index ff672ad2adb..4ed59eb88c8 100644
> --- a/gcc/combine.c
> +++ b/gcc/combine.c
> @@ -8897,14 +8897,20 @@ force_int_to_mode (rtx x, scalar_int_mode mode, 
> scalar_int_mode xmode,
>  However, we cannot do anything with shifts where we cannot
>  guarantee that the counts are smaller than the size of the mode
>  because such a count will have a different meaning in a
> -wider mode.  */
> +different mode.  If we are narrowing the mode, the shift count must
> +be compared against mode.  If we are widening the mode, and shift
> +counts are truncated, then the shift count must be compared against
> +xmode.  */
>
>if (! (CONST_INT_P (XEXP (x, 1))
>  && INTVAL (XEXP (x, 1)) >= 0
>  && INTVAL (XEXP (x, 1)) < GET_MODE_PRECISION (mode))
>   && ! (GET_MODE (XEXP (x, 1)) != VOIDmode
> && (nonzero_bits (XEXP (x, 1), GET_MODE (XEXP (x, 1)))
> -   < (unsigned HOST_WIDE_INT) GET_MODE_PRECISION (mode
> +   < (unsigned HOST_WIDE_INT) GET_MODE_PRECISION (mode))
> +   && (! SHIFT_COUNT_TRUNCATED
> +   || (nonzero_bits (XEXP (x, 1), GET_MODE (XEXP (x, 1)))
> +   < (unsigned HOST_WIDE_INT) GET_MODE_PRECISION 
> (xmode)
> break;
>
>/* If the shift count is a constant and we can do arithmetic in
> diff --git a/gcc/testsuite/gcc.dg/pr84660.c b/gcc/testsuite/gcc.dg/pr84660.c
> new file mode 100644
> index 000..a87fa0a914d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr84660.c
> @@ -0,0 +1,17 @@
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +
> +extern void abort (void);
> +extern void exit (int);
> +
> +unsigned int __attribute__ ((noinline, noclone))
> +foo(unsigned int i) {
> +
> +  return 0x & (0xd066 << (((i & 0x1) ^ 0x2f) & 0xf));
> +}
> +
> +int main() {
> +  if (foo (1) != 0x8000)
> +abort ();
> +  exit (0);
> +}
> --
> 2.14.1


Re: poly_span_traits fixes (PR 84811)

2018-03-21 Thread Richard Biener
On Wed, Mar 21, 2018 at 9:31 AM, Richard Sandiford
 wrote:
> This patch fixes incorrect results for HOST_WIDE_INT positions
> at opposite extremes when used with HOST_WIDE_INT sizes.  It also
> fixes UB when comparing such positions with unsigned HOST_WIDE_INT
> sizes (although the results in that case were wrapv-correct).
>
> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

OK.

Richard.

> Richard
>
>
> 2018-03-21  Richard Sandiford  
>
> gcc/
> PR tree-optimization/84811
> * poly-int.h (poly_span_traits): Remove the T3 parameter and
> promote HOST_WIDE_INT T2 - T1 results to unsigned HOST_WIDE_INT.
> (maybe_in_range_p, known_in_range_p, ranges_known_overlap_p):
> (known_subrange_p): Update accordingly.  Cast each value involved
> in the size comparison, rather than casting the result of the
> subtraction.
>
> gcc/testsuite/
> PR tree-optimization/84811
> * gcc.dg/torture/pr84811.c: New test.
>
> Index: gcc/poly-int.h
> ===
> --- gcc/poly-int.h  2018-01-14 08:42:44.497155977 +
> +++ gcc/poly-int.h  2018-03-21 08:28:14.656720617 +
> @@ -2399,30 +2399,34 @@ print_dec (const poly_int_pod &val
>  poly_coeff_traits::signedness ? SIGNED : UNSIGNED);
>  }
>
> -/* Helper for correctly comparing Pos - Start with Size in cases where
> -   known_ge (Pos, Start), Pos and Start are potentially signed, and Size is
> -   potentially unsigned.  Applying the cast function to the result of
> -   Pos - Start gives the value that should be compared with the size.
> -
> -   Try to avoid doing any unnecessary arithmetic or copying.  */
> -template -typename Diff = POLY_BINARY_COEFF (Start, Pos),
> -typename Res = POLY_BINARY_COEFF (Size, Diff)>
> +/* Helper for calculating the distance between two points P1 and P2,
> +   in cases where known_le (P1, P2).  T1 and T2 are the types of the
> +   two positions, in either order.  The coefficients of P2 - P1 have
> +   type unsigned HOST_WIDE_INT if the coefficients of both T1 and T2
> +   have C++ primitive type, otherwise P2 - P1 has its usual
> +   wide-int-based type.
> +
> +   The actual subtraction should look something like this:
> +
> + typedef poly_span_traits span_traits;
> + span_traits::cast (P2) - span_traits::cast (P1)
> +
> +   Applying the cast before the subtraction avoids undefined overflow
> +   for signed T1 and T2.
> +
> +   The implementation of the cast tries to avoid unnecessary arithmetic
> +   or copying.  */
> +template +typename Res = POLY_BINARY_COEFF (POLY_BINARY_COEFF (T1, T2),
> +  unsigned HOST_WIDE_INT)>
>  struct poly_span_traits
>  {
> -  /* Assume no cast is needed.  We'll get a warning about signed vs.
> - unsigned comparisons if the assumption is wrong.  */
>template
>static const T &cast (const T &x) { return x; }
>  };
>
> -/* The only case a change in type is needed is this one, in which the
> -   subtraction would give a HOST_WIDE_INT-based result if done on poly_ints
> -   and adding a zero size would give an unsigned HOST_WIDE_INT-based
> -   result.  Since we know known_ge (Pos, Start), it is safe to treat
> -   Pos - Start as an unsigned HOST_WIDE_INT.  */
> -template
> -struct poly_span_traits
> +template
> +struct poly_span_traits
>  {
>template
>static typename if_nonpoly::type
> @@ -2451,7 +2455,8 @@ known_size_p (const T &a)
>  inline bool
>  maybe_in_range_p (const T1 &val, const T2 &pos, const T3 &size)
>  {
> -  typedef poly_span_traits span;
> +  typedef poly_span_traits start_span;
> +  typedef poly_span_traits size_span;
>if (known_lt (val, pos))
>  return false;
>if (!known_size_p (size))
> @@ -2462,7 +2467,8 @@ maybe_in_range_p (const T1 &val, const T
>  /* In this case we don't know whether VAL >= POS is true at compile
> time, so we can't prove that VAL >= POS + SIZE.  */
>  return true;
> -  return maybe_lt (span::cast (val - pos), size);
> +  return maybe_lt (start_span::cast (val) - start_span::cast (pos),
> +  size_span::cast (size));
>  }
>
>  /* Return true if range [POS, POS + SIZE) is known to include VAL.
> @@ -2473,10 +2479,12 @@ maybe_in_range_p (const T1 &val, const T
>  inline bool
>  known_in_range_p (const T1 &val, const T2 &pos, const T3 &size)
>  {
> -  typedef poly_span_traits span;
> +  typedef poly_span_traits start_span;
> +  typedef poly_span_traits size_span;
>return (known_size_p (size)
>   && known_ge (val, pos)
> - && known_lt (span::cast (val - pos), size));
> + && known_lt (start_span::cast (val) - start_span::cast (pos),
> +  size_span::cast (size)));
>  }
>
>  /* Return true if the two ranges [POS1, POS1 + SIZE1) and [POS2, POS2 + 
> SIZE2)
> @@ -2504,8 +2512,9 @@ ranges_maybe_overlap_p (const T1 &pos1,
>  ranges_known_overlap_p (con

Re: Fix ICE after sorry for big stack arguments (PR 84964)

2018-03-21 Thread Richard Biener
On Wed, Mar 21, 2018 at 9:33 AM, Richard Sandiford
 wrote:
> After the sorry we'd skip storing the argument, but that just creates an
> inconsistency later when we try to deallocate the arguments.  This used to
> "work" because pending_stack_adjust and stack_pointer_delta were int rather
> than HWI, so 1<<33 got truncated to 0.
>
> It's not easy to back out at this point because there's so much global
> state floating around.  One option I tried was to put the sorry after:
>
>   unadjusted_args_size
> = compute_argument_block_size (reg_parm_stack_space,
>&adjusted_args_size,
>fndecl, fntype,
>(pass == 0 ? 0
> : preferred_stack_boundary));
>
> and then zero the argument sizes and num_actual.  That avoids the
> ICE too, but who knows what other problems it creates.
>
> In the end it seemed easier just to stop.
>
> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
>
> Richard
>
>
> 2018-03-21  Richard Sandiford  
>
> gcc/
> PR error-recovery/84964
> * diagnostic-core.h (fatal_sorry): Declare.
> * diagnostic.c (fatal_sorry): New function.
> * calls.c (expand_call): Use it instead of sorry.
>
> gcc/testsuite/
> PR error-recovery/84964
> * g++.dg/diagnostic/pr84964.C: New test.
>
> Index: gcc/diagnostic-core.h
> ===
> --- gcc/diagnostic-core.h   2018-03-01 08:20:43.590534306 +
> +++ gcc/diagnostic-core.h   2018-03-21 08:31:34.635677798 +
> @@ -87,6 +87,8 @@ extern bool permerror (location_t, const
>  extern bool permerror (rich_location *, const char *,
>...) ATTRIBUTE_GCC_DIAG(2,3);
>  extern void sorry (const char *, ...) ATTRIBUTE_GCC_DIAG(1,2);
> +extern void fatal_sorry (const char *, ...) ATTRIBUTE_GCC_DIAG(1,2)
> +ATTRIBUTE_NORETURN;
>  extern void inform (location_t, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3);
>  extern void inform (rich_location *, const char *, ...) 
> ATTRIBUTE_GCC_DIAG(2,3);
>  extern void inform_n (location_t, unsigned HOST_WIDE_INT, const char *,
> Index: gcc/diagnostic.c
> ===
> --- gcc/diagnostic.c2018-03-01 08:20:43.589534337 +
> +++ gcc/diagnostic.c2018-03-21 08:31:34.635677798 +
> @@ -1407,6 +1407,20 @@ sorry (const char *gmsgid, ...)
>va_end (ap);
>  }
>
> +/* Same, but stop compilation immediately.  */
> +
> +void
> +fatal_sorry (const char *gmsgid, ...)
> +{
> +  va_list ap;
> +  va_start (ap, gmsgid);
> +  rich_location richloc (line_table, input_location);
> +  diagnostic_impl (&richloc, -1, gmsgid, &ap, DK_SORRY);
> +  va_end (ap);
> +
> +  exit (FATAL_EXIT_CODE);

This is somewhat not in style of the other "fatal" handlers.  I guess
you don't get "sorry: " when using DK_FATAL, but ...

> +}
> +
>  /* Return true if an error or a "sorry" has been seen.  Various
> processing is disabled after errors.  */
>  bool
> Index: gcc/calls.c
> ===
> --- gcc/calls.c 2018-03-17 08:30:20.937100705 +
> +++ gcc/calls.c 2018-03-21 08:31:34.635677798 +
> @@ -4052,13 +4052,10 @@ expand_call (tree exp, rtx target, int i
>   rtx_insn *before_arg = get_last_insn ();
>
>   /* We don't allow passing huge (> 2^30 B) arguments
> -by value.  It would cause an overflow later on.  */
> +by value.  It would cause an overflow later on.  */
>   if (constant_lower_bound (adjusted_args_size.constant)
>   >= (1 << (HOST_BITS_PER_INT - 2)))
> -   {
> - sorry ("passing too large argument on stack");
> - continue;
> -   }
> +   fatal_sorry ("passing too large argument on stack");

... why not use fatal_error here?  It doesn't seem to be something
that is just not implemented but impossible to implement?

So, OK with just using fatal_error here (please add a comment why
we're not continuing).

Richard.

>
>   if (store_one_arg (&args[i], argblock, flags,
>  adjusted_args_size.var != 0,
> Index: gcc/testsuite/g++.dg/diagnostic/pr84964.C
> ===
> --- /dev/null   2018-03-17 08:19:33.716019995 +
> +++ gcc/testsuite/g++.dg/diagnostic/pr84964.C   2018-03-21 08:31:34.635677798 
> +
> @@ -0,0 +1,6 @@
> +/* { dg-do compile { target lp64 } } */
> +
> +struct a {
> +  short b : 1ULL << 33; /* { dg-warning "width of 'a::b' exceeds its type" } 
> */
> +};
> +void c(...) { c(a()); } /* { dg-message "sorry, unimplemented: passing too 
> large argument on stack" } */


[PATCH] Fix up handling of bool BIT_NOT_EXPRs in store-merging (PR tree-optimization/84982, take 2)

2018-03-21 Thread Jakub Jelinek
On Wed, Mar 21, 2018 at 09:20:40AM +0100, Richard Biener wrote:
> > + tree lhs = gimple_assign_lhs (info->stmt);
> > + if (TREE_CODE (TREE_TYPE (lhs)) == BOOLEAN_TYPE && info->bitsize > 1)
> 
> So I'm slightly uncomfortable with keying this just on BOOLEAN_TYPE.
> Do you think anything would go wrong with simply using
> 
>  if (INTEGRAL_TYPE_P (TREE_TYPE (lhs))
>  && TYPE_PRECISION (TREE_TYPE (lhs)) < info->bitsize)
> 
> ?
> 
> It would then be any_padding rather than any_bool.

So like this?  The assembly for the testcase is still identical to previous
patch on both x86_64-linux and powerpc-linux.
I've tried to reproduce the case with non-bool integral types with precision
smaller than size, but even with C++ -fstrict-enums haven't succeeded, those
enums have different TYPE_*_VALUE, but TYPE_PRECISION is still equal to the
TYPE_SIZE, in the end I've just changed in gdb TYPE_PRECISION of the
enumerated type and checked that there is (without store merging) xor with
the mask of only precision bits emitted.  Perhaps in Ada one can construct
something?  I don't speak Ada though...

2018-03-21  Jakub Jelinek  

PR tree-optimization/84982
* gimple-ssa-store-merging.c (invert_op): Handle boolean inversion
by flipping the least significant bit rather than all bits from
bitpos to bitpos + bitsize - 1.

* c-c++-common/pr84982.c: New test.

--- gcc/gimple-ssa-store-merging.c.jj   2018-03-20 22:05:54.368430762 +0100
+++ gcc/gimple-ssa-store-merging.c  2018-03-21 10:45:39.919458647 +0100
@@ -3248,16 +3248,23 @@ invert_op (split_store *split_store, int
   unsigned int i;
   store_immediate_info *info;
   unsigned int cnt = 0;
+  bool any_paddings = false;
   FOR_EACH_VEC_ELT (split_store->orig_stores, i, info)
 {
   bool bit_not_p = idx < 2 ? info->ops[idx].bit_not_p : info->bit_not_p;
   if (bit_not_p)
-   ++cnt;
+   {
+ ++cnt;
+ tree lhs = gimple_assign_lhs (info->stmt);
+ if (INTEGRAL_TYPE_P (TREE_TYPE (lhs))
+ && TYPE_PRECISION (TREE_TYPE (lhs)) < info->bitsize)
+   any_paddings = true;
+   }
 }
   mask = NULL_TREE;
   if (cnt == 0)
 return NOP_EXPR;
-  if (cnt == split_store->orig_stores.length ())
+  if (cnt == split_store->orig_stores.length () && !any_paddings)
 return BIT_NOT_EXPR;
 
   unsigned HOST_WIDE_INT try_bitpos = split_store->bytepos * BITS_PER_UNIT;
@@ -3274,14 +3281,42 @@ invert_op (split_store *split_store, int
 clear regions with !bit_not_p, so that gaps in between stores aren't
 set in the mask.  */
   unsigned HOST_WIDE_INT bitsize = info->bitsize;
+  unsigned HOST_WIDE_INT prec = bitsize;
   unsigned int pos_in_buffer = 0;
+  if (any_paddings)
+   {
+ tree lhs = gimple_assign_lhs (info->stmt);
+ if (INTEGRAL_TYPE_P (TREE_TYPE (lhs))
+ && TYPE_PRECISION (TREE_TYPE (lhs)) < bitsize)
+   prec = TYPE_PRECISION (TREE_TYPE (lhs));
+   }
   if (info->bitpos < try_bitpos)
{
  gcc_assert (info->bitpos + bitsize > try_bitpos);
- bitsize -= (try_bitpos - info->bitpos);
+ if (!BYTES_BIG_ENDIAN)
+   {
+ if (prec <= try_bitpos - info->bitpos)
+   continue;
+ prec -= try_bitpos - info->bitpos;
+   }
+ bitsize -= try_bitpos - info->bitpos;
+ if (BYTES_BIG_ENDIAN && prec > bitsize)
+   prec = bitsize;
}
   else
pos_in_buffer = info->bitpos - try_bitpos;
+  if (prec < bitsize)
+   {
+ /* If this is a bool inversion, invert just the least significant
+prec bits rather than all bits of it.  */
+ if (BYTES_BIG_ENDIAN)
+   {
+ pos_in_buffer += bitsize - prec;
+ if (pos_in_buffer >= split_store->size)
+   continue;
+   }
+ bitsize = prec;
+   }
   if (pos_in_buffer + bitsize > split_store->size)
bitsize = split_store->size - pos_in_buffer;
   unsigned char *p = buf + (pos_in_buffer / BITS_PER_UNIT);
--- gcc/testsuite/c-c++-common/pr84982.c.jj 2018-03-20 14:49:00.259744750 
+0100
+++ gcc/testsuite/c-c++-common/pr84982.c2018-03-20 12:27:34.111363552 
+0100
@@ -0,0 +1,38 @@
+/* PR tree-optimization/84982 */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+#ifndef __cplusplus
+#define bool _Bool
+#define true 1
+#define false 0
+#endif
+
+struct S { bool a, b, c, d; };
+
+__attribute__((noipa)) void
+bar (bool *x)
+{
+  if (x[0] || !x[1] || !x[2] || x[3])
+__builtin_abort ();
+}
+
+__attribute__((noipa)) void
+foo (struct S *x)
+{
+  bool a[4];
+  a[0] = !x->a;
+  a[1] = !x->b;
+  a[2] = x->c;
+  a[3] = !x->d;
+  bar (a);
+} 
+
+int
+main ()
+{
+  struct S s;
+  s.a = true; s.b = false; s.c = true; s.d = true;
+  foo (&s);
+  return 0;
+}


Jakub


Re: [PATCH] Fix up handling of bool BIT_NOT_EXPRs in store-merging (PR tree-optimization/84982, take 2)

2018-03-21 Thread Richard Biener
On Wed, 21 Mar 2018, Jakub Jelinek wrote:

> On Wed, Mar 21, 2018 at 09:20:40AM +0100, Richard Biener wrote:
> > > +   tree lhs = gimple_assign_lhs (info->stmt);
> > > +   if (TREE_CODE (TREE_TYPE (lhs)) == BOOLEAN_TYPE && info->bitsize > 1)
> > 
> > So I'm slightly uncomfortable with keying this just on BOOLEAN_TYPE.
> > Do you think anything would go wrong with simply using
> > 
> >  if (INTEGRAL_TYPE_P (TREE_TYPE (lhs))
> >  && TYPE_PRECISION (TREE_TYPE (lhs)) < info->bitsize)
> > 
> > ?
> > 
> > It would then be any_padding rather than any_bool.
> 
> So like this?  The assembly for the testcase is still identical to previous
> patch on both x86_64-linux and powerpc-linux.

Yes.

> I've tried to reproduce the case with non-bool integral types with precision
> smaller than size, but even with C++ -fstrict-enums haven't succeeded, those
> enums have different TYPE_*_VALUE, but TYPE_PRECISION is still equal to the
> TYPE_SIZE, in the end I've just changed in gdb TYPE_PRECISION of the
> enumerated type and checked that there is (without store merging) xor with
> the mask of only precision bits emitted.  Perhaps in Ada one can construct
> something?  I don't speak Ada though...

Yeah, I'm just fearing there are no rules prohibiting such types ;)
Like for C++ and -fstrict-enums we have constrained TYPE_MIN/MAX_VALUE
but not TYPE_PRECISION (for whatever reason ...).

Patch is ok.

Richard.

> 2018-03-21  Jakub Jelinek  
> 
>   PR tree-optimization/84982
>   * gimple-ssa-store-merging.c (invert_op): Handle boolean inversion
>   by flipping the least significant bit rather than all bits from
>   bitpos to bitpos + bitsize - 1.
> 
>   * c-c++-common/pr84982.c: New test.
> 
> --- gcc/gimple-ssa-store-merging.c.jj 2018-03-20 22:05:54.368430762 +0100
> +++ gcc/gimple-ssa-store-merging.c2018-03-21 10:45:39.919458647 +0100
> @@ -3248,16 +3248,23 @@ invert_op (split_store *split_store, int
>unsigned int i;
>store_immediate_info *info;
>unsigned int cnt = 0;
> +  bool any_paddings = false;
>FOR_EACH_VEC_ELT (split_store->orig_stores, i, info)
>  {
>bool bit_not_p = idx < 2 ? info->ops[idx].bit_not_p : info->bit_not_p;
>if (bit_not_p)
> - ++cnt;
> + {
> +   ++cnt;
> +   tree lhs = gimple_assign_lhs (info->stmt);
> +   if (INTEGRAL_TYPE_P (TREE_TYPE (lhs))
> +   && TYPE_PRECISION (TREE_TYPE (lhs)) < info->bitsize)
> + any_paddings = true;
> + }
>  }
>mask = NULL_TREE;
>if (cnt == 0)
>  return NOP_EXPR;
> -  if (cnt == split_store->orig_stores.length ())
> +  if (cnt == split_store->orig_stores.length () && !any_paddings)
>  return BIT_NOT_EXPR;
>  
>unsigned HOST_WIDE_INT try_bitpos = split_store->bytepos * BITS_PER_UNIT;
> @@ -3274,14 +3281,42 @@ invert_op (split_store *split_store, int
>clear regions with !bit_not_p, so that gaps in between stores aren't
>set in the mask.  */
>unsigned HOST_WIDE_INT bitsize = info->bitsize;
> +  unsigned HOST_WIDE_INT prec = bitsize;
>unsigned int pos_in_buffer = 0;
> +  if (any_paddings)
> + {
> +   tree lhs = gimple_assign_lhs (info->stmt);
> +   if (INTEGRAL_TYPE_P (TREE_TYPE (lhs))
> +   && TYPE_PRECISION (TREE_TYPE (lhs)) < bitsize)
> + prec = TYPE_PRECISION (TREE_TYPE (lhs));
> + }
>if (info->bitpos < try_bitpos)
>   {
> gcc_assert (info->bitpos + bitsize > try_bitpos);
> -   bitsize -= (try_bitpos - info->bitpos);
> +   if (!BYTES_BIG_ENDIAN)
> + {
> +   if (prec <= try_bitpos - info->bitpos)
> + continue;
> +   prec -= try_bitpos - info->bitpos;
> + }
> +   bitsize -= try_bitpos - info->bitpos;
> +   if (BYTES_BIG_ENDIAN && prec > bitsize)
> + prec = bitsize;
>   }
>else
>   pos_in_buffer = info->bitpos - try_bitpos;
> +  if (prec < bitsize)
> + {
> +   /* If this is a bool inversion, invert just the least significant
> +  prec bits rather than all bits of it.  */
> +   if (BYTES_BIG_ENDIAN)
> + {
> +   pos_in_buffer += bitsize - prec;
> +   if (pos_in_buffer >= split_store->size)
> + continue;
> + }
> +   bitsize = prec;
> + }
>if (pos_in_buffer + bitsize > split_store->size)
>   bitsize = split_store->size - pos_in_buffer;
>unsigned char *p = buf + (pos_in_buffer / BITS_PER_UNIT);
> --- gcc/testsuite/c-c++-common/pr84982.c.jj   2018-03-20 14:49:00.259744750 
> +0100
> +++ gcc/testsuite/c-c++-common/pr84982.c  2018-03-20 12:27:34.111363552 
> +0100
> @@ -0,0 +1,38 @@
> +/* PR tree-optimization/84982 */
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +
> +#ifndef __cplusplus
> +#define bool _Bool
> +#define true 1
> +#define false 0
> +#endif
> +
> +struct S { bool a, b, c, d; };
> +
> +__attribute__((noipa)) void
> +bar (bool *x)
> +{
> +  if (x[0] || !x[1] || !x[2] || x[3])

Re: [C++ PATCH] Fix ICE with inline asm and MODIFY_EXPR/preinc/predec in output operand (PR c++/84961)

2018-03-21 Thread Jakub Jelinek
On Tue, Mar 20, 2018 at 05:58:43PM -0400, Jason Merrill wrote:
> On Tue, Mar 20, 2018 at 5:04 PM, Jakub Jelinek  wrote:
> > --- gcc/cp/semantics.c.jj   2018-03-20 11:58:17.069356145 +0100
> > +++ gcc/cp/semantics.c  2018-03-20 21:56:43.745292245 +0100
> > @@ -1512,6 +1512,26 @@ finish_asm_stmt (int volatile_p, tree st
> >   && C_TYPE_FIELDS_READONLY (TREE_TYPE (operand)
> > cxx_readonly_error (operand, lv_asm);
> >
> > + tree *op = &operand;
> > + while (TREE_CODE (*op) == COMPOUND_EXPR)
> > +   op = &TREE_OPERAND (*op, 1);
> > + switch (TREE_CODE (*op))
> > +   {
> > +   case PREINCREMENT_EXPR:
> > +   case PREDECREMENT_EXPR:
> > +   case MODIFY_EXPR:
> > + if (TREE_SIDE_EFFECTS (TREE_OPERAND (*op, 0)))
> > +   *op = build2 (TREE_CODE (*op), TREE_TYPE (*op),
> > + cp_stabilize_reference (TREE_OPERAND (*op, 
> > 0)),
> > + TREE_OPERAND (*op, 1));
> > + *op = build2 (COMPOUND_EXPR, TREE_TYPE (*op), *op,
> > +   TREE_OPERAND (*op, 0));
> > + op = &TREE_OPERAND (*op, 1);
> > + break;
> > +   default:
> > + break;
> > +   }
> 
> Hmm, it would be nice to share this with the similar patterns in
> unary_complex_lvalue and cp_build_modify_expr.

You mean just outline the
  if (TREE_SIDE_EFFECTS (TREE_OPERAND (lhs, 0)))
lhs = build2 (TREE_CODE (lhs), TREE_TYPE (lhs),
  cp_stabilize_reference (TREE_OPERAND (lhs, 0)),
  TREE_OPERAND (lhs, 1));
  lhs = build2 (COMPOUND_EXPR, lhstype, lhs, TREE_OPERAND (lhs, 0));
into lhs = some_function (lhs); and use that in finish_asm_stmt,
unary_complex_lvalue and cp_build_modify_expr in these spots?
I really have no idea how to commonize anything else, e.g. the COMPOUND_EXPR
handling is substantially different between the 3 functions.

Any suggestion for the some_function name if you want that?

> Does COND_EXPR work without adjustment?

It seems to be:
int a, b;

void
foo (bool x)
{
  asm volatile ("" : "=r" (x ? a : b));
}

void
bar (bool x)
{
  asm volatile ("" : : "m" (x ? a : b));
}

I guess it should be added as another testcase.

Jakub


Re: [PATCH] Prefer mempcpy to memcpy on x86_64 target (PR middle-end/81657).

2018-03-21 Thread Jakub Jelinek
On Wed, Mar 14, 2018 at 02:54:00PM +0100, Martin Liška wrote:
> > The variable is not named very well, shouldn't it be avoid_libcall or
> > something similar?  Perhaps the variable should have a comment describing
> > what it is.  Do you need separate argument for that bool and
> > is_move_done, rather than just the flag being that some pointer to bool is
> > non-NULL?
> 
> Can you please explain me how to replace the 2 new arguments?

So you have one bool arg and one pointer arg into which the function
stores true and optionally based on that other bool arg and other conditions
stores false.
I'm suggesting just one bool * argument, which is NULL if the bool arg would
be false, and non-NULL otherwise.  The single argument still could be
called bool *avoid_libcall, and you'd just
if (avoid_libcall) { *avoid_libcall = true; return retval; }
instead of emitting a libcall, the caller would initialize the bool
variable to false.

Jakub


Re: [parloops, PR83126], Use cached affine_ivs canonicalize_loop_ivs

2018-03-21 Thread Tom de Vries

On 03/12/2018 01:14 PM, Richard Biener wrote:

On Thu, 22 Feb 2018, Tom de Vries wrote:

I can
rework the bit of the patch that adds an assert after canonicalize_loop_ivs
into a patch that aborts the optimization instead of ICE-ing.


I think that's something reasonable anyway.



Patch attached below implements that approach.

The difference between before parloops, and after ompexpssa2 is this 
(showing the effects of canonicalize_loop_ivs):

...
$ diff -u pr83126.c.156t.dce5 pr83126.c.158t.ompexpssa2
   ...
 ew (short unsigned int c9, int stuff)
 {
   int * fd;
@@ -9,53 +103,53 @@
   int _3;
   short unsigned int _4;
   unsigned int _5;
+  unsigned int ivtmp_8;
   unsigned int _22;
   unsigned int ivtmp_24;
   unsigned int ivtmp_26;
+  unsigned int ivtmp_28;

[local count: 11811]:

[local count: 118111601]:
-  # c9_1 = PHI 
+  # c9_1 = PHI 
   _2 = (long int) c9_1;
   fd_13 = (int *) _2;
   _3 = *fd_13;

[local count: 1073741825]:
   if (_3 != 0)
-goto ; [11.00%]
+goto ; [11.00%]
   else
 goto ; [89.00%]

[local count: 955630225]:
   goto ; [100.00%]

-   [local count: 118111600]:
+   [local count: 94489281]:

-   [local count: 118111600]:
-
-   [local count: 236258638]:
-  # _22 = PHI <0(10), _5(8)>
-  # c9_23 = PHI 
-  # e1_27 = PHI <0(10), e1_17(8)>
-  # ivtmp_26 = PHI <2(10), ivtmp_24(8)>
+   [local count: 189006911]:
+  # c9_23 = PHI 
+  # e1_27 = PHI 
+  # ivtmp_8 = PHI 
+  _22 = ivtmp_8;
+  ivtmp_26 = 2 - ivtmp_8;
   _4 = (short unsigned int) e1_27;
   c9_14 = _4 * c9_23;
   _5 = _22 + 1;
   e1_17 = (int) _5;
   ivtmp_24 = ivtmp_26 - 1;
-  if (ivtmp_24 != 0)
+  if (ivtmp_8 < 1)
 goto ; [66.67%]
   else
 goto ; [33.33%]

-   [local count: 157513634]:
+   [local count: 126010908]:
+  ivtmp_28 = ivtmp_8 + 1;
   goto ; [100.00%]

[local count: 78745004]:
   # c9_21 = PHI 
-
-   [local count: 78745004]:
   goto ; [100.00%]

 }
...


Bootstrapped and reg-tested on x86_64.

OK for stage4 trunk?

Thanks,
- Tom
[parloops] Handle canonicalize_loop_ivs failure

2018-03-19  Tom de Vries  

	PR tree-optimization/83126
	* tree-parloops.c (num_phis): New function.
	(gen_parallel_loop): Detect and handle canonicalize_loop_ivs failure.

	* gcc.dg/graphite/pr83126.c: New test.

---
 gcc/testsuite/gcc.dg/graphite/pr83126.c | 19 
 gcc/tree-parloops.c | 39 +
 2 files changed, 58 insertions(+)

diff --git a/gcc/testsuite/gcc.dg/graphite/pr83126.c b/gcc/testsuite/gcc.dg/graphite/pr83126.c
new file mode 100644
index 000..663d059
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/graphite/pr83126.c
@@ -0,0 +1,19 @@
+/* { dg-additional-options "-w -ftree-parallelize-loops=2 -floop-parallelize-all -O1" }  */
+
+void
+ew (unsigned short int c9, int stuff)
+{
+  int e1;
+
+  for (;;)
+{
+  unsigned int *by = &e1;
+  int *fd = &stuff;
+
+  *fd = c9;
+  fd = *fd;
+  if (*fd != 0)
+	for (*by = 0; *by < 2; ++*by)
+	  c9 *= e1;
+}
+}
diff --git a/gcc/tree-parloops.c b/gcc/tree-parloops.c
index e44ad5e..3a788cc 100644
--- a/gcc/tree-parloops.c
+++ b/gcc/tree-parloops.c
@@ -2235,6 +2235,25 @@ create_parallel_loop (struct loop *loop, tree loop_fn, tree data,
   calculate_dominance_info (CDI_DOMINATORS);
 }
 
+/* Return number of phis in bb.  If COUNT_VIRTUAL_P is false, don't count the
+   virtual phi.  */
+
+static unsigned int
+num_phis (basic_block bb, bool count_virtual_p)
+{
+  unsigned int nr_phis = 0;
+  gphi_iterator gsi;
+  for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+{
+  if (!count_virtual_p && virtual_operand_p (PHI_RESULT (gsi.phi (
+	continue;
+
+  nr_phis++;
+}
+
+  return nr_phis;
+}
+
 /* Generates code to execute the iterations of LOOP in N_THREADS
threads in parallel, which can be 0 if that number is to be determined
later.
@@ -2370,6 +2389,26 @@ gen_parallel_loop (struct loop *loop,
 
   /* Base all the induction variables in LOOP on a single control one.  */
   canonicalize_loop_ivs (loop, &nit, true);
+  if (num_phis (loop->header, false) != reduction_list->elements () + 1)
+{
+  /* The call to canonicalize_loop_ivs above failed to "base all the
+	 induction variables in LOOP on a single control one".  Do damage
+	 control.  */
+  basic_block preheader = loop_preheader_edge (loop)->src;
+  basic_block cond_bb = single_pred (preheader);
+  gcond *cond = as_a  (gsi_stmt (gsi_last_bb (cond_bb)));
+  gimple_cond_make_true (cond);
+  update_stmt (cond);
+  /* We've gotten rid of the duplicate loop created by loop_version, but
+	 we can't undo whatever canonicalize_loop_ivs has done.
+	 TODO: Fix this properly by ensuring that the call to
+	 canonicalize_loop_ivs succeeds.  */
+  if (dump_file
+	  && (dump_flags & TDF_DETAILS))
+	fprintf (dump_file, "canonicalize_loop_ivs failed for loop %d,"
+		 " aborting transformation\n", loop->num);
+  return;
+}
 
   /* Ensure that the exit condition is th

Re: Use SCEV information when aligning for vectorisation (PR 84005)

2018-03-21 Thread Richard Biener
On Tue, Mar 20, 2018 at 4:26 PM, Richard Sandiford
 wrote:
> Richard Biener  writes:
>> On Mon, Mar 19, 2018 at 10:27 PM, Richard Sandiford
>>  wrote:
>>> Richard Biener  writes:
 On Sat, Mar 17, 2018 at 11:45 AM, Richard Sandiford
  wrote:
> Index: gcc/tree-data-ref.c
> ===
> --- gcc/tree-data-ref.c 2018-02-14 13:14:36.268006810 +
> +++ gcc/tree-data-ref.c 2018-03-17 10:43:42.544669549 +
> @@ -5200,6 +5200,75 @@ dr_alignment (innermost_loop_behavior *d
>return alignment;
>  }
>
> +/* If BASE is a pointer-typed SSA name, try to find the object that it
> +   is based on.  Return this object X on success and store the alignment
> +   in bytes of BASE - &X in *ALIGNMENT_OUT.  */
> +
> +static tree
> +get_base_for_alignment_1 (tree base, unsigned int *alignment_out)
> +{
> +  if (TREE_CODE (base) != SSA_NAME || !POINTER_TYPE_P (TREE_TYPE (base)))
> +return NULL_TREE;
> +
> +  gimple *def = SSA_NAME_DEF_STMT (base);
> +  base = analyze_scalar_evolution (loop_containing_stmt (def), base);

 I think it is an error to use the def stmt of base here.  Instead you
 need to pass down the point/loop of the use.  For the same reason you
 also want to instantiate parameters after analyzing the evolution.

 In the end, if the BB to be vectorized is contained in a loop nest we want 
 to
 instantiate up to the point where (eventually) a DECL we can re-align
 appears,
 but using the containing loop for now looks OK.
>>>
>>> Why's that necessary/better though?  We're not interested in the
>>> evolution of the value at the point it*s used by the potential vector
>>> code, but in how we get from the ultimate base (if there is one) to the
>>> definition of the SSA name.
>>
>> Hmm, ok.
>>
>>> I don't think instantiating the SCEV would give any new information,
>>> but it could lose some.  E.g. if we have:
>>>
>>>   x = &foo;
>>>   do
>>> x += 8;
>>>   while (*y++ < 10)
>>>   ...potential vector use of *x...
>>>
>>> we wouldn't be able to express the address of x after the do-while
>>> loop, because there's nothing that counts the number of iterations.
>>> But the uninstantiated SCEV could tell us that x = &foo + N * 8 for
>>> some (unknown) N.
>>
>> Not sure that it works that way.  I'm not 100% sure what kind of
>> parameters are left symbolic, and if analysis loop and instantiation
>> "loop" are the same I guess it doesn't make any difference...
>>
>>> (OK, so that doesn't actually work unless we take the effort
>>> to look through loop-closed SSA form, but still...)
>>>
> +  /* Peel chrecs and record the minimum alignment preserved by
> + all steps.  */
> +  unsigned int alignment = MAX_OFILE_ALIGNMENT / BITS_PER_UNIT;
> +  while (TREE_CODE (base) == POLYNOMIAL_CHREC)
> +{
> + unsigned int step_alignment = highest_pow2_factor (CHREC_RIGHT
> (base));
> +  alignment = MIN (alignment, step_alignment);
> +  base = CHREC_LEFT (base);
> +}
> +
> +  /* Punt if the expression is too complicated to handle.  */
> + if (tree_contains_chrecs (base, NULL) || !POINTER_TYPE_P (TREE_TYPE
> (base)))
> +return NULL_TREE;
> +
> +  /* Analyze the base to which the steps we peeled were applied.  */
> +  poly_int64 bitsize, bitpos, bytepos;
> +  machine_mode mode;
> +  int unsignedp, reversep, volatilep;
> +  tree offset;
> +  base = get_inner_reference (build_fold_indirect_ref (base),

 ick :/

 what happens if you simply use get_pointer_alignment here and
 strip down POINTER_PLUS_EXPRs to the ultimate LHS?  (basically
 what get_pointer_alignment_1 does)  After all get_base_for_alignment_1
 itself only deals with plain SSA_NAMEs, not, say, &a_1->b.c or &MEM[a_1 + 
 4].
>>>
>>> Yeah, but those have (hopefully) been handled by dr_analyze_innermost
>>> already, which should have stripped off both the constant and variable
>>> parts of the offset.  So I think SSA names are the only interesting
>>> input.  The problem is that once we follow the definitions of an SSA
>>> name through CHREC_LEFTs, we get a general address again.
>>>
>>> get_pointer_alignment and get_pointer_alignment_1 don't do what we want
>>> because they take the alignment of the existing object into account,
>>> whereas here we explicitly want to ignore that and only calculate the
>>> alignment of the offset.
>>
>> Ah, indeed - I missed that fact.
>>
>>> I guess we could pass a flag down to ignore the current alignment though?
>>
>> But we need the base anyway...  so, given we can only ever re-align decls
>> and never plain pointers instead of using build_fold_indirect_ref do
>>
>>  if (TREE_CODE (base) != ADDR_EXPR)
>>return NULL_TREE;
>>
>> else use TREE_OPERAND (base, 0)?
>
> The build_fold_indirect_ref also helps for POINTER_P

Re: [parloops, PR83126], Use cached affine_ivs canonicalize_loop_ivs

2018-03-21 Thread Richard Biener
On Wed, 21 Mar 2018, Tom de Vries wrote:

> On 03/12/2018 01:14 PM, Richard Biener wrote:
> > On Thu, 22 Feb 2018, Tom de Vries wrote:
> > > I can
> > > rework the bit of the patch that adds an assert after
> > > canonicalize_loop_ivs
> > > into a patch that aborts the optimization instead of ICE-ing.
> > 
> > I think that's something reasonable anyway.
> > 
> 
> Patch attached below implements that approach.
> 
> The difference between before parloops, and after ompexpssa2 is this (showing
> the effects of canonicalize_loop_ivs):
> ...
> $ diff -u pr83126.c.156t.dce5 pr83126.c.158t.ompexpssa2
>...
>  ew (short unsigned int c9, int stuff)
>  {
>int * fd;
> @@ -9,53 +103,53 @@
>int _3;
>short unsigned int _4;
>unsigned int _5;
> +  unsigned int ivtmp_8;
>unsigned int _22;
>unsigned int ivtmp_24;
>unsigned int ivtmp_26;
> +  unsigned int ivtmp_28;
> 
> [local count: 11811]:
> 
> [local count: 118111601]:
> -  # c9_1 = PHI 
> +  # c9_1 = PHI 
>_2 = (long int) c9_1;
>fd_13 = (int *) _2;
>_3 = *fd_13;
> 
> [local count: 1073741825]:
>if (_3 != 0)
> -goto ; [11.00%]
> +goto ; [11.00%]
>else
>  goto ; [89.00%]
> 
> [local count: 955630225]:
>goto ; [100.00%]
> 
> -   [local count: 118111600]:
> +   [local count: 94489281]:
> 
> -   [local count: 118111600]:
> -
> -   [local count: 236258638]:
> -  # _22 = PHI <0(10), _5(8)>
> -  # c9_23 = PHI 
> -  # e1_27 = PHI <0(10), e1_17(8)>
> -  # ivtmp_26 = PHI <2(10), ivtmp_24(8)>
> +   [local count: 189006911]:
> +  # c9_23 = PHI 
> +  # e1_27 = PHI 
> +  # ivtmp_8 = PHI 
> +  _22 = ivtmp_8;
> +  ivtmp_26 = 2 - ivtmp_8;
>_4 = (short unsigned int) e1_27;
>c9_14 = _4 * c9_23;
>_5 = _22 + 1;
>e1_17 = (int) _5;
>ivtmp_24 = ivtmp_26 - 1;
> -  if (ivtmp_24 != 0)
> +  if (ivtmp_8 < 1)
>  goto ; [66.67%]
>else
>  goto ; [33.33%]
> 
> -   [local count: 157513634]:
> +   [local count: 126010908]:
> +  ivtmp_28 = ivtmp_8 + 1;
>goto ; [100.00%]
> 
> [local count: 78745004]:
># c9_21 = PHI 
> -
> -   [local count: 78745004]:
>goto ; [100.00%]
> 
>  }
> ...
> 
> 
> Bootstrapped and reg-tested on x86_64.
> 
> OK for stage4 trunk?

OK.

Thanks,
Richard.


Re: Fix ICE after sorry for big stack arguments (PR 84964)

2018-03-21 Thread Richard Sandiford
Richard Biener  writes:
> On Wed, Mar 21, 2018 at 9:33 AM, Richard Sandiford
>  wrote:
>> Index: gcc/diagnostic.c
>> ===
>> --- gcc/diagnostic.c2018-03-01 08:20:43.589534337 +
>> +++ gcc/diagnostic.c2018-03-21 08:31:34.635677798 +
>> @@ -1407,6 +1407,20 @@ sorry (const char *gmsgid, ...)
>>va_end (ap);
>>  }
>>
>> +/* Same, but stop compilation immediately.  */
>> +
>> +void
>> +fatal_sorry (const char *gmsgid, ...)
>> +{
>> +  va_list ap;
>> +  va_start (ap, gmsgid);
>> +  rich_location richloc (line_table, input_location);
>> +  diagnostic_impl (&richloc, -1, gmsgid, &ap, DK_SORRY);
>> +  va_end (ap);
>> +
>> +  exit (FATAL_EXIT_CODE);
>
> This is somewhat not in style of the other "fatal" handlers.  I guess
> you don't get "sorry: " when using DK_FATAL, but ...

I didn't add another DK_* because I don't think we want a different prefix
(unliked "error:" vs. "fatal error:").

>> +}
>> +
>>  /* Return true if an error or a "sorry" has been seen.  Various
>> processing is disabled after errors.  */
>>  bool
>> Index: gcc/calls.c
>> ===
>> --- gcc/calls.c 2018-03-17 08:30:20.937100705 +
>> +++ gcc/calls.c 2018-03-21 08:31:34.635677798 +
>> @@ -4052,13 +4052,10 @@ expand_call (tree exp, rtx target, int i
>>   rtx_insn *before_arg = get_last_insn ();
>>
>>   /* We don't allow passing huge (> 2^30 B) arguments
>> -by value.  It would cause an overflow later on.  */
>> +by value.  It would cause an overflow later on.  */
>>   if (constant_lower_bound (adjusted_args_size.constant)
>>   >= (1 << (HOST_BITS_PER_INT - 2)))
>> -   {
>> - sorry ("passing too large argument on stack");
>> - continue;
>> -   }
>> +   fatal_sorry ("passing too large argument on stack");
>
> ... why not use fatal_error here?  It doesn't seem to be something
> that is just not implemented but impossible to implement?

It's HOST_BITS_PER_INT rather than _WIDE_INT, so I think we could
implement it if we were tighter with types.  E.g. this PR occurred
because we went from int to HWI for some things, which should have
fixed part of the problem.  There might still be other uses of
"int" that need to change though.

Thanks,
Richard

> So, OK with just using fatal_error here (please add a comment why
> we're not continuing).


Re: Fix ICE after sorry for big stack arguments (PR 84964)

2018-03-21 Thread Richard Biener
On Wed, Mar 21, 2018 at 11:48 AM, Richard Sandiford
 wrote:
> Richard Biener  writes:
>> On Wed, Mar 21, 2018 at 9:33 AM, Richard Sandiford
>>  wrote:
>>> Index: gcc/diagnostic.c
>>> ===
>>> --- gcc/diagnostic.c2018-03-01 08:20:43.589534337 +
>>> +++ gcc/diagnostic.c2018-03-21 08:31:34.635677798 +
>>> @@ -1407,6 +1407,20 @@ sorry (const char *gmsgid, ...)
>>>va_end (ap);
>>>  }
>>>
>>> +/* Same, but stop compilation immediately.  */
>>> +
>>> +void
>>> +fatal_sorry (const char *gmsgid, ...)
>>> +{
>>> +  va_list ap;
>>> +  va_start (ap, gmsgid);
>>> +  rich_location richloc (line_table, input_location);
>>> +  diagnostic_impl (&richloc, -1, gmsgid, &ap, DK_SORRY);
>>> +  va_end (ap);
>>> +
>>> +  exit (FATAL_EXIT_CODE);
>>
>> This is somewhat not in style of the other "fatal" handlers.  I guess
>> you don't get "sorry: " when using DK_FATAL, but ...
>
> I didn't add another DK_* because I don't think we want a different prefix
> (unliked "error:" vs. "fatal error:").
>
>>> +}
>>> +
>>>  /* Return true if an error or a "sorry" has been seen.  Various
>>> processing is disabled after errors.  */
>>>  bool
>>> Index: gcc/calls.c
>>> ===
>>> --- gcc/calls.c 2018-03-17 08:30:20.937100705 +
>>> +++ gcc/calls.c 2018-03-21 08:31:34.635677798 +
>>> @@ -4052,13 +4052,10 @@ expand_call (tree exp, rtx target, int i
>>>   rtx_insn *before_arg = get_last_insn ();
>>>
>>>   /* We don't allow passing huge (> 2^30 B) arguments
>>> -by value.  It would cause an overflow later on.  */
>>> +by value.  It would cause an overflow later on.  */
>>>   if (constant_lower_bound (adjusted_args_size.constant)
>>>   >= (1 << (HOST_BITS_PER_INT - 2)))
>>> -   {
>>> - sorry ("passing too large argument on stack");
>>> - continue;
>>> -   }
>>> +   fatal_sorry ("passing too large argument on stack");
>>
>> ... why not use fatal_error here?  It doesn't seem to be something
>> that is just not implemented but impossible to implement?
>
> It's HOST_BITS_PER_INT rather than _WIDE_INT, so I think we could
> implement it if we were tighter with types.  E.g. this PR occurred
> because we went from int to HWI for some things, which should have
> fixed part of the problem.  There might still be other uses of
> "int" that need to change though.

Ah, missed the BITS part.  I wonder if we can get away with simply
setting adjusted_args_size.constant to some small constant?  For
bits we generally need offset_ints, just HWI isn't enough anyway.

Richard.

> Thanks,
> Richard
>
>> So, OK with just using fatal_error here (please add a comment why
>> we're not continuing).


[PR c++/84836] ICE with local scopes

2018-03-21 Thread Nathan Sidwell
I'd flubbed the condition on when a local binding was updated, leading 
to an ICE when popping them.  For a local overload set we need to look 
in the binding list and find the one to update.  We didn't do that when 
we'd already hidden a TYPE_DECL of the same name.


Fixed thusly.

nathan
--
Nathan Sidwell
2018-03-21  Nathan Sidwell  

	PR c++/84836
	* name-lookup.c (update_binding): Correct logic for local binding
	update.

	PR c++/84836
	* g++.dg/lookup/pr84836.C: New.

Index: gcc/cp/name-lookup.c
===
--- gcc/cp/name-lookup.c	(revision 258710)
+++ gcc/cp/name-lookup.c	(working copy)
@@ -2481,21 +2481,12 @@ update_binding (cp_binding_level *level,
  done:
   if (to_val)
 {
-  if (level->kind != sk_namespace
-	  && !to_type && binding->value && OVL_P (to_val))
-	update_local_overload (binding, to_val);
+  if (level->kind == sk_namespace || to_type == decl || to_val == decl)
+	add_decl_to_level (level, decl);
   else
 	{
-	  tree to_add = to_val;
-  
-	  if (level->kind == sk_namespace)
-	to_add = decl;
-	  else if (to_type == decl)
-	to_add = decl;
-	  else if (TREE_CODE (to_add) == OVERLOAD)
-	to_add = build_tree_list (NULL_TREE, to_add);
-
-	  add_decl_to_level (level, to_add);
+	  gcc_checking_assert (binding->value && OVL_P (binding->value));
+	  update_local_overload (binding, to_val);
 	}
 
   if (slot)
Index: gcc/testsuite/g++.dg/lookup/pr84836.C
===
--- gcc/testsuite/g++.dg/lookup/pr84836.C	(revision 0)
+++ gcc/testsuite/g++.dg/lookup/pr84836.C	(working copy)
@@ -0,0 +1,9 @@
+// PR c++/84836
+// ICE popping local binding
+
+void foo (void)
+{
+  struct A;
+  void A (int);
+  void A (long);
+}


Remove long-dead deprecation documentation

2018-03-21 Thread Nathan Sidwell

Sandra,
this removes the now-deleted deprecated features. AFAICT these have been 
gone for a long time -- 2008 is the most recent changelog entry -- so no 
need to put it in any release notes.


ok?

nathan
--
Nathan Sidwell
2018-03-21  Nathan Sidwell  

	* doc/extend.texi (Deprecated Features): Remove mention of
	long-deleted deprecations.

Index: doc/extend.texi
===
--- doc/extend.texi	(revision 258712)
+++ doc/extend.texi	(working copy)
@@ -23827,19 +23827,9 @@ While the list below is not exhaustive,
 that are now deprecated or have been removed:
 
 @table @code
-@item -fexternal-templates
-@itemx -falt-external-templates
-These are two options provided alternative methods of template
-instantiation.  @xref{Template Instantiation}.  The options have been removed.
-
-@item -fstrict-prototype
-@itemx -fno-strict-prototype
-Previously it was possible to use an empty prototype parameter list to
-indicate an unspecified number of parameters (like C), rather than no
-parameters, as C++ demands.  This feature has been removed.
 
 @item -fno-for-scope
-@item -ffriend-injection
+@itemx -ffriend-injection
 These two options provide compatibility with pre-standard C++.
 @xref{Backwards Compatibility}.
 
@@ -23850,23 +23840,6 @@ by one returning a different pointer typ
 covariant return type rules is now deprecated and will be removed from a
 future version.
 
-The G++ minimum and maximum operators (@samp{?}) and
-their compound forms (@samp{?=}) have been deprecated
-and are now removed from G++.  Code using these operators should be
-modified to use @code{std::min} and @code{std::max} instead.
-
-The named return value extension has been deprecated, and is now
-removed from G++.
-
-The use of initializer lists with new expressions has been deprecated,
-and is now removed from G++.
-
-Floating and complex non-type template parameters have been deprecated,
-and are now removed from G++.
-
-The implicit typename extension has been deprecated and is now
-removed from G++.
-
 The use of default arguments in function pointers, function typedefs
 and other places where they are not permitted by the standard is
 deprecated and will be removed from a future version of G++.


Re: Fix ICE after sorry for big stack arguments (PR 84964)

2018-03-21 Thread Richard Sandiford
Richard Biener  writes:
> On Wed, Mar 21, 2018 at 11:48 AM, Richard Sandiford
>  wrote:
>> Richard Biener  writes:
>>> On Wed, Mar 21, 2018 at 9:33 AM, Richard Sandiford
>>>  wrote:
 Index: gcc/diagnostic.c
 ===
 --- gcc/diagnostic.c2018-03-01 08:20:43.589534337 +
 +++ gcc/diagnostic.c2018-03-21 08:31:34.635677798 +
 @@ -1407,6 +1407,20 @@ sorry (const char *gmsgid, ...)
va_end (ap);
  }

 +/* Same, but stop compilation immediately.  */
 +
 +void
 +fatal_sorry (const char *gmsgid, ...)
 +{
 +  va_list ap;
 +  va_start (ap, gmsgid);
 +  rich_location richloc (line_table, input_location);
 +  diagnostic_impl (&richloc, -1, gmsgid, &ap, DK_SORRY);
 +  va_end (ap);
 +
 +  exit (FATAL_EXIT_CODE);
>>>
>>> This is somewhat not in style of the other "fatal" handlers.  I guess
>>> you don't get "sorry: " when using DK_FATAL, but ...
>>
>> I didn't add another DK_* because I don't think we want a different prefix
>> (unliked "error:" vs. "fatal error:").
>>
 +}
 +
  /* Return true if an error or a "sorry" has been seen.  Various
 processing is disabled after errors.  */
  bool
 Index: gcc/calls.c
 ===
 --- gcc/calls.c 2018-03-17 08:30:20.937100705 +
 +++ gcc/calls.c 2018-03-21 08:31:34.635677798 +
 @@ -4052,13 +4052,10 @@ expand_call (tree exp, rtx target, int i
   rtx_insn *before_arg = get_last_insn ();

   /* We don't allow passing huge (> 2^30 B) arguments
 -by value.  It would cause an overflow later on.  */
 +by value.  It would cause an overflow later on.  */
   if (constant_lower_bound (adjusted_args_size.constant)
   >= (1 << (HOST_BITS_PER_INT - 2)))
 -   {
 - sorry ("passing too large argument on stack");
 - continue;
 -   }
 +   fatal_sorry ("passing too large argument on stack");
>>>
>>> ... why not use fatal_error here?  It doesn't seem to be something
>>> that is just not implemented but impossible to implement?
>>
>> It's HOST_BITS_PER_INT rather than _WIDE_INT, so I think we could
>> implement it if we were tighter with types.  E.g. this PR occurred
>> because we went from int to HWI for some things, which should have
>> fixed part of the problem.  There might still be other uses of
>> "int" that need to change though.
>
> Ah, missed the BITS part.  I wonder if we can get away with simply
> setting adjusted_args_size.constant to some small constant?  For
> bits we generally need offset_ints, just HWI isn't enough anyway.

This is a byte quantity, so HWI should be OK.  We're just using
HOST_BITS_PER_INT to calculate the range of int on the host.

I think we'd run into other problems if the accumulated size didn't
match the sizes of the individual arguments.  That's why setting the
sizes to zero and the number of arguments to zero seemed "safer".
But I don't think anyone's really thought about what else could go wrong
if we make arbitrary changes like that.  Just bailing out seems better.

Thanks,
Richard


MAINTAINERS with no port

2018-03-21 Thread Nathan Sidwell
I just had reason to look at the target maintainer list.  There seems to 
be a couple of CPU ports listed in MAINTAINERS that do not exist in the 
config directory.


Ok to remove?  (Jeff you're the lucky, 'who do I ask' recipient)

nathan
--
Nathan Sidwell
2018-03-21  Nathan Sidwell  

	* MAINTAINERS: Remove picochip and score port entries.

Index: MAINTAINERS
===
--- MAINTAINERS	(revision 258711)
+++ MAINTAINERS	(working copy)
@@ -91,7 +91,6 @@ nios2 port		Chung-Lin Tang		
 nvptx port		Tom de Vries		
 pdp11 port		Paul Koning		
-picochip port		Daniel Towner		
 powerpcspe port		Andrew Jenner		
 riscv port		Kito Cheng		
 riscv port		Palmer Dabbelt		
@@ -105,7 +104,6 @@ rx port			Nick Clifton		
 s390 port		Ulrich Weigand		
 s390 port		Andreas Krebbel		
-score port		Chen Liqin		
 sh port			Alexandre Oliva		
 sh port			Oleg Endo		
 sparc port		David S. Miller		


Re: [PATCH][ARM][PR82989] Fix unexpected use of NEON instructions for shifts

2018-03-21 Thread Sudakshina Das

Hi

On 21/03/18 08:51, Christophe Lyon wrote:

On 20 March 2018 at 11:58, Sudakshina Das  wrote:

Hi

On 20/03/18 10:03, Richard Earnshaw (lists) wrote:


On 14/03/18 10:11, Sudakshina Das wrote:


Hi

This patch fixes PR82989 so that we avoid NEON instructions when
-mneon-for-64bits is not enabled. This is more of a short term fix for
the real deeper problem of making and early decision of choosing or
rejecting NEON instructions. There is now a new ticket PR84467 to deal
with the longer term solution.
(Please refer to the discussion in the bug report for more details).

Testing: Bootstrapped and regtested on arm-none-linux-gnueabihf and
added a new test case based on the test given on the bug report.

Ok for trunk and backports for gcc-7 and gcc-6 branches?



OK for trunk.  Please leave it a couple of days before backporting to
ensure that the testcase doesn't tickle any multilib issues.

R.



Thanks. Committed to trunk as r258677. Will wait a week for backporting.

Sudi



Hi Sudi,

I've noticed that:
FAIL:gcc.target/arm/pr82989.c scan-assembler-times lsl\\tr[0-9]+,
r[0-9]+, r[0-9] 2
FAIL:gcc.target/arm/pr82989.c scan-assembler-times lsr\\tr[0-9]+,
r[0-9]+, r[0-9] 2
on target armeb-none-linux-gnueabihf
--with-mode thumb
--with-cpu cortex-a9
--with-fpu neon-fp16

The tests pass when using --with-mode arm

Can you check?


Yes I see this as well. Sorry about this. I am testing a quick fix for 
this at the moment.


Thanks
Sudi



Thanks

Christophe






Sudi


*** gcc/ChangeLog ***

2018-03-14  Sudakshina Das  

  * config/arm/neon.md (ashldi3_neon): Update ?s for constraints
  to favor GPR over NEON registers.
  (di3_neon): Likewise.

*** gcc/testsuite/ChangeLog ***

2018-03-14  Sudakshina Das  

  * gcc.target/arm/pr82989.c: New test.

pr82989.diff


diff --git a/gcc/config/arm/neon.md b/gcc/config/arm/neon.md
index 6a6f5d7..1646b21 100644
--- a/gcc/config/arm/neon.md
+++ b/gcc/config/arm/neon.md
@@ -1180,12 +1180,12 @@
   )
 (define_insn_and_split "ashldi3_neon"
-  [(set (match_operand:DI 0 "s_register_operand"   "= w,
w,?&r,?r,?&r, ?w,w")
-   (ashift:DI (match_operand:DI 1 "s_register_operand" " 0w, w, 0r,
0,  r, 0w,w")
-  (match_operand:SI 2 "general_operand""rUm, i,  r,
i,  i,rUm,i")))
-   (clobber (match_scratch:SI 3"= X,
X,?&r, X,  X,  X,X"))
-   (clobber (match_scratch:SI 4"= X,
X,?&r, X,  X,  X,X"))
-   (clobber (match_scratch:DI 5"=&w,
X,  X, X,  X, &w,X"))
+  [(set (match_operand:DI 0 "s_register_operand"   "= w, w, &r,
r, &r, ?w,?w")
+   (ashift:DI (match_operand:DI 1 "s_register_operand" " 0w, w, 0r,
0,  r, 0w, w")
+  (match_operand:SI 2 "general_operand""rUm, i,  r,
i,  i,rUm, i")))
+   (clobber (match_scratch:SI 3"= X,
X, &r, X,  X,  X, X"))
+   (clobber (match_scratch:SI 4"= X,
X, &r, X,  X,  X, X"))
+   (clobber (match_scratch:DI 5"=&w,
X,  X, X,  X, &w, X"))
  (clobber (reg:CC_C CC_REGNUM))]
 "TARGET_NEON"
 "#"
@@ -1276,7 +1276,7 @@
   ;; ashrdi3_neon
   ;; lshrdi3_neon
   (define_insn_and_split "di3_neon"
-  [(set (match_operand:DI 0 "s_register_operand""= w,
w,?&r,?r,?&r,?w,?w")
+  [(set (match_operand:DI 0 "s_register_operand""= w, w, &r,
r, &r,?w,?w")
 (RSHIFTS:DI (match_operand:DI 1 "s_register_operand" " 0w, w, 0r,
0,  r,0w, w")
 (match_operand:SI 2 "reg_or_int_operand" "  r, i,  r,
i,  i, r, i")))
  (clobber (match_scratch:SI 3
"=2r, X, &r, X,  X,2r, X"))
diff --git a/gcc/testsuite/gcc.target/arm/pr82989.c
b/gcc/testsuite/gcc.target/arm/pr82989.c
new file mode 100644
index 000..1295ee6
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr82989.c
@@ -0,0 +1,38 @@
+/* PR target/82989 */
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_neon_ok } */
+/* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } {
"-mcpu=*" } { "-mcpu=cortex-a8" } } */
+/* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } {
"-mfpu=*" } { "-mfpu=neon" } } */
+/* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } {
"-mfloat-abi=*" } { "-mfloat-abi=hard" } } */
+/* { dg-options "-O2 -mcpu=cortex-a8 -mfpu=neon -mfloat-abi=hard" } */
+/* { dg-add-options arm_neon } */
+
+typedef unsigned long long uint64_t;
+
+void f_shr_imm (uint64_t *a )
+{
+  *a += *a >> 32;
+}
+/* { dg-final { scan-assembler-not "vshr*" } } */
+
+void f_shr_reg (uint64_t *a, uint64_t b)
+{
+  *a += *a >> b;
+}
+/* { dg-final { scan-assembler-not "vshl*" } } */
+/* Only 2 times for f_shr_reg. f_shr_imm should not have any.  */
+/* { dg-final { scan-assembler-times {lsr\tr[0-9]+, r[0-9]+, r[0-9]} 2 }
} */
+
+void f_shl_imm (uint64_t *a)
+{
+  *a += *a << 32;
+}
+/* { dg-final { scan-assembler-not "vshl*" } } */
+
+void f_

Re: Fix ICE after sorry for big stack arguments (PR 84964)

2018-03-21 Thread Richard Biener
On Wed, Mar 21, 2018 at 12:16 PM, Richard Sandiford
 wrote:
> Richard Biener  writes:
>> On Wed, Mar 21, 2018 at 11:48 AM, Richard Sandiford
>>  wrote:
>>> Richard Biener  writes:
 On Wed, Mar 21, 2018 at 9:33 AM, Richard Sandiford
  wrote:
> Index: gcc/diagnostic.c
> ===
> --- gcc/diagnostic.c2018-03-01 08:20:43.589534337 +
> +++ gcc/diagnostic.c2018-03-21 08:31:34.635677798 +
> @@ -1407,6 +1407,20 @@ sorry (const char *gmsgid, ...)
>va_end (ap);
>  }
>
> +/* Same, but stop compilation immediately.  */
> +
> +void
> +fatal_sorry (const char *gmsgid, ...)
> +{
> +  va_list ap;
> +  va_start (ap, gmsgid);
> +  rich_location richloc (line_table, input_location);
> +  diagnostic_impl (&richloc, -1, gmsgid, &ap, DK_SORRY);
> +  va_end (ap);
> +
> +  exit (FATAL_EXIT_CODE);

 This is somewhat not in style of the other "fatal" handlers.  I guess
 you don't get "sorry: " when using DK_FATAL, but ...
>>>
>>> I didn't add another DK_* because I don't think we want a different prefix
>>> (unliked "error:" vs. "fatal error:").
>>>
> +}
> +
>  /* Return true if an error or a "sorry" has been seen.  Various
> processing is disabled after errors.  */
>  bool
> Index: gcc/calls.c
> ===
> --- gcc/calls.c 2018-03-17 08:30:20.937100705 +
> +++ gcc/calls.c 2018-03-21 08:31:34.635677798 +
> @@ -4052,13 +4052,10 @@ expand_call (tree exp, rtx target, int i
>   rtx_insn *before_arg = get_last_insn ();
>
>   /* We don't allow passing huge (> 2^30 B) arguments
> -by value.  It would cause an overflow later on.  */
> +by value.  It would cause an overflow later on.  */
>   if (constant_lower_bound (adjusted_args_size.constant)
>   >= (1 << (HOST_BITS_PER_INT - 2)))
> -   {
> - sorry ("passing too large argument on stack");
> - continue;
> -   }
> +   fatal_sorry ("passing too large argument on stack");

 ... why not use fatal_error here?  It doesn't seem to be something
 that is just not implemented but impossible to implement?
>>>
>>> It's HOST_BITS_PER_INT rather than _WIDE_INT, so I think we could
>>> implement it if we were tighter with types.  E.g. this PR occurred
>>> because we went from int to HWI for some things, which should have
>>> fixed part of the problem.  There might still be other uses of
>>> "int" that need to change though.
>>
>> Ah, missed the BITS part.  I wonder if we can get away with simply
>> setting adjusted_args_size.constant to some small constant?  For
>> bits we generally need offset_ints, just HWI isn't enough anyway.
>
> This is a byte quantity, so HWI should be OK.  We're just using
> HOST_BITS_PER_INT to calculate the range of int on the host.

Ok...

> I think we'd run into other problems if the accumulated size didn't
> match the sizes of the individual arguments.  That's why setting the
> sizes to zero and the number of arguments to zero seemed "safer".
> But I don't think anyone's really thought about what else could go wrong
> if we make arbitrary changes like that.  Just bailing out seems better.

Fine.  Then sorry () plus fatal_error () with a comment still sounds best here
(if we care at all about error-recovery issues - care to "paper over
them" instead
of fixing them for real, whatever that means)

Richard.

> Thanks,
> Richard


Re: MAINTAINERS with no port

2018-03-21 Thread Richard Biener
On Wed, Mar 21, 2018 at 12:38 PM, Nathan Sidwell  wrote:
> I just had reason to look at the target maintainer list.  There seems to be
> a couple of CPU ports listed in MAINTAINERS that do not exist in the config
> directory.
>
> Ok to remove?  (Jeff you're the lucky, 'who do I ask' recipient)

Please simply move them to the write-after-approval section.

Richard.

> nathan
> --
> Nathan Sidwell


[PATCH][ARM] Fix test pr82989.c for big endian and mthumb

2018-03-21 Thread Sudakshina Das

Hi

The test pr82989.c which was added in one of previous commits is failing
for mthumb and big-endian configurations. The aim of this test was to
check that NEON instructions are not being used for simple shift
operations. The scanning of lsl and lsr instructions and checking its
counts were just too restrictive for different configurations. So I
have now simplified the test to only check for the absence of NEON
instructions.

Testing: Only test case change so only tested the said test on
differently configured toolchain.
@Christophe can you confirm this patch fixes the failure for you?

Thanks
Sudi

*** gcc/testsuite/ChangeLog ***

2018-03-21  Sudakshina Das  

PR target/82989
* gcc.target/arm/pr82989.c: Change dg-scan-assembly directives.
diff --git a/gcc/testsuite/gcc.target/arm/pr82989.c b/gcc/testsuite/gcc.target/arm/pr82989.c
index 6f74dba..8519c3f 100644
--- a/gcc/testsuite/gcc.target/arm/pr82989.c
+++ b/gcc/testsuite/gcc.target/arm/pr82989.c
@@ -13,26 +13,21 @@ void f_shr_imm (uint64_t *a)
 {
   *a += *a >> 32;
 }
-/* { dg-final { scan-assembler-not "vshr*" } } */
 
 void f_shr_reg (uint64_t *a, uint64_t b)
 {
   *a += *a >> b;
 }
-/* { dg-final { scan-assembler-not "vshl*" } } */
-/* Only 2 times for f_shr_reg. f_shr_imm should not have any.  */
-/* { dg-final { scan-assembler-times {lsr\tr[0-9]+, r[0-9]+, r[0-9]} 2 } } */
 
 void f_shl_imm (uint64_t *a)
 {
   *a += *a << 32;
 }
-/* { dg-final { scan-assembler-not "vshl*" } } */
 
 void f_shl_reg (uint64_t *a, uint64_t b)
 {
   *a += *a << b;
 }
 /* { dg-final { scan-assembler-not "vshl*" } } */
-/* Only 2 times for f_shl_reg. f_shl_imm should not have any.  */
-/* { dg-final { scan-assembler-times {lsl\tr[0-9]+, r[0-9]+, r[0-9]} 2 } } */
+/* { dg-final { scan-assembler-not "vshr*" } } */
+/* { dg-final { scan-assembler-not "vmov*" } } */


Deprecate not defining NO_IMPLICIT_EXTERN_C

2018-03-21 Thread Nathan Sidwell

Port maintainers CC'd, apologies if I've missed you.

NO_IMPLICIT_EXTERN_C is a target machine define that turns off wrapping 
system header files in 'extern "C" { ... }'.  It is the remaining 
non-deprecated ARM-era compatibility behaviour.  Can we deprecated it?


Unfortunately it's a negative-sense define, that now at least most ports 
define.  Do all ports define it?  It's hard to determine that, because 
many ports get it set via config/gnu-user.h or similar common file.


It also has the following undocumented features (when not set):
1) in a system header in an extern "C" region, a prototype of the form 
'T func ()' means unspecified parms.   We treat it as (...), but that's 
not valid C and probably will do the wrong thing with today's more 
complex ABIs.


2) Again, in a system header for extern "C" fns, enables matching 
between function prototypes via self-promoting parameter types.  I.e. 
'extern "C" T foo (char)' and 'extern "C" T foo (int)' are the same fn.


The path by which the extern "C" wrapping happens is tortuous though 
c-family/c-lex.c via a global variable and the parser setting flags on 
each token.  Ugh!


I think the way of going about that would be to require it to be defined 
to either 0 or 1, and test that, rather than its definedness.  Then any 
ports that require the old behaviour will need to set it to zero, 
explicitly indicating the requirement.  Something like:


// in default.h
#if !defined (NO_IMPLICIT_EXTERN_C) \
  || (0 - NO_IMPLICIT_EXTERN_C - 1) > 0 /* Detect blank */
#error NO_IMPLICIT_EXTERN_C must be defined to 0 or 1, see XXX
#endif

modify the existing #defines to provide a value.

modify the uses from
  #ifndef NO_IMPLICIT_EXTERN_C
into
  #if !NO_IMPLICIT_EXTERN_C

(That's just as vulnerable to misspellings of the name as the ifndef 
case, and we're converting existing code, so probably quite safe.)


My suspicion is that pdp11 and/or vax may care?

Comments/Objections?

nathan
--
Nathan Sidwell


Re: Results for 8.0.1 20180316 (experimental) [trunk revision 258610] (GCC) libstdc++ testsuite on x86_64-pc-linux-gnu

2018-03-21 Thread Jonathan Wakely
On 20 March 2018 at 21:06, Jonathan Wakely wrote:
> On 20 March 2018 at 21:02, François Dumont wrote:
>> On 20/03/2018 19:20, Jonathan Wakely wrote:
>>>
>>> On 17 March 2018 at 09:01, Jonathan Wakely wrote:

 Native configuration is x86_64-pc-linux-gnu

  === libstdc++ tests ===


 Running target unix/-std=gnu++11/-D_GLIBCXX_ASSERTIONS

  === libstdc++ Summary for
 unix/-std=gnu++11/-D_GLIBCXX_ASSERTIONS ===

 # of expected passes12330
 # of expected failures  71
 # of unsupported tests  579

 Running target unix/-std=gnu++11/-D_GLIBCXX_DEBUG
 XPASS: 21_strings/basic_string_view/element_access/char/2.cc execution
 test
 XPASS: 21_strings/basic_string_view/element_access/wchar_t/2.cc execution
 test
 FAIL: 23_containers/bitset/hash/1.cc (test for excess errors)
 UNRESOLVED: 23_containers/bitset/hash/1.cc compilation failed to produce
 executable
>>>
>>> These debug mode failures are regressions, reported as
>>> https://gcc.gnu.org/PR84998
>>>
>>> I think they all have the same root cause (Nathan fixed a bug in G++).
>>>
>> I had already prepared this patch. Thanks for explaining why we have this
>> problem now.
>
> Ah I was just about to commit my own fix.
>
>> Note that I chose to use full specialization with std::hash<> for
>> std::bitset and std::vector.
>
> Great, that's an improvement over my patch.
>
>> It also fix a versioned namespace issue when partially specializing
>> std::hash for debug vector.
>
> Even better.
>
>> Tested under Linux x86_64 normal, debug and versioned namespace modes.
>>
>> Ok to commit ?
>
> Yes please!
>
> Please be sure to add "PR libstdc++/84998" to the changelog entry so
> it updates bugzilla.
>
> Thanks very much.

This adds a test for the hash specializations in debug mode, and fixes
another failure caused by different line numbers in debug mode.

Committed to trunk.
commit 347afbec5010ed9edeff70ce18d5e2511a7c27f4
Author: Jonathan Wakely 
Date:   Sun Mar 18 00:01:07 2018 +

Fix some libstdc++ testsuite failures

* testsuite/20_util/function_objects/comparisons_pointer.cc: Use
VERIFY instead of assert.
* testsuite/20_util/hash/84998.cc: New test.
* testsuite/23_containers/vector/cons/destructible_debug_neg.cc: New
copy of test adjusted for Debug Mode.
* testsuite/23_containers/vector/cons/destructible_neg.cc: Do not 
run
test in Debug Mode.

diff --git 
a/libstdc++-v3/testsuite/20_util/function_objects/comparisons_pointer.cc 
b/libstdc++-v3/testsuite/20_util/function_objects/comparisons_pointer.cc
index aad0bc3dd2e..474190cdf81 100644
--- a/libstdc++-v3/testsuite/20_util/function_objects/comparisons_pointer.cc
+++ b/libstdc++-v3/testsuite/20_util/function_objects/comparisons_pointer.cc
@@ -174,7 +174,7 @@ test05()
   int n = 0;
   while (ss >> n)
 sum += n;
-  assert( sum == 1 );
+  VERIFY( sum == 1 );
 
 #if __cplusplus >= 201402L
   static_assert( lt(y, y+1), "constexpr less" );
diff --git a/libstdc++-v3/testsuite/20_util/hash/84998.cc 
b/libstdc++-v3/testsuite/20_util/hash/84998.cc
new file mode 100644
index 000..72bd203cb04
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/hash/84998.cc
@@ -0,0 +1,40 @@
+// Copyright (C) 2018 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// .
+
+// { dg-options "-D_GLIBCXX_DEBUG" }
+// { dg-do compile { target c++11 } }
+
+// PR libstdc++/84998
+
+#include 
+#include 
+
+std::size_t
+test01()
+{
+  std::bitset<1> b;
+  std::hash> h;
+  return h(b);
+}
+
+std::size_t
+test02()
+{
+  std::vector b;
+  std::hash> h;
+  return h(b);
+}
diff --git 
a/libstdc++-v3/testsuite/23_containers/vector/cons/destructible_debug_neg.cc 
b/libstdc++-v3/testsuite/23_containers/vector/cons/destructible_debug_neg.cc
new file mode 100644
index 000..5127f5105f4
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/vector/cons/destructible_debug_neg.cc
@@ -0,0 +1,48 @@
+// Copyright (C) 2017-2018 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public Lice

lm32 port?

2018-03-21 Thread Nathan Sidwell

I got a bounce from:
lm32 port   Sebastien Bourdeauducq  

2011 seems the most recent period of activity (not counting mechanical 
interface changes).

2011-11-04  Ralf Corsépius  

* config/lm32/t-rtems: New.
* config.gcc (lm32-*-rtems*): Add t-rtems.

is it dead?

nathan
--
Nathan Sidwell


Re: MAINTAINERS with no port

2018-03-21 Thread Nathan Sidwell

On 03/21/2018 08:02 AM, Richard Biener wrote:

On Wed, Mar 21, 2018 at 12:38 PM, Nathan Sidwell  wrote:

I just had reason to look at the target maintainer list.  There seems to be
a couple of CPU ports listed in MAINTAINERS that do not exist in the config
directory.

Ok to remove?  (Jeff you're the lucky, 'who do I ask' recipient)


Please simply move them to the write-after-approval section.


Done

nathan

--
Nathan Sidwell
2018-03-21  Nathan Sidwell  

	* MAINTAINERS: Move maintainers of now-removed picochip and score
	ports to write after approval.

Index: MAINTAINERS
===
--- MAINTAINERS	(revision 258711)
+++ MAINTAINERS	(working copy)
@@ -91,7 +91,6 @@ nios2 port		Chung-Lin Tang		
 nvptx port		Tom de Vries		
 pdp11 port		Paul Koning		
-picochip port		Daniel Towner		
 powerpcspe port		Andrew Jenner		
 riscv port		Kito Cheng		
 riscv port		Palmer Dabbelt		
@@ -105,7 +104,6 @@ rx port			Nick Clifton		
 s390 port		Ulrich Weigand		
 s390 port		Andreas Krebbel		
-score port		Chen Liqin		
 sh port			Alexandre Oliva		
 sh port			Oleg Endo		
 sparc port		David S. Miller		
@@ -480,6 +478,7 @@ James Lemke	
 Kriang Lerdsuwanakij
 Renlin Li	
 Xinliang David Li
+Chen Liqin	
 Martin Liska	
 Jiangning Liu	
 Sa Liu		
@@ -609,6 +608,7 @@ Caroline Tice	
 Kai Tietz	
 Ilya Tocar	
 Philipp Tomsich	
+Daniel Towner	
 Konrad Trifunovic
 Markus Trippelsdorf
 Igor Tsimbalist	


Re: [PATCH] Fix compile-time hog in MPX boundary checking (PR target/84988).

2018-03-21 Thread Martin Liška

On 03/21/2018 10:39 AM, Martin Liška wrote:

On 03/21/2018 10:36 AM, Richard Biener wrote:

On Tue, Mar 20, 2018 at 10:44 PM, Richard Sandiford
 wrote:

Jeff Law  writes:

On 03/20/2018 01:36 PM, Martin Liška wrote:

Hi.

This is a work-around to not iterate all members of array that can be huge.
As MPX will be removed in GCC 9.x, I hope it's acceptable. I don't want
to come
up with a new param for it.

Survives tests&bootstrap on x86_64-linux-gnu.

Martin

gcc/ChangeLog:

2018-03-20  Martin Liska  

 PR target/84988
 * tree-chkp.c (CHKP_ARRAY_MAX_CHECK_STEPS): Define a new macro.
 (chkp_find_bound_slots_1): Limit number of iterations.

Or just CLOSE/WONTFIX :-)

I've got no objections here -- we want to  minimize the effort put into
CHKP given its going to be deprecated.


The problem is that this affects normal configs, not just ones with
MPX enabled.


Indeed.  It get's called via

#0  chkp_find_bound_slots_1 (type=0x769ee9d8, have_bound=0x2ed3868, offs=0)
 at /space/rguenther/src/svn/early-lto-debug/gcc/tree-chkp.c:1708
#1  0x01379a13 in chkp_find_bound_slots (type=0x769ee9d8,
 res=0x2ed3868)
 at /space/rguenther/src/svn/early-lto-debug/gcc/tree-chkp.c:1754
#2  0x01377054 in chkp_type_bounds_count (type=0x769ee9d8)
 at /space/rguenther/src/svn/early-lto-debug/gcc/tree-chkp.c:1009
#3  0x016c664f in ix86_function_arg_advance (cum_v=...,
 mode=E_BLKmode, type=0x769ee9d8, named=true)
 at /space/rguenther/src/svn/early-lto-debug/gcc/config/i386/i386.c:8621
8616    {
8617  /* Track if there are outgoing arguments on stack.  */
8618  if (cum->caller)
8619    cfun->machine->outgoing_args_on_stack = true;
8620
8621  cum->bnds_in_bt = chkp_type_bounds_count (type);
8622    }
8623    }
8624
8625    /* Define where to put the arguments to a function.

but I think we know POINTER_BOUNDS_TYPE_P etc. never return
true if -fcheck-pointer-* or -mmpx is not enabled, right?  So we can
guard the above call appropriately and save some compile-time
for all of us?


Good observation, let me enhance the patch for the PR.

Martin



Richard.


Thanks,
Richard




Hi.

I've got this, that survives bootstrap®ression tests on x86_64-linux-gnu.

May I install it?
Thanks,
Martin
>From 15f95ac5117d9e2bab96c725716e5cb2832d3589 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Wed, 21 Mar 2018 10:51:32 +0100
Subject: [PATCH] Do not call chkp_type_bounds_count if MPX is not enabled (PR
 target/84988).

gcc/ChangeLog:

2018-03-21  Martin Liska  

	PR target/84988
	* config/i386/i386.c (ix86_function_arg_advance): Do not call
	chkp_type_bounds_count if MPX is not enabled.
---
 gcc/config/i386/i386.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 5b1e962dedb..0693f8fc451 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -8618,7 +8618,8 @@ ix86_function_arg_advance (cumulative_args_t cum_v, machine_mode mode,
   if (cum->caller)
 	cfun->machine->outgoing_args_on_stack = true;
 
-  cum->bnds_in_bt = chkp_type_bounds_count (type);
+  if (type && POINTER_BOUNDS_TYPE_P (type))
+	cum->bnds_in_bt = chkp_type_bounds_count (type);
 }
 }
 
-- 
2.16.2



Re: [PATCH] Fix compile-time hog in MPX boundary checking (PR target/84988).

2018-03-21 Thread Jakub Jelinek
On Wed, Mar 21, 2018 at 01:40:08PM +0100, Martin Liška wrote:
> 2018-03-21  Martin Liska  
> 
>   PR target/84988
>   * config/i386/i386.c (ix86_function_arg_advance): Do not call
>   chkp_type_bounds_count if MPX is not enabled.
> ---
>  gcc/config/i386/i386.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index 5b1e962dedb..0693f8fc451 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -8618,7 +8618,8 @@ ix86_function_arg_advance (cumulative_args_t cum_v, 
> machine_mode mode,
>if (cum->caller)
>   cfun->machine->outgoing_args_on_stack = true;
>  
> -  cum->bnds_in_bt = chkp_type_bounds_count (type);
> +  if (type && POINTER_BOUNDS_TYPE_P (type))
> + cum->bnds_in_bt = chkp_type_bounds_count (type);

This is weird.  POINTER_BOUNDS_TYPE_P (type)
is TREE_CODE (type) == POINTER_BOUNDS_TYPE,
and for POINTER_BOUNDS_TYPE chkp_type_bounds_count will just unconditionally
return 0.

Jakub


Re: [PATCH, gotools]: Add -test.timeout to to runtime gotools tests

2018-03-21 Thread Ian Lance Taylor
On Wed, Mar 21, 2018 at 2:07 AM, Uros Bizjak  wrote:
>
> Attached patch adds -test.timeout argument to runtime gotools test.
> Without this argument, default 4 minute timeout instead of
> $(GOTOOLS_TEST_TIMEOUT) is used which is way to short for slow
> machines.
>
> Patch was tested on alphaev68-linux-gnu.

This is OK with a ChangeLog entry.  Go ahead and commit.

Thanks.  I guess I missed that one somehow.

Ian


Re: lm32 port?

2018-03-21 Thread Sebastian Huber
The lm32 architecture is still included in the standard RTEMS tool set. 
However, I noticed that lm32-specific GCC bugs get not fixed. I was 
about to suggest to deprecated this architecture for RTEMS.


--
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax : +49 89 189 47 41-09
E-Mail  : sebastian.hu...@embedded-brains.de
PGP : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.



Re: Deprecate not defining NO_IMPLICIT_EXTERN_C

2018-03-21 Thread Richard Earnshaw (lists)
Isn't this an OS issue rather than a general port issue.  Seems to me it
will depend on the system header files.  With a few notable exceptions
I'm not sure the port maintainers can answer this for all their target OSs.

R.

On 21/03/18 12:15, Nathan Sidwell wrote:
> Port maintainers CC'd, apologies if I've missed you.
> 
> NO_IMPLICIT_EXTERN_C is a target machine define that turns off wrapping
> system header files in 'extern "C" { ... }'.  It is the remaining
> non-deprecated ARM-era compatibility behaviour.  Can we deprecated it?
> 
> Unfortunately it's a negative-sense define, that now at least most ports
> define.  Do all ports define it?  It's hard to determine that, because
> many ports get it set via config/gnu-user.h or similar common file.
> 
> It also has the following undocumented features (when not set):
> 1) in a system header in an extern "C" region, a prototype of the form
> 'T func ()' means unspecified parms.   We treat it as (...), but that's
> not valid C and probably will do the wrong thing with today's more
> complex ABIs.
> 
> 2) Again, in a system header for extern "C" fns, enables matching
> between function prototypes via self-promoting parameter types.  I.e.
> 'extern "C" T foo (char)' and 'extern "C" T foo (int)' are the same fn.
> 
> The path by which the extern "C" wrapping happens is tortuous though
> c-family/c-lex.c via a global variable and the parser setting flags on
> each token.  Ugh!
> 
> I think the way of going about that would be to require it to be defined
> to either 0 or 1, and test that, rather than its definedness.  Then any
> ports that require the old behaviour will need to set it to zero,
> explicitly indicating the requirement.  Something like:
> 
> // in default.h
> #if !defined (NO_IMPLICIT_EXTERN_C) \
>    || (0 - NO_IMPLICIT_EXTERN_C - 1) > 0 /* Detect blank */
> #error NO_IMPLICIT_EXTERN_C must be defined to 0 or 1, see XXX
> #endif
> 
> modify the existing #defines to provide a value.
> 
> modify the uses from
>    #ifndef NO_IMPLICIT_EXTERN_C
> into
>    #if !NO_IMPLICIT_EXTERN_C
> 
> (That's just as vulnerable to misspellings of the name as the ifndef
> case, and we're converting existing code, so probably quite safe.)
> 
> My suspicion is that pdp11 and/or vax may care?
> 
> Comments/Objections?
> 
> nathan
> -- 
> Nathan Sidwell



Re: Deprecate not defining NO_IMPLICIT_EXTERN_C

2018-03-21 Thread David Edelsohn
AIX still requires implicit extern C.

I previously have tried to disable the feature on AIX and it failed miserably.

Thanks, David


On Wed, Mar 21, 2018 at 8:15 AM, Nathan Sidwell  wrote:
> Port maintainers CC'd, apologies if I've missed you.
>
> NO_IMPLICIT_EXTERN_C is a target machine define that turns off wrapping
> system header files in 'extern "C" { ... }'.  It is the remaining
> non-deprecated ARM-era compatibility behaviour.  Can we deprecated it?
>
> Unfortunately it's a negative-sense define, that now at least most ports
> define.  Do all ports define it?  It's hard to determine that, because many
> ports get it set via config/gnu-user.h or similar common file.
>
> It also has the following undocumented features (when not set):
> 1) in a system header in an extern "C" region, a prototype of the form 'T
> func ()' means unspecified parms.   We treat it as (...), but that's not
> valid C and probably will do the wrong thing with today's more complex ABIs.
>
> 2) Again, in a system header for extern "C" fns, enables matching between
> function prototypes via self-promoting parameter types.  I.e. 'extern "C" T
> foo (char)' and 'extern "C" T foo (int)' are the same fn.
>
> The path by which the extern "C" wrapping happens is tortuous though
> c-family/c-lex.c via a global variable and the parser setting flags on each
> token.  Ugh!
>
> I think the way of going about that would be to require it to be defined to
> either 0 or 1, and test that, rather than its definedness.  Then any ports
> that require the old behaviour will need to set it to zero, explicitly
> indicating the requirement.  Something like:
>
> // in default.h
> #if !defined (NO_IMPLICIT_EXTERN_C) \
>   || (0 - NO_IMPLICIT_EXTERN_C - 1) > 0 /* Detect blank */
> #error NO_IMPLICIT_EXTERN_C must be defined to 0 or 1, see XXX
> #endif
>
> modify the existing #defines to provide a value.
>
> modify the uses from
>   #ifndef NO_IMPLICIT_EXTERN_C
> into
>   #if !NO_IMPLICIT_EXTERN_C
>
> (That's just as vulnerable to misspellings of the name as the ifndef case,
> and we're converting existing code, so probably quite safe.)
>
> My suspicion is that pdp11 and/or vax may care?
>
> Comments/Objections?
>
> nathan
> --
> Nathan Sidwell


Re: [PATCH, rs6000] Finish implementation of __builtin_atlivec_lvx_v1ti

2018-03-21 Thread Peter Bergner
On 3/14/18 4:27 PM, Kelvin Nilsen wrote:
> 2018-03-14  Kelvin Nilsen  
>
>   * config/rs6000/rs6000-c.c (altivec_overloaded_builtins): Add
>   entries for V1TI variants of __builtin_altivec_ld builtin.
>   * config/rs6000/rs6000.c (altivec_expand_lv_builtin): Add test and
>   handling of V1TI variant of LVX icode pattern.
>   (altivec_expand_builtin): Add case for ALTIVEC_BUILTIN_LVX_V1TI.
>   (rs6000_gimple_fold_builtin): Likewise.
>   (altivec_init_builtins): Add code to define
>   __builtin_altivec_lvx_v1ti function.
>   * doc/extend.texi: Add four new prototypes for vec_ld.
>
> gcc/testsuite/ChangeLog:
>
> 2018-03-14  Kelvin Nilsen  
>
>   * gcc.target/powerpc/altivec-ld-1.c: New test.

You'll also need to add:

PR target/84760

...to both ChangeLog entries.

Peter




Re: [parloops, PR83126], Use cached affine_ivs canonicalize_loop_ivs

2018-03-21 Thread Tom de Vries

On 03/12/2018 01:14 PM, Richard Biener wrote:

On Thu, 22 Feb 2018, Tom de Vries wrote:


Hi,

this patch fixes an ICE in the parloops pass.

The ICE (when compiling the test-case in attached patch) follows from the fact
that here in gen_parallel_loop the call to canonicalize_loop_ivs fails to
"base all the induction variables in LOOP on a single control one":
...
   /* Base all the induction variables in LOOP on a single control one.*/
   canonicalize_loop_ivs (loop, &nit, true);
...

This is caused by the fact that for one of the induction variables, simple_iv
no longer returns true (as was the case at the start of gen_parallel_loop).
This seems to be triggered by the fact that during loop_version we call
scev_reset (although I'm not sure why when recalculating scev info we're not
reaching the same conclusion as before).

I guess the real reason is that canonicalize_loop_ivs calls
create_iv first which in the parloop case with bump-in-latch true
and then incrementally re-writes PHIs, eventually wrecking other
PHIs SCEV.  In this case the 2nd PHIs evolution depends on the
first one but the rewritten ones depend on the new IV already.

So the better fix (maybe not 100% enough) would be to re-organize
canonicalize_loop_ivs to first do analysis of all PHIs and then
rewrite them all.



Focusing on the re-organize first, is this sort of what you had in mind?

Bootstrapped and reg-tested on x86_64.

Thanks,
- Tom
Split canonicalize_loop_ivs in analyze and rewrite parts

2018-03-21  Tom de Vries  

	* tree-ssa-loop-manip.c (rewrite_phi_with_iv): Remove loop parameter.
	Add iv parameter.  Move early-out tests ...
	(rewrite_all_phi_nodes_with_iv): ... here.  Remove loop argument and add
	iv argument to rewrite_phi_with_iv call.
	(get_all_phi_affine_ivs): New function.
	(canonicalize_loop_ivs): Call get_all_phi_affine_ivs and pass result as
	argument to rewrite_all_phi_nodes_with_iv.

---
 gcc/tree-ssa-loop-manip.c | 82 +++
 1 file changed, 61 insertions(+), 21 deletions(-)

diff --git a/gcc/tree-ssa-loop-manip.c b/gcc/tree-ssa-loop-manip.c
index bf425af..15a5795 100644
--- a/gcc/tree-ssa-loop-manip.c
+++ b/gcc/tree-ssa-loop-manip.c
@@ -43,6 +43,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree-scalar-evolution.h"
 #include "params.h"
 #include "tree-inline.h"
+#include "hash-map.h"
 
 /* All bitmaps for rewriting into loop-closed SSA go on this obstack,
so that we can free them all at once.  */
@@ -1430,48 +1431,65 @@ tree_unroll_loop (struct loop *loop, unsigned factor,
induction variable MAIN_IV and insert the generated code at GSI.  */
 
 static void
-rewrite_phi_with_iv (loop_p loop,
-		 gphi_iterator *psi,
+rewrite_phi_with_iv (gphi_iterator *psi,
 		 gimple_stmt_iterator *gsi,
-		 tree main_iv)
+		 tree main_iv, affine_iv *iv)
 {
-  affine_iv iv;
   gassign *stmt;
   gphi *phi = psi->phi ();
   tree atype, mtype, val, res = PHI_RESULT (phi);
 
-  if (virtual_operand_p (res) || res == main_iv)
-{
-  gsi_next (psi);
-  return;
-}
-
-  if (!simple_iv (loop, loop, res, &iv, true))
-{
-  gsi_next (psi);
-  return;
-}
-
   remove_phi_node (psi, false);
 
   atype = TREE_TYPE (res);
   mtype = POINTER_TYPE_P (atype) ? sizetype : atype;
-  val = fold_build2 (MULT_EXPR, mtype, unshare_expr (iv.step),
+  val = fold_build2 (MULT_EXPR, mtype, unshare_expr (iv->step),
 		 fold_convert (mtype, main_iv));
   val = fold_build2 (POINTER_TYPE_P (atype)
 		 ? POINTER_PLUS_EXPR : PLUS_EXPR,
-		 atype, unshare_expr (iv.base), val);
+		 atype, unshare_expr (iv->base), val);
   val = force_gimple_operand_gsi (gsi, val, false, NULL_TREE, true,
   GSI_SAME_STMT);
   stmt = gimple_build_assign (res, val);
   gsi_insert_before (gsi, stmt, GSI_SAME_STMT);
 }
 
+/* Return map of phi node result to affine_iv, for all phis in LOOPS.  */
+
+static void
+get_all_phi_affine_ivs (loop_p loop, hash_map *simple_ivs)
+{
+  unsigned i;
+  basic_block *bbs = get_loop_body_in_dom_order (loop);
+  gphi_iterator psi;
+
+  for (i = 0; i < loop->num_nodes; i++)
+{
+  basic_block bb = bbs[i];
+
+  if (bb->loop_father != loop)
+	continue;
+
+  for (psi = gsi_start_phis (bb); !gsi_end_p (psi);
+	   gsi_next (&psi))
+	{
+	  gphi *phi = psi.phi ();
+	  affine_iv iv;
+	  tree res = PHI_RESULT (phi);
+	  if (simple_iv (loop, loop, res, &iv, true))
+	simple_ivs->put (res, iv);
+	}
+}
+
+  free (bbs);
+}
+
 /* Rewrite all the phi nodes of LOOP in function of the main induction
variable MAIN_IV.  */
 
 static void
-rewrite_all_phi_nodes_with_iv (loop_p loop, tree main_iv)
+rewrite_all_phi_nodes_with_iv (loop_p loop, tree main_iv,
+			   hash_map *simple_ivs)
 {
   unsigned i;
   basic_block *bbs = get_loop_body_in_dom_order (loop);
@@ -1486,7 +1504,25 @@ rewrite_all_phi_nodes_with_iv (loop_p loop, tree main_iv)
 	continue;
 
   for (psi = gsi_start_phis (bb); !gsi_end_p (psi); )
-	rewr

Re: [parloops, PR83126], Use cached affine_ivs canonicalize_loop_ivs

2018-03-21 Thread Bin.Cheng
On Wed, Mar 21, 2018 at 3:06 PM, Tom de Vries  wrote:
> On 03/12/2018 01:14 PM, Richard Biener wrote:
>>
>> On Thu, 22 Feb 2018, Tom de Vries wrote:
>>
>>> Hi,
>>>
>>> this patch fixes an ICE in the parloops pass.
>>>
>>> The ICE (when compiling the test-case in attached patch) follows from the
>>> fact
>>> that here in gen_parallel_loop the call to canonicalize_loop_ivs fails to
>>> "base all the induction variables in LOOP on a single control one":
>>> ...
>>>/* Base all the induction variables in LOOP on a single control one.*/
>>>canonicalize_loop_ivs (loop, &nit, true);
>>> ...
>>>
>>> This is caused by the fact that for one of the induction variables,
>>> simple_iv
>>> no longer returns true (as was the case at the start of
>>> gen_parallel_loop).
>>> This seems to be triggered by the fact that during loop_version we call
>>> scev_reset (although I'm not sure why when recalculating scev info we're
>>> not
>>> reaching the same conclusion as before).
>>
>> I guess the real reason is that canonicalize_loop_ivs calls
>> create_iv first which in the parloop case with bump-in-latch true
>> and then incrementally re-writes PHIs, eventually wrecking other
>> PHIs SCEV.  In this case the 2nd PHIs evolution depends on the
>> first one but the rewritten ones depend on the new IV already.
>>
>> So the better fix (maybe not 100% enough) would be to re-organize
>> canonicalize_loop_ivs to first do analysis of all PHIs and then
>> rewrite them all.
>>
>
> Focusing on the re-organize first, is this sort of what you had in mind?

FYI, we have multiple interfaces manipulating on IVs, sometimes in a similar
way.  An example would be create_canonical_iv in tree-ssa-loop-ivcanon.c.

One proposal for discussion is to refactor such interfaces and put them in a
single source file, like tree-ssa-loop-ivopts.  For example of
create_canonical_iv,
it can be removed by simply adding the corresponding candidate in IVOPTs.

Thanks,
bin
>
> Bootstrapped and reg-tested on x86_64.
>
> Thanks,
> - Tom


Re: Deprecate not defining NO_IMPLICIT_EXTERN_C

2018-03-21 Thread Nathan Sidwell

On 03/21/2018 10:18 AM, David Edelsohn wrote:

AIX still requires implicit extern C.

I previously have tried to disable the feature on AIX and it failed miserably.


That's unfortunate :(

nathan

--
Nathan Sidwell


[PATCH], PR target/84914, Fix complex long double multiply/divide on PowerPC -mabi=ieeelongdouble

2018-03-21 Thread Michael Meissner
Joseph Myers pointed out that we call the wrong function for complex long
double multiply and divide.  This patch changes the functions called from
__multc3/__divtc3 to __mulkc3/__divkc3.

I have built this on power8 systems, both a little endian system, and a big
endian system using --with-cpu=power8.  Both compilers built without error, and
had no regressions in the test suite.  Can I check this into the trunk?

[gcc]
2018-03-21  Michael Meissner  

PR target/84914
* config/rs6000/rs6000.c (create_complex_muldiv): New helper
function to create the function decl for complex long double
multiply and divide for -mabi=ieeelongdouble.
(init_float128_ieee): Call it.

[gcc/testsuite]
2018-03-21  Michael Meissner  

PR target/84914
* gcc.target/powerpc/mulkc-2.c: New tests to make sure complex
long double multiply/divide uses the correct function.
* gcc.target/powerpc/mulkc-3.c: Likewise.
* gcc.target/powerpc/divkc-2.c: Likewise.
* gcc.target/powerpc/divkc-3.c: Likewise.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797
Index: gcc/config/rs6000/rs6000.c
===
--- gcc/config/rs6000/rs6000.c  (revision 258601)
+++ gcc/config/rs6000/rs6000.c  (working copy)
@@ -18678,6 +18678,27 @@ init_float128_ibm (machine_mode mode)
 }
 }
 
+/* Create a decl for either complex long double multiply or complex long double
+   divide when long double is IEEE 128-bit floating point.  We can't use
+   __multc3 and __divtc3 because the original long double using IBM extended
+   double used those names.  The complex multiply/divide functions are encoded
+   as builtin functions with a complex result and 4 scalar inputs.  */
+
+static void
+create_complex_muldiv (const char *name, built_in_function fncode, tree fntype)
+{
+  tree fndecl = add_builtin_function (name, fntype, fncode, BUILT_IN_NORMAL,
+ name, NULL_TREE);
+
+  set_builtin_decl (fncode, fndecl, true);
+
+  if (TARGET_DEBUG_BUILTIN)
+fprintf (stderr, "create complex %s, fncode: %d, fndecl: 0x%lx\n", name,
+(int) fncode, (unsigned long)fndecl);
+
+  return;
+}
+
 /* Set up IEEE 128-bit floating point routines.  Use different names if the
arguments can be passed in a vector register.  The historical PowerPC
implementation of IEEE 128-bit floating point used _q_ for the names, so
@@ -18689,6 +18710,24 @@ init_float128_ieee (machine_mode mode)
 {
   if (FLOAT128_VECTOR_P (mode))
 {
+  /* Set up to call __mulkc3 and __divkc3 under -mabi=ieeelongdouble.  */
+ if (mode == TFmode && TARGET_IEEEQUAD)
+   {
+built_in_function fncode_mul =
+  (built_in_function)(BUILT_IN_COMPLEX_MUL_MIN + TCmode - 
MIN_MODE_COMPLEX_FLOAT);
+built_in_function fncode_div =
+  (built_in_function)(BUILT_IN_COMPLEX_DIV_MIN + TCmode - 
MIN_MODE_COMPLEX_FLOAT);
+tree fntype = build_function_type_list (complex_long_double_type_node,
+long_double_type_node,
+long_double_type_node,
+long_double_type_node,
+long_double_type_node,
+NULL_TREE);
+
+create_complex_muldiv ("__mulkc3", fncode_mul, fntype);
+create_complex_muldiv ("__divkc3", fncode_div, fntype);
+   }
+
   set_optab_libfunc (add_optab, mode, "__addkf3");
   set_optab_libfunc (sub_optab, mode, "__subkf3");
   set_optab_libfunc (neg_optab, mode, "__negkf2");
Index: gcc/testsuite/gcc.target/powerpc/mulkc3-3.c
===
--- gcc/testsuite/gcc.target/powerpc/mulkc3-3.c (revision 0)
+++ gcc/testsuite/gcc.target/powerpc/mulkc3-3.c (revision 0)
@@ -0,0 +1,16 @@
+/* { dg-do compile { target { powerpc64*-*-* } } } */
+/* { dg-require-effective-target powerpc_p9vector_ok } */
+/* { dg-options "-O2 -mpower8-vector -mabi=ibmlongdouble -Wno-psabi" } */
+
+/* Check that complex multiply generates the right call when long double is
+   IBM extended double floating point.  */
+
+typedef _Complex long double cld_t;
+
+void
+multiply (cld_t *p, cld_t *q, cld_t *r)
+{
+  *p = *q * *r;
+}
+
+/* { dg-final { scan-assembler "bl __multc3" } } */
Index: gcc/testsuite/gcc.target/powerpc/divkc3-3.c
===
--- gcc/testsuite/gcc.target/powerpc/divkc3-3.c (revision 0)
+++ gcc/testsuite/gcc.target/powerpc/divkc3-3.c (revision 0)
@@ -0,0 +1,16 @@
+/* { dg-do compile { target { powerpc64*-*-* } } } */
+/* { dg-require-effective-target powerpc_p9vector_ok } */
+/* { dg-options "-O2 -mpower8-vector -mabi=ibmlongdouble -Wno-psabi" } */
+
+/*

Re: [parloops, PR83126], Use cached affine_ivs canonicalize_loop_ivs

2018-03-21 Thread Richard Biener
On Wed, 21 Mar 2018, Tom de Vries wrote:

> On 03/12/2018 01:14 PM, Richard Biener wrote:
> > On Thu, 22 Feb 2018, Tom de Vries wrote:
> > 
> > > Hi,
> > > 
> > > this patch fixes an ICE in the parloops pass.
> > > 
> > > The ICE (when compiling the test-case in attached patch) follows from the
> > > fact
> > > that here in gen_parallel_loop the call to canonicalize_loop_ivs fails to
> > > "base all the induction variables in LOOP on a single control one":
> > > ...
> > >/* Base all the induction variables in LOOP on a single control one.*/
> > >canonicalize_loop_ivs (loop, &nit, true);
> > > ...
> > > 
> > > This is caused by the fact that for one of the induction variables,
> > > simple_iv
> > > no longer returns true (as was the case at the start of
> > > gen_parallel_loop).
> > > This seems to be triggered by the fact that during loop_version we call
> > > scev_reset (although I'm not sure why when recalculating scev info we're
> > > not
> > > reaching the same conclusion as before).
> > I guess the real reason is that canonicalize_loop_ivs calls
> > create_iv first which in the parloop case with bump-in-latch true
> > and then incrementally re-writes PHIs, eventually wrecking other
> > PHIs SCEV.  In this case the 2nd PHIs evolution depends on the
> > first one but the rewritten ones depend on the new IV already.
> > 
> > So the better fix (maybe not 100% enough) would be to re-organize
> > canonicalize_loop_ivs to first do analysis of all PHIs and then
> > rewrite them all.
> > 
> 
> Focusing on the re-organize first, is this sort of what you had in mind?

Yes, though not sure why you need to have a hash-map here, just pushing
to a vector of PHIs would work, no?  Or alternatively a bitmap
of SSA versions so you have a way to match the gphi iterator with
the set of IVs.  But the vector should be sorted in PHI order so
just traversing the PHIs looking for the "next" PHI in the vector
would work I guess.

Does it help with the original issue btw?

Richard.

> Bootstrapped and reg-tested on x86_64.
> 
> Thanks,
> - Tom
> 

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


Re: [PR c++/84789] do not resolve typename into template-independent

2018-03-21 Thread Jason Merrill
On Tue, Mar 20, 2018 at 11:27 PM, Alexandre Oliva  wrote:
> On Mar 20, 2018, Jason Merrill  wrote:
>
>> On Tue, Mar 20, 2018 at 6:07 PM, Alexandre Oliva  wrote:
>>> On Mar 20, 2018, Jason Merrill  wrote:
 that doesn't mean it's wrong to peek.
>
>>> Huh?  We're referencing members of an unrelated template that AFAIK
>>> needs not even be defined at that point, and even if it is, it could
>>> still be specialized afterwards.  How can it possibly be right to
>>> short-circuit such nested names?
>
>>> template struct B : A {}; // would /*: A {}*/ make any diff?
>>> template struct C : B // would /* : B*/ make any diff?
>>> {
>>>   B::A::I::I i; // { dg-error "typename" }
>>> };
>
>> No, we look inside when we're trying to parse the qualified-id as the
>> name of a declaration
>
> Yeah, but that's not the case at hand.  I guess we're miscommunicating.
> I understood you were saying it was ok to peek in this case.  Would you
> please state, for clarity, what your stance is on peeking in this case,
> specifically, in the B::A::I::I within the definition of C above?

It's OK when we're tentatively trying to parse it as a declarator, not
when we're trying to parse it as a type-specifier.

We seem to be talking past each other on this point: We already don't
peek when parsing a type-specifier, we only call resolve_typename_type
with false for only_current_p in the context of trying to parse a
declarator.  The parser tries both interpretations for B::A::I::I.
The type interpretation is ill-formed because of missing 'typename',
so it tries again as a declarator, and therefore tries to resolve the
TYPENAME_TYPE that it built while trying for a type, and trips the
assert.

>> in a declarator-id we can look into uninstantiated classes, otherwise
>> there would be no way to define members of class templates.
>
>>   void X::N::f() { } // looks inside X
>
> Of course, but then, we wouldn't get to a template-independent member.
> A member of a template-dependent name is still template-dependent.  It's
> only when a qualified name maps to an external name that it might become
> template-independent, and when this happens, I believe the qualified-id
> is not one that can be used to define a member. Specifically:
>
> struct K { struct L { static double j; }; };
> template  struct M { struct N { static int i; }; };
> template  struct O { typedef M P; typedef K Q; };
> template  int O::P::N::i = 42;
> template  double O::Q::L::j = 42.0;
>
> if we remap O::P to M and O::Q to K, how will we realize the
> given type-ids are not appropriate?  Where will the template parmlist
> belong when the qualified-id is taken as equivalent to K::L::j?

Agreed, these look bizarre because the template parm is used in O,
which is not an enclosing class of i or j.  And j is clearly
ill-formed because it declares a non-template as a template.

But it's not clear to me that i is ill-formed; we allow

struct A { static int i; };
struct B { typedef ::A A; };
int B::A::i = 0;

and the i example seems analogous.  I wouldn't argue against making it
ill-formed, but I don't see a rule that would make it so in the
current draft standard.  And even resolving the scope to be
non-dependent doesn't necessarily mean the declaration will be, if the
template parameter list ends up being for a member template:

struct K { struct L { template  static void f(T); }; };
template  struct O { typedef K Q; };
template  void O::Q::L::f(T) { }

 I disagree; it seems to me that the assert should allow the case where
 the scope was originally dependent, but got resolved earlier in the
 function.
>>>
>>> Doesn't the function always take dependent scopes?  I for some reason
>>> thought that was the case.
>
>> Yes, as the comment says, a TYPENAME_TYPE should always have a
>> dependent scope.  In this case, the dependent scope was "typename
>> B::A", but just above we resolved it to just A, making it no longer
>> dependent.
>
> Then you got me thoroughly confused: what did you mean by 'the assert
> should allow' a case that's a given?

asserts are often used to verify that things we think of as givens are
actually true.  :)

I suppose we could remove the assert, but I'd probably move it up
above where we start messing with 'scope'.

Jason


Re: [og7] vector_length extension part 4: target hooks and automatic parallelism

2018-03-21 Thread Tom de Vries

On 03/02/2018 08:18 PM, Cesar Philippidis wrote:


og7-vl-part4-hooks.diff



diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c
index 5642941c6a3..507c8671704 100644
--- a/gcc/config/nvptx/nvptx.c
+++ b/gcc/config/nvptx/nvptx.c
@@ -5205,14 +5205,36 @@ nvptx_simt_vf ()
return PTX_WARP_SIZE;
  }
  
+#define NVPTX_GOACC_VL_WARP "nvptx vl warp"

+
+/* Return true of the offloaded function needs a vector_length of
+   PTX_WARP_SIZE.  */
+
+static bool
+nvptx_goacc_needs_vl_warp ()
+{
+  tree attr = lookup_attribute (NVPTX_GOACC_VL_WARP,
+   DECL_ATTRIBUTES (current_function_decl));
+  return attr == NULL_TREE;
+}
+


I just wrote an example using "#pragma acc parallel vector_length (128)" 
and looked at the generated code. I found that the actual vector_length 
was still 32. I tracked this back to this function returning true.


I think we need "return attr != NULL_TREE".

Thanks,
- Tom



[PR c++/84804] ICE with default arg, template friend & lambda

2018-03-21 Thread Nathan Sidwell
When instantiating a template we end up with a rather strange binding 
hierarchy -- the innermost binding is the template and the next one is 
the instantiation.  Anyway, the upshot is we end up trying to push a 
tsubst'd lambda into the template itself and die because that's marked 
as a complete type.  All somewhat suspicious.


However, there as a 'scope == ts_lambda' escape on that check, and I'd 
removed it because it seemed unneeded.  Sadly not.  As we now never pass 
ts_lambda into do_pushtag, we have to inspect the type being pushed to 
determine this check.  This gets us back to the older behaviour, but at 
least we push into the instantiation, not the template because of 
changes I made earlier.


nathan
--
Nathan Sidwell
2018-03-21  Nathan Sidwell  

	PR c++/84804
	* name-lookup.c (do_pushtag): Permit lambdas to be pushed into
	complete classes.


	PR c++/84804
	* g++.dg/lookup/pr84804.C: New.

Index: cp/name-lookup.c
===
--- cp/name-lookup.c	(revision 258716)
+++ cp/name-lookup.c	(working copy)
@@ -6436,7 +6436,8 @@ do_pushtag (tree name, tree type, tag_sc
 
   if (b->kind == sk_class)
 	{
-	  if (!TYPE_BEING_DEFINED (current_class_type))
+	  if (!TYPE_BEING_DEFINED (current_class_type)
+	  && !LAMBDA_TYPE_P (type))
 	return error_mark_node;
 
 	  if (!PROCESSING_REAL_TEMPLATE_DECL_P ())
Index: testsuite/g++.dg/lookup/pr84804.C
===
--- testsuite/g++.dg/lookup/pr84804.C	(revision 0)
+++ testsuite/g++.dg/lookup/pr84804.C	(working copy)
@@ -0,0 +1,12 @@
+// { dg-do compile { target c++11 } }
+// PR c++/84804 ICE instantiating friend with default arg containing a lambda
+template struct A
+{
+  // Note, instantiation injects this into ::, so there can only be one!
+  friend void foo(int i = []{ return 0;}()) {}
+};
+
+void bar()
+{
+  A<0> x;
+}


Re: [C++ PATCH] Fix FIX_TRUNC_EXPR instantiation (PR c++/84942)

2018-03-21 Thread Jason Merrill
On Wed, Mar 21, 2018 at 4:42 AM, Jakub Jelinek  wrote:
> On Tue, Mar 20, 2018 at 05:00:34PM -0400, Jason Merrill wrote:
>> On Mon, Mar 19, 2018 at 3:50 PM, Jakub Jelinek  wrote:
>> > +int a(__attribute__((b((int)__builtin_inf() * 1ULL / auto;
>>
>> This seems like another situation like 84610 and 84642 that Alex sent
>> a patch for, of 'auto' in an attribute wrongly being treated as an
>> implicit template parameter.  As I suggested in response to his patch,
>> we ought to disable auto_is_implicit_function_parm_p within an
>> attribute-specifier.
>
> Ok, for this specific testcase I'll wait for Alex.  That doesn't mean
> the FIX_TRUNC_EXPR handling isn't completely wrong though.
> So, shall we fix it anyway, or remove or just gcc_unreachable ()?
> It was added in PR51150 (it couldn't work even at that point,
> cp_build_unary_op would use wrong return type even back then), but the
> testcase never invokes this anymore, uses CAST_EXPR, STATIC_CAST_EXPR etc.
> instead.

Hmm, I think remove.  It should then hit the gcc_unreachable() in tsubst_copy.

Jason


Re: [PATCH][ARM] Fix test pr82989.c for big endian and mthumb

2018-03-21 Thread Christophe Lyon
On 21 March 2018 at 13:11, Sudakshina Das  wrote:
> Hi
>
> The test pr82989.c which was added in one of previous commits is failing
> for mthumb and big-endian configurations. The aim of this test was to
> check that NEON instructions are not being used for simple shift
> operations. The scanning of lsl and lsr instructions and checking its
> counts were just too restrictive for different configurations. So I
> have now simplified the test to only check for the absence of NEON
> instructions.
>
> Testing: Only test case change so only tested the said test on
> differently configured toolchain.
> @Christophe can you confirm this patch fixes the failure for you?
>

Yes, the validations are now OK my side.

Thanks

Christophe

> Thanks
> Sudi
>
> *** gcc/testsuite/ChangeLog ***
>
> 2018-03-21  Sudakshina Das  
>
> PR target/82989
> * gcc.target/arm/pr82989.c: Change dg-scan-assembly directives.


[PATCH v2] C++: show private field accessor hints with -g and optimization (PR c++/84994)

2018-03-21 Thread David Malcolm
On Tue, 2018-03-20 at 21:51 -0400, Jason Merrill wrote:
> On Tue, Mar 20, 2018 at 7:37 PM, David Malcolm 
> wrote:
> > PR c++/84894 reports that the fix-it hints suggesting accessor
> > calls for
> > private fields doesn't work with -g for -O1 and above.
> > 
> > The issue is that field_accessor_p requires DECL_SAVED_TREE (fn) to
> > be
> > a RETURN_EXPR, but the former is a STATEMENT_LIST, created in
> > start_preparsed_function here:
> 
> Would constexpr_fn_retval be useful here?

Aha!  Indeed it is.  Thanks; this simplifies things considerably.

Here's an updated patch which uses constexpr_fn_retval, along with an
indentation fix and a little more test coverage.

Successfully bootstrapped and regression-tested on x86_64-pc-linux-gnu;
adds 486 PASS results to g++.sum (mostly due to the tests moving to
gcc+.dg/torture).

OK for trunk?

gcc/cp/ChangeLog:
PR c++/84994
* constexpr.c (constexpr_fn_retval): Make non-"static".
* cp-tree.h (constexpr_fn_retval): New decl.
* search.c (direct_accessor_p): Update leading comment.
(reference_accessor_p): Likewise.
(field_accessor_p): Replace check that function body is a
RETURN_EXPR with a call to constexpr_fn_retval.  Fix
indentation of "field_type" decl.

gcc/testsuite/ChangeLog:
PR c++/84994
* g++.dg/other/accessor-fixits-1.C: Move to...
* g++.dg/torture/accessor-fixits-1.C: ...here.
* g++.dg/other/accessor-fixits-2.C: Move to...
* g++.dg/torture/accessor-fixits-2.C: ...here.
* g++.dg/other/accessor-fixits-3.C: Move to...
* g++.dg/torture/accessor-fixits-3.C: ...here.
* g++.dg/other/accessor-fixits-4.C: Move to...
* g++.dg/torture/accessor-fixits-4.C: ...here.
* g++.dg/other/accessor-fixits-5.C: Move to...
* g++.dg/torture/accessor-fixits-5.C: ...here.
* g++.dg/torture/accessor-fixits-6.C: New testcase.
* g++.dg/torture/accessor-fixits-7.C: New testcase.
* g++.dg/torture/accessor-fixits-8.C: New testcase.
---
 gcc/cp/constexpr.c   |   2 +-
 gcc/cp/cp-tree.h |   1 +
 gcc/cp/search.c  |  21 ++-
 gcc/testsuite/g++.dg/other/accessor-fixits-1.C   | 222 ---
 gcc/testsuite/g++.dg/other/accessor-fixits-2.C   | 104 ---
 gcc/testsuite/g++.dg/other/accessor-fixits-3.C   |  15 --
 gcc/testsuite/g++.dg/other/accessor-fixits-4.C   |  48 -
 gcc/testsuite/g++.dg/other/accessor-fixits-5.C   |  33 
 gcc/testsuite/g++.dg/torture/accessor-fixits-1.C | 222 +++
 gcc/testsuite/g++.dg/torture/accessor-fixits-2.C | 104 +++
 gcc/testsuite/g++.dg/torture/accessor-fixits-3.C |  15 ++
 gcc/testsuite/g++.dg/torture/accessor-fixits-4.C |  48 +
 gcc/testsuite/g++.dg/torture/accessor-fixits-5.C |  33 
 gcc/testsuite/g++.dg/torture/accessor-fixits-6.C |  22 +++
 gcc/testsuite/g++.dg/torture/accessor-fixits-7.C |  22 +++
 gcc/testsuite/g++.dg/torture/accessor-fixits-8.C |  31 
 16 files changed, 510 insertions(+), 433 deletions(-)
 delete mode 100644 gcc/testsuite/g++.dg/other/accessor-fixits-1.C
 delete mode 100644 gcc/testsuite/g++.dg/other/accessor-fixits-2.C
 delete mode 100644 gcc/testsuite/g++.dg/other/accessor-fixits-3.C
 delete mode 100644 gcc/testsuite/g++.dg/other/accessor-fixits-4.C
 delete mode 100644 gcc/testsuite/g++.dg/other/accessor-fixits-5.C
 create mode 100644 gcc/testsuite/g++.dg/torture/accessor-fixits-1.C
 create mode 100644 gcc/testsuite/g++.dg/torture/accessor-fixits-2.C
 create mode 100644 gcc/testsuite/g++.dg/torture/accessor-fixits-3.C
 create mode 100644 gcc/testsuite/g++.dg/torture/accessor-fixits-4.C
 create mode 100644 gcc/testsuite/g++.dg/torture/accessor-fixits-5.C
 create mode 100644 gcc/testsuite/g++.dg/torture/accessor-fixits-6.C
 create mode 100644 gcc/testsuite/g++.dg/torture/accessor-fixits-7.C
 create mode 100644 gcc/testsuite/g++.dg/torture/accessor-fixits-8.C

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 941562e..02bfb8e 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -657,7 +657,7 @@ get_function_named_in_call (tree t)
return value if suitable, error_mark_node for a statement not allowed in
a constexpr function, or NULL_TREE if no return value was found.  */
 
-static tree
+tree
 constexpr_fn_retval (tree body)
 {
   switch (TREE_CODE (body))
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 17d8c6d..2293394 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7400,6 +7400,7 @@ extern bool literal_type_p  (tree);
 extern tree register_constexpr_fundef   (tree, tree);
 extern bool is_valid_constexpr_fn  (tree, bool);
 extern bool check_constexpr_ctor_body   (tree, tree, bool);
+extern tree constexpr_fn_retval(tree);
 extern tree ensure_literal_type_for_constexpr_object (tree);
 extern bool potential_constant_expre

Re: [C++ PATCH] Fix ICE with inline asm and MODIFY_EXPR/preinc/predec in output operand (PR c++/84961)

2018-03-21 Thread Jason Merrill
On Wed, Mar 21, 2018 at 6:26 AM, Jakub Jelinek  wrote:
> On Tue, Mar 20, 2018 at 05:58:43PM -0400, Jason Merrill wrote:
>> On Tue, Mar 20, 2018 at 5:04 PM, Jakub Jelinek  wrote:
>> > --- gcc/cp/semantics.c.jj   2018-03-20 11:58:17.069356145 +0100
>> > +++ gcc/cp/semantics.c  2018-03-20 21:56:43.745292245 +0100
>> > @@ -1512,6 +1512,26 @@ finish_asm_stmt (int volatile_p, tree st
>> >   && C_TYPE_FIELDS_READONLY (TREE_TYPE (operand)
>> > cxx_readonly_error (operand, lv_asm);
>> >
>> > + tree *op = &operand;
>> > + while (TREE_CODE (*op) == COMPOUND_EXPR)
>> > +   op = &TREE_OPERAND (*op, 1);
>> > + switch (TREE_CODE (*op))
>> > +   {
>> > +   case PREINCREMENT_EXPR:
>> > +   case PREDECREMENT_EXPR:
>> > +   case MODIFY_EXPR:
>> > + if (TREE_SIDE_EFFECTS (TREE_OPERAND (*op, 0)))
>> > +   *op = build2 (TREE_CODE (*op), TREE_TYPE (*op),
>> > + cp_stabilize_reference (TREE_OPERAND (*op, 
>> > 0)),
>> > + TREE_OPERAND (*op, 1));
>> > + *op = build2 (COMPOUND_EXPR, TREE_TYPE (*op), *op,
>> > +   TREE_OPERAND (*op, 0));
>> > + op = &TREE_OPERAND (*op, 1);
>> > + break;
>> > +   default:
>> > + break;
>> > +   }
>>
>> Hmm, it would be nice to share this with the similar patterns in
>> unary_complex_lvalue and cp_build_modify_expr.

> You mean just outline the
>   if (TREE_SIDE_EFFECTS (TREE_OPERAND (lhs, 0)))
> lhs = build2 (TREE_CODE (lhs), TREE_TYPE (lhs),
>   cp_stabilize_reference (TREE_OPERAND (lhs, 0)),
>   TREE_OPERAND (lhs, 1));
>   lhs = build2 (COMPOUND_EXPR, lhstype, lhs, TREE_OPERAND (lhs, 0));
> into lhs = some_function (lhs); and use that in finish_asm_stmt,
> unary_complex_lvalue and cp_build_modify_expr in these spots?

> I really have no idea how to commonize anything else, e.g. the COMPOUND_EXPR
> handling is substantially different between the 3 functions.

> Any suggestion for the some_function name if you want that?

Sure, that's something.  How about genericize_compound_lvalue?

Jason


Re: [PATCH][ARM] Fix test pr82989.c for big endian and mthumb

2018-03-21 Thread Kyrill Tkachov


On 21/03/18 16:33, Christophe Lyon wrote:

On 21 March 2018 at 13:11, Sudakshina Das  wrote:

Hi

The test pr82989.c which was added in one of previous commits is failing
for mthumb and big-endian configurations. The aim of this test was to
check that NEON instructions are not being used for simple shift
operations. The scanning of lsl and lsr instructions and checking its
counts were just too restrictive for different configurations. So I
have now simplified the test to only check for the absence of NEON
instructions.

Testing: Only test case change so only tested the said test on
differently configured toolchain.
@Christophe can you confirm this patch fixes the failure for you?


Yes, the validations are now OK my side.


Thanks, the patch is ok for trunk.

Kyrill


Thanks

Christophe


Thanks
Sudi

*** gcc/testsuite/ChangeLog ***

2018-03-21  Sudakshina Das  

 PR target/82989
 * gcc.target/arm/pr82989.c: Change dg-scan-assembly directives.




Re: [PATCH v2] C++: show private field accessor hints with -g and optimization (PR c++/84994)

2018-03-21 Thread Jason Merrill
OK.

On Wed, Mar 21, 2018 at 12:52 PM, David Malcolm  wrote:
> On Tue, 2018-03-20 at 21:51 -0400, Jason Merrill wrote:
>> On Tue, Mar 20, 2018 at 7:37 PM, David Malcolm 
>> wrote:
>> > PR c++/84894 reports that the fix-it hints suggesting accessor
>> > calls for
>> > private fields doesn't work with -g for -O1 and above.
>> >
>> > The issue is that field_accessor_p requires DECL_SAVED_TREE (fn) to
>> > be
>> > a RETURN_EXPR, but the former is a STATEMENT_LIST, created in
>> > start_preparsed_function here:
>>
>> Would constexpr_fn_retval be useful here?
>
> Aha!  Indeed it is.  Thanks; this simplifies things considerably.
>
> Here's an updated patch which uses constexpr_fn_retval, along with an
> indentation fix and a little more test coverage.
>
> Successfully bootstrapped and regression-tested on x86_64-pc-linux-gnu;
> adds 486 PASS results to g++.sum (mostly due to the tests moving to
> gcc+.dg/torture).
>
> OK for trunk?
>
> gcc/cp/ChangeLog:
> PR c++/84994
> * constexpr.c (constexpr_fn_retval): Make non-"static".
> * cp-tree.h (constexpr_fn_retval): New decl.
> * search.c (direct_accessor_p): Update leading comment.
> (reference_accessor_p): Likewise.
> (field_accessor_p): Replace check that function body is a
> RETURN_EXPR with a call to constexpr_fn_retval.  Fix
> indentation of "field_type" decl.
>
> gcc/testsuite/ChangeLog:
> PR c++/84994
> * g++.dg/other/accessor-fixits-1.C: Move to...
> * g++.dg/torture/accessor-fixits-1.C: ...here.
> * g++.dg/other/accessor-fixits-2.C: Move to...
> * g++.dg/torture/accessor-fixits-2.C: ...here.
> * g++.dg/other/accessor-fixits-3.C: Move to...
> * g++.dg/torture/accessor-fixits-3.C: ...here.
> * g++.dg/other/accessor-fixits-4.C: Move to...
> * g++.dg/torture/accessor-fixits-4.C: ...here.
> * g++.dg/other/accessor-fixits-5.C: Move to...
> * g++.dg/torture/accessor-fixits-5.C: ...here.
> * g++.dg/torture/accessor-fixits-6.C: New testcase.
> * g++.dg/torture/accessor-fixits-7.C: New testcase.
> * g++.dg/torture/accessor-fixits-8.C: New testcase.
> ---
>  gcc/cp/constexpr.c   |   2 +-
>  gcc/cp/cp-tree.h |   1 +
>  gcc/cp/search.c  |  21 ++-
>  gcc/testsuite/g++.dg/other/accessor-fixits-1.C   | 222 
> ---
>  gcc/testsuite/g++.dg/other/accessor-fixits-2.C   | 104 ---
>  gcc/testsuite/g++.dg/other/accessor-fixits-3.C   |  15 --
>  gcc/testsuite/g++.dg/other/accessor-fixits-4.C   |  48 -
>  gcc/testsuite/g++.dg/other/accessor-fixits-5.C   |  33 
>  gcc/testsuite/g++.dg/torture/accessor-fixits-1.C | 222 
> +++
>  gcc/testsuite/g++.dg/torture/accessor-fixits-2.C | 104 +++
>  gcc/testsuite/g++.dg/torture/accessor-fixits-3.C |  15 ++
>  gcc/testsuite/g++.dg/torture/accessor-fixits-4.C |  48 +
>  gcc/testsuite/g++.dg/torture/accessor-fixits-5.C |  33 
>  gcc/testsuite/g++.dg/torture/accessor-fixits-6.C |  22 +++
>  gcc/testsuite/g++.dg/torture/accessor-fixits-7.C |  22 +++
>  gcc/testsuite/g++.dg/torture/accessor-fixits-8.C |  31 
>  16 files changed, 510 insertions(+), 433 deletions(-)
>  delete mode 100644 gcc/testsuite/g++.dg/other/accessor-fixits-1.C
>  delete mode 100644 gcc/testsuite/g++.dg/other/accessor-fixits-2.C
>  delete mode 100644 gcc/testsuite/g++.dg/other/accessor-fixits-3.C
>  delete mode 100644 gcc/testsuite/g++.dg/other/accessor-fixits-4.C
>  delete mode 100644 gcc/testsuite/g++.dg/other/accessor-fixits-5.C
>  create mode 100644 gcc/testsuite/g++.dg/torture/accessor-fixits-1.C
>  create mode 100644 gcc/testsuite/g++.dg/torture/accessor-fixits-2.C
>  create mode 100644 gcc/testsuite/g++.dg/torture/accessor-fixits-3.C
>  create mode 100644 gcc/testsuite/g++.dg/torture/accessor-fixits-4.C
>  create mode 100644 gcc/testsuite/g++.dg/torture/accessor-fixits-5.C
>  create mode 100644 gcc/testsuite/g++.dg/torture/accessor-fixits-6.C
>  create mode 100644 gcc/testsuite/g++.dg/torture/accessor-fixits-7.C
>  create mode 100644 gcc/testsuite/g++.dg/torture/accessor-fixits-8.C
>
> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> index 941562e..02bfb8e 100644
> --- a/gcc/cp/constexpr.c
> +++ b/gcc/cp/constexpr.c
> @@ -657,7 +657,7 @@ get_function_named_in_call (tree t)
> return value if suitable, error_mark_node for a statement not allowed in
> a constexpr function, or NULL_TREE if no return value was found.  */
>
> -static tree
> +tree
>  constexpr_fn_retval (tree body)
>  {
>switch (TREE_CODE (body))
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 17d8c6d..2293394 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -7400,6 +7400,7 @@ extern bool literal_type_p  (tree);
>  extern tree register_constexpr_fundef   (tree, tree);
>  extern bool is_valid_constexpr_fn

Re: Remove long-dead deprecation documentation

2018-03-21 Thread Sandra Loosemore

On 03/21/2018 05:11 AM, Nathan Sidwell wrote:

Sandra,
this removes the now-deleted deprecated features. AFAICT these have been 
gone for a long time -- 2008 is the most recent changelog entry -- so no 
need to put it in any release notes.


ok?


Yes, this is fine.  At some point we might want to do more cleanup here, 
but getting rid of obsolete stuff means there's less left to clean up.  :-)


-Sandra


Re: [og7] vector_length extension part 2: Generalize state propagation and synchronization

2018-03-21 Thread Tom de Vries

On 03/02/2018 05:55 PM, Cesar Philippidis wrote:

In addition, nvptx_cta_sync and the corresponding nvptx_barsync insn,
have been extended to take a barrier ID and a thread count. The idea
here is to assign one barrier for each logical vector. Worker-single
synchronization is controlled by barrier 0. Therefore, the vector
barrier ID is set to tid.y+1 (because there's one vector unit per
worker) in nvptx_init_oacc_workers and placed into a register stored in
cfun->machine->sync_bar. If no workers are present, then the barrier ID
falls back to 0.


I compiled a worker loop before and after the patch series, and observed 
this change:

...
@@ -70,7 +71,7 @@
  $L2:
   // joining 2;
  $L5:
-  bar.sync 1;
+  bar.sync 0;
   // join 2;
   ret;
 }
...

AFAICT from your explanation above, that change is intentional.

Changing the code generation scheme for workers is fine, but obviously 
that should be a minimal, separate patch that we can bisect back to.


Thanks,
- Tom


Re: [PATCH][ARM] Fix test pr82989.c for big endian and mthumb

2018-03-21 Thread Sudakshina Das

Hi

On 21/03/18 17:03, Kyrill Tkachov wrote:


On 21/03/18 16:33, Christophe Lyon wrote:

On 21 March 2018 at 13:11, Sudakshina Das  wrote:

Hi

The test pr82989.c which was added in one of previous commits is failing
for mthumb and big-endian configurations. The aim of this test was to
check that NEON instructions are not being used for simple shift
operations. The scanning of lsl and lsr instructions and checking its
counts were just too restrictive for different configurations. So I
have now simplified the test to only check for the absence of NEON
instructions.

Testing: Only test case change so only tested the said test on
differently configured toolchain.
@Christophe can you confirm this patch fixes the failure for you?


Yes, the validations are now OK my side.


Thanks, the patch is ok for trunk.


Thanks Christophe for validating and Kyrill for the Ok.
Committed to trunk as r258723.

Thanks
Sudi



Kyrill


Thanks

Christophe


Thanks
Sudi

*** gcc/testsuite/ChangeLog ***

2018-03-21  Sudakshina Das  

 PR target/82989
 * gcc.target/arm/pr82989.c: Change dg-scan-assembly directives.






Re: Desire to allocate bit in DT_PARM bitmask for DEC FORMAT compatibility purposes

2018-03-21 Thread Jakub Jelinek
On Tue, Mar 20, 2018 at 12:41:25PM -0600, Jeff Law wrote:
> This is documented in the old manuals from DEC and I've found
> essentially the same documentation in Oracle/Sun's current documentation
> as well as old MIPS documentation.   I have a high degree of confidence
> it exists in IBM's Fortran compilers as well.  In contrast Intel & PCG's
> Fortran compilers do not seem to support this extension.

My testing disagrees with the last one, at least ifort 17.0.4 handles
the default widths like this patch does with -fdec.

Jakub


Re: Desire to allocate bit in DT_PARM bitmask for DEC FORMAT compatibility purposes

2018-03-21 Thread Jeff Law
On 03/21/2018 11:25 AM, Jakub Jelinek wrote:
> On Tue, Mar 20, 2018 at 12:41:25PM -0600, Jeff Law wrote:
>> This is documented in the old manuals from DEC and I've found
>> essentially the same documentation in Oracle/Sun's current documentation
>> as well as old MIPS documentation.   I have a high degree of confidence
>> it exists in IBM's Fortran compilers as well.  In contrast Intel & PCG's
>> Fortran compilers do not seem to support this extension.
> 
> My testing disagrees with the last one, at least ifort 17.0.4 handles
> the default widths like this patch does with -fdec.
I was going from what I could find in the online documentation.  I could
have missed it, or it could be undocumented behavior.  Similarly for
Portland Group's compiler -- I'm going on documented behavior that I
could find online.

I don't think it materially changes things though.
jeff


[PATCH][ARM][PR target/84826] Fix ICE in extract_insn, at recog.c:2304 on arm-linux-gnueabi

2018-03-21 Thread Sudakshina Das

Hi

The ICE in the bug report was happening because the macro
USE_RETURN_INSN (FALSE) was returning different values at different
points in the compilation. This was internally occurring because the
function arm_compute_static_chain_stack_bytes () which was dependent on
arm_r3_live_at_start_p () was giving a different value after the
cond_exec instructions were created in ce3 causing the liveness of r3
to escape up to the start block.

The function arm_compute_static_chain_stack_bytes () should really
only compute the value once during epilogue/prologue stage. This pass
introduces a new member 'static_chain_stack_bytes' to the target
definition of the struct machine_function which gets calculated in
expand_prologue and is the value that is returned by
arm_compute_static_chain_stack_bytes () beyond that.

Testing done: Bootstrapped and regtested on arm-none-linux-gnueabihf
and added the reported test to the testsuite.

Is this ok for trunk?

Sudi


ChangeLog entries:

*** gcc/ChangeLog ***

2018-03-21  Sudakshina Das  

PR target/84826
* config/arm/arm.h (machine_function): Add
static_chain_stack_bytes.
* config/arm/arm.c (arm_compute_static_chain_stack_bytes):
Avoid re-computing once computed.
(arm_expand_prologue): Compute machine->static_chain_stack_bytes.
(arm_init_machine_status): Initialize
machine->static_chain_stack_bytes.

*** gcc/testsuite/ChangeLog ***

2018-03-21  Sudakshina Das  

PR target/84826
* gcc.target/arm/pr84826.c: New test
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index bbf3937..2809112 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -1384,6 +1384,9 @@ typedef struct GTY(()) machine_function
   machine_mode thumb1_cc_mode;
   /* Set to 1 after arm_reorg has started.  */
   int after_arm_reorg;
+  /* The number of bytes used to store the static chain register on the
+ stack, above the stack frame.  */
+  int static_chain_stack_bytes;
 }
 machine_function;
 #endif
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index cb6ab81..bc31810 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -19392,6 +19392,11 @@ arm_r3_live_at_start_p (void)
 static int
 arm_compute_static_chain_stack_bytes (void)
 {
+  /* Once the value is updated from the init value of -1, do not
+ re-compute.  */
+  if (cfun->machine->static_chain_stack_bytes != -1)
+return cfun->machine->static_chain_stack_bytes;
+
   /* See the defining assertion in arm_expand_prologue.  */
   if (IS_NESTED (arm_current_func_type ())
   && ((TARGET_APCS_FRAME && frame_pointer_needed && TARGET_ARM)
@@ -21699,6 +21704,11 @@ arm_expand_prologue (void)
   emit_insn (gen_movsi (stack_pointer_rtx, r1));
 }
 
+  /* Let's compute the static_chain_stack_bytes required and store it.  Right
+ now the value must the -1 as stored by arm_init_machine_status ().  */
+  cfun->machine->static_chain_stack_bytes
+= arm_compute_static_chain_stack_bytes ();
+
   /* The static chain register is the same as the IP register.  If it is
  clobbered when creating the frame, we need to save and restore it.  */
   clobber_ip = IS_NESTED (func_type)
@@ -24875,6 +24885,7 @@ arm_init_machine_status (void)
 #if ARM_FT_UNKNOWN != 0
   machine->func_type = ARM_FT_UNKNOWN;
 #endif
+  machine->static_chain_stack_bytes = -1;
   return machine;
 }
 
diff --git a/gcc/testsuite/gcc.target/arm/pr84826.c b/gcc/testsuite/gcc.target/arm/pr84826.c
new file mode 100644
index 000..c61c548
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr84826.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast -fstack-clash-protection" } */
+
+void d (void *);
+
+void a ()
+{
+  int b;
+  void bar (int c)
+  {
+if (__builtin_expect (c, 0))
+  ++b;
+  }
+  d (bar);
+}


Re: [PATCH] PR 84615 Regressions due to type mismatch with character functions

2018-03-21 Thread Steve Kargl
On Wed, Mar 21, 2018 at 10:39:06AM +0200, Janne Blomqvist wrote:
> Since the kind of the hidden character length variable is not part of
> the character variable definition, we must ensure that character
> lengths are always of the same kind in interfaces, regardless of how
> they were declared in the source. This patch ensures this when calling
> a procedure.
> 
> Regtested on x86_64-pc-linux-gnu and i686-pc-linux-gnu, Ok for trunk?
> 

OK.

-- 
Steve


Re: [PATCH, rs6000] Fix PR83789: __builtin_altivec_lvx fails for powerpc for altivec-4.c

2018-03-21 Thread Peter Bergner
On 3/20/18 12:27 PM, Peter Bergner wrote:
> On 3/20/18 11:42 AM, Segher Boessenkool wrote:
>> On Mon, Mar 19, 2018 at 09:12:08PM -0500, Peter Bergner wrote:
>>> Looking at mu build dirs insn-modes.h, I don't see HAVE_V8HFmode being
>>> defined on either my LE or BE builds.  What am I missing?
>>
>> Nothing, I just was confused (we always define the mode in rs6000-modes.def,
>> but that is not the same thing).  Sorry.
> 
> Ok then, patch committed with the expander change you requested and
> leaving the HAVE_V8HFmode code as is in the patch.  Thanks!

Kaushik confirmed this is broken on GCC 7 as well.  Ok to backport
this patch to GCC 7, assuming regtesting is clean?

I'll note that GCC 7 does not need any of the changes to rs6000-p8swap.c,
since that file doesn't exist and doesn't need to exist in GCC 7, so I've
left those changes out.

Peter





Re: [C++ Patch] PR 84972 ("[6/7/8 Regression] ICE: tree check: expected class 'type', have 'exceptional' (error_mark)...")

2018-03-21 Thread Jason Merrill
OK.

On Wed, Mar 21, 2018 at 5:28 AM, Paolo Carlini  wrote:
> Hi,
>
> as I told Marek in the audit trail, I'm working on a more complete patch
> consistently changing maybe_deduce_size_from_array_init for various error
> recovery issues (see the trail of c++/84632 too), but since this specific
> regression ultimately started with a previous change of mine and it's very
> simple I decided to send it now, for 8.1.0. Essentially, the idea is setting
> TREE_TYPE (decl) to error_mark_node - the normal error recovery mechanism in
> this function via cp_complete_array_type - also when the early
> check_array_designated_initializer fails.
>
> Tested x86_64-linux, Paolo.
>
> //
>


Re: [C++ PATCH] Fix ICE with inline asm and MODIFY_EXPR/preinc/predec in output operand (PR c++/84961, take 2)

2018-03-21 Thread Jason Merrill
OK.

On Wed, Mar 21, 2018 at 1:47 PM, Jakub Jelinek  wrote:
> On Wed, Mar 21, 2018 at 01:01:38PM -0400, Jason Merrill wrote:
>> >> Hmm, it would be nice to share this with the similar patterns in
>> >> unary_complex_lvalue and cp_build_modify_expr.
>>
>> > You mean just outline the
>> >   if (TREE_SIDE_EFFECTS (TREE_OPERAND (lhs, 0)))
>> > lhs = build2 (TREE_CODE (lhs), TREE_TYPE (lhs),
>> >   cp_stabilize_reference (TREE_OPERAND (lhs, 0)),
>> >   TREE_OPERAND (lhs, 1));
>> >   lhs = build2 (COMPOUND_EXPR, lhstype, lhs, TREE_OPERAND (lhs, 0));
>> > into lhs = some_function (lhs); and use that in finish_asm_stmt,
>> > unary_complex_lvalue and cp_build_modify_expr in these spots?
>>
>> > I really have no idea how to commonize anything else, e.g. the 
>> > COMPOUND_EXPR
>> > handling is substantially different between the 3 functions.
>>
>> > Any suggestion for the some_function name if you want that?
>>
>> Sure, that's something.  How about genericize_compound_lvalue?
>
> So like this (if it passes bootstrap/regtest)?
>
> 2018-03-21  Jakub Jelinek  
>
> PR c++/84961
> * cp-tree.h (genericize_compound_lvalue): Declare.
> * typeck.c (genericize_compound_lvalue): New function.
> (unary_complex_lvalue, cp_build_modify_expr): Use it.
> * semantics.c (finish_asm_stmt): Replace MODIFY_EXPR, 
> PREINCREMENT_EXPR
> and PREDECREMENT_EXPR in output and "m" constrained input operands 
> with
> COMPOUND_EXPR.  Call cxx_mark_addressable on the rightmost
> COMPOUND_EXPR operand.
>
> * c-c++-common/pr43690.c: Don't expect errors on "m" (--x) and
> "m" (++x) in C++.
> * g++.dg/torture/pr84961-1.C: New test.
> * g++.dg/torture/pr84961-2.C: New test.
>
> --- gcc/cp/cp-tree.h.jj 2018-03-20 22:05:57.053431471 +0100
> +++ gcc/cp/cp-tree.h2018-03-21 18:41:42.301838677 +0100
> @@ -7145,6 +7145,7 @@ extern tree cp_build_addressof
> (locati
>  extern tree cp_build_addr_expr (tree, tsubst_flags_t);
>  extern tree cp_build_unary_op   (enum tree_code, tree, bool,
>   tsubst_flags_t);
> +extern tree genericize_compound_lvalue (tree);
>  extern tree unary_complex_lvalue   (enum tree_code, tree);
>  extern tree build_x_conditional_expr   (location_t, tree, tree, tree,
>   tsubst_flags_t);
> --- gcc/cp/typeck.c.jj  2018-03-06 08:01:37.827883402 +0100
> +++ gcc/cp/typeck.c 2018-03-21 18:40:23.350862956 +0100
> @@ -6357,6 +6357,25 @@ build_unary_op (location_t /*location*/,
>return cp_build_unary_op (code, xarg, noconvert, tf_warning_or_error);
>  }
>
> +/* Adjust LVALUE, an MODIFY_EXPR, PREINCREMENT_EXPR or PREDECREMENT_EXPR,
> +   so that it is a valid lvalue even for GENERIC by replacing
> +   (lhs = rhs) with ((lhs = rhs), lhs)
> +   (--lhs) with ((--lhs), lhs)
> +   (++lhs) with ((++lhs), lhs)
> +   and if lhs has side-effects, calling cp_stabilize_reference on it, so
> +   that it can be evaluated multiple times.  */
> +
> +tree
> +genericize_compound_lvalue (tree lvalue)
> +{
> +  if (TREE_SIDE_EFFECTS (TREE_OPERAND (lvalue, 0)))
> +lvalue = build2 (TREE_CODE (lvalue), TREE_TYPE (lvalue),
> +cp_stabilize_reference (TREE_OPERAND (lvalue, 0)),
> +TREE_OPERAND (lvalue, 1));
> +  return build2 (COMPOUND_EXPR, TREE_TYPE (TREE_OPERAND (lvalue, 0)),
> +lvalue, TREE_OPERAND (lvalue, 0));
> +}
> +
>  /* Apply unary lvalue-demanding operator CODE to the expression ARG
> for certain kinds of expressions which are not really lvalues
> but which we can accept as lvalues.
> @@ -6391,17 +6410,7 @@ unary_complex_lvalue (enum tree_code cod
>if (TREE_CODE (arg) == MODIFY_EXPR
>|| TREE_CODE (arg) == PREINCREMENT_EXPR
>|| TREE_CODE (arg) == PREDECREMENT_EXPR)
> -{
> -  tree lvalue = TREE_OPERAND (arg, 0);
> -  if (TREE_SIDE_EFFECTS (lvalue))
> -   {
> - lvalue = cp_stabilize_reference (lvalue);
> - arg = build2 (TREE_CODE (arg), TREE_TYPE (arg),
> -   lvalue, TREE_OPERAND (arg, 1));
> -   }
> -  return unary_complex_lvalue
> -   (code, build2 (COMPOUND_EXPR, TREE_TYPE (lvalue), arg, lvalue));
> -}
> +return unary_complex_lvalue (code, genericize_compound_lvalue (arg));
>
>if (code != ADDR_EXPR)
>  return NULL_TREE;
> @@ -7887,11 +7896,7 @@ cp_build_modify_expr (location_t loc, tr
>  case PREINCREMENT_EXPR:
>if (compound_side_effects_p)
> newrhs = rhs = stabilize_expr (rhs, &preeval);
> -  if (TREE_SIDE_EFFECTS (TREE_OPERAND (lhs, 0)))
> -   lhs = build2 (TREE_CODE (lhs), TREE_TYPE (lhs),
> - cp_stabilize_reference (TREE_OPERAND (lhs, 0)),
> - TREE_OPERAND (lhs, 1));
> -  lhs = build2 (C

Re: [PATCH] Fix PR84512

2018-03-21 Thread Rainer Orth
Hi Eric,

>> So it failed before Toms original patch.  Please add sparc-solaris
>> to the list of XFAILed targets.
>
> SPARC/Linux is affected too so sparc*-*-* instead.

actually, it's sparc*-*-* && lp64 only.  Done like this after testing on
sparc-sun-solaris2.11 and i386-pc-solaris2.11.

Rainer

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


2018-03-21  Rainer Orth  

* gcc.dg/tree-ssa/pr84512.c: xfail on 64-bit SPARC.

# HG changeset patch
# Parent  50996d41bbbc78ab2cf0002ba6479559089a2337
xfail gcc.dg/tree-ssa/pr84512.c on 64-bit sparc

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr84512.c b/gcc/testsuite/gcc.dg/tree-ssa/pr84512.c
--- a/gcc/testsuite/gcc.dg/tree-ssa/pr84512.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr84512.c
@@ -13,4 +13,4 @@ int foo()
 }
 
 /* Target nvptx xfail due to PR84958.  */
-/* { dg-final { scan-tree-dump "return 285;" "optimized" { xfail nvptx*-*-* } } } */
+/* { dg-final { scan-tree-dump "return 285;" "optimized" { xfail { nvptx*-*-* || { sparc*-*-* && lp64 } } } } } */


Re: C++ PATCH to fix an ICE on invalid with OVERLOADs (PR c++/84854)

2018-03-21 Thread Marek Polacek
On Thu, Mar 15, 2018 at 08:55:59AM -0400, Jason Merrill wrote:
> On Thu, Mar 15, 2018 at 7:49 AM, Marek Polacek  wrote:
> > On Wed, Mar 14, 2018 at 02:06:36PM -0400, Jason Merrill wrote:
> >> On Wed, Mar 14, 2018 at 11:59 AM, Marek Polacek  wrote:
> >> > cxx_constant_value doesn't understand template codes, and neither it
> >> > understands OVERLOADs, so if we pass an OVERLOAD to it, we crash.  Here
> >> > instantiate_non_dependent_expr got an OVERLOAD, but since it calls
> >> > is_nondependent_constant_expression which checks type_unknown_p, it left 
> >> > the
> >> > expression as it was.  We can't use is_nondependent_constant_expression 
> >> > in
> >> > finish_if_stmt_cond because i_n_c_e checks is_constant_expression and 
> >> > that is
> >> > not suitable here; we'd miss diagnostics.  So I did the following; I 
> >> > think we
> >> > should reject the testcase with an error.
> >> >
> >> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> >> >
> >> > 2018-03-14  Marek Polacek  
> >> >
> >> > PR c++/84854
> >> > * semantics.c (finish_if_stmt_cond): Give error if the condition
> >> > is an overloaded function with no contextual type information.
> >> >
> >> > * g++.dg/cpp1z/constexpr-if15.C: New test.
> >> >
> >> > diff --git gcc/cp/semantics.c gcc/cp/semantics.c
> >> > index fdf37bea770..a056e9445e9 100644
> >> > --- gcc/cp/semantics.c
> >> > +++ gcc/cp/semantics.c
> >> > @@ -735,8 +735,16 @@ finish_if_stmt_cond (tree cond, tree if_stmt)
> >> >&& require_constant_expression (cond)
> >> >&& !value_dependent_expression_p (cond))
> >> >  {
> >> > -  cond = instantiate_non_dependent_expr (cond);
> >> > -  cond = cxx_constant_value (cond, NULL_TREE);
> >> > +  if (type_unknown_p (cond))
> >> > +   {
> >> > + cxx_incomplete_type_error (cond, TREE_TYPE (cond));
> >> > + cond = error_mark_node;
> >>
> >> I think I'd prefer to skip this block when type_unknown_p, and leave
> >> error handling up to the code shared with regular if.
> >
> > Like this?
> 
> Yes, thanks.
> 
> > It was my first version, but I thought we wanted the error;
> 
> Getting an error is an improvement, but it should apply to
> non-constexpr if as well, so checking in maybe_convert_cond would be
> better.  Actually, if we deal with unknown type there, we shouldn't
> need this patch at all.
> 
> ...except I also notice that since maybe_convert_cond doesn't do any
> conversion in a template, we're trying to extract the constant value
> without first converting to bool, which breaks this testcase:
> 
> struct A
> {
>   constexpr operator bool () { return true; }
>   int i;
> };
> 
> A a;
> 
> template  void f()
> {
>   constexpr bool b = a;  // works
>   if constexpr (a) { }   // breaks
> }
> 
> int main()
> {
>   f();
> }
> 
> A simple fix would be to replace your type_unknown_p check here with a
> comparison to boolean_type_node, so we wait until instantiation time
> to require a constant value.

Ok, that works.

We should also make g++ accept the testcase with "static_assert(a)" instead of
"if constexpr (a) { }" probably.  I guess the cxx_constant_value call in
finish_static_assert should happen earlier?

> Better would be to actually do the conversion.  Perhaps this could
> share code (for converting and getting a constant value) with
> finish_static_assert.

But this I didn't manage to make to work.  If I call 
perform_implicit_conversion_flags
in maybe_convert_cond, I get
error: cannot resolve overloaded function ‘foo’ based on conversion to type 
‘bool’
so I'm not sure how the conversion would help.

Anyway, here's at least the boolean_type_node version.

Bootstrapped/regtested on x86_64-linux.

2018-03-21  Marek Polacek  

PR c++/84854
* semantics.c (finish_if_stmt_cond): Check if the type of the condition
is boolean.
(finish_static_assert): Remove redundant variable.

* g++.dg/cpp1z/constexpr-if15.C: New test.
* g++.dg/cpp1z/constexpr-if16.C: New test.

diff --git gcc/cp/semantics.c gcc/cp/semantics.c
index fdf37bea770..39455c0168d 100644
--- gcc/cp/semantics.c
+++ gcc/cp/semantics.c
@@ -733,7 +733,10 @@ finish_if_stmt_cond (tree cond, tree if_stmt)
   if (IF_STMT_CONSTEXPR_P (if_stmt)
   && !type_dependent_expression_p (cond)
   && require_constant_expression (cond)
-  && !value_dependent_expression_p (cond))
+  && !value_dependent_expression_p (cond)
+  /* Wait until instantiation time, since only then COND has been
+converted to bool.  */
+  && TREE_TYPE (cond) == boolean_type_node)
 {
   cond = instantiate_non_dependent_expr (cond);
   cond = cxx_constant_value (cond, NULL_TREE);
@@ -8619,8 +8622,6 @@ void
 finish_static_assert (tree condition, tree message, location_t location, 
   bool member_p)
 {
-  tsubst_flags_t complain = tf_warning_or_error;
-
   if (message == NULL_TREE
   || message == error_mark_node
   || conditio

Re: Desire to allocate bit in DT_PARM bitmask for DEC FORMAT compatibility purposes

2018-03-21 Thread Janne Blomqvist
On Wed, Mar 21, 2018 at 7:29 PM, Jeff Law  wrote:
> On 03/21/2018 11:25 AM, Jakub Jelinek wrote:
>> On Tue, Mar 20, 2018 at 12:41:25PM -0600, Jeff Law wrote:
>>> This is documented in the old manuals from DEC and I've found
>>> essentially the same documentation in Oracle/Sun's current documentation
>>> as well as old MIPS documentation.   I have a high degree of confidence
>>> it exists in IBM's Fortran compilers as well.  In contrast Intel & PCG's
>>> Fortran compilers do not seem to support this extension.
>>
>> My testing disagrees with the last one, at least ifort 17.0.4 handles
>> the default widths like this patch does with -fdec.
> I was going from what I could find in the online documentation.  I could
> have missed it, or it could be undocumented behavior.

FWIW, IIRC the lineage of the Intel Fortran compiler frontend is that
at some point DEC did a Fortran compiler for Windows (Digital Visual
Fortran, DVF. Though I don't know to which extent the actual compiler
differed from the DEC Unix/VAX Fortran compilers, was it a port or a
completely different codebase?), which became Compaq Visual Fortran
(CVF). Then at some point Intel bought the product and the team which
developed it from Compaq (or maybe it was already HP at the time, I
don't remember), and it became Intel Fortran.

So if any current compiler would support the various DEC extensions,
it would be Intel. And if anything, I suppose Intel behavior defines
the "canonical" behavior for the DEC extensions.

 As for the implementation of the patch itself, you might want to
discuss it with Fritz Reese who has somewhat recently implemented the
other -fdec-* stuff. One thing which struck me was that you (or
whoever implemented it???) has reinvented setting the various default
widths, code which already exists in libgfortran in order to support
list directed output and zero width formats (zero width formats being,
roughly, the standardized version of the DEC Format extension).



-- 
Janne Blomqvist


Re: C++ PATCH to fix an ICE on invalid with OVERLOADs (PR c++/84854)

2018-03-21 Thread Jason Merrill
On Wed, Mar 21, 2018 at 2:37 PM, Marek Polacek  wrote:
> On Thu, Mar 15, 2018 at 08:55:59AM -0400, Jason Merrill wrote:
>> On Thu, Mar 15, 2018 at 7:49 AM, Marek Polacek  wrote:
>> > On Wed, Mar 14, 2018 at 02:06:36PM -0400, Jason Merrill wrote:
>> >> On Wed, Mar 14, 2018 at 11:59 AM, Marek Polacek  
>> >> wrote:
>> >> > cxx_constant_value doesn't understand template codes, and neither it
>> >> > understands OVERLOADs, so if we pass an OVERLOAD to it, we crash.  Here
>> >> > instantiate_non_dependent_expr got an OVERLOAD, but since it calls
>> >> > is_nondependent_constant_expression which checks type_unknown_p, it 
>> >> > left the
>> >> > expression as it was.  We can't use is_nondependent_constant_expression 
>> >> > in
>> >> > finish_if_stmt_cond because i_n_c_e checks is_constant_expression and 
>> >> > that is
>> >> > not suitable here; we'd miss diagnostics.  So I did the following; I 
>> >> > think we
>> >> > should reject the testcase with an error.
>> >> >
>> >> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
>> >> >
>> >> > 2018-03-14  Marek Polacek  
>> >> >
>> >> > PR c++/84854
>> >> > * semantics.c (finish_if_stmt_cond): Give error if the condition
>> >> > is an overloaded function with no contextual type information.
>> >> >
>> >> > * g++.dg/cpp1z/constexpr-if15.C: New test.
>> >> >
>> >> > diff --git gcc/cp/semantics.c gcc/cp/semantics.c
>> >> > index fdf37bea770..a056e9445e9 100644
>> >> > --- gcc/cp/semantics.c
>> >> > +++ gcc/cp/semantics.c
>> >> > @@ -735,8 +735,16 @@ finish_if_stmt_cond (tree cond, tree if_stmt)
>> >> >&& require_constant_expression (cond)
>> >> >&& !value_dependent_expression_p (cond))
>> >> >  {
>> >> > -  cond = instantiate_non_dependent_expr (cond);
>> >> > -  cond = cxx_constant_value (cond, NULL_TREE);
>> >> > +  if (type_unknown_p (cond))
>> >> > +   {
>> >> > + cxx_incomplete_type_error (cond, TREE_TYPE (cond));
>> >> > + cond = error_mark_node;
>> >>
>> >> I think I'd prefer to skip this block when type_unknown_p, and leave
>> >> error handling up to the code shared with regular if.
>> >
>> > Like this?
>>
>> Yes, thanks.
>>
>> > It was my first version, but I thought we wanted the error;
>>
>> Getting an error is an improvement, but it should apply to
>> non-constexpr if as well, so checking in maybe_convert_cond would be
>> better.  Actually, if we deal with unknown type there, we shouldn't
>> need this patch at all.
>>
>> ...except I also notice that since maybe_convert_cond doesn't do any
>> conversion in a template, we're trying to extract the constant value
>> without first converting to bool, which breaks this testcase:
>>
>> struct A
>> {
>>   constexpr operator bool () { return true; }
>>   int i;
>> };
>>
>> A a;
>>
>> template  void f()
>> {
>>   constexpr bool b = a;  // works
>>   if constexpr (a) { }   // breaks
>> }
>>
>> int main()
>> {
>>   f();
>> }
>>
>> A simple fix would be to replace your type_unknown_p check here with a
>> comparison to boolean_type_node, so we wait until instantiation time
>> to require a constant value.
>
> Ok, that works.
>
> We should also make g++ accept the testcase with "static_assert(a)" instead of
> "if constexpr (a) { }" probably.

> I guess the cxx_constant_value call in
> finish_static_assert should happen earlier?

fold_non_dependent_expr should already have gotten the constant value,
the call to cxx_constant_value is just for giving an error.

The bug seems to be that is_nondependent_constant_expression doesn't
realize that the conversion to bool is OK because it uses a constexpr
member function.

>> Better would be to actually do the conversion.  Perhaps this could
>> share code (for converting and getting a constant value) with
>> finish_static_assert.
>
> But this I didn't manage to make to work.  If I call 
> perform_implicit_conversion_flags
> in maybe_convert_cond, I get
> error: cannot resolve overloaded function ‘foo’ based on conversion to type 
> ‘bool’
> so I'm not sure how the conversion would help.

That looks like a good diagnostic to me, what's the problem?

> Anyway, here's at least the boolean_type_node version.
>
> Bootstrapped/regtested on x86_64-linux.
>
> 2018-03-21  Marek Polacek  
>
> PR c++/84854
> * semantics.c (finish_if_stmt_cond): Check if the type of the 
> condition
> is boolean.

OK.

> (finish_static_assert): Remove redundant variable.

But not this hunk; I like to be able to use the name "complain" even
when it isn't a parameter.

Jason


[PR c++/85008] ICE looking for clone

2018-03-21 Thread Nathan Sidwell
This ICE turned out to be a latent bug exposed by moving the member fns 
onto the FIELDS list.


We should be using DECL_CLONED_FUNCTION_P not DECL_CLONED_FUNCTION. 
Grepping showed another place (doing a similar linkage check) affected too.


nathan
--
Nathan Sidwell
2018-03-21  Nathan Sidwell  

	PR c++/85008
	* tree.c (decl_linkage): Use DECL_CLONED_FUNCTION_P.
	* decl2.c (vague_linkage_p): Likewise.

	PR c++/85008
	* g++.dg/pr85008.C: New.

Index: cp/decl2.c
===
--- cp/decl2.c	(revision 258722)
+++ cp/decl2.c	(working copy)
@@ -1940,7 +1940,7 @@ vague_linkage_p (tree decl)
   if ((DECL_MAYBE_IN_CHARGE_DESTRUCTOR_P (decl)
 	   || DECL_MAYBE_IN_CHARGE_CONSTRUCTOR_P (decl))
 	  && DECL_CHAIN (decl)
-	  && DECL_CLONED_FUNCTION (DECL_CHAIN (decl)))
+	  && DECL_CLONED_FUNCTION_P (DECL_CHAIN (decl)))
 	return vague_linkage_p (DECL_CHAIN (decl));
 
   gcc_checking_assert (!DECL_COMDAT (decl));
Index: cp/tree.c
===
--- cp/tree.c	(revision 258722)
+++ cp/tree.c	(working copy)
@@ -5022,7 +5022,7 @@ decl_linkage (tree decl)
   if ((DECL_MAYBE_IN_CHARGE_DESTRUCTOR_P (decl)
|| DECL_MAYBE_IN_CHARGE_CONSTRUCTOR_P (decl))
   && DECL_CHAIN (decl)
-  && DECL_CLONED_FUNCTION (DECL_CHAIN (decl)))
+  && DECL_CLONED_FUNCTION_P (DECL_CHAIN (decl)))
 return decl_linkage (DECL_CHAIN (decl));
 
   if (TREE_CODE (decl) == NAMESPACE_DECL)
Index: testsuite/g++.dg/pr85008.C
===
--- testsuite/g++.dg/pr85008.C	(revision 0)
+++ testsuite/g++.dg/pr85008.C	(working copy)
@@ -0,0 +1,8 @@
+// PR c++/85008 ICE concerning dtor clones
+
+void a() {
+  struct b {
+~b();
+int r [!!&b::~b]; // { dg-error "address of " }
+  };
+}


Re: Desire to allocate bit in DT_PARM bitmask for DEC FORMAT compatibility purposes

2018-03-21 Thread Jeff Law
On 03/21/2018 12:38 PM, Janne Blomqvist wrote:
> On Wed, Mar 21, 2018 at 7:29 PM, Jeff Law  wrote:
>> On 03/21/2018 11:25 AM, Jakub Jelinek wrote:
>>> On Tue, Mar 20, 2018 at 12:41:25PM -0600, Jeff Law wrote:
 This is documented in the old manuals from DEC and I've found
 essentially the same documentation in Oracle/Sun's current documentation
 as well as old MIPS documentation.   I have a high degree of confidence
 it exists in IBM's Fortran compilers as well.  In contrast Intel & PCG's
 Fortran compilers do not seem to support this extension.
>>>
>>> My testing disagrees with the last one, at least ifort 17.0.4 handles
>>> the default widths like this patch does with -fdec.
>> I was going from what I could find in the online documentation.  I could
>> have missed it, or it could be undocumented behavior.
> 
> FWIW, IIRC the lineage of the Intel Fortran compiler frontend is that
> at some point DEC did a Fortran compiler for Windows (Digital Visual
> Fortran, DVF. Though I don't know to which extent the actual compiler
> differed from the DEC Unix/VAX Fortran compilers, was it a port or a
> completely different codebase?), which became Compaq Visual Fortran
> (CVF). Then at some point Intel bought the product and the team which
> developed it from Compaq (or maybe it was already HP at the time, I
> don't remember), and it became Intel Fortran.
Thanks.

> 
> So if any current compiler would support the various DEC extensions,
> it would be Intel. And if anything, I suppose Intel behavior defines
> the "canonical" behavior for the DEC extensions.
> 
>  As for the implementation of the patch itself, you might want to
> discuss it with Fritz Reese who has somewhat recently implemented the
> other -fdec-* stuff. One thing which struck me was that you (or
> whoever implemented it???) has reinvented setting the various default
> widths, code which already exists in libgfortran in order to support
> list directed output and zero width formats (zero width formats being,
> roughly, the standardized version of the DEC Format extension).
Happy to ping Fritz on this stuff.  I've looked at some of his changes
while cleaning up the Codethink patches.

Also happy to look into sharing those bits rather than reinvention.

Thanks!

jeff


Re: C++ PATCH to fix an ICE on invalid with OVERLOADs (PR c++/84854)

2018-03-21 Thread Marek Polacek
On Wed, Mar 21, 2018 at 02:55:36PM -0400, Jason Merrill wrote:
> On Wed, Mar 21, 2018 at 2:37 PM, Marek Polacek  wrote:
> > On Thu, Mar 15, 2018 at 08:55:59AM -0400, Jason Merrill wrote:
> >> On Thu, Mar 15, 2018 at 7:49 AM, Marek Polacek  wrote:
> >> > On Wed, Mar 14, 2018 at 02:06:36PM -0400, Jason Merrill wrote:
> >> >> On Wed, Mar 14, 2018 at 11:59 AM, Marek Polacek  
> >> >> wrote:
> >> >> > cxx_constant_value doesn't understand template codes, and neither it
> >> >> > understands OVERLOADs, so if we pass an OVERLOAD to it, we crash.  
> >> >> > Here
> >> >> > instantiate_non_dependent_expr got an OVERLOAD, but since it calls
> >> >> > is_nondependent_constant_expression which checks type_unknown_p, it 
> >> >> > left the
> >> >> > expression as it was.  We can't use 
> >> >> > is_nondependent_constant_expression in
> >> >> > finish_if_stmt_cond because i_n_c_e checks is_constant_expression and 
> >> >> > that is
> >> >> > not suitable here; we'd miss diagnostics.  So I did the following; I 
> >> >> > think we
> >> >> > should reject the testcase with an error.
> >> >> >
> >> >> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> >> >> >
> >> >> > 2018-03-14  Marek Polacek  
> >> >> >
> >> >> > PR c++/84854
> >> >> > * semantics.c (finish_if_stmt_cond): Give error if the 
> >> >> > condition
> >> >> > is an overloaded function with no contextual type information.
> >> >> >
> >> >> > * g++.dg/cpp1z/constexpr-if15.C: New test.
> >> >> >
> >> >> > diff --git gcc/cp/semantics.c gcc/cp/semantics.c
> >> >> > index fdf37bea770..a056e9445e9 100644
> >> >> > --- gcc/cp/semantics.c
> >> >> > +++ gcc/cp/semantics.c
> >> >> > @@ -735,8 +735,16 @@ finish_if_stmt_cond (tree cond, tree if_stmt)
> >> >> >&& require_constant_expression (cond)
> >> >> >&& !value_dependent_expression_p (cond))
> >> >> >  {
> >> >> > -  cond = instantiate_non_dependent_expr (cond);
> >> >> > -  cond = cxx_constant_value (cond, NULL_TREE);
> >> >> > +  if (type_unknown_p (cond))
> >> >> > +   {
> >> >> > + cxx_incomplete_type_error (cond, TREE_TYPE (cond));
> >> >> > + cond = error_mark_node;
> >> >>
> >> >> I think I'd prefer to skip this block when type_unknown_p, and leave
> >> >> error handling up to the code shared with regular if.
> >> >
> >> > Like this?
> >>
> >> Yes, thanks.
> >>
> >> > It was my first version, but I thought we wanted the error;
> >>
> >> Getting an error is an improvement, but it should apply to
> >> non-constexpr if as well, so checking in maybe_convert_cond would be
> >> better.  Actually, if we deal with unknown type there, we shouldn't
> >> need this patch at all.
> >>
> >> ...except I also notice that since maybe_convert_cond doesn't do any
> >> conversion in a template, we're trying to extract the constant value
> >> without first converting to bool, which breaks this testcase:
> >>
> >> struct A
> >> {
> >>   constexpr operator bool () { return true; }
> >>   int i;
> >> };
> >>
> >> A a;
> >>
> >> template  void f()
> >> {
> >>   constexpr bool b = a;  // works
> >>   if constexpr (a) { }   // breaks
> >> }
> >>
> >> int main()
> >> {
> >>   f();
> >> }
> >>
> >> A simple fix would be to replace your type_unknown_p check here with a
> >> comparison to boolean_type_node, so we wait until instantiation time
> >> to require a constant value.
> >
> > Ok, that works.
> >
> > We should also make g++ accept the testcase with "static_assert(a)" instead 
> > of
> > "if constexpr (a) { }" probably.
> 
> > I guess the cxx_constant_value call in
> > finish_static_assert should happen earlier?
> 
> fold_non_dependent_expr should already have gotten the constant value,
> the call to cxx_constant_value is just for giving an error.

Oop, right.

> The bug seems to be that is_nondependent_constant_expression doesn't
> realize that the conversion to bool is OK because it uses a constexpr
> member function.

OK, I can look into this separately.

> >> Better would be to actually do the conversion.  Perhaps this could
> >> share code (for converting and getting a constant value) with
> >> finish_static_assert.
> >
> > But this I didn't manage to make to work.  If I call 
> > perform_implicit_conversion_flags
> > in maybe_convert_cond, I get
> > error: cannot resolve overloaded function ‘foo’ based on conversion to type 
> > ‘bool’
> > so I'm not sure how the conversion would help.
> 
> That looks like a good diagnostic to me, what's the problem?

Ugh, I got this wrong.  That diagnostic is fine, because we can reject
constexpr-if15.C, but with perform_implicit_conversion_flags we then
can't evaluate constexpr-if16.C:
error: the value of ‘a’ is not usable in a constant expression

> > Anyway, here's at least the boolean_type_node version.
> >
> > Bootstrapped/regtested on x86_64-linux.
> >
> > 2018-03-21  Marek Polacek  
> >
> > PR c++/84854
> > * semantics.c (finish_if_stmt_cond): Check if the type of the 
> > condition
>

Re: [og7] vector_length extension part 4: target hooks and automatic parallelism

2018-03-21 Thread Cesar Philippidis
On 03/21/2018 08:49 AM, Tom de Vries wrote:
> On 03/02/2018 08:18 PM, Cesar Philippidis wrote:
> 
>> og7-vl-part4-hooks.diff
> 
>> diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c
>> index 5642941c6a3..507c8671704 100644
>> --- a/gcc/config/nvptx/nvptx.c
>> +++ b/gcc/config/nvptx/nvptx.c
>> @@ -5205,14 +5205,36 @@ nvptx_simt_vf ()
>>     return PTX_WARP_SIZE;
>>   }
>>   +#define NVPTX_GOACC_VL_WARP "nvptx vl warp"
>> +
>> +/* Return true of the offloaded function needs a vector_length of
>> +   PTX_WARP_SIZE.  */
>> +
>> +static bool
>> +nvptx_goacc_needs_vl_warp ()
>> +{
>> +  tree attr = lookup_attribute (NVPTX_GOACC_VL_WARP,
>> +    DECL_ATTRIBUTES (current_function_decl));
>> +  return attr == NULL_TREE;
>> +}
>> +
> 
> I just wrote an example using "#pragma acc parallel vector_length (128)"
> and looked at the generated code. I found that the actual vector_length
> was still 32. I tracked this back to this function returning true.
> 
> I think we need "return attr != NULL_TREE".

Yes. Good catch. I've added another test case for this.

Thanks,
Cesar


[PATCH] Fix C FE ICE with vector comparison (PR c/84999)

2018-03-21 Thread Jakub Jelinek
Hi!

On ia32, we don't support __int128, nor mode (TI) integers, but we do
support 128-bit __float128 and (generic) vectors containing it.  The result
of a comparison of such vectors is supposed to be integral vector with the
same element size, but we really don't want to allow one in this case,
it would be a loophole for full support of __int128/mode(TI), just do:
typedef __float128 V __attribute__ ((__vector_size__ (2 * sizeof 
(__float128;
extern V v;
typedef typeof ((v != 0)[0]) my_int128;
and suddenly you have __int128.

The C++ errors out in this case, you can use such floating point vectors,
but can't use them in comparisons, the C FE instead just segfaulted.

Fixed thusly, making the C FE consistent with what C++ FE does,
bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2018-03-21  Jakub Jelinek  

PR c/84999
* c-typeck.c (build_binary_op): If c_common_type_for_size fails when
building vector comparison, diagnose it and return error_mark_node.

* c-c++-common/pr84999.c: New test.

--- gcc/c/c-typeck.c.jj 2018-03-15 08:37:11.640779243 +0100
+++ gcc/c/c-typeck.c2018-03-21 14:01:25.088183657 +0100
@@ -11504,6 +11504,13 @@ build_binary_op (location_t location, en
   intt = c_common_type_for_size (GET_MODE_BITSIZE
 (SCALAR_TYPE_MODE
  (TREE_TYPE (type0))), 0);
+ if (!intt)
+   {
+ error_at (location, "could not find an integer type "
+ "of the same size as %qT",
+   TREE_TYPE (type0));
+ return error_mark_node;
+   }
   result_type = build_opaque_vector_type (intt,
  TYPE_VECTOR_SUBPARTS (type0));
   converted = 1;
@@ -11665,6 +11672,13 @@ build_binary_op (location_t location, en
   intt = c_common_type_for_size (GET_MODE_BITSIZE
 (SCALAR_TYPE_MODE
  (TREE_TYPE (type0))), 0);
+ if (!intt)
+   {
+ error_at (location, "could not find an integer type "
+ "of the same size as %qT",
+   TREE_TYPE (type0));
+ return error_mark_node;
+   }
   result_type = build_opaque_vector_type (intt,
  TYPE_VECTOR_SUBPARTS (type0));
   converted = 1;
--- gcc/testsuite/c-c++-common/pr84999.c.jj 2018-03-21 14:34:12.819329771 
+0100
+++ gcc/testsuite/c-c++-common/pr84999.c2018-03-21 14:32:37.816361905 
+0100
@@ -0,0 +1,12 @@
+/* PR c/84999 */
+/* { dg-do compile { target { i?86-*-* x86_64-*-* } } } */
+/* { dg-options "" } */
+
+typedef __float128 V __attribute__ ((__vector_size__ (2 * sizeof 
(__float128;
+V a;
+typeof (a != 0) b; /* { dg-error "could not find an integer type of the 
same size as" "" { target ia32 } } */
+typeof (a == 0) c; /* { dg-error "could not find an integer type of the 
same size as" "" { target ia32 } } */
+typeof (a < 0) d;  /* { dg-error "could not find an integer type of the 
same size as" "" { target ia32 } } */
+typeof (a <= 0) e; /* { dg-error "could not find an integer type of the 
same size as" "" { target ia32 } } */
+typeof (a > 0) f;  /* { dg-error "could not find an integer type of the 
same size as" "" { target ia32 } } */
+typeof (a >= 0) g; /* { dg-error "could not find an integer type of the 
same size as" "" { target ia32 } } */

Jakub


[C++ PATCH] Fix ICE with inline asm and MODIFY_EXPR/preinc/predec in output operand (PR c++/84961, take 2)

2018-03-21 Thread Jakub Jelinek
On Wed, Mar 21, 2018 at 01:01:38PM -0400, Jason Merrill wrote:
> >> Hmm, it would be nice to share this with the similar patterns in
> >> unary_complex_lvalue and cp_build_modify_expr.
> 
> > You mean just outline the
> >   if (TREE_SIDE_EFFECTS (TREE_OPERAND (lhs, 0)))
> > lhs = build2 (TREE_CODE (lhs), TREE_TYPE (lhs),
> >   cp_stabilize_reference (TREE_OPERAND (lhs, 0)),
> >   TREE_OPERAND (lhs, 1));
> >   lhs = build2 (COMPOUND_EXPR, lhstype, lhs, TREE_OPERAND (lhs, 0));
> > into lhs = some_function (lhs); and use that in finish_asm_stmt,
> > unary_complex_lvalue and cp_build_modify_expr in these spots?
> 
> > I really have no idea how to commonize anything else, e.g. the COMPOUND_EXPR
> > handling is substantially different between the 3 functions.
> 
> > Any suggestion for the some_function name if you want that?
> 
> Sure, that's something.  How about genericize_compound_lvalue?

So like this (if it passes bootstrap/regtest)?

2018-03-21  Jakub Jelinek  

PR c++/84961
* cp-tree.h (genericize_compound_lvalue): Declare.
* typeck.c (genericize_compound_lvalue): New function.
(unary_complex_lvalue, cp_build_modify_expr): Use it.
* semantics.c (finish_asm_stmt): Replace MODIFY_EXPR, PREINCREMENT_EXPR
and PREDECREMENT_EXPR in output and "m" constrained input operands with
COMPOUND_EXPR.  Call cxx_mark_addressable on the rightmost
COMPOUND_EXPR operand.

* c-c++-common/pr43690.c: Don't expect errors on "m" (--x) and
"m" (++x) in C++.
* g++.dg/torture/pr84961-1.C: New test.
* g++.dg/torture/pr84961-2.C: New test.

--- gcc/cp/cp-tree.h.jj 2018-03-20 22:05:57.053431471 +0100
+++ gcc/cp/cp-tree.h2018-03-21 18:41:42.301838677 +0100
@@ -7145,6 +7145,7 @@ extern tree cp_build_addressof(locati
 extern tree cp_build_addr_expr (tree, tsubst_flags_t);
 extern tree cp_build_unary_op   (enum tree_code, tree, bool,
  tsubst_flags_t);
+extern tree genericize_compound_lvalue (tree);
 extern tree unary_complex_lvalue   (enum tree_code, tree);
 extern tree build_x_conditional_expr   (location_t, tree, tree, tree, 
  tsubst_flags_t);
--- gcc/cp/typeck.c.jj  2018-03-06 08:01:37.827883402 +0100
+++ gcc/cp/typeck.c 2018-03-21 18:40:23.350862956 +0100
@@ -6357,6 +6357,25 @@ build_unary_op (location_t /*location*/,
   return cp_build_unary_op (code, xarg, noconvert, tf_warning_or_error);
 }
 
+/* Adjust LVALUE, an MODIFY_EXPR, PREINCREMENT_EXPR or PREDECREMENT_EXPR,
+   so that it is a valid lvalue even for GENERIC by replacing
+   (lhs = rhs) with ((lhs = rhs), lhs)
+   (--lhs) with ((--lhs), lhs)
+   (++lhs) with ((++lhs), lhs)
+   and if lhs has side-effects, calling cp_stabilize_reference on it, so
+   that it can be evaluated multiple times.  */
+
+tree
+genericize_compound_lvalue (tree lvalue)
+{
+  if (TREE_SIDE_EFFECTS (TREE_OPERAND (lvalue, 0)))
+lvalue = build2 (TREE_CODE (lvalue), TREE_TYPE (lvalue),
+cp_stabilize_reference (TREE_OPERAND (lvalue, 0)),
+TREE_OPERAND (lvalue, 1));
+  return build2 (COMPOUND_EXPR, TREE_TYPE (TREE_OPERAND (lvalue, 0)),
+lvalue, TREE_OPERAND (lvalue, 0));
+}
+
 /* Apply unary lvalue-demanding operator CODE to the expression ARG
for certain kinds of expressions which are not really lvalues
but which we can accept as lvalues.
@@ -6391,17 +6410,7 @@ unary_complex_lvalue (enum tree_code cod
   if (TREE_CODE (arg) == MODIFY_EXPR
   || TREE_CODE (arg) == PREINCREMENT_EXPR
   || TREE_CODE (arg) == PREDECREMENT_EXPR)
-{
-  tree lvalue = TREE_OPERAND (arg, 0);
-  if (TREE_SIDE_EFFECTS (lvalue))
-   {
- lvalue = cp_stabilize_reference (lvalue);
- arg = build2 (TREE_CODE (arg), TREE_TYPE (arg),
-   lvalue, TREE_OPERAND (arg, 1));
-   }
-  return unary_complex_lvalue
-   (code, build2 (COMPOUND_EXPR, TREE_TYPE (lvalue), arg, lvalue));
-}
+return unary_complex_lvalue (code, genericize_compound_lvalue (arg));
 
   if (code != ADDR_EXPR)
 return NULL_TREE;
@@ -7887,11 +7896,7 @@ cp_build_modify_expr (location_t loc, tr
 case PREINCREMENT_EXPR:
   if (compound_side_effects_p)
newrhs = rhs = stabilize_expr (rhs, &preeval);
-  if (TREE_SIDE_EFFECTS (TREE_OPERAND (lhs, 0)))
-   lhs = build2 (TREE_CODE (lhs), TREE_TYPE (lhs),
- cp_stabilize_reference (TREE_OPERAND (lhs, 0)),
- TREE_OPERAND (lhs, 1));
-  lhs = build2 (COMPOUND_EXPR, lhstype, lhs, TREE_OPERAND (lhs, 0));
+  lhs = genericize_compound_lvalue (lhs);
 maybe_add_compound:
   /* If we had (bar, --foo) = 5; or (bar, (baz, --foo)) = 5;
 and looked through the COMPOUND_EXPRs, readd them no

[PATCH, fortran] PR84957 - [8 Regression] ICE in gfc_sym_type, at fortran/trans-types.c:2255

2018-03-21 Thread Harald Anlauf
The attached obvious patch fixes a NULL pointer dereference.
Testcase derived from report.  Changelogs below.

Regtested on i686-pc-linux-gnu.

Whoever reviews this, please feel free to commit.

Thanks,
Harald


2018-03-21  Harald Anlauf  

PR fortran/84957
* trans-types.c (gfc_sym_type): Do not dereference NULL pointer.


2018-03-21  Harald Anlauf  

PR fortran/84957
* gfortran.dg/pr84957.f90: New test.

Index: gcc/fortran/trans-types.c
===
--- gcc/fortran/trans-types.c   (revision 258740)
+++ gcc/fortran/trans-types.c   (working copy)
@@ -2252,6 +2252,7 @@
   && sym->ts.type == BT_CHARACTER
   && sym->ts.u.cl->backend_decl == NULL_TREE
   && sym->ns->proc_name
+  && sym->ns->proc_name->ts.u.cl
   && sym->ns->proc_name->ts.u.cl->backend_decl != NULL_TREE)
 sym->ts.u.cl->backend_decl = sym->ns->proc_name->ts.u.cl->backend_decl;
 
Index: gcc/testsuite/gfortran.dg/pr84957.f90
===
--- gcc/testsuite/gfortran.dg/pr84957.f90   (revision 0)
+++ gcc/testsuite/gfortran.dg/pr84957.f90   (revision 0)
@@ -0,0 +1,17 @@
+! { dg-do compile }
+! PR 84957
+!
+! Testcase derived from PR by G. Steinmetz  
+!
+function f() result(u)
+  entry g() result(v)
+contains
+  function v(x) result(z)
+character :: x(2)
+character(sum(len_trim(x))) :: z
+  end function v
+  function u(x) result(z)
+character :: x(2)
+character(sum(len_trim(x))) :: z
+  end function u
+end function f


Re: [PATCH] Fix ICE from path isolation (PR tree-optimization/84960)

2018-03-21 Thread Richard Biener
On March 21, 2018 9:24:06 PM GMT+01:00, Jakub Jelinek  wrote:
>Hi!
>
>On the following testcase, path isolation decides to duplicate a bb and
>redirect edge from ENTRY bb to its successor to this duplicate bb and
>tree cleanup then removes all other basic blocks as unreachable.
>
>When blocks are removed, forced labels are moved to their bb->prev_bb
>block,
>but inserting any stmts into the ENTRY bb is of course invalid.
>
>This patch makes sure we insert it into the ENTRY successor instead in
>that
>case.  Bootstrapped/regtested on x86_64-linux and i686-linux, ok for
>trunk?

OK. 

Richard. 

>2018-03-21  Jakub Jelinek  
>
>   PR tree-optimization/84960
>   * tree-cfg.c (remove_bb): Don't move forced labels into bb->prev_bb
>   if it is ENTRY block, move them into single succ of ENTRY in that
>case.
>
>   * gcc.c-torture/compile/pr84960.c: New test.
>
>--- gcc/tree-cfg.c.jj  2018-02-09 06:44:38.445804410 +0100
>+++ gcc/tree-cfg.c 2018-03-21 12:43:57.835337795 +0100
>@@ -2301,6 +2301,12 @@ remove_bb (basic_block bb)
>   }
> 
> new_bb = bb->prev_bb;
>+/* Don't move any labels into ENTRY block.  */
>+if (new_bb == ENTRY_BLOCK_PTR_FOR_FN (cfun))
>+  {
>+new_bb = single_succ (new_bb);
>+gcc_assert (new_bb != bb);
>+  }
> new_gsi = gsi_start_bb (new_bb);
> gsi_remove (&i, false);
> gsi_insert_before (&new_gsi, stmt, GSI_NEW_STMT);
>--- gcc/testsuite/gcc.c-torture/compile/pr84960.c.jj   2018-03-21
>12:49:15.299278319 +0100
>+++ gcc/testsuite/gcc.c-torture/compile/pr84960.c  2018-03-21
>12:49:04.245280389 +0100
>@@ -0,0 +1,17 @@
>+/* PR tree-optimization/84960 */
>+/* { dg-do compile { target indirect_jumps } } */
>+
>+void
>+foo (unsigned int a, float b, void *c)
>+{
>+lab:
>+  if ((b - (a %= 0) < 1U) * -1U)
>+;
>+  else
>+{
>+  unsigned int f = a;
>+  __builtin_unreachable ();
>+  c = &&lab;
>+}
>+  goto *c;
>+}
>
>   Jakub



Re: [PATCH, fortran] PR84957 - [8 Regression] ICE in gfc_sym_type, at fortran/trans-types.c:2255

2018-03-21 Thread Thomas Koenig

Hi Harald,


The attached obvious patch fixes a NULL pointer dereference.
Testcase derived from report.  Changelogs below.

Regtested on i686-pc-linux-gnu.

Whoever reviews this, please feel free to commit.


Reviewed and committed as r258745.

Thanks for the patch!

Regards

Thomas


Re: Deprecate not defining NO_IMPLICIT_EXTERN_C

2018-03-21 Thread Joseph Myers
On Wed, 21 Mar 2018, Nathan Sidwell wrote:

> Unfortunately it's a negative-sense define, that now at least most ports
> define.  Do all ports define it?  It's hard to determine that, because many
> ports get it set via config/gnu-user.h or similar common file.

Bare-metal ports often fail to define it.

As far as I know, newlib headers at least do not require implicit extern 
C.  If there are any that are missing explicit extern C, I'd rather that 
was addressed by (a) fixing them and (b) adding fixes in fixincludes if we 
wish to work with older header versions.

My list from 2005 of targets not defining NO_IMPLICIT_EXTERN_C then is 
.

I think this should become a target hook, where the default is 
implicit_extern_c returning false and only AIX (and any other OSes 
requiring it) sets the hook to return true.

> It also has the following undocumented features (when not set):

Whether these are needed would depend on the properties of headers for 
OSes setting the hook to return true.  Even if needed, one might consider 
fixing such header issues with fixincludes as a replacement for those 
features in the compiler.

-- 
Joseph S. Myers
jos...@codesourcery.com


Re: [PATCH] Fix C FE ICE with vector comparison (PR c/84999)

2018-03-21 Thread Joseph Myers
On Wed, 21 Mar 2018, Jakub Jelinek wrote:

> Hi!
> 
> On ia32, we don't support __int128, nor mode (TI) integers, but we do
> support 128-bit __float128 and (generic) vectors containing it.  The result
> of a comparison of such vectors is supposed to be integral vector with the
> same element size, but we really don't want to allow one in this case,
> it would be a loophole for full support of __int128/mode(TI), just do:
> typedef __float128 V __attribute__ ((__vector_size__ (2 * sizeof 
> (__float128;
> extern V v;
> typedef typeof ((v != 0)[0]) my_int128;
> and suddenly you have __int128.
> 
> The C++ errors out in this case, you can use such floating point vectors,
> but can't use them in comparisons, the C FE instead just segfaulted.
> 
> Fixed thusly, making the C FE consistent with what C++ FE does,
> bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

-- 
Joseph S. Myers
jos...@codesourcery.com


[PATCH] Fix ICE from path isolation (PR tree-optimization/84960)

2018-03-21 Thread Jakub Jelinek
Hi!

On the following testcase, path isolation decides to duplicate a bb and
redirect edge from ENTRY bb to its successor to this duplicate bb and
tree cleanup then removes all other basic blocks as unreachable.

When blocks are removed, forced labels are moved to their bb->prev_bb block,
but inserting any stmts into the ENTRY bb is of course invalid.

This patch makes sure we insert it into the ENTRY successor instead in that
case.  Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2018-03-21  Jakub Jelinek  

PR tree-optimization/84960
* tree-cfg.c (remove_bb): Don't move forced labels into bb->prev_bb
if it is ENTRY block, move them into single succ of ENTRY in that case.

* gcc.c-torture/compile/pr84960.c: New test.

--- gcc/tree-cfg.c.jj   2018-02-09 06:44:38.445804410 +0100
+++ gcc/tree-cfg.c  2018-03-21 12:43:57.835337795 +0100
@@ -2301,6 +2301,12 @@ remove_bb (basic_block bb)
}
 
  new_bb = bb->prev_bb;
+ /* Don't move any labels into ENTRY block.  */
+ if (new_bb == ENTRY_BLOCK_PTR_FOR_FN (cfun))
+   {
+ new_bb = single_succ (new_bb);
+ gcc_assert (new_bb != bb);
+   }
  new_gsi = gsi_start_bb (new_bb);
  gsi_remove (&i, false);
  gsi_insert_before (&new_gsi, stmt, GSI_NEW_STMT);
--- gcc/testsuite/gcc.c-torture/compile/pr84960.c.jj2018-03-21 
12:49:15.299278319 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr84960.c   2018-03-21 
12:49:04.245280389 +0100
@@ -0,0 +1,17 @@
+/* PR tree-optimization/84960 */
+/* { dg-do compile { target indirect_jumps } } */
+
+void
+foo (unsigned int a, float b, void *c)
+{
+lab:
+  if ((b - (a %= 0) < 1U) * -1U)
+;
+  else
+{
+  unsigned int f = a;
+  __builtin_unreachable ();
+  c = &&lab;
+}
+  goto *c;
+}

Jakub


Re: [PATCH] [PR c++/84610,84642] recover from implicit template parms gracefully

2018-03-21 Thread Alexandre Oliva
On Mar 20, 2018, Jason Merrill  wrote:

> On Tue, Mar 20, 2018 at 5:57 PM, Alexandre Oliva  wrote:
>> On Mar 20, 2018, Jason Merrill  wrote:
> Let's put this in cp-tree.h, with warning_sentinel.

>> +  (void)cleanup;

> There are lots of RAII variables without this explicit cast to void,
> and they don't seem to have been a problem; let's drop it here as
> well.

Done, here's what passed regstrap on i686- and x86_64-linux-gnu last
night.  Ok to install?

Disable auto_is_implicit_function_template_parm_p while parsing attributes

for  gcc/cp/ChangeLog

PR c++/84610
PR c++/84642
* cp-tree.h (temp_override): New template class, generalizing
a cleanup that was only used...
* parser.c (cp_parser_parameter_declaration_clause):
... here for auto_is_implicit_function_template_parm_p.
(cp_parser_gnu_attributes_opt): Use it here as well.
(cp_parser_std_attribute): Likewise.
---
 gcc/cp/cp-tree.h |   19 +++
 gcc/cp/parser.c  |   18 --
 2 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index c07aaa5781ac..c8f4bc43fa3c 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -1657,6 +1657,25 @@ struct warning_sentinel
   ~warning_sentinel() { flag = val; }
 };
 
+/* RAII sentinel that saves the value of a variable, optionally
+   overrides it right away, and restores its value when the sentinel
+   id destructed.  */
+
+template 
+class temp_override
+{
+  T& overridden_variable;
+  T saved_value;
+public:
+  temp_override(T& var) : overridden_variable (var), saved_value (var) {}
+  temp_override(T& var, T overrider)
+: overridden_variable (var), saved_value (var)
+  {
+overridden_variable = overrider;
+  }
+  ~temp_override() { overridden_variable = saved_value; }
+};
+
 /* The cached class binding level, from the most recently exited
class, or NULL if none.  */
 
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 6dcfae125b7b..34619293120b 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -21196,16 +21196,8 @@ cp_parser_parameter_declaration_clause (cp_parser* 
parser)
   bool ellipsis_p;
   bool is_error;
 
-  struct cleanup {
-cp_parser* parser;
-int auto_is_implicit_function_template_parm_p;
-~cleanup() {
-  parser->auto_is_implicit_function_template_parm_p
-   = auto_is_implicit_function_template_parm_p;
-}
-  } cleanup = { parser, parser->auto_is_implicit_function_template_parm_p };
-
-  (void) cleanup;
+  temp_override cleanup
+(parser->auto_is_implicit_function_template_parm_p);
 
   if (!processing_specialization
   && !processing_template_parmlist
@@ -24968,6 +24960,9 @@ cp_parser_gnu_attributes_opt (cp_parser* parser)
 {
   tree attributes = NULL_TREE;
 
+  temp_override cleanup
+(parser->auto_is_implicit_function_template_parm_p, false);
+
   while (true)
 {
   cp_token *token;
@@ -25159,6 +25154,9 @@ cp_parser_std_attribute (cp_parser *parser, tree 
attr_ns)
   tree attribute, attr_id = NULL_TREE, arguments;
   cp_token *token;
 
+  temp_override cleanup
+(parser->auto_is_implicit_function_template_parm_p, false);
+
   /* First, parse name of the attribute, a.k.a attribute-token.  */
 
   token = cp_lexer_peek_token (parser->lexer);


-- 
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer


Re: [PR c++/84729] convert new init to array elt type

2018-03-21 Thread Alexandre Oliva
On Mar 20, 2018, Jason Merrill  wrote:

>> -permerror (input_location,
>> -  "parenthesized initializer in array new");
>> +error_at (input_location,
>> + "parenthesized initializer in array new");

> I suspect you'll need to make the return unconditional to avoid the
> ICE; OK either way.

I didn't, but I had to adjust 3 preexisting testcases that relied on
this extension.  Last night I still had them with dg-do run, and that
didn't work because compilation fails, so now I've adjusted them to
compile only, and will retest.  While at that, I simplified
  error_at (input_location, "...
to
  error ("...

I've also updated the patch description and the ChangeLog entry, which I
had failed to do before posting the previous version of the patch.

I looked for this extension in gcc/doc/extend.texi, to remove it, but I
couldn't find it; is it really not there?

Ok to install if regstrap succeeds?


[PR c++/84729] reject parenthesized array init

A parenthesized initializer was only accepted when new()ing an array in
permissive mode.  We were not careful, however, to convert the
TREE_LIST initializer to the array element type in this extension.

Instead of fixing it, converting the initializer to the base type
after turning the TREE_LIST initializer to a compound_expr, we disable
this deprecated extension.


for  gcc/cp/ChangeLog

PR c++/84729
* init.c (build_vec_init): Error at parenthesized array init.

for  gcc/testsuite/ChangeLog

PR c++/84729
* g++.dg/pr84729.C: New.
* g++.old-deja/g++.ext/arrnew2.C: Require error.
* g++.old-deja/g++.robertl/eb58.C: Likewise.
* g++.old-deja/g++.robertl/eb63.C: Likewise.
---
 gcc/cp/init.c |7 ++-
 gcc/testsuite/g++.dg/pr84729.C|7 +++
 gcc/testsuite/g++.old-deja/g++.ext/arrnew2.C  |4 ++--
 gcc/testsuite/g++.old-deja/g++.robertl/eb58.C |4 ++--
 gcc/testsuite/g++.old-deja/g++.robertl/eb63.C |4 ++--
 5 files changed, 15 insertions(+), 11 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/pr84729.C

diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 15cee17c780c..2263d12563cd 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -3370,11 +3370,8 @@ build_new_1 (vec **placement, tree type, 
tree nelts,
  else if (*init)
 {
   if (complain & tf_error)
-permerror (input_location,
-  "parenthesized initializer in array new");
-  else
-return error_mark_node;
- vecinit = build_tree_list_vec (*init);
+error ("parenthesized initializer in array new");
+ return error_mark_node;
 }
  init_expr
= build_vec_init (data_addr,
diff --git a/gcc/testsuite/g++.dg/pr84729.C b/gcc/testsuite/g++.dg/pr84729.C
new file mode 100644
index ..e5d689e0460c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr84729.C
@@ -0,0 +1,7 @@
+// { dg-do compile }
+// { dg-options "-fpermissive" }
+
+typedef int b[2];
+void a() {
+  new b(a); // { dg-error "parenthesized initializer in array new" }
+}
diff --git a/gcc/testsuite/g++.old-deja/g++.ext/arrnew2.C 
b/gcc/testsuite/g++.old-deja/g++.ext/arrnew2.C
index c6a967ccc385..aff6b9c7c63b 100644
--- a/gcc/testsuite/g++.old-deja/g++.ext/arrnew2.C
+++ b/gcc/testsuite/g++.old-deja/g++.ext/arrnew2.C
@@ -1,7 +1,7 @@
-// { dg-do run }
+// { dg-do compile }
 // { dg-options "-w -fpermissive" }
 
-int *foo = new int[1](42); // { dg-bogus "" }
+int *foo = new int[1](42); // { dg-error "parenthesized" }
 int main ()
 {
   return foo[0] != 42;
diff --git a/gcc/testsuite/g++.old-deja/g++.robertl/eb58.C 
b/gcc/testsuite/g++.old-deja/g++.robertl/eb58.C
index 04ec92a30afc..d702296bdc78 100644
--- a/gcc/testsuite/g++.old-deja/g++.robertl/eb58.C
+++ b/gcc/testsuite/g++.old-deja/g++.robertl/eb58.C
@@ -1,4 +1,4 @@
-// { dg-do run  }
+// { dg-do compile  }
 // { dg-options "-w -fpermissive" }
 // Test for g++ array init extension 
 
@@ -11,5 +11,5 @@ private:
 
 main()
 {
-  A *list = new A[10](4);
+  A *list = new A[10](4); // { dg-error "parenthesized" }
 }
diff --git a/gcc/testsuite/g++.old-deja/g++.robertl/eb63.C 
b/gcc/testsuite/g++.old-deja/g++.robertl/eb63.C
index a49fb02641cd..653351b8dfad 100644
--- a/gcc/testsuite/g++.old-deja/g++.robertl/eb63.C
+++ b/gcc/testsuite/g++.old-deja/g++.robertl/eb63.C
@@ -1,4 +1,4 @@
-// { dg-do run  }
+// { dg-do compile  }
 // { dg-options "-w -fpermissive" }
 //This uses GNU extensions, so disable -ansi
 #include 
@@ -13,5 +13,5 @@ public:
 main() {
 A* a;
 
-a = new A[2](1,false);
+a = new A[2](1,false); // { dg-error "parenthesized" }
 }


-- 
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evang

Re: [PATCH] [PR c++/71965] silence multi-dim array init sorry without tf_error

2018-03-21 Thread Alexandre Oliva
On Mar 20, 2018, Jason Merrill  wrote:

>> + if ((complain & tf_error))
>> +   error ("cannot initialize multi-dimensional"
>> +  " array with initializer");

> Let's also use the other diagnostic message: "array must be
> initialized with a brace-enclosed initializer".

> OK with that change.

Thanks.  Besides changing the message, I added the location of the
initializer to the message.

Here's what I'm checking in:

[PR c++/71965] silence multi-dim array init sorry without tf_error

We shouldn't substitute templates into short-circuited-out concepts
constraints, but we do, and to add insult to injury, we issue a
sorry() error when a concept that shouldn't even have been substituted
attempts to perform a multi-dimensional array initialization with a
new{} expression.

Although fixing the requirements short-circuiting is probably too
risky at this point, we can get closer to the intended effect by
silencing that sorry just as we silence other errors.

for  gcc/cp/ChangeLog

PR c++/71965
* init.c (build_vec_init): Silence error, former sorry,
without tf_error.

for  gcc/testsuite/ChangeLog

PR c++/71965
* g++.dg/concepts/pr71965.C: New.
---
 gcc/cp/init.c   |   19 ---
 gcc/testsuite/g++.dg/concepts/pr71965.C |   27 +++
 2 files changed, 39 insertions(+), 7 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/concepts/pr71965.C

diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 2263d12563cd..ff52c42c1ad8 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -4381,12 +4381,17 @@ build_vec_init (tree base, tree maxindex, tree init,
   else if (TREE_CODE (type) == ARRAY_TYPE)
{
  if (init && !BRACE_ENCLOSED_INITIALIZER_P (init))
-   sorry
- ("cannot initialize multi-dimensional array with initializer");
- elt_init = build_vec_init (build1 (INDIRECT_REF, type, base),
-0, init,
-explicit_value_init_p,
-0, complain);
+   {
+ if ((complain & tf_error))
+   error_at (loc, "array must be initialized "
+ "with a brace-enclosed initializer");
+ elt_init = error_mark_node;
+   }
+ else
+   elt_init = build_vec_init (build1 (INDIRECT_REF, type, base),
+  0, init,
+  explicit_value_init_p,
+  0, complain);
}
   else if (explicit_value_init_p)
{
@@ -4446,7 +4451,7 @@ build_vec_init (tree base, tree maxindex, tree init,
}
 
   current_stmt_tree ()->stmts_are_full_exprs_p = 1;
-  if (elt_init)
+  if (elt_init && !errors)
finish_expr_stmt (elt_init);
   current_stmt_tree ()->stmts_are_full_exprs_p = 0;
 
diff --git a/gcc/testsuite/g++.dg/concepts/pr71965.C 
b/gcc/testsuite/g++.dg/concepts/pr71965.C
new file mode 100644
index ..6bfaef19211f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr71965.C
@@ -0,0 +1,27 @@
+// { dg-do compile { target c++14 } }
+// { dg-options "-fconcepts" }
+
+template 
+concept bool Destructible() {
+return false;
+}
+
+template 
+concept bool ConstructibleObject =
+// Concept evaluation should short-circuit even the template
+// substitution, so we shouldn't even substitute into the requires
+// constraint and the unimplemented multi-dimensional new T{...}
+// initialization.  ATM we do, but as long as we don't output the
+// sorry() message we used to for such constructs when asked not
+// to issue errors, this shouldn't be a problem for this and
+// similar cases.
+Destructible() && requires (Args&&...args) {
+new T{ (Args&&)args... };
+};
+
+int main() {
+using T = int[2][2];
+// GCC has not implemented initialization of multi-dimensional
+// arrays with new{} expressions.
+static_assert(!ConstructibleObject);
+}


-- 
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer


[PATCH,rs6000] GCC -7, no swap optimization for vpermxor instruction

2018-03-21 Thread Carl Love
GCC Maintainers:

The following patch is a back port from mainlin of a fix for
define_insn crypto_vpermxor_ in gcc/config/rs6000/crypto.md.  The
issue is the vpermxor instruction does not work correctly when the swap
optimization is applied.  

The issue was found as part of commit 258530 to mainline which added
support for the vec_permxor builtin.  The builtin uses
crypto_vpermxor_.  The third operand of the vpermxor  instruction
specifies how the bytes are to be permuted.  With swap optimization,
the bytes in operands have been moved around prior to the instruction
being executed resulting the the wrong swapping.

The patch adds UNSPEC_VPERMXOR to the list of instruction to not apply
swap optimization to. A new test case was added to verify the results
with -O2 match the expected results from -O0.

The patch has been tested on GCC 7 on

  powerpc64-unknown-linux-gnu (Power 8 BE)
  powerpc64le-unknown-linux-gnu (Power 8 LE)
  powerpc64le-unknown-linux-gnu (Power 9 LE)

with no regressions.

Please let me know if the patch looks OK for the GCC 7 branch.

 Carl Love
--

2018-03-21  Carl Love  

* config/rs6000/r6000.c (rtx_is_swappable_p): Add case UNSPEC_VPERMXOR.

gcc/testsuite/ChangeLog:

2018-03-21  Carl Love  
* gcc.target/powerpc/crypto-builtin-1-runnable.c: New test file.
---
 gcc/config/rs6000/rs6000.c |   1 +
 .../gcc.target/powerpc/crypto-builtin-1-runnable.c | 110 +
 2 files changed, 111 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/crypto-builtin-1-runnable.c

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 0d7fc51..9b50a74 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -41872,6 +41872,7 @@ rtx_is_swappable_p (rtx op, unsigned int *special)
  case UNSPEC_VPERM_UNS:
  case UNSPEC_VPERMHI:
  case UNSPEC_VPERMSI:
+ case UNSPEC_VPERMXOR:
  case UNSPEC_VPKPX:
  case UNSPEC_VSLDOI:
  case UNSPEC_VSLO:
diff --git a/gcc/testsuite/gcc.target/powerpc/crypto-builtin-1-runnable.c 
b/gcc/testsuite/gcc.target/powerpc/crypto-builtin-1-runnable.c
new file mode 100644
index 000..912cd45
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/crypto-builtin-1-runnable.c
@@ -0,0 +1,110 @@
+/* { dg-do run { target { powerpc*-*-* } } } */
+/* { dg-require-effective-target powerpc_p8vector_ok } */
+/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { 
"-mcpu=power8" } } */
+/* { dg-options "-mcpu=power8 -O2 " } */
+
+/* Make sure the test case compiled with -O2 generates the same expected
+   results.  The expected results were generated with -O0.  */
+
+#include 
+#define TRUE 1
+#define FALSE 0
+
+#define DEBUG 1
+
+#ifdef DEBUG
+#include 
+#endif
+
+void abort (void);
+
+typedef vector unsigned long long  crypto_t;
+typedef vector unsigned long long  v2di_t;
+typedef vector unsigned intv4si_t;
+typedef vector unsigned short  v8hi_t;
+typedef vector unsigned char   v16qi_t;
+
+v16qi_t crypto6a (v16qi_t a, v16qi_t b, v16qi_t c)
+{
+  return __builtin_crypto_vpermxor (a, b, c);
+}
+
+v8hi_t crypto6b (v8hi_t a, v8hi_t b, v8hi_t c)
+{
+  return __builtin_crypto_vpermxor (a, b, c);
+}
+
+v4si_t crypto6c (v4si_t a, v4si_t b, v4si_t c)
+{
+  return __builtin_crypto_vpermxor (a, b, c);
+}
+
+v2di_t crypto6d (v2di_t a, v2di_t b, v2di_t c)
+{
+  return __builtin_crypto_vpermxor (a, b, c);
+}
+
+int main()
+{
+  int i;
+  v16qi_t expected_v16qi, result_v16qi;
+  v8hi_t expected_v8hi, result_v8hi;
+  v4si_t expected_v4si, result_v4si;
+  v2di_t expected_v2di, result_v2di;
+  v16qi_t v16qi_arg_a, v16qi_arg_b, v16qi_arg_c;
+  v8hi_t v8hi_arg_a, v8hi_arg_b, v8hi_arg_c;
+  v4si_t v4si_arg_a, v4si_arg_b, v4si_arg_c;
+  v2di_t v2di_arg_a, v2di_arg_b, v2di_arg_c;
+
+  v16qi_arg_a = (vector unsigned char){ 7, 6, 5, 4, 3, 2, 1, 0,
+   1, 2, 3, 4, 5, 6, 7, 8 };
+  v16qi_arg_b = (vector unsigned char){ 1, 2, 3, 4, 5, 6, 7, 8,
+   7, 6, 5, 4, 3, 2, 1, 0 };
+  v16qi_arg_c = (vector unsigned char){ 7, 2, 5, 4, 3, 6, 1, 8,
+   1, 6, 3, 4, 5, 2, 7, 0 };
+  expected_v16qi = (vector unsigned char){ 15, 10, 13, 12, 11, 14, 9, 0,
+  9, 14, 11, 12, 13, 10, 15, 8 };
+   
+  result_v16qi = crypto6a (v16qi_arg_a, v16qi_arg_b, v16qi_arg_c);
+ 
+  for (i = 0; i < 16; i++)
+if (expected_v16qi[i] != result_v16qi[i])
+  printf("crypto6a: result_v16qi[%d] =  %d, expected = %d\n",
+i, result_v16qi[i], expected_v16qi[i]);
+
+  v8hi_arg_a = (vector unsigned short int){ 7, 6, 5, 4, 3, 2, 1, 0};
+  v8hi_arg_b = (vector unsigned short int){ 1, 2, 3, 4, 5, 6, 7, 8};
+  v8hi_arg_c = (vector unsigned short int){ 7, 2, 5, 

[PATCH] PR fortran/84922 -- Check MODULE prefix is used

2018-03-21 Thread Steve Kargl
If an interface body includes a MODULE prefix on a subroutine
or function, then the prefix must appear on the contained
subprogram.  This patch does that check, and issues an error
message.

2018-03-21  Steven G. Kargl  

PR fortran/84922
* decl.c (get_proc_name): If the MODULE prefix appears in interface
body, then it must appear on the contained subroutine or function.
While here, fix nearby mis-indented code.

2018-03-21  Steven G. Kargl  Index: gcc/fortran/decl.c
===
--- gcc/fortran/decl.c	(revision 258727)
+++ gcc/fortran/decl.c	(working copy)
@@ -1245,16 +1245,27 @@ get_proc_name (const char *name, gfc_symbol **result, 
 		   "from a previous declaration",  name);
 }
 
-if (sym && !sym->gfc_new
-	&& sym->attr.flavor != FL_UNKNOWN
-	&& sym->attr.referenced == 0 && sym->attr.subroutine == 1
-	&& gfc_state_stack->state == COMP_CONTAINS
-	&& gfc_state_stack->previous->state == COMP_SUBROUTINE)
-{
-	gfc_error_now ("Procedure %qs at %C is already defined at %L",
-		   name, &sym->declared_at);
-}
+  /* C1246 (R1225) MODULE shall appear only in the function-stmt or
+ subroutine-stmt of a module subprogram or of a nonabstract interface
+ body that is declared in the scoping unit of a module or submodule.  */
+  if (sym->attr.external
+  && (sym->attr.subroutine || sym->attr.function)
+  && sym->attr.if_source == IFSRC_IFBODY
+  && !current_attr.module_procedure
+  && sym->attr.proc == PROC_MODULE
+  && gfc_state_stack->state == COMP_CONTAINS)
+gfc_error_now ("Procedure %qs defined in interface body at %L "
+		   "clashes with internal procedure defined at %C",
+		name, &sym->declared_at);
 
+  if (sym && !sym->gfc_new
+  && sym->attr.flavor != FL_UNKNOWN
+  && sym->attr.referenced == 0 && sym->attr.subroutine == 1
+  && gfc_state_stack->state == COMP_CONTAINS
+  && gfc_state_stack->previous->state == COMP_SUBROUTINE)
+gfc_error_now ("Procedure %qs at %C is already defined at %L",
+		name, &sym->declared_at);
+
   if (gfc_current_ns->parent == NULL || *result == NULL)
 return rc;
 
Index: gcc/testsuite/gfortran.dg/interface_42.f90
===
--- gcc/testsuite/gfortran.dg/interface_42.f90	(nonexistent)
+++ gcc/testsuite/gfortran.dg/interface_42.f90	(working copy)
@@ -0,0 +1,24 @@
+! { dg-do compile }
+! { dg-options "-fmax-errors=1" }
+! PR fortran/84922
+! Code contributed by William Clodius, but simplified by me.
+module copy
+
+   interface
+  module subroutine foo_da(da, copy) ! { dg-error "(1)" }
+ integer, intent(in) :: da(:)
+ integer, allocatable, intent(out) :: copy(:)
+  end subroutine foo_da
+   end interface
+
+   contains
+
+  subroutine foo_da(da, copy) ! { dg-error "defined in interface body" }
+ integer, intent(in) :: da(:)
+ integer, allocatable, intent(out) :: copy(:)
+ allocate( copy( size(da) ) )
+ copy = da
+  end subroutine foo_da
+
+end module copy
+{ dg-prune-output "compilation terminated" }
Index: gcc/testsuite/gfortran.dg/interface_43.f90
===
--- gcc/testsuite/gfortran.dg/interface_43.f90	(nonexistent)
+++ gcc/testsuite/gfortran.dg/interface_43.f90	(working copy)
@@ -0,0 +1,25 @@
+! { dg-do compile }
+! PR fortran/84922
+! This should compile without error.
+module foom
+
+   implicit none
+
+   interface foo
+  module procedure foo_sngl
+  module procedure foo_dble
+   end interface foo
+
+   contains
+
+  subroutine foo_sngl(n, f, g, h)
+ integer n
+ real f, g, h
+  end subroutine foo_sngl
+
+  subroutine foo_dble(n, f, g, h)
+ integer n
+ double precision f, g, h
+  end subroutine foo_dble
+
+end module foom


Adjust __builtin_tgmath handling of integer arguments to _FloatN narrowing macros

2018-03-21 Thread Joseph Myers
When adding __builtin_tgmath to support a better tgmath.h
implementation, I noted that further changes might be needed regarding
the TS 18661 functions that round their results to a narrower type,
because of unresolved issues with how the corresponding type-generic
macros are defined in TS 18661.

The resolution of those issues is still in flux, but the latest
version does indeed require something slightly different from
__builtin_tgmath.  It specifies that integer arguments to type-generic
macros such as f32xadd are treated as _Float64 not double - which was
also present in earlier versions of the resolution - but then it also
specifies different handling for _Float64 arguments and double
arguments, which wasn't in earlier versions.  Specifically, in the
latest version
, f32xadd
with _Float64 arguments would call f32xaddf64, while f32xadd with
double arguments would call f32xaddf64x.  Since integer arguments are
converted directly to the argument type of the selected function (not
to double / _Float64x unless that ends up as the argument type), this
is a user-visible difference in semantics that means __builtin_tgmath
actually needs to implement treating integer arguments as _Float64 in
this case (the rest of the latest semantics can then be implemented in
the header, with a few inline functions there).

To avoid releasing with the older version of the __builtin_tgmath
semantics that doesn't work with the latest proposed DR#13 resolution,
this patch implements a rule in __builtin_tgmath that maps integer
types to _Float64 (respectively _Complex _Float64 for complex integer
types) where all the specified functions return the same _FloatN or
_FloatNx type.  This does not affect any existing uses of
__builtin_tgmath in glibc's or GCC's tgmath.h since I haven't yet
added any of these type-generic macros to glibc when adding the
corresponding narrowing functions.

Bootstrapped with no regressions on x86_64-pc-linux-gnu.  Applied to 
mainline.

2018-03-21  Joseph Myers  

* doc/extend.texi (__builtin_tgmath): Document when complex
integer types are treated as _Complex _Float64.

gcc/c:
2018-03-21  Joseph Myers  

* c-parser.c (c_parser_postfix_expression): For __builtin_tgmath
where all functions return the same _FloatN or _FloatNx type,
treat integer types as _Float64 instead of double.

gcc/testsuite:
2018-03-21  Joseph Myers  

* gcc.dg/builtin-tgmath-3.c: New test.

Index: gcc/c/c-parser.c
===
--- gcc/c/c-parser.c(revision 258722)
+++ gcc/c/c-parser.c(working copy)
@@ -8530,10 +8530,12 @@ c_parser_postfix_expression (c_parser *parser)
   argument is decimal, or if the only alternatives for
   type-generic arguments are of decimal types, and are
   otherwise treated as double (or _Complex double for
-  complex integer types).  After that adjustment, types
-  are combined following the usual arithmetic
-  conversions.  If the function only accepts complex
-  arguments, a complex type is produced.  */
+  complex integer types, or _Float64 or _Complex _Float64
+  if all the return types are the same _FloatN or
+  _FloatNx type).  After that adjustment, types are
+  combined following the usual arithmetic conversions.
+  If the function only accepts complex arguments, a
+  complex type is produced.  */
bool arg_complex = all_complex;
bool arg_binary = all_binary;
bool arg_int_decimal = all_decimal;
@@ -8632,6 +8634,19 @@ c_parser_postfix_expression (c_parser *parser)
  }
  }
  }
+   /* For a macro rounding its result to a narrower type, map
+  integer types to _Float64 not double if the return type
+  is a _FloatN or _FloatNx type.  */
+   bool arg_int_float64 = false;
+   if (parm_kind[0] == tgmath_fixed
+   && SCALAR_FLOAT_TYPE_P (parm_first[0])
+   && float64_type_node != NULL_TREE)
+ for (unsigned int j = 0; j < NUM_FLOATN_NX_TYPES; j++)
+   if (parm_first[0] == FLOATN_TYPE_NODE (j))
+ {
+   arg_int_float64 = true;
+   break;
+ }
tree arg_real = NULL_TREE;
for (unsigned int j = 1; j <= nargs; j++)
  {
@@ -8644,6 +8659,8 @@ c_parser_postfix_expression (c_parser *parser)
if (INTEGRAL_TYPE_P (type))
  type = (arg_int_decimal
  ? dfloat64_type_node
+ : arg_int_float64
+ ? float64_type_node
  : double_type_node);
if (arg_real == NULL_TREE)
  arg_real = 

Re: [PATCH, rs6000] Fix PR83789: __builtin_altivec_lvx fails for powerpc for altivec-4.c

2018-03-21 Thread Segher Boessenkool
On Wed, Mar 21, 2018 at 12:47:41PM -0500, Peter Bergner wrote:
> On 3/20/18 12:27 PM, Peter Bergner wrote:
> > On 3/20/18 11:42 AM, Segher Boessenkool wrote:
> >> On Mon, Mar 19, 2018 at 09:12:08PM -0500, Peter Bergner wrote:
> >>> Looking at mu build dirs insn-modes.h, I don't see HAVE_V8HFmode being
> >>> defined on either my LE or BE builds.  What am I missing?
> >>
> >> Nothing, I just was confused (we always define the mode in 
> >> rs6000-modes.def,
> >> but that is not the same thing).  Sorry.
> > 
> > Ok then, patch committed with the expander change you requested and
> > leaving the HAVE_V8HFmode code as is in the patch.  Thanks!
> 
> Kaushik confirmed this is broken on GCC 7 as well.  Ok to backport
> this patch to GCC 7, assuming regtesting is clean?
> 
> I'll note that GCC 7 does not need any of the changes to rs6000-p8swap.c,
> since that file doesn't exist and doesn't need to exist in GCC 7, so I've
> left those changes out.

Did that same code live in rs6000.c in GCC 7, or is it new since
rs6000-p8swap.c was split off from there?  And what about GCC 6?


Segher


[C++ PATCH] Implement P0962

2018-03-21 Thread Ville Voutilainen
Tested on Linux-PPC64.

2018-03-22  Ville Voutilainen  

gcc/cp/

Implement P0962
* parser.c (cp_parser_perform_range_for_lookup): Change
the condition for deciding whether to use members.

testsuite/

Implement P0962
   * g++.dg/cpp0x/range-for13.C: Adjust.
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 4e3e1dc..ed3c085 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -12046,7 +12046,7 @@ cp_parser_perform_range_for_lookup (tree range, tree *begin, tree *end)
   /*protect=*/2, /*want_type=*/false,
   tf_warning_or_error);
 
-  if (member_begin != NULL_TREE || member_end != NULL_TREE)
+  if (member_begin != NULL_TREE && member_end != NULL_TREE)
 	{
 	  /* Use the member functions.  */
 	  if (member_begin != NULL_TREE)
diff --git a/gcc/testsuite/g++.dg/cpp0x/range-for13.C b/gcc/testsuite/g++.dg/cpp0x/range-for13.C
index 9ed0458..7babd71 100644
--- a/gcc/testsuite/g++.dg/cpp0x/range-for13.C
+++ b/gcc/testsuite/g++.dg/cpp0x/range-for13.C
@@ -3,18 +3,6 @@
 
 // { dg-do compile { target c++11 } }
 
-//These should not be used
-template int *begin(T &t)
-{
-T::fail;
-return 0;
-}
-template int *end(T &t)
-{
-T::fail;
-return 0;
-}
-
 struct container1
 {
 int *begin();
@@ -89,10 +77,37 @@ struct container10
 static function end;
 };
 
+namespace N
+{
+template int *begin(T &t)
+{
+return 0;
+}
+template int *end(T &t)
+{
+return 0;
+}
+struct container11
+{
+int *begin();
+//no end
+};
+
+struct container12
+{
+int *end();
+//no begin
+};
+
+struct container13
+{
+};
+}
+
 void test1()
 {
-  for (int x : container1()); // { dg-error "member but not" }
-  for (int x : container2()); // { dg-error "member but not" }
+  for (int x : container1()); // { dg-error "'begin' was not declared|'end' was not declared" }
+  for (int x : container2()); // { dg-error "'begin' was not declared|'end' was not declared" }
   for (int x : container3()); // { dg-error "within this context" }
   for (int x : container4()); // { dg-error "cannot be used as a function" }
   for (int x : container5()); // { dg-error "invalid use of" }
@@ -101,4 +116,7 @@ void test1()
   for (int x : container8());
   for (int x : container9()); // { dg-error "within this context" }
   for (int x : container10());
+  for (int x : N::container11());
+  for (int x : N::container12());
+  for (int x : N::container13());
 }


Re: [PR c++/84789] do not resolve typename into template-independent

2018-03-21 Thread Alexandre Oliva
On Mar 21, 2018, Jason Merrill  wrote:

> On Tue, Mar 20, 2018 at 11:27 PM, Alexandre Oliva  wrote:

>> I understood you were saying it was ok to peek in this case.  Would you
>> please state, for clarity, what your stance is on peeking in this case,
>> specifically, in the B::A::I::I within the definition of C above?

> It's OK when we're tentatively trying to parse it as a declarator, not
> when we're trying to parse it as a type-specifier.

Thanks for the clarification; it had seemed to me that you were
rejecting my understanding that we shouldn't look up within the
unrelated template type when parsing the type id.  Now I understand you
were only rejecting the notion that we were parsing it as a type id.
Once I understood that, and realized why you were speaking of
declarators although the testcase at hand had a template-dependent type
id that was obviously (to me) not a declarator, it all made sense ;-)
Sorry for being so dense.


> I wouldn't argue against making it ill-formed, but I don't see a rule
> that would make it so in the current draft standard.

I have just spent some time looking for such a rule and failed to find
it too.  So I put in all of the bizarre declarators from this
conversation into new testcases, but expecting them to be accepted.

> I disagree; it seems to me that the assert should allow the case where
> the scope was originally dependent, but got resolved earlier in the
> function.

 Doesn't the function always take dependent scopes?  I for some reason
 thought that was the case.

>>> Yes, as the comment says, a TYPENAME_TYPE should always have a
>>> dependent scope.  In this case, the dependent scope was "typename
>>> B::A", but just above we resolved it to just A, making it no longer
>>> dependent.

>> Then you got me thoroughly confused: what did you mean by 'the assert
>> should allow' a case that's a given?

> asserts are often used to verify that things we think of as givens are
> actually true.  :)

Yeah, but the wording 'allow' rather than 'require'/'verify' set me off.
We used to require the opposite of X, so allowing X amounted to assert
(!X || X), which didn't sound like something you'd mean.

> I suppose we could remove the assert, but I'd probably move it up
> above where we start messing with 'scope'.

Now that makes sense to me, and it works, but I was not happy with the
errors we got, so I went ahead and arranged for the type name to be
reparsed as a non-declarator for the diagnostic, so that we get a better
diagnostic.

Regstrapping, after a successful check-g++ of a non-bootstrapped
x86_64-linux-gnu build.  Ok to install if the regstrap passes?


[PR c++/84789] do not fail to resolve typename into template-independent

Although resolve_typename_type always takes a template-dependent
type-id, and it usually resolves it to another template-dependent
type-id, it is not correct to require the latter: in declarators,
template-dependent scopes may turn out to name template-independent
types, as in the pr84789-2.C and pr84789-3.C testcases.

The ill-formed testcase pr84789.C trips the same too-strict assert,
and also gets fixed by removing the assertion on the simplified scope.
However, whereas when the dependent type cannot be resolved, we get an
error that suggests 'typename' is missing:

pr84789.C:12:3: error: need ‘typename’ before ‘typename B::A::I::I’
because ‘typename B::A::I’ is a dependent scope
   B::A::I::I i;
   ^~~~

when it can, we got errors that did not point at that possibility,
which may be confusing:

pr84789.C:9:15: error: ‘A::I’ {aka ‘int’} is not a class type
   B::A::I::I i; // { dg-error "typename" }
   ^
pr84789.C:9:15: error: ‘I’ in ‘A::I’ {aka ‘int’} does not name a type

Changing the parser diagnostic code that reports an invalid type name
so that it does not attempt to reparse the name as a declarator gets
us the superior diagnostic of a missing 'typename' keyword.


for  gcc/cp/ChangeLog

PR c++/84789
* pt.c (resolve_typename_type): Drop assert that stopped
simplification to template-independent types.  Add assert to
verify the initial scope is template dependent.
* parser.c (cp_parser_parse_and_diagnose_invalid_type_name):
Reparse the id expression as a type-name, not a declarator.

for  gcc/testsuite/ChangeLog

PR c++/84789
* g++.dg/template/pr84789.C: New.
* g++.dg/template/pr84789-2.C: New.
* g++.dg/template/pr84789-3.C: New.
* g++.dg/parse/dtor11.C: Accept alternate error message.
---
 gcc/cp/parser.c   |2 +-
 gcc/cp/pt.c   |5 +++--
 gcc/testsuite/g++.dg/parse/dtor11.C   |2 +-
 gcc/testsuite/g++.dg/template/pr84789-2.C |   26 
 gcc/testsuite/g++.dg/template/pr84789-3.C |   31 +
 gcc/testsuite/g++.dg/template/pr84789.C   |   13 
 6 files changed, 75 insertions(+), 4 deletions(-)
 create

Re: [Aarch64] Fix conditional branches with target far away.

2018-03-21 Thread Sameera Deshpande
Hi Sudakshina,

As per the ARMv8 ARM, for the offset range (-1048576 ,1048572), the
far branch instruction offset is inclusive of both the offsets. Hence,
I am using <=||=> and not <||>= as it was in previous implementation.

On 16 March 2018 at 00:51, Sudakshina Das  wrote:
> On 15/03/18 15:27, Sameera Deshpande wrote:
>>
>> Ping!
>>
>> On 28 February 2018 at 16:18, Sameera Deshpande
>>  wrote:
>>>
>>> On 27 February 2018 at 18:25, Ramana Radhakrishnan
>>>  wrote:

 On Wed, Feb 14, 2018 at 8:30 AM, Sameera Deshpande
  wrote:
>
> Hi!
>
> Please find attached the patch to fix bug in branches with offsets over
> 1MiB.
> There has been an attempt to fix this issue in commit
> 050af05b9761f1979f11c151519e7244d5becd7c
>
> However, the far_branch attribute defined in above patch used
> insn_length - which computes incorrect offset. Hence, eliminated the
> attribute completely, and computed the offset from insn_addresses
> instead.
>
> Ok for trunk?
>
> gcc/Changelog
>
> 2018-02-13 Sameera Deshpande 
>  * config/aarch64/aarch64.md (far_branch): Remove attribute.
> Eliminate
>  all the dependencies on the attribute from RTL patterns.
>

 I'm not a maintainer but this looks good to me modulo notes about how
 this was tested. What would be nice is a testcase for the testsuite as
 well as ensuring that the patch has been bootstrapped and regression
 tested. AFAIR, the original patch was put in because match.pd failed
 when bootstrap in another context.


 regards
 Ramana

> --
> - Thanks and regards,
>Sameera D.
>>>
>>>
>>> The patch is tested with GCC testsuite and bootstrapping successfully.
>>> Also tested for spec benchmark.
>>>
>
> I am not a maintainer either. I noticed that the range check you do for
> the offset has a (<= || >=). The "far_branch" however did (< || >=) for a
> positive value. Was that also part of the incorrect offset calculation?
>
> @@ -692,7 +675,11 @@
> {
>   if (get_attr_length (insn) =3D=3D 8)
> {
> -   if (get_attr_far_branch (insn) =3D=3D 1)
> +   long long int offset;
> +   offset =3D INSN_ADDRESSES (INSN_UID (XEXP (operands[2], 0)))
> + - INSN_ADDRESSES (INSN_UID (insn));
> +
> +   if (offset <=3D -1048576 || offset >=3D 1048572)
>return aarch64_gen_far_branch (operands, 2, "Ltb",
>   "\\t%0, %1, ");
>  else
> @@ -709,12 +696,7 @@
>  (if_then_else (and (ge (minus (match_dup 2) (pc)) (const_int
> -32768))
> (lt (minus (match_dup 2) (pc)) (const_int
> 32764)))
>(const_int 4)
> - (const_int 8)))
> -   (set (attr "far_branch")
> -   (if_then_else (and (ge (minus (match_dup 2) (pc)) (const_int
> -1048576))
> -  (lt (minus (match_dup 2) (pc)) (const_int
> 1048572)))
> - (const_int 0)
> - (const_int 1)))]
> + (const_int 8)))]
>
>   )
>
> Thanks
> Sudi
>
>>> --
>>> - Thanks and regards,
>>>Sameera D.
>>
>>
>>
>>
>



-- 
- Thanks and regards,
  Sameera D.


Re: [PATCH, rs6000] Fix PR83789: __builtin_altivec_lvx fails for powerpc for altivec-4.c

2018-03-21 Thread Peter Bergner
On 3/21/18 7:37 PM, Segher Boessenkool wrote:
> On Wed, Mar 21, 2018 at 12:47:41PM -0500, Peter Bergner wrote:
>> I'll note that GCC 7 does not need any of the changes to rs6000-p8swap.c,
>> since that file doesn't exist and doesn't need to exist in GCC 7, so I've
>> left those changes out.
> 
> Did that same code live in rs6000.c in GCC 7, or is it new since
> rs6000-p8swap.c was split off from there?  And what about GCC 6?

The trunk patch's changes to rs6000-p8swap.c were to rs6000_gen_lvx and
rs6000_gen_stvx and those are new to GCC 8.  They are used as part of
Kelvin's optimization that changes the little endian unfriendly vsx
loads and stores that require swaps, with the altivec lvx/stvx which
do not need swaps.  That optimization was new to GCC 8, so there was
no need for the rs6000-p8swap.c changes.

If you're asking about whether we also need to backport to GCC 6, I
believe Kaushik said in the bugzilla that he only encountered the
ICE on GCC 7 and trunk, so I don't think we need a GCC 6 backport.

Peter




  1   2   >