[PATCH] Fix *ivdep* tests on SPARC etc. (PR testsuite/88369)

[Jakub Jelinek]

Hi!

On (at least some of these) tests and on some targets, GCC prints messages
like:
./cc1 -quiet -O3 -fopt-info-vec-optimized -mvis vect-ivdep-1.c
vect-ivdep-1.c:11:3: optimized: loop vectorized using 8 byte vectors
vect-ivdep-1.c:11:3: optimized:  loop versioned for vectorization to enhance 
alignment
and the versioning for alignment message according to this PR makes those
tests FAIL.  We just want to make sure in these testcases we don't version
for alias, so this patch just prunes the versioning for alignment
diagnostics.

Bootstrapped/regtested on x86_64-linux and i686-linux, does it work on SPARC
and ok for trunk in that case?

2018-12-06  Jakub Jelinek  

PR testsuite/88369
* gcc.dg/vect/vect-ivdep-1.c: Prune versioning for alignment messages.
* gcc.dg/vect/vect-ivdep-2.c: Likewise.
* g++.dg/vect/pr33426-ivdep.cc: Likewise.
* g++.dg/vect/pr33426-ivdep-2.cc: Likewise. 
* g++.dg/vect/pr33426-ivdep-3.cc: Likewise.
* g++.dg/vect/pr33426-ivdep-4.cc: Likewise.

--- gcc/testsuite/gcc.dg/vect/vect-ivdep-1.c.jj 2015-05-29 15:04:27.894882932 
+0200
+++ gcc/testsuite/gcc.dg/vect/vect-ivdep-1.c2018-12-05 17:37:54.032485003 
+0100
@@ -15,3 +15,4 @@ void foo(int n, int *a, int *b, int *c,
 
 /* { dg-message "loop vectorized" "" { target *-*-* } 0 } */
 /* { dg-bogus " version\[^\n\r]* alias" "" { target *-*-* } 0 } */
+/* { dg-prune-output " version\[^\n\r]* alignment" } */
--- gcc/testsuite/gcc.dg/vect/vect-ivdep-2.c.jj 2015-05-29 15:04:27.908882716 
+0200
+++ gcc/testsuite/gcc.dg/vect/vect-ivdep-2.c2018-12-05 17:38:00.528377814 
+0100
@@ -31,3 +31,4 @@ void bar(int n, int *a, int *b, int *c)
 
 /* { dg-message "loop vectorized" "" { target *-*-* } 0 } */
 /* { dg-bogus " version\[^\n\r]* alias" "" { target *-*-* } 0 } */
+/* { dg-prune-output " version\[^\n\r]* alignment" } */
--- gcc/testsuite/g++.dg/vect/pr33426-ivdep.cc.jj   2015-05-29 
15:04:31.748823367 +0200
+++ gcc/testsuite/g++.dg/vect/pr33426-ivdep.cc  2018-12-05 17:35:08.818212056 
+0100
@@ -15,3 +15,4 @@ void foo(int n, int *a, int *b, int *c,
 
 /* { dg-message "loop vectorized" "" { target *-*-* } 0 } */
 /* { dg-bogus " version\[^\n\r]* alias" "" { target *-*-* } 0 } */
+/* { dg-prune-output " version\[^\n\r]* alignment" } */
--- gcc/testsuite/g++.dg/vect/pr33426-ivdep-2.cc.jj 2015-05-29 
15:04:31.739823506 +0200
+++ gcc/testsuite/g++.dg/vect/pr33426-ivdep-2.cc2018-12-05 
17:35:28.772881921 +0100
@@ -30,6 +30,7 @@ void bar(int n, int *a, int *b, int *c)
 
 /* { dg-message "loop vectorized" "" { target *-*-* } 0 } */
 /* { dg-bogus " version\[^\n\r]* alias" "" { target *-*-* } 0 } */
+/* { dg-prune-output " version\[^\n\r]* alignment" } */
 
 /* { dg-final { scan-tree-dump-times "ANNOTATE_EXPR " 2 "original" } } */
 /* { dg-final { scan-tree-dump-times "ANNOTATE " 2 "gimple" } } */
--- gcc/testsuite/g++.dg/vect/pr33426-ivdep-3.cc.jj 2015-05-29 
15:04:31.747823383 +0200
+++ gcc/testsuite/g++.dg/vect/pr33426-ivdep-3.cc2018-12-05 
17:37:10.182208572 +0100
@@ -17,6 +17,7 @@ void foo(int *a) {
 
 /* { dg-message "loop vectorized" "" { target *-*-* } 0 } */
 /* { dg-bogus " version\[^\n\r]* alias" "" { target *-*-* } 0 } */
+/* { dg-prune-output " version\[^\n\r]* alignment" } */
 
 /* { dg-final { scan-tree-dump-times "ANNOTATE_EXPR " 1 "original" } } */
 /* { dg-final { scan-tree-dump-times "ANNOTATE " 1 "gimple" } } */
--- gcc/testsuite/g++.dg/vect/pr33426-ivdep-4.cc.jj 2015-05-29 
15:04:31.740823491 +0200
+++ gcc/testsuite/g++.dg/vect/pr33426-ivdep-4.cc2018-12-05 
17:37:43.715655239 +0100
@@ -22,6 +22,7 @@ void foo(std::vector *ar, int *b) {
 
 /* { dg-message "loop vectorized" "" { target *-*-* } 0 } */
 /* FIXME: dg-bogus " version\[^\n\r]* alias" "" { target *-*-* } 0  */
+/* { dg-prune-output " version\[^\n\r]* alignment" } */
 
 /* { dg-final { scan-tree-dump-times "ANNOTATE_EXPR " 1 "original" } } */
 /* { dg-final { scan-tree-dump-times "ANNOTATE " 1 "gimple" } } */

Jakub

[PATCH] Fix VRP with -fno-delete-null-pointer-checks (PR c/88367)

[Jakub Jelinek]

Hi!

If we consider -fno-delete-null-pointer-checks as a way to support e.g. AVR
and other targets which can validly place objects at NULL rather than a way
to workaround UBs in code, I believe the following testcase must pass if
there is e.g.
char a[32]; // And this object ends up at address 0
void bar (void);
int main () { foo ([3]); baz ([6]); }
but fails right now.  As mentioned in the PR, in GCC 8 we used to do:
  else if (code == POINTER_PLUS_EXPR)
{
  /* For pointer types, we are really only interested in asserting
 whether the expression evaluates to non-NULL.  */
  if (range_is_nonnull () || range_is_nonnull ())
set_value_range_to_nonnull (vr, expr_type);
and that triggered pretty much never, as range_is_nonnull requires that the
offset is ~[0, 0] exactly, e.g. if it is a constant, it is never that way,
but now we do:
  if (!range_includes_zero_p () || !range_includes_zero_p ())
which is e.g. always if the offset is constant non-zero.

I hope we can still say that pointer wrapping even with
-fno-delete-null-pointer-checks is UB, so this patch differentiates between
positive offsets (in ssizetype), negative offsets (in ssizetype) and zero
offsets and handles both the same for both ptr p+ offset and _REF[ptr, 
offset]
If offset is 0 and ptr is ~[0, 0], then the result is ~[0, 0] as before.
If offset is positive in ssizetype, then even for VARYING ptr the result is
~[0, 0] pointer.  If the offset is (or maybe could be) negative in
ssizetype, then for -fno-delete-null-pointer-checks the result is VARYING,
as we could go from a non-NULL pointer back to NULL on those targets; for
-fdelete-null-pointer-checks we do what we've done before, i.e. ~[0, 0].

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

2018-12-06  Jakub Jelinek  

PR c/88367
* tree-vrp.c (extract_range_from_binary_expr): For POINTER_PLUS_EXPR
with -fno-delete-null-pointer-checks, set_nonnull only if the pointer
is non-NULL and offset is known to have most significant bit clear.
* vr-values.c (vr_values::vrp_stmt_computes_nonzero): For ADDR_EXPR
of MEM_EXPR, return true if the MEM_EXPR has non-zero offset with
most significant bit clear.  If offset does have most significant bit
set and -fno-delete-null-pointer-checks, don't return true even if
the base pointer is non-NULL.

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

--- gcc/tree-vrp.c.jj   2018-12-04 13:00:02.408635579 +0100
+++ gcc/tree-vrp.c  2018-12-05 19:07:36.187567781 +0100
@@ -1673,9 +1673,25 @@ extract_range_from_binary_expr (value_ra
   else if (code == POINTER_PLUS_EXPR)
{
  /* For pointer types, we are really only interested in asserting
-whether the expression evaluates to non-NULL.  */
- if (!range_includes_zero_p ()
- || !range_includes_zero_p ())
+whether the expression evaluates to non-NULL.
+With -fno-delete-null-pointer-checks we need to be more
+conservative.  As some object might reside at address 0,
+then some offset could be added to it and the same offset
+subtracted again and the result would be NULL.
+E.g.
+static int a[12]; where [0] is NULL and
+ptr = [6];
+ptr -= 6;
+ptr will be NULL here, even when there is POINTER_PLUS_EXPR
+where the first range doesn't include zero and the second one
+doesn't either.  As the second operand is sizetype (unsigned),
+consider all ranges where the MSB could be set as possible
+subtractions where the result might be NULL.  */
+ if ((!range_includes_zero_p ()
+  || !range_includes_zero_p ())
+ && (flag_delete_null_pointer_checks
+ || (range_int_cst_p ()
+ && !tree_int_cst_sign_bit (vr1.max ()
vr->set_nonnull (expr_type);
  else if (range_is_null () && range_is_null ())
vr->set_null (expr_type);
--- gcc/vr-values.c.jj  2018-11-29 08:41:33.152749436 +0100
+++ gcc/vr-values.c 2018-12-05 19:37:56.222582823 +0100
@@ -303,8 +303,17 @@ vr_values::vrp_stmt_computes_nonzero (gi
  && TREE_CODE (base) == MEM_REF
  && TREE_CODE (TREE_OPERAND (base, 0)) == SSA_NAME)
{
- value_range *vr = get_value_range (TREE_OPERAND (base, 0));
- if (!range_includes_zero_p (vr))
+ if (integer_zerop (TREE_OPERAND (base, 1))
+ || flag_delete_null_pointer_checks)
+   {
+ value_range *vr = get_value_range (TREE_OPERAND (base, 0));
+ if (!range_includes_zero_p (vr))
+   return true;
+   }
+ /* If MEM_REF has a "positive" offset, consider it non-NULL
+always.  */
+ if (integer_nonzerop (TREE_OPERAND (base, 1))
+ && 

[PATCH] Don't optimize successive divs if there is also a mod with the same last arg (PR tree-optimization/85726)

[Jakub Jelinek]

Hi!

This is my proposal for fixing this PR, just a heuristics when optimizing
successive divides might not be a good idea (first testcase), and includes
Marc's testcases which showed cases where optimizing successive divides is a
good idea even if the inner divide is not a single use.

Unfortunately match.pd doesn't allow to capture the outermost expression, so
I can't narrow it even more by checking if the outer divide is in the same
bb as the modulo.

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

2018-12-06  Jakub Jelinek  

PR tree-optimization/85726
* generic-match-head.c (optimize_successive_divisions_p): New function.
* gimple-match-head.c (optimize_successive_divisions_p): Likewise.
* match.pd: Don't combine successive divisions if they aren't exact
and optimize_successive_divisions_p is false.

* gcc.dg/tree-ssa/pr85726-1.c: New test.
* gcc.dg/tree-ssa/pr85726-2.c: New test.
* gcc.dg/tree-ssa/pr85726-3.c: New test.
* gcc.dg/tree-ssa/pr85726-4.c: New test.

--- gcc/generic-match-head.c.jj 2018-03-28 21:14:28.124743854 +0200
+++ gcc/generic-match-head.c2018-12-05 16:07:43.710801347 +0100
@@ -77,3 +77,12 @@ canonicalize_math_after_vectorization_p
 {
   return false;
 }
+
+/* Return true if successive divisions can be optimized.
+   Defer to GIMPLE opts.  */
+
+static inline bool
+optimize_successive_divisions_p (tree, tree)
+{
+  return false;
+}
--- gcc/gimple-match-head.c.jj  2018-10-10 10:50:55.812109572 +0200
+++ gcc/gimple-match-head.c 2018-12-05 16:39:50.358122589 +0100
@@ -1163,3 +1163,27 @@ optimize_pow_to_exp (tree arg0, tree arg
 return false;
   return true;
 }
+
+/* Return true if a division INNER_DIV / DIVISOR where INNER_DIV
+   is another division can be optimized.  Don't optimize if INNER_DIV
+   is used in a TRUNC_MOD_EXPR with DIVISOR as second operand.  */
+
+static bool
+optimize_successive_divisions_p (tree divisor, tree inner_div)
+{
+  if (!gimple_in_ssa_p (cfun) || TREE_CODE (inner_div) != SSA_NAME)
+return false;
+
+  imm_use_iterator imm_iter;
+  use_operand_p use_p;
+  FOR_EACH_IMM_USE_FAST (use_p, imm_iter, inner_div)
+{
+  gimple *use_stmt = USE_STMT (use_p);
+  if (!is_gimple_assign (use_stmt)
+ || gimple_assign_rhs_code (use_stmt) != TRUNC_MOD_EXPR
+ || !operand_equal_p (gimple_assign_rhs2 (use_stmt), divisor, 0))
+   continue;
+  return false;
+}
+  return true;
+}
--- gcc/match.pd.jj 2018-11-30 21:36:22.273762329 +0100
+++ gcc/match.pd2018-12-05 16:47:27.798596450 +0100
@@ -312,17 +312,19 @@ (define_operator_list COND_TERNARY
and floor_div is trickier and combining round_div even more so.  */
 (for div (trunc_div exact_div)
  (simplify
-  (div (div @0 INTEGER_CST@1) INTEGER_CST@2)
+  (div (div@3 @0 INTEGER_CST@1) INTEGER_CST@2)
   (with {
 wi::overflow_type overflow;
 wide_int mul = wi::mul (wi::to_wide (@1), wi::to_wide (@2),
TYPE_SIGN (type), );
}
-   (if (!overflow)
-(div @0 { wide_int_to_tree (type, mul); })
-(if (TYPE_UNSIGNED (type)
-|| mul != wi::min_value (TYPE_PRECISION (type), SIGNED))
- { build_zero_cst (type); })
+   (if (div == EXACT_DIV_EXPR
+   || optimize_successive_divisions_p (@2, @3))
+(if (!overflow)
+ (div @0 { wide_int_to_tree (type, mul); })
+ (if (TYPE_UNSIGNED (type)
+ || mul != wi::min_value (TYPE_PRECISION (type), SIGNED))
+  { build_zero_cst (type); }))
 
 /* Combine successive multiplications.  Similar to above, but handling
overflow is different.  */
--- gcc/testsuite/gcc.dg/tree-ssa/pr85726-1.c.jj2018-12-05 
16:55:24.852680964 +0100
+++ gcc/testsuite/gcc.dg/tree-ssa/pr85726-1.c   2018-12-05 16:50:14.489853926 
+0100
@@ -0,0 +1,19 @@
+/* PR tree-optimization/85726 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+/* { dg-final { scan-tree-dump " / 16;" "optimized" } } */
+/* { dg-final { scan-tree-dump " / 3;" "optimized" } } */
+/* { dg-final { scan-tree-dump " % 3;" "optimized" } } */
+/* { dg-final { scan-tree-dump-not " / 48;" "optimized" } } */
+
+int ww, vv;
+
+int
+foo (int y)
+{
+  int z = y / 16;
+  int w = z / 3;
+  int v = z % 3;
+  ww = w;
+  return v;
+}
--- gcc/testsuite/gcc.dg/tree-ssa/pr85726-2.c.jj2018-12-05 
16:55:27.620634707 +0100
+++ gcc/testsuite/gcc.dg/tree-ssa/pr85726-2.c   2018-12-05 16:51:25.636678886 
+0100
@@ -0,0 +1,15 @@
+/* PR tree-optimization/85726 */
+/* { dg-do compile { target int32 } } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+/* { dg-final { scan-tree-dump " / 3145728;" "optimized" } } */
+/* { dg-final { scan-tree-dump "y = 0;" "optimized" } } */
+
+int x, y;
+
+void
+foo (int n)
+{
+  int c = 3 << 20;
+  x = n / c;
+  y = x / c;
+}
--- gcc/testsuite/gcc.dg/tree-ssa/pr85726-3.c.jj2018-12-05 
16:55:30.948579089 +0100
+++ gcc/testsuite/gcc.dg/tree-ssa/pr85726-3.c   

[PATCH] Handle clobber stmts in convert_nonlocal_reference_stmt (PR fortran/88304)

[Jakub Jelinek]

Hi!

The following patch handles clobber stmts the same how we handle them in
convert_local_reference_stmt, if it is the clobber with decl on the lhs and
that lhs needs to be replaced by a field inside of a structure, the clobber
is replaced by GIMPLE_NOP.

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

2018-12-06  Jakub Jelinek  

PR fortran/88304
* tree-nested.c (convert_nonlocal_reference_stmt): Remove clobbers
for non-local automatic decls.

* gfortran.fortran-torture/compile/pr88304.f90: New test.

--- gcc/tree-nested.c.jj2018-12-02 13:50:20.186440444 +0100
+++ gcc/tree-nested.c   2018-12-05 11:28:54.979178773 +0100
@@ -1648,6 +1648,21 @@ convert_nonlocal_reference_stmt (gimple_
   *handled_ops_p = false;
   return NULL_TREE;
 
+case GIMPLE_ASSIGN:
+  if (gimple_clobber_p (stmt))
+   {
+ tree lhs = gimple_assign_lhs (stmt);
+ if (DECL_P (lhs)
+ && !(TREE_STATIC (lhs) || DECL_EXTERNAL (lhs))
+ && decl_function_context (lhs) != info->context)
+   {
+ gsi_replace (gsi, gimple_build_nop (), true);
+ break;
+   }
+   }
+  *handled_ops_p = false;
+  return NULL_TREE;
+
 default:
   /* For every other statement that we are not interested in
 handling here, let the walker traverse the operands.  */
--- gcc/testsuite/gfortran.fortran-torture/compile/pr88304.f90.jj   
2018-12-05 11:31:04.232046102 +0100
+++ gcc/testsuite/gfortran.fortran-torture/compile/pr88304.f90  2018-12-05 
11:30:57.092163910 +0100
@@ -0,0 +1,24 @@
+! PR fortran/88304
+
+module pr88304
+  implicit none
+  type t
+ integer :: b = -1
+  end type t
+contains
+  subroutine f1 (x, y)
+integer, intent(out) :: x, y
+x = 5
+y = 6
+  end subroutine f1
+  subroutine f2 ()
+type(t) :: x
+integer :: y
+call f3
+if (x%b .ne. 5 .or. y .ne. 6) stop 1
+  contains
+subroutine f3
+  call f1 (x%b, y)
+end subroutine f3
+  end subroutine f2
+end module pr88304

Jakub

Re: add taishanv110 pipeline scheduling

[Terry Guo]

On Thu, Dec 6, 2018 at 9:31 AM wuyuan (E)  wrote:
>
> Hi ARM maintainers:
> The taishanv110 core uses generic pipeline scheduling, which 
> restricted the performance of taishanv110 core. By adding the pipeline 
> scheduling of taishanv110 core in GCC,The performance of taishanv110 has been 
> improved.
> The patch  as follows, please join.
>
>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> old mode 100644
> new mode 100755
> index c4ec556..d6cf1d3
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,9 @@
> +2018-12-05  wuyuan  
> +

Better be "Wu Yuan"

> +   * config/aarch64/aarch64-cores.def: New CPU.
> +   * config/aarch64/aarch64.md : Add "tsv110.md"
> +   * gcc/config/aarch64/tsv110.md : pipeline description
> +
Can remove the "gcc/" part.

> 2018-11-26  David Malcolm  
>
>   * dump-context.h (dump_context::dump_loc): Convert 1st param from
> diff --git a/gcc/config/aarch64/aarch64-cores.def 
> b/gcc/config/aarch64/aarch64-cores.def
> index 74be5db..8e84844 100644
> --- a/gcc/config/aarch64/aarch64-cores.def
> +++ b/gcc/config/aarch64/aarch64-cores.def
> @@ -99,7 +99,7 @@ AARCH64_CORE("ares",  ares, cortexa57, 8_2A,  
> AARCH64_FL_FOR_ARCH8_2 | AARCH64_F
> /* ARMv8.4-A Architecture Processors.  */
>
> /* HiSilicon ('H') cores. */
> -AARCH64_CORE("tsv110", tsv110,cortexa57,8_4A, 
> AARCH64_FL_FOR_ARCH8_4 | AARCH64_FL_CRYPTO | AARCH64_FL_F16 | AARCH64_FL_AES 
> | AARCH64_FL_SHA2, tsv110,   0x48, 0xd01, -1)
> +AARCH64_CORE("tsv110", tsv110,tsv110,8_4A, 
> AARCH64_FL_FOR_ARCH8_4 | AARCH64_FL_CRYPTO | AARCH64_FL_F16 | AARCH64_FL_AES 
> | AARCH64_FL_SHA2, tsv110,   0x48, 0xd01, -1)
>
> /* Qualcomm ('Q') cores. */
> AARCH64_CORE("saphira", saphira,saphira,8_4A,  
> AARCH64_FL_FOR_ARCH8_4 | AARCH64_FL_CRYPTO | AARCH64_FL_RCPC, saphira,   
> 0x51, 0xC01, -1)
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 82af4d4..5278d6b 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -348,7 +348,7 @@
> (include "thunderx.md")
> (include "../arm/xgene1.md")
> (include "thunderx2t99.md")
> -
> +(include "tsv110.md")
> ;; ---
> ;; Jumps and other miscellaneous insns
> ;; ---
> diff --git a/gcc/config/aarch64/tsv110.md b/gcc/config/aarch64/tsv110.md
> new file mode 100644
> index 000..e912447
> --- /dev/null
> +++ b/gcc/config/aarch64/tsv110.md
> @@ -0,0 +1,708 @@
> +;; tsv110 pipeline description
> +;; Copyright (C) 2014-2016 Free Software Foundation, Inc.
> +;;

Given this is a new file, I think the copyright year should be updated.

BR,
Terry

> +;; This file is part of GCC.
> +;;
> +;; GCC is free software; you can redistribute it and/or modify it
> +;; under the terms of the GNU General Public License as published by
> +;; the Free Software Foundation; either version 3, or (at your option)
> +;; any later version.
> +;;
> +;; GCC is distributed in the hope that it will be useful, but
> +;; WITHOUT ANY WARRANTY; without even the implied warranty of
> +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +;; General Public License for more details.
> +;;
> +;; You should have received a copy of the GNU General Public License
> +;; along with GCC; see the file COPYING3.  If not see
> +;; .
> +
> +(define_automaton "tsv110")
> +
> +(define_attr "tsv110_neon_type"
> +  "neon_arith_acc, neon_arith_acc_q,
> +   neon_arith_basic, neon_arith_complex,
> +   neon_reduc_add_acc, neon_multiply, neon_multiply_q,
> +   neon_multiply_long, neon_mla, neon_mla_q, neon_mla_long,
> +   neon_sat_mla_long, neon_shift_acc, neon_shift_imm_basic,
> +   neon_shift_imm_complex,
> +   neon_shift_reg_basic, neon_shift_reg_basic_q, neon_shift_reg_complex,
> +   neon_shift_reg_complex_q, neon_fp_negabs, neon_fp_arith,
> +   neon_fp_arith_q, neon_fp_reductions_q, neon_fp_cvt_int,
> +   neon_fp_cvt_int_q, neon_fp_cvt16, neon_fp_minmax, neon_fp_mul,
> +   neon_fp_mul_q, neon_fp_mla, neon_fp_mla_q, neon_fp_recpe_rsqrte,
> +   neon_fp_recpe_rsqrte_q, neon_fp_recps_rsqrts, neon_fp_recps_rsqrts_q,
> +   neon_bitops, neon_bitops_q, neon_from_gp,
> +   neon_from_gp_q, neon_move, neon_tbl3_tbl4, neon_zip_q, neon_to_gp,
> +   neon_load_a, neon_load_b, neon_load_c, neon_load_d, neon_load_e,
> +   neon_load_f, neon_store_a, neon_store_b, neon_store_complex,
> +   unknown"
> +  (cond [
> + (eq_attr "type" "neon_arith_acc, neon_reduc_add_acc,\
> +  neon_reduc_add_acc_q")
> +   (const_string "neon_arith_acc")
> + (eq_attr "type" "neon_arith_acc_q")
> +   (const_string "neon_arith_acc_q")
> + (eq_attr "type" "neon_abs,neon_abs_q,neon_add, neon_add_q, 
> neon_add_long,\
> +  neon_add_widen, neon_neg, neon_neg_q,\
> +  

[PATCH] handle expressions in __builtin_has_attribute (PR 88383)

[Martin Sebor]

The __builtin_has_attribute function fails with an ICE when
its first argument is an expression such as INDIRECT_REF, or
many others.  The code simply assumes it's either a type or
a decl.  The attached patch corrects this oversight.

While testing the fix I also noticed the C++ front end expects
the first operand to be a unary expression, which causes most
other kinds of expressions to be rejected.  The patch fixes
that as well.

Finally, while testing the fix even more I realized that
the built-in considers the underlying array itself in ARRAY_REF
expressions rather than its type, which leads to inconsistent
results for overaligned arrays (it's the array itself that's
overaligned, not its elements).  So I fixed that too and
adjusted the one test designed to verify this.

Tested on x86_64-linux.

Martin
PR c/88383 - ICE calling __builtin_has_attribute on a reference

gcc/c-family/ChangeLog:

	PR c/88383
	* c-attribs.c (validate_attribute): Handle expressions.
	(has_attribute): Handle types referenced by expressions.
	Avoid considering array attributes in ARRAY_REF expressions .

gcc/cp/ChangeLog:

	PR c/88383
	* parser.c (cp_parser_has_attribute_expression): Handle assignment
	expressions.

gcc/testsuite/ChangeLog:

	PR c/88383
	* c-c++-common/builtin-has-attribute-4.c: Adjust expectations.
	* c-c++-common/builtin-has-attribute-6.c: New test.

Index: gcc/c-family/c-attribs.c
===
--- gcc/c-family/c-attribs.c	(revision 266799)
+++ gcc/c-family/c-attribs.c	(working copy)
@@ -3994,8 +3994,10 @@ validate_attribute (location_t atloc, tree oper, t
 
   if (TYPE_P (oper))
 tmpdecl = build_decl (atloc, TYPE_DECL, tmpid, oper);
-  else
+  else if (DECL_P (oper))
 tmpdecl = build_decl (atloc, TREE_CODE (oper), tmpid, TREE_TYPE (oper));
+  else if (EXPR_P (oper))
+tmpdecl = build_decl (atloc, TYPE_DECL, tmpid, TREE_TYPE (oper));
 
   /* Temporarily clear CURRENT_FUNCTION_DECL to make decl_attributes
  believe the DECL declared above is at file scope.  (See bug 87526.)  */
@@ -4040,11 +4042,17 @@ has_attribute (location_t atloc, tree t, tree attr
   do
 	{
 	  /* Determine the array element/member declaration from
-	 an ARRAY/COMPONENT_REF.  */
+	 a COMPONENT_REF and an INDIRECT_REF involving a refeence.  */
 	  STRIP_NOPS (t);
 	  tree_code code = TREE_CODE (t);
-	  if (code == ARRAY_REF)
-	t = TREE_OPERAND (t, 0);
+	  if (code == INDIRECT_REF)
+	{
+	  tree op0 = TREE_OPERAND (t, 0);
+	  if (TREE_CODE (TREE_TYPE (op0)) == REFERENCE_TYPE)
+		t = op0;
+	  else
+		break;
+	}
 	  else if (code == COMPONENT_REF)
 	t = TREE_OPERAND (t, 1);
 	  else
@@ -4095,7 +4103,8 @@ has_attribute (location_t atloc, tree t, tree attr
 	}
   else
 	{
-	  atlist = TYPE_ATTRIBUTES (TREE_TYPE (expr));
+	  type = TREE_TYPE (expr);
+	  atlist = TYPE_ATTRIBUTES (type);
 	  done = true;
 	}
 
Index: gcc/cp/parser.c
===
--- gcc/cp/parser.c	(revision 266799)
+++ gcc/cp/parser.c	(working copy)
@@ -8480,9 +8480,9 @@ cp_parser_has_attribute_expression (cp_parser *par
   cp_parser_parse_definitely (parser);
 
   /* If the type-id production did not work out, then we must be
- looking at the unary-expression production.  */
+ looking at an expression.  */
   if (!oper || oper == error_mark_node)
-oper = cp_parser_unary_expression (parser);
+oper = cp_parser_assignment_expression (parser);
 
   /* Go back to evaluating expressions.  */
   --cp_unevaluated_operand;
Index: gcc/testsuite/c-c++-common/builtin-has-attribute-4.c
===
--- gcc/testsuite/c-c++-common/builtin-has-attribute-4.c	(revision 266799)
+++ gcc/testsuite/c-c++-common/builtin-has-attribute-4.c	(working copy)
@@ -151,7 +151,8 @@ void test_packed (struct PackedMember *p)
   A (0, gpak[0].c, packed);
   A (0, gpak[1].s, packed);
   A (1, gpak->a, packed);
-  A (1, (*gpak).a[0], packed);
+  /* It's the array that's declared packed but not its elements.  */
+  A (0, (*gpak).a[0], packed);
 
   /* The following fails because in C it's represented as
INDIRECT_REF (POINTER_PLUS (NOP_EXPR (ADDR_EXPR (gpak)), ...))
@@ -161,7 +162,8 @@ void test_packed (struct PackedMember *p)
   A (0, p->c, packed);
   A (0, p->s, packed);
   A (1, p->a, packed);
-  A (1, p->a[0], packed);
+  /* It's the array that's declared packed but not its elements.  */
+  A (0, p->a[0], packed);
   /* Similar to the comment above.
A (1, *p->a, packed);  */
 }
Index: gcc/testsuite/c-c++-common/builtin-has-attribute-6.c
===
--- gcc/testsuite/c-c++-common/builtin-has-attribute-6.c	(nonexistent)
+++ gcc/testsuite/c-c++-common/builtin-has-attribute-6.c	(working copy)
@@ -0,0 +1,105 @@
+/* PR c/88383 - ICE calling _builtin_has_attribute(r, aligned(N)))
+   on an overaligned reference r
+   { 

Re: [PATCH 2/2] asm inline

[Segher Boessenkool]

Hi!

On Tue, Dec 04, 2018 at 03:30:47PM +, Richard Sandiford wrote:
> Segher Boessenkool  writes:

> Hmm, so we allow top-level "asm volatile" in C++ but not C?

Apparently.  Evert top-level asm is effectively volatile, so this doesn't
really matter; but should we try to resolve the difference?

> Probably worth having tests to show that we (intentionally) don't
> allow top-level "asm inline".

Okay, I'll do this separately.

> > --- a/gcc/doc/extend.texi
> > +++ b/gcc/doc/extend.texi
> > @@ -8382,6 +8382,10 @@ various @option{-std} options, use @code{__asm__} 
> > instead of
> >  @item volatile
> >  The optional @code{volatile} qualifier has no effect. 
> >  All basic @code{asm} blocks are implicitly volatile.
> > +
> > +@item inline
> > +If you use the @code{inline} qualifier, then for inlining purposes the size
> > +of the asm is taken as the smallest size possible (@pxref{Size of an asm}).
> >  @end table
> >  
> >  @subsubheading Parameters
> 
> You need to update the syntax above too, which currently reads:
> 
> @example
> asm @r{[} volatile @r{]} ( @var{AssemblerInstructions} )
> @end example

That was modified in patch 1?

@example
asm @var{asm-qualifiers} ( @var{AssemblerInstructions} )
@end example

> > +/* Return true ASM_STMT ASM_STMT is an asm statement marked inline.  */
> > +
> > +static inline bool
> > +gimple_asm_inline_p (const gasm *asm_stmt)
> > +{
> > +  return (asm_stmt->subcode & GF_ASM_INLINE) != 0;
> > +}
> 
> Return true if ASM_STMT is ...

Heh, I copied this from the "volatile" one above.  I'll fix that one, too.

> (Or "Return true if asm statement ASM_STMT is marked inline", since gasm
> forces it to be an asm statement.)

Yup, same deal.

> > diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
> > index 5aa782b..7e9ed99 100644
> > --- a/gcc/tree-inline.c
> > +++ b/gcc/tree-inline.c
> > @@ -4109,6 +4109,9 @@ estimate_num_insns (gimple *stmt, eni_weights 
> > *weights)
> >with very long asm statements.  */
> > if (count > 1000)
> >   count = 1000;
> > +   /* If this asm is asm inline, count anything as minimum size.  */
> > +   if (gimple_asm_inline_p (as_a  (stmt)))
> > + count = !!count;
> 
> FWIW, since Marc found it confusing too... I think MIN (count, 1) would
> show the intent more clearerly.  But that's just personal preference,
> no need to change.

MIN is a nice way to say it, thanks!  Better than "!!" or "!= 0" here.

Thanks for the review, I'm committing it with the changes above (later
today, after a final bootstrap),


Segher

Re: [PATCH, Fortran] pad char to int conversions with spaces instead of zeros (legacy)

[Jerry DeLisle]

On 12/4/18 9:04 AM, Fritz Reese wrote:

On Tue, Dec 4, 2018 at 10:12 AM Jakub Jelinek  wrote:


Just a couple of random comments.
-fdec-pad-with-spaces option name doesn't look right, because it doesn't say
what the option affects.  So perhaps have transfer in the option name?

[...]

Wouldn't it be better to allow specifying whatever character you want to pad
with, so that users can choose something even different?


I concur with this. In that case some option like -ftransfer-pad-char=
may be a more appriopriate name, where -fdec may enable a default
transfer-pad-char of \x20 rather than \x00.


I disagree completely. I assume the idea of -fdec-pad-with-spaces is to 
accomodate some old dec fortran code. The only reason to use some other 
character is if someone is writing new dec fortran code, which is 
implying encouraging people to be writing non standard conforming code.


Even if it is conforming in the sense that it is processor dependent you 
are encouraging people to create new non portable code across compilers. 
Please just stay consistent with Intel.


This whole padding business just stinks to me. There are better ways to 
accomplish these results without using transfer to convert ascii strings 
into bit patterns or whatever the heck some programmer is trying to do 
here, like maybe use 'ABCE ' if thats what is really needed. And, 
please everyone please quit fiddling with the compiler to fix problems 
in the source code. Are people so lazy or such cheapskates they won't do 
this the right way and update the damn source code if it needs to be used.


We have truly more serious and real problems/bugs in gfortran that 
people should be spending the scarce resources on and not this junk.


Jerry

add taishanv110 pipeline scheduling

[wuyuan (E)]

Hi ARM maintainers:
The taishanv110 core uses generic pipeline scheduling, which restricted 
the performance of taishanv110 core. By adding the pipeline scheduling of 
taishanv110 core in GCC,The performance of taishanv110 has been improved.
The patch  as follows, please join.


diff --git a/gcc/ChangeLog b/gcc/ChangeLog
old mode 100644
new mode 100755
index c4ec556..d6cf1d3
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2018-12-05  wuyuan  
+
+   * config/aarch64/aarch64-cores.def: New CPU.
+   * config/aarch64/aarch64.md : Add "tsv110.md"
+   * gcc/config/aarch64/tsv110.md : pipeline description
+
2018-11-26  David Malcolm  

  * dump-context.h (dump_context::dump_loc): Convert 1st param from
diff --git a/gcc/config/aarch64/aarch64-cores.def 
b/gcc/config/aarch64/aarch64-cores.def
index 74be5db..8e84844 100644
--- a/gcc/config/aarch64/aarch64-cores.def
+++ b/gcc/config/aarch64/aarch64-cores.def
@@ -99,7 +99,7 @@ AARCH64_CORE("ares",  ares, cortexa57, 8_2A,  
AARCH64_FL_FOR_ARCH8_2 | AARCH64_F
/* ARMv8.4-A Architecture Processors.  */

/* HiSilicon ('H') cores. */
-AARCH64_CORE("tsv110", tsv110,cortexa57,8_4A, 
AARCH64_FL_FOR_ARCH8_4 | AARCH64_FL_CRYPTO | AARCH64_FL_F16 | AARCH64_FL_AES | 
AARCH64_FL_SHA2, tsv110,   0x48, 0xd01, -1)
+AARCH64_CORE("tsv110", tsv110,tsv110,8_4A, AARCH64_FL_FOR_ARCH8_4 
| AARCH64_FL_CRYPTO | AARCH64_FL_F16 | AARCH64_FL_AES | AARCH64_FL_SHA2, 
tsv110,   0x48, 0xd01, -1)

/* Qualcomm ('Q') cores. */
AARCH64_CORE("saphira", saphira,saphira,8_4A,  
AARCH64_FL_FOR_ARCH8_4 | AARCH64_FL_CRYPTO | AARCH64_FL_RCPC, saphira,   0x51, 
0xC01, -1)
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 82af4d4..5278d6b 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -348,7 +348,7 @@
(include "thunderx.md")
(include "../arm/xgene1.md")
(include "thunderx2t99.md")
-
+(include "tsv110.md")
;; ---
;; Jumps and other miscellaneous insns
;; ---
diff --git a/gcc/config/aarch64/tsv110.md b/gcc/config/aarch64/tsv110.md
new file mode 100644
index 000..e912447
--- /dev/null
+++ b/gcc/config/aarch64/tsv110.md
@@ -0,0 +1,708 @@
+;; tsv110 pipeline description
+;; Copyright (C) 2014-2016 Free Software Foundation, Inc.
+;;
+;; This file is part of GCC.
+;;
+;; GCC is free software; you can redistribute it and/or modify it
+;; under the terms of the GNU General Public License as published by
+;; the Free Software Foundation; either version 3, or (at your option)
+;; any later version.
+;;
+;; GCC is distributed in the hope that it will be useful, but
+;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+;; General Public License for more details.
+;;
+;; You should have received a copy of the GNU General Public License
+;; along with GCC; see the file COPYING3.  If not see
+;; .
+
+(define_automaton "tsv110")
+
+(define_attr "tsv110_neon_type"
+  "neon_arith_acc, neon_arith_acc_q,
+   neon_arith_basic, neon_arith_complex,
+   neon_reduc_add_acc, neon_multiply, neon_multiply_q,
+   neon_multiply_long, neon_mla, neon_mla_q, neon_mla_long,
+   neon_sat_mla_long, neon_shift_acc, neon_shift_imm_basic,
+   neon_shift_imm_complex,
+   neon_shift_reg_basic, neon_shift_reg_basic_q, neon_shift_reg_complex,
+   neon_shift_reg_complex_q, neon_fp_negabs, neon_fp_arith,
+   neon_fp_arith_q, neon_fp_reductions_q, neon_fp_cvt_int,
+   neon_fp_cvt_int_q, neon_fp_cvt16, neon_fp_minmax, neon_fp_mul,
+   neon_fp_mul_q, neon_fp_mla, neon_fp_mla_q, neon_fp_recpe_rsqrte,
+   neon_fp_recpe_rsqrte_q, neon_fp_recps_rsqrts, neon_fp_recps_rsqrts_q,
+   neon_bitops, neon_bitops_q, neon_from_gp,
+   neon_from_gp_q, neon_move, neon_tbl3_tbl4, neon_zip_q, neon_to_gp,
+   neon_load_a, neon_load_b, neon_load_c, neon_load_d, neon_load_e,
+   neon_load_f, neon_store_a, neon_store_b, neon_store_complex,
+   unknown"
+  (cond [
+ (eq_attr "type" "neon_arith_acc, neon_reduc_add_acc,\
+  neon_reduc_add_acc_q")
+   (const_string "neon_arith_acc")
+ (eq_attr "type" "neon_arith_acc_q")
+   (const_string "neon_arith_acc_q")
+ (eq_attr "type" "neon_abs,neon_abs_q,neon_add, neon_add_q, 
neon_add_long,\
+  neon_add_widen, neon_neg, neon_neg_q,\
+  neon_reduc_add, neon_reduc_add_q,\
+  neon_reduc_add_long, neon_sub, neon_sub_q,\
+  neon_sub_long, neon_sub_widen, neon_logic,\
+  neon_logic_q, neon_tst, neon_tst_q,\
+  neon_compare, neon_compare_q,\
+  neon_compare_zero, neon_compare_zero_q,\
+  

Re: [PATCH] [PR87012] canonicalize ref type for tmpl arg

[Jason Merrill]

On 11/16/18 5:32 PM, Alexandre Oliva wrote:

When binding an object to a template parameter of reference type, we
take the address of the object and dereference that address.  The type
of the address may still carry (template) typedefs, but
verify_unstripped_args_1 rejects such typedefs other than in the top
level of template arguments.

We might special-case INDIRECT_REFs, but I suppose we're better off
canonicalizing the desired type for the address before converting to
it.

Regstrapped on i686- and x86_64-linux-gnu.  Ok to install?


for  gcc/cp/ChangeLog

PR c++/87012
* pt.c (canonicalize_type_argument): Declare earlier.
(convert_nontype_argument): Call it on addr expr.

for  gcc/testsuite/ChangeLog

PR c++/87012
* g++.dg/cpp0x/pr87012.C: New.
---
  gcc/cp/pt.c  |4 
  gcc/testsuite/g++.dg/cpp0x/pr87012.C |   11 +++
  2 files changed, 15 insertions(+)
  create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr87012.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index b58ec06a09e5..83d0a74b209f 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -227,6 +227,8 @@ static tree make_argument_pack (tree);
  static void register_parameter_specializations (tree, tree);
  static tree enclosing_instantiation_of (tree tctx);
  
+static tree canonicalize_type_argument (tree arg, tsubst_flags_t complain);

+
  /* Make the current scope suitable for access checking when we are
 processing T.  T can be FUNCTION_DECL for instantiated function
 template, VAR_DECL for static member variable, or TYPE_DECL for
@@ -7016,6 +7018,8 @@ convert_nontype_argument (tree type, tree expr, 
tsubst_flags_t complain)
return NULL_TREE;
}
  
+  type = canonicalize_type_argument (type, complain);


I would expect that this same issue would come up with other types; I 
think we want to fix this sooner, when we are figuring out what type we 
want to convert the argument to.


Jason

Re: [PATCH] [PR86747] tsubst friend tpl ctxt before looking it up for dupes

[Jason Merrill]

On 11/16/18 5:32 PM, Alexandre Oliva wrote:

When a member template is redeclared as a friend, we enter the context
of the member before looking it up, and then we check that the decls
are compatible.  However, when the member template references template
types of the enclosing context, say an enclosing template class, the
compare fails because the friend decl is already tsubsted, whereas the
looked up name isn't.

The problem is that the enclosing context is taken from the friend
declaration before tsubsting it, so we look up in the context of the
generic template instead of that of the tsubsted one we're
specializing.  The solution is to tsubst the enclosing context when
it's a non-namespace scope.

Regstrapped on i686- and x86_64-linux-gnu.  Ok to install?


OK.




for  gcc/cp/ChangeLog

PR c++/86747
* pt.c (tsubst_friend_class): Enter tsubsted class context.

for  gcc/testsuite/ChangeLog

PR c++/86747
* g++.dg/pr86747.C: New.
---
  gcc/cp/pt.c|5 -
  gcc/testsuite/g++.dg/pr86747.C |8 
  2 files changed, 12 insertions(+), 1 deletion(-)
  create mode 100644 gcc/testsuite/g++.dg/pr86747.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 83d0a74b209f..82c8019431b8 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -10568,7 +10568,10 @@ tsubst_friend_class (tree friend_tmpl, tree args)
if (TREE_CODE (context) == NAMESPACE_DECL)
  push_nested_namespace (context);
else
-push_nested_class (context);
+{
+  context = tsubst (context, args, tf_error, NULL_TREE);
+  push_nested_class (context);
+}
  
tmpl = lookup_name_real (DECL_NAME (friend_tmpl), /*prefer_type=*/false,

   /*non_class=*/false, /*block_p=*/false,
diff --git a/gcc/testsuite/g++.dg/pr86747.C b/gcc/testsuite/g++.dg/pr86747.C
new file mode 100644
index ..5b0a0bb95146
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr86747.C
@@ -0,0 +1,8 @@
+// { dg-do compile }
+
+template  class A {
+  template  class C; // #1
+  template  friend class C; // #2
+};
+
+A a;

Re: [PATCH[ Fix pr87269.C testcase

[Jason Merrill]

On 11/16/18 3:50 PM, Jakub Jelinek wrote:

On Fri, Nov 16, 2018 at 10:01:05AM -0500, Nathan Sidwell wrote:

2018-11-16  Nathan Sidwell  

PR c++/87269
* parser.c (lookup_literal_operator): Mark overload for keeping
when inside template.  Refactor.

* g++.dg/lookup/pr87269.C: New.


This test fails on i686-linux (and on any other where std::size_t is
not unsigned long.

Fixed thusly, tested on x86_64-linux and i686-linux, ok for trunk?


OK.  This sort of fix qualifies as obvious IMO.


2018-11-16  Jakub Jelinek  

PR c++/87269
* g++.dg/lookup/pr87269.C (std::size_t): New typedef.
(operator"" _a) Change unsigned long type to std::size_t.

--- gcc/testsuite/g++.dg/lookup/pr87269.C.jj2018-11-16 17:33:44.534188632 
+0100
+++ gcc/testsuite/g++.dg/lookup/pr87269.C   2018-11-16 21:48:00.243033194 
+0100
@@ -1,8 +1,12 @@
  // { dg-do compile { target c++11 } }
  // PR c++/87269 ICE failing to keep a lookup
  
+namespace std {

+  typedef decltype (sizeof (0)) size_t;
+}
+
  namespace {
-  void  operator"" _a (const char *, unsigned long) {}
+  void  operator"" _a (const char *, std::size_t) {}
  }
  
  void operator"" _a (unsigned long long);



Jakub

Re: [PATCH] [PR86823] retain deferred access checks from outside firewall

[Jason Merrill]

On 11/16/18 9:16 PM, Alexandre Oliva wrote:

We used to preserve deferred access check along with resolved template
ids, but a tentative parsing firewall introduced additional layers of
deferred access checks, so that we don't preserve the checks we
want to any more.

This patch collapses the access check levels introduced by the
firewall with those we wanted to preserve to begin with, saves them,
pushes them one more layer up so that the firewall doesn't drop them,
and then pushes new empty levels so balance out with the upcoming
popping.

We may want to avoid the pop_to_parent and one push after the save, and
instead leave the firewall's scope before the final pop_to_parent.
However, this patch is what I regression-tested successfully with
check-g++ on x86_64-linux-gnu.  Regstrapping on i686- and
x86_64-linux-gnu now.  Ok to install?


for  gcc/cp/ChangeLog

PR c++/86823
* parser.c (cp_parser_template_id): Merge access checks from
outside the tentative parsing firewall with those from inside,
and save them all with the template id token.

for  gcc/testsuite/ChangeLog

PR c++/86823
* g++.dg/pr86823.C: New.
---
  gcc/cp/parser.c|   12 
  gcc/testsuite/g++.dg/pr86823.C |   15 +++
  2 files changed, 27 insertions(+)
  create mode 100644 gcc/testsuite/g++.dg/pr86823.C

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index db0f0338179e..6a08b09715a7 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -16185,7 +16185,19 @@ cp_parser_template_id (cp_parser *parser,
 so the memory will not be reclaimed during token replacing below.  */
token->u.tree_check_value = ggc_cleared_alloc ();
token->u.tree_check_value->value = template_id;
+  /* Collapse the access check levels introduced by
+tentative_firewall with the one we introduced ourselves.  */
+  pop_to_parent_deferring_access_checks ();
+  pop_to_parent_deferring_access_checks ();
token->u.tree_check_value->checks = get_deferred_access_checks ();
+  /* Push the checks up one more level, so that the firewall
+doesn't drop them on the floor when we return.  Then, push
+empty levels back in place so that they are popped
+properly.  */
+  pop_to_parent_deferring_access_checks ();
+  push_deferring_access_checks (dk_deferred);
+  push_deferring_access_checks (dk_deferred);
+  push_deferring_access_checks (dk_deferred);


Hmm, I'm uncomfortable with how this depends on the specific 
implementation of tentative_firewall.  What do you think of this 
alternate approach (untested other than with the testcase)?


Jason
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index ac19cb4b9bb..c7ec7dcf413 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -16182,16 +16182,18 @@ cp_parser_template_id (cp_parser *parser,
    is_declaration,
    tag_type,
    _identifier);
+
+  /* Push any access checks inside the firewall we're about to create.  */
+  vec *checks = get_deferred_access_checks ();
+  pop_deferring_access_checks ();
   if (templ == error_mark_node || is_identifier)
-{
-  pop_deferring_access_checks ();
-  return templ;
-}
+return templ;
 
   /* Since we're going to preserve any side-effects from this parse, set up a
  firewall to protect our callers from cp_parser_commit_to_tentative_parse
  in the template arguments.  */
   tentative_firewall firewall (parser);
+  reopen_deferring_access_checks (checks);
 
   /* If we find the sequence `[:' after a template-name, it's probably
  a digraph-typo for `< ::'. Substitute the tokens and check if we can

Re: [PATCH v2] C++: improvements to diagnostics using %P (more PR c++/85110)

[Jason Merrill]

On 12/3/18 5:54 PM, David Malcolm wrote:

I was going to ping this patch:
   https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00875.html
but it has bit-rotted somewhat, so here's a refreshed version of it.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.

OK for trunk?

Thanks
Dave


Blurb from v1:

This patch is based on grepping the C++ frontend for %P
i.e. diagnostics that refer to a parameter number.  It fixes up
these diagnostics to highlight the pertinent param where appropriate
(and possible), along with various other tweaks, as described in the
ChangeLog.

gcc/cp/ChangeLog:
PR c++/85110
* call.c (conversion_null_warnings): Try to use the location of
the expression for the warnings.  Add notes showing the parameter
of the function decl, where available.
(get_fndecl_argument_location): Gracefully reject
non-FUNCTION_DECLs.  For implicitly-declared functions, use the
fndecl location rather than that of the param.
(maybe_inform_about_fndecl_for_bogus_argument_init): New function.
(convert_like_real): Use it in various places to avoid repetition.
* cp-tree.h (maybe_inform_about_fndecl_for_bogus_argument_init):
New declaration.
* decl2.c: Include "gcc-rich-location.h".
(check_default_args): Use the location of the parameter when
complaining about parameters with missing default arguments in
preference to that of the fndecl.
Attempt to record the location of first parameter with a default
argument and add it as a secondary range to such errors.
* typeck.c (convert_arguments): When complaining about parameters
with incomplete types, attempt to use the location of the
argument.  Where available, add a note showing the pertinent
parameter in the fndecl.
(convert_for_assignment): When complaining about bad conversions
at function calls, use the location of the unstripped argument.
(convert_for_initialization): When checking for bogus references,
add an auto_diagnostic_group, and update the note to use the
location of the pertinent parameter, rather than just the callee.

gcc/testsuite/ChangeLog:
PR c++/85110
* g++.dg/diagnostic/missing-default-args.C: New test.
* g++.dg/diagnostic/param-type-mismatch-3.C: New test.
* g++.dg/diagnostic/param-type-mismatch.C: Add tests for invalid
references and incomplete types.
* g++.dg/warn/Wconversion-null-4.C: New test.
---
  gcc/cp/call.c  | 88 ++
  gcc/cp/cp-tree.h   |  1 +
  gcc/cp/decl2.c | 16 +++-
  gcc/cp/typeck.c| 22 --
  .../g++.dg/diagnostic/missing-default-args.C   | 53 +
  .../g++.dg/diagnostic/param-type-mismatch-3.C  | 26 +++
  .../g++.dg/diagnostic/param-type-mismatch.C| 41 ++
  gcc/testsuite/g++.dg/warn/Wconversion-null-4.C | 43 +++
  8 files changed, 251 insertions(+), 39 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/diagnostic/missing-default-args.C
  create mode 100644 gcc/testsuite/g++.dg/diagnostic/param-type-mismatch-3.C
  create mode 100644 gcc/testsuite/g++.dg/warn/Wconversion-null-4.C

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index ee099cc..cfc5641 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -6681,16 +6681,24 @@ conversion_null_warnings (tree totype, tree expr, tree 
fn, int argnum)
if (null_node_p (expr) && TREE_CODE (totype) != BOOLEAN_TYPE
&& ARITHMETIC_TYPE_P (totype))
  {
-  location_t loc =
-   expansion_point_location_if_in_system_header (input_location);
-
if (fn)
-   warning_at (loc, OPT_Wconversion_null,
-   "passing NULL to non-pointer argument %P of %qD",
-   argnum, fn);
+   {
+ location_t loc = EXPR_LOC_OR_LOC (expr, input_location);
+ loc = expansion_point_location_if_in_system_header (loc);
+ auto_diagnostic_group d;
+ if (warning_at (loc, OPT_Wconversion_null,
+ "passing NULL to non-pointer argument %P of %qD",
+ argnum, fn))
+   inform (get_fndecl_argument_location (fn, argnum),
+   "  declared here");
+   }
else
-   warning_at (loc, OPT_Wconversion_null,
-   "converting to non-pointer type %qT from NULL", totype);
+   {
+ location_t loc
+   = expansion_point_location_if_in_system_header (input_location);
+ warning_at (loc, OPT_Wconversion_null,
+ "converting to non-pointer type %qT from NULL", totype);
+   }


Why is 'loc' different between the branches?


@@ -6698,9 +6706,15 @@ conversion_null_warnings (tree totype, tree expr, tree 
fn, int argnum)
   && TYPE_PTR_P 

[committed] Add test for PR85770.

[Jeff Law]

PR85770 is fixed by Segher's combiner patch to avoid combining hard
regs.  Presumably it helps because it gives the allocators more freedom.

I'm adding the testcase from the PR to the regression suite.

Jeff
commit 40fc691eac0ea9414f7908826c91afc70ff78617
Author: law 
Date:   Thu Dec 6 00:40:08 2018 +

PR rtl-optimization/85770
* gcc.target/i386/pr85770.c: New test.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@266839 
138bc75d-0d04-0410-961f-82ee72b054a4

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 8bed4b455e0..cc5d556eeca 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2018-12-05  Jeff Law  
+
+   PR rtl-optimization/85770
+   * gcc.target/i386/pr85770.c: New test.
+
 2018-12-05  Martin Sebor  
 
PR c/87028
diff --git a/gcc/testsuite/gcc.target/i386/pr85770.c 
b/gcc/testsuite/gcc.target/i386/pr85770.c
new file mode 100644
index 000..dbb685fd83f
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr85770.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -march=nano-1000 -fnon-call-exceptions 
-fno-tree-coalesce-vars" } */
+
+unsigned a, b, c, d, e, f, g, h, i;
+unsigned __int128 j;
+
+__int128 foo(char k, unsigned short l, unsigned m, unsigned n, __int128 o,
+ unsigned char p) {
+  long q;
+  p |= -k;
+  __builtin_add_overflow(p, m, );
+  m *= ~__builtin_clrsbll(0);
+  j = j >> (o & 127) | j << (o & 7);
+  return k + l + m + n + o + a + b + c + d + j + l + e + f + q + 4294967295 +
+ p + g + h + i;
+}
+

Re: Fortran patches

[Steve Kargl]

On Wed, Dec 05, 2018 at 04:48:28PM -0500, Fritz Reese wrote:
> On Wed, Dec 5, 2018 at 12:00 AM Steve Kargl
>  wrote:
> >
> > I intend to commit the attached patch on Saturday.
> 
> Thanks for the work. I assume the patch bootstraps and passes
> regression tests?

The patch passed regression testing on i586-*-freebsd.
I'll also do regression testing on x86_64-*-freebsd 
prior to the commit.

> RE:
> > PR fortran/88228
> > * expr.c (check_null, check_elemental): Work around -fdec and
> > initialization with logical operators operating on integers.
> 
> I plan to review this section of the patch later today -- though the
> patch hides the segfault from the PR, I need more time to determine
> whether it is correct and complete.

By the time the gfc_expr is given to check_check and check_elemental,
it has been reduced to a EXPR_CONSTANT, which neither routine expected.
I simply return early in that case.

> RE:
> >PR fortran/88139
> >* dump-parse-tree.c (write_proc): Alternate return.
> I dissent with this patch. The introduced error is meaningless and, as
> mentioned by comment #3 in the PR, avoiding the ICE in dump-parse-tree
> is not directly the issue. The code should be rejected in parsing. In
> gcc-8.1 the invalid code is accepted (without an ICE) even without the
> -fc-prototypes flag: I haven't finished building the compiler with
> your changes yet to see whether that is still true afterwards, but at
> least the test case doesn't try this, so I strongly suspect the patch
> is incomplete to fix the PR.
 
Comment #3 does not contain a patch to fix the problem elsewhere.

In F2003, 15.2.6 "Interoperability of procedures and procedure interfaces",
I cannot find a prohibition on an alternate return in a subroutine
interface with BIND(C).

I'm disinclined to let a patch fester in bugzilla to only attain
the same fate as my patch to PR68544.

> RE:
> >PR fortran/88205
> >* io.c (gfc_match_open): STATUS must be CHARACTER type.
> [...]
> >@@ -2161,6 +2167,12 @@ gfc_match_open (void)
> >
> >   if (!open->file && open->status)
> > {
> >+ if (open->status->ts.type != BT_CHARACTER)
> >+   {
> >+gfc_error ("STATUS must be a default character type at %C");
> >+goto cleanup;
> >+   }
> >+
> >  if (open->status->expr_type == EXPR_CONSTANT
> > && gfc_wide_strncasecmp (open->status->value.character.string,
> >   "scratch", 7) != 0)
> 
> Both resolve_tag() and is_char_type() actually already catch type
> mismatches for STATUS (the latter for constant expressions). The real
> problem is the following condition which checks STATUS before it has
> been processed yet, since NEWUNIT is processed before STATUS. I think
> the correct thing to do is actually to move the NEWUNIT/UNIT if-block
> after the STATUS if-block, rather than adding a new phrasing for the
> same error.

OK. I'll check to see if this works.

> Then we should see:
> 
> pr88205.f90:13:29:
>open (newunit=n, status=status)
>  1
> Error: STATUS requires a scalar-default-char-expr at (1)
> 
> RE:
> >PR fortran/88328
> >* io.c (resolve_tag_format): Detect zero-sized array.
> [...]
> >Index: gcc/fortran/io.c
> >===
> >--- gcc/fortran/io.c(revision 266718)
> >+++ gcc/fortran/io.c(working copy)
> >@@ -1636,6 +1636,12 @@ resolve_tag_format (gfc_expr *e)
> >  gfc_expr *r;
> >  gfc_char_t *dest, *src;
> >
> >+ if (e->value.constructor == NULL)
> >+   {
> >+ gfc_error ("FORMAT tag at %C cannot be a zero-sized array");
> >+ return false;
> >+   }
> >+
> >  n = 0;
> >  c = gfc_constructor_first (e->value.constructor);
> >  len = c->expr->value.character.length;
> [...]
> >@ -3231,12 +3257,21 @@ gfc_resolve_dt (gfc_dt *dt, locus *loc)
> > {
> >   gfc_expr *e;
> >   io_kind k;
> >+  locus loc_tmp;
> >
> >   /* This is set in any case.  */
> >   gcc_assert (dt->dt_io_kind);
> >   k = dt->dt_io_kind->value.iokind;
> >
> >-  RESOLVE_TAG (_format, dt->format_expr);
> >+  loc_tmp = gfc_current_locus;
> >+  gfc_current_locus = *loc;
> >+  if (!resolve_tag (_format, dt->format_expr))
> >+{
> >+  gfc_current_locus = loc_tmp;
> >+  return false;
> >+}
> >+  gfc_current_locus = loc_tmp;
> >+
> >   RESOLVE_TAG (_rec, dt->rec);
> >   RESOLVE_TAG (_spos, dt->pos);
> >   RESOLVE_TAG (_advance, dt->advance);
> 
> Is it really true that resolve_tag_format needs the locus at
> gfc_resolve_dt::loc instead of e->where as with the other errors in
> resolve_tag_format? If so, are the other errors also incorrect in
> using e->where? Might it then be better to pass loc from
> gfc_resolve_dt down to resolve_tag/RESOLVE_TAG and then
> resolve_tag_format, instead of swapping gfc_current_locus?

program p
   

Re: [PATCH]: Remove #ifdef PCC_BITFIELD_TYPE_MATTERS check from dwarf2out.c

[Jeff Law]

On 12/5/18 1:56 PM, Uros Bizjak wrote:
> This macro is always defined through defaults.h. We can remove #ifdef
> check from dwarf2out.c
> 
> 2018-12-05  Uros Bizjak  
> 
> * dwarf2out.c (field_byte_offset): Remove
> #ifdef PCC_BITFIELD_TYPE_MATTERS check.
> 
> Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
> 
> OK for mainline?
OK
jeff

Re: [patch,openacc] Propagate independent clause for OpenACC kernels pass

[Julian Brown]

On Tue, 4 Dec 2018 14:55:03 +0100
Richard Biener  wrote:

> On Tue, 4 Dec 2018, Jakub Jelinek wrote:
> 
> > On Mon, Dec 03, 2018 at 11:40:39PM +, Julian Brown wrote:  
> > > Jakub asked in the following email at the time of the patch
> > > submission for the gomp4 branch what the difference was between
> > > the new marked_independent flag and safelen == INT_MAX:
> > > 
> > >   https://gcc.gnu.org/ml/gcc-patches/2015-07/msg01100.html
> > > 
> > > If I understand the followup correctly,
> > > 
> > >   https://gcc.gnu.org/ml/gcc-patches/2015-07/msg01117.html
> > > 
> > > a setting of safelen > 1 means that up to that number of loop
> > > iterations can run together in lockstep (as if each insn in the
> > > loop was blindly rewritten to a safelen-width SIMD equivalent) --
> > > but anything that happens in iteration N + 1 cannot happen before
> > > something that happens in iteration N. Chung-Lin pointed out that
> > > OpenACC's semantics are even less strict (allowing iterations to
> > > proceed fully independently in an arbitrary order), so the
> > > marked_independent flag does carry non-redundant information --
> > > even with safelen set to INT_MAX.  
> > 
> > OpenMP 5 (not implemented in GCC 9 though) has order(concurrent)
> > clause for this (no cross-iteration dependencies at all, iterations
> > can be run in any order, in parallel etc.).
> > 
> > I believe it matches the can_be_parallel flag we now have, but I
> > remember there were some issues with that flag for use in DO
> > CONCURRENT.
> > 
> > Or do we want to have some other flag for really independent
> > iterations? What passes could use that?  Would the vectorizer
> > appreciate the stronger assertion in some cases?  
> 
> The vectorizer doesn't really care.  It would be autopar that should.
> The issue with using can_be_parallel for DO CONCURRENT was that the
> middle-end introduces non-trivial sharing between iterations,
> introducing dependences that then make the loop no longer
> can_be_parallel.  I believe similar things could happen with
> ->safelen (consider loop reversal and existing forward dependences).
> I guess we're simply lucky in that area ;)

I wondered if I should try modifying the patch to set the
can_be_parallel flag for kernels loops with an "independent" clause
instead (and try to address Jakub's other comments). Do I understand
right that the issue with the can_be_parallel flag is that it does not
necessarily guarantee safety of optimisations for loops which are
supposed to have fully-independent iterations, rather than that it has
different semantics from the proposed marked_independent flag?

However, it turns out that this patch has a dependency on this one:

  https://gcc.gnu.org/ml/gcc-patches/2018-09/msg01179.html

and, according to Cesar, that in turn has a dependency on another patch:

  https://gcc.gnu.org/ml/gcc-patches/2018-09/msg01189.html

so, it might take me a little time to untangle all that. Does the rough
idea sound plausible, though? Or is modifying this patch to use
can_be_parallel likely to just cause problems at present?

Thanks,

Julian

Question on Disable no throw for -flive-patching master option.

[Qing Zhao]

Hi, Honza,

I have one question relate to whether to disable nothrow for -flive-patching as 
following:

actually, there are two passes here:

1. local nothrow pass:  in this pass, nothrow attribute is set locally after 
analyzing every stmt of the function
body: 

unsigned int
pass_nothrow::execute (function *)
{
  struct cgraph_node *node;
  basic_block this_block;
…

}

2. nothrow propagation pass:  (it’s included in the ipa_pure_const pass as 
following, propagate the nothrow 
attribute along callcgraph)

unsigned int
pass_ipa_pure_const::
execute (function *)
{
  bool remove_p;

  /* Nothrow makes more function to not lead to return and improve
 later analysis.  */
  propagate_nothrow ();
  propagate_malloc ();
  remove_p = propagate_pure_const ();

  delete funct_state_summaries;
  return remove_p ? TODO_remove_functions : 0;
}

the nothrow propagation pass is included in ipa_pure_const pass, and is guarded 
by flag_ipa_pure_const, 
this flag_ipa_pure_const is disabled when -flive-patching is ON.


So, my question is:

shall we disable local nothrow pass as well when -flive-patching is ON?

my understanding is: we should. 

the reason is:   the local nothrow pass is setting the nothrow attribute for 
the current routine based on its
body,  and this “nothrow” attribute will be used  when generating EH code 
around callsite from the caller
of the routine. so the nothrow attribute might impact other routines than the 
current routine.  as a result,
we should disable this pass?

what’s your opinion on this?

thanks.

Qing

> On Nov 28, 2018, at 2:24 PM, Qing Zhao  wrote:
>> 
>> Shall we also disable nothrow or we will worry about C++ only ter?
> 
> This is also mainly for C++ applications, so currently should not be a 
> problem.
> But I can add a separate simple patch to add another flag to control nothrow 
> propagation and disable it when -flive-patching is ON.
> 
>> 
>> Patch is OK,
> 
> thanks for the review.
> 
> I will commit the patch very soon.
> 
> Qing
>> thanks!
>> Honza

Re: [PING #4][PATCH] avoid warning on constant strncpy until next statement is reachable (PR 87028)

[Jeff Law]

On 11/29/18 4:43 PM, Martin Sebor wrote:
> On 11/29/18 4:07 PM, Jeff Law wrote:
>> On 11/29/18 1:34 PM, Martin Sebor wrote:
>>> On 11/16/2018 02:07 AM, Richard Biener wrote:
 On Fri, Nov 16, 2018 at 4:12 AM Martin Sebor  wrote:
>
> Ping: https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01818.html
>
> Please let me know if there is something I need to change here
> to make the fix acceptable or if I should stop trying.

 I have one more comment about

 +  /* Defer warning (and folding) until the next statement in the basic
 + block is reachable.  */
 +  if (!gimple_bb (stmt))
 +return false;
 +

 it's not about the next statement in the basic-block being "reachable"
 (even w/o a CFG you can use gsi_next()) but rather that the next
 stmt isn't yet gimplified and thus not inserted into the gimple sequence,

 right?
>>>
>>> No, it's about the current statement not being associated with
>>> a basic block yet when the warning code runs for the first time
>>> (during gimplify_expr), and so gsi_next() returning null.
>>>
 You apply this to gimple_fold_builtin_strncpy but I'd rather
 see us not sprinkling this over gimple-fold.c but instead do this
 in gimplify.c:maybe_fold_stmt, delaying folding until say lowering.

 See the attached (untested).
>>>
>>> I would also prefer this solution.  I had tested it (in response
>>> to you first mentioning it back in September) and it causes quite
>>> a bit of fallout in tests that look for the folding to take place
>>> very early.  See the end of my reply here:
>>>
>>>    https://gcc.gnu.org/ml/gcc-patches/2018-09/msg01248.html
>>>
>>> But I'm willing to do the test suite cleanup if you think it's
>>> suitable for GCC 9.  (If you're thinking GCC 10 please let me
>>> know now.)
>> The fallout on existing tests is minimal.  What's more concerning is
>> that it doesn't actually pass the new test from Martin's original
>> submission.  We get bogus warnings.
>>
>> At least part of the problem is weakness in maybe_diag_stxncpy_trunc.
>> It can't handle something like this:
>>
>> test_literal (char * d, struct S * s)
>> {
>>    strncpy (d, "1234567890", 3);
>>    _1 = d + 3;
>>    *_1 = 0;
>> }
>>
>>
>> Note the pointer arithmetic between the strncpy and storing the NUL
>> terminator.
> 
> Right.  I'm less concerned about this case because it involves
> a literal that's obviously longer than the destination but it
> would be nice if the suppression worked here as well in case
> the literal comes from macro expansion.  It will require
> another tweak.
> 
> But the test from my patch passes with the changes to calls.c
> from my patch, so that's an improvement.
> 
> I have done the test suite cleanup in the attached patch.  It
> was indeed minimal -- not sure why I saw so many failures with
> my initial approach.
> 
> I can submit a patch to handle the literal case above as
> a followup unless you would prefer it done at the same time.
> 
> Martin
> 
> gcc-87028.diff
> 
> PR tree-optimization/87028 - false positive -Wstringop-truncation strncpy 
> with global variable source string
> 
> gcc/ChangeLog:
> 
>   PR tree-optimization/87028
>   * calls.c (get_attr_nonstring_decl): Avoid setting *REF to
>   SSA_NAME_VAR.
>   * gcc/gimple-low.c (lower_stmt): Delay foldin built-ins.
>   * gimplify (maybe_fold_stmt): Avoid folding statements that
>   don't belong to a basic block.
>   * tree.h (SSA_NAME_VAR): Update comment.
>   * tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Simplify.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR tree-optimization/87028
>   * c-c++-common/Wstringop-truncation.c: Remove xfails.
>   * gcc.dg/Wstringop-truncation-5.c: New test.
>   * gcc.dg/strcmpopt_1.c: Adjust.
>   * gcc.dg/tree-ssa/pr79697.c: Same.
I fixed up the ChangeLog a little and installed the patch.

Thanks,
jeff

libgo patch committed: Add precise stack scan support

[Ian Lance Taylor]

This libgo patch by Cherry Zhang adds support for precise stack
scanning to the Go runtime.  This uses per-function stack maps stored
in the exception tables in the language-specific data area.  The
compiler needs to generate these stack maps; currently this is only
done by a version of LLVM, not by GCC.  Each safepoint in a function
is associated with a (real or dummy) landing pad, and its "type info"
in the exception table is a pointer to the stack map. When a stack is
scanned, the stack map is found by the stack unwinding code.

For precise stack scan we need to unwind the stack. There are three cases:

- If a goroutine is scanning its own stack, it can unwind the stack
and scan the frames.

- If a goroutine is scanning another, stopped, goroutine, it cannot
directly unwind the target stack. We handle this by switching
(runtime.gogo) to the target g, letting it unwind and scan the stack,
and switch back.

- If we are scanning a goroutine that is blocked in a syscall, we send
a signal to the target goroutine's thread, and let the signal handler
unwind and scan the stack. Extra care is needed as this races with
enter/exit syscall.

Currently this is only implemented on GNU/Linux.

Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 266812)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-d3a98b7a9ea8032be22ebb3ea2f389ce22669d53
+edc7e7172e674b8c7e9c3caa30e24280cd289a9c
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/go/runtime/mgcmark.go
===
--- libgo/go/runtime/mgcmark.go (revision 266510)
+++ libgo/go/runtime/mgcmark.go (working copy)
@@ -664,7 +664,10 @@ func gcFlushBgCredit(scanWork int64) {
 }
 
 // We use a C function to find the stack.
-func doscanstack(*g, *gcWork)
+// Returns whether we succesfully scanned the stack.
+func doscanstack(*g, *gcWork) bool
+
+func doscanstackswitch(*g, *g)
 
 // scanstack scans gp's stack, greying all pointers found on the stack.
 //
@@ -691,7 +694,16 @@ func scanstack(gp *g, gcw *gcWork) {
return
case _Grunning:
// ok for gccgo, though not for gc.
-   case _Grunnable, _Gsyscall, _Gwaiting:
+   if usestackmaps {
+   print("runtime: gp=", gp, ", goid=", gp.goid, ", 
gp->atomicstatus=", readgstatus(gp), "\n")
+   throw("scanstack: goroutine not stopped")
+   }
+   case _Gsyscall:
+   if usestackmaps {
+   print("runtime: gp=", gp, ", goid=", gp.goid, ", 
gp->atomicstatus=", readgstatus(gp), "\n")
+   throw("scanstack: goroutine in syscall")
+   }
+   case _Grunnable, _Gwaiting:
// ok
}
 
@@ -701,15 +713,64 @@ func scanstack(gp *g, gcw *gcWork) {
}
 
// Scan the stack.
-   doscanstack(gp, gcw)
-
-   // Conservatively scan the saved register values.
-   scanstackblock(uintptr(unsafe.Pointer()), 
unsafe.Sizeof(gp.gcregs), gcw)
-   scanstackblock(uintptr(unsafe.Pointer()), 
unsafe.Sizeof(gp.context), gcw)
+   if usestackmaps {
+   g := getg()
+   if g == gp {
+   // Scan its own stack.
+   doscanstack(gp, gcw)
+   } else if gp.entry != nil {
+   // This is a newly created g that hasn't run. No stack 
to scan.
+   } else {
+   // Scanning another g's stack. We need to switch to 
that g
+   // to unwind its stack. And switch back after scan.
+   scanstackswitch(gp, gcw)
+   }
+   } else {
+   doscanstack(gp, gcw)
+
+   // Conservatively scan the saved register values.
+   scanstackblock(uintptr(unsafe.Pointer()), 
unsafe.Sizeof(gp.gcregs), gcw)
+   scanstackblock(uintptr(unsafe.Pointer()), 
unsafe.Sizeof(gp.context), gcw)
+   }
 
gp.gcscanvalid = true
 }
 
+// scanstackswitch scans gp's stack by switching (gogo) to gp and
+// letting it scan its own stack, and switching back upon finish.
+//
+//go:nowritebarrier
+func scanstackswitch(gp *g, gcw *gcWork) {
+   g := getg()
+
+   // We are on the system stack which prevents preemption. But
+   // we are going to switch to g stack. Lock m to block preemption.
+   mp := acquirem()
+
+   // The doscanstackswitch function will modify the current g's
+   // context. Preserve it.
+   // The stack scan code may call systemstack, which will modify
+   // gp's context. Preserve it as well so we can resume gp.
+   context := g.context
+   stackcontext := g.stackcontext
+   context2 := 

Re: [PATCH 1/2] asm qualifiers (PR55681)

[Segher Boessenkool]

Hi!

On Wed, Dec 05, 2018 at 04:47:37PM -0500, Jason Merrill wrote:
> On 12/2/18 11:38 AM, Segher Boessenkool wrote:
> >PR55681 observes that currently only one qualifier is allowed for
> >inline asm, so that e.g. "volatile asm" is allowed, "const asm" is also
> >okay (with a warning), but "const volatile asm" gives an error.  Also
> >"goto" has to be last.
> >
> >This patch changes things so that only "asm-qualifiers" are allowed,
> >that is "volatile" and "goto", in any combination, in any order, but
> >without repetitions.
> >
> >
> >2018-12-02  Segher Boessenkool  
> >
> > PR inline-asm/55681
> > * doc/extend.texi (Basic Asm): Update grammar.
> > (Extended Asm): Update grammar.
> >
> >gcc/c/
> > PR inline-asm/55681
> > * c-parser.c (c_parser_for_statement): Update grammar.  Allow any
> > combination of volatile and goto, in any order, without repetitions.
> >
> >gcc/cp/
> > PR inline-asm/55681
> > * parser.c (cp_parser_using_directive): Update grammar.  Allow any
> > combination of volatile and goto, in any order, without repetitions.
> 
> You don't actually change cp_parser_using_directive, despite what diff 
> says: you're changing cp_parser_asm_definition.

I trust diff too much, sigh.

> >+for (bool done = false; !done ; )
> >+  switch (cp_lexer_peek_token (parser->lexer)->keyword)
> >+{
> >+case RID_VOLATILE:
> >+  if (!volatile_p)
> >+{
> >+  /* Remember that we saw the `volatile' keyword.  */
> >+  volatile_p = true;
> >+  /* Consume the token.  */
> >+  cp_lexer_consume_token (parser->lexer);
> >+}
> >+  else
> >+done = true;
> >+  break;
> >+case RID_GOTO:
> >+  if (!goto_p && parser->in_function_body)
> >+{
> >+  /* Remember that we saw the `goto' keyword.  */
> >+  goto_p = true;
> >+  /* Consume the token.  */
> >+  cp_lexer_consume_token (parser->lexer);
> >+}
> >+  else
> >+done = true;
> >+  break;
> >+default:
> >+  done = true;
> >+}
> 
> An arguably simpler alternative to using the 'done' variable would be to 
> 'break' out of the loop after the switch, and have the consume_token 
> cases explicitly 'continue'.

Yeah, that is neater, continue only deals with the loop.  Nice.

> We also might remember the old tokens and give a more helpful error 
> message in the case of duplicate keywords.
> 
> But I won't insist on either of these, the C++ changes are OK as-is.

I'll commit it like this then, and work on the improvements afterwards
(they also apply to the C frontend).

Thanks for the review!


Segher

Re: [C++ Patch] Fix grokbitfield location

[Jason Merrill]

On 12/5/18 5:34 PM, Paolo Carlini wrote:

Hi,

On 05/12/18 20:31, Jason Merrill wrote:

On 12/5/18 7:45 AM, Paolo Carlini wrote:

Hi,

as mentioned in one of my last patches, we can now improve this 
location. Note: in the same function there are a few further issues 
which I mean to incrementally fix (eg, the diagnostics for 
warn_if_not_aligned ICEs for unnamed bit-fields). Tested x86_64-linux.
As long as we're messing with this diagnostic, let's also print the 
type in question.


Agreed. Thus I tested on x86_64-linux the below.

Thanks, Paolo.

/


-  if (!INTEGRAL_OR_ENUMERATION_TYPE_P (TREE_TYPE (value))
-  && (INDIRECT_TYPE_P (value)
-  || !dependent_type_p (TREE_TYPE (value
+  if (!INTEGRAL_OR_ENUMERATION_TYPE_P (type)
+  && (INDIRECT_TYPE_P (value) || !dependent_type_p (type)))


Hmm, surely this should be INDIRECT_TYPE_P (type).  OK with that change.

Jason

Re: PR C++/88114 - patch for destructor not generated for "virtual ~destructor() = default"

[Jason Merrill]

On 11/21/18 7:19 AM, Tobias Burnus wrote:

Hello all,

if a class contains any 'virtual ... = 0', it's an abstract class and for an
abstract class, the destructor not added to the vtable.

For a normal
   virtual ~class() { }
that's not a problem as the class::~class() destructor will be generated during
the parsing of the function.

But for
   virtual ~class() = default;
the destructor will be generated via mark_used via the vtable.


If one now declares a derived class and uses it, the class::~class() is 
generated
in that translation unit.  Unless, #pragma interface/implementation is used.

In that case, the 'default' destructor will never be generated.


The following code seems to work both for the big code and for the example;
without '#pragma implementation', the destructor is not generated for the 
example,
only with.

The patch survived boostrapping GCC with default languages on x86-64-gnu-linux
and "make check-g++".*

[One probably could get rid of some of the conditions for generating the code,
e.g. TREE_USED and DECL_DEFAULTED_FN are probably not both needed; one might
want to set some additional DECL to the fn decl.]


You can get at the destructor with CLASSTYPE_DESTRUCTOR rather than 
walking through TYPE_FIELDS.  I'd also check DECL_DEFAULTED_IN_CLASS_P.


I'd also do this in maybe_emit_vtables rather than here, so that it only 
happens once per class.


Jason

Re: [C++ Patch] Fix grokbitfield location

[Paolo Carlini]

Hi,

On 05/12/18 20:31, Jason Merrill wrote:

On 12/5/18 7:45 AM, Paolo Carlini wrote:

Hi,

as mentioned in one of my last patches, we can now improve this 
location. Note: in the same function there are a few further issues 
which I mean to incrementally fix (eg, the diagnostics for 
warn_if_not_aligned ICEs for unnamed bit-fields). Tested x86_64-linux.
As long as we're messing with this diagnostic, let's also print the 
type in question.


Agreed. Thus I tested on x86_64-linux the below.

Thanks, Paolo.

/


Index: cp/decl2.c
===
--- cp/decl2.c  (revision 266818)
+++ cp/decl2.c  (working copy)
@@ -1016,7 +1016,9 @@ grokbitfield (const cp_declarator *declarator,
 
   if (value == error_mark_node)
 return NULL_TREE; /* friends went bad.  */
-  if (TREE_TYPE (value) == error_mark_node)
+
+  tree type = TREE_TYPE (value);
+  if (type == error_mark_node)
 return value;
 
   /* Pass friendly classes back.  */
@@ -1023,11 +1025,12 @@ grokbitfield (const cp_declarator *declarator,
   if (VOID_TYPE_P (value))
 return void_type_node;
 
-  if (!INTEGRAL_OR_ENUMERATION_TYPE_P (TREE_TYPE (value))
-  && (INDIRECT_TYPE_P (value)
-  || !dependent_type_p (TREE_TYPE (value
+  if (!INTEGRAL_OR_ENUMERATION_TYPE_P (type)
+  && (INDIRECT_TYPE_P (value) || !dependent_type_p (type)))
 {
-  error ("bit-field %qD with non-integral type", value);
+  error_at (DECL_SOURCE_LOCATION (value),
+   "bit-field %qD with non-integral type %qT",
+   value, type);
   return error_mark_node;
 }
 
@@ -1048,7 +1051,7 @@ grokbitfield (const cp_declarator *declarator,
   return NULL_TREE;
 }
 
-  if (width && TYPE_WARN_IF_NOT_ALIGN (TREE_TYPE (value)))
+  if (width && TYPE_WARN_IF_NOT_ALIGN (type))
 {
   error ("cannot declare bit-field %qD with % type",
 DECL_NAME (value));
Index: testsuite/g++.dg/parse/bitfield3.C
===
--- testsuite/g++.dg/parse/bitfield3.C  (revision 266818)
+++ testsuite/g++.dg/parse/bitfield3.C  (working copy)
@@ -5,5 +5,5 @@ typedef void (func_type)();
 
 struct A
 {
-  friend func_type f : 2; /* { dg-error "with non-integral type" } */
+  friend func_type f : 2; /* { dg-error "20:bit-field .void f\\(\\). with 
non-integral type .func_type." } */
 };
Index: testsuite/g++.dg/parse/bitfield6b.C
===
--- testsuite/g++.dg/parse/bitfield6b.C (nonexistent)
+++ testsuite/g++.dg/parse/bitfield6b.C (working copy)
@@ -0,0 +1,4 @@
+typedef void a();
+struct A {
+a a1: 1;  // { dg-error "3:bit-field .void A::a1\\(\\). with non-integral type 
.void \\(A::\\)\\(\\)." }
+};

Re: Fortran patches

[Fritz Reese]

On Wed, Dec 5, 2018 at 12:00 AM Steve Kargl
 wrote:
>
> I intend to commit the attached patch on Saturday.

Thanks for the work. I assume the patch bootstraps and passes regression tests?

RE:
> PR fortran/88228
> * expr.c (check_null, check_elemental): Work around -fdec and
> initialization with logical operators operating on integers.

I plan to review this section of the patch later today -- though the
patch hides the segfault from the PR, I need more time to determine
whether it is correct and complete.

RE:
>PR fortran/88139
>* dump-parse-tree.c (write_proc): Alternate return.
I dissent with this patch. The introduced error is meaningless and, as
mentioned by comment #3 in the PR, avoiding the ICE in dump-parse-tree
is not directly the issue. The code should be rejected in parsing. In
gcc-8.1 the invalid code is accepted (without an ICE) even without the
-fc-prototypes flag: I haven't finished building the compiler with
your changes yet to see whether that is still true afterwards, but at
least the test case doesn't try this, so I strongly suspect the patch
is incomplete to fix the PR.

RE:
>PR fortran/88205
>* io.c (gfc_match_open): STATUS must be CHARACTER type.
[...]
>@@ -2161,6 +2167,12 @@ gfc_match_open (void)
>
>   if (!open->file && open->status)
> {
>+ if (open->status->ts.type != BT_CHARACTER)
>+   {
>+gfc_error ("STATUS must be a default character type at %C");
>+goto cleanup;
>+   }
>+
>  if (open->status->expr_type == EXPR_CONSTANT
> && gfc_wide_strncasecmp (open->status->value.character.string,
>   "scratch", 7) != 0)

Both resolve_tag() and is_char_type() actually already catch type
mismatches for STATUS (the latter for constant expressions). The real
problem is the following condition which checks STATUS before it has
been processed yet, since NEWUNIT is processed before STATUS. I think
the correct thing to do is actually to move the NEWUNIT/UNIT if-block
after the STATUS if-block, rather than adding a new phrasing for the
same error. Then we should see:

pr88205.f90:13:29:
   open (newunit=n, status=status)
 1
Error: STATUS requires a scalar-default-char-expr at (1)

RE:
>PR fortran/88328
>* io.c (resolve_tag_format): Detect zero-sized array.
[...]
>Index: gcc/fortran/io.c
>===
>--- gcc/fortran/io.c(revision 266718)
>+++ gcc/fortran/io.c(working copy)
>@@ -1636,6 +1636,12 @@ resolve_tag_format (gfc_expr *e)
>  gfc_expr *r;
>  gfc_char_t *dest, *src;
>
>+ if (e->value.constructor == NULL)
>+   {
>+ gfc_error ("FORMAT tag at %C cannot be a zero-sized array");
>+ return false;
>+   }
>+
>  n = 0;
>  c = gfc_constructor_first (e->value.constructor);
>  len = c->expr->value.character.length;
[...]
>@ -3231,12 +3257,21 @@ gfc_resolve_dt (gfc_dt *dt, locus *loc)
> {
>   gfc_expr *e;
>   io_kind k;
>+  locus loc_tmp;
>
>   /* This is set in any case.  */
>   gcc_assert (dt->dt_io_kind);
>   k = dt->dt_io_kind->value.iokind;
>
>-  RESOLVE_TAG (_format, dt->format_expr);
>+  loc_tmp = gfc_current_locus;
>+  gfc_current_locus = *loc;
>+  if (!resolve_tag (_format, dt->format_expr))
>+{
>+  gfc_current_locus = loc_tmp;
>+  return false;
>+}
>+  gfc_current_locus = loc_tmp;
>+
>   RESOLVE_TAG (_rec, dt->rec);
>   RESOLVE_TAG (_spos, dt->pos);
>   RESOLVE_TAG (_advance, dt->advance);

Is it really true that resolve_tag_format needs the locus at
gfc_resolve_dt::loc instead of e->where as with the other errors in
resolve_tag_format? If so, are the other errors also incorrect in
using e->where? Might it then be better to pass loc from
gfc_resolve_dt down to resolve_tag/RESOLVE_TAG and then
resolve_tag_format, instead of swapping gfc_current_locus?

RE:
>PR fortran/88048
>* resolve.c (check_data_variable): Convert gfc_internal_error to
>an gfc_error.  Add a nearby missing 'return false;'
[...]
>PR fortran/88025
>* expr.c (gfc_apply_init): Remove asserts and check for valid
>ts->u.cl->length.
[...]
>PR fortran/88116
>* simplify.c: Remove internal error and return gfc_bad_expr.

These look good.

A few pedantic comments:

RE:
> PR fortran/88269
> * io.c (io_constraint): Update macro.  Remove incompatible use
> of io_constraint and give explicit error.
[...]

There should be two separate references to io_constraint and
check_io_constraints:

> * io.c (io_constraint): Update macro.
> (check_io_constraints) Remove incompatible use
> of io_constraint and give explicit error.

RE:
> #define io_constraint(condition,msg,arg)\
> if (condition) \
>   {\
>-gfc_error(msg,arg);\
>+if ((arg)->lb != NULL) \
>+  

[Ping][Aarch64] v2: Implement Aarch64 SIMD ABI

[Steve Ellcey]

Ping.

This is a ping of my patch set to implement the SIMD ABI on Aarch64.

https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00636.html
https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00637.html
https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00639.html
https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00641.html
https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00642.html

Steve Ellcey
sell...@marvell.com

Re: [PATCH 1/2] asm qualifiers (PR55681)

[Jason Merrill]

On 12/2/18 11:38 AM, Segher Boessenkool wrote:

PR55681 observes that currently only one qualifier is allowed for
inline asm, so that e.g. "volatile asm" is allowed, "const asm" is also
okay (with a warning), but "const volatile asm" gives an error.  Also
"goto" has to be last.

This patch changes things so that only "asm-qualifiers" are allowed,
that is "volatile" and "goto", in any combination, in any order, but
without repetitions.


2018-12-02  Segher Boessenkool  

PR inline-asm/55681
* doc/extend.texi (Basic Asm): Update grammar.
(Extended Asm): Update grammar.

gcc/c/
PR inline-asm/55681
* c-parser.c (c_parser_for_statement): Update grammar.  Allow any
combination of volatile and goto, in any order, without repetitions.

gcc/cp/
PR inline-asm/55681
* parser.c (cp_parser_using_directive): Update grammar.  Allow any
combination of volatile and goto, in any order, without repetitions.


You don't actually change cp_parser_using_directive, despite what diff 
says: you're changing cp_parser_asm_definition.



+for (bool done = false; !done ; )
+  switch (cp_lexer_peek_token (parser->lexer)->keyword)
+   {
+   case RID_VOLATILE:
+ if (!volatile_p)
+   {
+ /* Remember that we saw the `volatile' keyword.  */
+ volatile_p = true;
+ /* Consume the token.  */
+ cp_lexer_consume_token (parser->lexer);
+   }
+ else
+   done = true;
+ break;
+   case RID_GOTO:
+ if (!goto_p && parser->in_function_body)
+   {
+ /* Remember that we saw the `goto' keyword.  */
+ goto_p = true;
+ /* Consume the token.  */
+ cp_lexer_consume_token (parser->lexer);
+   }
+ else
+   done = true;
+ break;
+   default:
+ done = true;
+   }


An arguably simpler alternative to using the 'done' variable would be to 
'break' out of the loop after the switch, and have the consume_token 
cases explicitly 'continue'.


We also might remember the old tokens and give a more helpful error 
message in the case of duplicate keywords.


But I won't insist on either of these, the C++ changes are OK as-is.

Jason

Re: [PATCH, OpenACC] (1/2) Fix implicit mapping for array slices on lexically-enclosing data constructs (PR70828)

[Julian Brown]

On Tue, 4 Dec 2018 15:02:15 +0100
Jakub Jelinek  wrote:

> On Tue, Aug 28, 2018 at 03:19:19PM -0400, Julian Brown wrote:
> > 2018-08-28  Julian Brown  
> > Cesar Philippidis  
> > 
> > PR middle-end/70828
> > 
> > gcc/
> > * gimplify.c (gimplify_omp_ctx): Add decl_data_clause hash
> > map. (new_omp_context): Initialise above.
> > (delete_omp_context): Delete above.
> > (gimplify_scan_omp_clauses): Scan for array mappings on
> > data constructs, and record in above map.
> > (gomp_needs_data_present): New function.
> > (gimplify_adjust_omp_clauses_1): Handle data mappings (e.g.
> > array slices) declared in lexically-enclosing data constructs.
> > * omp-low.c (lower_omp_target): Allow decl for bias not to
> > be present in omp context.
> > 
> > gcc/testsuite/
> > * c-c++-common/goacc/acc-data-chain.c: New test.
> > * gfortran.dg/goacc/pr70828.f90: New test.
> > * gfortran.dg/goacc/pr70828-2.f90: New test.
> > 
> > libgomp/
> > * testsuite/libgomp.oacc-c-c++-common/pr70828.c: New test.
> > * testsuite/libgomp.oacc-fortran/implicit_copy.f90: New
> > test.
> > * testsuite/libgomp.oacc-fortran/pr70828.f90: New test.
> > * testsuite/libgomp.oacc-fortran/pr70828-2.f90: New test.
> > * testsuite/libgomp.oacc-fortran/pr70828-3.f90: New test.
> > * testsuite/libgomp.oacc-fortran/pr70828-5.f90: New test.  
> 
> > --- a/gcc/gimplify.c
> > +++ b/gcc/gimplify.c
> > @@ -191,6 +191,7 @@ struct gimplify_omp_ctx
> >bool target_map_scalars_firstprivate;
> >bool target_map_pointers_as_0len_arrays;
> >bool target_firstprivatize_array_bases;
> > +  hash_map > *decl_data_clause;
> >  };
> >  
> >  static struct gimplify_ctx *gimplify_ctxp;
> > @@ -413,6 +414,7 @@ new_omp_context (enum omp_region_type
> > region_type) c->default_kind = OMP_CLAUSE_DEFAULT_SHARED;
> >else
> >  c->default_kind = OMP_CLAUSE_DEFAULT_UNSPECIFIED;
> > +  c->decl_data_clause = new hash_map
> > >;  
> 
> Not really happy about creating this unconditionally.  Can you leave
> it NULL by default and only initialize for contexts where it will be
> needed?
> 
> > @@ -7793,8 +7796,21 @@ gimplify_scan_omp_clauses (tree *list_p,
> > gimple_seq *pre_p, case OMP_TARGET:
> >   break;
> > case OACC_DATA:
> > - if (TREE_CODE (TREE_TYPE (decl)) != ARRAY_TYPE)
> > -   break;
> > + {
> > +   tree nextc = OMP_CLAUSE_CHAIN (c);
> > +   if (nextc
> > +   && OMP_CLAUSE_CODE (nextc) == OMP_CLAUSE_MAP
> > +   && (OMP_CLAUSE_MAP_KIND (nextc)
> > + == GOMP_MAP_FIRSTPRIVATE_POINTER
> > +   || OMP_CLAUSE_MAP_KIND (nextc) ==
> > GOMP_MAP_POINTER))
> > + {
> > +   tree base_addr = OMP_CLAUSE_DECL (nextc);
> > +   ctx->decl_data_clause->put (base_addr,
> > + std::make_pair (unshare_expr (c),
> > unshare_expr (nextc)));  
> 
> Don't like the wrapping here, can you just split it up:
>   std::pair p
> = std::make_pair (unshare_expr (c),
>   unshare_expr (nextc));
>   ctx->decl_data_clause->put (base_addr, p);
> or similar?
> 
> > +
> > +static std::pair *
> > +gomp_needs_data_present (tree decl)  
> 
> Would be helpful to have acc/oacc in the function name.
> > +{
> > +  gimplify_omp_ctx *ctx = NULL;
> > +
> > +  if (TREE_CODE (TREE_TYPE (decl)) != ARRAY_TYPE
> > +  && TREE_CODE (TREE_TYPE (decl)) != POINTER_TYPE
> > +  && (TREE_CODE (TREE_TYPE (decl)) != POINTER_TYPE
> > + || TREE_CODE (TREE_TYPE (TREE_TYPE (decl))) !=
> > ARRAY_TYPE))
> > +return NULL;
> > +
> > +  if (gimplify_omp_ctxp->region_type != ORT_ACC_PARALLEL
> > +  && gimplify_omp_ctxp->region_type != ORT_ACC_KERNELS)
> > +return NULL;  
> 
> And move this test to the top.
> 
> > --- a/gcc/omp-low.c
> > +++ b/gcc/omp-low.c
> > @@ -8411,9 +8411,10 @@ lower_omp_target (gimple_stmt_iterator
> > *gsi_p, omp_context *ctx) x = fold_convert_loc (clause_loc, type,
> > x); if (!integer_zerop (OMP_CLAUSE_SIZE (c)))
> >   {
> > -   tree bias = OMP_CLAUSE_SIZE (c);
> > -   if (DECL_P (bias))
> > - bias = lookup_decl (bias, ctx);
> > +   tree bias = OMP_CLAUSE_SIZE (c), remapped_bias;
> > +   if (DECL_P (bias)
> > +   && (remapped_bias = maybe_lookup_decl
> > (bias, ctx)))
> > + bias = remapped_bias;
> > bias = fold_convert_loc (clause_loc, sizetype,
> > bias); bias = fold_build1_loc (clause_loc, NEGATE_EXPR, sizetype,
> > bias);  
> 
> This is shared with OpenMP and must be conditionalized for OpenACC
> only.

Thanks for review! How's this version?

I took the liberty of fixing the patch for Fortran array-descriptor
mappings that use a PSET, also, and adding another test for that
functionality.

[PATCH]: Remove #ifdef PCC_BITFIELD_TYPE_MATTERS check from dwarf2out.c

[Uros Bizjak]

This macro is always defined through defaults.h. We can remove #ifdef
check from dwarf2out.c

2018-12-05  Uros Bizjak  

* dwarf2out.c (field_byte_offset): Remove
#ifdef PCC_BITFIELD_TYPE_MATTERS check.

Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

OK for mainline?

Uros.
Index: dwarf2out.c
===
--- dwarf2out.c (revision 266788)
+++ dwarf2out.c (working copy)
@@ -18985,7 +18985,6 @@ field_byte_offset (const_tree decl, struct vlr_con
   if (TREE_CODE (DECL_FIELD_BIT_OFFSET (decl)) != INTEGER_CST)
 return NULL;
 
-#ifdef PCC_BITFIELD_TYPE_MATTERS
   /* We used to handle only constant offsets in all cases.  Now, we handle
  properly dynamic byte offsets only when PCC bitfield type doesn't
  matter.  */
@@ -19100,7 +19099,6 @@ field_byte_offset (const_tree decl, struct vlr_con
   tree_result = wide_int_to_tree (sizetype, object_offset_in_bytes);
 }
   else
-#endif /* PCC_BITFIELD_TYPE_MATTERS */
 tree_result = byte_position (decl);
 
   if (ctx->variant_part_offset != NULL_TREE)

Re: [C++ PATCH] Fix clone_body (PR c++/86669)

[Jakub Jelinek]

On Wed, Dec 05, 2018 at 03:49:26PM -0500, Jason Merrill wrote:
> On 11/28/18 3:42 AM, Jakub Jelinek wrote:
> > Whenever we need to clone a cdtor (either because the target doesn't support
> > aliases the way we need, e.g. initlist105.C testcase on darwin, where it has
> > been originally reported, or when it has virtual bases, like the made up
> > initlist106.C on x86_64-linux), we rely on DECL_INITIAL of the local
> > variables being unshared, because the tree unsharing gimplify.c performs
> > doesn't unshare DECL_INITIAL.  clone_body has some code to recurse on the
> > DECL_INITIAL, but it handles just decls FOR_EACH_LOCAL_DECL walks.  I
> > believe it is generally ok that not all temporaries are covered in local
> > decls, the gimplifier should take care of those fine if we don't need
> > debug info for them.
> 
> I think any temporaries that have DECL_INITIAL should be pushed so that they
> end up in local_decls.  set_up_extended_ref_temp already adds a DECL_EXPR
> for it, I guess we need a pushdecl as well.  Though the comment for
> get_temp_regvar suggests that this is problematic somehow.

Ok, will play with it tomorrow.

Jakub

Re: [C++ PATCH] Fix clone_body (PR c++/86669)

[Jason Merrill]

On 11/28/18 3:42 AM, Jakub Jelinek wrote:

Whenever we need to clone a cdtor (either because the target doesn't support
aliases the way we need, e.g. initlist105.C testcase on darwin, where it has
been originally reported, or when it has virtual bases, like the made up
initlist106.C on x86_64-linux), we rely on DECL_INITIAL of the local
variables being unshared, because the tree unsharing gimplify.c performs
doesn't unshare DECL_INITIAL.  clone_body has some code to recurse on the
DECL_INITIAL, but it handles just decls FOR_EACH_LOCAL_DECL walks.  I
believe it is generally ok that not all temporaries are covered in local
decls, the gimplifier should take care of those fine if we don't need
debug info for them.


I think any temporaries that have DECL_INITIAL should be pushed so that 
they end up in local_decls.  set_up_extended_ref_temp already adds a 
DECL_EXPR for it, I guess we need a pushdecl as well.  Though the 
comment for get_temp_regvar suggests that this is problematic somehow.


Jason

[PATCH, i386]: Remove two obsolete/superfluous defines from cygming.h

[Uros Bizjak]

GROUP_BITFIELDS_BY_ALIGN was long gone, PCC_BITFIELD_TYPE_MATTERS
defaults to 1 in i386.h for all x86 targets.

2018-12-05  Uros Bizjak  

* config/i386/cygming.h (PCC_BITFIELD_TYPE_MATTERS): Remove.
(GROUP_BITFIELDS_BY_ALIGN): Ditto.

Tested by building crosscompiler to x86_64-w64-mingw32.

Committed to mainline SVN as obvious.

Uros.
Index: config/i386/cygming.h
===
--- config/i386/cygming.h   (revision 266788)
+++ config/i386/cygming.h   (working copy)
@@ -420,11 +420,6 @@
 #define BIGGEST_FIELD_ALIGNMENT 64
 #endif
 
-/* A bit-field declared as `int' forces `int' alignment for the struct.  */
-#undef PCC_BITFIELD_TYPE_MATTERS
-#define PCC_BITFIELD_TYPE_MATTERS 1
-#define GROUP_BITFIELDS_BY_ALIGN TYPE_NATIVE(rec)
-
 /* Enable alias attribute support.  */
 #ifndef SET_ASM_OP
 #define SET_ASM_OP "\t.set\t"

[PATCH] C/C++: don't suggest decls that are being initialized (PR c++/88320)

[David Malcolm]

PR c++/88320 reports that the C and C++ FEs can offer bogus suggestions
for misspelled identifiers within initializers, where the thing being
initialized is suggested.  If the user follows these suggestions,
it will lead to a -Wuninitialized warning.  For example:

test.c:9:19: error: 'aresults' was not declared in this scope; did you
   mean 'aresult'?
9 | int aresult = aresults + 1;
  |   ^~~~
  |   aresult

This patch filters out any decls being initialized when considering
candidates for suggestions, fixing the issue.

For the C frontend, it makes use of the pre-existing "constructor_decl"
and "initializer_stack" globals.

For the C++ frontend, it adds a couple of fields to cp_parser.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.

OK for trunk?

gcc/c/ChangeLog:
PR c++/88320
* c-decl.c (lookup_name_fuzzy): Don't suggest vars that we're
currently parsing an initializer for.
* c-tree.h (within_initializer_for_p): New decl.
* c-typeck.c (within_initializer_for_p): New function.

gcc/cp/ChangeLog:
PR c++/88320
* name-lookup.c (consider_binding_level): Don't suggest fields
if we're currently parsing a mem-initializer-list.  Don't suggest
vars that we're currently parsing an initializer for.
* parser.c (cp_parser_mem_initializer_list): Update
parser->within_mem_initializer_list on entry and exit.
(cp_parser_init_declarator): Push the decl to
parser->decls_being_initialized for the duration of the call to
cp_parser_initializer.
(within_initializer_for_p): New function.
(within_mem_initializer_list_p): New function.
* parser.h (struct cp_parser): Add fields
"decls_being_initialized" and "within_mem_initializer_list";
(within_initializer_for_p): New decl.
(within_mem_initializer_list_p): New decl.

gcc/testsuite/ChangeLog:
PR c++/88320
* c-c++-common/spellcheck-in-initializer.c: New test.
* g++.dg/spellcheck-in-initializer-2.C: New test.
---
 gcc/c/c-decl.c |  7 
 gcc/c/c-tree.h |  1 +
 gcc/c/c-typeck.c   | 22 
 gcc/cp/name-lookup.c   | 11 +-
 gcc/cp/parser.c| 42 ++
 gcc/cp/parser.h| 12 +++
 .../c-c++-common/spellcheck-in-initializer.c   | 22 
 gcc/testsuite/g++.dg/spellcheck-in-initializer-2.C | 26 ++
 8 files changed, 142 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/c-c++-common/spellcheck-in-initializer.c
 create mode 100644 gcc/testsuite/g++.dg/spellcheck-in-initializer-2.C

diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index b50f2bf..05be78c 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -4152,6 +4152,13 @@ lookup_name_fuzzy (tree name, enum 
lookup_name_fuzzy_kind kind, location_t loc)
  default:
break;
  }
+   /* Don't suggest vars that we're in the middle of parsing an
+  initializer for, since otherwise if the user follows the
+  suggestion they'll get a -Wuninitialized warning. */
+   if (VAR_P (binding->decl)
+   && within_initializer_for_p (binding->decl))
+ continue;
+
bm.consider (binding->id);
   }
 
diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index 5ed2f48..ce1e702 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -663,6 +663,7 @@ extern void store_init_value (location_t, tree, tree, tree);
 extern void maybe_warn_string_init (location_t, tree, struct c_expr);
 extern void start_init (tree, tree, int, rich_location *);
 extern void finish_init (void);
+extern bool within_initializer_for_p (tree);
 extern void really_start_incremental_init (tree);
 extern void finish_implicit_inits (location_t, struct obstack *);
 extern void push_init_level (location_t, int, struct obstack *);
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 8fbecfc..2573a42 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -8233,6 +8233,28 @@ finish_init (void)
   initializer_stack = p->next;
   free (p);
 }
+
+/* Return true if we're currently parsing an initializer for DECL.
+
+   This allows us to prevent offering DECL as a suggestion for an
+   unrecognized identifier - following such suggestions would lead
+   to -Wuninitialized warnings.  */
+
+bool
+within_initializer_for_p (tree decl)
+{
+  if (decl == constructor_decl)
+return true;
+
+  for (const struct initializer_stack *p = initializer_stack;
+   p; p = p->next)
+if (decl == p->decl)
+  return true;
+
+  return false;
+}
+
+
 
 /* Call here when we see the initializer is surrounded by braces.
This is instead of a call to push_init_level;
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 

Re: [committed] Handle targets with 2 byte wchar_t better in strlenopt-58.c

[Martin Sebor]

On 12/5/18 12:13 PM, Jeff Law wrote:

On 12/5/18 11:59 AM, David Edelsohn wrote:

Jeff,

Thanks for the patch.

I continue to see a failure on AIX 32 bit mode (2 byte wchar).

FAIL: gcc.dg/strlenopt-58.c scan-tree-dump-times optimized
"call_in_true_branch_not_eliminated" 0

I'm not certain if this is AIX-specific or more fallout from wchar
size.  I could ignore with target powerpc-ibm-aix* or with target !
4byte_wchar_t.

Any further insight / preference?

I didn't try to address that specific failure yet -- my patch just fixed
two earlier warnings.

That specific failure either requires a deeper testsuite fix for
possibly even a fix to the compiler -- I haven't really analyzed it yet
(it's a failure to optimize issue and I've generally avoided looking at
those).


You could xfail it on 4byte_wchar_t.


I fixed similar failures in gcc.c-torture/execute/memchr-1.c some
time ago by adjusting the test (r265020) so I suspect something
along those lines would work here as well if we would rather avoid
XFAILing it.

Martin

Re: [C++ Patch] Fix grokbitfield location

[Jason Merrill]

On 12/5/18 7:45 AM, Paolo Carlini wrote:

Hi,

as mentioned in one of my last patches, we can now improve this 
location. Note: in the same function there are a few further issues 
which I mean to incrementally fix (eg, the diagnostics for 
warn_if_not_aligned ICEs for unnamed bit-fields). Tested x86_64-linux.


As long as we're messing with this diagnostic, let's also print the type 
in question.


Jason

Re: [committed] Handle targets with 2 byte wchar_t better in strlenopt-58.c

[Jeff Law]

On 12/5/18 11:59 AM, David Edelsohn wrote:
> Jeff,
> 
> Thanks for the patch.
> 
> I continue to see a failure on AIX 32 bit mode (2 byte wchar).
> 
> FAIL: gcc.dg/strlenopt-58.c scan-tree-dump-times optimized
> "call_in_true_branch_not_eliminated" 0
> 
> I'm not certain if this is AIX-specific or more fallout from wchar
> size.  I could ignore with target powerpc-ibm-aix* or with target !
> 4byte_wchar_t.
> 
> Any further insight / preference?
I didn't try to address that specific failure yet -- my patch just fixed
two earlier warnings.

That specific failure either requires a deeper testsuite fix for
possibly even a fix to the compiler -- I haven't really analyzed it yet
(it's a failure to optimize issue and I've generally avoided looking at
those).


You could xfail it on 4byte_wchar_t.


jeff

Re: [committed] Handle targets with 2 byte wchar_t better in strlenopt-58.c

[David Edelsohn]

Jeff,

Thanks for the patch.

I continue to see a failure on AIX 32 bit mode (2 byte wchar).

FAIL: gcc.dg/strlenopt-58.c scan-tree-dump-times optimized
"call_in_true_branch_not_eliminated" 0

I'm not certain if this is AIX-specific or more fallout from wchar
size.  I could ignore with target powerpc-ibm-aix* or with target !
4byte_wchar_t.

Any further insight / preference?

Thanks, David

Re: [PATCH, og8] Add OpenACC 2.6 `acc_get_property' support

[Maciej W. Rozycki]

Hi Chung-Lin,

> > +module openacc_c_string
> > +  implicit none
> > +
> > +  interface
> > +function strlen (s) bind (C, name = "strlen")
> > +  use iso_c_binding, only: c_ptr, c_size_t
> > +  type (c_ptr), intent(in), value :: s
> > +  integer (c_size_t) :: strlen
> > +end function
> > +  end interface
> > +
> > +end module
> 
> > +subroutine acc_get_property_string_h (n, d, p, s)
> > +  use iso_c_binding, only: c_char, c_int, c_ptr, c_f_pointer
> > +  use openacc_internal, only: acc_get_property_string_l
> > +  use openacc_c_string, only: strlen
> > +  use openacc_kinds
> ...> +  pint = int (p, c_int)
> > +  cptr = acc_get_property_string_l (n, d, pint)
> > +  clen = int (strlen (cptr))
> > +  call c_f_pointer (cptr, sptr, [clen])
> 
> AFAIK, things like strlen are already available in iso_c_binding, in forms
> like "C_strlen".
> Can you check again if that 'openacc_c_string' module is really necessary?

 Any pointers please?

 I can't see `c_strlen' or any equivalent interface defined either in the 
Fortran 2003 language standard or in GCC documentation, and neither `grep' 
over the GCC tree shows anything relevant.  The `iso_c_binding' module 
defines only a bunch of procedures according to said documentation.  The 
`strlen' function provided here has been taken from one of our Fortran 
test cases, which strongly indicates there's no such API already available 
or whoever wrote the test case would have chosen to use it I suppose.

> > +union gomp_device_property_value
> > +GOMP_OFFLOAD_get_property (int n, int prop)
> > +{
> > +  union gomp_device_property_value propval = { .val = 0 };
> > +
> > +  pthread_mutex_lock (_dev_lock);
> > +
> > +  if (!nvptx_init () || n >= nvptx_get_num_devices ())
> > +{
> > +  pthread_mutex_unlock (_dev_lock);
> > +  return propval;
> > +}
> > +
> > +  switch (prop)
> > +{
> > +case GOMP_DEVICE_PROPERTY_MEMORY:
> > +  {
> > +   size_t total_mem;
> > +   CUdevice dev;
> > +
> > +   CUDA_CALL_ERET (propval, cuDeviceGet, , n);
> > +   CUDA_CALL_ERET (propval, cuDeviceTotalMem, _mem, dev);
> > +   propval.val = total_mem;
> > +  }
> > +  break;
> > +case GOMP_DEVICE_PROPERTY_FREE_MEMORY:
> > +  {
> > +   size_t total_mem;
> > +   size_t free_mem;
> > +   CUdevice ctxdev;
> > +   CUdevice dev;
> > +
> > +   CUDA_CALL_ERET (propval, cuCtxGetDevice, );
> > +   CUDA_CALL_ERET (propval, cuDeviceGet, , n);
> > +   if (dev == ctxdev)
> > + CUDA_CALL_ERET (propval, cuMemGetInfo, _mem, _mem);
> > +   else if (ptx_devices[n])
> > + {
> > +   CUcontext old_ctx;
> > +
> > +   CUDA_CALL_ERET (propval, cuCtxPushCurrent, ptx_devices[n]->ctx);
> > +   CUDA_CALL_ERET (propval, cuMemGetInfo, _mem, _mem);
> > +   CUDA_CALL_ASSERT (cuCtxPopCurrent, _ctx);
> > + }
> > +   else
> > + {
> > +   CUcontext new_ctx;
> > +
> > +   CUDA_CALL_ERET (propval, cuCtxCreate, _ctx, CU_CTX_SCHED_AUTO,
> > +   dev);
> > +   CUDA_CALL_ERET (propval, cuMemGetInfo, _mem, _mem);
> > +   CUDA_CALL_ASSERT (cuCtxDestroy, new_ctx);
> > + }
> 
> (I'm CCing Tom here, as he is maintainer for these parts)
> 
> As we discussed earlier on our internal list, I think properly using
> GOMP_OFFLOAD_init_device
> is the right way, instead of using the lower level CUDA context
> create/destroy.
> 
> I did not mean for you to first init the device and then immediately destroy
> it by
> GOMP_OFFLOAD_fini_device, just to obtain the property, but for you to just
> take the opportunity to initialize
> it for use, and leave it there until program exit. That should save resources
> overall.
> (BTW, CUDA contexts should be quite expensive to create/destroy, using a
> cuCtxCreate/Destroy pair is probably
> almost as slow)

 I have argued that this looks like a corner-case use case to me, as 
querying for the remaining (rather than total) memory available to a 
device that hasn't been (yet) used looks like of hardly any use to me, 
because obviously at such a stage no memory has been used.  The OpenACC 
standard does require us to handle such a request somehow, with returning 
0 being another option, however I thought we may well have a quick peek 
without pulling in all the state.

 I guess I have no strong opinion either way and I can adapt accordingly.

 NB that would have to be `gomp_init_device' rather than 
`GOMP_OFFLOAD_init_device' AFAICS.

  Maciej

Re: [PATCH, PPC] Fix PR88343.

[Segher Boessenkool]

On Wed, Dec 05, 2018 at 05:10:07PM +, Iain Sandoe wrote:
> Hi,
> 
> The PR is about unnecessary saves of the pic base register, it shows on m32 
> Linux and m32/m64 Darwin.
> 
> The fix is to check that we are in a pic mode and that the picbase has 
> actually been used.
> As a bonus, some #ifdef’d TARGET_MACHO code is no longer required.
> 
> Tested on power7, bootstrapped on Darwin (testing continues).
> 
> OK for trunk?

Okay for trunk if testing completes successfully :-)  Thanks!

> Branches?

Sure.  Please let is soak on trunk for a week first; and don't do 7
until 7.4 has been made?


> 2018-xx-xx  Segher Boessenkool  
>  Iain Sandoe  
> 
> gcc/
>   * config/rs6000/rs6000.c (save_reg_p): Do not save the picbase reg
>   unless it has been used.  (first_reg_to_save): Remove dead code.

Your changelogs still wrap (the second "(" should start a new line).


Segher

Re: [PATCH/coding style] clarify pointers and operators

[Segher Boessenkool]

On Wed, Dec 05, 2018 at 11:37:27AM -0600, Segher Boessenkool wrote:
> On Wed, Dec 05, 2018 at 10:04:56AM +, Richard Sandiford wrote:
> > Martin Sebor  writes:
> > > Martin suggested we update the Coding Conventions to describe
> > > the expected style for function declarations with a pointer
> > > return types, and for overloaded operators.  Below is the patch.
> > >
> > > As an aside, regarding the space convention in casts: a crude
> > > grep search yields about 10,000 instances of the "(type)x" kinds
> > > of casts in GCC sources and 40,000 of the preferred "(type) x"
> > > style with the space.  That's a consistency of only 80%.  Is
> > > it worth documenting a preference for a convention that's so
> > > inconsistently followed?
> > 
> > Just to be sure, does that grep include things like the go frontend
> > and its GCC interface, which deliberately don't follow GNU conventions?
> > A crude grep for me gives 92% consistency in gcc/* itself (excluding
> > subdirectories), although that's still disappointingly low...
> 
> I get:
> 
> $ grep '([a-zA-Z_][a-zA-Z0-9_]*)[a-zA-Z_]' *.[ch]|wc -l
> 454
> $ grep '([a-zA-Z_][a-zA-Z0-9_]*) ' *.[ch]|wc -l
> 28690
> 
> (that's gcc/*.[ch]).  About 1.6%, not so terrible.
> 
> $ grep '([a-zA-Z_][a-zA-Z0-9_]*\( \?\*\+\)\?)[a-zA-Z_]' *.[ch]|wc -l
> 631
> $ grep '([a-zA-Z_][a-zA-Z0-9_]*\( \?\*\+\)\?) ' *.[ch]|wc -l
> 29426
> 
> With pointer casts it is worse, but still only about 2.2%.
> 
> Files other than *.[ch] will probably have many more false hits?

Ugh.

$ grep '([a-zA-Z_][a-zA-Z0-9_]*\( \?\*\+\)\?)[a-zA-Z_]' *.[ch]|wc -l
631
$ grep '([a-zA-Z_][a-zA-Z0-9_]*\( \?\*\+\)\?) [a-zA-Z_]' *.[ch]|wc -l
3875

(REs, like all sharp tools, are dangerous).

14%?  Ouch.  And lamely filtering out some comments makes it only worse.


Segher

Re: [PATCH/coding style] clarify pointers and operators

[Martin Sebor]

On 12/5/18 3:04 AM, Richard Sandiford wrote:

Thanks for doing this,

Martin Sebor  writes:

Martin suggested we update the Coding Conventions to describe
the expected style for function declarations with a pointer
return types, and for overloaded operators.  Below is the patch.

As an aside, regarding the space convention in casts: a crude
grep search yields about 10,000 instances of the "(type)x" kinds
of casts in GCC sources and 40,000 of the preferred "(type) x"
style with the space.  That's a consistency of only 80%.  Is
it worth documenting a preference for a convention that's so
inconsistently followed?


Just to be sure, does that grep include things like the go frontend
and its GCC interface, which deliberately don't follow GNU conventions?
A crude grep for me gives 92% consistency in gcc/* itself (excluding
subdirectories), although that's still disappointingly low...


The only thing I excluded was tests.  I don't have the find/grep
script around anymore but I think this should be pretty close:

  $ find git-svn/gcc -name "*.[ch]" ! -path "*/testsuite/*" | xargs 
grep ") [a-zA-Z_0-9]" | wc -l

  42391

  $ find git-svn/gcc -name "*.[ch]" ! -path "*/testsuite/*" | xargs 
grep ")[a-zA-Z_0-9]" | wc -l

  9712

As I said, it was only a crude search so I'm sure it includes
stuff that's outside the policy (e.g., comments).

Martin

Re: Make claer, when contributions will be ignored

[Joseph Myers]

On Wed, 5 Dec 2018, Segher Boessenkool wrote:

> Patches are usually ignored because everyone thinks someone else will
> handle it.

And in this case, it looks like this patch would best be reviewed first in 
the GDB context - then once committed to binutils-gdb, the committer could 
post to gcc-patches (CC:ing build system maintainers) requesting a commit 
to GCC if they don't have write access to GCC themselves.  I consider 
synchronizing changes to such top-level files in either direction to be 
obvious and not to need a separate review.

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

Re: [PATCH/coding style] clarify pointers and operators

[Segher Boessenkool]

On Wed, Dec 05, 2018 at 10:04:56AM +, Richard Sandiford wrote:
> Martin Sebor  writes:
> > Martin suggested we update the Coding Conventions to describe
> > the expected style for function declarations with a pointer
> > return types, and for overloaded operators.  Below is the patch.
> >
> > As an aside, regarding the space convention in casts: a crude
> > grep search yields about 10,000 instances of the "(type)x" kinds
> > of casts in GCC sources and 40,000 of the preferred "(type) x"
> > style with the space.  That's a consistency of only 80%.  Is
> > it worth documenting a preference for a convention that's so
> > inconsistently followed?
> 
> Just to be sure, does that grep include things like the go frontend
> and its GCC interface, which deliberately don't follow GNU conventions?
> A crude grep for me gives 92% consistency in gcc/* itself (excluding
> subdirectories), although that's still disappointingly low...

I get:

$ grep '([a-zA-Z_][a-zA-Z0-9_]*)[a-zA-Z_]' *.[ch]|wc -l
454
$ grep '([a-zA-Z_][a-zA-Z0-9_]*) ' *.[ch]|wc -l
28690

(that's gcc/*.[ch]).  About 1.6%, not so terrible.

$ grep '([a-zA-Z_][a-zA-Z0-9_]*\( \?\*\+\)\?)[a-zA-Z_]' *.[ch]|wc -l
631
$ grep '([a-zA-Z_][a-zA-Z0-9_]*\( \?\*\+\)\?) ' *.[ch]|wc -l
29426

With pointer casts it is worse, but still only about 2.2%.

Files other than *.[ch] will probably have many more false hits?


Segher

Re: [PATCH][RFC] Poison bitmap_head->obstack

[Jeff Law]

On 12/5/18 7:58 AM, Richard Biener wrote:
> On Wed, 5 Dec 2018, Jeff Law wrote:
> 
>> On 12/4/18 6:16 AM, Richard Biener wrote:
>>>
>>> This tries to make bugs like that in PR88317 harder to create by
>>> introducing a bitmap_release function that can be used as
>>> pendant to bitmap_initialize for non-allocated bitmap heads.
>>> The function makes sure to poison the bitmaps obstack member
>>> so the obstack the bitmap was initialized with can be safely
>>> released.
>>>
>>> The patch also adds a default constructor to bitmap_head
>>> doing the same, but for C++ reason initializes to a
>>> all-zero bitmap_obstack rather than 0xdeadbeef because
>>> the latter isn't possible in constexpr context (it is
>>> by using unions but then things start to look even more ugly).
>>>
>>> The stage1 compiler might end up with a few extra runtime
>>> initializers but constexpr makes sure they'll vanish for
>>> later stages.
>>>
>>> I had to paper over that you-may-not-use-memset-to-zero classes
>>> with non-trivial constructors warning in two places and I
>>> had to teach gengtype about CONSTEXPR (probably did so in
>>> an awkward way - suggestions and pointers into gengtype
>>> appreciated).
>>>
>>> Bootstrapped (with host GCC 4.8) on x86_64-unknown-linux-gnu,
>>> testing in progress.
>>>
>>> The LRA issue seems to be rare enough (on x86_64...) that
>>> I didn't trip over it sofar.
>>>
>>> Comments?  Do we want this?  Not sure how we can easily
>>> discover all bitmap_clear () users that should really
>>> use bitmap_release (suggestion for a better name appreciated
>>> as well - I thought about bitmap_uninitialize)
>>>
>>> Richard.
>>>
>>> 2018-12-04  Richard Biener  
>>>
>>> * bitmap.c (bitmap_head::crashme): Define.
>>> * bitmap.h (bitmap_head): Add constexpr default constructor
>>> poisoning the obstack member.
>>> (bitmap_head::crashme): Declare.
>>> (bitmap_release): New function clearing a bitmap and poisoning
>>> the obstack member.
>>> * gengtype.c (main): Make it recognize CONSTEXPR.
>>>
>>> * lra-constraints.c (lra_inheritance): Use bitmap_release
>>> instead of bitmap_clear.
>>>
>>> * ira.c (ira): Work around warning.
>>> * regrename.c (create_new_chain): Likewise.
>> I don't see enough complexity in here to be concerning -- so if it makes
>> it harder to make mistakes, then I'm for it.
> 
> Any comment about the -Wclass-memaccess workaround sprinkling around two
> (void *) conversions?  I didn't dig deep enough to look for a more
> appropriate solution, also because there were some issues with older
> host compilers and workarounds we installed elsewhere...
Not really.  It was just a couple casts and a normal looking ctor, so it
didn't seem terrible.  Someone with more C++-fu may have a better
suggestion, but it seemed reasonable to me.

> 
> Otherwise yes, it makes it harder to do mistakes.  I'll probably
> use bitmap_head::crashme instead of 0xdeadbeef in bitmap_release.
> And of course we'd need to hunt down users of bitmap_clear that
> should be bitmap_release instead...
Right, but when we trip this kind of thing we'll know to starting
digging around the bitmap_clear calls :-)  That's a huge head start.

jeff

Re: Make claer, when contributions will be ignored

[Maciej W. Rozycki]

On Wed, 5 Dec 2018, Дилян Палаузов wrote:

> Can you please make it clear, when contributions will be ignored, or
> agree on some procedures, where all contributions are handled in
> reasonable time withot reminders, in a way that the processes work
> reliably.

1. You do not have a contract in place that would guarantee you any 
   specific timelines; if you want that, then hire someone to do the
   maintainer's duties for you.

2. Maintainers are volunteers working on the best-effort basis, and
   have other duties.

3. 

4. Wrong mailing list for GDB matters.

 HTH,

  Maciej

Re: [doc PATCH] update attribute docs for C++

[Martin Sebor]

On 12/4/18 8:49 PM, Sandra Loosemore wrote:

On 12/4/18 8:13 PM, Martin Sebor wrote:

On 12/4/18 2:04 PM, Sandra Loosemore wrote:

On 12/4/18 9:26 AM, Martin Sebor wrote:


[snip]

+The keyword @code{__attribute__} allows you to specify various special
+properties of types.  Some type attributes apply only to structure and
+union types, and in C++, also class types, while others can apply to
+any type defined via a @code{typedef} declaration.  Unless otherwise
+specified, the same restrictions and effects apply to attributes 
regardless
+of whether a type is a trivial structure or a C++ class with 
user-defined

+constructors, destructors, or a copy assignment.


And here I would really prefer to use standard terminology than 
trying to inaccurately summarize what the terminology means.  E.g.


"...whether or not a class is trivial (in the C++11 sense) or POD (in 
C++98)."


This doesn't say what we want to say (nor is it accurate).

Here are the user's questions again:

   The documentation should clarify how it handles structs/
   classes/unions and references.  Does it threat references
   like pointers? Does it only allow PODs/trivial types to be
   returned, or does it invoke the copy constructor, when it
   is used again?

They were about const/pure but the same questions could be asked
about other attributes as well.  The simple answer I'm trying to
give is that it doesn't matter: (unless the manual says otherwise)
references [to pointers] are treated the same as pointers, and
there is no difference between functions that take arguments or
return classes with user-defined ctors and plain old C structs,
or between attributes applied to such types.  It doesn't help
to use C++ standard terms when they are subtly or substantially
different between C++ revisions, and then try to draw
a distinction between those different terms, when they don't
matter.


I'm getting even more confused about what you're trying to communicate 
here.  :-(


I explained the intent above:  "The simple answer I'm trying to
give is that [whether an attribute applies to a struct or a C++
class] doesn't matter."

It's really that simple so I'm not sure why we are spending so
much time on it.  Far more time even than it took me to explain
it to the user.

What is the "it" referenced in the user's questions you quoted?  The 
const/pure attributes?  Those are function attributes.  The text you are 
adding is in the type attribute section, so it seemed like it was trying 
to address a different problem: stating that you can attach type 
attributes to any struct/class type whether or not it is a "trivial" 
class, by some definition of that term.  If that's not the purpose of 
this paragraph, what is it?


Again: the purpose is to explain that it doesn't matter whether
an attribute applies to a struct or a C++ class with ctors, in
any context with any attribute, either function, or variable,
or even type(*).

I answered the user's question below (and I CC'd you on it for
context):

  https://gcc.gnu.org/ml/gcc/2018-11/msg00146.html

The "it" in the question refers to GCC in the context of
the const and pure attributes.  As I explained above, the same
question could be asked about other attributes, including type
attributes.  The manual has historically referred to C structs
and hasn't consistently mentioned C++ classes so I can see how
someone might wonder if they are treated differently.
The purpose of this change is to make it clear that they
are not.

Martin

[*] For instance, that the effect of the attribute here:

  struct __attribute__ ((aligned (32))) A
  {
int i;
  };

is the same as its effect here:

  class __attribute__ ((aligned (32))) B
  {
  public:
B ();
~B ();
int i;
  };

The description of the aligned attribute in the Common Type
Attributes chapter of the manual refers to "the alignment of
any given struct or union type" and doesn't mention classes,
so one might wonder whether the attribute has a different
effect on those.  Note that this is in contrast to the packed
attribute in the same chapter which does mention C++ classes:

  This attribute, attached to a struct, union, or C++ class
  type definition, ...

To make things clear, we could (and at some point perhaps
should) also go and edit all the places that talk about
structs and unions and mention C++ classes.  It won't
unambiguously answer the user's question about PODs vs
non-trivial classes with ctors, but it would help.

Re: Make claer, when contributions will be ignored

[Segher Boessenkool]

Hi!

On Wed, Dec 05, 2018 at 07:36:09AM +, Дилян Палаузов wrote:
> on 27th October I sent to gcc-patches a mail with the subject “Don’t
> build gdb/readline/libreadline.a, when --with-system-readline is
> supplied”

The patch looks fine to me, fwiw.

> and on 14th November I sent a reminder.  I got no answer. 
> Before sending the emails I filled a bugzilla ticket.
> 
> Can you please make it clear, when contributions will be ignored, or
> agree on some procedures, where all contributions are handled in
> reasonable time withot reminders, in a way that the processes work
> reliably.

You can cc: people you think will be able to handle it.  You can ping
your patches regularly (say, every week).

Patches are usually ignored because everyone thinks someone else will
handle it.


Segher

[PATCH, PPC] Fix PR88343.

[Iain Sandoe]

Hi,

The PR is about unnecessary saves of the pic base register, it shows on m32 
Linux and m32/m64 Darwin.

The fix is to check that we are in a pic mode and that the picbase has actually 
been used.
As a bonus, some #ifdef’d TARGET_MACHO code is no longer required.

Tested on power7, bootstrapped on Darwin (testing continues).

OK for trunk?

Branches?

Iain

2018-xx-xx  Segher Boessenkool  
   Iain Sandoe  

gcc/
* config/rs6000/rs6000.c (save_reg_p): Do not save the picbase reg
unless it has been used.  (first_reg_to_save): Remove dead code.


diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index dfd5303..380cf9d 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -23961,7 +23961,7 @@ save_reg_p (int reg)
return true;
 
   if ((DEFAULT_ABI == ABI_V4 || DEFAULT_ABI == ABI_DARWIN)
- && flag_pic)
+ && flag_pic && crtl->uses_pic_offset_table)
return true;
 }
 
@@ -23981,13 +23981,6 @@ first_reg_to_save (void)
 if (save_reg_p (first_reg))
   break;
 
-#if TARGET_MACHO
-  if (flag_pic
-  && crtl->uses_pic_offset_table
-  && first_reg > RS6000_PIC_OFFSET_TABLE_REGNUM)
-return RS6000_PIC_OFFSET_TABLE_REGNUM;
-#endif
-
   return first_reg;
 }
 

Re: [PATCH] v4: C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850)

[David Malcolm]

On Wed, 2018-12-05 at 11:03 -0500, Jason Merrill wrote:
> On Mon, Dec 3, 2018 at 5:14 PM Joseph Myers 
> wrote:
> > 
> > On Sat, 1 Dec 2018, Jason Merrill wrote:
> > 
> > > Hmm, it looks like the C front-end comptypes will return 1 for
> > > e.g. enum and
> > > int.  It seems to me that what you want for this warning is
> > > actually to check
> > > for the same type.  Perhaps you want to use
> > > comptypes_check_different_types?
> > > Joseph would know better what's correct for the C front-end.
> > 
> > Well, it's valid to pass a pointer to enum where a pointer to the
> > compatible integer type is required, or vice versa, but I don't
> > have
> > advice on which cases you want to accept for this particular fix-
> > it.
> 
> Since the diagnostic is about returning, e.g. an int when an int* is
> needed, or vice versa, I don't think considering that kind of
> conversion is helpful.

FWIW I'd be happy to use simple pointer equality of the types for the C
FE, so that we only emit the suggestion for an exact match (better to
fail to offer a suggestion than to offer a bogus one, I think).

Dave

Re: [PATCH 2/9]: C++ P0482R5 char8_t: Core language support

[Jason Merrill]

On 12/5/18 2:09 AM, Tom Honermann wrote:

On 12/3/18 5:01 PM, Jason Merrill wrote:

On 12/3/18 4:51 PM, Jason Merrill wrote:

On 11/5/18 2:39 PM, Tom Honermann wrote:
This patch adds support for the P0482R5 core language changes.  This 
includes:

- The -fchar8_t and -fno_char8_t command line options.
- char8_t as a keyword.
- The char8_t builtin type as a non-aliasing unsigned integral
   character type of size 1.
- Use of char8_t as a simple type specifier.
- u8 character literals with type char8_t.
- u8 string literals with type array of const char8_t.
- User defined literal operators that accept char8_1 and char8_t 
pointer

   types.
- New __cpp_char8_t predefined feature test macro.
- New __CHAR8_TYPE__ and __GCC_ATOMIC_CHAR8_T_LOCK_FREE predefined
   macros .
- Name mangling and demangling for char8_t (using Du).

gcc/ChangeLog:

2018-11-04  Tom Honermann  

  * defaults.h: Define CHAR8_TYPE.

gcc/c-family/ChangeLog:

2018-11-04  Tom Honermann  
  * c-family/c-common.c (c_common_reswords): Add char8_t.
  (fix_string_type): Use char8_t for the type of u8 string 
literals.

  (c_common_get_alias_set): char8_t doesn't alias.
  (c_common_nodes_and_builtins): Define char8_t as a builtin 
type in

  C++.
  (c_stddef_cpp_builtins): Add __CHAR8_TYPE__.
  (keyword_begins_type_specifier): Add RID_CHAR8.
  * gcc/c-family/c-common.h (rid): Add RID_CHAR8.
  (c_tree_index): Add CTI_CHAR8_TYPE and CTI_CHAR8_ARRAY_TYPE.
  Define D_CXX_CHAR8_T and D_CXX_CHAR8_T_FLAGS.
  Define char8_type_node and char8_array_type_node.
  * c-family/c-cppbuiltin.c (cpp_atomic_builtins): Predefine
  __GCC_ATOMIC_CHAR8_T_LOCK_FREE.
  (c_cpp_builtins): Predefine __cpp_char8_t.
  * c-family/c-lex.c (lex_string): Use char8_array_type_node as the
  type of CPP_UTF8STRING.
  (lex_charconst): Use char8_type_node as the type of CPP_UTF8CHAR.
  * c-family/c.opt: Add the -fchar8_t command line option.

gcc/c/ChangeLog:

2018-11-04  Tom Honermann  

  * c/c-typeck.c (char_type_p): Add char8_type_node.
  (digest_init): Handle initialization by a u8 string literal of
  char8_t type.

gcc/cp/ChangeLog:

2018-11-04  Tom Honermann  

  * cp/cvt.c (type_promotes_to): Handle char8_t promotion.
  * cp/decl.c (grokdeclarator): Handle invalid type specifier
  combinations involving char8_t.
  * cp/lex.c (init_reswords): Add char8_t as a reserved word.
  * cp/mangle.c (write_builtin_type): Add name mangling for char8_t
  (Du).
  * cp/parser.c (cp_keyword_starts_decl_specifier_p,
  cp_parser_simple_type_specifier): Recognize char8_t as a simple
  type specifier.
  (cp_parser_string_literal): Use char8_array_type_node for the 
type

  of CPP_UTF8STRING.
  (cp_parser_set_decl_spec_type): Tolerate char8_t typedefs in 
system

  headers.
  * cp/rtti.c (emit_support_tinfos): type_info support for char8_t.
  * cp/tree.c (char_type_p): Recognize char8_t as a character type.
  * cp/typeck.c (string_conv_p): Handle conversions of u8 string
  literals of char8_t type.
  (check_literal_operator_args): Handle UDLs with u8 string 
literals

  of char8_t type.
  * cp/typeck2.c (digest_init_r): Disallow initializing a char 
array

  with a u8 string literal.

libiberty/ChangeLog:

2018-10-31  Tom Honermann  
  * cp-demangle.c (cplus_demangle_builtin_types,
  cplus_demangle_type): Add name demangling for char8_t (Du).
  * cp-demangle.h: Increase D_BUILTIN_TYPE_COUNT to accommodate the
  new char8_t type.



@@ -3543,6 +3556,10 @@ c_common_get_alias_set (tree t)
   if (!TYPE_P (t))
 return -1;



+  /* Unlike char, char8_t doesn't alias. */
+  if (flag_char8_t && t == char8_type_node)
+    return -1;


This seems unnecessary; doesn't the existing code have the same 
effect? I think we could do with just an adjustment to the existing 
comment.
I'm not sure.  I had concerns about unintended matching due to char8_t 
having an underlying type of unsigned char.


That shouldn't be a problem: if char8_t is a distinct type, it won't 
match unsigned char, and if it's the same as unsigned char, flag_char8_t 
will be false.



+  else if (flag_char8_t && TREE_TYPE (value) == char8_array_type_node)
+  || (flag_char8_t && type == char8_type_node)
+  bool char8_array = (flag_char8_t && !!comptypes (typ1, 
char8_type_node));

+   || (flag_char8_t && type == char8_type_node
In many places you check the flag and then for one of the char8 
types. Since the types won't be used without the flag, checking the 
flag seems redundant?


This was again protection against unintended matching of the underlying 
unsigned char type, particularly when compiling as C. char8_type_node is 
constructed (in c_common_nodes_and_builtins) following the pattern in 
place for char16_t and char32_t with the following code:


+  char8_type_node = get_identifier (CHAR8_TYPE);
+  char8_type_node = TREE_TYPE 

Re: [PATCH] be more permissive about function alignments (PR 88208)

[Rainer Orth]

Hi Martin,

>>> PS I'm not happy about duplicating the same test across all those
>>> targets.  It would be much nicer to have a single test somewhere
>>> in dg.exp #include a target-specific header with macros describing
>>> the target-specific parameters.
>>
>> why so complicated?  Just have a single attr-aligned.c test, restricted
>> to the target cpus it supports via dg directives and with
>> MINALIGN/MAXALIGN definitions controlled by appropriate target macros?
>
> I have done that in the past(*) but hardcoding target-specific
> assumptions into a general test didn't feel right either given
> the design of the test suite (separate target tests).
>
> [*] see the tangle of #ifdefs in gcc/testsuite/gcc.dg/attr-aligned.c

however, gcc.target, g++.target are only meant for tests that only make
sense on a particular target and cannot work elsewhere.  The current
amount of duplication between the various gcc.target/*/attr-aligned.c
tests (where the only variation I could see are the MINALIGN/MAXALIGN
definitions) seems way worse.  For someone adapting the test to his
target, it's way easier to skim through the variations (hopefully with
appropriate comments where non-obvious) in a single file rather than
looking at many almost identical files in gcc.target.

Rainer

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

Re: [PATCH] be more permissive about function alignments (PR 88208)

[Martin Sebor]

On 12/5/18 6:09 AM, Rainer Orth wrote:

Hi Martin,


The tests for the new __builtin_has_attribute function have been
failing on a number of targets because of a couple of assumptions
that only hold on some.

First, they expect that it's safe to apply attribute aligned with
a smaller alignment than the target provides when GCC rejects such
arguments.  The tests pass on i86 and elsewhere but fail on
strictly aligned targets like aarch64 or sparc.  After some testing
and thinking I don't think this is helpful -- I believe it's better
to instead silently accept attributes that ask for a less restrictive
alignment than the function ultimately ends up with (see * below).
This is what testing shows Clang does on those targets.  The attached
patch implements this change.

Second, the tests assume that the priority forms of the constructor
and destructor attributes are universally supported.  That's also
not the case, even though the manual doesn't mention that.  To
avoid these failures the attached patch moves the priority forms
of the attribute constructor and destructor tests into its own
file that's compiled only for init_priority targets.

Finally, I noticed that attribute aligned accepts zero as
an argument even though it's not a power of two as the manual
documents as a precondition (zero is treated the same as
the attribute without an argument).  A zero argument is likely
to be a mistake, especially when the zero comes from macro
expansion, that users might want to know about.  Clang rejects
a zero with an error but I think a warning is more in line with
established GCC practice, so the patch also implements that.

Besides x86_64-linux, I tested this change with cross-compilers
for aarch64-linux-elf, powerpc64le-linux, and sparc-solaris2.11.
I added tests for the changed aligned attribute for those targets
To make the gcc.dg/builtin-has-attribute.c test pass with
the cross-compilers I changed from a runtime test into a compile
only one.

Martin

PS I'm not happy about duplicating the same test across all those
targets.  It would be much nicer to have a single test somewhere
in dg.exp #include a target-specific header with macros describing
the target-specific parameters.


why so complicated?  Just have a single attr-aligned.c test, restricted
to the target cpus it supports via dg directives and with
MINALIGN/MAXALIGN definitions controlled by appropriate target macros?


I have done that in the past(*) but hardcoding target-specific
assumptions into a general test didn't feel right either given
the design of the test suite (separate target tests).

[*] see the tangle of #ifdefs in gcc/testsuite/gcc.dg/attr-aligned.c



Besides, the new gcc.target/sparc/attr-aligned.c test currently FAILs on
64-bit sparc:

FAIL: gcc.target/sparc/attr-aligned.c (test for excess errors)

Excess errors:
/vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.target/sparc/attr-aligned.c:29:1: error: 
static assertion failed: "alignof (f_) == MAXALIGN"

alignof (f_) is 16 for sparcv9.  The following patch fixes this, tested
on sparc-sun-solaris2.11 with -m32 and -m64.

Ok for mainline?


Thanks!
Martin

Re: [PATCH] v4: C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850)

[Jason Merrill]

On Mon, Dec 3, 2018 at 5:14 PM Joseph Myers  wrote:
>
> On Sat, 1 Dec 2018, Jason Merrill wrote:
>
> > Hmm, it looks like the C front-end comptypes will return 1 for e.g. enum and
> > int.  It seems to me that what you want for this warning is actually to 
> > check
> > for the same type.  Perhaps you want to use comptypes_check_different_types?
> > Joseph would know better what's correct for the C front-end.
>
> Well, it's valid to pass a pointer to enum where a pointer to the
> compatible integer type is required, or vice versa, but I don't have
> advice on which cases you want to accept for this particular fix-it.

Since the diagnostic is about returning, e.g. an int when an int* is
needed, or vice versa, I don't think considering that kind of
conversion is helpful.

Jason

Re: [PATCH] [PR86397] set p_t_decl while canonicalizing eh specs for mangling

[Jason Merrill]

OK, thanks.
On Wed, Dec 5, 2018 at 1:32 AM Alexandre Oliva  wrote:
>
> On Nov 29, 2018, Jason Merrill  wrote:
>
> > Let's go with this.  And remove the comment.
>
> > And the !processing_template_decl is also redundant, since that's
> > checked at the top of value_dependent_expression_p.
>
> I've tested this on i686- and x86_64-linux-gnu.  Ok to install?
>
>
> [PR86397] resolve nondependent noexcept specs early in C++1[14]
>
> From: Alexandre Oliva 
>
> build_noexcept_spec refrained from resolving nondependent noexcept
> expressions when they were not part of the function types (C++ 11 and
> 14).  This caused problems during mangling: canonical_eh_spec, when
> called on the template function type, would find an unresolved but not
> explicitly deferred expression, and nothrow_spec_p would reject it.
>
> We could relax the mangling logic to skip canonical_eh_spec, but since
> -Wnoexcept-type warns when mangling function names that change as
> noexcept specs become part of types and of mangling in C++17, and the
> test at mangling time may give incorrect results if the spec is not
> resolved, we might as well keep things simple and resolve nondependent
> noexcept specs sooner rather than later.  This is what this patch does.
>
>
> for  gcc/cp/ChangeLog
>
> PR c++/86397
> * except.c (build_noexcept_spec): Resolve nondependent
> expressions.
>
> for gcc/testsuite/ChangeLog
>
> PR c++/86397
> * g++.dg/cpp0x/pr86397-1.C: New.
> * g++.dg/cpp0x/pr86397-2.C: New.
> ---
>  gcc/cp/except.c|5 +
>  gcc/testsuite/g++.dg/cpp0x/pr86397-1.C |4 
>  gcc/testsuite/g++.dg/cpp0x/pr86397-2.C |4 
>  3 files changed, 9 insertions(+), 4 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr86397-1.C
>  create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr86397-2.C
>
> diff --git a/gcc/cp/except.c b/gcc/cp/except.c
> index 3449b59b3cc0..a6951baa35c6 100644
> --- a/gcc/cp/except.c
> +++ b/gcc/cp/except.c
> @@ -1189,11 +1189,8 @@ type_throw_all_p (const_tree type)
>  tree
>  build_noexcept_spec (tree expr, tsubst_flags_t complain)
>  {
> -  /* This isn't part of the signature, so don't bother trying to evaluate
> - it until instantiation.  */
>if (TREE_CODE (expr) != DEFERRED_NOEXCEPT
> -  && (!processing_template_decl
> - || (flag_noexcept_type && !value_dependent_expression_p (expr
> +  && !value_dependent_expression_p (expr))
>  {
>expr = perform_implicit_conversion_flags (boolean_type_node, expr,
> complain,
> diff --git a/gcc/testsuite/g++.dg/cpp0x/pr86397-1.C 
> b/gcc/testsuite/g++.dg/cpp0x/pr86397-1.C
> new file mode 100644
> index ..4f9f5fa7e4c8
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/pr86397-1.C
> @@ -0,0 +1,4 @@
> +// { dg-do compile { target c++11 } }
> +void e();
> +template  void f(int() noexcept(e)) {}
> +template void f(int()); // { dg-error "does not match" "" { target 
> c++17 } }
> diff --git a/gcc/testsuite/g++.dg/cpp0x/pr86397-2.C 
> b/gcc/testsuite/g++.dg/cpp0x/pr86397-2.C
> new file mode 100644
> index ..fb43499526e8
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/pr86397-2.C
> @@ -0,0 +1,4 @@
> +// { dg-do compile { target c++11 } }
> +void e();
> +template  void f(int() noexcept(e)) {}
> +template void f(int() noexcept);
>
>
> --
> Alexandre Oliva, freedom fighter   https://FSFLA.org/blogs/lxo
> Be the change, be Free! FSF Latin America board member
> GNU Toolchain EngineerFree Software Evangelist
> Hay que enGNUrecerse, pero sin perder la terGNUra jamás-GNUChe

Re: [SPARC] Fix PR target/87807

[Eric Botcazou]

> unfortunately, the new tests FAIL to compile, both 32 and 64-bit:
> 
> +FAIL: gcc.target/sparc/20181129-1.c (test for excess errors)
> +UNRESOLVED: gcc.target/sparc/20181129-1.c compilation failed to produce
> executable +FAIL: gcc.target/sparc/20181129-2.c (test for excess errors)
> +UNRESOLVED: gcc.target/sparc/20181129-2.c compilation failed to produce
> executable

Not clear how I missed them, they are in my logs too...

> Excess errors:
> /vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.target/sparc/20181129-1.c:17:3
> : error: ISO C90 forbids mixed declarations and code
> [-Wdeclaration-after-statement]
> 
> /vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.target/sparc/20181129-2.c: In
> function 'f':
> /vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.target/sparc/20181129-2.c:17:
> 3: error: ISO C90 forbids mixed declarations and code
> [-Wdeclaration-after-statement]
> 
> The following patch fixes this by building as C99.  Alternatively, it
> would work just as well to split the x declaration and initialization.

Thanks for fixing this.

-- 
Eric Botcazou

Re: [PATCH] Fix PR88301

[Christophe Lyon]

On Tue, 4 Dec 2018 at 14:17, Richard Biener  wrote:
>
> On Tue, 4 Dec 2018, Christophe Lyon wrote:
>
> > Hi Richard,
> > On Mon, 3 Dec 2018 at 14:37, Richard Biener  wrote:
> > >
> > >
> > > This fixes a missed optimization in EVRP by teaching the code
> > > figuring out conditional asserts about conversions that preserve
> > > the converted value.
> > >
> > > Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk
> > > sofar.
> > >
> > > Richard.
> > >
> > > 2018-12-03  Richard Biener  
> > >
> > > PR tree-optimization/88301
> > > * tree-vrp.c (register_edge_assert_for_2): Handle conversions
> > > that do not change the value by registering the same assert
> > > for the operand.
> > >
> > > * gcc.dg/tree-ssa/evrp13.c: New testcase.
> > >
> >
> > I noticed a regression in g++ on aarch64, and git bisect indicates
> > this commits is the culprit.
> > g++.dg/warn/Wstringop-overflow-1.C  -std=c++14  (test for warnings, 
> > line 14)
> > g++.dg/warn/Wstringop-overflow-1.C  -std=c++14 (test for excess errors)
> > g++.dg/warn/Wstringop-overflow-1.C  -std=c++17  (test for warnings, 
> > line 14)
> > g++.dg/warn/Wstringop-overflow-1.C  -std=c++17 (test for excess errors)
> > g++.dg/warn/Wstringop-overflow-1.C  -std=c++98  (test for warnings, 
> > line 14)
> > g++.dg/warn/Wstringop-overflow-1.C  -std=c++98 (test for excess errors)
> >
> > g++.log says:
> > Excess errors:
> > /gcc/testsuite/g++.dg/warn/Wstringop-overflow-1.C:14:21: warning:
> > 'char* __builtin_strncpy(char*, const char*, long unsigned int)'
> > writing between 6 and 2147483647 bytes into a region of size 5
> > overflows the destination [-Wstringop-overflow=]
>
> Yes, sorry.  r266773 should have fixed this.
>
Indeed, thanks.

> Richard.

Re: [PATCH 1/9]: C++ P0482R5 char8_t: Documentation updates

[Jason Merrill]

On Wed, Dec 5, 2018 at 2:10 AM Tom Honermann  wrote:
>
> On 12/3/18 2:59 PM, Jason Merrill wrote:
> > On 11/5/18 2:39 PM, Tom Honermann wrote:
> >> This patch adds documentation for new -fchar8_t and -fno-char8_t
> >> options.
> >>
> >> gcc/ChangeLog:
> >>
> >> 2018-11-04  Tom Honermann  
> >>   * doc/invoke.texi (-fchar8_t): Document new option.
> >
> >> +Enable support for the P0482 proposal including the addition of a
> >> +new @code{char8_t} fundamental type, changes to the types of UTF-8
> >
> > Now that the proposal has been accepted, I'd refer to C++2a instead.
>
> Agreed.  I also need to make the changes to implicitly enable -fchar8_t
> with -std=c++2a.
>
> The list of impacted standard library features was incomplete and I
> suspect it isn't worth mentioning them specifically.  Perhaps mentioning
> the feature test macros would be helpful as well?
>
> How does the following sound?
>
> Enable support for @code{char8_t} as adopted for C++2a.  This includes
> the addition of a new @code{char8_t} fundamental type, changes to the
> types of UTF-8 string and character literals, new signatures for user
> defined literals, associated standard library updates, and new
> @code{__cpp_char8_t} and @code{__cpp_lib_char8_t} feature test macros.

Sounds good.

Jason

Re: [PATCH][RFC] Poison bitmap_head->obstack

[Richard Biener]

On Wed, 5 Dec 2018, Jeff Law wrote:

> On 12/4/18 6:16 AM, Richard Biener wrote:
> > 
> > This tries to make bugs like that in PR88317 harder to create by
> > introducing a bitmap_release function that can be used as
> > pendant to bitmap_initialize for non-allocated bitmap heads.
> > The function makes sure to poison the bitmaps obstack member
> > so the obstack the bitmap was initialized with can be safely
> > released.
> > 
> > The patch also adds a default constructor to bitmap_head
> > doing the same, but for C++ reason initializes to a
> > all-zero bitmap_obstack rather than 0xdeadbeef because
> > the latter isn't possible in constexpr context (it is
> > by using unions but then things start to look even more ugly).
> > 
> > The stage1 compiler might end up with a few extra runtime
> > initializers but constexpr makes sure they'll vanish for
> > later stages.
> > 
> > I had to paper over that you-may-not-use-memset-to-zero classes
> > with non-trivial constructors warning in two places and I
> > had to teach gengtype about CONSTEXPR (probably did so in
> > an awkward way - suggestions and pointers into gengtype
> > appreciated).
> > 
> > Bootstrapped (with host GCC 4.8) on x86_64-unknown-linux-gnu,
> > testing in progress.
> > 
> > The LRA issue seems to be rare enough (on x86_64...) that
> > I didn't trip over it sofar.
> > 
> > Comments?  Do we want this?  Not sure how we can easily
> > discover all bitmap_clear () users that should really
> > use bitmap_release (suggestion for a better name appreciated
> > as well - I thought about bitmap_uninitialize)
> > 
> > Richard.
> > 
> > 2018-12-04  Richard Biener  
> > 
> > * bitmap.c (bitmap_head::crashme): Define.
> > * bitmap.h (bitmap_head): Add constexpr default constructor
> > poisoning the obstack member.
> > (bitmap_head::crashme): Declare.
> > (bitmap_release): New function clearing a bitmap and poisoning
> > the obstack member.
> > * gengtype.c (main): Make it recognize CONSTEXPR.
> > 
> > * lra-constraints.c (lra_inheritance): Use bitmap_release
> > instead of bitmap_clear.
> > 
> > * ira.c (ira): Work around warning.
> > * regrename.c (create_new_chain): Likewise.
> I don't see enough complexity in here to be concerning -- so if it makes
> it harder to make mistakes, then I'm for it.

Any comment about the -Wclass-memaccess workaround sprinkling around two
(void *) conversions?  I didn't dig deep enough to look for a more
appropriate solution, also because there were some issues with older
host compilers and workarounds we installed elsewhere...

Otherwise yes, it makes it harder to do mistakes.  I'll probably
use bitmap_head::crashme instead of 0xdeadbeef in bitmap_release.
And of course we'd need to hunt down users of bitmap_clear that
should be bitmap_release instead...

Thanks,
Richard.

[PATCH] Testcases PR63184

[Richard Biener]

Committed.

Richard.

2018-12-05  Richard Biener  

PR middle-end/63184
* c-c++-common/pr19807-2.c: New testcase.
* c-c++-common/pr19807-3.c: Likewise.

diff --git a/gcc/testsuite/c-c++-common/pr19807-2.c 
b/gcc/testsuite/c-c++-common/pr19807-2.c
new file mode 100644
index 000..c8b2a57d654
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr19807-2.c
@@ -0,0 +1,12 @@
+/* { dg-do link } */
+/* { dg-options "-O" } */
+
+extern void link_error(void);
+int i;
+int main()
+{
+  int a[4];
+  if ((char*)[1] + 4*i + 4 != (char*)[i+2])
+link_error();
+  return 0;
+}
diff --git a/gcc/testsuite/c-c++-common/pr19807-3.c 
b/gcc/testsuite/c-c++-common/pr19807-3.c
new file mode 100644
index 000..d882bd369bf
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr19807-3.c
@@ -0,0 +1,12 @@
+/* { dg-do link } */
+/* { dg-options "-O" } */
+
+extern void link_error(void);
+int i;
+int main()
+{
+  int a[4];
+  if ([1] + i + 1 != [i+2])
+link_error();
+  return 0;
+}

Re: [PATCH][RFC] Poison bitmap_head->obstack

[Jeff Law]

On 12/4/18 6:16 AM, Richard Biener wrote:
> 
> This tries to make bugs like that in PR88317 harder to create by
> introducing a bitmap_release function that can be used as
> pendant to bitmap_initialize for non-allocated bitmap heads.
> The function makes sure to poison the bitmaps obstack member
> so the obstack the bitmap was initialized with can be safely
> released.
> 
> The patch also adds a default constructor to bitmap_head
> doing the same, but for C++ reason initializes to a
> all-zero bitmap_obstack rather than 0xdeadbeef because
> the latter isn't possible in constexpr context (it is
> by using unions but then things start to look even more ugly).
> 
> The stage1 compiler might end up with a few extra runtime
> initializers but constexpr makes sure they'll vanish for
> later stages.
> 
> I had to paper over that you-may-not-use-memset-to-zero classes
> with non-trivial constructors warning in two places and I
> had to teach gengtype about CONSTEXPR (probably did so in
> an awkward way - suggestions and pointers into gengtype
> appreciated).
> 
> Bootstrapped (with host GCC 4.8) on x86_64-unknown-linux-gnu,
> testing in progress.
> 
> The LRA issue seems to be rare enough (on x86_64...) that
> I didn't trip over it sofar.
> 
> Comments?  Do we want this?  Not sure how we can easily
> discover all bitmap_clear () users that should really
> use bitmap_release (suggestion for a better name appreciated
> as well - I thought about bitmap_uninitialize)
> 
> Richard.
> 
> 2018-12-04  Richard Biener  
> 
>   * bitmap.c (bitmap_head::crashme): Define.
>   * bitmap.h (bitmap_head): Add constexpr default constructor
>   poisoning the obstack member.
>   (bitmap_head::crashme): Declare.
>   (bitmap_release): New function clearing a bitmap and poisoning
>   the obstack member.
>   * gengtype.c (main): Make it recognize CONSTEXPR.
> 
>   * lra-constraints.c (lra_inheritance): Use bitmap_release
>   instead of bitmap_clear.
> 
>   * ira.c (ira): Work around warning.
>   * regrename.c (create_new_chain): Likewise.
I don't see enough complexity in here to be concerning -- so if it makes
it harder to make mistakes, then I'm for it.

jeff

[PATCH, libgcc] Bug fix in udivmodhi4.c

[Paul Koning]

This fixes a cut & paste oversight in udivmodhi4 (which is currently used only 
by the pdp11 target) reported by Stefan Kanthak.

Committed as obvious.

paul

ChangeLog:

2018-12-05  Paul Koning  

* udivmodhi4.c (__udivmodhi4): Fix loop end check.

Index: libgcc/udivmodhi4.c
===
--- libgcc/udivmodhi4.c (revision 266823)
+++ libgcc/udivmodhi4.c (working copy)
@@ -27,7 +27,7 @@ __udivmodhi4(unsigned short num, unsigned short de
   unsigned short bit = 1;
   unsigned short res = 0;
 
-  while (den < num && bit && !(den & (1L<<31)))
+  while (den < num && bit && !(den & (1U<<15)))
 {
   den <<=1;
   bit <<=1;

Re: OpenACC ICV acc-default-async-var

[Thomas Schwinge]

Hi Chung-Lin!

On Mon, 19 Nov 2018 16:33:30 +0900, Chung-Lin Tang  
wrote:
> Hi Thomas,
> actually the current version of the acc_get/set_default_async patch is
> combined into:
> https://gcc.gnu.org/ml/gcc-patches/2018-09/msg01426.html
> 
> This patch you're referring here was a version from early 2017.

I know, but I intend to handle these changes individually, for these are
separate things.

> I'll try to reply to the still applying comments here below.

Thanks.

> On 2018/11/18 10:36 AM, Thomas Schwinge wrote:
> >> --- include/gomp-constants.h   (revision 245382)
> >> +++ include/gomp-constants.h   (working copy)
> > 
> >>   /* Asynchronous behavior.  Keep in sync with
> >>  libgomp/{openacc.h,openacc.f90,openacc_lib.h}:acc_async_t.  */
> >>   
> >> +#define GOMP_ASYNC_DEFAULT0
> >>   #define GOMP_ASYNC_NOVAL -1
> >>   #define GOMP_ASYNC_SYNC  -2
> > 
> > This means that "acc_set_default_async(acc_async_default)" will set
> > acc-default-async-var to "0", that is, the same as
> > "acc_set_default_async(0)".  It thus follows that
> > "async"/"async(acc_async_noval)" is the same as "async(0)".  Is that
> > intentional?
> > 
> > It is in line with the OpenACC 2.5 specification: "initial value [...] is
> > implementation defined", but I wonder why map it to "async(0)", and not
> > to its own, unspecified, but separate queue.  In the latter case,
> > "acc_async_default" etc. would then map to a negative value to denote
> > this unspecified, but separate queue (and your changes would need to be
> > adapted for that).
> > 
> > I have not verified whether we're currently already having (on trunk
> > and/or openacc-gcc-8-branch) the semantics of the queue of
> > "async(acc_async_noval)" mapping to the same queue as "async(0)"?
> 
> As long as the thr->default_async variable == 0 (as it is initially)
> then async(acc_async_noval) maps to async(0).
> 
> > I'm fine to accept your changes as proposed (basically, everthing from
> > your patch posted that has a "default_async" in it), for that's an
> > incremental improvement anyway.  But -- unless you tell me I've
> > misunderstood something -- I'll get the issue I raised clarified with the
> > OpenACC technical committee, and we will then later improve this further.
> > 
> > No matter what the outcome, the implementation-defined behavior should be
> > documented.  (Can do that once we get the intentions clarified.)
> 
> Well, the current submitted implementation of async queues manages an array 
> of them
> for each thread. So the intuitive default queue is the first (index 0), and
> to support reverting to default when accepting 'acc_async_default' as an 
> argument,
> defining acc_async_default == 0 is the logical choice.
> 
> The 'default' async is not symbolically a specific queue, it is simply a 
> thread-local
> variable for what is referred by default when 'acc_async_noval' is 
> encountered.
>  From that sense, initializing it as some negative integer doesn't make sense

That's not my understanding, though, and is not what is currently
implemented in trunk, where "async(acc_async_noval)" ("async" without an
argument) currently is a separate queue, different from all "async(a)"
with "a" nonnegative.

> Of course, if really desired, we implement the "default default" to be an 
> alternative
> queue separate from the non-negative queue space, but I feel this is overkill.

I'm looking into clarifying that, and then adjusting the code
accordingly; shouldn't be too difficult.


Grüße
 Thomas

[PR88370] acc_get_cuda_stream/acc_set_cuda_stream: acc_async_sync, acc_async_noval (was: OpenACC ICV acc-default-async-var)

[Thomas Schwinge]

Hi Chung-Lin!

On Mon, 19 Nov 2018 16:33:30 +0900, Chung-Lin Tang  
wrote:
> On 2018/11/18 10:36 AM, Thomas Schwinge wrote:
> > Generally, I envision test cases running a few "acc_get_cuda_stream"
> > calls with relevant argument values, to see whether the expected
> > queues/streames are being used.  (Similar for other offload targets.)
> > 
> > But I suppose we might again need to get clarified whether
> > "acc_get_cuda_stream(acc_async_sync)",
> > "acc_get_cuda_stream(acc_async_noval)", or
> > "acc_get_cuda_stream(acc_async_default)" are actually valid calls (given
> > that these argument values are not valid "async value"s), and these would
> > then return the respective CUDA stream handles, different from the one
> > returned for "acc_get_cuda_stream(0)" etc.
> > 
> > That said, we can certainly implement it that way, because that's not
> > against the specification.
> 
> I think the likely clarification we'll ever get on this is that it's
> implementation defined :P

Well, actually, I've been able to convince myself ;-) to a reading of the
specification so that this is supported, and filed
.

Does the following look alright to you?

Do you agree that 'Refusing request to set CUDA stream associated with
"acc_async_sync"' should just be an informational debug message, instead
of a hard error?  (This restriction might disappear in the future.)  (Oh,
and other negative values will still be diagnosed as errors by
"select_stream_for_async".)

commit 9dd878052a3c19876c15b77ac0dde2829874e413
Author: Thomas Schwinge 
Date:   Wed Dec 5 12:51:30 2018 +0100

[PR88370] acc_get_cuda_stream/acc_set_cuda_stream: acc_async_sync, 
acc_async_noval

libgomp/
PR libgomp/88370
* libgomp.texi (acc_get_current_cuda_context, acc_get_cuda_stream)
(acc_set_cuda_stream): Clarify.
* oacc-cuda.c (acc_get_cuda_stream, acc_set_cuda_stream): Use
"async_valid_p".
* plugin/plugin-nvptx.c (nvptx_set_cuda_stream): Refuse "async ==
acc_async_sync".
* testsuite/libgomp.oacc-c-c++-common/acc_set_cuda_stream-1.c: New 
file.
* testsuite/libgomp.oacc-c-c++-common/async_queue-1.c: Likewise.
* testsuite/libgomp.oacc-c-c++-common/lib-84.c: Update.
* testsuite/libgomp.oacc-c-c++-common/lib-85.c: Likewise.
---
 libgomp/libgomp.texi   | 17 ++--
 libgomp/oacc-cuda.c|  4 +-
 libgomp/plugin/plugin-nvptx.c  | 10 ++-
 .../acc_set_cuda_stream-1.c| 42 ++
 .../libgomp.oacc-c-c++-common/async_queue-1.c  | 97 ++
 .../testsuite/libgomp.oacc-c-c++-common/lib-84.c   | 31 +--
 .../testsuite/libgomp.oacc-c-c++-common/lib-85.c   | 27 +-
 7 files changed, 208 insertions(+), 20 deletions(-)

diff --git libgomp/libgomp.texi libgomp/libgomp.texi
index 3fa8eb8165e5..e6c20525bc0c 100644
--- libgomp/libgomp.texi
+++ libgomp/libgomp.texi
@@ -2768,7 +2768,7 @@ as used by the CUDA Runtime or Driver API's.
 
 @item @emph{C/C++}:
 @multitable @columnfractions .20 .80
-@item @emph{Prototype}: @tab @code{acc_get_current_cuda_context(void);}
+@item @emph{Prototype}: @tab @code{void *acc_get_current_cuda_context(void);}
 @end multitable
 
 @item @emph{Reference}:
@@ -2782,12 +2782,12 @@ A.2.1.2.
 @section @code{acc_get_cuda_stream} -- Get CUDA stream handle.
 @table @asis
 @item @emph{Description}
-This function returns the CUDA stream handle. This handle is the same
-as used by the CUDA Runtime or Driver API's.
+This function returns the CUDA stream handle for the queue @var{async}.
+This handle is the same as used by the CUDA Runtime or Driver API's.
 
 @item @emph{C/C++}:
 @multitable @columnfractions .20 .80
-@item @emph{Prototype}: @tab @code{acc_get_cuda_stream(void);}
+@item @emph{Prototype}: @tab @code{void *acc_get_cuda_stream(int async);}
 @end multitable
 
 @item @emph{Reference}:
@@ -2802,11 +2802,16 @@ A.2.1.3.
 @table @asis
 @item @emph{Description}
 This function associates the stream handle specified by @var{stream} with
-the asynchronous value specified by @var{async}.
+the queue @var{async}.
+
+This cannot be used to change the stream handle associated with
+@code{acc_async_sync}.
+
+The return value is not specified.
 
 @item @emph{C/C++}:
 @multitable @columnfractions .20 .80
-@item @emph{Prototype}: @tab @code{acc_set_cuda_stream(int async void 
*stream);}
+@item @emph{Prototype}: @tab @code{int acc_set_cuda_stream(int async, void 
*stream);}
 @end multitable
 
 @item @emph{Reference}:
diff --git libgomp/oacc-cuda.c libgomp/oacc-cuda.c
index 20774c1b4876..4ee4c9b08576 100644
--- libgomp/oacc-cuda.c
+++ libgomp/oacc-cuda.c
@@ -58,7 +58,7 @@ acc_get_cuda_stream (int async)
 {
   struct goacc_thread *thr = goacc_thread ();
 
-  if (!async_valid_stream_id_p (async))
+  if (!async_valid_p (async))
 return NULL;
 
   if (thr && thr->dev && 

Re: [PATCH] dumpfile.c: use prefixes other that 'note: ' for MSG_{OPTIMIZED_LOCATIONS|MISSED_OPTIMIZATION}

[Rainer Orth]

Rainer Orth  writes:

> Hi Andreas,
>
>> On Okt 01 2018, David Malcolm  wrote:
>>
>>> Is there a link to the .log files somewhere so I can see the precise
>>> messages in question?  (e.g. are they all "loop versioned for
>>> vectorization to enhance alignment"?).
>>
>> Yes, they are all the same message.
>
> just for the record: I'm seeing exactly the same on Solaris/SPARC (32
> and 64-bit).

I've now filed PR testsuite/88369 for this issue.

Rainer

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

Re: [PATCH] be more permissive about function alignments (PR 88208)

[Jeff Law]

On 12/5/18 6:09 AM, Rainer Orth wrote:
> Hi Martin,
> 
>> The tests for the new __builtin_has_attribute function have been
>> failing on a number of targets because of a couple of assumptions
>> that only hold on some.
>>
>> First, they expect that it's safe to apply attribute aligned with
>> a smaller alignment than the target provides when GCC rejects such
>> arguments.  The tests pass on i86 and elsewhere but fail on
>> strictly aligned targets like aarch64 or sparc.  After some testing
>> and thinking I don't think this is helpful -- I believe it's better
>> to instead silently accept attributes that ask for a less restrictive
>> alignment than the function ultimately ends up with (see * below).
>> This is what testing shows Clang does on those targets.  The attached
>> patch implements this change.
>>
>> Second, the tests assume that the priority forms of the constructor
>> and destructor attributes are universally supported.  That's also
>> not the case, even though the manual doesn't mention that.  To
>> avoid these failures the attached patch moves the priority forms
>> of the attribute constructor and destructor tests into its own
>> file that's compiled only for init_priority targets.
>>
>> Finally, I noticed that attribute aligned accepts zero as
>> an argument even though it's not a power of two as the manual
>> documents as a precondition (zero is treated the same as
>> the attribute without an argument).  A zero argument is likely
>> to be a mistake, especially when the zero comes from macro
>> expansion, that users might want to know about.  Clang rejects
>> a zero with an error but I think a warning is more in line with
>> established GCC practice, so the patch also implements that.
>>
>> Besides x86_64-linux, I tested this change with cross-compilers
>> for aarch64-linux-elf, powerpc64le-linux, and sparc-solaris2.11.
>> I added tests for the changed aligned attribute for those targets
>> To make the gcc.dg/builtin-has-attribute.c test pass with
>> the cross-compilers I changed from a runtime test into a compile
>> only one.
>>
>> Martin
>>
>> PS I'm not happy about duplicating the same test across all those
>> targets.  It would be much nicer to have a single test somewhere
>> in dg.exp #include a target-specific header with macros describing
>> the target-specific parameters.
> 
> why so complicated?  Just have a single attr-aligned.c test, restricted
> to the target cpus it supports via dg directives and with
> MINALIGN/MAXALIGN definitions controlled by appropriate target macros?
> 
> Besides, the new gcc.target/sparc/attr-aligned.c test currently FAILs on
> 64-bit sparc:
> 
> FAIL: gcc.target/sparc/attr-aligned.c (test for excess errors)
> 
> Excess errors:
> /vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.target/sparc/attr-aligned.c:29:1:
>  error: static assertion failed: "alignof (f_) == MAXALIGN"
> 
> alignof (f_) is 16 for sparcv9.  The following patch fixes this, tested
> on sparc-sun-solaris2.11 with -m32 and -m64.
> 
> Ok for mainline?
OK
jeff

Re: [SPARC] Fix PR target/87807

[Jeff Law]

On 12/5/18 6:15 AM, Rainer Orth wrote:
> Hi Eric,
> 
>> This started as a simple fix for a small issue (passing floating-point 
>> vectors 
>> to variadic functions in 64-bit mode) and then evolved into a small cleanup 
>> of 
>> the code implementing the calling conventions of the 2 SPARC ABIs.
>>
>> Tested and compat-regtested on SPARC/Solaris 11, applied on the mainline.
>>
>>
>> 2018-11-29  Eric Botcazou  
>>
>>  PR target/87807
>>  * config/sparc/sparc-modes.def: Minor tweak.
>>  * config/sparc/sparc.c: Minor reordering.
>>  (sparc_pass_by_reference): Move around.
>>  (traverse_record_type): Change offset from HOST_WIDE_INT to int.
>>  (classify_registers): Likewise for bitpos.
>>  (function_arg_slotno): Remove dead test and tweak comments.
>>  : Remove useless assertion and test whether the
>>  parameter is named in order to pass it in FP registers.  Return
>>  the regno for floating-point vector types.
>>  (compute_int_layout): Change bitpos from HOST_WIDE_INT to int.
>>  (compute_fp_layout): Likewise.
>>  (count_registers): Likewise.
>>  (assign_int_registers): Likewise.
>>  (assign_fp_registers): Likewise.
>>  (assign_registers): Likewise.
>>  (function_arg_record_value): Change size from HOST_WIDE_INT to int
>>  and use CEIL_NWORDS to compute the number of registers.
>>  (function_arg_union_value): Minor tweaks.
>>  (function_arg_vector_value): Add slotno and named parameters, use
>>  CEIL_NWORDS to compute the number of registers.
>>  (sparc_function_arg_1): Rework handling of vector types.  Change
>>  size from HOST_WIDE_INT to int.
>>  (sparc_arg_partial_bytes): Rework handling of 32-bit ABI and deal
>>  with vector types for the 64-bt ABI.
>>  (sparc_function_arg_advance): Likewise.
>>  (sparc_return_in_memory): Add reference to -fpcc-struct-return.
>>  (sparc_struct_value_rtx): Return NULL_RTX instead of 0.
>>  (sparc_function_value_1): Rework handling of vector types.  Change
>>  size from HOST_WIDE_INT to int.
>>
>>
>> 2018-11-29  Eric Botcazou  
>>
>>  * gcc.target/sparc/20181129-1.c: New test.
>>  * gcc.target/sparc/20181129-2.c: Likewise.
> 
> unfortunately, the new tests FAIL to compile, both 32 and 64-bit:
> 
> +FAIL: gcc.target/sparc/20181129-1.c (test for excess errors)
> +UNRESOLVED: gcc.target/sparc/20181129-1.c compilation failed to produce 
> executable
> +FAIL: gcc.target/sparc/20181129-2.c (test for excess errors)
> +UNRESOLVED: gcc.target/sparc/20181129-2.c compilation failed to produce 
> executable
> 
> Excess errors:
> /vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.target/sparc/20181129-1.c:17:3: 
> error: ISO C90 forbids mixed declarations and code 
> [-Wdeclaration-after-statement]
> 
> /vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.target/sparc/20181129-2.c: In 
> function 'f':
> /vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.target/sparc/20181129-2.c:17:3: 
> error: ISO C90 forbids mixed declarations and code 
> [-Wdeclaration-after-statement]
> 
> The following patch fixes this by building as C99.  Alternatively, it
> would work just as well to split the x declaration and initialization.
> 
> Tested on sparc-sun-solaris2.11.  Ok for mainline?
OK
jeff

Re: [SPARC] Fix PR target/87807

[Rainer Orth]

Hi Eric,

> This started as a simple fix for a small issue (passing floating-point 
> vectors 
> to variadic functions in 64-bit mode) and then evolved into a small cleanup 
> of 
> the code implementing the calling conventions of the 2 SPARC ABIs.
>
> Tested and compat-regtested on SPARC/Solaris 11, applied on the mainline.
>
>
> 2018-11-29  Eric Botcazou  
>
>   PR target/87807
>   * config/sparc/sparc-modes.def: Minor tweak.
>   * config/sparc/sparc.c: Minor reordering.
>   (sparc_pass_by_reference): Move around.
>   (traverse_record_type): Change offset from HOST_WIDE_INT to int.
>   (classify_registers): Likewise for bitpos.
>   (function_arg_slotno): Remove dead test and tweak comments.
>   : Remove useless assertion and test whether the
>   parameter is named in order to pass it in FP registers.  Return
>   the regno for floating-point vector types.
>   (compute_int_layout): Change bitpos from HOST_WIDE_INT to int.
>   (compute_fp_layout): Likewise.
>   (count_registers): Likewise.
>   (assign_int_registers): Likewise.
>   (assign_fp_registers): Likewise.
>   (assign_registers): Likewise.
>   (function_arg_record_value): Change size from HOST_WIDE_INT to int
>   and use CEIL_NWORDS to compute the number of registers.
>   (function_arg_union_value): Minor tweaks.
>   (function_arg_vector_value): Add slotno and named parameters, use
>   CEIL_NWORDS to compute the number of registers.
>   (sparc_function_arg_1): Rework handling of vector types.  Change
>   size from HOST_WIDE_INT to int.
>   (sparc_arg_partial_bytes): Rework handling of 32-bit ABI and deal
>   with vector types for the 64-bt ABI.
>   (sparc_function_arg_advance): Likewise.
>   (sparc_return_in_memory): Add reference to -fpcc-struct-return.
>   (sparc_struct_value_rtx): Return NULL_RTX instead of 0.
>   (sparc_function_value_1): Rework handling of vector types.  Change
>   size from HOST_WIDE_INT to int.
>
>
> 2018-11-29  Eric Botcazou  
>
>   * gcc.target/sparc/20181129-1.c: New test.
>   * gcc.target/sparc/20181129-2.c: Likewise.

unfortunately, the new tests FAIL to compile, both 32 and 64-bit:

+FAIL: gcc.target/sparc/20181129-1.c (test for excess errors)
+UNRESOLVED: gcc.target/sparc/20181129-1.c compilation failed to produce 
executable
+FAIL: gcc.target/sparc/20181129-2.c (test for excess errors)
+UNRESOLVED: gcc.target/sparc/20181129-2.c compilation failed to produce 
executable

Excess errors:
/vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.target/sparc/20181129-1.c:17:3: 
error: ISO C90 forbids mixed declarations and code 
[-Wdeclaration-after-statement]

/vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.target/sparc/20181129-2.c: In 
function 'f':
/vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.target/sparc/20181129-2.c:17:3: 
error: ISO C90 forbids mixed declarations and code 
[-Wdeclaration-after-statement]

The following patch fixes this by building as C99.  Alternatively, it
would work just as well to split the x declaration and initialization.

Tested on sparc-sun-solaris2.11.  Ok for mainline?

Rainer

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


2018-12-04  Rainer Orth  

* gcc.target/sparc/20181129-1.c: Compile with -std=c99.
* gcc.target/sparc/20181129-2.c: Likewise.

# HG changeset patch
# Parent  3a9647e8254db33d54bc0ed4f11e3b89fcda10b2
Build gcc.target/sparc/20181129-?.c as C99

	* gcc.target/sparc/20181129-1.c: Compile with -std=c99.
	* gcc.target/sparc/20181129-2.c: Likewise.

diff --git a/gcc/testsuite/gcc.target/sparc/20181129-1.c b/gcc/testsuite/gcc.target/sparc/20181129-1.c
--- a/gcc/testsuite/gcc.target/sparc/20181129-1.c
+++ b/gcc/testsuite/gcc.target/sparc/20181129-1.c
@@ -2,6 +2,7 @@
 /* Reported by Rainer Orth  */
 
 /* { dg-do run } */
+/* { dg-options "-std=c99" } */
 
 #include 
 
diff --git a/gcc/testsuite/gcc.target/sparc/20181129-2.c b/gcc/testsuite/gcc.target/sparc/20181129-2.c
--- a/gcc/testsuite/gcc.target/sparc/20181129-2.c
+++ b/gcc/testsuite/gcc.target/sparc/20181129-2.c
@@ -2,6 +2,7 @@
 /* Reported by Rainer Orth  */
 
 /* { dg-do run } */
+/* { dg-options "-std=c99" } */
 
 #include 
 

Re: [PATCH] be more permissive about function alignments (PR 88208)

[Rainer Orth]

Hi Martin,

> The tests for the new __builtin_has_attribute function have been
> failing on a number of targets because of a couple of assumptions
> that only hold on some.
>
> First, they expect that it's safe to apply attribute aligned with
> a smaller alignment than the target provides when GCC rejects such
> arguments.  The tests pass on i86 and elsewhere but fail on
> strictly aligned targets like aarch64 or sparc.  After some testing
> and thinking I don't think this is helpful -- I believe it's better
> to instead silently accept attributes that ask for a less restrictive
> alignment than the function ultimately ends up with (see * below).
> This is what testing shows Clang does on those targets.  The attached
> patch implements this change.
>
> Second, the tests assume that the priority forms of the constructor
> and destructor attributes are universally supported.  That's also
> not the case, even though the manual doesn't mention that.  To
> avoid these failures the attached patch moves the priority forms
> of the attribute constructor and destructor tests into its own
> file that's compiled only for init_priority targets.
>
> Finally, I noticed that attribute aligned accepts zero as
> an argument even though it's not a power of two as the manual
> documents as a precondition (zero is treated the same as
> the attribute without an argument).  A zero argument is likely
> to be a mistake, especially when the zero comes from macro
> expansion, that users might want to know about.  Clang rejects
> a zero with an error but I think a warning is more in line with
> established GCC practice, so the patch also implements that.
>
> Besides x86_64-linux, I tested this change with cross-compilers
> for aarch64-linux-elf, powerpc64le-linux, and sparc-solaris2.11.
> I added tests for the changed aligned attribute for those targets
> To make the gcc.dg/builtin-has-attribute.c test pass with
> the cross-compilers I changed from a runtime test into a compile
> only one.
>
> Martin
>
> PS I'm not happy about duplicating the same test across all those
> targets.  It would be much nicer to have a single test somewhere
> in dg.exp #include a target-specific header with macros describing
> the target-specific parameters.

why so complicated?  Just have a single attr-aligned.c test, restricted
to the target cpus it supports via dg directives and with
MINALIGN/MAXALIGN definitions controlled by appropriate target macros?

Besides, the new gcc.target/sparc/attr-aligned.c test currently FAILs on
64-bit sparc:

FAIL: gcc.target/sparc/attr-aligned.c (test for excess errors)

Excess errors:
/vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.target/sparc/attr-aligned.c:29:1: 
error: static assertion failed: "alignof (f_) == MAXALIGN"

alignof (f_) is 16 for sparcv9.  The following patch fixes this, tested
on sparc-sun-solaris2.11 with -m32 and -m64.

Ok for mainline?

Rainer

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


2018-12-04  Rainer Orth  

PR testsuite/88208
* gcc.target/sparc/attr-aligned.c (MAXALIGN) [__sparcv9 ||
__arch64__]: Define.

# HG changeset patch
# Parent  bd6c4f1f5f5ef5a6d912a66d6f3b40f21c8dd411
Provide SPARCv9 MAXALIGN in gcc.target/sparc/attr-aligned.c

diff --git a/gcc/testsuite/gcc.target/sparc/attr-aligned.c b/gcc/testsuite/gcc.target/sparc/attr-aligned.c
--- a/gcc/testsuite/gcc.target/sparc/attr-aligned.c
+++ b/gcc/testsuite/gcc.target/sparc/attr-aligned.c
@@ -10,7 +10,11 @@
 #define HAS_ALIGN(f, n)  __builtin_has_attribute (f, __aligned__ (n))
 
 #define MINALIGN(N)   ((N) < 4 ? 4 : (N))
+#if defined(__sparcv9) || defined(__arch64__)
+#define MAXALIGN  16
+#else
 #define MAXALIGN  8
+#endif
 
 /* No alignment specified.  */
 void f (void) { }

[C++ Patch] Fix grokbitfield location

[Paolo Carlini]

Hi,

as mentioned in one of my last patches, we can now improve this 
location. Note: in the same function there are a few further issues 
which I mean to incrementally fix (eg, the diagnostics for 
warn_if_not_aligned ICEs for unnamed bit-fields). Tested x86_64-linux.


Thanks, Paolo.



/cp
2018-12-05  Paolo Carlini  

* decl2.c (grokbitfield): USe DECL_SOURCE_LOCATION in error message.

/testsuite
2018-12-05  Paolo Carlini  

* g++.dg/parse/bitfield6b.C: New.
Index: cp/decl2.c
===
--- cp/decl2.c  (revision 266818)
+++ cp/decl2.c  (working copy)
@@ -1027,7 +1027,8 @@ grokbitfield (const cp_declarator *declarator,
   && (INDIRECT_TYPE_P (value)
   || !dependent_type_p (TREE_TYPE (value
 {
-  error ("bit-field %qD with non-integral type", value);
+  error_at (DECL_SOURCE_LOCATION (value),
+   "bit-field %qD with non-integral type", value);
   return error_mark_node;
 }
 
Index: testsuite/g++.dg/parse/bitfield6b.C
===
--- testsuite/g++.dg/parse/bitfield6b.C (nonexistent)
+++ testsuite/g++.dg/parse/bitfield6b.C (working copy)
@@ -0,0 +1,4 @@
+typedef void a();
+struct A {
+a a1: 1;  // { dg-error "3:bit-field .void A::a1\\(\\). with non-integral 
type" }
+};

[PATCH] Fix PR86637

[Richard Biener]

SLP vectorization forgot to reset vect_location which then "leaks"
to autopar eventually accessing stale data.

Committed to trunk.

Richard.

2018-12-05  Richard Biener  

PR tree-optimization/86637
* tree-vectorizer.c (pass_slp_vectorize::execute): Reset
vect_location at the end.

diff --git a/gcc/tree-vectorizer.c b/gcc/tree-vectorizer.c
index 0a4eca51ad7..1a6cb56a872 100644
--- a/gcc/tree-vectorizer.c
+++ b/gcc/tree-vectorizer.c
@@ -1303,6 +1303,8 @@ pass_slp_vectorize::execute (function *fun)
   loop_optimizer_finalize ();
 }
 
+  vect_location = dump_user_location_t ();
+
   return 0;
 }
 

Re: [PATCH] PR86957

[Thomas Schwinge]

Hi!

Sorry for my late follow-up; had a lot of catch up to do back then.

On Thu, 27 Sep 2018 11:47:31 +0200, Richard Biener  
wrote:
> On Mon, Sep 24, 2018 at 9:14 PM Indu Bhagat  wrote:
> >
> > Done. Attached is updated patch.
> >
> > Patch is tested on x86_64
> 
> You obviously did _not_ properly test the patch since it causes a
> bunch of new testsuite
> failures: [...]
> 
> and more.  Please get up to speed with GCC development and testing 
> requirements!

Also, with this commit, several test cases regressed from PASS to
UNSUPPORTED because of "{ dg-require-effective-target freorder }" running
into:

fprofile_use_freorder16732.c: In function 'void foo()':
fprofile_use_freorder16732.c:2:20: warning: 
'[...]/gcc/testsuite/g++/fprofile_use_freorder16732.gcda' profile count data 
file not found [-Wmissing-profile]

Iain had just mentioned that in , and fixed
in r266785, .
(Back then, I had produced the same fix, but not yet posted.)  :-|

With "{ dg-require-effective-target freorder }" thusly restored, I then
also saw two other regressions/FAILs, back then:

[-UNSUPPORTED: g++.dg/tree-prof/pr63581.C-]
UNSUPPORTED: g++.dg/tree-prof/pr63581.C -fauto-profile
{+PASS: g++.dg/tree-prof/pr63581.C compilation,  -fprofile-generate 
-D_PROFILE_GENERATE+}
{+FAIL: g++.dg/tree-prof/pr63581.C compilation,  -fprofile-use 
-D_PROFILE_USE+}
{+PASS: g++.dg/tree-prof/pr63581.C execution,-fprofile-generate 
-D_PROFILE_GENERATE+}
{+UNRESOLVED: g++.dg/tree-prof/pr63581.C execution,-fprofile-use 
-D_PROFILE_USE+}

[...]/gcc/testsuite/g++.dg/tree-prof/pr63581.C: In function 'int 
uptodate(page*)':
[...]/gcc/testsuite/g++.dg/tree-prof/pr63581.C:28:19: warning: profile for 
function 'int uptodate(page*)' not found in profile data [-Wmissing-profile]

..., and:

[-UNSUPPORTED: gcc.dg/tree-prof/20041218-1.c-]
UNSUPPORTED: gcc.dg/tree-prof/20041218-1.c -fauto-profile
{+PASS: gcc.dg/tree-prof/20041218-1.c compilation,  -fprofile-generate 
-D_PROFILE_GENERATE+}
{+FAIL: gcc.dg/tree-prof/20041218-1.c compilation,  -fprofile-use 
-D_PROFILE_USE+}
{+PASS: gcc.dg/tree-prof/20041218-1.c execution,-fprofile-generate 
-D_PROFILE_GENERATE+}
{+UNRESOLVED: gcc.dg/tree-prof/20041218-1.c execution,-fprofile-use 
-D_PROFILE_USE+}

[...]/gcc/testsuite/gcc.dg/tree-prof/20041218-1.c: In function 'bar':
[...]/gcc/testsuite/gcc.dg/tree-prof/20041218-1.c:58:1: warning: profile 
for function 'bar' not found in profile data [-Wmissing-profile]

..., for which I came up with the following patch.

But, this doesn't now seem to be necessary anymore, or am I confused?
Maybe this got fixed differently -- or is anybody still seeing these
FAILs?  (If not, then I'm not proposing to commit this, of course.)

commit 04ad00f79075a0c5eff1f806959918081e54684c
Author: Thomas Schwinge 
Date:   Wed Oct 31 23:29:32 2018 +0100

Avoid two -Wmissing-profile diagnostics
---
 gcc/testsuite/g++.dg/tree-prof/pr63581.C|5 +
 gcc/testsuite/gcc.dg/tree-prof/20041218-1.c |5 +
 2 files changed, 10 insertions(+)

diff --git gcc/testsuite/g++.dg/tree-prof/pr63581.C 
gcc/testsuite/g++.dg/tree-prof/pr63581.C
index c8caf07..9800851 100644
--- gcc/testsuite/g++.dg/tree-prof/pr63581.C
+++ gcc/testsuite/g++.dg/tree-prof/pr63581.C
@@ -25,10 +25,15 @@ __attribute__((noinline)) static int zero (void)
   return ii;
 }
 
+#pragma GCC diagnostic push
+/* WARNING: profopt.exp does not support dg-warning */
+/* "profile for function 'int uptodate(page*)' not found in profile data" */
+#pragma GCC diagnostic ignored "-Wmissing-profile"
 static inline int uptodate (struct page* p)
 {
   return (p->i < 709);
 }
+#pragma GCC diagnostic pop
 
 static struct page* bar(int i)
 {
diff --git gcc/testsuite/gcc.dg/tree-prof/20041218-1.c 
gcc/testsuite/gcc.dg/tree-prof/20041218-1.c
index cbd1c7c..41c60a7 100644
--- gcc/testsuite/gcc.dg/tree-prof/20041218-1.c
+++ gcc/testsuite/gcc.dg/tree-prof/20041218-1.c
@@ -54,6 +54,10 @@ check (void *x, struct S *y)
   return 1;
 }
 
+#pragma GCC diagnostic push
+/* WARNING: profopt.exp does not support dg-warning */
+/* "profile for function 'bar' not found in profile data" */
+#pragma GCC diagnostic ignored "-Wmissing-profile"
 static struct V *
 bar (unsigned int x, void *y)
 {
@@ -75,6 +79,7 @@ bar (unsigned int x, void *y)
 return (void *) 0;
   return u;
 }
+#pragma GCC diagnostic pop
 
 int
 foo (unsigned int *x, unsigned int y, void **z)


Grüße
 Thomas

Patch ping (Re: [C++ PATCH] Fix clone_body (PR c++/86669))

[Jakub Jelinek]

Hi!

On Wed, Nov 28, 2018 at 09:42:47AM +0100, Jakub Jelinek wrote:
> 2018-11-28  Jakub Jelinek  
> 
>   PR c++/86669
>   * optimize.c (clone_body_copy_decl): New function.
>   (clone_body): Use it for base cdtors.  Remap here only
>   DECL_INITIAL of decls that don't have DECL_CONTEXT of the
>   new clone.
> 
>   * g++.dg/cpp0x/initlist105.C: New test.
>   * g++.dg/cpp0x/initlist106.C: New test.
>   * g++.dg/other/pr86669.C: New test.

I've like to ping this patch.

Jakub

Re: [PATCH, og8] Add OpenACC 2.6 `acc_get_property' support

[Chung-Lin Tang]

Hi Maciej, please see below:

On 2018/12/4 12:51 AM, Maciej W. Rozycki wrote:

+module openacc_c_string
+  implicit none
+
+  interface
+function strlen (s) bind (C, name = "strlen")
+  use iso_c_binding, only: c_ptr, c_size_t
+  type (c_ptr), intent(in), value :: s
+  integer (c_size_t) :: strlen
+end function
+  end interface
+
+end module



+subroutine acc_get_property_string_h (n, d, p, s)
+  use iso_c_binding, only: c_char, c_int, c_ptr, c_f_pointer
+  use openacc_internal, only: acc_get_property_string_l
+  use openacc_c_string, only: strlen
+  use openacc_kinds

...> +  pint = int (p, c_int)

+  cptr = acc_get_property_string_l (n, d, pint)
+  clen = int (strlen (cptr))
+  call c_f_pointer (cptr, sptr, [clen])


AFAIK, things like strlen are already available in iso_c_binding, in forms like 
"C_strlen".
Can you check again if that 'openacc_c_string' module is really necessary?


+union gomp_device_property_value
+GOMP_OFFLOAD_get_property (int n, int prop)
+{
+  union gomp_device_property_value propval = { .val = 0 };
+
+  pthread_mutex_lock (_dev_lock);
+
+  if (!nvptx_init () || n >= nvptx_get_num_devices ())
+{
+  pthread_mutex_unlock (_dev_lock);
+  return propval;
+}
+
+  switch (prop)
+{
+case GOMP_DEVICE_PROPERTY_MEMORY:
+  {
+   size_t total_mem;
+   CUdevice dev;
+
+   CUDA_CALL_ERET (propval, cuDeviceGet, , n);
+   CUDA_CALL_ERET (propval, cuDeviceTotalMem, _mem, dev);
+   propval.val = total_mem;
+  }
+  break;
+case GOMP_DEVICE_PROPERTY_FREE_MEMORY:
+  {
+   size_t total_mem;
+   size_t free_mem;
+   CUdevice ctxdev;
+   CUdevice dev;
+
+   CUDA_CALL_ERET (propval, cuCtxGetDevice, );
+   CUDA_CALL_ERET (propval, cuDeviceGet, , n);
+   if (dev == ctxdev)
+ CUDA_CALL_ERET (propval, cuMemGetInfo, _mem, _mem);
+   else if (ptx_devices[n])
+ {
+   CUcontext old_ctx;
+
+   CUDA_CALL_ERET (propval, cuCtxPushCurrent, ptx_devices[n]->ctx);
+   CUDA_CALL_ERET (propval, cuMemGetInfo, _mem, _mem);
+   CUDA_CALL_ASSERT (cuCtxPopCurrent, _ctx);
+ }
+   else
+ {
+   CUcontext new_ctx;
+
+   CUDA_CALL_ERET (propval, cuCtxCreate, _ctx, CU_CTX_SCHED_AUTO,
+   dev);
+   CUDA_CALL_ERET (propval, cuMemGetInfo, _mem, _mem);
+   CUDA_CALL_ASSERT (cuCtxDestroy, new_ctx);
+ }


(I'm CCing Tom here, as he is maintainer for these parts)

As we discussed earlier on our internal list, I think properly using 
GOMP_OFFLOAD_init_device
is the right way, instead of using the lower level CUDA context create/destroy.

I did not mean for you to first init the device and then immediately destroy it 
by
GOMP_OFFLOAD_fini_device, just to obtain the property, but for you to just take 
the opportunity to initialize
it for use, and leave it there until program exit. That should save resources 
overall.
(BTW, CUDA contexts should be quite expensive to create/destroy, using a 
cuCtxCreate/Destroy pair is probably
almost as slow)

Tom, do you have any comments on how to best write this part?

Thanks,
Chung-Lin

Re: [PATCH/coding style] clarify pointers and operators

[Richard Sandiford]

Thanks for doing this,

Martin Sebor  writes:
> Martin suggested we update the Coding Conventions to describe
> the expected style for function declarations with a pointer
> return types, and for overloaded operators.  Below is the patch.
>
> As an aside, regarding the space convention in casts: a crude
> grep search yields about 10,000 instances of the "(type)x" kinds
> of casts in GCC sources and 40,000 of the preferred "(type) x"
> style with the space.  That's a consistency of only 80%.  Is
> it worth documenting a preference for a convention that's so
> inconsistently followed?

Just to be sure, does that grep include things like the go frontend
and its GCC interface, which deliberately don't follow GNU conventions?
A crude grep for me gives 92% consistency in gcc/* itself (excluding
subdirectories), although that's still disappointingly low...

Thanks,
Richard

Re: Use unsigned arithmetic for demoted vector plus/minus/mult (PR 88064)

[Richard Biener]

On Wed, Dec 5, 2018 at 9:39 AM Richard Sandiford
 wrote:
>
> As Jakub pointed out, if we narrow a plus, minus or mult operation based
> on the number of bits that consumers need, we have to convert a signed
> operation to an unsigned one in order to avoid new undefined behaviour.
> This patch does that and generalises vect_convert_input and
> vect_recog_over_widening_pattern to cope with the extra casts.
> (The changes to both functions are covered by existing tests.)
>
> Tested on x86_64-linux-gnu, aarch64-linux-gnu and aarch64_be-elf.
> OK to install?

OK.

Richard.

> Richard
>
>
> 2018-12-05  Richard Sandiford  
>
> gcc/
> PR tree-optimization/88064
> * tree-vect-patterns.c (vect_convert_input): Convert the result of
> an existing cast if it has the right width but the wrong sign.
> Do not test the signedness of the required result when
> considering whether to split an existing cast; instead split to
> a type with the same signedness as the source of the cast, then
> convert it to the opposite signedness where necessary.
> (vect_recog_over_widening_pattern): Handle sign changes between
> the final PLUS_EXPR and the RSHIFT_EXPR.
> (vect_recog_average_pattern): Use an unsigned operation when
> truncating an addition, subtraction or multiplication.  Cast the
> result back to the "real" signedness before promoting.
>
> gcc/testsuite/
> PR tree-optimization/88064
> * gcc.dg/vect/vect-over-widen-23.c: New test.
>
> Index: gcc/tree-vect-patterns.c
> ===
> --- gcc/tree-vect-patterns.c2018-12-05 08:33:40.658922796 +
> +++ gcc/tree-vect-patterns.c2018-12-05 08:33:55.130797055 +
> @@ -720,34 +720,60 @@ vect_convert_input (stmt_vec_info stmt_i
>if (TREE_CODE (unprom->op) == INTEGER_CST)
>  return wide_int_to_tree (type, wi::to_widest (unprom->op));
>
> -  /* See if we can reuse an existing result.  */
> +  tree input = unprom->op;
>if (unprom->caster)
>  {
>tree lhs = gimple_get_lhs (unprom->caster->stmt);
> -  if (types_compatible_p (TREE_TYPE (lhs), type))
> -   return lhs;
> +  tree lhs_type = TREE_TYPE (lhs);
> +
> +  /* If the result of the existing cast is the right width, use it
> +instead of the source of the cast.  */
> +  if (TYPE_PRECISION (lhs_type) == TYPE_PRECISION (type))
> +   input = lhs;
> +  /* If the precision we want is between the source and result
> +precisions of the existing cast, try splitting the cast into
> +two and tapping into a mid-way point.  */
> +  else if (TYPE_PRECISION (lhs_type) > TYPE_PRECISION (type)
> +  && TYPE_PRECISION (type) > TYPE_PRECISION (unprom->type))
> +   {
> + /* In order to preserve the semantics of the original cast,
> +give the mid-way point the same signedness as the input value.
> +
> +It would be possible to use a signed type here instead if
> +TYPE is signed and UNPROM->TYPE is unsigned, but that would
> +make the sign of the midtype sensitive to the order in
> +which we process the statements, since the signedness of
> +TYPE is the signedness required by just one of possibly
> +many users.  Also, unsigned promotions are usually as cheap
> +as or cheaper than signed ones, so it's better to keep an
> +unsigned promotion.  */
> + tree midtype = build_nonstandard_integer_type
> +   (TYPE_PRECISION (type), TYPE_UNSIGNED (unprom->type));
> + tree vec_midtype = get_vectype_for_scalar_type (midtype);
> + if (vec_midtype)
> +   {
> + input = vect_recog_temp_ssa_var (midtype, NULL);
> + gassign *new_stmt = gimple_build_assign (input, NOP_EXPR,
> +  unprom->op);
> + if (!vect_split_statement (unprom->caster, input, new_stmt,
> +vec_midtype))
> +   append_pattern_def_seq (stmt_info, new_stmt, vec_midtype);
> +   }
> +   }
> +
> +  /* See if we can reuse an existing result.  */
> +  if (types_compatible_p (type, TREE_TYPE (input)))
> +   return input;
>  }
>
>/* We need a new conversion statement.  */
>tree new_op = vect_recog_temp_ssa_var (type, NULL);
> -  gassign *new_stmt = gimple_build_assign (new_op, NOP_EXPR, unprom->op);
> -
> -  /* If the operation is the input to a vectorizable cast, try splitting
> - that cast into two, taking the required result as a mid-way point.  */
> -  if (unprom->caster)
> -{
> -  tree lhs = gimple_get_lhs (unprom->caster->stmt);
> -  if (TYPE_PRECISION (TREE_TYPE (lhs)) > TYPE_PRECISION (type)
> - && TYPE_PRECISION (type) > TYPE_PRECISION (unprom->type)
> - && (TYPE_UNSIGNED (unprom->type) 

Re: [PATCH] testsuite: turn down verbosity of "process-message"

[Richard Biener]

On Tue, Dec 4, 2018 at 10:36 PM David Malcolm  wrote:
>
> When debugging a failing test, I typically invoke DejaGnu at
> verbosity level 2 (via RUNTESTFLAGS="-v -v dg.exp=something"),
> so that DejaGnu prints the command line used to invoke the
> compiler; specifically these two sites:
>   target.exp "Invoking the compiler as "
>   remote.exp "Executing on $hostname"
> which are both verbosity level 2.
>
> Unfortunately I run into an O(n^2) issue with logging from
> process-message:
>
>   verbose "process-message:\n${dg-messages}" 2
>
> where, as each message each processed, it emits the state
> of dg-messages, containing the new message and all messages so far,
> leading to exponentially-increasing output at level 2 as more test
> messages are added.
>
> This patch papers over the problem by moving the
> problematic message to verbosity level 3.
>
> Successfully regrtested on x86_64-pc-linux-gnu.
>
> OK for trunk?

OK

> gcc/testsuite/ChangeLog:
> * lib/gcc-dg.exp (process-message): Change verbosity level of
> "verbose" from 2 to 3.
> (dg-locus): Likewise.
> ---
>  gcc/testsuite/lib/gcc-dg.exp | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/testsuite/lib/gcc-dg.exp b/gcc/testsuite/lib/gcc-dg.exp
> index 305dd3c..054d884 100644
> --- a/gcc/testsuite/lib/gcc-dg.exp
> +++ b/gcc/testsuite/lib/gcc-dg.exp
> @@ -1165,7 +1165,7 @@ proc process-message { msgproc msgprefix dgargs } {
>  set newentry [lreplace $newentry 2 2 $expmsg]
>
>  set dg-messages [lreplace ${dg-messages} end end $newentry]
> -verbose "process-message:\n${dg-messages}" 2
> +verbose "process-message:\n${dg-messages}" 3
>  }
>
>  # Look for messages that don't have standard prefixes.
> @@ -1199,7 +1199,7 @@ proc dg-locus { args } {
>
>  set newentry [lreplace $newentry 2 2 $expmsg]
>  set dg-messages [lreplace ${dg-messages} end end $newentry]
> -verbose "process-message:\n${dg-messages}" 2
> +verbose "process-message:\n${dg-messages}" 3
>  }
>
>  # Handle output from -fopt-info for MSG_OPTIMIZED_LOCATIONS:
> --
> 1.8.5.3
>

Re: [PATCH] Fix unroll-and-jam (PR tree-optimization/87360)

[Richard Biener]

On Wed, 5 Dec 2018, Jakub Jelinek wrote:

> Hi!
> 
> The following testcases ICE, because tree_loop_unroll_and_jam optimizes one
> loop and on another one after it fails to analyze data dependencies and
> returns.  The end effect of that is that neither the code at the end of the
> function to do final cleanups, nor TODO_cleanup_cfg is done, and the latter
> means that we don't reset some number of initialization stuff that keeps
> referencing already removed stmts.
> 
> The following patch fixes that by replacing return false; with continue;,
> so that it will just ignore the loop which couldn't be optimized and will
> try to optimize other further loops and at the end if at least one loop is
> optimized, return TODO_cleanup_cfg.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Thanks,
Richard.

> 2018-12-04  Jakub Jelinek  
> 
>   PR tree-optimization/87360
>   * gimple-loop-jam.c (tree_loop_unroll_and_jam): On failure to analyze
>   data dependencies, don't return false, just continue.  Formatting
>   fixes.
>   (merge_loop_tree, bb_prevents_fusion_p, unroll_jam_possible_p,
>   fuse_loops): Formatting fixes.
> 
>   * g++.dg/opt/pr87360.C: New test.
>   * gfortran.dg/pr87360.f90: New test.
> 
> --- gcc/gimple-loop-jam.c.jj  2018-09-04 19:48:19.238613844 +0200
> +++ gcc/gimple-loop-jam.c 2018-12-04 18:53:45.983850575 +0100
> @@ -118,7 +118,7 @@ merge_loop_tree (struct loop *loop, stru
>for (i = 0; i < n; i++)
>  {
>/* If the block was direct child of OLD loop it's now part
> - of LOOP.  If it was outside OLD, then it moved into LOOP
> +  of LOOP.  If it was outside OLD, then it moved into LOOP
>as well.  This avoids changing the loop father for BBs
>in inner loops of OLD.  */
>if (bbs[i]->loop_father == old
> @@ -167,7 +167,7 @@ bb_prevents_fusion_p (basic_block bb)
> * stores or unknown side-effects prevent fusion
> * loads don't
> * computations into SSA names: these aren't problematic.  Their
> - result will be unused on the exit edges of the first N-1 copies
> +  result will be unused on the exit edges of the first N-1 copies
>(those aren't taken after unrolling).  If they are used on the
>other edge (the one leading to the outer latch block) they are
>loop-carried (on the outer loop) and the Nth copy of BB will
> @@ -282,12 +282,12 @@ unroll_jam_possible_p (struct loop *oute
>if (!simple_iv (loop, loop, op, , true))
>   return false;
>/* The inductions must be regular, loop invariant step and initial
> - value.  */
> +  value.  */
>if (!expr_invariant_in_loop_p (outer, iv.step)
> || !expr_invariant_in_loop_p (outer, iv.base))
>   return false;
>/* XXX With more effort we could also be able to deal with inductions
> - where the initial value is loop variant but a simple IV in the
> +  where the initial value is loop variant but a simple IV in the
>outer loop.  The initial value for the second body would be
>the original initial value plus iv.base.step.  The next value
>for the fused loop would be the original next value of the first
> @@ -322,7 +322,7 @@ fuse_loops (struct loop *loop)
>gcc_assert (EDGE_COUNT (next->header->preds) == 1);
>  
>/* The PHI nodes of the second body (single-argument now)
> - need adjustments to use the right values: either directly
> +  need adjustments to use the right values: either directly
>the value of the corresponding PHI in the first copy or
>the one leaving the first body which unrolling did for us.
>  
> @@ -449,13 +449,13 @@ tree_loop_unroll_and_jam (void)
>dependences.create (10);
>datarefs.create (10);
>if (!compute_data_dependences_for_loop (outer, true, _nest,
> -, ))
> +   , ))
>   {
> if (dump_file && (dump_flags & TDF_DETAILS))
>   fprintf (dump_file, "Cannot analyze data dependencies\n");
> free_data_refs (datarefs);
> free_dependence_relations (dependences);
> -   return false;
> +   continue;
>   }
>if (!datarefs.length ())
>   continue;
> @@ -490,7 +490,7 @@ tree_loop_unroll_and_jam (void)
>))
>   {
> /* Couldn't get the distance vector.  For two reads that's
> -  harmless (we assume we should unroll).  For at least
> +  harmless (we assume we should unroll).  For at least
>one write this means we can't check the dependence direction
>and hence can't determine safety.  */
>  
> @@ -503,7 +503,7 @@ tree_loop_unroll_and_jam (void)
>   }
>  
>/* We regard a user-specified minimum percentage of zero as a request
> - to ignore all profitability 

[committed] Add testcase for already fixed PR c++/87897

[Jakub Jelinek]

On Wed, Dec 05, 2018 at 04:34:14AM -0200, Alexandre Oliva wrote:
> for  gcc/cp/ChangeLog
> 
>   PR c++/85569
>   * constexpr.c (adjust_temp_type): Test for type equality with
>   same_type_p.
>   (constexpr_call_hasher::equal): Likewise.
> 
> for  gcc/testsuite
> 
>   PR c++/85569
>   * g++.dg/cpp1z/pr85569.C: New.

This patch fixed also PR87897, I've added the following testcase from it,
regtested on x86_64-linux and committed as obvious:

2018-12-05  Jakub Jelinek  

PR c++/87897
* g++.dg/init/const13.C: New test.

--- gcc/testsuite/g++.dg/init/const13.C.jj  2018-12-05 09:39:04.604974302 
+0100
+++ gcc/testsuite/g++.dg/init/const13.C 2018-12-05 09:38:55.961117120 +0100
@@ -0,0 +1,5 @@
+// PR c++/87897
+// { dg-do compile }
+
+typedef struct A {} B;
+A const a = B ();


Jakub

Use unsigned arithmetic for demoted vector plus/minus/mult (PR 88064)

[Richard Sandiford]

As Jakub pointed out, if we narrow a plus, minus or mult operation based
on the number of bits that consumers need, we have to convert a signed
operation to an unsigned one in order to avoid new undefined behaviour.
This patch does that and generalises vect_convert_input and
vect_recog_over_widening_pattern to cope with the extra casts.
(The changes to both functions are covered by existing tests.)

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

Richard


2018-12-05  Richard Sandiford  

gcc/
PR tree-optimization/88064
* tree-vect-patterns.c (vect_convert_input): Convert the result of
an existing cast if it has the right width but the wrong sign.
Do not test the signedness of the required result when
considering whether to split an existing cast; instead split to
a type with the same signedness as the source of the cast, then
convert it to the opposite signedness where necessary.
(vect_recog_over_widening_pattern): Handle sign changes between
the final PLUS_EXPR and the RSHIFT_EXPR.
(vect_recog_average_pattern): Use an unsigned operation when
truncating an addition, subtraction or multiplication.  Cast the
result back to the "real" signedness before promoting.

gcc/testsuite/
PR tree-optimization/88064
* gcc.dg/vect/vect-over-widen-23.c: New test.

Index: gcc/tree-vect-patterns.c
===
--- gcc/tree-vect-patterns.c2018-12-05 08:33:40.658922796 +
+++ gcc/tree-vect-patterns.c2018-12-05 08:33:55.130797055 +
@@ -720,34 +720,60 @@ vect_convert_input (stmt_vec_info stmt_i
   if (TREE_CODE (unprom->op) == INTEGER_CST)
 return wide_int_to_tree (type, wi::to_widest (unprom->op));
 
-  /* See if we can reuse an existing result.  */
+  tree input = unprom->op;
   if (unprom->caster)
 {
   tree lhs = gimple_get_lhs (unprom->caster->stmt);
-  if (types_compatible_p (TREE_TYPE (lhs), type))
-   return lhs;
+  tree lhs_type = TREE_TYPE (lhs);
+
+  /* If the result of the existing cast is the right width, use it
+instead of the source of the cast.  */
+  if (TYPE_PRECISION (lhs_type) == TYPE_PRECISION (type))
+   input = lhs;
+  /* If the precision we want is between the source and result
+precisions of the existing cast, try splitting the cast into
+two and tapping into a mid-way point.  */
+  else if (TYPE_PRECISION (lhs_type) > TYPE_PRECISION (type)
+  && TYPE_PRECISION (type) > TYPE_PRECISION (unprom->type))
+   {
+ /* In order to preserve the semantics of the original cast,
+give the mid-way point the same signedness as the input value.
+
+It would be possible to use a signed type here instead if
+TYPE is signed and UNPROM->TYPE is unsigned, but that would
+make the sign of the midtype sensitive to the order in
+which we process the statements, since the signedness of
+TYPE is the signedness required by just one of possibly
+many users.  Also, unsigned promotions are usually as cheap
+as or cheaper than signed ones, so it's better to keep an
+unsigned promotion.  */
+ tree midtype = build_nonstandard_integer_type
+   (TYPE_PRECISION (type), TYPE_UNSIGNED (unprom->type));
+ tree vec_midtype = get_vectype_for_scalar_type (midtype);
+ if (vec_midtype)
+   {
+ input = vect_recog_temp_ssa_var (midtype, NULL);
+ gassign *new_stmt = gimple_build_assign (input, NOP_EXPR,
+  unprom->op);
+ if (!vect_split_statement (unprom->caster, input, new_stmt,
+vec_midtype))
+   append_pattern_def_seq (stmt_info, new_stmt, vec_midtype);
+   }
+   }
+
+  /* See if we can reuse an existing result.  */
+  if (types_compatible_p (type, TREE_TYPE (input)))
+   return input;
 }
 
   /* We need a new conversion statement.  */
   tree new_op = vect_recog_temp_ssa_var (type, NULL);
-  gassign *new_stmt = gimple_build_assign (new_op, NOP_EXPR, unprom->op);
-
-  /* If the operation is the input to a vectorizable cast, try splitting
- that cast into two, taking the required result as a mid-way point.  */
-  if (unprom->caster)
-{
-  tree lhs = gimple_get_lhs (unprom->caster->stmt);
-  if (TYPE_PRECISION (TREE_TYPE (lhs)) > TYPE_PRECISION (type)
- && TYPE_PRECISION (type) > TYPE_PRECISION (unprom->type)
- && (TYPE_UNSIGNED (unprom->type) || !TYPE_UNSIGNED (type))
- && vect_split_statement (unprom->caster, new_op, new_stmt, vectype))
-   return new_op;
-}
+  gassign *new_stmt = gimple_build_assign (new_op, NOP_EXPR, input);
 
   /* If OP is an external value, see if we can insert the 

Re: avoid null ptr deref in cselib_record_sets

[Jakub Jelinek]

On Wed, Dec 05, 2018 at 04:50:19AM -0200, Alexandre Oliva wrote:
> Jeff tested this with a cross compiler to h8300-elf, and several other
> native and cross toolchains IIUC.  I'm regstrapping it myself on i686-
> and x86_64-linux-gnu.  Ok to install?
> 
> 
> for  gcc/ChangeLog
> 
>   * cselib.c (cselib_record_sets): Skip strict low part sets
>   with NULL src_elt.

Ok, thanks.

> --- a/gcc/cselib.c
> +++ b/gcc/cselib.c
> @@ -2616,6 +2616,7 @@ cselib_record_sets (rtx_insn *insn)
>preserves the upper bits that di:SI=zero_extend(flags:CCNO<=0).  */
>scalar_int_mode mode;
>if (dest != orig
> +   && sets[i].src_elt
> && cselib_record_sets_hook
> && REG_P (dest)
> && HARD_REGISTER_P (dest)

Jakub