Re: [C++ PATCH] Fix a recent regression (PR c++/90947)

[Jason Merrill]

On 10/30/19 7:29 PM, Jakub Jelinek wrote:

On Tue, Oct 29, 2019 at 04:26:14PM -0400, Jason Merrill wrote:

I think type_initializer_zero_p should return false if
CLASSTYPE_NON_AGGREGATE; we can't expect that value-initialization will have
the intended effect in that case.


That indeed works for this testcase and doesn't break anything else in the
testsuite.  I've actually used TYPE_NON_AGGREGATE_CLASS because a
RECORD_TYPE might not be always a CLASS_TYPE_P.

Here is what I've bootstrapped/regtested on x86_64-linux and i686-linux, ok
for trunk?


OK.


2019-10-30  Jakub Jelinek  

PR c++/90947
* tree.h (type_initializer_zero_p): Remove.
* tree.c (type_initializer_zero_p): Remove.
cp/
* cp-tree.h (type_initializer_zero_p): Declare.
* decl.c (reshape_init_array_1): Formatting fix.
* tree.c (type_initializer_zero_p): New function.  Moved from
../tree.c, use next_initializable_field, formatting fix.  Return
false for TYPE_NON_AGGREGATE_CLASS types.

--- gcc/tree.h.jj   2019-10-30 08:13:49.017848260 +0100
+++ gcc/tree.h  2019-10-30 08:57:59.457046198 +0100
@@ -4690,12 +4690,6 @@ extern tree first_field (const_tree);
  extern bool initializer_zerop (const_tree, bool * = NULL);
  extern bool initializer_each_zero_or_onep (const_tree);
  
-/* Analogous to initializer_zerop but also examines the type for

-   which the initializer is being used.  Unlike initializer_zerop,
-   considers empty strings to be zero initializers for arrays and
-   non-zero for pointers.  */
-extern bool type_initializer_zero_p (tree, tree);
-
  extern wide_int vector_cst_int_elt (const_tree, unsigned int);
  extern tree vector_cst_elt (const_tree, unsigned int);
  
--- gcc/tree.c.jj	2019-10-30 08:13:48.974848922 +0100

+++ gcc/tree.c  2019-10-30 08:57:59.460046152 +0100
@@ -11396,73 +11396,6 @@ initializer_each_zero_or_onep (const_tre
  }
  }
  
-/* Given an initializer INIT for a TYPE, return true if INIT is zero

-   so that it can be replaced by value initialization.  This function
-   distinguishes betwen empty strings as initializers for arrays and
-   for pointers (which make it return false).  */
-
-bool
-type_initializer_zero_p (tree type, tree init)
-{
-  if (type  == error_mark_node || init == error_mark_node)
-return false;
-
-  STRIP_NOPS (init);
-
-  if (POINTER_TYPE_P (type))
-return TREE_CODE (init) != STRING_CST && initializer_zerop (init);
-
-  if (TREE_CODE (init) != CONSTRUCTOR)
-return initializer_zerop (init);
-
-  if (TREE_CODE (type) == ARRAY_TYPE)
-{
-  tree elt_type = TREE_TYPE (type);
-  elt_type = TYPE_MAIN_VARIANT (elt_type);
-  if (elt_type == char_type_node)
-   return initializer_zerop (init);
-
-  tree elt_init;
-  unsigned HOST_WIDE_INT i;
-  FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), i, elt_init)
-   if (!type_initializer_zero_p (elt_type, elt_init))
- return false;
-  return true;
-}
-
-  if (TREE_CODE (type) != RECORD_TYPE)
-return initializer_zerop (init);
-
-  tree fld = TYPE_FIELDS (type);
-
-  tree fld_init;
-  unsigned HOST_WIDE_INT i;
-  FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), i, fld_init)
-{
-  /* Advance to the next member, skipping over everything that
-canot be initialized (including unnamed bit-fields).  */
-  while (TREE_CODE (fld) != FIELD_DECL
-|| DECL_ARTIFICIAL (fld)
-|| (DECL_BIT_FIELD (fld) && !DECL_NAME (fld)))
-   {
- fld = DECL_CHAIN (fld);
- if (!fld)
-   return true;
- continue;
-   }
-
-  tree fldtype = TREE_TYPE (fld);
-  if (!type_initializer_zero_p (fldtype, fld_init))
-   return false;
-
-  fld = DECL_CHAIN (fld);
-  if (!fld)
-   break;
-}
-
-  return true;
-}
-
  /* Check if vector VEC consists of all the equal elements and
 that the number of elements corresponds to the type of VEC.
 The function returns first element of the vector
--- gcc/cp/cp-tree.h.jj 2019-10-30 08:13:48.819851308 +0100
+++ gcc/cp/cp-tree.h2019-10-30 08:57:59.462046121 +0100
@@ -7380,6 +7380,11 @@ extern tree cxx_copy_lang_qualifiers (c
  
  extern void cxx_print_statistics		(void);

  extern bool maybe_warn_zero_as_null_pointer_constant (tree, location_t);
+/* Analogous to initializer_zerop but also examines the type for
+   which the initializer is being used.  Unlike initializer_zerop,
+   considers empty strings to be zero initializers for arrays and
+   non-zero for pointers.  */
+extern bool type_initializer_zero_p(tree, tree);
  
  /* in ptree.c */

  extern void cxx_print_xnode   (FILE *, tree, int);
--- gcc/cp/decl.c.jj2019-10-30 08:13:48.885850291 +0100
+++ gcc/cp/decl.c   2019-10-30 08:57:59.466046060 +0100
@@ -5972,9 +5972,8 @@ reshape_init_array_1 (tree elt_type, tre
/* Pointers initialized to strings must be treated as non-zero
 even if the 

Re: [PATCH] PR c++/84810 - constraints on lambdas

[Jason Merrill]

On 10/30/19 7:20 AM, Jeff Chapman wrote:

Hello,

Attached is a patch that adds parsing of the optional requires-clause in a
lambda-expression and lambda-declarator. Additionally, shorthand constraints
from the template-parameter-list are now actually applied and constrain the
synthesized operator().

Previously we were not parsing the requires clauses at all and not saving the
shorthand constraints in the place expected by grokfndecl.

The trailing requires-clause is now also used to suppress synthesis of the
conversion to function pointer for non-capturing non-generic lambdas as per
expr.prim.lambda.closure/7.

This includes a fix to template_class_depth. Previously it was computing the
wrong depth for lambdas in the initializer of a static member of a class
template, exhibited by the concepts-lambda4 test which currently fails on
trunk. The bug was causing grokfndecl to use the constraints from the template
class for the lambda.

gcc/cp/
2019-10-30  Jeff Chapman II  

PR c++/84810 - constraints on lambdas
* lambda.c (maybe_add_lambda_conv_op): Do not synthesize
conversion if the call operator does not satisfy its constraints.
* parser.c (cp_parser_lambda_declarator_opt): Parse
requires-clause on generic lambdas; combine with shorthand
constraints. Parse trailing requires-clause and attach to the
synthesized call operator.
* pt.c (template_class_depth): Only inspect
LAMBDA_TYPE_EXTRA_SCOPE if it is present. This fixes an
incorrect depth calculation for lambdas inside the initializer
of a static data member of a template class.

gcc/testsuite/
2019-10-30  Jeff Chapman II  

PR c++/84810 - constraints on lambdas
* g++.dg/cpp2a/concepts-lambda2.C: New test.
* g++.dg/cpp2a/concepts-lambda3.C: Ditto.
* g++.dg/cpp2a/concepts-lambda4.C: Ditto.
* g++.dg/cpp2a/concepts-pr84810.C: Ditto.

Bootstrapped and tested on x86_64-pc-linux-gnu.

Please let me know if there's any issues.


Applied, thanks.

Jason

Re: [PATCH V2] rs6000: Refine unroll factor with target unroll_adjust hook.

[Jiufu Guo]

Segher Boessenkool  writes:

> Hi!
>
> On Wed, Oct 30, 2019 at 11:34:42AM +0800, Jiufu Guo wrote:
>> In this patch, loop unroll adjust hook is introduced for powerpc. In this 
>> hook,
>> we can do target related hueristic adjustment. For this patch, we tunned for
>> O2 to unroll small loops with small unroll factor (2 times), for other
>> optimization, default unroll factor is used.
>
>> 2019-10-30  Jiufu Guo
>> 
>>  PR tree-optimization/88760
>
> I think this should be "target"?  (Change it in bugzilla if you change it
> here, of course).
Or should I open a new bug, then PR88760 could be keep for tree-optimization?
>
>>  * config/rs6000/rs6000.c (rs6000_option_override_internal): Remove
>>  code which changes PARAM_MAX_UNROLL_TIMES and PARAM_MAX_UNROLLED_INSNS.
>>  (TARGET_LOOP_UNROLL_ADJUST): Add loop unroll adjust hook.
>>  (rs6000_loop_unroll_adjust): New hook for loop unroll adjust. Unrolling
>>  small loop 2 times for -O2.
>
> (Two spaces after a full stop).
>
>> +/* This target hook implementation for TARGET_LOOP_UNROLL_ADJUST calculates
>> +   a new heristic number struct loop *loop should be unrolled.  */
>
> "heuristic". 
>
> Argument names are written in ALL CAPS, and repeating the type is useless
> here (you can see it just a few lines down).  But anyway, everything else
> here just says things like
>
> /*  Implement targetm.loop_unroll_adjust.  */
>
> so just do that?
>
>> +static unsigned
>> +rs6000_loop_unroll_adjust (unsigned nunroll, struct loop * loop)
>> +{
>> +  /* For -O2, we only unroll small loops with small unroll factor.  */
>> +  if (optimize == 2)
>> +{
>> +  /* If the loop contains few insns, treated it as small loops.
>> + TODO: Uing 10 hard code for now.  Continue to refine, For example,
>> + if loop constians only 1-2 insns, we may unroll more times(4).
>> + And we may use PARAM to control kinds of loop size.  */
>
> That first line has no new information.  So maybe something like
>   /* TODO: This is hardcoded to 10 right now.  It can be refined, for
>example we may want to unroll very small loops more times (4 perhaps).
>We also should use a PARAM for this.  */
Thanks for your so detail and careful suggestions which always help me
to make patch better! 
>
> Okay for trunk like that.  Thanks!
>
>
> Segher

[C++ PATCH] PR c++/92268 - hard error satisfying return-type-requirement

[Jason Merrill]

Previously we would put the template arguments for the concept-check in a
TEMPLATE_ID and then also pass them to constraints_satisfied_p, which meant
that we would try to normalize the concept-check with the fully instantiated
arguments, leading to sadness.  Simply not passing the args to
constraints_satisfied_p fixes the problem.

I also noticed that we weren't detecting substitution failure in the
constraints, but were silently treating it as success.

Tested x86_64-pc-linux-gnu, applying to trunk.

* constraint.cc (type_deducible_p): Check for substitution failure.
(diagnose_compound_requirement): Adjust diagnostic.
* pt.c (do_auto_deduction): Don't pass cargs to
constraints_satisfied_p.
---
 gcc/cp/constraint.cc  | 15 +--
 gcc/cp/pt.c   |  2 +-
 gcc/testsuite/g++.dg/concepts/diagnostic1.C   |  4 ++--
 gcc/testsuite/g++.dg/concepts/placeholder3.C  |  2 +-
 gcc/testsuite/g++.dg/concepts/placeholder4.C  |  2 +-
 gcc/testsuite/g++.dg/cpp2a/concepts-pr67178.C |  2 +-
 .../g++.dg/cpp2a/concepts-requires6.C |  2 +-
 .../g++.dg/cpp2a/concepts-return-req1.C   | 19 +++
 8 files changed, 35 insertions(+), 13 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-return-req1.C

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index b8a2645d8c9..db2a30ced7c 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -1822,10 +1822,7 @@ tsubst_type_requirement (tree t, tree args, subst_info 
info)
   return finish_type_requirement (EXPR_LOCATION (t), type);
 }
 
-/* True if TYPE can be deduced from EXPR.
-
-   FIXME: C++20 compound requirement constraints should be normalized and then
-   satisfied rather than substituted.  */
+/* True if TYPE can be deduced from EXPR.  */
 
 static bool
 type_deducible_p (tree expr, tree type, tree placeholder, tree args,
@@ -1839,12 +1836,17 @@ type_deducible_p (tree expr, tree type, tree 
placeholder, tree args,
  substitutes args into any template parameters in the trailing
  result type.  */
   tree saved_constr = PLACEHOLDER_TYPE_CONSTRAINTS (placeholder);
-  PLACEHOLDER_TYPE_CONSTRAINTS (placeholder)
+  tree subst_constr
 = tsubst_constraint (saved_constr,
 args,
 info.complain | tf_partial,
 info.in_decl);
 
+  if (subst_constr == error_mark_node)
+return false;
+
+  PLACEHOLDER_TYPE_CONSTRAINTS (placeholder) = subst_constr;
+
   /* Temporarily unlink the canonical type.  */
   tree saved_type = TYPE_CANONICAL (placeholder);
   TYPE_CANONICAL (placeholder) = NULL_TREE;
@@ -3139,7 +3141,8 @@ diagnose_compound_requirement (tree req, tree args, tree 
in_decl)
  if (!type_deducible_p (expr, type, placeholder, args, quiet))
{
  tree orig_expr = TREE_OPERAND (req, 0);
- inform (loc, "type deduction from %qE failed", orig_expr);
+ inform (loc, "%qE does not satisfy return-type-requirement",
+ orig_expr);
 
  /* Further explain the reason for the error.  */
  type_deducible_p (expr, type, placeholder, args, noisy);
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index c5675dd8e3f..414140ade6c 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -28138,7 +28138,7 @@ do_auto_deduction (tree type, tree init, tree auto_node,
/* Rebuild the check using the deduced arguments.  */
check = build_concept_check (cdecl, cargs, tf_none);
 
-   if (!constraints_satisfied_p (check, cargs))
+   if (!constraints_satisfied_p (check))
   {
 if (complain & tf_warning_or_error)
   {
diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic1.C 
b/gcc/testsuite/g++.dg/concepts/diagnostic1.C
index ced56d400ba..7da08db2792 100644
--- a/gcc/testsuite/g++.dg/concepts/diagnostic1.C
+++ b/gcc/testsuite/g++.dg/concepts/diagnostic1.C
@@ -8,12 +8,12 @@ concept bool SameAs = __is_same_as(T, U);
 template 
 concept bool R1 = requires (T& t) { // { dg-message "in requirements" }
   { t.begin() } -> T;  // { dg-error "no match" }
-  { t.end() } -> SameAs;   // { dg-error "does not satisfy" }
+  { t.end() } -> SameAs;   // { dg-message "does not satisfy" }
 };
 
 template 
 concept bool R2 = requires (T& t) { // { dg-message "in requirements" }
-  { t.end() } -> SameAs;   // { dg-error "does not satisfy" }
+  { t.end() } -> SameAs;   // { dg-message "does not satisfy" }
 };
 
 struct foo {
diff --git a/gcc/testsuite/g++.dg/concepts/placeholder3.C 
b/gcc/testsuite/g++.dg/concepts/placeholder3.C
index 4f8600bd07f..d90e5cfb02f 100644
--- a/gcc/testsuite/g++.dg/concepts/placeholder3.C
+++ b/gcc/testsuite/g++.dg/concepts/placeholder3.C
@@ -8,7 +8,7 @@ concept bool Same = __is_same_as(T, U);
 template 
 concept bool C =
   requires { // { dg-message "in requirements" }
-{ 0 } -> Same;  // { dg-error "does not satisfy" }
+{ 0 } 

Re: [PATCH v2 2/2] sched-deps.c: Avoid replacing address if it increases address cost

[Jim Wilson]

On Fri, Oct 25, 2019 at 10:40 AM Craig Blackmore
 wrote:
> The sched2 pass undoes some of the addresses generated by the RISC-V
> shorten_memrefs code size optimization (patch 1/2) and consequently increases
> code size. This patch prevents sched-deps.c from changing an address if it is
> expected to increase address cost.

This should be rewritten as a target hook, and then we can define the
hook to do what we want for RISC-V.  It isn't OK to make this change
for other targets without testing them.  If the default hook does
nothing, then other targets won't be affected.

Jim

Re: [PATCH v2 1/2] RISC-V: Add shorten_memrefs pass

[Jim Wilson]

FYI the testcase I'm using to test the patch.  Some of the functions
get smaller, some of them get bigger, and some don't change in size
but should when compiled for an rv64 target.

Jim
void
store1z (int *array)
{
  array[200] = 0;
  array[201] = 0;
  array[202] = 0;
  array[203] = 0;
}

void
store2z (long long *array)
{
  array[200] = 0;
  array[201] = 0;
  array[202] = 0;
  array[203] = 0;
}

void
store1a (int *array, int a)
{
  array[200] = a;
  array[201] = a;
  array[202] = a;
  array[203] = a;
}

void
store2a (long long *array, long long a)
{
  array[200] = a;
  array[201] = a;
  array[202] = a;
  array[203] = a;
}

int
load1r (int *array)
{
  int a = 0;
  a += array[200];
  a += array[201];
  a += array[202];
  a += array[203];
  return a;
}

long long
load2r (long long *array)
{
  int a = 0;
  a += array[200];
  a += array[201];
  a += array[202];
  a += array[203];
  return a;
}

extern int sub1 (int, int, int, int, int, int, int);

int
load1a (int a0, int a1, int a2, int a3, int a4, int *array)
{
  int a = 0;
  a += array[200];
  a += array[201];
  a += array[202];
  a += array[203];
  return sub1 (a0, a1, a2, a3, a4, 0, a);
}

extern long long sub2 (long long, long long, long long, long long, long long,
		   long long, long long);

long long
load2a (long long a0, long long a1, long long a2, long long a3, long long a4,
	long long *array)
{
  int a = 0;
  a += array[200];
  a += array[201];
  a += array[202];
  a += array[203];
  return sub2 (a0, a1, a2, a3, a4, 0, a);
}

Re: [PATCH v2 1/2] RISC-V: Add shorten_memrefs pass

[Jim Wilson]

On Fri, Oct 25, 2019 at 10:40 AM Craig Blackmore
 wrote:
> This patch aims to allow more load/store instructions to be compressed by
> replacing a load/store of 'base register + large offset' with a new load/store
> of 'new base + small offset'. If the new base gets stored in a compressed
> register, then the new load/store can be compressed. Since there is an 
> overhead
> in creating the new base, this change is only attempted when 'base register' 
> is
> referenced in at least 4 load/stores in a basic block.
>
> The optimization is implemented in a new RISC-V specific pass called
> shorten_memrefs which is enabled for RVC targets. It has been developed for 
> the
> 32-bit lw/sw instructions but could also be extended to 64-bit ld/sd in 
> future.

The fact that this needs 4 load/stores in a block with the same base
address means it won't trigger very often.  But it does seem to work.
I see about a 0.05% code size reduction for a rv32gc newlib nano
libc.a.  There might be other codes that benefit more.

I'm concerned about the number of RISC-V specific optimization passes
people are writing.  I've seen at least 3 so far.  This one is at
least small and simple enough to understand that it doesn't look like
it will cause maintenance problems.

The config.gcc change conflicts with the save-restore optimization
pass that Andrew Burgess added, but that is a trivial fix.

The code only works for 32-bit load/stores.  rv64 has compressed
64-bit load/stores, and the F and D extensions have float and double
compressed loads and stores.  The patch would be more useful if it
handled all of these.

The patch doesn't check the other operand, it only looks at the memory
operand.  This results in some cases where the code rewrites
instructions that can never be compressed.  For instance, given
void
store1z (int *array)
{
  array[200] = 0;
  array[201] = 0;
  array[202] = 0;
  array[203] = 0;
}
the patch increases code size by 4 bytes because it rewrites the
stores, but we don't have compressed store 0, and x0 is not a
compressed reg, so there is no point in rewriting the stores.  I can
also create examples that show a size increase with loads but it is a
little trickier.
extern int sub1 (int, int, int, int, int, int, int);
int
load1a (int a0, int a1, int a2, int a3, int a4, int *array)
{
  int a = 0;
  a += array[200];
  a += array[201];
  a += array[202];
  a += array[203];
  return sub1 (a0, a1, a2, a3, a4, 0, a);
}
The register allocator will pass a0-a4 directly through from args to
the call, leaving only a5-a7 for the load base address and dest, and
only one of those regs is a compressed reg, but we need two, so these
loads can't be compressed.  The code still gets rewritten, resulting
in a size increase for the extra add.  Not sure if you can do anything
about that though, since you are doing this before register
allocation.

It isn't clear that the change riscv_address_cost is for.  That should
be commented.

I'd suggest parens in the riscv_compressed_lw_offset_p return
statement, that makes it easier to align the code correctly (in emacs
at least).

The RISCV_MAX_COMPRESSED_LW_OFFSET should be moved down to the "ISA
constants" section of riscv.h, and maybe renamed similar to the other
constants.

There is a REG_P check in get_si_mem_reg.  That should probably handle
SUBREGs too.

A testcase to verify the patch would be nice.  I have one I wrote for
testing that shows some tests that work, some tests that don't work,
and some that work but maybe shouldn't.

I vaguely remember Micheal Meissner talking about doing something
similar for PPC in a GNU Cauldron maybe 2 years ago.  But I don't know
if he tried, or how far he got if he did try.  It might be useful to
ask him about that work.

Otherwise this looks OK to me.

Jim

[Patch,Fortran] PR92284 – gfc_desc_to_cfi_desc fixes

[Tobias Burnus]

Playing with the PR92284 test case revealed two issues related to 
gfc_desc_to_cfi_desc:


* Access of uninitialized memory – copying the array bounds (in 
libgfortran) does not make sense for unallocted allocatables and 
nullified pointers. Hence, check for ".data == NULL".


* There is a memory leak. I misunderstood the dump when fixing PR91863 
(rev.277502).

https://gcc.gnu.org/ml/gcc-patches/2019-10/msg01651.html

Regarding the latter: If one passed gfc_desc_to_cfi_desc a pointer var, 
pointing to NULL, as CFI (Bind(C) array descriptor) argument, 
libgfortran allocates the memory for the descriptor – which at some 
point has to be freed.


Contrary to the original version, one can free that memory 
unconditionally. (Not only because "free" handles NULL pointers but – 
unless "malloc" failed – we know that ptr has been malloced.) I also 
tried to make the comments a bit clearer.


Build and regtested.
OK for trunk and GCC 9 (the latter is also affected)?

Tobias

PR: Related pending patch: 
https://gcc.gnu.org/ml/gcc-patches/2019-10/msg02148.html
Also missing: At the end of a bind(C) procedure written in Fortran, 
allocatable/pointers array arguments need get updated: the "data" and 
the bounds part of the array descriptor might have changed while running 
the procedure body. Cf. this PR and PR 92189


	gcc/fortran/
	PR fortran/92284.
	* trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Free CFI descriptor
	at the end; partial revised revert of Rev. 277502.

 	libgfortran/
	PR fortran/92284.
	* runtime/ISO_Fortran_binding.c (gfc_desc_to_cfi_desc):

 	gcc/testsuite/
	PR fortran/92284.
	* gfortran.dg/bind-c-intent-out.f90: Update expected dump;
	extend comment.
	* gfortran.dg/bind_c_array_params_3.f90: New.
	* gfortran.dg/bind_c_array_params_3_aux.c: New.

diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c
index 7eba1bbd082..f800faaa4e5 100644
--- a/gcc/fortran/trans-expr.c
+++ b/gcc/fortran/trans-expr.c
@@ -5303,13 +5303,13 @@ gfc_conv_gfc_desc_to_cfi_desc (gfc_se *parmse, gfc_expr *e, gfc_symbol *fsym)
   /* Now pass the gfc_descriptor by reference.  */
   parmse->expr = gfc_build_addr_expr (NULL_TREE, parmse->expr);
 
-  /* Variables to point to the gfc and CFI descriptors.  */
+  /* Variables to point to the gfc and CFI descriptors; cfi = NULL implies
+ that the CFI descriptor is allocated by the gfor_fndecl_gfc_to_cfi call.  */
   gfc_desc_ptr = parmse->expr;
   cfi_desc_ptr = gfc_create_var (pvoid_type_node, "cfi");
-  gfc_add_modify (>pre, cfi_desc_ptr,
-		  build_int_cst (pvoid_type_node, 0));
+  gfc_add_modify (>pre, cfi_desc_ptr, null_pointer_node);
 
-  /* Allocate the CFI descriptor and fill the fields.  */
+  /* Allocate the CFI descriptor itself and fill the fields.  */
   tmp = gfc_build_addr_expr (NULL_TREE, cfi_desc_ptr);
   tmp = build_call_expr_loc (input_location,
 			 gfor_fndecl_gfc_to_cfi, 2, tmp, gfc_desc_ptr);
@@ -5324,6 +5324,10 @@ gfc_conv_gfc_desc_to_cfi_desc (gfc_se *parmse, gfc_expr *e, gfc_symbol *fsym)
   /* The CFI descriptor is passed to the bind_C procedure.  */
   parmse->expr = cfi_desc_ptr;
 
+  /* Free the CFI descriptor.  */
+  tmp = gfc_call_free (cfi_desc_ptr);
+  gfc_prepend_expr_to_block (>post, tmp);
+
   /* Transfer values back to gfc descriptor.  */
   tmp = gfc_build_addr_expr (NULL_TREE, parmse->expr);
   tmp = build_call_expr_loc (input_location,
diff --git a/gcc/testsuite/gfortran.dg/bind-c-intent-out.f90 b/gcc/testsuite/gfortran.dg/bind-c-intent-out.f90
index 493e546d45d..39822c0753a 100644
--- a/gcc/testsuite/gfortran.dg/bind-c-intent-out.f90
+++ b/gcc/testsuite/gfortran.dg/bind-c-intent-out.f90
@@ -35,7 +35,8 @@ end program p
 ! the intent(out) implies freeing in the callee (!), hence the "free"
 ! It is the only 'free' as 'a' is part of the main program and, hence, implicitly has the SAVE attribute.
 ! The  'cfi = 0' appears before the call due to the deallocate and when preparing the C descriptor
+! As cfi (i.e. the descriptor itself) is allocated in libgomp, it has to be freed after the call.
 
-! { dg-final { scan-tree-dump-times "__builtin_free" 1 "original" } }
-! { dg-final { scan-tree-dump-times "__builtin_free \\(cfi\\.\[0-9\]+\\);" 1 "original" } }
+! { dg-final { scan-tree-dump-times "__builtin_free" 2 "original" } }
+! { dg-final { scan-tree-dump-times "__builtin_free \\(cfi\\.\[0-9\]+\\);" 2 "original" } }
 ! { dg-final { scan-tree-dump-times "cfi\\.\[0-9\]+ = 0B;" 2 "original" } }
diff --git a/gcc/testsuite/gfortran.dg/bind_c_array_params_3.f90 b/gcc/testsuite/gfortran.dg/bind_c_array_params_3.f90
new file mode 100644
index 000..d5bad7d03f2
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/bind_c_array_params_3.f90
@@ -0,0 +1,39 @@
+! { dg-do run }
+! { dg-additional-sources bind_c_array_params_3_aux.c }
+!
+! PR fortran/92284
+!
+! Contributed by José Rui Faustino de Sousa
+!
+program arr_p
+  use, intrinsic :: iso_c_binding, only: c_int
+  implicit none (type, external)
+
+  integer(kind=c_int), pointer :: arr(:)

[C++ PATCH] Fix a recent regression (PR c++/90947)

[Jakub Jelinek]

On Tue, Oct 29, 2019 at 04:26:14PM -0400, Jason Merrill wrote:
> I think type_initializer_zero_p should return false if
> CLASSTYPE_NON_AGGREGATE; we can't expect that value-initialization will have
> the intended effect in that case.

That indeed works for this testcase and doesn't break anything else in the
testsuite.  I've actually used TYPE_NON_AGGREGATE_CLASS because a
RECORD_TYPE might not be always a CLASS_TYPE_P.

Here is what I've bootstrapped/regtested on x86_64-linux and i686-linux, ok
for trunk?

2019-10-30  Jakub Jelinek  

PR c++/90947
* tree.h (type_initializer_zero_p): Remove.
* tree.c (type_initializer_zero_p): Remove.
cp/
* cp-tree.h (type_initializer_zero_p): Declare.
* decl.c (reshape_init_array_1): Formatting fix.
* tree.c (type_initializer_zero_p): New function.  Moved from
../tree.c, use next_initializable_field, formatting fix.  Return
false for TYPE_NON_AGGREGATE_CLASS types.

--- gcc/tree.h.jj   2019-10-30 08:13:49.017848260 +0100
+++ gcc/tree.h  2019-10-30 08:57:59.457046198 +0100
@@ -4690,12 +4690,6 @@ extern tree first_field (const_tree);
 extern bool initializer_zerop (const_tree, bool * = NULL);
 extern bool initializer_each_zero_or_onep (const_tree);
 
-/* Analogous to initializer_zerop but also examines the type for
-   which the initializer is being used.  Unlike initializer_zerop,
-   considers empty strings to be zero initializers for arrays and
-   non-zero for pointers.  */
-extern bool type_initializer_zero_p (tree, tree);
-
 extern wide_int vector_cst_int_elt (const_tree, unsigned int);
 extern tree vector_cst_elt (const_tree, unsigned int);
 
--- gcc/tree.c.jj   2019-10-30 08:13:48.974848922 +0100
+++ gcc/tree.c  2019-10-30 08:57:59.460046152 +0100
@@ -11396,73 +11396,6 @@ initializer_each_zero_or_onep (const_tre
 }
 }
 
-/* Given an initializer INIT for a TYPE, return true if INIT is zero
-   so that it can be replaced by value initialization.  This function
-   distinguishes betwen empty strings as initializers for arrays and
-   for pointers (which make it return false).  */
-
-bool
-type_initializer_zero_p (tree type, tree init)
-{
-  if (type  == error_mark_node || init == error_mark_node)
-return false;
-
-  STRIP_NOPS (init);
-
-  if (POINTER_TYPE_P (type))
-return TREE_CODE (init) != STRING_CST && initializer_zerop (init);
-
-  if (TREE_CODE (init) != CONSTRUCTOR)
-return initializer_zerop (init);
-
-  if (TREE_CODE (type) == ARRAY_TYPE)
-{
-  tree elt_type = TREE_TYPE (type);
-  elt_type = TYPE_MAIN_VARIANT (elt_type);
-  if (elt_type == char_type_node)
-   return initializer_zerop (init);
-
-  tree elt_init;
-  unsigned HOST_WIDE_INT i;
-  FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), i, elt_init)
-   if (!type_initializer_zero_p (elt_type, elt_init))
- return false;
-  return true;
-}
-
-  if (TREE_CODE (type) != RECORD_TYPE)
-return initializer_zerop (init);
-
-  tree fld = TYPE_FIELDS (type);
-
-  tree fld_init;
-  unsigned HOST_WIDE_INT i;
-  FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), i, fld_init)
-{
-  /* Advance to the next member, skipping over everything that
-canot be initialized (including unnamed bit-fields).  */
-  while (TREE_CODE (fld) != FIELD_DECL
-|| DECL_ARTIFICIAL (fld)
-|| (DECL_BIT_FIELD (fld) && !DECL_NAME (fld)))
-   {
- fld = DECL_CHAIN (fld);
- if (!fld)
-   return true;
- continue;
-   }
-
-  tree fldtype = TREE_TYPE (fld);
-  if (!type_initializer_zero_p (fldtype, fld_init))
-   return false;
-
-  fld = DECL_CHAIN (fld);
-  if (!fld)
-   break;
-}
-
-  return true;
-}
-
 /* Check if vector VEC consists of all the equal elements and
that the number of elements corresponds to the type of VEC.
The function returns first element of the vector
--- gcc/cp/cp-tree.h.jj 2019-10-30 08:13:48.819851308 +0100
+++ gcc/cp/cp-tree.h2019-10-30 08:57:59.462046121 +0100
@@ -7380,6 +7380,11 @@ extern tree cxx_copy_lang_qualifiers (c
 
 extern void cxx_print_statistics   (void);
 extern bool maybe_warn_zero_as_null_pointer_constant (tree, location_t);
+/* Analogous to initializer_zerop but also examines the type for
+   which the initializer is being used.  Unlike initializer_zerop,
+   considers empty strings to be zero initializers for arrays and
+   non-zero for pointers.  */
+extern bool type_initializer_zero_p(tree, tree);
 
 /* in ptree.c */
 extern void cxx_print_xnode(FILE *, tree, int);
--- gcc/cp/decl.c.jj2019-10-30 08:13:48.885850291 +0100
+++ gcc/cp/decl.c   2019-10-30 08:57:59.466046060 +0100
@@ -5972,9 +5972,8 @@ reshape_init_array_1 (tree elt_type, tre
   /* Pointers initialized to strings must be treated as non-zero
 even if the string is empty.  */
   tree init_type = 

Re: [PATCH] PR92090: Fix testcase failures by r276469

[Segher Boessenkool]

Hi!

On Wed, Oct 30, 2019 at 02:02:44AM -0500, Xiong Hu Luo wrote:
> -finline-functions is enabled by default for O2 since r276469, update the
> test cases that inline small functions caused instruction number difference.

> --- a/gcc/testsuite/gcc.target/powerpc/pr79439-1.c
> +++ b/gcc/testsuite/gcc.target/powerpc/pr79439-1.c
> @@ -22,7 +22,7 @@ rec (int a)
>return a + ret;
>  }
>  
> -/* { dg-final { scan-assembler-times {\mbl f\M}   1 } } */
> -/* { dg-final { scan-assembler-times {\mbl g\M}   1 } } */
> +/* { dg-final { scan-assembler-times {\mbl f\M}   2 } } */
> +/* { dg-final { scan-assembler-times {\mbl g\M}   2 } } */
>  /* { dg-final { scan-assembler-times {\mbl rec\M} 1 } } */
> -/* { dg-final { scan-assembler-times {\mnop\M}2 } } */
> +/* { dg-final { scan-assembler-times {\mnop\M}4 } } */

It's probably better to disable inlining for this one.

> diff --git a/gcc/testsuite/gcc.target/powerpc/vsx-builtin-7.c 
> b/gcc/testsuite/gcc.target/powerpc/vsx-builtin-7.c
> index 5d31309f272..66b526832eb 100644
> --- a/gcc/testsuite/gcc.target/powerpc/vsx-builtin-7.c
> +++ b/gcc/testsuite/gcc.target/powerpc/vsx-builtin-7.c
> @@ -192,7 +192,7 @@ vector unsigned __int128 splat_uint128 (unsigned __int128 
> x) { return vec_splats
>  
>  /* { dg-final { scan-assembler-times {\mrldic\M} 0  { target { be && ilp32 } 
> } } } */
>  /* { dg-final { scan-assembler-times {\mrldic\M} 64 { target { be && lp64 } 
> } } } */
> -/* { dg-final { scan-assembler-times {\mrldic\M} 64 { target le } } } */
> +/* { dg-final { scan-assembler-times {\mrldic\M} 65 { target le } } } */
>  /* { dg-final { scan-assembler-times "xxpermdi" 4 { target be } } } */
>  /* { dg-final { scan-assembler-times "xxpermdi" 6 { target le } } } */
>  /* { dg-final { scan-assembler-times "vspltisb" 2 } } */

This one, too.

(Also needs testing on BE btw -- it's often fine if you don't, but only
fixing the tests for LE isn't great ;-) )


Segher

Re: [PATCH 2/2] Remove cgraph_local_info structure.

[Jeff Law]

On 10/30/19 4:06 PM, Joseph Myers wrote:
> On Wed, 30 Oct 2019, Joseph Myers wrote:
> 
>> This appears to have broken the build for Arm.
> 
> And probably bfin and c6x as well, based on grep, but my bot only covers 
> glibc targets so doesn't test those.
> 
bfin and c6x are definitely affected.  My tester flagged them.

jeff

Re: [PATCH V2] rs6000: Refine unroll factor with target unroll_adjust hook.

[Segher Boessenkool]

Hi!

On Wed, Oct 30, 2019 at 11:34:42AM +0800, Jiufu Guo wrote:
> In this patch, loop unroll adjust hook is introduced for powerpc. In this 
> hook,
> we can do target related hueristic adjustment. For this patch, we tunned for
> O2 to unroll small loops with small unroll factor (2 times), for other
> optimization, default unroll factor is used.

> 2019-10-30  Jiufu Guo 
> 
>   PR tree-optimization/88760

I think this should be "target"?  (Change it in bugzilla if you change it
here, of course).

>   * config/rs6000/rs6000.c (rs6000_option_override_internal): Remove
>   code which changes PARAM_MAX_UNROLL_TIMES and PARAM_MAX_UNROLLED_INSNS.
>   (TARGET_LOOP_UNROLL_ADJUST): Add loop unroll adjust hook.
>   (rs6000_loop_unroll_adjust): New hook for loop unroll adjust. Unrolling
>   small loop 2 times for -O2.

(Two spaces after a full stop).

> +/* This target hook implementation for TARGET_LOOP_UNROLL_ADJUST calculates
> +   a new heristic number struct loop *loop should be unrolled.  */

"heuristic".

Argument names are written in ALL CAPS, and repeating the type is useless
here (you can see it just a few lines down).  But anyway, everything else
here just says things like

/*  Implement targetm.loop_unroll_adjust.  */

so just do that?

> +static unsigned
> +rs6000_loop_unroll_adjust (unsigned nunroll, struct loop * loop)
> +{
> +  /* For -O2, we only unroll small loops with small unroll factor.  */
> +  if (optimize == 2)
> +{
> +  /* If the loop contains few insns, treated it as small loops.
> +  TODO: Uing 10 hard code for now.  Continue to refine, For example,
> +  if loop constians only 1-2 insns, we may unroll more times(4).
> +  And we may use PARAM to control kinds of loop size.  */

That first line has no new information.  So maybe something like
  /* TODO: This is hardcoded to 10 right now.  It can be refined, for
 example we may want to unroll very small loops more times (4 perhaps).
 We also should use a PARAM for this.  */

Okay for trunk like that.  Thanks!


Segher

Re: [PATCH 2/2] Remove cgraph_local_info structure.

[Joseph Myers]

On Wed, 30 Oct 2019, Joseph Myers wrote:

> This appears to have broken the build for Arm.

And probably bfin and c6x as well, based on grep, but my bot only covers 
glibc targets so doesn't test those.

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

Re: [C++ PATCH] P0784R7 constexpr new fixes (PR c++/91369)

[Jakub Jelinek]

On Wed, Oct 30, 2019 at 04:26:17PM -0400, Jason Merrill wrote:
> OK.   
>  

Thanks, committed.

> > Now, for the accepts invalid issues.
> >  From what I understood, in constant evaluation
> > ::operator new{,[]} or ::operator delete{,[]}
> > can't be called from anywhere, only from new/delete expressions or
> > std::allocator::{,de}allocate, is that correct?
> > If so, perhaps we need some CALL_EXPR flag that we'd set on the call
> > coming from new/delete expressions and disallow calls to
> > replaceable global allocator functions in constexpr evaluation unless
> > that flag is set or it is in std::allocator::{,de}allocate.
> 
> Looks like there used to be a TREE_CALLS_NEW flag in TREE_LANG_FLAG_1, but
> that flag is now free for CALL_EXPR.

I'll try a CALL_EXPR flag first.

> > Another thing is that even with that change,
> >std::allocator a;
> >auto p = a.allocate (1);
> >*p = 1;
> >a.deallocate (p, 1);
> > would be accepted during constexpr evaluation, because allocate already
> > has the cast which turns "heap uninit" variable into "heap " and assigns
> > it a type, so there is nothing that will prevent the store from succeeding.
> 
> What's wrong with the store?

If it is fine, the better.  I'd still think that
  struct S { constexpr S () : s (0) {}; int s; };
  std::allocator a;
  auto p = a.allocate (1);
  p->s = 1;
  a.deallocate (p, 1);
would still be invalid though, because that type needs constructing and
the construction didn't happen in that case.  Or:
  struct S { constexpr S () : s (0) {}; int s; };
  std::allocator a;
  auto p = a.allocate (1);
  std::construct_at (p);
  p->s++; // ok
  std::destruct_at (p);
  p->s = 1; // shouldn't this be an error?
  a.deallocate (p, 1);
but I admit I haven't tried to back that up by some standard wording, just a
general principle that objects with TYPE_ADDRESSABLE types need to be
constructed first and destructed last and accesses to it are only valid in
between those in normal code and constexpr should flag any UB as errors.

Jakub

Re: [PATCH 2/2] Remove cgraph_local_info structure.

[Joseph Myers]

This appears to have broken the build for Arm.

/scratch/jmyers/glibc-bot/src/gcc/gcc/config/arm/arm.c: In function 'arm_pcs 
arm_get_pcs_model(const_tree, const_tree)':
/scratch/jmyers/glibc-bot/src/gcc/gcc/config/arm/arm.c:5959:4: error: 
'cgraph_local_info' was not declared in this scope
cgraph_local_info *i = cgraph_node::local_info (CONST_CAST_TREE(decl));
^
/scratch/jmyers/glibc-bot/src/gcc/gcc/config/arm/arm.c:5959:23: error: 'i' was 
not declared in this scope
cgraph_local_info *i = cgraph_node::local_info (CONST_CAST_TREE(decl));
   ^
/scratch/jmyers/glibc-bot/src/gcc/gcc/config/arm/arm.c:5959:27: error: 
'local_info' is not a member of 'cgraph_node'
cgraph_local_info *i = cgraph_node::local_info (CONST_CAST_TREE(decl));
   ^
Makefile:2277: recipe for target 'arm.o' failed
make[3]: *** [arm.o] Error 1

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

Re: [RFC PATCH] targetm.omp.device_kind_arch_isa and OpenMP declare variant kind/arch/isa handling

[Segher Boessenkool]

On Tue, Oct 29, 2019 at 11:57:40PM +0100, Jakub Jelinek wrote:
> On Tue, Oct 29, 2019 at 05:40:11PM -0500, Segher Boessenkool wrote:
> > On Tue, Oct 29, 2019 at 06:15:31PM +0100, Jakub Jelinek wrote:
> > There already are a lot of different ways to get information about the
> > execution environment you're running on; why is this any better?
> 
> There is no question of better or worse, it is simply a part of a standard
> that GCC is trying to implement, like we try to implement all of C++20, we
> also try to implement all of OpenMP or OpenACC etc.

We now get three levels: "arch", "isa", "extensions".  Power has only at
most half of such levels: arch is Power, isa is Power, you can say things
like AltiVec (aka VMX) and VSX are extensions (that's what the X stands
for anyway :-) ).  In older versions of the architecture we had (optional)
"categories", but that is gone as well.

So yes I wonder how we should map reality to these three levels.  And I
also do wonder how this is better than one level as anything else has :-)

> The exact identifiers are implementation defined though, and that is why we
> need to think of what is reasonable for each target (at least each target
> where OpenMP is used often, powerpc is certainly one of those).

Do you know any OpenMP on Power users we can ask for their opinions?


Segher

Re: [C++ PATCH] Implement P1073R3: Immediate functions (PR c++/88335)

[Jason Merrill]

On 10/24/19 7:47 AM, Jakub Jelinek wrote:

On Tue, Oct 22, 2019 at 10:57:42AM -0400, Jason Merrill wrote:

So, do you prefer to do it the other way during build_cxx_call?


It seems more straightforward.


I've tried this approach, but am running into issues:
1) the normal constructors aren't an issue, all I was missing is passing
the object argument down to cxx_constant_value.  The only problem
(I'm aware of) is then the case where build_over_call is called on
a constructor on is_dummy_object, because that obviously isn't
usable in constant expression as something to store into.  This is
used e.g. in the value initialization in consteval8.C test, but probably
for vec init too and other cases.  I've tried to create a temporary in
that case, but consteval8.C still ICEs
/usr/src/gcc/gcc/testsuite/g++.dg/cpp2a/consteval8.C: In function 'int foo()':
/usr/src/gcc/gcc/testsuite/g++.dg/cpp2a/consteval8.C:13:13: internal compiler 
error: tree check: expected aggr_init_expr, have target_expr in build
_value_init, at cp/init.c:372
0x190aa02 tree_check_failed(tree_node const*, char const*, int, char const*, 
...)
 ../../gcc/tree.c:9925
0x8c37d8 tree_check(tree_node*, char const*, int, char const*, tree_code)
 ../../gcc/tree.h:3267
0xa254b5 build_value_init(tree_node*, int)
 ../../gcc/cp/init.c:372


build_cplus_new does the magic of replacing a dummy argument, e.g.


  if (obj_arg && is_dummy_object (obj_arg))
{
  call = build_cplus_new (DECL_CONTEXT (fndecl), call, complain);
  obj_arg = NULL_TREE;
}


And then if the return value from build_special_member_call is 
TREE_CONSTANT, we can return that directly from build_value_init rather 
than look for an AGGR_INIT_EXPR.



2) all other testcases in the testsuite pass, but I'm worried about
default arguments in consteval lambdas.
consteval int bar () { return 42; }
consteval int baz () { return 1; }
typedef int (*fnptr) ();
consteval fnptr quux () { return bar; }

void
foo ()
{
   auto qux = [] (fnptr a = quux ()) consteval { return a (); };
   constexpr auto c = qux (baz);
   constexpr auto d = qux (bar);
   constexpr auto e = qux ();
   static_assert (c == 1);
   static_assert (d == 42);
   static_assert (e == 42);
}
   I believe qux (baz) and qux (bar) are invalid and the patch rejects
   it (I think innermost non-block scope for the baz in qux (baz) is
   not a function parameter scope of an immediate function and so taking
   the address there is invalid.  But isn't the qux () call ok?
   I mean it is similar to the non-lambda calls in the example in the
   standard.  Unfortunately, when parsing the default arguments of a
   lambda, we haven't seen the consteval keyword yet.  I think we could
   tentatively set the consteval argument scope when parsing any lambda
   and if it is not consteval, call a cxx_eval_consteval like function
   to evaluate it at that point.  Thoughts on that?


Hmm, it shouldn't be hard to scan ahead to see if there's a consteval 
after the parameter list.


On the other hand, your suggestion also sounds reasonable, and if we do 
that for lambdas, we could do it for all functions instead of messing 
with the binding level.


Up to you.


3) compared to the May version of the patch, I also found that
build_over_call has a completely separate path if
processing_template_decl and does something much simpler in that
case, but I believe we still need to evaluate consteval calls
even if processing_template_decl if they aren't dependent.


I think such calls fall under the rule that if no valid instantiation is 
possible, the function is ill-formed, no diagnostic required.


So it's desirable, but not necessary.


2019-10-24  Jakub Jelinek  

PR c++/88335 - Implement P1073R3: Immediate functions
c-family/
* c-common.h (enum rid): Add RID_CONSTEVAL.
* c-common.c (c_common_reswords): Add consteval.
cp/
* cp-tree.h (struct lang_decl_fn): Add immediate_fn_p bit.
(DECL_IMMEDIATE_FUNCTION_P, SET_DECL_IMMEDIATE_FUNCTION_P): Define.
(enum cp_decl_spec): Add ds_consteval.
(build_local_temp): Declare.
(fold_non_dependent_expr): Add another tree argument defaulted to
NULL_TREE.
* name-lookup.h (struct cp_binding_level): Add immediate_fn_ctx_p
member.
* tree.c (build_local_temp): Remove forward declaration, no longer
static.
* parser.c (cp_keyword_starts_decl_specifier_p): Adjust comments
for C++11 and C++20 specifiers.  Handle RID_CONSTEVAL.
(CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR): Adjust comment.
(CP_PARSER_FLAGS_CONSTEVAL): New.
(cp_parser_lambda_declarator_opt): Handle ds_consteval.
(cp_parser_decl_specifier_seq): Handle RID_CONSTEVAL.
(cp_parser_explicit_instantiation): Diagnose explicit instantiation
with consteval specifier.

Re: [Darwin, testsuite, committed] Fix Wnonnull on Darwin.

[Iain Sandoe]

Iain Sandoe  wrote:

> Iain Sandoe  wrote:
> 
>> Martin Sebor  wrote:
>> 
>>> On 10/20/2019 07:27 AM, Iain Sandoe wrote:
 Martin Sebor  wrote:
> On 10/19/19 2:56 AM, Iain Sandoe wrote:
>> Andreas Schwab  wrote:
>>> On Okt 19 2019, Iain Sandoe  wrote:
>>> 
 This test has failed always on Darwin, because Darwin does not mark
 entries in string.h with nonnull attributes.  Since the purpose of the 
 test
 is to check that the warnings are issued for an inlined function, not 
 that
 the target headers are marked up, we can provide locally marked up
 function declarations for Darwin.
>>> 
>>> If the test depends on the non-std declarations, then it should use them
>>> everywhere.
>> from my perspective, agreed, Martin?
> 
> I don't see a problem with it.  I prefer tests that don't depend
> on system headers to avoid these kinds of issues
 We can do that anyway then, - I can just adjust the current code tor 
 remove the
 special-casing, and to use __SIZE_TYPE__ instead of size_t everywhere, OK?
>>> 
>>> Sure.
>> 
>> Will try to get to it later today.
> 
> This is what I committed (will sort out backports, in due course).

backported to gcc-9 as r277647 (still needed on gcc-8).
Iain

Re: [C++ PATCH] P0784R7 constexpr new fixes (PR c++/91369)

[Jason Merrill]

On 10/24/19 5:11 AM, Jakub Jelinek wrote:

Hi!

Jonathan has showed me a testcase with std::allocator::{,de}allocate
and std::construct_at which FAILs with the current constexpr new
implementation.

There are two problems that make the testcase rejected, and further issues
(not addressed by this patch) where supposedly invalid C++20 code is
accepted.

The first problem was that cxx_replaceable_global_alloc_fn was actually
treating placement new as replaceable global allocation function, when it is
not.
The second problem is that std::construct_at uses placement new under the
hood.  From what I understood, placement new is not allowed in C++20 in
constexpr context and it is unspecified whatever magic std::construct_at
uses to get similar effect.

The fix for the first problem is easy, just add the
DECL_IS_REPLACEABLE_OPERATOR_NEW_P || DECL_IS_OPERATOR_DELETE_P
checks to cxx_replaceable_global_alloc_fn, placement new nor some random
user added ::operator new (size_t, float, double) etc. should not have that
set.
For the second one, the patch is allowing placement new in constexpr
evaluation only in std::construct_at and not in other functions.

With this, Jonathan's testcase works fine.  Ok for trunk if it passes
bootstrap/regtest?


OK.


Now, for the accepts invalid issues.
 From what I understood, in constant evaluation
::operator new{,[]} or ::operator delete{,[]}
can't be called from anywhere, only from new/delete expressions or
std::allocator::{,de}allocate, is that correct?
If so, perhaps we need some CALL_EXPR flag that we'd set on the call
coming from new/delete expressions and disallow calls to
replaceable global allocator functions in constexpr evaluation unless
that flag is set or it is in std::allocator::{,de}allocate.


Looks like there used to be a TREE_CALLS_NEW flag in TREE_LANG_FLAG_1, 
but that flag is now free for CALL_EXPR.


Or we could check whether the call is wrapped as expected by 
maybe_wrap_new_for_constexpr, but that doesn't help delete.


Or we could build NEW_EXPR and DELETE_EXPR in non-dependent context and 
lower them later.



Another thing is that even with that change,
   std::allocator a;
   auto p = a.allocate (1);
   *p = 1;
   a.deallocate (p, 1);
would be accepted during constexpr evaluation, because allocate already
has the cast which turns "heap uninit" variable into "heap " and assigns
it a type, so there is nothing that will prevent the store from succeeding.


What's wrong with the store?


Any thoughts on what to do with that?  Even if the magic cast (perhaps
with some flag on it) is moved from whatever the first cast to non-void*
is to the placement new or start of corresponding constructor, would we
need to unmark it somehow if we say std::destroy_at it but allow
next std::construct_at to construct it again?

2019-10-24  Jakub Jelinek  

PR c++/91369 - Implement P0784R7: constexpr new
* constexpr.c (cxx_replaceable_global_alloc_fn): Don't return true
for placement new.
(cxx_placement_new_fn, is_std_construct_at): New functions.
(cxx_eval_call_expression): Allow placement new in std::construct_at.
(potential_constant_expression_1): Likewise.

* g++.dg/cpp2a/constexpr-new5.C: New test.

--- gcc/cp/constexpr.c.jj   2019-10-23 20:37:59.981872274 +0200
+++ gcc/cp/constexpr.c  2019-10-24 10:20:57.127193138 +0200
@@ -1601,7 +1601,41 @@ cxx_replaceable_global_alloc_fn (tree fn
  {
return (cxx_dialect >= cxx2a
  && IDENTIFIER_NEWDEL_OP_P (DECL_NAME (fndecl))
- && CP_DECL_CONTEXT (fndecl) == global_namespace);
+ && CP_DECL_CONTEXT (fndecl) == global_namespace
+ && (DECL_IS_REPLACEABLE_OPERATOR_NEW_P (fndecl)
+ || DECL_IS_OPERATOR_DELETE_P (fndecl)));
+}
+
+/* Return true if FNDECL is a placement new function that should be
+   useable during constant expression evaluation of std::construct_at.  */
+
+static inline bool
+cxx_placement_new_fn (tree fndecl)
+{
+  if (cxx_dialect >= cxx2a
+  && IDENTIFIER_NEW_OP_P (DECL_NAME (fndecl))
+  && CP_DECL_CONTEXT (fndecl) == global_namespace
+  && !DECL_IS_REPLACEABLE_OPERATOR_NEW_P (fndecl)
+  && TREE_CODE (TREE_TYPE (fndecl)) == FUNCTION_TYPE)
+{
+  tree first_arg = TREE_CHAIN (TYPE_ARG_TYPES (TREE_TYPE (fndecl)));
+  if (TREE_VALUE (first_arg) == ptr_type_node
+ && TREE_CHAIN (first_arg) == void_list_node)
+   return true;
+}
+  return false;
+}
+
+/* Return true if FNDECL is std::construct_at.  */
+
+static inline bool
+is_std_construct_at (tree fndecl)
+{
+  if (!decl_in_std_namespace_p (fndecl))
+return false;
+
+  tree name = DECL_NAME (fndecl);
+  return name && id_equal (name, "construct_at");
  }
  
  /* Subroutine of cxx_eval_constant_expression.

@@ -1738,6 +1772,27 @@ cxx_eval_call_expression (const constexp
  return t;
}
}
+  /* Allow placement new in std::construct_at, just return the second
+argument.  */
+  if 

Re: [PATCH] Fix PR c++/92024

[Jason Merrill]

On 10/12/19 2:10 PM, Bernd Edlinger wrote:

On 10/11/19 6:31 PM, Jason Merrill wrote:

On 10/10/19 2:06 PM, Bernd Edlinger wrote:

On 10/10/19 7:49 PM, Jason Merrill wrote:

On 10/10/19 10:42 AM, Bernd Edlinger wrote:

Hi,

this fixes a crash when -Wshadow=compatible-local is
enabled in the testcase g++.dg/parse/crash68.C


Why does that flag cause this crash?



gcc/cp/name-lookup.c:

    if (warn_shadow)
  warning_code = OPT_Wshadow;
    else if (warn_shadow_local)
  warning_code = OPT_Wshadow_local;
    else if (warn_shadow_compatible_local
     && (same_type_p (TREE_TYPE (old), TREE_TYPE (decl))
     || (!dependent_type_p (TREE_TYPE (decl))
     && !dependent_type_p (TREE_TYPE (old))
     /* If the new decl uses auto, we don't yet know
    its type (the old type cannot be using auto
    at this point, without also being
    dependent).  This is an indication we're
    (now) doing the shadow checking too
    early.  */
     && !type_uses_auto (TREE_TYPE (decl))
     && can_convert (TREE_TYPE (old), TREE_TYPE (decl),
     tf_none
  warning_code = OPT_Wshadow_compatible_local;

if -Wshadow=compatible-local is used, the can_convert function crashes
in instantiate_class_template_1.


Right, checking can_convert is problematic here, as it can cause template 
instantiations that change the semantics of the program.  Or, in this case, 
crash.



So I try to make C++ behave more consistently with the code in c-decl.c,
thus dependent on warn_shadow but not on warn_shadow_local and/or
warn_shadow_compatible_local:

if (warn_shadow)
   warning_code = OPT_Wshadow;
 else if (comptypes (TREE_TYPE (old_decl), TREE_TYPE (new_decl)))
   warning_code = OPT_Wshadow_compatible_local;
 else
   warning_code = OPT_Wshadow_local;
 warned = warning_at (DECL_SOURCE_LOCATION (new_decl), warning_code,
  "declaration of %qD shadows a parameter",
  new_decl);

I cannot remove the if (warn_shadow) since this breaks gcc.dg/pr48062.c
which uses:

#pragma GCC diagnostic ignored "-Wshadow"

to disable a -Wshadow=compatible-local warning, but while -Wno-shadow on the
command line disables also dependent warnings the pragma does not (always) do 
that.

So instead I'd like to adjust the doc of -Wshadow to reflect the implementation
and remove the if(warn_shadow_local) to have C and C++ behave identical and
hopefully now in sync with the doc.


Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


OK, thanks.

Jason

Re: [PATCH 2/2][vect]Make vect-epilogues-nomask=1 default

[Richard Biener]

On October 30, 2019 7:16:43 PM GMT+01:00, "Andre Vieira (lists)" 
 wrote:
>Hi,
>
>In this patch I turn epilogue vectorization on by default for all 
>targets. After some discussions I decided to take the following testing
>
>approach:
>
>1) I have disabled epilogue vectorization for all tests that failed due
>
>to scan-tree-dump failures inside:
>   - gcc.dg/vect
>   - gcc.target/i386
>   - gcc.target/aarch6
>   - gfortran.dg/vect
>
>2) Added the Bugzilla's reported testcase but xfail the scan for 
>EPILOGUE VECTORIZED for arm*-*-* as that target does not vectorize the 
>epilogue.
>
>
>I have only been able to test this for aarch64, arm and x86_64 (with 
>avx512).  Other targets may also want to disable epilogue vectorization
>
>like I did for gcc.target and gcc.dg/vect tests or xfail the 
>gcc.dg/vect/vect-epilogues.c test. I am relying on others to test this 
>for their available targets, thanks in advance!
>
>Is this OK for trunk?

Ok. 

Thanks, 
Richard. 

>
>Cheers,
>Andre
>
>gcc/ChangeLog:
>
>2019-10-30  Andre Vieira  
>
> * params.def (PARAM_VECT_EPILOGUES_NOMASK): Enable by default.
>
>gcc/testsuite/ChangeLog:
>
>2019-10-30  Andre Vieira  
>
> * gcc.dg/vect/vect-epilogues.c: New test.
> * gcc.dg/vect/fast-math-vect-call-1.c: Disable for epilogue
> vectorization.
> * gcc.dg/vect/no-fast-math-vect16.c: Likewise.
> * gcc.dg/vect/no-scevccp-noreassoc-slp-reduc-7.c: Likewise.
> * gcc.dg/vect/no-scevccp-vect-iv-3.c: Likewise.
> * gcc.dg/vect/no-section-anchors-vect-31.c: Likewise.
> * gcc.dg/vect/no-section-anchors-vect-64.c: Likewise.
> * gcc.dg/vect/no-section-anchors-vect-66.c: Likewise.
> * gcc.dg/vect/no-section-anchors-vect-68.c: Likewise.
> * gcc.dg/vect/no-vfa-vect-dv-2.c: Likewise.
> * gcc.dg/vect/pr33804.c: Likewise.
> * gcc.dg/vect/pr53773.c: Likewise.
> * gcc.dg/vect/pr65947-1.c: Likewise.
> * gcc.dg/vect/pr65947-13.c: Likewise.
> * gcc.dg/vect/pr65947-14.c: Likewise.
> * gcc.dg/vect/pr65947-4.c: Likewise.
> * gcc.dg/vect/pr80631-1.c: Likewise.
> * gcc.dg/vect/pr80631-2.c: Likewise.
> * gcc.dg/vect/slp-23.c: Likewise.
> * gcc.dg/vect/slp-25.c: Likewise.
> * gcc.dg/vect/slp-reduc-2.c: Likewise.
> * gcc.dg/vect/slp-reduc-5.c: Likewise.
> * gcc.dg/vect/slp-reduc-6.c: Likewise.
> * gcc.dg/vect/slp-reduc-sad-2.c: Likewise.
> * gcc.dg/vect/slp-widen-mult-half.c: Likewise.
> * gcc.dg/vect/trapv-vect-reduc-4.c: Likewise.
> * gcc.dg/vect/vect-103.c: Likewise.
> * gcc.dg/vect/vect-109.c: Likewise.
> * gcc.dg/vect/vect-119.c: Likewise.
> * gcc.dg/vect/vect-24.c: Likewise.
> * gcc.dg/vect/vect-26.c: Likewise.
> * gcc.dg/vect/vect-27.c: Likewise.
> * gcc.dg/vect/vect-29.c: Likewise.
> * gcc.dg/vect/vect-42.c: Likewise.
> * gcc.dg/vect/vect-44.c: Likewise.
> * gcc.dg/vect/vect-48.c: Likewise.
> * gcc.dg/vect/vect-50.c: Likewise.
> * gcc.dg/vect/vect-52.c: Likewise.
> * gcc.dg/vect/vect-54.c: Likewise.
> * gcc.dg/vect/vect-56.c: Likewise.
> * gcc.dg/vect/vect-58.c: Likewise.
> * gcc.dg/vect/vect-60.c: Likewise.
> * gcc.dg/vect/vect-72.c: Likewise.
> * gcc.dg/vect/vect-75-big-array.c: Likewise.
> * gcc.dg/vect/vect-75.c: Likewise.
> * gcc.dg/vect/vect-77-alignchecks.c: Likewise.
> * gcc.dg/vect/vect-77-global.c: Likewise.
> * gcc.dg/vect/vect-78-alignchecks.c: Likewise.
> * gcc.dg/vect/vect-78-global.c: Likewise.
> * gcc.dg/vect/vect-89-big-array.c: Likewise.
> * gcc.dg/vect/vect-89.c: Likewise.
> * gcc.dg/vect/vect-91.c: Likewise.
> * gcc.dg/vect/vect-92.c: Likewise.
> * gcc.dg/vect/vect-96.c: Likewise.
> * gcc.dg/vect/vect-cond-reduc-3.c: Likewise.
> * gcc.dg/vect/vect-cond-reduc-4.c: Likewise.
> * gcc.dg/vect/vect-live-1.c: Likewise.
> * gcc.dg/vect/vect-live-2.c: Likewise.
> * gcc.dg/vect/vect-live-3.c: Likewise.
> * gcc.dg/vect/vect-live-4.c: Likewise.
> * gcc.dg/vect/vect-live-slp-1.c: Likewise.
> * gcc.dg/vect/vect-live-slp-2.c: Likewise.
> * gcc.dg/vect/vect-live-slp-3.c: Likewise.
> * gcc.dg/vect/vect-multitypes-3.c: Likewise.
> * gcc.dg/vect/vect-multitypes-4.c: Likewise.
> * gcc.dg/vect/vect-multitypes-6.c: Likewise.
> * gcc.dg/vect/vect-peel-1-epilogue.c: Likewise. New test.
> * gcc.dg/vect/vect-peel-1-src.c: Likewise. New test.
> * gcc.dg/vect/vect-peel-1.c: Likewise.
> * gcc.dg/vect/vect-peel-3-epilogue.c: Likewise. New test.
> * gcc.dg/vect/vect-peel-3-src.c: Likewise. New test.
> * gcc.dg/vect/vect-peel-3.c: Likewise.
> * gcc.dg/vect/vect-peel-4-epilogue.c: Likewise. New test.
> * 

Re: [C++ PATCH] Fix up decl_in_std_namespace_p handling of --enable-symvers=gnu-versioned-namespace

[Jason Merrill]

On 10/30/19 9:30 AM, Marek Polacek wrote:

On Wed, Oct 30, 2019 at 10:58:57AM +0100, Jakub Jelinek wrote:

On Fri, Oct 25, 2019 at 10:44:18AM -0400, Marek Polacek wrote:

That is... sneaky.  I guess I/we need to test with
--enable-symvers=gnu-versioned-namespace every now and then.


Indeed.


Probably deserves a comment.


Ok, here it is with a comment, bootstrapped/regtested again last night:


Thanks, looks good!


OK, thanks.  I'm a bit surprised we don't already have something like 
decl_non_inline_namespace_context, but don't need to add it now.


Jason

[PATCH][C++] Use OVL_OP_INFO in get_fold_operator.

[Jason Merrill]

A cleanup I noticed while working on operator<=>.

Tested x86_64-pc-linux-gnu, applying to trunk.
---
 gcc/cp/cxx-pretty-print.c | 48 ++-
 1 file changed, 2 insertions(+), 46 deletions(-)

diff --git a/gcc/cp/cxx-pretty-print.c b/gcc/cp/cxx-pretty-print.c
index 2a129a3bff7..8ece11d276e 100644
--- a/gcc/cp/cxx-pretty-print.c
+++ b/gcc/cp/cxx-pretty-print.c
@@ -2551,52 +2551,8 @@ static char const*
 get_fold_operator (tree t)
 {
   int op = int_cst_value (FOLD_EXPR_OP (t));
-  if (FOLD_EXPR_MODIFY_P (t))
-{
-  switch (op)
-{
-case NOP_EXPR: return "=";
-case PLUS_EXPR: return "+=";
-case MINUS_EXPR: return "-=";
-case MULT_EXPR: return "*=";
-case TRUNC_DIV_EXPR: return "/=";
-case TRUNC_MOD_EXPR: return "%=";
-case BIT_XOR_EXPR: return "^=";
-case BIT_AND_EXPR: return "&=";
-case BIT_IOR_EXPR: return "|=";
-case LSHIFT_EXPR: return "<<=";
-case RSHIFT_EXPR: return ">>=";
-default: gcc_unreachable ();
-}
-}
-  else
-{
-  switch (op)
-{
-case PLUS_EXPR: return "+";
-case MINUS_EXPR: return "-";
-case MULT_EXPR: return "*";
-case TRUNC_DIV_EXPR: return "/";
-case TRUNC_MOD_EXPR: return "%";
-case BIT_XOR_EXPR: return "^";
-case BIT_AND_EXPR: return "&";
-case BIT_IOR_EXPR: return "|";
-case LSHIFT_EXPR: return "<<";
-case RSHIFT_EXPR: return ">>";
-case EQ_EXPR: return "==";
-case NE_EXPR: return "!=";
-case LT_EXPR: return "<";
-case GT_EXPR: return ">";
-case LE_EXPR: return "<=";
-case GE_EXPR: return ">=";
-case TRUTH_ANDIF_EXPR: return "&&";
-case TRUTH_ORIF_EXPR: return "||";
-case MEMBER_REF: return "->*";
-case DOTSTAR_EXPR: return ".*";
-case OFFSET_REF: return ".*";
-default: return ","; /* FIXME: Not the right default.  */
-}
-}
+  ovl_op_info_t *info = OVL_OP_INFO (FOLD_EXPR_MODIFY_P (t), op);
+  return info->name;
 }
 
 void

base-commit: 88ee7dfcc7ea7aef3194e66beb571558382e9ddc
-- 
2.18.1

Re: Deprecating cc0 (and consequently cc0 targets)

[Paul Koning]

> On Oct 30, 2019, at 2:24 PM, Jeff Law  wrote:
> 
> On 10/30/19 2:12 AM, Richard Biener wrote:
>> On Tue, Oct 29, 2019 at 8:34 PM Jeff Law  wrote:
> 
>> 
>> I think the wiki has better examples.  That said, I wonder how much can
>> be automated here, for example when just considering CCmode (m68k has
>> setcc IIRC) then for example all define_insns like
>> 
>> (define_insn "*cmpsi_cf"
>>  [(set (cc0)
>>(compare (match_operand:SI 0 "nonimmediate_operand" "mrKs,r")
>> (match_operand:SI 1 "general_operand" "r,mrKs")))]
>>  "TARGET_COLDFIRE"
>> {...}
> The compare/test insns are relatively straightforward as they're a
> fairly simple substitution.  It's all the other insns that implicitly
> set cc0 that are so painful.
> 
> 
>> 
>> The various expanders need to be handled manually I guess, though somehow
>> automagically generating the define_insn_and_split might be possible
>> as well, no?
> I was looking at define_subst to help here, but it didn't look like it
> was really going to simplify things in any significant way.  Maybe
> someone with more experience with define_subst would see something that
> would dramtically simplify this process.

I've found define_subst to be quite helpful for generating the pairs of 
"clobbers CC" and "sets CC in a useful way from the operation result" pairs 
that Richard was referring to.  pdp11.md shows what I learned; it may be 
applicable elsewhere too.  This is for a "case 2" target, one that modifies CC 
on most operations (essentially all those that do arithmetic).

paul

Re: [Patch][Fortran] PR 92208 don't use function-result dummy variable as actual argument

[Thomas Koenig]

Hi Tobias,


OK for the trunk and GCC 9?


As far as I can see, this looks good.

So, OK for trunk. If it later turns out that there are problems
caused by this, I suspect we will hear about them soon enough :-)

Thanks for taking this on!

Regards

Thomas

Re: [PATCH 3/3][rs6000] vector conversion RTL pattern update for diff unit size

[Segher Boessenkool]

Hi!

On Wed, Oct 23, 2019 at 05:42:45PM +0800, Kewen.Lin wrote:
> Following the previous one 2/3, this patch is to update the
> vector conversions between fixed point and floating point
> with different element unit sizes, such as: SP <-> DI, DP <-> SI.

>   (vsx_xvcvdp[su]xws): New define_expand, old one split to...

You mean  here, please fix (never use wildcards like [su] in changelogs:
people grep for things in changelogs, which misses entries with wildcards).

> +/* Half VMX/VSX vector (for select)  */
> +VECTOR_MODE (FLOAT, SF, 2);   /* V2SF  */
> +VECTOR_MODE (INT, SI, 2); /* V2SI  */

Or "for internal use", in general.  What happens if a user tries to create
something of such a mode?  I hope we don't ICE :-/

> -(define_insn "vsx_xvcvspdp"
> +(define_insn "vsx_xvcvspdp_be"
>[(set (match_operand:V2DF 0 "vsx_register_operand" "=v,?wa")
> - (unspec:V2DF [(match_operand:V4SF 1 "vsx_register_operand" "wa,wa")]
> -   UNSPEC_VSX_CVSPDP))]
> -  "VECTOR_UNIT_VSX_P (V4SFmode)"
> + (float_extend:V2DF
> +   (vec_select:V2SF (match_operand:V4SF 1 "vsx_register_operand" "wa,wa")
> +  (parallel [(const_int 0) (const_int 2)]]
> +  "VECTOR_UNIT_VSX_P (V4SFmode) && BYTES_BIG_ENDIAN"
> +  "xvcvspdp %x0,%x1"
> +  [(set_attr "type" "vecdouble")])
> +
> +(define_insn "vsx_xvcvspdp_le"
> +  [(set (match_operand:V2DF 0 "vsx_register_operand" "=v,?wa")
> + (float_extend:V2DF
> +   (vec_select:V2SF (match_operand:V4SF 1 "vsx_register_operand" "wa,wa")
> +  (parallel [(const_int 1) (const_int 3)]]
> +  "VECTOR_UNIT_VSX_P (V4SFmode) && !BYTES_BIG_ENDIAN"
>"xvcvspdp %x0,%x1"
>[(set_attr "type" "vecdouble")])

It would be nice if there was a nicer way to describe this, but this is
the best I can see to do as well.

> +;; Convert vector of 64-bit floating point numbers to vector of
> +;; 32-bit signed/unsigned integers.
> +(define_insn "vsx_xvcvdpxws_be"
>[(set (match_operand:V4SI 0 "vsx_register_operand" "=v,?wa")
> - (unspec:V4SI [(match_operand:V2DF 1 "vsx_register_operand" "wa,wa")]
> -  UNSPEC_VSX_CVDPSXWS))]
> -  "VECTOR_UNIT_VSX_P (V2DFmode)"
> -  "xvcvdpsxws %x0,%x1"
> + (any_fix:V4SI
> +   (vec_concat:V4DF (match_operand:V2DF 1 "vsx_register_operand" "wa,wa")
> +  (vec_select:V2DF (match_dup 1)
> +(parallel [(const_int 1) (const_int 0)])]
> +  "VECTOR_UNIT_VSX_P (V2DFmode) && BYTES_BIG_ENDIAN"
> +  "xvcvdpxws %x0,%x1"
>[(set_attr "type" "vecdouble")])

This doesn't work, I think: the insns actually leaves words 1 and 3
undefined, but this pattern gives them a meaning.

I don't think we can do better than unspecs for such insns.  Or change
the pattern to only describe the defined parts (this works for e.g. mulhw
that describes its result as SImode: its DImode result has the high half
undefined).

> +;; Convert vector of 32-bit signed/unsigned integers to vector of
> +;; 64-bit floating point numbers.
> +(define_insn "vsx_xvcvxwdp_be"
> +  [(set (match_operand:V2DF 0 "vsx_register_operand" "=wa")
> + (any_float:V2DF
> +   (vec_select:V2SI (match_operand:V4SI 1 "vsx_register_operand" "wa")
> +  (parallel [(const_int 0) (const_int 2)]]
> +  "VECTOR_UNIT_VSX_P (V2DFmode) && BYTES_BIG_ENDIAN"
> +  "xvcvxwdp %x0,%x1"
>[(set_attr "type" "vecdouble")])

This one for example is fine: words 1 and 3 of the input are unused, but
that is just fine.


Segher

Re: C++ PATCH for c++/92134 - constinit malfunction in static data member

[Jason Merrill]

On 10/24/19 6:30 PM, Marek Polacek wrote:

I wasn't properly setting LOOKUP_CONSTINIT in grokfield and so we didn't
detect a non-const initializer.

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


OK.


2019-10-24  Marek Polacek  

PR c++/92134 - constinit malfunction in static data member.
* decl2.c (grokfield): Set LOOKUP_CONSTINIT.

* g++.dg/cpp2a/constinit14.C: New test.

diff --git gcc/cp/decl2.c gcc/cp/decl2.c
index 6d5e973b487..a630ee31397 100644
--- gcc/cp/decl2.c
+++ gcc/cp/decl2.c
@@ -990,6 +990,9 @@ grokfield (const cp_declarator *declarator,
else
  flags = LOOKUP_IMPLICIT;
  
+  if (decl_spec_seq_has_spec_p (declspecs, ds_constinit))

+flags |= LOOKUP_CONSTINIT;
+
switch (TREE_CODE (value))
  {
  case VAR_DECL:
diff --git gcc/testsuite/g++.dg/cpp2a/constinit14.C 
gcc/testsuite/g++.dg/cpp2a/constinit14.C
new file mode 100644
index 000..72bfab667b8
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp2a/constinit14.C
@@ -0,0 +1,13 @@
+// PR c++/92134 - constinit malfunction in static data member.
+// { dg-do compile { target c++2a } }
+
+struct Value {
+  Value() : v{new int{42}} {}
+  int* v;
+};
+
+struct S {
+  static constinit inline Value v{}; // { dg-error "variable .S::v. does not have a 
constant initializer|call to non-.constexpr. function" }
+};
+
+int main() { return *S::v.v; }

Re: C++ PATCH for c++/92215 - flawed diagnostic for bit-field with non-integral type

[Jason Merrill]

On 10/24/19 5:50 PM, Marek Polacek wrote:

I noticed that for code like

   struct S {
 int *foo : 3;
   };

we generate nonsensical

   r.C:2:8: error: function definition does not declare parameters
   2 |   int *foo : 3;

It talks about a function because after parsing the declspecs of 'foo' we don't
see either ':' or "name :", so we think it's not a bit-field decl.  So we parse
the declarator and since a ctor-initializer begins with a ':', we try to parse
it as a function body, generating the awful diagnostic.  With this patch, we
issue:

   r.C:2:8: error: bit-field ‘foo’ has non-integral type ‘int*’
   2 |   int *foo : 3;

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

2019-10-24  Marek Polacek  

PR c++/92215 - flawed diagnostic for bit-field with non-integral type.
* parser.c (cp_parser_member_declaration): Add a diagnostic for
bit-fields with non-integral types.

* g++.dg/diagnostic/bitfld4.C: New test.

diff --git gcc/cp/parser.c gcc/cp/parser.c
index 3857fe47d67..84d2121cae2 100644
--- gcc/cp/parser.c
+++ gcc/cp/parser.c
@@ -24971,6 +24971,29 @@ cp_parser_member_declaration (cp_parser* parser)
  else
initializer = cp_parser_initializer (parser, , );
}
+ /* Detect invalid bit-field cases such as
+
+  int *p : 4;
+  int & : 3;
+
+and similar.  */
+ else if (cp_lexer_next_token_is (parser->lexer, CPP_COLON)
+  && decl_specifiers.any_type_specifiers_p)


Please add a comment that you're checking any_type_specifiers_p to 
exclude constructors.  OK with that change.



+   {
+ /* This is called for a decent diagnostic only.  */
+ tree d = grokdeclarator (declarator, _specifiers,
+  BITFIELD, /*initialized=*/false,
+  );
+ error_at (DECL_SOURCE_LOCATION (d),
+   "bit-field %qD has non-integral type %qT",
+   d, TREE_TYPE (d));
+ cp_parser_skip_to_end_of_statement (parser);
+ /* Avoid "extra ;" pedwarns.  */
+ if (cp_lexer_next_token_is (parser->lexer,
+ CPP_SEMICOLON))
+   cp_lexer_consume_token (parser->lexer);
+ goto out;
+   }
  /* Otherwise, there is no initializer.  */
  else
initializer = NULL_TREE;
diff --git gcc/testsuite/g++.dg/diagnostic/bitfld4.C 
gcc/testsuite/g++.dg/diagnostic/bitfld4.C
new file mode 100644
index 000..d6aa9a5513c
--- /dev/null
+++ gcc/testsuite/g++.dg/diagnostic/bitfld4.C
@@ -0,0 +1,16 @@
+// PR c++/92215 - flawed diagnostic for bit-field with non-integral type.
+// { dg-do compile { target c++11 } }
+
+struct S {
+  int *f1 : 3; // { dg-error "bit-field .f1. has non-integral type .int\\*." }
+  int  : 3; // { dg-error "bit-field .f2. has non-integral type .int&." }
+  int & : 3; // { dg-error "bit-field .f3. has non-integral type .int&&." }
+  int f4[1] : 3; // { dg-error "bit-field .f4. has non-integral type .int 
\\\[1\\\]." }
+  int *f5 __attribute__((deprecated)) : 3; // { dg-error "bit-field .f5. has 
non-integral type .int\\*." }
+  int f6[1] __attribute__((deprecated)) : 3; // { dg-error "bit-field .f6. has 
non-integral type .int \\\[1\\\]." }
+  int  __attribute__((deprecated)): 3; // { dg-error "bit-field .f7. has 
non-integral type .int&." }
+  int : 3; // { dg-error "expected" }
+  int *f9[1] : 3; // { dg-error "bit-field .f9. has non-integral type .int\\* 
\\\[1\\\]." }
+  int (*f10)() : 3; // { dg-error "bit-field .f10. has non-integral type .int 
\\(\\*\\)\\(\\)." }
+  int [][2] : 3; // { dg-error "expected" }
+};

Re: C++ PATCH for c++/91962 - ICE with reference binding and qualification conversion

[Jason Merrill]

On 10/24/19 3:24 PM, Marek Polacek wrote:

When fixing c++/91889 (r276251) I was assuming that we couldn't have a ck_qual
under a ck_ref_bind, and I was introducing it in the patch and so this
+   if (next_conversion (convs)->kind == ck_qual)
+ {
+   gcc_assert (same_type_p (TREE_TYPE (expr),
+next_conversion (convs)->type));
+   /* Strip the cast created by the ck_qual; cp_build_addr_expr
+  below expects an lvalue.  */
+   STRIP_NOPS (expr);
+ }
in convert_like_real was supposed to handle it.  But that assumption was wrong
as this test shows; here we have "(int *)f" where f is of type long int, and
we're converting it to "const int *const &", so we have both ck_ref_bind and
ck_qual.  That means that the new STRIP_NOPS strips an expression it shouldn't
have, and that then breaks when creating a TARGET_EXPR.


Are there two conversions in that case, such that stripping only one 
would also fix the problem?



So we want to limit
the stripping to the new case only.  This I do by checking need_temporary_p,
which will be 0 in the new case.  Yes, we can set need_temporary_p when
binding a reference directly, but then we won't have a qualification
conversion.  It is possible to have a bit-field, convert it to a pointer,
and then convert that pointer to a more-qualified pointer, but in that case
we're not dealing with an lvalue, so gl_kind is 0, so we won't enter this
block in reference_binding:
  1747   if ((related_p || compatible_p) && gl_kind)

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


OK.


2019-10-24  Marek Polacek  

PR c++/91962 - ICE with reference binding and qualification conversion.
* call.c (convert_like_real) : Check need_temporary_p.

* g++.dg/cpp0x/ref-bind7.C: New test.

diff --git gcc/cp/call.c gcc/cp/call.c
index 55d2abaaddd..d4674a77078 100644
--- gcc/cp/call.c
+++ gcc/cp/call.c
@@ -7386,7 +7386,8 @@ convert_like_real (conversion *convs, tree expr, tree fn, 
int argnum,
/* direct_reference_binding might have inserted a ck_qual under
   this ck_ref_bind for the benefit of conversion sequence ranking.
   Ignore the conversion; we'll create our own below.  */
-   if (next_conversion (convs)->kind == ck_qual)
+   if (next_conversion (convs)->kind == ck_qual
+   && !convs->need_temporary_p)
  {
gcc_assert (same_type_p (TREE_TYPE (expr),
 next_conversion (convs)->type));
diff --git gcc/testsuite/g++.dg/cpp0x/ref-bind7.C 
gcc/testsuite/g++.dg/cpp0x/ref-bind7.C
new file mode 100644
index 000..e3675bc560d
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/ref-bind7.C
@@ -0,0 +1,13 @@
+// PR c++/91962 - ICE with reference binding and qualification conversion.
+// { dg-do compile { target c++11 } }
+
+template  class b {
+public:
+  void c(const a &);
+};
+class B {
+  void d();
+  b e;
+};
+long f;
+void B::d() { e.c((const int *)f); }

Re: Deprecating cc0 (and consequently cc0 targets)

[Jeff Law]

On 10/30/19 2:12 AM, Richard Biener wrote:
> On Tue, Oct 29, 2019 at 8:34 PM Jeff Law  wrote:

> 
> I think the wiki has better examples.  That said, I wonder how much can
> be automated here, for example when just considering CCmode (m68k has
> setcc IIRC) then for example all define_insns like
> 
> (define_insn "*cmpsi_cf"
>   [(set (cc0)
> (compare (match_operand:SI 0 "nonimmediate_operand" "mrKs,r")
>  (match_operand:SI 1 "general_operand" "r,mrKs")))]
>   "TARGET_COLDFIRE"
> {...}
The compare/test insns are relatively straightforward as they're a
fairly simple substitution.  It's all the other insns that implicitly
set cc0 that are so painful.


> 
> The various expanders need to be handled manually I guess, though somehow
> automagically generating the define_insn_and_split might be possible
> as well, no?
I was looking at define_subst to help here, but it didn't look like it
was really going to simplify things in any significant way.  Maybe
someone with more experience with define_subst would see something that
would dramtically simplify this process.

> 
> That is, the work is quite repetitive (looking at m68k.md) and for this reason
> error-prone.  But a case#2 conversion could essentially be automated?
It's a case2.  ANd yes, it's highly repetitive.  That's the part of the
v850 conversion my son did.  Once he had all the mechanical stuff in
place I started banging it into shape.

> 
> That is, do you see it as impossible to implement a case#2 cc0 "conversion"
> in gen*?
I'd be surprised if it could be that simple.

> 
>> Some have claimed this might be easier with define_subst, but I wandered
>> it a bit and it didn't seem necessarily simpler in the end.  But maybe
>> someone with more experience with define_subst could make the transition
>> simpler.
>>
>> Anyway, that has to be done for each and every pattern that modifies the
>> condition codes.  It's a ton of mechanical work.  The m68k has the
>> additional complication that condition codes can be either in cc0 or
>> fcc0.  So that has to be taken into account as well.
> 
> I don't see that it does this currently though (cc0 doesn't allow this).
You're right.  fcmp seems to just set the normal condition codes, just
using a different insn.  So the FP unit shouldn't be a major problem.

> 
>> You also have to rewrite the tst/cmp and conditional branching patterns
>> to use the new style.
>>
>> Sadly until you do *all* of that you don't have anything you can really
>> test.  And even once you've done all that, the code quality is going to
>> suffer because we're not doing any compare/tst elimination.  To enable
>> that you have to go back and start adding patterns like this:
> 
> For m68k what helps is that you could concentrate on TARGET_COLDFIRE
> (or !TARGET_COLDFIRE).  Maybe even split m68k.md along this.
> 
> A lot of the define_insns also look like optimization (so could be disabled
> for the moment).
That's certainly how I'd approach it.  Disable everything not strictly
needed, convert what has to be converted, then bring back the patterns
incrementally.

Jeff

Re: [PATCH 2/2][vect]Make vect-epilogues-nomask=1 default

[Andre Vieira (lists)]

Hi,

In this patch I turn epilogue vectorization on by default for all 
targets. After some discussions I decided to take the following testing 
approach:


1) I have disabled epilogue vectorization for all tests that failed due 
to scan-tree-dump failures inside:

  - gcc.dg/vect
  - gcc.target/i386
  - gcc.target/aarch6
  - gfortran.dg/vect

2) Added the Bugzilla's reported testcase but xfail the scan for 
EPILOGUE VECTORIZED for arm*-*-* as that target does not vectorize the 
epilogue.



I have only been able to test this for aarch64, arm and x86_64 (with 
avx512).  Other targets may also want to disable epilogue vectorization 
like I did for gcc.target and gcc.dg/vect tests or xfail the 
gcc.dg/vect/vect-epilogues.c test. I am relying on others to test this 
for their available targets, thanks in advance!


Is this OK for trunk?


Cheers,
Andre

gcc/ChangeLog:

2019-10-30  Andre Vieira  

* params.def (PARAM_VECT_EPILOGUES_NOMASK): Enable by default.

gcc/testsuite/ChangeLog:

2019-10-30  Andre Vieira  

* gcc.dg/vect/vect-epilogues.c: New test.
* gcc.dg/vect/fast-math-vect-call-1.c: Disable for epilogue
vectorization.
* gcc.dg/vect/no-fast-math-vect16.c: Likewise.
* gcc.dg/vect/no-scevccp-noreassoc-slp-reduc-7.c: Likewise.
* gcc.dg/vect/no-scevccp-vect-iv-3.c: Likewise.
* gcc.dg/vect/no-section-anchors-vect-31.c: Likewise.
* gcc.dg/vect/no-section-anchors-vect-64.c: Likewise.
* gcc.dg/vect/no-section-anchors-vect-66.c: Likewise.
* gcc.dg/vect/no-section-anchors-vect-68.c: Likewise.
* gcc.dg/vect/no-vfa-vect-dv-2.c: Likewise.
* gcc.dg/vect/pr33804.c: Likewise.
* gcc.dg/vect/pr53773.c: Likewise.
* gcc.dg/vect/pr65947-1.c: Likewise.
* gcc.dg/vect/pr65947-13.c: Likewise.
* gcc.dg/vect/pr65947-14.c: Likewise.
* gcc.dg/vect/pr65947-4.c: Likewise.
* gcc.dg/vect/pr80631-1.c: Likewise.
* gcc.dg/vect/pr80631-2.c: Likewise.
* gcc.dg/vect/slp-23.c: Likewise.
* gcc.dg/vect/slp-25.c: Likewise.
* gcc.dg/vect/slp-reduc-2.c: Likewise.
* gcc.dg/vect/slp-reduc-5.c: Likewise.
* gcc.dg/vect/slp-reduc-6.c: Likewise.
* gcc.dg/vect/slp-reduc-sad-2.c: Likewise.
* gcc.dg/vect/slp-widen-mult-half.c: Likewise.
* gcc.dg/vect/trapv-vect-reduc-4.c: Likewise.
* gcc.dg/vect/vect-103.c: Likewise.
* gcc.dg/vect/vect-109.c: Likewise.
* gcc.dg/vect/vect-119.c: Likewise.
* gcc.dg/vect/vect-24.c: Likewise.
* gcc.dg/vect/vect-26.c: Likewise.
* gcc.dg/vect/vect-27.c: Likewise.
* gcc.dg/vect/vect-29.c: Likewise.
* gcc.dg/vect/vect-42.c: Likewise.
* gcc.dg/vect/vect-44.c: Likewise.
* gcc.dg/vect/vect-48.c: Likewise.
* gcc.dg/vect/vect-50.c: Likewise.
* gcc.dg/vect/vect-52.c: Likewise.
* gcc.dg/vect/vect-54.c: Likewise.
* gcc.dg/vect/vect-56.c: Likewise.
* gcc.dg/vect/vect-58.c: Likewise.
* gcc.dg/vect/vect-60.c: Likewise.
* gcc.dg/vect/vect-72.c: Likewise.
* gcc.dg/vect/vect-75-big-array.c: Likewise.
* gcc.dg/vect/vect-75.c: Likewise.
* gcc.dg/vect/vect-77-alignchecks.c: Likewise.
* gcc.dg/vect/vect-77-global.c: Likewise.
* gcc.dg/vect/vect-78-alignchecks.c: Likewise.
* gcc.dg/vect/vect-78-global.c: Likewise.
* gcc.dg/vect/vect-89-big-array.c: Likewise.
* gcc.dg/vect/vect-89.c: Likewise.
* gcc.dg/vect/vect-91.c: Likewise.
* gcc.dg/vect/vect-92.c: Likewise.
* gcc.dg/vect/vect-96.c: Likewise.
* gcc.dg/vect/vect-cond-reduc-3.c: Likewise.
* gcc.dg/vect/vect-cond-reduc-4.c: Likewise.
* gcc.dg/vect/vect-live-1.c: Likewise.
* gcc.dg/vect/vect-live-2.c: Likewise.
* gcc.dg/vect/vect-live-3.c: Likewise.
* gcc.dg/vect/vect-live-4.c: Likewise.
* gcc.dg/vect/vect-live-slp-1.c: Likewise.
* gcc.dg/vect/vect-live-slp-2.c: Likewise.
* gcc.dg/vect/vect-live-slp-3.c: Likewise.
* gcc.dg/vect/vect-multitypes-3.c: Likewise.
* gcc.dg/vect/vect-multitypes-4.c: Likewise.
* gcc.dg/vect/vect-multitypes-6.c: Likewise.
* gcc.dg/vect/vect-peel-1-epilogue.c: Likewise. New test.
* gcc.dg/vect/vect-peel-1-src.c: Likewise. New test.
* gcc.dg/vect/vect-peel-1.c: Likewise.
* gcc.dg/vect/vect-peel-3-epilogue.c: Likewise. New test.
* gcc.dg/vect/vect-peel-3-src.c: Likewise. New test.
* gcc.dg/vect/vect-peel-3.c: Likewise.
* gcc.dg/vect/vect-peel-4-epilogue.c: Likewise. New test.
* gcc.dg/vect/vect-peel-4-src.c: Likewise. New test.
* gcc.dg/vect/vect-peel-4.c: Likewise.
* gcc.dg/vect/vect-reduc-6.c: Likewise.
* gcc.dg/vect/vect-reduc-dot-s16a.c: Likewise.
* gcc.dg/vect/vect-reduc-dot-s8a.c: Likewise.
* gcc.dg/vect/vect-reduc-dot-s8b.c: 

Re: introduce -fcallgraph-info option

[Joseph Myers]

On Wed, 30 Oct 2019, Alexandre Oliva wrote:

> On Oct 28, 2019, Joseph Myers  wrote:
> 
> > We have a test in the testsuite that all option help text consistently 
> > ends with '.' (see gcc.misc-tests/help.exp).  I'd have expected these 
> > options, lacking that '.', to cause that test to fail.
> 
> Here's the patch.  Tested on x86_64-linux-gnu.  Ok to install?
> 
> 
> Test --help=common for full sentences
> 
> The portion of help.exp that checks that help output contains full
> sentences failed to cover common options.

OK.

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

Re: Deprecating cc0 (and consequently cc0 targets)

[Jeff Law]

On 10/30/19 11:57 AM, John Paul Adrian Glaubitz wrote:
> On 10/30/19 6:52 PM, Gunther Nikl wrote:
>> Richard Sandiford  wrote:
>>> FWIW it's already possible to do the transform you mention with:
>>>
>>>   s/(cc0)/(reg:CC CC_REGNUM_RC)/g
>>>
>>>   (define_int_iterator CC_REGNUM_RC [(CC_REGNUM "reload_completed")])
>>
>> Since "reload_completed" is referenced, this is only about the CC0
>> conversion, right? Switching to LRA is not required for this step.
> 
> I think it would be nice though that anyone who does the cc0 transition
> would also switch over to LRA unless that would be too much of a burden.
True, but the priority is the cc0 transition.

Jeff

Re: [PATCH 2/3][rs6000] vector conversion RTL pattern update for same unit size

[Segher Boessenkool]

Hi!

On Wed, Oct 23, 2019 at 05:40:35PM +0800, Kewen.Lin wrote:
> For those fixed point <-> floating point vector conversion with
> same element unit size, such as: SP <-> SI, DP <-> DI, it's fine
> to use the existing RTL operations like any_fix/any_float for them.
> 
> This patch is to update them with any_fix/any_float.

Okay for trunk.  Thanks!


Segher


> 2019-10-23  Kewen Lin  
> 
>   * config/rs6000/vsx.md (UNSPEC_VSX_CV[SU]XWSP,
>   UNSPEC_VSX_XVCV[SU]XDDP, UNSPEC_VSX_XVCVDP[SU]XDS,
>   UNSPEC_VSX_XVCVSPSXWS): Remove.
>   (vsx_xvcv[su]xddp, vsx_xvcvdp[su]xds, vsx_xvcvsp[su]xws,
>   vsx_xvcv[su]xwsp): Update define_insn RTL patterns.

Re: [PATCH 1/3][rs6000] Replace vsx_xvcdpsp by vsx_xvcvdpsp

[Segher Boessenkool]

Hi!

On Wed, Oct 23, 2019 at 05:39:14PM +0800, Kewen.Lin wrote:
> I noticed that vsx_xvcdpsp and vsx_xvcvdpsp are almost the same,
> and vsx_xvcdpsp looks replaceable with vsx_xvcvdpsp since it's only
> called by gen_*.

Okay for trunk.  Thanks!


Segher


> 2019-10-23  Kewen Lin  
> 
>   * config/rs6000/vsx.md (vsx_xvcdpsp): Remove define_insn.
>   (UNSPEC_VSX_XVCDPSP): Remove.
>   * config/rs6000/rs6000.c (rs6000_generate_float2_double_code):
>   Replace gen_vsx_xvcdpsp by gen_vsx_xvcvdpsp.

Re: Deprecating cc0 (and consequently cc0 targets)

[Jeff Law]

On 10/30/19 11:52 AM, Gunther Nikl wrote:
> Richard Sandiford  wrote:
>> FWIW it's already possible to do the transform you mention with:
>>
>>   s/(cc0)/(reg:CC CC_REGNUM_RC)/g
>>
>>   (define_int_iterator CC_REGNUM_RC [(CC_REGNUM "reload_completed")])
> 
> Since "reload_completed" is referenced, this is only about the CC0
> conversion, right? Switching to LRA is not required for this step.
Correct

jeff

Re: Deprecating cc0 (and consequently cc0 targets)

[John Paul Adrian Glaubitz]

On 10/30/19 6:52 PM, Gunther Nikl wrote:
> Richard Sandiford  wrote:
>> FWIW it's already possible to do the transform you mention with:
>>
>>   s/(cc0)/(reg:CC CC_REGNUM_RC)/g
>>
>>   (define_int_iterator CC_REGNUM_RC [(CC_REGNUM "reload_completed")])
> 
> Since "reload_completed" is referenced, this is only about the CC0
> conversion, right? Switching to LRA is not required for this step.

I think it would be nice though that anyone who does the cc0 transition
would also switch over to LRA unless that would be too much of a burden.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaub...@debian.org
`. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: Deprecating cc0 (and consequently cc0 targets)

[Gunther Nikl]

Richard Sandiford  wrote:
> FWIW it's already possible to do the transform you mention with:
> 
>   s/(cc0)/(reg:CC CC_REGNUM_RC)/g
> 
>   (define_int_iterator CC_REGNUM_RC [(CC_REGNUM "reload_completed")])

Since "reload_completed" is referenced, this is only about the CC0
conversion, right? Switching to LRA is not required for this step.

Regards,
Gunther

Re: [PATCH] Use if-constexpr instead of overloading for customization point

[Jonathan Wakely]

On 30/10/19 17:42 +, Jonathan Wakely wrote:

This combines two of the std::ranges::swap.operator() overloads into a
single function template. Using if-constexpr to choose between
implementations should give the compiler less work to do than using
overloading.


P.S. this is how all the other std::ranges::* customization points are
defined, but I only started doing that after adding std::ranges::swap.
Now they're all done this way.

[PATCH] Use if-constexpr instead of overloading for customization point

[Jonathan Wakely]

This combines two of the std::ranges::swap.operator() overloads into a
single function template. Using if-constexpr to choose between
implementations should give the compiler less work to do than using
overloading.

* include/std/concepts (std::ranges::swap): Use a single overload for
the non-array cases, and switch using if-constexpr.

Tested powerpc64le-linux, committed to trunk.


commit 1a09c76c7256ef400ef8be96937b25ab3ed6d585
Author: Jonathan Wakely 
Date:   Wed Oct 30 17:03:46 2019 +

Use if-constexpr instead of overloading for customization point

This combines two of the std::ranges::swap.operator() overloads into a
single function template. Using if-constexpr to choose between
implementations should give the compiler less work to do than using
overloading.

* include/std/concepts (std::ranges::swap): Use a single overload 
for
the non-array cases, and switch using if-constexpr.

diff --git a/libstdc++-v3/include/std/concepts 
b/libstdc++-v3/include/std/concepts
index 68cbba9b8a7..c4acfd2e212 100644
--- a/libstdc++-v3/include/std/concepts
+++ b/libstdc++-v3/include/std/concepts
@@ -173,12 +173,37 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   struct _Swap
   {
+  private:
+   template
+ static constexpr bool
+ _S_noexcept()
+ {
+   if constexpr (__adl_swap<_Tp, _Up>)
+ return noexcept(swap(std::declval<_Tp>(), std::declval<_Up>()));
+   else
+ return is_nothrow_move_constructible_v>
+  && is_nothrow_move_assignable_v>;
+ }
+
+  public:
template
  requires __adl_swap<_Tp, _Up>
+ || (same_as<_Tp, _Up> && is_lvalue_reference_v<_Tp>
+ && move_constructible>
+ && assignable_from<_Tp, remove_reference_t<_Tp>>)
  constexpr void
  operator()(_Tp&& __t, _Up&& __u) const
- noexcept(noexcept(swap(std::declval<_Tp>(), std::declval<_Up>(
- { swap(static_cast<_Tp&&>(__t), static_cast<_Up&&>(__u)); }
+ noexcept(_S_noexcept<_Tp, _Up>())
+ {
+   if constexpr (__adl_swap<_Tp, _Up>)
+ swap(static_cast<_Tp&&>(__t), static_cast<_Up&&>(__u));
+   else
+ {
+   auto __tmp = static_cast&&>(__t);
+   __t = static_cast&&>(__u);
+   __u = static_cast&&>(__tmp);
+ }
+ }
 
template
  requires requires(const _Swap& __swap, _Tp& __e1, _Up& __e2) {
@@ -191,19 +216,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
for (size_t __n = 0; __n < _Num; ++__n)
  (*this)(__e1[__n], __e2[__n]);
  }
-
-   template
- requires (!__adl_swap<_Tp&, _Tp&>
-   && move_constructible<_Tp> && assignable_from<_Tp&, _Tp>)
- constexpr void
- operator()(_Tp& __e1, _Tp& __e2) const
- noexcept(is_nothrow_move_constructible_v<_Tp>
-  && is_nothrow_move_assignable_v<_Tp>)
- {
-   _Tp __tmp = static_cast<_Tp&&>(__e1);
-   __e1 = static_cast<_Tp&&>(__e2);
-   __e2 = static_cast<_Tp&&>(__tmp);
- }
   };
 } // namespace __cust_swap
 

Re: [C++ PATCH] Feature macros are long

[Nathan Sidwell]

On 10/30/19 12:36 PM, Jakub Jelinek wrote:

On Wed, Oct 30, 2019 at 12:33:00PM -0400, Nathan Sidwell wrote:

As discussed on IRC, this adds an L suffix to C++ feature macros, as
specified by the std.  I'd forgotten that in preprocessor-land, expressions
are evaluated as longs anyway, but the user might be trying to printf these
constants, or similar, where the type suffix is significant.

Jakub, Thomas:
 cpp_define (pfile, "_OPENACC=201306");
 cpp_define (pfile, "_OPENMP=201511");
those two macros might need suffixing too, but I don't have a std handy.


"the _OPENMP macro name is defined to have the
decimal value mm where  and mm are the year and month designations
of the version of the OpenMP API that the implementation supports."
I think that doesn't imply L suffix, but can ask.


'decimal value' is a confused term.  It's a number with value, type and 
representation.  If it said the token mmm where ..., I'd understand.


nathan

--
Nathan Sidwell

Re: [RFC PATCH] targetm.omp.device_kind_arch_isa and OpenMP declare variant kind/arch/isa handling

[Szabolcs Nagy]

On 30/10/2019 14:48, Jakub Jelinek wrote:
> ARM is an OpenMP member, so if you want, you can participate too.
> https://github.com/OpenMP/spec/issues/2028
> is where I'm trying to track all the declare variant issues that need
> clarification (plus in two examples tickets).

it's unfortunate that neither the mailing list nor
the github repo for the spec are public, i'll try
to get access to them to see the discussions.
thanks for the explanations.

>> for simd variants it means i need to declare the function
>> with the right simd types and attributes.
> 
> For simd it is actually not finished yet, what needs to be done is that
> given the declare simd clauses used as properties of the simd selector
> the FEs use some target hook that will guide it how to transform
> the parameters like targetm.simd_clone.compute_vecsize_and_simdlen
> does and for C tries to just match the types against it and determine
> through that the ABI and perhaps missing simd clauses like
> notinbranch/inbranch, simdlen etc., for C++ actually for each possibility
> will try to construct a call with such arguments and then compare the types.

this omp declare variant mechanism seems fairly complicated.

i need is a way to specify a simd variant unambiguously,
which requires is_inbranch, simd_len and vector_call_abi
setting as far as i can tell (potentially a symbol_name
too if the vector abi mangled name is not good enough).

i think i can extend the simd attribute to do this e.g.

__attribute__((simd("notinbranch", 4, "sse2")))
float expf(float);

would declare the _ZGVbN4v_expf simd variant of expf.
(multiple attributes can be used to declare multiple
variants.)

or

__attribute__((simd("notinbranch", 4, "sse2", "my_vexpf")))
float expf(float);

or maybe

typedef float vfloat __attribute__((vector_size(16)));
vfloat my_vexpf(vfloat);

__attribute__((simd("notinbranch", 4, "sse2", my_vexpf)))
float expf(float);

if we allow custom symbol name for the simd variant.

here "sse2" is not specifying a gcc target nor instruction
set, but the vector call convention, e.g. on x86_64 there
could be "sse2", "avx", "avx2" and "avx512", on aarch64
"advsimd" and "sve".

i thought this would match the omp isa, but based on your
description omp isa will be something else.

there is a further complication that vector length agnostic
(scalable) sve calls need a special simd len value: i'd
reserve 0 for it, but it seems internally that means
'simd len is unset' so that has to change.

i will try to prepare an initial patch for such attribute
to make the proposal more concrete, but if you have any
concerns please let me know.

Re: [PATCH] Apply C++20 changes to various iterator types

[Jonathan Wakely]

On 30/10/19 15:47 +, Jonathan Wakely wrote:

This ensures that __normal_iterator satisfies the
contiguous_iterator concept, by defining the iterator_concept member
type.

Also update vector's iterators, reverse_iterator,
istreambuf_iterator and ostreambuf_iterator to meet the C++20
requirements.

PR libstdc++/92272
* include/bits/stl_bvector.h (_Bit_iterator::pointer)
(_Bit_const_iterator::pointer): Define as void for C++20.
* include/bits/stl_iterator.h (reverse_iterator::operator->()): Add
constraints for C++20.
(__normal_iterator::iterator_concept): Define for C++20.


I broke Clang again with that part. Fixed like so.

Tested powerpc64le-linux, committed to trunk.


commit 0649504b11da5e63511b6ee61b165149a153abe5
Author: Jonathan Wakely 
Date:   Wed Oct 30 16:03:01 2019 +

Fix another compilation error with Clang

* include/bits/stl_iterator.h (__normal_iterator::iterator_concept):
Guard with __cpp_lib_concepts macro.

diff --git a/libstdc++-v3/include/bits/stl_iterator.h b/libstdc++-v3/include/bits/stl_iterator.h
index ecc06178c34..411feba90e0 100644
--- a/libstdc++-v3/include/bits/stl_iterator.h
+++ b/libstdc++-v3/include/bits/stl_iterator.h
@@ -809,7 +809,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   typedef typename __traits_type::reference 	reference;
   typedef typename __traits_type::pointer   	pointer;
 
-#if __cplusplus > 201703L
+#if __cplusplus > 201703L && __cpp_lib_concepts
   using iterator_concept = std::__detail::__iter_concept<_Iterator>;
 #endif
 

[PATCH] Fix some missing/incorrect feature test macros

[Jonathan Wakely]

* include/std/bit (__cpp_lib_bitops): Define.
* include/std/version (__cpp_lib_constexpr): Remove.
(__cpp_lib_bitops, __cpp_lib_constexpr_dynamic_alloc): Define.
* testsuite/26_numerics/bit/header.cc: New test.
* testsuite/26_numerics/bit/header-2.cc: New test.
* testsuite/20_util/allocator_traits/header.cc: New test.
* testsuite/20_util/allocator_traits/header-2.cc: New test.

Tested powerpc64le-linux, committed to trunk.


commit 3921fbce406b969aa971fbdee2c154ab8349ea37
Author: Jonathan Wakely 
Date:   Wed Oct 30 15:52:13 2019 +

Fix some missing/incorrect feature test macros

* include/std/bit (__cpp_lib_bitops): Define.
* include/std/version (__cpp_lib_constexpr): Remove.
(__cpp_lib_bitops, __cpp_lib_constexpr_dynamic_alloc): Define.
* testsuite/26_numerics/bit/header.cc: New test.
* testsuite/26_numerics/bit/header-2.cc: New test.
* testsuite/20_util/allocator_traits/header.cc: New test.
* testsuite/20_util/allocator_traits/header-2.cc: New test.

diff --git a/libstdc++-v3/include/std/bit b/libstdc++-v3/include/std/bit
index 914cdfe629a..e89bca2c359 100644
--- a/libstdc++-v3/include/std/bit
+++ b/libstdc++-v3/include/std/bit
@@ -263,6 +263,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
 #if __cplusplus > 201703L
 
+#define __cpp_lib_bitops 201907L
+
   /// @cond undoc
   template
 using _If_is_unsigned_integer
diff --git a/libstdc++-v3/include/std/version b/libstdc++-v3/include/std/version
index ccaedd090b0..a01a8b2fea1 100644
--- a/libstdc++-v3/include/std/version
+++ b/libstdc++-v3/include/std/version
@@ -107,7 +107,7 @@
 # define __cpp_lib_has_unique_object_representations 201606
 #endif
 #define __cpp_lib_hypot 201603
-#define __cpp_lib_invoke 201411
+#define __cpp_lib_invoke 201411L
 #ifdef _GLIBCXX_HAVE_BUILTIN_IS_AGGREGATE
 # define __cpp_lib_is_aggregate 201703
 #endif
@@ -152,12 +152,13 @@
 // c++2a
 #define __cpp_lib_atomic_ref 201806L
 #define __cpp_lib_bind_front 201907L
+#define __cpp_lib_bitops 201907L
 #define __cpp_lib_bounded_array_traits 201902L
 #if __cpp_concepts
 # define __cpp_lib_concepts 201806L
 #endif
-#define __cpp_lib_constexpr 201711L
 #define __cpp_lib_constexpr_algorithms 201806L
+#define __cpp_lib_constexpr_dynamic_alloc 201907L
 #define __cpp_lib_constexpr_invoke 201907L
 #if __cpp_impl_destroying_delete
 # define __cpp_lib_destroying_delete 201806L
diff --git a/libstdc++-v3/testsuite/20_util/allocator_traits/header-2.cc 
b/libstdc++-v3/testsuite/20_util/allocator_traits/header-2.cc
new file mode 100644
index 000..e43b6f9e798
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/allocator_traits/header-2.cc
@@ -0,0 +1,27 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// .
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a } }
+
+#include 
+
+#ifndef __cpp_lib_constexpr_dynamic_alloc
+# error "Feature test macro for constexpr_dynamic_alloc is missing in 
"
+#elif __cpp_lib_constexpr_dynamic_alloc < 201907L
+# error "Feature test macro for constexpr_dynamic_alloc has wrong value in 
"
+#endif
diff --git a/libstdc++-v3/testsuite/20_util/allocator_traits/header.cc 
b/libstdc++-v3/testsuite/20_util/allocator_traits/header.cc
new file mode 100644
index 000..6687929c5e2
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/allocator_traits/header.cc
@@ -0,0 +1,27 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// .
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { 

Re: [PATCH] Refactor rust-demangle to be independent of C++ demangling.

[Eduard-Mihai Burtescu]

Ping: https://gcc.gnu.org/ml/gcc-patches/2019-10/msg01830.html
Original patch (without the early exit optimization): 
https://gcc.gnu.org/ml/gcc-patches/2019-10/msg01591.html

Thanks,
- Eddy B.

On Fri, Oct 25, 2019, at 3:44 PM, Eduard-Mihai Burtescu wrote:
> > This can be further optimized by using memcmp in place of strncmp, since 
> > from
> > the length check you know that you won't see the null terminator among the 
> > three
> > chars you're checking.
> 
> Fair enough, here's the combined changelog/diff, with memcmp:

Re: [C++ PATCH] Feature macros are long

[Jakub Jelinek]

On Wed, Oct 30, 2019 at 12:33:00PM -0400, Nathan Sidwell wrote:
> As discussed on IRC, this adds an L suffix to C++ feature macros, as
> specified by the std.  I'd forgotten that in preprocessor-land, expressions
> are evaluated as longs anyway, but the user might be trying to printf these
> constants, or similar, where the type suffix is significant.
> 
> Jakub, Thomas:
> cpp_define (pfile, "_OPENACC=201306");
> cpp_define (pfile, "_OPENMP=201511");
> those two macros might need suffixing too, but I don't have a std handy.

"the _OPENMP macro name is defined to have the
decimal value mm where  and mm are the year and month designations
of the version of the OpenMP API that the implementation supports."
I think that doesn't imply L suffix, but can ask.

Jakub

Re: [Patch][Fortran/OpenMP] Don't create "alloc:" for 'target exit data'

[Tobias Burnus]

On 10/30/19 4:55 PM, Jakub Jelinek wrote:
Do they? At least the C/C++ FEs should complain/remove before it makes 
its way into the middle-end. […]

Haven't checked the Fortran FE.


The Fortran FE lacks many checks the C/C++ FE has – but, admittedly, it 
*does* have this check. (Which obviously does not apply to FE generated 
code.)


Ok. 


Thanks for the quick review. (Committed as Rev. 277631.)

Tobias

[C++ PATCH] Feature macros are long

[Nathan Sidwell]

As discussed on IRC, this adds an L suffix to C++ feature macros, as 
specified by the std.  I'd forgotten that in preprocessor-land, 
expressions are evaluated as longs anyway, but the user might be trying 
to printf these constants, or similar, where the type suffix is significant.


Jakub, Thomas:
cpp_define (pfile, "_OPENACC=201306");
cpp_define (pfile, "_OPENMP=201511");
those two macros might need suffixing too, but I don't have a std handy.

nathan
--
Nathan Sidwell
2019-10-30  Nathan Sidwell  

	* c-cppbuiltin.c (c_cpp_builtins): Add 'L' suffix to feature
	macros.

Index: c-family/c-cppbuiltin.c
===
--- c-family/c-cppbuiltin.c	(revision 277620)
+++ c-family/c-cppbuiltin.c	(working copy)
@@ -890,7 +890,7 @@ c_cpp_builtins (cpp_reader *pfile)
   if (flag_rtti)
 	{
 	  cpp_define (pfile, "__GXX_RTTI");
-	  cpp_define (pfile, "__cpp_rtti=199711");
+	  cpp_define (pfile, "__cpp_rtti=199711L");
 	}
 
   if (cxx_dialect >= cxx11)
@@ -899,11 +899,11 @@ c_cpp_builtins (cpp_reader *pfile)
   /* Binary literals have been allowed in g++ before C++11
 	 and were standardized for C++14.  */
   if (!pedantic || cxx_dialect > cxx11)
-	cpp_define (pfile, "__cpp_binary_literals=201304");
+	cpp_define (pfile, "__cpp_binary_literals=201304L");
 
   /* Similarly for hexadecimal floating point literals and C++17.  */
   if (!pedantic || cpp_get_options (parse_in)->extended_numbers)
-	cpp_define (pfile, "__cpp_hex_float=201603");
+	cpp_define (pfile, "__cpp_hex_float=201603L");
 
   /* Arrays of runtime bound were removed from C++14, but we still
 	 support GNU VLAs.  Let's define this macro to a low number
@@ -911,112 +911,112 @@ c_cpp_builtins (cpp_reader *pfile)
 	 complain about use of VLAs.  */
   if (c_dialect_cxx ()
 	  && (pedantic ? warn_vla == 0 : warn_vla <= 0))
-	cpp_define (pfile, "__cpp_runtime_arrays=198712");
+	cpp_define (pfile, "__cpp_runtime_arrays=198712L");
 
   if (cxx_dialect >= cxx11)
 	{
 	  /* Set feature test macros for C++11.  */
 	  if (cxx_dialect <= cxx14)
-	cpp_define (pfile, "__cpp_unicode_characters=200704");
-	  cpp_define (pfile, "__cpp_raw_strings=200710");
-	  cpp_define (pfile, "__cpp_unicode_literals=200710");
-	  cpp_define (pfile, "__cpp_user_defined_literals=200809");
-	  cpp_define (pfile, "__cpp_lambdas=200907");
+	cpp_define (pfile, "__cpp_unicode_characters=200704L");
+	  cpp_define (pfile, "__cpp_raw_strings=200710L");
+	  cpp_define (pfile, "__cpp_unicode_literals=200710L");
+	  cpp_define (pfile, "__cpp_user_defined_literals=200809L");
+	  cpp_define (pfile, "__cpp_lambdas=200907L");
 	  if (cxx_dialect == cxx11)
-	cpp_define (pfile, "__cpp_constexpr=200704");
+	cpp_define (pfile, "__cpp_constexpr=200704L");
 	  if (cxx_dialect <= cxx14)
-	cpp_define (pfile, "__cpp_range_based_for=200907");
+	cpp_define (pfile, "__cpp_range_based_for=200907L");
 	  if (cxx_dialect <= cxx14)
-	cpp_define (pfile, "__cpp_static_assert=200410");
-	  cpp_define (pfile, "__cpp_decltype=200707");
-	  cpp_define (pfile, "__cpp_attributes=200809");
-	  cpp_define (pfile, "__cpp_rvalue_reference=200610");
-	  cpp_define (pfile, "__cpp_rvalue_references=200610");
-	  cpp_define (pfile, "__cpp_variadic_templates=200704");
-	  cpp_define (pfile, "__cpp_initializer_lists=200806");
-	  cpp_define (pfile, "__cpp_delegating_constructors=200604");
-	  cpp_define (pfile, "__cpp_nsdmi=200809");
+	cpp_define (pfile, "__cpp_static_assert=200410L");
+	  cpp_define (pfile, "__cpp_decltype=200707L");
+	  cpp_define (pfile, "__cpp_attributes=200809L");
+	  cpp_define (pfile, "__cpp_rvalue_reference=200610L");
+	  cpp_define (pfile, "__cpp_rvalue_references=200610L");
+	  cpp_define (pfile, "__cpp_variadic_templates=200704L");
+	  cpp_define (pfile, "__cpp_initializer_lists=200806L");
+	  cpp_define (pfile, "__cpp_delegating_constructors=200604L");
+	  cpp_define (pfile, "__cpp_nsdmi=200809L");
 	  if (!flag_new_inheriting_ctors)
-	cpp_define (pfile, "__cpp_inheriting_constructors=200802");
+	cpp_define (pfile, "__cpp_inheriting_constructors=200802L");
 	  else
-	cpp_define (pfile, "__cpp_inheriting_constructors=201511");
-	  cpp_define (pfile, "__cpp_ref_qualifiers=200710");
-	  cpp_define (pfile, "__cpp_alias_templates=200704");
+	cpp_define (pfile, "__cpp_inheriting_constructors=201511L");
+	  cpp_define (pfile, "__cpp_ref_qualifiers=200710L");
+	  cpp_define (pfile, "__cpp_alias_templates=200704L");
 	}
   if (cxx_dialect > cxx11)
 	{
 	  /* Set feature test macros for C++14.  */
-	  cpp_define (pfile, "__cpp_return_type_deduction=201304");
-	  cpp_define (pfile, "__cpp_init_captures=201304");
-	  cpp_define (pfile, "__cpp_generic_lambdas=201304");
+	  cpp_define (pfile, "__cpp_return_type_deduction=201304L");
+	  cpp_define (pfile, "__cpp_init_captures=201304L");
+	  cpp_define (pfile, "__cpp_generic_lambdas=201304L");
 	  if (cxx_dialect <= cxx14)
-	  

Re: [8/n] Replace autovectorize_vector_sizes with autovectorize_vector_modes

[Richard Sandiford]

Richard Biener  writes:
> On Fri, Oct 25, 2019 at 2:37 PM Richard Sandiford
>  wrote:
>>
>> This is another patch in the series to remove the assumption that
>> all modes involved in vectorisation have to be the same size.
>> Rather than have the target provide a list of vector sizes,
>> it makes the target provide a list of vector "approaches",
>> with each approach represented by a mode.
>>
>> A later patch will pass this mode to targetm.vectorize.related_mode
>> to get the vector mode for a given element mode.  Until then, the modes
>> simply act as an alternative way of specifying the vector size.
>
> Is there a restriction to use integer vector modes for the hook
> or would FP vector modes be OK as well?

Conceptually, each mode returned by the hook represents a set of vector
modes, with the set containing one member for each supported element
type.  The idea is to represent the set using the member with the
smallest element type, preferring integer modes over floating-point
modes in the event of a tie.  So using a floating-point mode as the
representative mode is fine if floating-point elements are the smallest
(or only) supported element type.

> Note that your x86 change likely disables word_mode vectorization with
> -mno-sse?

No, that still works, because...

> That is, how do we represent GPR vectorization "size" here?
> The preferred SIMD mode hook may return an integer mode,
> are non-vector modes OK for autovectorize_vector_modes?

...at least with all current targets, preferred_simd_mode is only
an integer mode if the target has no "real" vectorisation support
for that element type.  There's no need to handle that case in
autovectorize_vector_sizes/modes, and e.g. the x86 hook does nothing
when SSE is disabled.

So while preferred_simd_mode can continue to return integer modes,
autovectorize_vector_modes always returns vector modes.

This patch just treats the mode as an alternative way of specifying
the vector size.  11/n then tries to use related_vector_mode to choose
the vector mode for each element type instead.  But 11/n only uses
related_vector_mode if vec_info::vector_mode is a vector mode.  If it's
an integer mode (as for -mno-sse), or if related_vector_mode fails to
find a vector mode, then we still fall back to mode_for_vector and so
pick an integer mode in the same cases as before.

Thanks,
Richard

Re: [Patch][Fortran/OpenMP] Don't create "alloc:" for 'target exit data'

[Jakub Jelinek]

On Wed, Oct 30, 2019 at 04:48:43PM +0100, Tobias Burnus wrote:
> On 10/30/19 11:12 AM, Jakub Jelinek wrote:
> > I believe it is easier to handle it at the same spot as we do it e.g.
> > for C/C++ pointer attachments (where we create the same clauses
> > regardless of the exact construct and then drop them later), in
> > particular in gimplify_scan_omp_clauses. […]
> 
> I concur. Semantically, it is not identical – but I think still okay.
> 
> For 'omp exit data', 'to:'/'alloc:' mapping does not make sense and it not
> handled in libgomp's gomp_exit_data. Hence, I exclude GOMP_MAP_POINTER
> (dump: 'alloc:') and GOMP_MAP_TO_PSET (dump: 'to:'). – Those are only
> internally used, hence, user-specified 'alloc:' will get diagnosed.
> 
> ['delete:'/'release:' in other directives than 'exit data' doesn't make much
> sense. Other directives accept it but their libgomp function silently ignore
> it.]

Do they?
At least the C/C++ FEs should complain/remove before it makes its way into the
middle-end.  E.g. c_parser_omp_target_enter_data has:
switch (OMP_CLAUSE_MAP_KIND (*pc))
  {
  case GOMP_MAP_TO:
  case GOMP_MAP_ALWAYS_TO:
  case GOMP_MAP_ALLOC:
map_seen = 3;
break;
  case GOMP_MAP_FIRSTPRIVATE_POINTER:
  case GOMP_MAP_ALWAYS_POINTER:
break;
  default:
map_seen |= 1;
error_at (OMP_CLAUSE_LOCATION (*pc),
  "%<#pragma omp target enter data%> with map-type other "
  "than % or % on % clause");
*pc = OMP_CLAUSE_CHAIN (*pc);
continue;
  }
Haven't checked the Fortran FE.

>   gcc/
>   * gimplify.c (gimplify_scan_omp_clauses): Remove FE-generated
>   GOMP_MAP_TO_PSET and GOMP_MAP_POINTER mapping for 'target update'
>   and 'target exit data'.
> 
>   libgomp/
>   * testsuite/libgomp.fortran/target9.f90: New.

Ok.

Jakub

Re: Watch for missing summaries even more

[Jan Hubicka]

> Hi,
> 
> On Wed, Oct 30 2019, Jan Hubicka wrote:
> >> 
> >> Looking at PR 92278, I think I found the real problem. In
> >> ipa_read_edge_info, you added code to throw away jump functions of edges
> >> that do not pass possibly_call_in_translation_unit_p() test.  But that
> >> predicate incorrectly - or at least I think so, see below - returns
> >> false for edges leading to interposable symbols.  The reason why I think
> >> it is incorrect is because a node which is considered interposable
> >> before merging can apparently later on be upgraded to a local one by
> >> ipa-visibility.
> >> 
> >> I am testing the following fix, is it OK if it passes?
> >> 
> >> The testcase passes a version script to the linker, I guess our LTO
> >> testsuite does not support that, does it?
> >> 
> >> Thanks,
> >> 
> >> Martin
> >> 
> >> 
> >> 2019-10-30  Martin Jambor  
> >> 
> >>ipa/92278
> >>* cgraph.c (cgraph_edge::possibly_call_in_translation_unit_p): Fix
> >>availability comparison.
> >
> > yes, this is OK - thanks for looking into this!
> > While jump functions are interesting only for available symbols
> > interposable symbols may be promoted to them based on resolution info.
> >
> > Lets see how much extra memory this will cost. If it is too bad I can
> > add resolution info logic but duplicating it from ipa-visibility is not
> > cool.
> 
> OK, I have committed the patch.  I wonder whether we want to revert the
> early exit in update_jump_functions_after_inlining - or replace it with
> a gcc_checking_assert - now.

I do not think we can revert it - we want to be able to inline functions
with no jump functions (-O0 and -fno-ipa-cp -fno-indirect-inlining).
Having assert would be useful (we had another two bugs in this direction
I fixed during weekend) - we probably could check that if summary is
missing then the function must be compiled without those flags?

Honza
> 
> Anyway, thanks,
> 
> Martin
> 
> 
> >
> > Honza
> >> ---
> >>  gcc/cgraph.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> 
> >> diff --git a/gcc/cgraph.c b/gcc/cgraph.c
> >> index d47d4128b1c..8057ccdb7c0 100644
> >> --- a/gcc/cgraph.c
> >> +++ b/gcc/cgraph.c
> >> @@ -3813,7 +3813,7 @@ cgraph_edge::possibly_call_in_translation_unit_p 
> >> (void)
> >>if (node->previous_sharing_asm_name)
> >>  node = symtab_node::get_for_asmname (DECL_ASSEMBLER_NAME 
> >> (callee->decl));
> >>gcc_assert (TREE_PUBLIC (node->decl));
> >> -  return node->get_availability () >= AVAIL_AVAILABLE;
> >> +  return node->get_availability () >= AVAIL_INTERPOSABLE;
> >>  }
> >>  
> >>  /* A stashed copy of "symtab" for use by selftest::symbol_table_test.
> >> -- 
> >> 2.23.0
> >> 

Re: Watch for missing summaries even more

[Martin Jambor]

Hi,

On Wed, Oct 30 2019, Jan Hubicka wrote:
>> 
>> Looking at PR 92278, I think I found the real problem. In
>> ipa_read_edge_info, you added code to throw away jump functions of edges
>> that do not pass possibly_call_in_translation_unit_p() test.  But that
>> predicate incorrectly - or at least I think so, see below - returns
>> false for edges leading to interposable symbols.  The reason why I think
>> it is incorrect is because a node which is considered interposable
>> before merging can apparently later on be upgraded to a local one by
>> ipa-visibility.
>> 
>> I am testing the following fix, is it OK if it passes?
>> 
>> The testcase passes a version script to the linker, I guess our LTO
>> testsuite does not support that, does it?
>> 
>> Thanks,
>> 
>> Martin
>> 
>> 
>> 2019-10-30  Martin Jambor  
>> 
>>  ipa/92278
>>  * cgraph.c (cgraph_edge::possibly_call_in_translation_unit_p): Fix
>>  availability comparison.
>
> yes, this is OK - thanks for looking into this!
> While jump functions are interesting only for available symbols
> interposable symbols may be promoted to them based on resolution info.
>
> Lets see how much extra memory this will cost. If it is too bad I can
> add resolution info logic but duplicating it from ipa-visibility is not
> cool.

OK, I have committed the patch.  I wonder whether we want to revert the
early exit in update_jump_functions_after_inlining - or replace it with
a gcc_checking_assert - now.

Anyway, thanks,

Martin


>
> Honza
>> ---
>>  gcc/cgraph.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/gcc/cgraph.c b/gcc/cgraph.c
>> index d47d4128b1c..8057ccdb7c0 100644
>> --- a/gcc/cgraph.c
>> +++ b/gcc/cgraph.c
>> @@ -3813,7 +3813,7 @@ cgraph_edge::possibly_call_in_translation_unit_p (void)
>>if (node->previous_sharing_asm_name)
>>  node = symtab_node::get_for_asmname (DECL_ASSEMBLER_NAME 
>> (callee->decl));
>>gcc_assert (TREE_PUBLIC (node->decl));
>> -  return node->get_availability () >= AVAIL_AVAILABLE;
>> +  return node->get_availability () >= AVAIL_INTERPOSABLE;
>>  }
>>  
>>  /* A stashed copy of "symtab" for use by selftest::symbol_table_test.
>> -- 
>> 2.23.0
>> 

Re: Re: [RFC/PATCH v2][PR89245] Check REG_CALL_DECL note during the tail-merging

[Dragan Mladjenovic]

On 01.10.2019. 21:35, Jeff Law wrote:
> On 9/6/19 4:23 AM, Dragan Mladjenovic wrote:
>> On 24.07.2019. 20:57, Jeff Law wrote:
>>> On 7/17/19 2:29 AM, Dragan Mladjenovic wrote:


 On 09.07.2019. 23:21, Jeff Law wrote:
> On 7/9/19 2:06 PM, Dragan Mladjenovic wrote:
>> This patch prevents merging of CALL instructions that that have different
>> REG_CALL_DECL notes attached to them.
>>
>> On most architectures this is not an important distinction. Usually 
>> instruction patterns
>> for calls to different functions reference different SYMBOL_REF-s, so 
>> they won't match.
>> On MIPS PIC calls get split into an got_load/*call_internal pair where 
>> the latter represents
>> indirect register call w/o SYMBOL_REF attached (until machine_reorg 
>> pass). The bugzilla issue
>> had such two internal_call-s merged despite the fact that they had 
>> different register usage
>> information assigned by ipa-ra.
>>
>> As per comment form Richard Sandiford, this version compares reg usage 
>> for both call
>> instruction instead of shallow comparing the notes. Tests updated 
>> accordingly.
>>
>> gcc/ChangeLog:
>>
>> 2019-07-09  Dragan Mladjenovic  
>>
>>  * cfgcleanup.c (old_insns_match_p): Check if used hard regs set is equal
>>  for both call instructions.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2019-07-09  Dragan Mladjenovic  
>>
>>  * gcc.target/mips/cfgcleanup-jalr1.c: New test.
>>  * gcc.target/mips/cfgcleanup-jalr2.c: New test.
>>  * gcc.target/mips/cfgcleanup-jalr3.c: New test.
> THanks.  I've installed this on the trunk.
>
> jeff
 Thanks. Can this be back-ported to active branches also. This issue
 seems to be there > since gcc6 if not gcc5.
>>> I've asked Matthew to handle the backport.  I'm going to be on PTO the
>>> next couple weeks.
>>>
>>> jeff
>>>
>>
>> Hi,
>>
>> Sorry, I forgot to check up on this patch. Is it still ok for me to try
>> to backport it to gcc 9 and gcc 8 branches?
> Yes, this would be fine to backport to gcc-8 and gcc-9 branches.  I'd
> expect it to be pretty easy as I don't think old_insns_match_p has
> changed much.

Thanks and sorry for the delay.
Backported as r277625 and r277626 to gcc-9 and gcc-8 branch respectively.

Best regards,
Dragan

Re: [Patch][Fortran/OpenMP] Don't create "alloc:" for 'target exit data'

[Tobias Burnus]

On 10/30/19 11:12 AM, Jakub Jelinek wrote:
I believe it is easier to handle it at the same spot as we do it e.g. 
for C/C++ pointer attachments (where we create the same clauses 
regardless of the exact construct and then drop them later), in 
particular in gimplify_scan_omp_clauses. […]


I concur. Semantically, it is not identical – but I think still okay.

For 'omp exit data', 'to:'/'alloc:' mapping does not make sense and it 
not handled in libgomp's gomp_exit_data. Hence, I exclude 
GOMP_MAP_POINTER (dump: 'alloc:') and GOMP_MAP_TO_PSET (dump: 'to:'). – 
Those are only internally used, hence, user-specified 'alloc:' will get 
diagnosed.


['delete:'/'release:' in other directives than 'exit data' doesn't make 
much sense. Other directives accept it but their libgomp function 
silently ignore it.]


'omp update': The gomp_update function only handles GOMP_MAP_COPY_TO_P 
and GOMP_MAP_COPY_FROM_P (and silently ignores others). Both macros have 
!((X) & GOMP_MAP_FLAG_SPECIAL). Hence, we can save a few bytes and avoid 
calling 'omp update' with GOMP_MAP_POINTER and GOMP_MAP_TO_PSET.


[TO_PSET only appears in gfc_trans_omp_clauses (once); POINTER appears 
there and in gfc_omp_finish_clause and in c/c-typeck.c's 
handle_omp_array_sections but only if "(ort != C_ORT_OMP && ort != 
C_ORT_ACC)".]



I moved trans-openmp.c change to gimplify.c and left the test case 
unchanged. Then, I bootstrapped on a non-offloading system and regtested 
it also with a nvptx system.


Tobias

 	gcc/
	* gimplify.c (gimplify_scan_omp_clauses): Remove FE-generated
	GOMP_MAP_TO_PSET and GOMP_MAP_POINTER mapping for 'target update'
	and 'target exit data'.

	libgomp/
	* testsuite/libgomp.fortran/target9.f90: New.

diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index fdf6b695003..12ed3f8eb21 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -8590,6 +8590,17 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p,
 	default:
 	  break;
 	}
+	  /* For Fortran, not only the pointer to the data is mapped but also
+	 the address of the pointer, the array descriptor etc.; for
+	 'exit data' - and in particular for 'delete:' - having an 'alloc:'
+	 does not make sense.  Likewise, for 'update' only transferring the
+	 data itself is needed as the rest has been handled in previous
+	 directives.  */
+	  if ((code == OMP_TARGET_EXIT_DATA || code == OMP_TARGET_UPDATE)
+	  && (OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_POINTER
+		  || OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_TO_PSET))
+	remove = true;
+
 	  if (remove)
 	break;
 	  if (DECL_P (decl) && outer_ctx && (region_type & ORT_ACC))
diff --git a/libgomp/testsuite/libgomp.fortran/target9.f90 b/libgomp/testsuite/libgomp.fortran/target9.f90
new file mode 100644
index 000..91d60a33307
--- /dev/null
+++ b/libgomp/testsuite/libgomp.fortran/target9.f90
@@ -0,0 +1,123 @@
+! { dg-require-effective-target offload_device_nonshared_as } */
+
+module target_test
+  implicit none (type, external)
+  integer, parameter :: N = 40
+  integer :: sum
+  integer :: var1 = 1
+  integer :: var2 = 2
+
+  !$omp declare target to(D)
+  integer :: D(N) = 0
+contains
+  subroutine enter_data (X)
+integer :: X(:)
+!$omp target enter data map(to: var1, var2, X) map(alloc: sum)
+  end subroutine enter_data
+
+  subroutine exit_data_0 (D)
+integer :: D(N)
+!$omp target exit data map(delete: D)
+  end subroutine exit_data_0
+
+  subroutine exit_data_1 ()
+!$omp target exit data map(from: var1)
+  end subroutine exit_data_1
+
+  subroutine exit_data_2 (X)
+integer :: X(N)
+!$omp target exit data map(from: var2) map(release: X, sum)
+  end subroutine exit_data_2
+
+  subroutine exit_data_3 (p, idx)
+integer :: p(:)
+integer, value :: idx
+!$omp target exit data map(from: p(idx))
+  end subroutine exit_data_3
+
+  subroutine test_nested ()
+integer :: X, Y, Z
+X = 0
+Y = 0
+Z = 0
+
+!$omp target data map(from: X, Y, Z)
+  !$omp target data map(from: X, Y, Z)
+!$omp target map(from: X, Y, Z)
+  X = 1337
+  Y = 1337
+  Z = 1337
+!$omp end target
+if (X /= 0) stop 11
+if (Y /= 0) stop 12
+if (Z /= 0) stop 13
+
+!$omp target exit data map(from: X) map(release: Y)
+if (X /= 0) stop 14
+if (Y /= 0) stop 15
+
+!$omp target exit data map(release: Y) map(delete: Z)
+if (Y /= 0) stop 16
+if (Z /= 0) stop 17
+  !$omp end target data
+  if (X /= 1337) stop 18
+  if (Y /= 0) stop 19
+  if (Z /= 0) stop 20
+
+  !$omp target map(from: X)
+X = 2448
+  !$omp end target
+  if (X /= 2448) stop 21
+  if (Y /= 0) stop 22
+  if (Z /= 0) stop 23
+
+  X = 4896
+!$omp end target data
+if (X /= 4896) stop 24
+if (Y /= 0) stop 25
+if (Z /= 0) stop 26
+  end subroutine test_nested
+end module target_test
+
+program main
+  use target_test
+  implicit none (type, 

[PATCH] Apply C++20 changes to various iterator types

[Jonathan Wakely]

This ensures that __normal_iterator satisfies the
contiguous_iterator concept, by defining the iterator_concept member
type.

Also update vector's iterators, reverse_iterator,
istreambuf_iterator and ostreambuf_iterator to meet the C++20
requirements.

PR libstdc++/92272
* include/bits/stl_bvector.h (_Bit_iterator::pointer)
(_Bit_const_iterator::pointer): Define as void for C++20.
* include/bits/stl_iterator.h (reverse_iterator::operator->()): Add
constraints for C++20.
(__normal_iterator::iterator_concept): Define for C++20.
* include/bits/streambuf_iterator.h (istreambuf_iterator::pointer):
Define as void for C++20.
(ostreambuf_iterator::difference_type): Define as ptrdiff_t for C++20.
(ostreambuf_iterator::ostreambuf_iterator()): Add default constructor
for C++20.
* testsuite/23_containers/vector/bool/iterator_c++20.cc: New test.
* testsuite/24_iterators/bidirectional/concept.cc: New test.
* testsuite/24_iterators/bidirectional/tag.cc: New test.
* testsuite/24_iterators/contiguous/concept.cc: New test.
* testsuite/24_iterators/contiguous/tag.cc: New test.
* testsuite/24_iterators/forward/concept.cc: New test.
* testsuite/24_iterators/forward/tag.cc: New test.
* testsuite/24_iterators/input/concept.cc: New test.
* testsuite/24_iterators/input/tag.cc: New test.
* testsuite/24_iterators/istreambuf_iterator/requirements/typedefs.cc:
New test.
* testsuite/24_iterators/ostreambuf_iterator/requirements/typedefs.cc:
New test.
* testsuite/24_iterators/output/concept.cc: New test.
* testsuite/24_iterators/output/tag.cc: New test.
* testsuite/24_iterators/random_access/concept.cc: New test.
* testsuite/24_iterators/random_access/tag.cc: New test.
* testsuite/24_iterators/range_operations/advance_debug_neg.cc: New
test.
* testsuite/24_iterators/random_access_iterator/26020.cc: Move to ...
* testsuite/24_iterators/operations/26020.cc: ... here.
* testsuite/24_iterators/random_access_iterator/
string_vector_iterators.cc: Move to ...
* testsuite/24_iterators/random_access/string_vector_iterators.cc: ...
here.

Tested powerpc64le-linux, committed to trunk.

commit 937b42d1b4b0b90da218b63608100f13725d19b7
Author: Jonathan Wakely 
Date:   Wed Oct 30 14:31:35 2019 +

Apply C++20 changes to various iterator types

This ensures that __normal_iterator satisfies the
contiguous_iterator concept, by defining the iterator_concept member
type.

Also update vector's iterators, reverse_iterator,
istreambuf_iterator and ostreambuf_iterator to meet the C++20
requirements.

PR libstdc++/92272
* include/bits/stl_bvector.h (_Bit_iterator::pointer)
(_Bit_const_iterator::pointer): Define as void for C++20.
* include/bits/stl_iterator.h (reverse_iterator::operator->()): Add
constraints for C++20.
(__normal_iterator::iterator_concept): Define for C++20.
* include/bits/streambuf_iterator.h (istreambuf_iterator::pointer):
Define as void for C++20.
(ostreambuf_iterator::difference_type): Define as ptrdiff_t for 
C++20.
(ostreambuf_iterator::ostreambuf_iterator()): Add default 
constructor
for C++20.
* testsuite/23_containers/vector/bool/iterator_c++20.cc: New test.
* testsuite/24_iterators/bidirectional/concept.cc: New test.
* testsuite/24_iterators/bidirectional/tag.cc: New test.
* testsuite/24_iterators/contiguous/concept.cc: New test.
* testsuite/24_iterators/contiguous/tag.cc: New test.
* testsuite/24_iterators/forward/concept.cc: New test.
* testsuite/24_iterators/forward/tag.cc: New test.
* testsuite/24_iterators/input/concept.cc: New test.
* testsuite/24_iterators/input/tag.cc: New test.
* 
testsuite/24_iterators/istreambuf_iterator/requirements/typedefs.cc:
New test.
* 
testsuite/24_iterators/ostreambuf_iterator/requirements/typedefs.cc:
New test.
* testsuite/24_iterators/output/concept.cc: New test.
* testsuite/24_iterators/output/tag.cc: New test.
* testsuite/24_iterators/random_access/concept.cc: New test.
* testsuite/24_iterators/random_access/tag.cc: New test.
* testsuite/24_iterators/range_operations/advance_debug_neg.cc: New
test.
* testsuite/24_iterators/random_access_iterator/26020.cc: Move to 
...
* testsuite/24_iterators/operations/26020.cc: ... here.
* testsuite/24_iterators/random_access_iterator/
string_vector_iterators.cc: Move to ...
* 

[PATCH] Remove some more using-declarations from namespace __gnu_cxx

[Jonathan Wakely]

Similar to some recent patches, this removes using-declarations for
names from namespace std, so that they are not redeclared in __gnu_cxx.

* include/bits/stl_iterator.h (namespace __gnu_cxx): Remove
using-declarations for std::iterator and std::iterator_traits.
(__gnu_cxx::__normal_iterator): Qualify iterator_traits.
* include/ext/algorithm (namespace __gnu_cxx): Remove
using-declarations for std names and qualify those names when used.
Also refer to std::min in parentheses to protect against function-like
macros.
* include/ext/rc_string_base.h: Qualify iterator_traits.
* include/ext/sso_string_base.h: Qualify iterator_traits.

Tested powerpc64le-linux, committed to trunk.

commit 0c978d0446b42573a5201b3b5f1b6b9f42160133
Author: Jonathan Wakely 
Date:   Wed Oct 30 15:31:25 2019 +

Remove some more using-declarations from namespace __gnu_cxx

Similar to some recent patches, this removes using-declarations for
names from namespace std, so that they are not redeclared in __gnu_cxx.

* include/bits/stl_iterator.h (namespace __gnu_cxx): Remove
using-declarations for std::iterator and std::iterator_traits.
(__gnu_cxx::__normal_iterator): Qualify iterator_traits.
* include/ext/algorithm (namespace __gnu_cxx): Remove
using-declarations for std names and qualify those names when used.
Also refer to std::min in parentheses to protect against 
function-like
macros.
* include/ext/rc_string_base.h: Qualify iterator_traits.
* include/ext/sso_string_base.h: Qualify iterator_traits.

diff --git a/libstdc++-v3/include/bits/stl_iterator.h 
b/libstdc++-v3/include/bits/stl_iterator.h
index 2a3b0231079..ecc06178c34 100644
--- a/libstdc++-v3/include/bits/stl_iterator.h
+++ b/libstdc++-v3/include/bits/stl_iterator.h
@@ -793,15 +793,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   // The _Container parameter exists solely so that different containers
   // using this template can instantiate different types, even if the
   // _Iterator parameter is the same.
-  using std::iterator_traits;
-  using std::iterator;
   template
 class __normal_iterator
 {
 protected:
   _Iterator _M_current;
 
-  typedef iterator_traits<_Iterator>   __traits_type;
+  typedef std::iterator_traits<_Iterator>  __traits_type;
 
 public:
   typedef _Iteratoriterator_type;
diff --git a/libstdc++-v3/include/ext/algorithm 
b/libstdc++-v3/include/ext/algorithm
index 2970b7d8dfe..ec244d91860 100644
--- a/libstdc++-v3/include/ext/algorithm
+++ b/libstdc++-v3/include/ext/algorithm
@@ -64,21 +64,14 @@ namespace __gnu_cxx _GLIBCXX_VISIBILITY(default)
 {
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
-  using std::ptrdiff_t;
-  using std::min;
-  using std::pair;
-  using std::input_iterator_tag;
-  using std::random_access_iterator_tag;
-  using std::iterator_traits;
-
   //--
   // copy_n (not part of the C++ standard)
 
   template
-pair<_InputIterator, _OutputIterator>
+std::pair<_InputIterator, _OutputIterator>
 __copy_n(_InputIterator __first, _Size __count,
 _OutputIterator __result,
-input_iterator_tag)
+std::input_iterator_tag)
 {
   for ( ; __count > 0; --__count)
{
@@ -86,17 +79,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
  ++__first;
  ++__result;
}
-  return pair<_InputIterator, _OutputIterator>(__first, __result);
+  return std::pair<_InputIterator, _OutputIterator>(__first, __result);
 }
 
   template
-inline pair<_RAIterator, _OutputIterator>
+inline std::pair<_RAIterator, _OutputIterator>
 __copy_n(_RAIterator __first, _Size __count,
 _OutputIterator __result,
-random_access_iterator_tag)
+std::random_access_iterator_tag)
 {
   _RAIterator __last = __first + __count;
-  return pair<_RAIterator, _OutputIterator>(__last, std::copy(__first,
+  return std::pair<_RAIterator, _OutputIterator>(__last, std::copy(__first,
  __last,
  __result));
 }
@@ -116,13 +109,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
*  @ingroup SGIextensions
   */
   template
-inline pair<_InputIterator, _OutputIterator>
+inline std::pair<_InputIterator, _OutputIterator>
 copy_n(_InputIterator __first, _Size __count, _OutputIterator __result)
 {
   // concept requirements
   __glibcxx_function_requires(_InputIteratorConcept<_InputIterator>)
   __glibcxx_function_requires(_OutputIteratorConcept<_OutputIterator,
-   typename iterator_traits<_InputIterator>::value_type>)
+   typename std::iterator_traits<_InputIterator>::value_type>)
 

[Patch, Fortran] PR92277 - Fix assumed-rank array with bind(C)

[Tobias Burnus]

The attached test case (w/o optional and with "this") gave an ICE as 
"*this" was passed to DECL_ARTIFICIAL and only "this" but not the 
INDIRECT_REF is a declaration. [I added optional as those often act 
slightly different, but it doesn't seem to make a difference here.]


Solution: If it is an INDIRECT_REF, check the the arg0 of it.

Note: The INDIRECT_REF not only comes about via the 
build_fold_indirect_ref_loc call in the line above "is_artificial" but 
can be produced earlier. An example is this test case.


(This is GCC 10 regression, only.)
OK for the trunk?

Tobias

PS: Review is also still pending for 
https://gcc.gnu.org/ml/fortran/2019-10/msg00237.html


	gcc/fortran/
	* trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Fix DECL_ARTIFICIAL
	checking.

	gcc/testsuite/
	* fortran.dg/pr92277.f90: New.

diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c
index 7eba1bbd082..381e314a9b5 100644
--- a/gcc/fortran/trans-expr.c
+++ b/gcc/fortran/trans-expr.c
@@ -5239,6 +5239,9 @@ gfc_conv_gfc_desc_to_cfi_desc (gfc_se *parmse, gfc_expr *e, gfc_symbol *fsym)
   if (POINTER_TYPE_P (TREE_TYPE (parmse->expr)))
 	parmse->expr = build_fold_indirect_ref_loc (input_location,
 		parmse->expr);
+  bool is_artificial = (INDIRECT_REF_P (parmse->expr)
+			? DECL_ARTIFICIAL (TREE_OPERAND (parmse->expr, 0))
+			: DECL_ARTIFICIAL (parmse->expr));
 
   /* Unallocated allocatable arrays and unassociated pointer arrays
 	 need their dtype setting if they are argument associated with
@@ -5258,7 +5261,7 @@ gfc_conv_gfc_desc_to_cfi_desc (gfc_se *parmse, gfc_expr *e, gfc_symbol *fsym)
   type = e->ts.type != BT_ASSUMED ? gfc_typenode_for_spec (>ts) :
 	NULL_TREE;
 
-  if (type && DECL_ARTIFICIAL (parmse->expr)
+  if (type && is_artificial
 	  && type != gfc_get_element_type (TREE_TYPE (parmse->expr)))
 	{
 	  /* Obtain the offset to the data.  */
@@ -5271,7 +5274,7 @@ gfc_conv_gfc_desc_to_cfi_desc (gfc_se *parmse, gfc_expr *e, gfc_symbol *fsym)
 			  gfc_get_dtype_rank_type (e->rank, type));
 	}
   else if (type == NULL_TREE
-	   || (!is_subref_array (e) && !DECL_ARTIFICIAL (parmse->expr)))
+	   || (!is_subref_array (e) && !is_artificial))
 	{
 	  /* Make sure that the span is set for expressions where it
 	 might not have been done already.  */
diff --git a/gcc/testsuite/gfortran.dg/pr92277.f90 b/gcc/testsuite/gfortran.dg/pr92277.f90
new file mode 100644
index 000..5121063f5f3
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr92277.f90
@@ -0,0 +1,32 @@
+! { dg-do compile }
+!
+! PR fortran/92277
+!
+! Contributed by José Rui Faustino de Sousa
+!
+module arr_m
+  implicit none
+contains
+  subroutine arr_set(this, that)
+integer, intent(out) :: this(..)
+integer, optional, intent(out) :: that(..)
+
+interface
+  subroutine arr_set_c(this) bind(c)
+use, intrinsic :: iso_c_binding, only: c_int
+implicit none
+integer(kind=c_int), intent(out) :: this(..)
+  end subroutine arr_set_c
+  subroutine arr_set_c_opt(this) bind(c)
+use, intrinsic :: iso_c_binding, only: c_int
+implicit none
+integer(kind=c_int), optional, intent(out) :: this(..)
+  end subroutine arr_set_c_opt
+end interface
+
+call arr_set_c(this)
+call arr_set_c(that)
+call arr_set_c_opt(this)
+call arr_set_c_opt(that)
+  end subroutine arr_set
+end module arr_m

Re: [SLP] SLP vectorization: vectorize vector constructors

[Joel Hutton]

On 30/10/2019 14:51, Richard Biener wrote:
> On Wed, 30 Oct 2019, Joel Hutton wrote:
>
>> On 30/10/2019 13:49, Richard Biener wrote:
>>> Why do you need this?  The vectorizer already creates such CTORs.  Any
>>> testcase that you can show?
>> typedef long v2di __attribute__((vector_size(16)));
>> v2di v;
>> void
>> foo()
>> {
>>     v = (v2di){v[1], v[0]};
>> }
> Huh.  On what architecture?  Is that for a V2DI constructor of
> V1DI vectors maybe?  On x86 the code doesn't trigger.

On aarch64.

emode = E_DImode

eltmode = E_DImode


> The condition was indeed to check for vector mode elements so
> maybe it should simply read
>
>if (VECTOR_MODE_P (emode))
>
> ?  eltmode is always scalar.
I'll try that, thanks.

Re: [PATCH] [MIPS] Mark built-in functions as pure

[Mihailo Stojanović]

Hello Jeff,

I already have write access on this e-mail address (but not on
the @wavecomp.com address, which you have been putting
into ChangeLogs), so I guess I could commit any further patches
that get approved.

Regards,
Mihailo

[PATCH] Come up with ggc_delete.

[Martin Liška]

Hi.

There's a small code refactoring.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin
From dc92c8c3e31887b23ef419bc60e3c1607d0e9e53 Mon Sep 17 00:00:00 2001
From: Martin Liska 
Date: Wed, 30 Oct 2019 12:50:51 +0100
Subject: [PATCH] Come up with ggc_delete.

gcc/ChangeLog:

2019-10-30  Martin Liska  

	* ggc.h (ggc_delete): New function.
	* ipa-fnsummary.c (ipa_free_fn_summary): Use it.
	* ipa-prop.c (ipa_free_all_edge_args): Likewise.
	(ipa_free_all_node_params): Likewise.
	* ipa-sra.c (ipa_sra_analysis): Likewise.
---
 gcc/ggc.h   | 10 ++
 gcc/ipa-fnsummary.c |  3 +--
 gcc/ipa-prop.c  |  6 ++
 gcc/ipa-sra.c   |  3 +--
 4 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/gcc/ggc.h b/gcc/ggc.h
index d5735d09ced..3e661ae730d 100644
--- a/gcc/ggc.h
+++ b/gcc/ggc.h
@@ -242,6 +242,16 @@ ggc_alloc_atomic (size_t s CXX_MEM_STAT_INFO)
 return ggc_internal_alloc (s PASS_MEM_STAT);
 }
 
+/* Call destructor and free the garbage collected memory.  */
+
+template 
+inline void
+ggc_delete (T *ptr)
+{
+  ptr->~T ();
+  ggc_free (ptr);
+}
+
 /* Allocate a gc-able string, and fill it with LENGTH bytes from CONTENTS.
If LENGTH is -1, then CONTENTS is assumed to be a
null-terminated string and the memory sized accordingly.  */
diff --git a/gcc/ipa-fnsummary.c b/gcc/ipa-fnsummary.c
index 5eee2416dd7..8f20d7e6b2a 100644
--- a/gcc/ipa-fnsummary.c
+++ b/gcc/ipa-fnsummary.c
@@ -3873,8 +3873,7 @@ ipa_free_fn_summary (void)
 {
   if (!ipa_call_summaries)
 return;
-  ipa_fn_summaries->~fast_function_summary  ();
-  ggc_free (ipa_fn_summaries);
+  ggc_delete (ipa_fn_summaries);
   ipa_fn_summaries = NULL;
   delete ipa_call_summaries;
   ipa_call_summaries = NULL;
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 336d271874d..91ac0fe7f4f 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -3740,8 +3740,7 @@ ipa_free_all_edge_args (void)
   if (!ipa_edge_args_sum)
 return;
 
-  ipa_edge_args_sum->~ipa_edge_args_sum_t ();
-  ggc_free (ipa_edge_args_sum);
+  ggc_delete (ipa_edge_args_sum);
   ipa_edge_args_sum = NULL;
 }
 
@@ -3750,8 +3749,7 @@ ipa_free_all_edge_args (void)
 void
 ipa_free_all_node_params (void)
 {
-  ipa_node_params_sum->~ipa_node_params_t ();
-  ggc_free (ipa_node_params_sum);
+  ggc_delete (ipa_node_params_sum);
   ipa_node_params_sum = NULL;
 }
 
diff --git a/gcc/ipa-sra.c b/gcc/ipa-sra.c
index 8f028438556..7367441b105 100644
--- a/gcc/ipa-sra.c
+++ b/gcc/ipa-sra.c
@@ -3989,8 +3989,7 @@ ipa_sra_analysis (void)
 process_isra_node_results (node, clone_num_suffixes);
 
   delete clone_num_suffixes;
-  func_sums->~ipa_sra_function_summaries ();
-  ggc_free (func_sums);
+  ggc_delete (func_sums);
   func_sums = NULL;
   delete call_sums;
   call_sums = NULL;
-- 
2.23.0



pEpkey.asc
Description: application/pgp-keys

Re: [SLP] SLP vectorization: vectorize vector constructors

[Richard Biener]

On Wed, 30 Oct 2019, Joel Hutton wrote:

> On 30/10/2019 13:49, Richard Biener wrote:
> > Why do you need this?  The vectorizer already creates such CTORs.  Any
> > testcase that you can show?
> 
> typedef long v2di __attribute__((vector_size(16)));
> v2di v;
> void
> foo()
> {
>    v = (v2di){v[1], v[0]};
> }

Huh.  On what architecture?  Is that for a V2DI constructor of
V1DI vectors maybe?  On x86 the code doesn't trigger.

The condition was indeed to check for vector mode elements so
maybe it should simply read

  if (VECTOR_MODE_P (emode))

?  eltmode is always scalar.

> 
> >>   * tree-vect-slp.c (vect_analyze_slp_instance): Add case for 
> >> vector constructors.
> >>   (vect_bb_slp_scalar_cost): Likewise.
> >>   (vect_slp_check_for_constructors): New function.
> >>   (vect_slp_analyze_bb_1): Add check for vector constructors.
> >>   (vect_schedule_slp_instance): Add case to fixup vector 
> >> constructor stmt.
> >>   (vectorize_slp_instance_root_stmt): New function
> >>   * tree-vectorizer.h (SLP_INSTANCE_ROOT_STMT): New field.
> > + SLP_INSTANCE_ROOT_STMT (new_instance) = constructor ?
> > stmt_info->stmt\
> > + : NULL;
> >
> > SLP_INSTANCE_ROOT_STMT should be a stmt_vec_info I guess...
> >
> > @@ -2801,6 +2830,10 @@ vect_bb_slp_scalar_cost (basic_block bb,
> >  stmt_vec_info use_stmt_info = vinfo->lookup_stmt
> > (use_stmt);
> >  if (!use_stmt_info || !PURE_SLP_STMT (use_stmt_info))
> >{
> > +   /* Check this is not a constructor that will be
> > vectorized
> > +  away.  */
> > +   if (BB_VINFO_GROUPED_STORES (vinfo).contains
> > (use_stmt_info))
> > +   continue;
> >  (*life)[i] = true;
> >
> > ... which you then simply mark as PURE_SLP_STMT where we call
> > vect_mark_slp_stmts in vect_slp_analyze_bb_1.
> >
> > I see you have the TYPE_VECTOR_SUBPARTS check still at transform
> > stage in vectorize_slp_instance_root_stmt, please simply move
> > it to vect_slp_check_for_constructors or to vect_analyze_slp_instance
> > where you have the other rejects (non-SSA names in the ctor).
> If the check is in vect_slp_check_for_constructors, which vector is used 
> as the input to tYPE_VECTOR_SUBPARTS, the lhs of the constructor?

The type of the constructor itself.

> > That is, vectorize_slp_instance_root_stmt may not fail.
> >
> > +bool
> > +vectorize_slp_instance_root_stmt (slp_tree node, slp_instance instance)
> > +{
> > +
> >
> > extra newline
> >
> > +  if (SLP_TREE_NUMBER_OF_VEC_STMTS (node) == 1)
> >
> > the stmt replacing can be commonized between == 1 and > 1 cases.
> >
> > FOR_EACH_VEC_ELT (slp_instances, i, instance)
> >   {
> > +  slp_tree node = SLP_INSTANCE_TREE (instance);
> > /* Schedule the tree of INSTANCE.  */
> > -  vect_schedule_slp_instance (SLP_INSTANCE_TREE (instance),
> > +  vect_schedule_slp_instance (node,
> >instance, bst_map);
> > +
> > +  /* Vectorize the instance root.  */
> > +  if (instance->root == node && SLP_INSTANCE_ROOT_STMT (instance)
> > + && SLP_TREE_VEC_STMTS (node).exists ())
> > +   if (!vectorize_slp_instance_root_stmt (node, instance))
> > + {
> >
> > instance->root == node is always true.  Likewise
> > SLP_TREE_VEC_STMTS (node).exists ().
> >
> > @@ -4061,15 +4201,42 @@ vect_schedule_slp (vec_info *vinfo)
> > if (is_a  (vinfo))
> >
> > the ChangeLog doesn't mention this.  I guess this isn't necessary
> > unless you fail vectorizing late which you shouldn't.
> >
> > diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h
> > index
> > 56be28b0cc5a77412f996e70636b08d5b615813e..9f8419e4208b7d438ace41892022f93ebcadd019
> > 100644
> > --- a/gcc/tree-vectorizer.h
> > +++ b/gcc/tree-vectorizer.h
> > @@ -151,6 +151,10 @@ public:
> > /* The root of SLP tree.  */
> > slp_tree root;
> >
> > +  /* For vector constructors, the constructor stmt that the SLP tree is
> > built
> > + from, NULL otherwise.  */
> > +  gimple *root_stmt;
> >
> > as said, make this a stmt_vec_info
> >
> > Thanks,
> > Richard.
> >
> >
> >> gcc/testsuite/ChangeLog:
> >>
> >> 2019-10-10  Joel Hutton  
> >>
> >>   * gcc.dg/vect/bb-slp-40.c: New test.
> >>   * gcc.dg/vect/bb-slp-41.c: New test.
> >>
> >>
> 

-- 
Richard Biener 
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

Re: [RFC PATCH] targetm.omp.device_kind_arch_isa and OpenMP declare variant kind/arch/isa handling

[Jakub Jelinek]

On Wed, Oct 30, 2019 at 02:12:30PM +, Szabolcs Nagy wrote:
> On 29/10/2019 17:15, Jakub Jelinek wrote:
> > +void f03 (void);
> > +#pragma omp declare variant (f03) match 
> > (device={kind(any),arch(x86_64),isa(avx512f,avx512bw)})
> > +void f04 (void);
> 
> 1) it's not clear from the omp spec what is the intended
> syntax for isa-name, arch-name and extension-name, but
> i expected strings in "".

Yes, it is indeed not clear, subject to ongoing discussions.
My reading of the spec was that all the *-name-list are comma
separated lists of identifiers, some others in the committee
want now (yesterday's discussions) string literals instead
when I and others pointed out that isa(core-avx512) can't be valid,
but strangely only for isa/arch/extension but not e.g. for
kind or vendor which would still take identifier lists etc.
My preference at this point would be probably to allow
in all vendor/kind/arch/isa/extension lists of either
identifiers or string literals, so for names which don't contain
characters invalid in identifiers users could choose what to write,
so both isa(avx512f,avx512bw) and isa("avx512f","avx512bw")
would be valid (and so would be isa(avx512f,"avx512bw")), obviously
for something that is not a valid identifier users wouldn't have a choice,
armv8.2-a+sve is not an identifier.

> what if an arch or isa name contains ',' ')' etc?
> 
> we were planing to use things like
> 
> isa("sve")
> arch("armv8.2-a+sve")
> extension("scalable")

That is subject to yet another ongoing discussion in the committee,
what shall be the meaning of isa and what shall be the meaning of arch,
I think we need something that will hold the GCC target or something similar
and something that holds the instruction sets for that target.
extension, given it is in the implementation trait set, is IMNSHO not meant
to hold device extensions, but rather software extensions or something
similar.
ARM is an OpenMP member, so if you want, you can participate too.
https://github.com/OpenMP/spec/issues/2028
is where I'm trying to track all the declare variant issues that need
clarification (plus in two examples tickets).

> 2) does f03 need to be declared before the declare variant
> pragma appears?

Yes.  For C++ the spec says that the actual function declaration
is determined by what would be called at the point of the pragma with
the given id-expression and arguments with types from the function
declaration on which the pragma is used, so it can involve ADL, needs to
deal with function overloading etc.

> for simd variants it means i need to declare the function
> with the right simd types and attributes.

For simd it is actually not finished yet, what needs to be done is that
given the declare simd clauses used as properties of the simd selector
the FEs use some target hook that will guide it how to transform
the parameters like targetm.simd_clone.compute_vecsize_and_simdlen
does and for C tries to just match the types against it and determine
through that the ABI and perhaps missing simd clauses like
notinbranch/inbranch, simdlen etc., for C++ actually for each possibility
will try to construct a call with such arguments and then compare the types.

I'd like to make non-simd working first though, for some cases it already
works and replacement is done in the gimplifier, but scoring needs to be
added, then some way to keep such info in the cgraph (will need to talk to
Honza) and after IPA perform another attempt to redirect.

Jakub

Re: [8/n] Replace autovectorize_vector_sizes with autovectorize_vector_modes

[Richard Biener]

On Fri, Oct 25, 2019 at 2:37 PM Richard Sandiford
 wrote:
>
> This is another patch in the series to remove the assumption that
> all modes involved in vectorisation have to be the same size.
> Rather than have the target provide a list of vector sizes,
> it makes the target provide a list of vector "approaches",
> with each approach represented by a mode.
>
> A later patch will pass this mode to targetm.vectorize.related_mode
> to get the vector mode for a given element mode.  Until then, the modes
> simply act as an alternative way of specifying the vector size.

Is there a restriction to use integer vector modes for the hook
or would FP vector modes be OK as well?  Note that your
x86 change likely disables word_mode vectorization with -mno-sse?

That is, how do we represent GPR vectorization "size" here?
The preferred SIMD mode hook may return an integer mode,
are non-vector modes OK for autovectorize_vector_modes?

Thanks,
Richard.
>
> 2019-10-24  Richard Sandiford  
>
> gcc/
> * target.h (vector_sizes, auto_vector_sizes): Delete.
> (vector_modes, auto_vector_modes): New typedefs.
> * target.def (autovectorize_vector_sizes): Replace with...
> (autovectorize_vector_modes): ...this new hook.
> * doc/tm.texi.in (TARGET_VECTORIZE_AUTOVECTORIZE_VECTOR_SIZES):
> Replace with...
> (TARGET_VECTORIZE_AUTOVECTORIZE_VECTOR_MODES): ...this new hook.
> * doc/tm.texi: Regenerate.
> * targhooks.h (default_autovectorize_vector_sizes): Delete.
> (default_autovectorize_vector_modes): New function.
> * targhooks.c (default_autovectorize_vector_sizes): Delete.
> (default_autovectorize_vector_modes): New function.
> * omp-general.c (omp_max_vf): Use autovectorize_vector_modes instead
> of autovectorize_vector_sizes.  Use the number of units in the mode
> to calculate the maximum VF.
> * omp-low.c (omp_clause_aligned_alignment): Use
> autovectorize_vector_modes instead of autovectorize_vector_sizes.
> Use a loop based on related_mode to iterate through all supported
> vector modes for a given scalar mode.
> * optabs-query.c (can_vec_mask_load_store_p): Use
> autovectorize_vector_modes instead of autovectorize_vector_sizes.
> * tree-vect-loop.c (vect_analyze_loop, vect_transform_loop): Likewise.
> * tree-vect-slp.c (vect_slp_bb_region): Likewise.
> * config/aarch64/aarch64.c (aarch64_autovectorize_vector_sizes):
> Replace with...
> (aarch64_autovectorize_vector_modes): ...this new function.
> (TARGET_VECTORIZE_AUTOVECTORIZE_VECTOR_SIZES): Delete.
> (TARGET_VECTORIZE_AUTOVECTORIZE_VECTOR_MODES): Define.
> * config/arc/arc.c (arc_autovectorize_vector_sizes): Replace with...
> (arc_autovectorize_vector_modes): ...this new function.
> (TARGET_VECTORIZE_AUTOVECTORIZE_VECTOR_SIZES): Delete.
> (TARGET_VECTORIZE_AUTOVECTORIZE_VECTOR_MODES): Define.
> * config/arm/arm.c (arm_autovectorize_vector_sizes): Replace with...
> (arm_autovectorize_vector_modes): ...this new function.
> (TARGET_VECTORIZE_AUTOVECTORIZE_VECTOR_SIZES): Delete.
> (TARGET_VECTORIZE_AUTOVECTORIZE_VECTOR_MODES): Define.
> * config/i386/i386.c (ix86_autovectorize_vector_sizes): Replace 
> with...
> (ix86_autovectorize_vector_modes): ...this new function.
> (TARGET_VECTORIZE_AUTOVECTORIZE_VECTOR_SIZES): Delete.
> (TARGET_VECTORIZE_AUTOVECTORIZE_VECTOR_MODES): Define.
> * config/mips/mips.c (mips_autovectorize_vector_sizes): Replace 
> with...
> (mips_autovectorize_vector_modes): ...this new function.
> (TARGET_VECTORIZE_AUTOVECTORIZE_VECTOR_SIZES): Delete.
> (TARGET_VECTORIZE_AUTOVECTORIZE_VECTOR_MODES): Define.
>
> Index: gcc/target.h
> ===
> --- gcc/target.h2019-09-30 17:19:39.843166118 +0100
> +++ gcc/target.h2019-10-25 13:27:15.525762975 +0100
> @@ -205,11 +205,11 @@ enum vect_cost_model_location {
>  class vec_perm_indices;
>
>  /* The type to use for lists of vector sizes.  */
> -typedef vec vector_sizes;
> +typedef vec vector_modes;
>
>  /* Same, but can be used to construct local lists that are
> automatically freed.  */
> -typedef auto_vec auto_vector_sizes;
> +typedef auto_vec auto_vector_modes;
>
>  /* The target structure.  This holds all the backend hooks.  */
>  #define DEFHOOKPOD(NAME, DOC, TYPE, INIT) TYPE NAME;
> Index: gcc/target.def
> ===
> --- gcc/target.def  2019-10-25 13:26:59.309877555 +0100
> +++ gcc/target.def  2019-10-25 13:27:15.525762975 +0100
> @@ -1894,20 +1894,28 @@ reached.  The default is @var{mode} whic
>  /* Returns a mask of vector sizes to iterate over when auto-vectorizing
> after processing the preferred one derived from 

Re: [committed][AArch64] Add support for the SVE PCS

[Andreas Schwab]

Same problem in the go frontend.

(gdb) r
Starting program: /opt/gcc/test/Build/gcc/go1 
../../../libgo/go/sync/atomic/doc.go ../../../libgo/go/sync/atomic/value.go 
-quiet -dumpbase doc.go -mlittle-endian -mabi=lp64 -auxbase-strip doc.o -g -O2 
-fgo-pkgpath=sync/atomic -I . -L/opt/gcc/test/Build/./gcc 
-L/usr/aarch64-suse-linux/bin -L/usr/aarch64-suse-linux/lib -L/lib/../lib64 
-L/usr/lib/../lib64

Program received signal SIGSEGV, Segmentation fault.
aarch64_sve::svbool_type_p (type=)
at ../../gcc/config/aarch64/aarch64-sve-builtins.cc:3239
3239  && TYPE_MAIN_VARIANT (type) == TYPE_MAIN_VARIANT (abi_type));

Andreas.

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

Re: [PATCH] Minor improvements to testsuite iterator utilities

[Jonathan Wakely]

On 29/10/19 17:15 +, Jonathan Wakely wrote:

* testsuite/util/testsuite_iterators.h (BoundsContainer::size()): Add
new member function.
(WritableObject::operator=): Constrain with enable_if when available.
(remove_cv): Use std::remove_if when available.
(test_container::it(int)): Use size().
(test_container::size()): Use BoundsContainer::size().

Tested powerpc64le-linux, committed to trunk.




@@ -182,10 +184,14 @@ namespace __gnu_test
void operator,(const T&, const output_iterator_wrapper&) = delete;
#endif

+#if __cplusplus >= 2011L


Oops! That should be 201103L.

Fixed by the attached patch, which I'll commit shortly.

commit 069df87c7e936d568a142df3930c700306546acc
Author: Jonathan Wakely 
Date:   Wed Oct 30 14:32:53 2019 +

Fix typo in preprocessor check

* testsuite/util/testsuite_iterators.h: Fix typo in __cplusplus check.

diff --git a/libstdc++-v3/testsuite/util/testsuite_iterators.h b/libstdc++-v3/testsuite/util/testsuite_iterators.h
index 70c1f9b6689..c5ae5b123fe 100644
--- a/libstdc++-v3/testsuite/util/testsuite_iterators.h
+++ b/libstdc++-v3/testsuite/util/testsuite_iterators.h
@@ -184,7 +184,7 @@ namespace __gnu_test
 void operator,(const T&, const output_iterator_wrapper&) = delete;
 #endif
 
-#if __cplusplus >= 2011L
+#if __cplusplus >= 201103L
   using std::remove_cv;
 #else
   template struct remove_cv { typedef T type; };

Re: [PATCH v2] PR85678: Change default to -fno-common

[Wilco Dijkstra]

Hi Richard,

> Please don't add -fcommon in lto.exp.

So what is the best way to add an extra option to lto.exp?
Note dg-lto-options completely overrides the options from lto.exp, so I can't
use that except in tests which already use it.

Cheers,
Wilco

Re: Watch for missing summaries even more

[Martin Jambor]

Hi again, now also CCing the mailing list,

 On Wed, Oct 30 2019, Jan Hubicka wrote:
>> Hi,
>> 
>> On Wed, Oct 30 2019, Jan Hubicka wrote:
>> > Hi,
>> > this patch fixes another place we may have missing argument summary.
>> > Here the situation is that the call site being inlined has no jump
>> > functions while function which is being inlines has another call with
>> > jump function.  This can validly happen when we inline into functions
>> > with indirect inlining and ipa-cp disabled but I am not 100% why it
>> > happens i.e. during Firefox builds.  Martin, do you have any ideas?
>> 
>> No, not without seeing what is going on.  I think we compute jump
>> functions also when we are inlining and IPA-CP is disabled, by the way.
>> 
>> It looks like ipa_compute_jump_functions_for_edge was never called on
>> that edge, so if the problem appeared in a non-LTO context the following
>> might help?  And I assume we want it either way, so OK for trunk after a
>> bootstrap?
>
> This is OK, though I think it will only result in fewer summaries,
> because aliases are definitions, but that code is obviously broken.
>

I have just committed it.

Looking at PR 92278, I think I found the real problem. In
ipa_read_edge_info, you added code to throw away jump functions of edges
that do not pass possibly_call_in_translation_unit_p() test.  But that
predicate incorrectly - or at least I think so, see below - returns
false for edges leading to interposable symbols.  The reason why I think
it is incorrect is because a node which is considered interposable
before merging can apparently later on be upgraded to a local one by
ipa-visibility.

I am testing the following fix, is it OK if it passes?

The testcase passes a version script to the linker, I guess our LTO
testsuite does not support that, does it?

Thanks,

Martin


2019-10-30  Martin Jambor  

ipa/92278
* cgraph.c (cgraph_edge::possibly_call_in_translation_unit_p): Fix
availability comparison.
---
 gcc/cgraph.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index d47d4128b1c..8057ccdb7c0 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -3813,7 +3813,7 @@ cgraph_edge::possibly_call_in_translation_unit_p (void)
   if (node->previous_sharing_asm_name)
 node = symtab_node::get_for_asmname (DECL_ASSEMBLER_NAME (callee->decl));
   gcc_assert (TREE_PUBLIC (node->decl));
-  return node->get_availability () >= AVAIL_AVAILABLE;
+  return node->get_availability () >= AVAIL_INTERPOSABLE;
 }
 
 /* A stashed copy of "symtab" for use by selftest::symbol_table_test.
-- 
2.23.0

Re: [7/n] Use consistent compatibility checks in vectorizable_shift

[Richard Biener]

On Fri, Oct 25, 2019 at 2:34 PM Richard Sandiford
 wrote:
>
> The validation phase of vectorizable_shift used TYPE_MODE to check
> whether the shift amount vector was compatible with the shifted vector:
>
>   if ((op1_vectype == NULL_TREE
>|| TYPE_MODE (op1_vectype) != TYPE_MODE (vectype))
>   && (!slp_node
>   || SLP_TREE_DEF_TYPE
>(SLP_TREE_CHILDREN (slp_node)[1]) != vect_constant_def))
>
> But the generation phase was stricter and required the element types to
> be equivalent:
>
>&& !useless_type_conversion_p (TREE_TYPE (vectype),
>   TREE_TYPE (op1)))
>
> This difference led to an ICE with a later patch.
>
> The first condition seems a bit too lax given that the function
> supports vect_worthwhile_without_simd_p, where two different vector
> types could have the same integer mode.  But it seems too strict
> to reject signed shifts by unsigned amounts or unsigned shifts by
> signed amounts; verify_gimple_assign_binary is happy with those.
>
> This patch therefore goes for a middle ground of checking both TYPE_MODE
> and TYPE_VECTOR_SUBPARTS, using the same condition in both places.

OK.  The whole vectorizable_shift needs a rewrite ;)  (no good reason to
not support widening/narrowing of a shift argument)

Richard.

>
> 2019-10-24  Richard Sandiford  
>
> gcc/
> * tree-vect-stmts.c (vectorizable_shift): Check the number
> of vector elements as well as the type mode when deciding
> whether an op1_vectype is compatible.  Reuse the result of
> this check when generating vector statements.
>
> Index: gcc/tree-vect-stmts.c
> ===
> --- gcc/tree-vect-stmts.c   2019-10-25 13:27:08.653811531 +0100
> +++ gcc/tree-vect-stmts.c   2019-10-25 13:27:12.121787027 +0100
> @@ -5522,6 +5522,7 @@ vectorizable_shift (stmt_vec_info stmt_i
>bool scalar_shift_arg = true;
>bb_vec_info bb_vinfo = STMT_VINFO_BB_VINFO (stmt_info);
>vec_info *vinfo = stmt_info->vinfo;
> +  bool incompatible_op1_vectype_p = false;
>
>if (!STMT_VINFO_RELEVANT_P (stmt_info) && !bb_vinfo)
>  return false;
> @@ -5666,8 +5667,12 @@ vectorizable_shift (stmt_vec_info stmt_i
>
>if (!op1_vectype)
> op1_vectype = get_same_sized_vectype (TREE_TYPE (op1), vectype_out);
> -  if ((op1_vectype == NULL_TREE
> -  || TYPE_MODE (op1_vectype) != TYPE_MODE (vectype))
> +  incompatible_op1_vectype_p
> +   = (op1_vectype == NULL_TREE
> +  || maybe_ne (TYPE_VECTOR_SUBPARTS (op1_vectype),
> +   TYPE_VECTOR_SUBPARTS (vectype))
> +  || TYPE_MODE (op1_vectype) != TYPE_MODE (vectype));
> +  if (incompatible_op1_vectype_p
>   && (!slp_node
>   || SLP_TREE_DEF_TYPE
>(SLP_TREE_CHILDREN (slp_node)[1]) != vect_constant_def))
> @@ -5813,9 +5818,7 @@ vectorizable_shift (stmt_vec_info stmt_i
>  }
>  }
>  }
> - else if (slp_node
> -  && !useless_type_conversion_p (TREE_TYPE (vectype),
> - TREE_TYPE (op1)))
> + else if (slp_node && incompatible_op1_vectype_p)
> {
>   if (was_scalar_shift_arg)
> {

Re: [6/n] Use build_vector_type_for_mode in get_vectype_for_scalar_type_and_size

[Richard Biener]

On Fri, Oct 25, 2019 at 2:32 PM Richard Sandiford
 wrote:
>
> Except for one case, get_vectype_for_scalar_type_and_size calculates
> what the vector mode should be and then calls build_vector_type,
> which recomputes the mode from scratch.  This patch makes it use
> build_vector_type_for_mode instead.
>
> The exception mentioned above is when preferred_simd_mode returns
> an integer mode, which it does if no appropriate vector mode exists.
> The integer mode in question is usually word_mode, although epiphany
> can return a doubleword mode in some cases.
>
> There's no guarantee that this integer mode is appropriate, since for
> example the scalar type could be a float.  The traditional behaviour is
> therefore to use the integer mode to determine a size only, and leave
> mode_for_vector to pick the TYPE_MODE.  (Note that it can actually end
> up picking a vector mode if the target defines a disabled vector mode.
> We therefore still need to check TYPE_MODE after building the type.)

OK.

Thanks,
Richard.

>
> 2019-10-24  Richard Sandiford  
>
> gcc/
> * tree-vect-stmts.c (get_vectype_for_scalar_type_and_size): If
> targetm.vectorize.preferred_simd_mode returns an integer mode,
> use mode_for_vector to decide what the vector type's mode
> should actually be.  Use build_vector_type_for_mode instead
> of build_vector_type.
>
> Index: gcc/tree-vect-stmts.c
> ===
> --- gcc/tree-vect-stmts.c   2019-10-25 13:26:59.309877555 +0100
> +++ gcc/tree-vect-stmts.c   2019-10-25 13:27:08.653811531 +0100
> @@ -11162,16 +11162,31 @@ get_vectype_for_scalar_type_and_size (tr
>/* If no size was supplied use the mode the target prefers.   Otherwise
>   lookup a vector mode of the specified size.  */
>if (known_eq (size, 0U))
> -simd_mode = targetm.vectorize.preferred_simd_mode (inner_mode);
> +{
> +  simd_mode = targetm.vectorize.preferred_simd_mode (inner_mode);
> +  if (SCALAR_INT_MODE_P (simd_mode))
> +   {
> + /* Traditional behavior is not to take the integer mode
> +literally, but simply to use it as a way of determining
> +the vector size.  It is up to mode_for_vector to decide
> +what the TYPE_MODE should be.
> +
> +Note that nunits == 1 is allowed in order to support single
> +element vector types.  */
> + if (!multiple_p (GET_MODE_SIZE (simd_mode), nbytes, )
> + || !mode_for_vector (inner_mode, nunits).exists (_mode))
> +   return NULL_TREE;
> +   }
> +}
>else if (!multiple_p (size, nbytes, )
>|| !mode_for_vector (inner_mode, nunits).exists (_mode))
>  return NULL_TREE;
> -  /* NOTE: nunits == 1 is allowed to support single element vector types.  */
> -  if (!multiple_p (GET_MODE_SIZE (simd_mode), nbytes, ))
> -return NULL_TREE;
>
> -  vectype = build_vector_type (scalar_type, nunits);
> +  vectype = build_vector_type_for_mode (scalar_type, simd_mode);
>
> +  /* In cases where the mode was chosen by mode_for_vector, check that
> + the target actually supports the chosen mode, or that it at least
> + allows the vector mode to be replaced by a like-sized integer.  */
>if (!VECTOR_MODE_P (TYPE_MODE (vectype))
>&& !INTEGRAL_MODE_P (TYPE_MODE (vectype)))
>  return NULL_TREE;

Re: RFC/A: Add a targetm.vectorize.related_mode hook

[Richard Biener]

On Wed, Oct 30, 2019 at 10:43 AM Richard Sandiford
 wrote:
>
> The series posted so far now shows how the hook would be used in practice.
> Just wanted to follow up on some points here:
>
> Richard Sandiford  writes:
> > Richard Biener  writes:
> >> On Wed, Oct 23, 2019 at 2:12 PM Richard Sandiford
> >>  wrote:
> >>>
> >>> Richard Biener  writes:
> >>> > On Wed, Oct 23, 2019 at 1:51 PM Richard Sandiford
> >>> >  wrote:
> >>> >>
> >>> >> Richard Biener  writes:
> >>> >> > On Wed, Oct 23, 2019 at 1:00 PM Richard Sandiford
> >>> >> >  wrote:
> >>> >> >>
> >>> >> >> This patch is the first of a series that tries to remove two
> >>> >> >> assumptions:
> >>> >> >>
> >>> >> >> (1) that all vectors involved in vectorisation must be the same size
> >>> >> >>
> >>> >> >> (2) that there is only one vector mode for a given element mode and
> >>> >> >> number of elements
> >>> >> >>
> >>> >> >> Relaxing (1) helps with targets that support multiple vector sizes 
> >>> >> >> or
> >>> >> >> that require the number of elements to stay the same.  E.g. if we're
> >>> >> >> vectorising code that operates on narrow and wide elements, and the
> >>> >> >> narrow elements use 64-bit vectors, then on AArch64 it would 
> >>> >> >> normally
> >>> >> >> be better to use 128-bit vectors rather than pairs of 64-bit vectors
> >>> >> >> for the wide elements.
> >>> >> >>
> >>> >> >> Relaxing (2) makes it possible for -msve-vector-bits=128 to preoduce
> >>> >> >> fixed-length code for SVE.  It also allows unpacked/half-size SVE
> >>> >> >> vectors to work with -msve-vector-bits=256.
> >>> >> >>
> >>> >> >> The patch adds a new hook that targets can use to control how we
> >>> >> >> move from one vector mode to another.  The hook takes a starting 
> >>> >> >> vector
> >>> >> >> mode, a new element mode, and (optionally) a new number of elements.
> >>> >> >> The flexibility needed for (1) comes in when the number of elements
> >>> >> >> isn't specified.
> >>> >> >>
> >>> >> >> All callers in this patch specify the number of elements, but a 
> >>> >> >> later
> >>> >> >> vectoriser patch doesn't.  I won't be posting the vectoriser patch
> >>> >> >> for a few days, hence the RFC/A tag.
> >>> >> >>
> >>> >> >> Tested individually on aarch64-linux-gnu and as a series on
> >>> >> >> x86_64-linux-gnu.  OK to install?  Or if not yet, does the idea
> >>> >> >> look OK?
> >>> >> >
> >>> >> > In isolation the idea looks good but maybe a bit limited?  I see
> >>> >> > how it works for the same-size case but if you consider x86
> >>> >> > where we have SSE, AVX256 and AVX512 what would it return
> >>> >> > for related_vector_mode (V4SImode, SImode, 0)?  Or is this
> >>> >> > kind of query not intended (where the component modes match
> >>> >> > but nunits is zero)?
> >>> >>
> >>> >> In that case we'd normally get V4SImode back.  It's an allowed
> >>> >> combination, but not very useful.
> >>> >>
> >>> >> > How do you get from SVE fixed 128bit to NEON fixed 128bit then?  Or 
> >>> >> > is
> >>> >> > it just used to stay in the same register set for different component
> >>> >> > modes?
> >>> >>
> >>> >> Yeah, the idea is to use the original vector mode as essentially
> >>> >> a base architecture.
> >>> >>
> >>> >> The follow-on patches replace vec_info::vector_size with
> >>> >> vec_info::vector_mode and targetm.vectorize.autovectorize_vector_sizes
> >>> >> with targetm.vectorize.autovectorize_vector_modes.  These are the
> >>> >> starting modes that would be passed to the hook in the nunits==0 case.
> >>> >>
> >>> >> E.g. for Advanced SIMD on AArch64, it would make more sense for
> >>> >> related_mode (V4HImode, SImode, 0) to be V4SImode rather than V2SImode.
> >>> >> I think things would work in a similar way for the x86_64 vector archs.
> >>> >>
> >>> >> For SVE we'd add both VNx16QImode (the SVE mode) and V16QImode (the
> >>> >> Advanced SIMD mode) to autovectorize_vector_modes, even though they
> >>> >> happen to be the same size for 128-bit SVE.  We can then compare
> >>> >> 128-bit SVE with 128-bit Advanced SIMD, with related_mode ensuring
> >>> >> that we consistently use all-SVE modes or all-Advanced SIMD modes
> >>> >> for each attempt.
> >>> >>
> >>> >> The plan for SVE is to add 4(!) modes to autovectorize_vector_modes:
> >>> >>
> >>> >> - VNx16QImode (full vector)
> >>> >> - VNx8QImode (half vector)
> >>> >> - VNx4QImode (quarter vector)
> >>> >> - VNx2QImode (eighth vector)
> >>> >>
> >>> >> and then pick the one with the lowest cost.  related_mode would
> >>> >> keep the number of units the same for nunits==0, within the limit
> >>> >> of the vector size.  E.g.:
> >>> >>
> >>> >> - related_mode (VNx16QImode, HImode, 0) == VNx8HImode (full vector)
> >>> >> - related_mode (VNx8QImode, HImode, 0) == VNx8HImode (full vector)
> >>> >> - related_mode (VNx4QImode, HImode, 0) == VNx4HImode (half vector)
> >>> >> - related_mode (VNx2QImode, HImode, 0) == VNx2HImode (quarter vector)
> >>> >>
> >>> >> and:
> >>> >>
> >>> >> - related_mode 

Re: Replace mode_for_int_vector with related_int_vector_mode

[Richard Biener]

On Wed, Oct 23, 2019 at 1:06 PM Richard Sandiford
 wrote:
>
> mode_for_int_vector, like mode_for_vector, can sometimes return
> an integer mode or an unsupported vector mode.  But no callers
> are interested in that case, and only want supported vector modes.
> This patch therefore replaces mode_for_int_vector with
> related_int_vector_mode, which gives the target a chance to pick
> its preferred vector mode for the given element mode and size.
>
> Tested individually on aarch64-linux-gnu and as a series on
> x86_64-linux-gnu.  OK to install?

OK.

Thanks,
Richard.

> Richard
>
>
> 2019-10-23  Richard Sandiford  
>
> gcc/
> * machmode.h (mode_for_int_vector): Delete.
> (related_int_vector_mode): Declare.
> * stor-layout.c (mode_for_int_vector): Delete.
> (related_int_vector_mode): New function.
> * optabs.c (expand_vec_perm_1): Use related_int_vector_mode
> instead of mode_for_int_vector.
> (expand_vec_perm_const): Likewise.
> * config/aarch64/aarch64.c (aarch64_emit_approx_sqrt): Likewise.
> (aarch64_evpc_sve_tbl): Likewise.
> * config/s390/s390.c (s390_expand_vec_compare_cc): Likewise.
> (s390_expand_vcond): Likewise.
>
> Index: gcc/machmode.h
> ===
> --- gcc/machmode.h  2019-10-23 11:33:01.564510281 +0100
> +++ gcc/machmode.h  2019-10-23 12:01:36.968336613 +0100
> @@ -879,22 +879,9 @@ smallest_int_mode_for_size (poly_uint64
>  extern opt_scalar_int_mode int_mode_for_mode (machine_mode);
>  extern opt_machine_mode bitwise_mode_for_mode (machine_mode);
>  extern opt_machine_mode mode_for_vector (scalar_mode, poly_uint64);
> -extern opt_machine_mode mode_for_int_vector (unsigned int, poly_uint64);
>  extern opt_machine_mode related_vector_mode (machine_mode, scalar_mode,
>  poly_uint64 = 0);
> -
> -/* Return the integer vector equivalent of MODE, if one exists.  In other
> -   words, return the mode for an integer vector that has the same number
> -   of bits as MODE and the same number of elements as MODE, with the
> -   latter being 1 if MODE is scalar.  The returned mode can be either
> -   an integer mode or a vector mode.  */
> -
> -inline opt_machine_mode
> -mode_for_int_vector (machine_mode mode)
> -{
> -  return mode_for_int_vector (GET_MODE_UNIT_BITSIZE (mode),
> - GET_MODE_NUNITS (mode));
> -}
> +extern opt_machine_mode related_int_vector_mode (machine_mode);
>
>  /* A class for iterating through possible bitfield modes.  */
>  class bit_field_mode_iterator
> Index: gcc/stor-layout.c
> ===
> --- gcc/stor-layout.c   2019-10-23 11:33:01.564510281 +0100
> +++ gcc/stor-layout.c   2019-10-23 12:01:36.972336585 +0100
> @@ -515,21 +515,6 @@ mode_for_vector (scalar_mode innermode,
>return opt_machine_mode ();
>  }
>
> -/* Return the mode for a vector that has NUNITS integer elements of
> -   INT_BITS bits each, if such a mode exists.  The mode can be either
> -   an integer mode or a vector mode.  */
> -
> -opt_machine_mode
> -mode_for_int_vector (unsigned int int_bits, poly_uint64 nunits)
> -{
> -  scalar_int_mode int_mode;
> -  machine_mode vec_mode;
> -  if (int_mode_for_size (int_bits, 0).exists (_mode)
> -  && mode_for_vector (int_mode, nunits).exists (_mode))
> -return vec_mode;
> -  return opt_machine_mode ();
> -}
> -
>  /* If a piece of code is using vector mode VECTOR_MODE and also wants
> to operate on elements of mode ELEMENT_MODE, return the vector mode
> it should use for those elements.  If NUNITS is nonzero, ensure that
> @@ -550,6 +535,26 @@ related_vector_mode (machine_mode vector
>return targetm.vectorize.related_mode (vector_mode, element_mode, nunits);
>  }
>
> +/* If a piece of code is using vector mode VECTOR_MODE and also wants
> +   to operate on integer vectors with the same element size and number
> +   of elements, return the vector mode it should use.  Return an empty
> +   opt_machine_mode if there is no supported vector mode with the
> +   required properties.
> +
> +   Unlike mode_for_vector. any returned mode is guaranteed to satisfy
> +   both VECTOR_MODE_P and targetm.vector_mode_supported_p.  */
> +
> +opt_machine_mode
> +related_int_vector_mode (machine_mode vector_mode)
> +{
> +  gcc_assert (VECTOR_MODE_P (vector_mode));
> +  scalar_int_mode int_mode;
> +  if (int_mode_for_mode (GET_MODE_INNER (vector_mode)).exists (_mode))
> +return related_vector_mode (vector_mode, int_mode,
> +   GET_MODE_NUNITS (vector_mode));
> +  return opt_machine_mode ();
> +}
> +
>  /* Return the alignment of MODE. This will be bounded by 1 and
> BIGGEST_ALIGNMENT.  */
>
> Index: gcc/optabs.c
> ===
> --- gcc/optabs.c2019-10-08 09:23:31.582531825 +0100
> +++ gcc/optabs.c  

Re: [2/3] Pass vec_infos to more routines

[Richard Biener]

On Sun, Oct 20, 2019 at 3:29 PM Richard Sandiford
 wrote:
>
> These 11 patches just pass vec_infos to one routine each.  Splitting
> them up make it easier to write the changelogs, but they're so trivial
> that it seemed better to send them all in one message.

OK.

>
> Pass a vec_info to vect_supportable_shift
>
> 2019-10-20  Richard Sandiford  
>
> gcc/
> * tree-vectorizer.h (vect_supportable_shift): Take a vec_info.
> * tree-vect-stmts.c (vect_supportable_shift): Likewise.
> * tree-vect-patterns.c (vect_synth_mult_by_constant): Update call
> accordingly.
>
> Index: gcc/tree-vectorizer.h
> ===
> --- gcc/tree-vectorizer.h   2019-10-20 13:58:02.095634389 +0100
> +++ gcc/tree-vectorizer.h   2019-10-20 14:14:00.632786715 +0100
> @@ -1634,7 +1634,7 @@ extern void vect_get_load_cost (stmt_vec
> stmt_vector_for_cost *, bool);
>  extern void vect_get_store_cost (stmt_vec_info, int,
>  unsigned int *, stmt_vector_for_cost *);
> -extern bool vect_supportable_shift (enum tree_code, tree);
> +extern bool vect_supportable_shift (vec_info *, enum tree_code, tree);
>  extern tree vect_gen_perm_mask_any (tree, const vec_perm_indices &);
>  extern tree vect_gen_perm_mask_checked (tree, const vec_perm_indices &);
>  extern void optimize_mask_stores (class loop*);
> Index: gcc/tree-vect-stmts.c
> ===
> --- gcc/tree-vect-stmts.c   2019-10-20 13:58:02.111634275 +0100
> +++ gcc/tree-vect-stmts.c   2019-10-20 14:14:00.628786742 +0100
> @@ -5465,7 +5465,7 @@ vectorizable_assignment (stmt_vec_info s
> either as shift by a scalar or by a vector.  */
>
>  bool
> -vect_supportable_shift (enum tree_code code, tree scalar_type)
> +vect_supportable_shift (vec_info *, enum tree_code code, tree scalar_type)
>  {
>
>machine_mode vec_mode;
> Index: gcc/tree-vect-patterns.c
> ===
> --- gcc/tree-vect-patterns.c2019-10-17 14:22:55.519309037 +0100
> +++ gcc/tree-vect-patterns.c2019-10-20 14:14:00.628786742 +0100
> @@ -2720,6 +2720,7 @@ apply_binop_and_append_stmt (tree_code c
>  vect_synth_mult_by_constant (tree op, tree val,
>  stmt_vec_info stmt_vinfo)
>  {
> +  vec_info *vinfo = stmt_vinfo->vinfo;
>tree itype = TREE_TYPE (op);
>machine_mode mode = TYPE_MODE (itype);
>struct algorithm alg;
> @@ -2738,7 +2739,7 @@ vect_synth_mult_by_constant (tree op, tr
>
>/* Targets that don't support vector shifts but support vector additions
>   can synthesize shifts that way.  */
> -  bool synth_shift_p = !vect_supportable_shift (LSHIFT_EXPR, multtype);
> +  bool synth_shift_p = !vect_supportable_shift (vinfo, LSHIFT_EXPR, 
> multtype);
>
>HOST_WIDE_INT hwval = tree_to_shwi (val);
>/* Use MAX_COST here as we don't want to limit the sequence on rtx costs.
>
>
> Pass a vec_info to vect_supportable_direct_optab_p
>
> 2019-10-20  Richard Sandiford  
>
> gcc/
> * tree-vect-patterns.c (vect_supportable_direct_optab_p): Take
> a vec_info.
> (vect_recog_dot_prod_pattern): Update call accordingly.
> (vect_recog_sad_pattern, vect_recog_pow_pattern): Likewise.
> (vect_recog_widen_sum_pattern): Likewise.
>
> Index: gcc/tree-vect-patterns.c
> ===
> --- gcc/tree-vect-patterns.c2019-10-20 14:14:00.628786742 +0100
> +++ gcc/tree-vect-patterns.c2019-10-20 14:14:03.588765602 +0100
> @@ -187,7 +187,7 @@ vect_get_external_def_edge (vec_info *vi
> is nonnull.  */
>
>  static bool
> -vect_supportable_direct_optab_p (tree otype, tree_code code,
> +vect_supportable_direct_optab_p (vec_info *, tree otype, tree_code code,
>  tree itype, tree *vecotype_out,
>  tree *vecitype_out = NULL)
>  {
> @@ -985,7 +985,7 @@ vect_recog_dot_prod_pattern (stmt_vec_in
>vect_pattern_detected ("vect_recog_dot_prod_pattern", last_stmt);
>
>tree half_vectype;
> -  if (!vect_supportable_direct_optab_p (type, DOT_PROD_EXPR, half_type,
> +  if (!vect_supportable_direct_optab_p (vinfo, type, DOT_PROD_EXPR, 
> half_type,
> type_out, _vectype))
>  return NULL;
>
> @@ -1143,7 +1143,7 @@ vect_recog_sad_pattern (stmt_vec_info st
>vect_pattern_detected ("vect_recog_sad_pattern", last_stmt);
>
>tree half_vectype;
> -  if (!vect_supportable_direct_optab_p (sum_type, SAD_EXPR, half_type,
> +  if (!vect_supportable_direct_optab_p (vinfo, sum_type, SAD_EXPR, half_type,
> type_out, _vectype))
>  return NULL;
>
> @@ -1273,6 +1273,7 @@ vect_recog_widen_mult_pattern (stmt_vec_
>  static gimple *
>  vect_recog_pow_pattern (stmt_vec_info stmt_vinfo, tree *type_out)
>  {

Re: [1/3] Avoid setting current_vector_size in get_vec_alignment_for_array_type

[Richard Biener]

On Sun, Oct 20, 2019 at 3:23 PM Richard Sandiford
 wrote:
>
> The increase_alignment pass was using get_vectype_for_scalar_type
> to get the preferred vector type for each array element type.
> This has the effect of carrying over the vector size chosen by
> the first successful call to all subsequent calls, whereas it seems
> more natural to treat each array type independently and pick the
> "best" vector type for each element type.

OK.

>
> 2019-10-20  Richard Sandiford  
>
> gcc/
> * tree-vectorizer.c (get_vec_alignment_for_array_type): Use
> get_vectype_for_scalar_type_and_size instead of
> get_vectype_for_scalar_type.
>
> Index: gcc/tree-vectorizer.c
> ===
> --- gcc/tree-vectorizer.c   2019-10-20 13:58:02.091634417 +0100
> +++ gcc/tree-vectorizer.c   2019-10-20 14:13:50.784857051 +0100
> @@ -1347,7 +1347,8 @@ get_vec_alignment_for_array_type (tree t
>gcc_assert (TREE_CODE (type) == ARRAY_TYPE);
>poly_uint64 array_size, vector_size;
>
> -  tree vectype = get_vectype_for_scalar_type (strip_array_types (type));
> +  tree scalar_type = strip_array_types (type);
> +  tree vectype = get_vectype_for_scalar_type_and_size (scalar_type, 0);
>if (!vectype
>|| !poly_int_tree_p (TYPE_SIZE (type), _size)
>|| !poly_int_tree_p (TYPE_SIZE (vectype), _size)

Re: [SLP] SLP vectorization: vectorize vector constructors

[Joel Hutton]

On 30/10/2019 13:49, Richard Biener wrote:
> Why do you need this?  The vectorizer already creates such CTORs.  Any
> testcase that you can show?

typedef long v2di __attribute__((vector_size(16)));
v2di v;
void
foo()
{
   v = (v2di){v[1], v[0]};
}


>>   * tree-vect-slp.c (vect_analyze_slp_instance): Add case for vector 
>> constructors.
>>   (vect_bb_slp_scalar_cost): Likewise.
>>   (vect_slp_check_for_constructors): New function.
>>   (vect_slp_analyze_bb_1): Add check for vector constructors.
>>   (vect_schedule_slp_instance): Add case to fixup vector constructor 
>> stmt.
>>   (vectorize_slp_instance_root_stmt): New function
>>   * tree-vectorizer.h (SLP_INSTANCE_ROOT_STMT): New field.
> + SLP_INSTANCE_ROOT_STMT (new_instance) = constructor ?
> stmt_info->stmt\
> + : NULL;
>
> SLP_INSTANCE_ROOT_STMT should be a stmt_vec_info I guess...
>
> @@ -2801,6 +2830,10 @@ vect_bb_slp_scalar_cost (basic_block bb,
>  stmt_vec_info use_stmt_info = vinfo->lookup_stmt
> (use_stmt);
>  if (!use_stmt_info || !PURE_SLP_STMT (use_stmt_info))
>{
> +   /* Check this is not a constructor that will be
> vectorized
> +  away.  */
> +   if (BB_VINFO_GROUPED_STORES (vinfo).contains
> (use_stmt_info))
> +   continue;
>  (*life)[i] = true;
>
> ... which you then simply mark as PURE_SLP_STMT where we call
> vect_mark_slp_stmts in vect_slp_analyze_bb_1.
>
> I see you have the TYPE_VECTOR_SUBPARTS check still at transform
> stage in vectorize_slp_instance_root_stmt, please simply move
> it to vect_slp_check_for_constructors or to vect_analyze_slp_instance
> where you have the other rejects (non-SSA names in the ctor).
If the check is in vect_slp_check_for_constructors, which vector is used 
as the input to tYPE_VECTOR_SUBPARTS, the lhs of the constructor?
> That is, vectorize_slp_instance_root_stmt may not fail.
>
> +bool
> +vectorize_slp_instance_root_stmt (slp_tree node, slp_instance instance)
> +{
> +
>
> extra newline
>
> +  if (SLP_TREE_NUMBER_OF_VEC_STMTS (node) == 1)
>
> the stmt replacing can be commonized between == 1 and > 1 cases.
>
> FOR_EACH_VEC_ELT (slp_instances, i, instance)
>   {
> +  slp_tree node = SLP_INSTANCE_TREE (instance);
> /* Schedule the tree of INSTANCE.  */
> -  vect_schedule_slp_instance (SLP_INSTANCE_TREE (instance),
> +  vect_schedule_slp_instance (node,
>instance, bst_map);
> +
> +  /* Vectorize the instance root.  */
> +  if (instance->root == node && SLP_INSTANCE_ROOT_STMT (instance)
> + && SLP_TREE_VEC_STMTS (node).exists ())
> +   if (!vectorize_slp_instance_root_stmt (node, instance))
> + {
>
> instance->root == node is always true.  Likewise
> SLP_TREE_VEC_STMTS (node).exists ().
>
> @@ -4061,15 +4201,42 @@ vect_schedule_slp (vec_info *vinfo)
> if (is_a  (vinfo))
>
> the ChangeLog doesn't mention this.  I guess this isn't necessary
> unless you fail vectorizing late which you shouldn't.
>
> diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h
> index
> 56be28b0cc5a77412f996e70636b08d5b615813e..9f8419e4208b7d438ace41892022f93ebcadd019
> 100644
> --- a/gcc/tree-vectorizer.h
> +++ b/gcc/tree-vectorizer.h
> @@ -151,6 +151,10 @@ public:
> /* The root of SLP tree.  */
> slp_tree root;
>
> +  /* For vector constructors, the constructor stmt that the SLP tree is
> built
> + from, NULL otherwise.  */
> +  gimple *root_stmt;
>
> as said, make this a stmt_vec_info
>
> Thanks,
> Richard.
>
>
>> gcc/testsuite/ChangeLog:
>>
>> 2019-10-10  Joel Hutton  
>>
>>   * gcc.dg/vect/bb-slp-40.c: New test.
>>   * gcc.dg/vect/bb-slp-41.c: New test.
>>
>>

Re: [RFC PATCH] targetm.omp.device_kind_arch_isa and OpenMP declare variant kind/arch/isa handling

[Szabolcs Nagy]

On 29/10/2019 17:15, Jakub Jelinek wrote:
> +void f03 (void);
> +#pragma omp declare variant (f03) match 
> (device={kind(any),arch(x86_64),isa(avx512f,avx512bw)})
> +void f04 (void);

1) it's not clear from the omp spec what is the intended
syntax for isa-name, arch-name and extension-name, but
i expected strings in "".

what if an arch or isa name contains ',' ')' etc?

we were planing to use things like

isa("sve")
arch("armv8.2-a+sve")
extension("scalable")

i think we can drop the ", but it looks a bit weird:
normal pp-token parsing of directives would break
the arch name up into multiple tokens (unless it's
special cased like include <...>, at least inside
_Pragma("omp ...") there is no expectation of normal
pp-token parsing), either way is fine with me, but
it may be worth asking clarification from omp?


2) does f03 need to be declared before the declare variant
pragma appears?

for simd variants it means i need to declare the function
with the right simd types and attributes.

Re: [committed][AArch64] Add support for the SVE PCS

[Andreas Schwab]

The D frontend is also broken.

(gdb) r
Starting program: /opt/gcc/test/Build/gcc/d21 
../../../../libphobos/libdruntime/core/sys/posix/utime.d -quiet -dumpbase 
utime.d -mlittle-endian -mabi=lp64 -auxbase-strip core/sys/posix/.libs/utime.o 
-g -O2 -fPIC -fversion=Shared -iprefix 
/opt/gcc/test/Build/gcc/../lib64/gcc/aarch64-suse-linux/10.0.0/ -isystem 
/opt/gcc/test/Build/./gcc/include -isystem 
/opt/gcc/test/Build/./gcc/include-fixed -nostdinc -isystem 
/usr/aarch64-suse-linux/include -isystem /usr/aarch64-suse-linux/sys-include -I 
../../../../libphobos/libdruntime -I .

Breakpoint 1, fancy_abort (file=file@entry=0x1832210 "../../gcc/poly-int.h", 
line=line@entry=504, function=function@entry=0x1832200 "to_constant")
at ../../gcc/diagnostic.c:1649
1649  internal_error ("in %s, at %s:%d", function, trim_filename (file), 
line);
(gdb) bt
#0  fancy_abort (file=file@entry=0x1832210 "../../gcc/poly-int.h", 
line=line@entry=504, function=function@entry=0x1832200 "to_constant")
at ../../gcc/diagnostic.c:1649
#1  0x00793278 in poly_int_pod<2u, unsigned long>::to_constant (
this=) at ../../gcc/poly-int.h:502
#2  build_frontend_type (type=)
at ../../gcc/d/d-builtins.cc:204
#3  0x00794ed4 in d_build_builtins_module (m=m@entry=0x214cc90)
at ../../gcc/tree.h:3382
#4  0x007a3ca8 in Compiler::loadModule (m=m@entry=0x214cc90)
at ../../gcc/d/d-frontend.cc:547
#5  0x006ab040 in Module::load (loc=..., packages=, 
ident=0x214fce0) at ../../gcc/d/dmd/dmodule.c:291
#6  0x0069057c in Import::load (this=this@entry=0x214fd10, 
sc=sc@entry=0x214c250) at ../../gcc/d/dmd/dimport.c:154
#7  0x0069076c in Import::importAll (sc=0x214c250, this=0x214fd10)
at ../../gcc/d/dmd/dimport.c:173
#8  Import::importAll (this=0x214fd10, sc=0x214c250)
at ../../gcc/d/dmd/dimport.c:169
#9  0x00657284 in AttribDeclaration::importAll (this=, 
sc=0x214c250) at ../../gcc/d/dmd/attrib.c:160
#10 0x00657284 in AttribDeclaration::importAll (this=, 
sc=0x214c250) at ../../gcc/d/dmd/attrib.c:160
#11 0x006a8e8c in Module::importAll (this=0x214bab0)
at ../../gcc/d/dmd/dmodule.c:716
#12 Module::importAll (this=0x214bab0) at ../../gcc/d/dmd/dmodule.c:655
#13 0x00690784 in Import::importAll (sc=0x213d9b0, this=0x213df60)
at ../../gcc/d/dmd/dimport.c:176
#14 Import::importAll (this=0x213df60, sc=0x213d9b0)
at ../../gcc/d/dmd/dimport.c:169
#15 0x00657284 in AttribDeclaration::importAll (this=, 
sc=0x213cc10) at ../../gcc/d/dmd/attrib.c:160
#16 0x006a8e8c in Module::importAll (this=0x213c4f0)
at ../../gcc/d/dmd/dmodule.c:716
#17 Module::importAll (this=0x213c4f0) at ../../gcc/d/dmd/dmodule.c:655
#18 0x00690784 in Import::importAll (sc=0x213c390, this=0x1ec8030)
at ../../gcc/d/dmd/dimport.c:176
#19 Import::importAll (this=0x1ec8030, sc=0x213c390)
at ../../gcc/d/dmd/dimport.c:169
#20 0x00657284 in AttribDeclaration::importAll (this=, 
sc=0x1f23140) at ../../gcc/d/dmd/attrib.c:160
#21 0x006a8e8c in Module::importAll (this=0x1f1ea40)
at ../../gcc/d/dmd/dmodule.c:716
#22 Module::importAll (this=0x1f1ea40) at ../../gcc/d/dmd/dmodule.c:655
#23 0x007ab114 in d_parse_file () at ../../gcc/d/d-lang.cc:1137
#24 0x00d16f18 in compile_file () at ../../gcc/toplev.c:456
#25 0x0064bdac in do_compile () at ../../gcc/toplev.c:2176
#26 toplev::main (this=this@entry=0xebf8, argc=, 
argc@entry=28, argv=, argv@entry=0xed48)
at ../../gcc/toplev.c:2311
#27 0x0064d8ac in main (argc=28, argv=0xed48)
at ../../gcc/main.c:39

Andreas.

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

Re: [PATCH v2] PR85678: Change default to -fno-common

[Richard Biener]

On Tue, Oct 29, 2019 at 1:33 PM Wilco Dijkstra  wrote:
>
> v2: Tweak testsuite options to avoid failures
>
> GCC currently defaults to -fcommon.  As discussed in the PR, this is an 
> ancient
> C feature which is not conforming with the latest C standards.  On many 
> targets
> this means global variable accesses have a codesize and performance penalty.
> This applies to C code only, C++ code is not affected by -fcommon.  It is 
> about
> time to change the default.
>
> Bootstrap OK, passes testsuite on AArch64. OK for commit?

Please don't add -fcommon in lto.exp.

> ChangeLog
> 2019-10-29  Wilco Dijkstra  
>
> PR85678
> * common.opt (fcommon): Change init to 1.
>
> doc/
> * invoke.texi (-fcommon): Update documentation.
>
> testsuite/
>
> * gcc.dg/alias-15.c: Add -fcommon.
> * gcc.dg/fdata-sections-1.c: Likewise.
> * gcc.dg/ipa/pr77653.c: Likewise.
> * gcc.dg/lto/20090729_0.c: Likewise.
> * gcc.dg/lto/20111207-1_0.c: Likewise.
> * gcc.dg/lto/c-compatible-types-1_0.c: Likewise.
> * gcc.dg/lto/pr55525_0.c: Likewise.
> * gcc.target/aarch64/sve/peel_ind_1.c: Allow ANCHOR0.
> * gcc.target/aarch64/sve/peel_ind_2.c: Likewise
> * gcc.target/aarch64/sve/peel_ind_3.c: Likewise
> * lib/lto.exp (lto_init): Add -fcommon.
> ---
>
> diff --git a/gcc/common.opt b/gcc/common.opt
> index 
> f74b10aafc223e4961915b009c092f4876eddba4..798b6aeff3536e21c95752b5dd085f8ffef04643
>  100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -1131,7 +1131,7 @@ Common Report Var(flag_combine_stack_adjustments) 
> Optimization
>  Looks for opportunities to reduce stack adjustments and stack references.
>
>  fcommon
> -Common Report Var(flag_no_common,0)
> +Common Report Var(flag_no_common,0) Init(1)
>  Put uninitialized globals in the common section.
>
>  fcompare-debug
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 
> 92fb316a368a4a36218fac6de2744c7ab6446ef5..18cfd07d4bbb4b866808db0701faf88bddbd9a94
>  100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -568,7 +568,7 @@ Objective-C and Objective-C++ Dialects}.
>  -fnon-call-exceptions  -fdelete-dead-exceptions  -funwind-tables @gol
>  -fasynchronous-unwind-tables @gol
>  -fno-gnu-unique @gol
> --finhibit-size-directive  -fno-common  -fno-ident @gol
> +-finhibit-size-directive  -fcommon  -fno-ident @gol
>  -fpcc-struct-return  -fpic  -fPIC  -fpie  -fPIE  -fno-plt @gol
>  -fno-jump-tables @gol
>  -frecord-gcc-switches @gol
> @@ -14049,35 +14049,27 @@ useful for building programs to run under WINE@.
>  code that is not binary compatible with code generated without that switch.
>  Use it to conform to a non-default application binary interface.
>
> -@item -fno-common
> -@opindex fno-common
> +@item -fcommon
>  @opindex fcommon
> +@opindex fno-common
>  @cindex tentative definitions
> -In C code, this option controls the placement of global variables
> -defined without an initializer, known as @dfn{tentative definitions}
> -in the C standard.  Tentative definitions are distinct from declarations
> +In C code, this option controls the placement of global variables
> +defined without an initializer, known as @dfn{tentative definitions}
> +in the C standard.  Tentative definitions are distinct from declarations
>  of a variable with the @code{extern} keyword, which do not allocate storage.
>
> -Unix C compilers have traditionally allocated storage for
> -uninitialized global variables in a common block.  This allows the
> -linker to resolve all tentative definitions of the same variable
> +The default is @option{-fno-common}, which specifies that the compiler places
> +uninitialized global variables in the BSS section of the object file.
> +This inhibits the merging of tentative definitions by the linker so you get a
> +multiple-definition error if the same variable is accidentally defined in 
> more
> +than one compilation unit.
> +
> +The @option{-fcommon} places uninitialized global variables in a common 
> block.
> +This allows the linker to resolve all tentative definitions of the same 
> variable
>  in different compilation units to the same object, or to a non-tentative
> -definition.
> -This is the behavior specified by @option{-fcommon}, and is the default for
> -GCC on most targets.
> -On the other hand, this behavior is not required by ISO
> -C, and on some targets may carry a speed or code size penalty on
> -variable references.
> -
> -The @option{-fno-common} option specifies that the compiler should instead
> -place uninitialized global variables in the BSS section of the object file.
> -This inhibits the merging of tentative definitions by the linker so
> -you get a multiple-definition error if the same
> -variable is defined in more than one compilation unit.
> -Compiling with @option{-fno-common} is useful on targets for which
> -it provides better performance, or if you wish to verify that the
> -program will work on other 

Re: Watch for missing summaries even more

[Richard Biener]

On Wed, Oct 30, 2019 at 1:06 PM Martin Jambor  wrote:
>
> Hi,
>
> On Wed, Oct 30 2019, Jan Hubicka wrote:
> > Hi,
> > this patch fixes another place we may have missing argument summary.
> > Here the situation is that the call site being inlined has no jump
> > functions while function which is being inlines has another call with
> > jump function.  This can validly happen when we inline into functions
> > with indirect inlining and ipa-cp disabled but I am not 100% why it
> > happens i.e. during Firefox builds.  Martin, do you have any ideas?
>
> No, not without seeing what is going on.  I think we compute jump
> functions also when we are inlining and IPA-CP is disabled, by the way.
>
> It looks like ipa_compute_jump_functions_for_edge was never called on
> that edge, so if the problem appeared in a non-LTO context the following
> might help?  And I assume we want it either way, so OK for trunk after a
> bootstrap?
>
> Martin
>
>
> diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
> index 0dd73561419..10fe1bc929f 100644
> --- a/gcc/ipa-prop.c
> +++ b/gcc/ipa-prop.c
> @@ -2040,7 +2040,7 @@ ipa_compute_jump_functions_for_bb (struct 
> ipa_func_body_info *fbi, basic_block b
>
>if (callee)
> {
> - callee->ultimate_alias_target ();
> + callee = callee->ultimate_alias_target ();

so that either was dead code or your fix is obvious ;)

So - OK.

Thanks,
Richard.

>   /* We do not need to bother analyzing calls to unknown functions
>  unless they may become known during lto/whopr.  */
>   if (!callee->definition && !flag_lto)
>

Re: [PATCH, rs6000] Fix PR92127

[Richard Biener]

On Wed, Oct 30, 2019 at 10:30 AM Kewen.Lin  wrote:
>
> Hi,
>
> As PR92127 shows, recent commit r276645 enables more unrollings,
> two ppc vectorization cost model test cases are fragile and failed
> after the change.  This patch is to disable unrolling for the
> loops of interest to make test cases more robust.
>
> Verified on ppc64-redhat-linux.  Should be fine on powerpc64le which
> supports hw_misalign and would be XFAIL.
>
> Is it ok for trunk?  Thanks in advance!

OK.

(since it's also on my TODO to eventually revert this change...)

Richard.

>
>
> Kewen
>
> 
>
> gcc/testsuite/ChangeLog
>
> 2019-10-30  Kewen Lin  
>
> PR testsuite/92127
> * gcc.dg/vect/costmodel/ppc/costmodel-pr37194.c: Disable unroll.
> * gcc.dg/vect/costmodel/ppc/costmodel-fast-math-vect-pr29925.c: 
> Likewise.
>
>
> diff --git 
> a/gcc/testsuite/gcc.dg/vect/costmodel/ppc/costmodel-fast-math-vect-pr29925.c 
> b/gcc/testsuite/gcc.dg/vect/costmodel/ppc/costmodel-fast-math-vect-pr29925.c
> index a3662e2..34445dc 100644
> --- 
> a/gcc/testsuite/gcc.dg/vect/costmodel/ppc/costmodel-fast-math-vect-pr29925.c
> +++ 
> b/gcc/testsuite/gcc.dg/vect/costmodel/ppc/costmodel-fast-math-vect-pr29925.c
> @@ -13,6 +13,8 @@ interp_pitch(float *exc, float *interp, int pitch, int len)
> for (i=0;i {
>float tmp = 0;
> +  /* PR92127, disable unroll to avoid unexpected profit calculation.  */
> +  #pragma GCC unroll 0
>for (k=0;k<7;k++)
>{
>   tmp += exc[i-pitch+k+maxj-6];
> diff --git a/gcc/testsuite/gcc.dg/vect/costmodel/ppc/costmodel-pr37194.c 
> b/gcc/testsuite/gcc.dg/vect/costmodel/ppc/costmodel-pr37194.c
> index f2395fc..c37e73e 100644
> --- a/gcc/testsuite/gcc.dg/vect/costmodel/ppc/costmodel-pr37194.c
> +++ b/gcc/testsuite/gcc.dg/vect/costmodel/ppc/costmodel-pr37194.c
> @@ -9,6 +9,8 @@ ggSpectrum_Set8(float * data, float d)
>  {
> int i;
>
> +   /* PR92127, disable unroll to avoid unexpected profit calculation.  */
> +   #pragma GCC unroll 0
> for (i = 0; i < 8; i++)
>data[i] = d;
>  }
>

Re: [RFC][PATCH] Show ltrans progress in lto1 process name.

[Richard Biener]

On Wed, Oct 30, 2019 at 9:50 AM Martin Liška  wrote:
>
> Hello.
>
> Have you ever waited for a LTO compilation and were curious about
> the progress? If so, the patch offers a way to communicate that information
> to user via setprocessname. The process will reflect progress of ltrans
> compilation. Please take a look at the attached screenshot.
>
> Thoughts?

Is it April 1st already?  ;)

Seriously no though.

Richard.

> Martin
>
> ---
>  gcc/lto-wrapper.c| 26 --
>  gcc/lto/lang.opt |  3 +++
>  gcc/lto/lto-common.c |  2 +-
>  3 files changed, 24 insertions(+), 7 deletions(-)
>
>

Re: [PATCH V2] rs6000: Refine unroll factor with target unroll_adjust hook.

[Richard Biener]

On Wed, 30 Oct 2019, Jiufu Guo wrote:

> Hi,
> 
> In this patch, loop unroll adjust hook is introduced for powerpc. In this 
> hook,
> we can do target related hueristic adjustment. For this patch, we tunned for
> O2 to unroll small loops with small unroll factor (2 times), for other
> optimization, default unroll factor is used.
> 
> Bootstrapped and regtested on powerpc64le.  Is this ok for trunk?

This looks good to me, needs ppc maintainer approval though.

Thanks,
Richard.

> 
> Jiufu
> BR.
> 
> 
> gcc/
> 2019-10-30  Jiufu Guo 
> 
>   PR tree-optimization/88760
>   * config/rs6000/rs6000.c (rs6000_option_override_internal): Remove
>   code which changes PARAM_MAX_UNROLL_TIMES and PARAM_MAX_UNROLLED_INSNS.
>   (TARGET_LOOP_UNROLL_ADJUST): Add loop unroll adjust hook.
>   (rs6000_loop_unroll_adjust): New hook for loop unroll adjust. Unrolling
>   small loop 2 times for -O2.
> 
>   
> gcc.testsuite/
> 2019-10-29  Jiufu Guo  
> 
>   PR tree-optimization/88760
>   * gcc.dg/pr59643.c: Update back to r277550.
>   * gcc.dg/unroll-8.c: Update cases.
>   * gcc.dg/var-expand3.c: Update cases.
> 
> ---
>  gcc/config/rs6000/rs6000.c | 37 ++---
>  gcc/testsuite/gcc.dg/pr59643.c |  3 ---
>  gcc/testsuite/gcc.dg/unroll-8.c|  4 
>  gcc/testsuite/gcc.dg/var-expand3.c |  2 ++
>  4 files changed, 32 insertions(+), 14 deletions(-)
> 
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index 9ed5151..183dceb 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -1428,6 +1428,9 @@ static const struct attribute_spec 
> rs6000_attribute_table[] =
>  #undef TARGET_VECTORIZE_DESTROY_COST_DATA
>  #define TARGET_VECTORIZE_DESTROY_COST_DATA rs6000_destroy_cost_data
>  
> +#undef TARGET_LOOP_UNROLL_ADJUST
> +#define TARGET_LOOP_UNROLL_ADJUST rs6000_loop_unroll_adjust
> +
>  #undef TARGET_INIT_BUILTINS
>  #define TARGET_INIT_BUILTINS rs6000_init_builtins
>  #undef TARGET_BUILTIN_DECL
> @@ -4540,20 +4543,11 @@ rs6000_option_override_internal (bool global_init_p)
>global_options.x_param_values,
>global_options_set.x_param_values);
>  
> -  /* unroll very small loops 2 time if no -funroll-loops.  */
> +  /* If funroll-loops is implicitly enabled, do not turn fweb or
> +  frename-registers on implicitly.  */
>if (!global_options_set.x_flag_unroll_loops
> && !global_options_set.x_flag_unroll_all_loops)
>   {
> -   maybe_set_param_value (PARAM_MAX_UNROLL_TIMES, 2,
> -  global_options.x_param_values,
> -  global_options_set.x_param_values);
> -
> -   maybe_set_param_value (PARAM_MAX_UNROLLED_INSNS, 20,
> -  global_options.x_param_values,
> -  global_options_set.x_param_values);
> -
> -   /* If fweb or frename-registers are not specificed in command-line,
> -  do not turn them on implicitly.  */
> if (!global_options_set.x_flag_web)
>   global_options.x_flag_web = 0;
> if (!global_options_set.x_flag_rename_registers)
> @@ -5101,6 +5095,27 @@ rs6000_destroy_cost_data (void *data)
>free (data);
>  }
>  
> +/* This target hook implementation for TARGET_LOOP_UNROLL_ADJUST calculates
> +   a new heristic number struct loop *loop should be unrolled.  */
> +
> +static unsigned
> +rs6000_loop_unroll_adjust (unsigned nunroll, struct loop * loop)
> +{
> +  /* For -O2, we only unroll small loops with small unroll factor.  */
> +  if (optimize == 2)
> +{
> +  /* If the loop contains few insns, treated it as small loops.
> +  TODO: Uing 10 hard code for now.  Continue to refine, For example,
> +  if loop constians only 1-2 insns, we may unroll more times(4).
> +  And we may use PARAM to control kinds of loop size.  */
> +  if (loop->ninsns <= 10)
> + return MIN (2, nunroll);
> +  else
> + return 0;
> +}
> +  return nunroll;
> +}
> +
>  /* Handler for the Mathematical Acceleration Subsystem (mass) interface to a
> library with vectorized intrinsics.  */
>  
> diff --git a/gcc/testsuite/gcc.dg/pr59643.c b/gcc/testsuite/gcc.dg/pr59643.c
> index 4446f6e..de78d60 100644
> --- a/gcc/testsuite/gcc.dg/pr59643.c
> +++ b/gcc/testsuite/gcc.dg/pr59643.c
> @@ -1,9 +1,6 @@
>  /* PR tree-optimization/59643 */
>  /* { dg-do compile } */
>  /* { dg-options "-O3 -fdump-tree-pcom-details" } */
> -/* { dg-additional-options "--param max-unrolled-insns=400" { target { 
> powerpc*-*-* } } } */
> -/* Implicit threashold of max-unrolled-insn on ppc at O3 is too small for the
> -   loop of this case.  */
>  
>  void
>  foo (double *a, double *b, double *c, double d, double e, int n)
> diff --git a/gcc/testsuite/gcc.dg/unroll-8.c b/gcc/testsuite/gcc.dg/unroll-8.c
> index b16df67..b1d38a6 100644
> --- a/gcc/testsuite/gcc.dg/unroll-8.c
> 

[PATCH] Fix PR92275

[Richard Biener]

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied.

Richard.

2019-10-30  Richard Biener  

PR tree-optimization/92275
* tree-vect-loop-manip.c (slpeel_update_phi_nodes_for_loops):
Copy all loop-closed PHIs.

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

Index: gcc/tree-vect-loop-manip.c
===
--- gcc/tree-vect-loop-manip.c  (revision 277602)
+++ gcc/tree-vect-loop-manip.c  (working copy)
@@ -2004,6 +2004,29 @@ vect_gen_vector_loop_niters_mult_vf (loo
   *niters_vector_mult_vf_ptr = niters_vector_mult_vf;
 }
 
+/* LCSSA_PHI is a lcssa phi of EPILOG loop which is copied from LOOP,
+   this function searches for the corresponding lcssa phi node in exit
+   bb of LOOP.  If it is found, return the phi result; otherwise return
+   NULL.  */
+
+static tree
+find_guard_arg (class loop *loop, class loop *epilog ATTRIBUTE_UNUSED,
+   gphi *lcssa_phi)
+{
+  gphi_iterator gsi;
+  edge e = single_exit (loop);
+
+  gcc_assert (single_pred_p (e->dest));
+  for (gsi = gsi_start_phis (e->dest); !gsi_end_p (gsi); gsi_next ())
+{
+  gphi *phi = gsi.phi ();
+  if (operand_equal_p (PHI_ARG_DEF (phi, 0),
+  PHI_ARG_DEF (lcssa_phi, 0), 0))
+   return PHI_RESULT (phi);
+}
+  return NULL_TREE;
+}
+
 /* Function slpeel_tree_duplicate_loop_to_edge_cfg duplciates FIRST/SECOND
from SECOND/FIRST and puts it at the original loop's preheader/exit
edge, the two loops are arranged as below:
@@ -2091,6 +2114,29 @@ slpeel_update_phi_nodes_for_loops (loop_
 incoming edge.  */
   adjust_phi_and_debug_stmts (update_phi, second_preheader_e, arg);
 }
+
+  /* For epilogue peeling we have to make sure to copy all LC PHIs
+ for correct vectorization of live stmts.  */
+  if (loop == first)
+{
+  basic_block orig_exit = single_exit (second)->dest;
+  for (gsi_orig = gsi_start_phis (orig_exit);
+  !gsi_end_p (gsi_orig); gsi_next (_orig))
+   {
+ gphi *orig_phi = gsi_orig.phi ();
+ tree orig_arg = PHI_ARG_DEF (orig_phi, 0);
+ if (TREE_CODE (orig_arg) != SSA_NAME || virtual_operand_p  (orig_arg))
+   continue;
+
+ /* Already created in the above loop.   */
+ if (find_guard_arg (first, second, orig_phi))
+   continue;
+
+ tree new_res = copy_ssa_name (orig_arg);
+ gphi *lcphi = create_phi_node (new_res, between_bb);
+ add_phi_arg (lcphi, orig_arg, single_exit (first), UNKNOWN_LOCATION);
+   }
+}
 }
 
 /* Function slpeel_add_loop_guard adds guard skipping from the beginning
@@ -2175,29 +2221,6 @@ slpeel_update_phi_nodes_for_guard1 (clas
 }
 }
 
-/* LCSSA_PHI is a lcssa phi of EPILOG loop which is copied from LOOP,
-   this function searches for the corresponding lcssa phi node in exit
-   bb of LOOP.  If it is found, return the phi result; otherwise return
-   NULL.  */
-
-static tree
-find_guard_arg (class loop *loop, class loop *epilog ATTRIBUTE_UNUSED,
-   gphi *lcssa_phi)
-{
-  gphi_iterator gsi;
-  edge e = single_exit (loop);
-
-  gcc_assert (single_pred_p (e->dest));
-  for (gsi = gsi_start_phis (e->dest); !gsi_end_p (gsi); gsi_next ())
-{
-  gphi *phi = gsi.phi ();
-  if (operand_equal_p (PHI_ARG_DEF (phi, 0),
-  PHI_ARG_DEF (lcssa_phi, 0), 0))
-   return PHI_RESULT (phi);
-}
-  return NULL_TREE;
-}
-
 /* LOOP and EPILOG are two consecutive loops in CFG and EPILOG is copied
from LOOP.  Function slpeel_add_loop_guard adds guard skipping from a
point between the two loops to the end of EPILOG.  Edges GUARD_EDGE
Index: gcc/testsuite/gcc.dg/torture/pr92275.c
===
--- gcc/testsuite/gcc.dg/torture/pr92275.c  (nonexistent)
+++ gcc/testsuite/gcc.dg/torture/pr92275.c  (working copy)
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-ftree-vectorize" } */
+
+unsigned long a, c;
+int *b, *b2;
+long d;
+
+void fn1()
+{
+  for (; b < b2; b++)
+d += *b * c;
+  d *= a;
+}

Re: [SLP] SLP vectorization: vectorize vector constructors

[Richard Biener]

On Wed, 30 Oct 2019, Joel Hutton wrote:

> On 15/10/2019 13:11, Richard Biener wrote:
>  >>  > You miss to check that CONSTRUCTOR_NELTS == TYPE_VECTOR_SUBPARTS
>  >>  > (we can have omitted trailing zeros).
>  >
>  > ^^^
>  >
>  > I don't see this being handled?  You give up on non-SSA names
>  > but not on the omitted trailing zeros.
> 
> I had thought checking the number of vectors produced would work.

But that's too late to fail.

> I've added that check.
> I'm slightly confused about what should be done for non-SSA names. 
> There's no scalar stmt to gather for a constant in a vector constructor.

For now simply give up.  What could be done is "compact" the
operands to vectorize to only contain SSA names, try to SLP
match and vectorize them and then combine the vectorized result
with the constant elements.

Not worth doing I guess unless it's sth regular like

 { a, b, c, d, 0, 0, 0, 0 }

or so.  But this can be handled as followup if necessary.

>  >
>  > You build a CONSTRUCTOR of vectors, thus
>  >
>  > _orig_ssa_1 = { vect_part1_2, vect_part2_3, ... };
> I've added code to do this, and a testcase which triggers it.

Great.

>  >
>  > +
>  > + if (constructor)
>  > +   {
>  > + SLP_INSTANCE_ROOT_STMT (new_instance) = stmt_info->stmt;
>  > +   }
>  > + else
>  > +   SLP_INSTANCE_ROOT_STMT (new_instance) = NULL;
>  > +
>  >
>  > too much vertical space, no {} around single-stmt if clauses
> Fixed.
> 
>  >
>  >
>  > @@ -2725,6 +2760,10 @@ vect_bb_slp_scalar_cost (basic_block bb,
>  > stmt_vec_info use_stmt_info = vinfo->lookup_stmt
>  > (use_stmt);
>  > if (!use_stmt_info || !PURE_SLP_STMT (use_stmt_info))
>  >   {
>  > +   /* Check this is not a constructor that will be
>  > vectorized
>  > +  away.  */
>  > +   if (BB_VINFO_GROUPED_STORES (vinfo).contains
>  > (use_stmt_info))
>  > +   continue;
>  >
>  > hmm, so why not set the slp type on SLP_INSTANCE_ROOT_STMT instead?
>  > In theory the stmt should be part of the SLP tree itself but that's
>  > probably too awkward to be made work at the moment ;)
> I did try this, but it was indeed very awkward to be made to work.
>
>  >
>  > vect_ssa_use_outside_bb and vect_slp_check_for_constructors are new
>  > functions which need comments.
> Fixed. I've also taken the 'vectorize the root' out into a separate 
> function.
> 
>  >
>  > +  /* For vector constructors, the same SSA name must be used to 
> maintain
>  > data
>  > + flow into other basic blocks.  */
>  > +  if (instance->root == node && SLP_INSTANCE_ROOT_STMT (instance)
>  > +  && SLP_TREE_NUMBER_OF_VEC_STMTS (node) == 1
>  > +  && SLP_TREE_VEC_STMTS (node).exists ())
>  > +    {
>  >
>  > it should read
>  >
>  >   /* Vectorize the instance root.  */
>  >
>  > and be in vect_schedule_slp after the vect_schedule_slp_instance.
>  > As said above you need to handle SLP_TREE_NUMBER_OF_VEC_STMTS > 1,
>  > you also cannot simply do "nothing" here, "failing" vectorization
>  > (well, you probably can - DCE will remove the vectorized code - but
>  > at least if there were calls in the SLP tree they will be mangled
>  > by vectorization so the scalar code is wrecked).  SO it should be
>  >
>  >  if (SLP_INSTANCE_ROOT_STMT (instance))
>  >    .. you may not fail to replace the scalar root stmt here ..
>  >
> So what should be done in the case that CONSTRUCTOR_NELTS < 
> TYPE_VECTOR_SUBPARTS?

You have to fail SLP discovery, not code generation.  The check
you did above should have done this.
 
>  > + if (CONSTRUCTOR_NELTS (rhs) == 0)
>  > +   vectorizable = false;
>  > +
>  >
>  > if you use continue; you can elide the 'vectorizable' variable.
> Done.
> 
>  >
>  > + if (!vect_ssa_use_outside_bb (gimple_assign_lhs (stmt)))
>  > +   vectorizable = false;
>  > +
>  >
>  > why's that?  no comments that clarify ;)  The vector may be
>  > used as argument to a call or as source of a store.  So I'd simply
>  > remove this check (and the function).
> 
> Done. The thinking was that if the vector was used as a source of a 
> store the SLP tree would be built from the grouped store instead. Will 
> it not cause problems if both the grouped store and the vector 
> constructor are used to build SLP trees?
> 
> 
> 
> 2019-10-10  Joel Hutton  
> 
>  * expr.c (store_constructor): Add case for constructor of vectors.

Why do you need this?  The vectorizer already creates such CTORs.  Any
testcase that you can show?

>  * tree-vect-slp.c (vect_analyze_slp_instance): Add case for vector 
> constructors.
>  (vect_bb_slp_scalar_cost): Likewise.
>  (vect_slp_check_for_constructors): New function.
>  (vect_slp_analyze_bb_1): Add check for vector constructors.
>  (vect_schedule_slp_instance): Add case to fixup vector constructor 
> stmt.
>  

Re: [C++ PATCH] Fix up decl_in_std_namespace_p handling of --enable-symvers=gnu-versioned-namespace

[Marek Polacek]

On Wed, Oct 30, 2019 at 10:58:57AM +0100, Jakub Jelinek wrote:
> On Fri, Oct 25, 2019 at 10:44:18AM -0400, Marek Polacek wrote:
> > That is... sneaky.  I guess I/we need to test with
> > --enable-symvers=gnu-versioned-namespace every now and then.
> 
> Indeed.
> 
> > Probably deserves a comment.
> 
> Ok, here it is with a comment, bootstrapped/regtested again last night:

Thanks, looks good!

--
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA

[SLP] SLP vectorization: vectorize vector constructors

[Joel Hutton]

On 15/10/2019 13:11, Richard Biener wrote:
 >>  > You miss to check that CONSTRUCTOR_NELTS == TYPE_VECTOR_SUBPARTS
 >>  > (we can have omitted trailing zeros).
 >
 > ^^^
 >
 > I don't see this being handled?  You give up on non-SSA names
 > but not on the omitted trailing zeros.

I had thought checking the number of vectors produced would work. I've 
added that check.
I'm slightly confused about what should be done for non-SSA names. 
There's no scalar stmt to gather for a constant in a vector constructor.

 >
 > You build a CONSTRUCTOR of vectors, thus
 >
 > _orig_ssa_1 = { vect_part1_2, vect_part2_3, ... };
I've added code to do this, and a testcase which triggers it.

 >
 > +
 > + if (constructor)
 > +   {
 > + SLP_INSTANCE_ROOT_STMT (new_instance) = stmt_info->stmt;
 > +   }
 > + else
 > +   SLP_INSTANCE_ROOT_STMT (new_instance) = NULL;
 > +
 >
 > too much vertical space, no {} around single-stmt if clauses
Fixed.

 >
 >
 > @@ -2725,6 +2760,10 @@ vect_bb_slp_scalar_cost (basic_block bb,
 > stmt_vec_info use_stmt_info = vinfo->lookup_stmt
 > (use_stmt);
 > if (!use_stmt_info || !PURE_SLP_STMT (use_stmt_info))
 >   {
 > +   /* Check this is not a constructor that will be
 > vectorized
 > +  away.  */
 > +   if (BB_VINFO_GROUPED_STORES (vinfo).contains
 > (use_stmt_info))
 > +   continue;
 >
 > hmm, so why not set the slp type on SLP_INSTANCE_ROOT_STMT instead?
 > In theory the stmt should be part of the SLP tree itself but that's
 > probably too awkward to be made work at the moment ;)
I did try this, but it was indeed very awkward to be made to work.

 >
 > vect_ssa_use_outside_bb and vect_slp_check_for_constructors are new
 > functions which need comments.
Fixed. I've also taken the 'vectorize the root' out into a separate 
function.

 >
 > +  /* For vector constructors, the same SSA name must be used to 
maintain
 > data
 > + flow into other basic blocks.  */
 > +  if (instance->root == node && SLP_INSTANCE_ROOT_STMT (instance)
 > +  && SLP_TREE_NUMBER_OF_VEC_STMTS (node) == 1
 > +  && SLP_TREE_VEC_STMTS (node).exists ())
 > +    {
 >
 > it should read
 >
 >   /* Vectorize the instance root.  */
 >
 > and be in vect_schedule_slp after the vect_schedule_slp_instance.
 > As said above you need to handle SLP_TREE_NUMBER_OF_VEC_STMTS > 1,
 > you also cannot simply do "nothing" here, "failing" vectorization
 > (well, you probably can - DCE will remove the vectorized code - but
 > at least if there were calls in the SLP tree they will be mangled
 > by vectorization so the scalar code is wrecked).  SO it should be
 >
 >  if (SLP_INSTANCE_ROOT_STMT (instance))
 >    .. you may not fail to replace the scalar root stmt here ..
 >
So what should be done in the case that CONSTRUCTOR_NELTS < 
TYPE_VECTOR_SUBPARTS?

 > + if (CONSTRUCTOR_NELTS (rhs) == 0)
 > +   vectorizable = false;
 > +
 >
 > if you use continue; you can elide the 'vectorizable' variable.
Done.

 >
 > + if (!vect_ssa_use_outside_bb (gimple_assign_lhs (stmt)))
 > +   vectorizable = false;
 > +
 >
 > why's that?  no comments that clarify ;)  The vector may be
 > used as argument to a call or as source of a store.  So I'd simply
 > remove this check (and the function).

Done. The thinking was that if the vector was used as a source of a 
store the SLP tree would be built from the grouped store instead. Will 
it not cause problems if both the grouped store and the vector 
constructor are used to build SLP trees?



2019-10-10  Joel Hutton  

 * expr.c (store_constructor): Add case for constructor of vectors.
 * tree-vect-slp.c (vect_analyze_slp_instance): Add case for vector 
constructors.
 (vect_bb_slp_scalar_cost): Likewise.
 (vect_slp_check_for_constructors): New function.
 (vect_slp_analyze_bb_1): Add check for vector constructors.
 (vect_schedule_slp_instance): Add case to fixup vector constructor 
stmt.
 (vectorize_slp_instance_root_stmt): New function
 * tree-vectorizer.h (SLP_INSTANCE_ROOT_STMT): New field.

gcc/testsuite/ChangeLog:

2019-10-10  Joel Hutton  

 * gcc.dg/vect/bb-slp-40.c: New test.
 * gcc.dg/vect/bb-slp-41.c: New test.

From 902510bd498acfc9e30636f8267b57027bc63254 Mon Sep 17 00:00:00 2001
From: Joel Hutton 
Date: Tue, 22 Oct 2019 10:05:19 +0100
Subject: [PATCH] SLP Vectorization: Vectorize vector constructors

---
 gcc/expr.c|   5 +-
 gcc/testsuite/gcc.dg/vect/bb-slp-40.c |  34 +
 gcc/testsuite/gcc.dg/vect/bb-slp-41.c |  61 +
 gcc/tree-vect-slp.c   | 171 +-
 gcc/tree-vectorizer.h |   5 +
 5 files changed, 273 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/vect/bb-slp-40.c
 create mode 100644 

[committed] Add testcase for C++ score parsing

[Jakub Jelinek]

Hi!

While in C a constant expression can't start with 
score(constant-integral-expression),
in C++11 it can and so we need to do tentative parsing or skipping to the
closing ) to check if there is : to find out if it is trait-score or just
part of a constant expression.

The following patch adds a testcase for that, tested on x86_64-linux,
committed to trunk.

2019-10-30  Jakub Jelinek  

* g++.dg/gomp/declare-variant-6.C: New test.

--- gcc/testsuite/g++.dg/gomp/declare-variant-6.C.jj2019-10-30 
13:11:36.534957481 +0100
+++ gcc/testsuite/g++.dg/gomp/declare-variant-6.C   2019-10-30 
13:12:14.797373585 +0100
@@ -0,0 +1,26 @@
+// Test parsing of #pragma omp declare variant
+// { dg-do compile { target c++11 } }
+// { dg-additional-options "-fdump-tree-gimple" }
+
+constexpr int score (int x) { return x; }
+void f0 ();
+#pragma omp declare variant (f0) match (user={condition(score(1))})
+void f1 ();
+void f2 ();
+#pragma omp declare variant (f2) match (user={condition(score(1):1)})
+void f3 ();
+void f4 ();
+#pragma omp declare variant (f4) match (user={condition(score(1):score(1))})
+void f5 ();
+void f6 ();
+#pragma omp declare variant (f6) match (user={condition(score(1)==0)})
+void f7 ();
+
+void
+test ()
+{
+  f1 ();   // { dg-final { scan-tree-dump-times "f0 \\\(\\\);" 1 "gimple" 
} }
+  f3 ();   // { dg-final { scan-tree-dump-times "f2 \\\(\\\);" 1 "gimple" 
} }
+  f5 ();   // { dg-final { scan-tree-dump-times "f4 \\\(\\\);" 1 "gimple" 
} }
+  f7 ();   // { dg-final { scan-tree-dump-times "f7 \\\(\\\);" 1 "gimple" 
} }
+}

Jakub

Re: Watch for missing summaries even more

[Martin Jambor]

Hi,

On Wed, Oct 30 2019, Jan Hubicka wrote:
> Hi,
> this patch fixes another place we may have missing argument summary.
> Here the situation is that the call site being inlined has no jump
> functions while function which is being inlines has another call with
> jump function.  This can validly happen when we inline into functions
> with indirect inlining and ipa-cp disabled but I am not 100% why it
> happens i.e. during Firefox builds.  Martin, do you have any ideas?

No, not without seeing what is going on.  I think we compute jump
functions also when we are inlining and IPA-CP is disabled, by the way.

It looks like ipa_compute_jump_functions_for_edge was never called on
that edge, so if the problem appeared in a non-LTO context the following
might help?  And I assume we want it either way, so OK for trunk after a
bootstrap?

Martin


diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 0dd73561419..10fe1bc929f 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -2040,7 +2040,7 @@ ipa_compute_jump_functions_for_bb (struct 
ipa_func_body_info *fbi, basic_block b
 
   if (callee)
{
- callee->ultimate_alias_target ();
+ callee = callee->ultimate_alias_target ();
  /* We do not need to bother analyzing calls to unknown functions
 unless they may become known during lto/whopr.  */
  if (!callee->definition && !flag_lto)

Re: [PATCH 14/14] Add D Phobos config, makefiles, and testsuite.

[Thomas Schwinge]

Hi!

Just for posterity.

On 2019-04-20T23:22:31+0200, Iain Buclaw  wrote:
> On Sat, 20 Apr 2019 at 22:30, Thomas Schwinge  wrote:
>> On Tue, 18 Sep 2018 02:39:46 +0200, Iain Buclaw  
>> wrote:
>> > This patch adds the configure and make files used for building D
>> > runtime and Phobos.  As well as running all unittests and the
>> > testsuite.
>>
>> With a x86_64-pc-linux-gnu build, I've noticed breakage in '-m32'
>> multilib testing, made apparent by message: "[...]:
>> /lib/i386-linux-gnu/libgcc_s.so.1: version `GCC_7.0.0' not found
>> (required by [...])".  (That is, the system 'libgcc_s.so.1' being
>> dynamically linked instead of the just built one.)  This is because of
>> incomplete 'gccdir' setup in the '*.exp' file.  In such a multilibbed
>> configuration, there are 'build-gcc/gcc/libgcc.*' and
>> 'build-gcc/gcc/32/libgcc.*' (for example); for '-m32' multilib testing,
>> paths need to be set up to point to the latter instead of the former.  It
>> seems as if some of this '*.exp' stuff has been copied from libffi (?);
>> the attached patch copies the missing pieces from there, too.  I've been
>> tempted to commit this "as obvious", but then thought I'll still get some
>> review/approval first.  If approving this patch, please respond with
>> "Reviewed-by: NAME " so that your effort will be recorded in the
>> commit log, see .
>
> Seems reasonable to me.

I didn't get this committed promptly -- and in the mean time (what's half
a year, eh?), Bernd addressed the same issue:
.

There remain a few incremental changes from his committed to my
originally proposed version (see attached), but I'm not committing that
now, as it's not affecting test results, and refactoring all the '*.exp'
files, unifying what has been copied from elsewhere, is a (big) task for
another day (weeks...).


Grüße
 Thomas


From 0bd05a08a98a95d56b4b469dd6e68d485e70ab69 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Fri, 4 Oct 2019 13:06:08 +0200
Subject: [PATCH] Fix libphobos testsuite libgcc multilib search path
 (remaining pieces)

Changes similar to my original ones have already been committed in r275332.
---
 libphobos/testsuite/lib/libphobos.exp | 21 ++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/libphobos/testsuite/lib/libphobos.exp b/libphobos/testsuite/lib/libphobos.exp
index 056e8f1d444..866090656c6 100644
--- a/libphobos/testsuite/lib/libphobos.exp
+++ b/libphobos/testsuite/lib/libphobos.exp
@@ -106,6 +106,15 @@ proc libphobos_init { args } {
 global gluefile wrap_flags
 global ld_library_path
 global DEFAULT_DFLAGS
+global GCC_UNDER_TEST
+
+if ![info exists GCC_UNDER_TEST] then {
+	if [info exists TOOL_EXECUTABLE] {
+	set GCC_UNDER_TEST $TOOL_EXECUTABLE
+	} else {
+	set GCC_UNDER_TEST "[find_gcc]"
+	}
+}
 
 # If a testcase doesn't have special options, use these.
 if ![info exists DEFAULT_DFLAGS] then {
@@ -153,13 +162,14 @@ proc libphobos_init { args } {
 	set exeext $env(EXEEXT)
 }
 
-# Compute what needs to be added to the existing LD_LIBRARY_PATH.
+# Compute what needs to be put into LD_LIBRARY_PATH
 set ld_library_path "."
 
+# Locate libgcc.a so we don't need to account for different values of
+# SHLIB_EXT on different platforms
 set gccdir [lookfor_file $tool_root_dir gcc/libgcc.a]
 if {$gccdir != ""} {
 	set gccdir [file dirname $gccdir]
-	append ld_library_path ":${gccdir}"
 }
 
 if { [file exists "${blddir}/libdruntime/.libs/libgdruntime.${shlib_ext}"] } {
@@ -172,7 +182,12 @@ proc libphobos_init { args } {
 
 # Compute what needs to be added to the existing LD_LIBRARY_PATH.
 if {$gccdir != ""} {
-	set compiler ${gccdir}/gdc
+	# Add AIX pthread directory first.
+	if { [llength [glob -nocomplain ${gccdir}/pthread/libgcc_s*.a]] >= 1 } {
+	append ld_library_path ":${gccdir}/pthread"
+	}
+	append ld_library_path ":${gccdir}"
+	set compiler [lindex $GCC_UNDER_TEST 0]
 
 	if { [is_remote host] == 0 && [which $compiler] != 0 } {
 	  foreach i "[exec $compiler --print-multi-lib]" {
-- 
2.17.1



signature.asc
Description: PGP signature

Re: [committed][AArch64] Add support for the SVE PCS

[Andreas Schwab]

Program received signal SIGSEGV, Segmentation fault.
aarch64_sve::svbool_type_p (type=)
at ../../gcc/config/aarch64/aarch64-sve-builtins.cc:3239
3239  && TYPE_MAIN_VARIANT (type) == TYPE_MAIN_VARIANT (abi_type));
(gdb) bt
#0  aarch64_sve::svbool_type_p (type=)
at ../../gcc/config/aarch64/aarch64-sve-builtins.cc:3239
#1  0x0143bf78 in aarch64_sve_argument_p (
type=, num_zr=0xe048, 
num_pr=0xe04c) at ../../gcc/config/aarch64/aarch64.c:1938
#2  0x01457c98 in aarch64_pass_by_reference (pcum_v=..., arg=...)
at ../../gcc/config/aarch64/aarch64.c:4746
#3  0x00b9b3c8 in pass_by_reference (ca=ca@entry=0x0, arg=...)
at ../../gcc/target.h:263
#4  0x0074e0a8 in default_pass_by_ref (
gnu_type=) at ../../gcc/tree.h:3391
#5  0x00760a5c in type_for_nonaliased_component_p (
gnu_type=)
at ../../gcc/ada/gcc-interface/utils.c:5683
#6  0x0077c76c in create_field_decl (
name=name@entry=, 
type=type@entry=, 
record_type=record_type@entry=, 
size=, size@entry=, pos=pos@entry=, 
packed=packed@entry=0, addressable=addressable@entry=0)
at ../../gcc/ada/gcc-interface/utils.c:2823
#7  0x00743f34 in gnat_to_gnu_field (gnat_field=gnat_field@entry=1692, 
gnu_record_type=gnu_record_type@entry=, packed=0, packed@entry=916696503, 
definition=definition@entry=false, 
debug_info_p=debug_info_p@entry=192)
at ../../gcc/ada/gcc-interface/decl.c:7282
#8  0x00745af8 in components_to_record (gnat_component_list=40, 
gnat_record_type=0, gnat_record_type@entry=1685, 
gnu_field_list=gnu_field_list@entry=, 
gnu_record_type=, packed=916696503, 
packed@entry=0, definition=definition@entry=18, 
cancel_alignment=cancel_alignment@entry=false, all_rep=all_rep@entry=64, 
unchecked_union=false, unchecked_union@entry=50, artificial=true, 
artificial@entry=false, debug_info=true, debug_info@entry=37, 
maybe_unused=maybe_unused@entry=false, 
first_free_pos=, 
p_gnu_rep_list=p_gnu_rep_list@entry=0x0)
at ../../gcc/ada/gcc-interface/decl.c:7596
#9  0x007334bc in gnat_to_gnu_entity (gnat_entity=, 
gnu_expr=gnu_expr@entry=, definition=definition@entry=false)
at ada/sinfo.h:493
#10 0x0073ef68 in gnat_to_gnu_type (gnat_entity=)
at ../../gcc/ada/gcc-interface/decl.c:4809
#11 0x007bb1ac in gigi (gnat_root=gnat_root@entry=2269, 
max_gnat_node=max_gnat_node@entry=3164, 
number_name=number_name@entry=1899, 
nodes_ptr=nodes_ptr@entry=0xbec9d010, 
flags_ptr=flags_ptr@entry=0x29183b0, next_node_ptr=, 
prev_node_ptr=, elists_ptr=, 
elmts_ptr=, strings_ptr=, 
strings_ptr@entry=0x2933540, string_chars_ptr=, 
string_chars_ptr@entry=0x2930e20, list_headers_ptr=, 
list_headers_ptr@entry=0x2924710, number_file=, 
file_info_ptr=, file_info_ptr@entry=0xe930, 
standard_boolean=, standard_integer=, 
standard_character=, 
standard_long_long_float=, 
standard_exception_type=, 
gigi_operating_mode=, gigi_operating_mode@entry=0)
at ../../gcc/ada/gcc-interface/trans.c:452
#12 0x00b5a2d4 in back_end.call_back_end (
mode=mode@entry=generate_object) at ../../gcc/ada/back_end.adb:155
#13 0x00b5ba24 in gnat1drv () at ../../gcc/ada/gnat1drv.adb:1649
#14 0x0074b0e8 in gnat_parse_file ()
at ../../gcc/ada/gcc-interface/misc.c:119
#15 0x01096df0 in compile_file () at ../../gcc/toplev.c:456
#16 0x0071b6a4 in do_compile () at ../../gcc/toplev.c:2176
#17 toplev::main (this=this@entry=0xec48, argc=, 
argc@entry=37, argv=, argv@entry=0xed98)
at ../../gcc/toplev.c:2311
#18 0x0071e37c in main (argc=37, argv=0xed98)
at ../../gcc/main.c:39
(gdb) p abi_type
$2 = 
(gdb) p abi_vector_types
$3 = { }

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

Re: [PATCH 0/9] IPA ICF overhaul

[Martin Liška]

Hi.

After a discussion with Honza, he recommended to install the patch set
(which should be completely approved) without the last missing piece:
[PATCH 2/9] operand_equal_p: add support for FIELD_DECL

He's going to write a function that will compare two access paths.
I've retested the whole patch without the patch and built also
Firefox with it. I'm going to install it.

Martin


pEpkey.asc
Description: application/pgp-keys

Re: [PATCH] [LIBPHOBOS] Fix multi-lib RUNTESTFLAGS handling

[Thomas Schwinge]

Hi!

On 2019-09-03T10:04:14+0200, Iain Buclaw  wrote:
> On Tue, 3 Sep 2019 at 08:10, Bernd Edlinger  wrote:
>> I've noticed that testing libphobos fails for multi-lib configs:
>>
>> $ make check-target-libphobos RUNTESTFLAGS="--target_board=unix\{-m32,\}"
>>
>> fails for every 32bit execution, because the host libgcc_s.so is used which
>> is not the correct version:
>>
>> spawn [open ...]
>> ./test_aa.exe: /lib/i386-linux-gnu/libgcc_s.so.1: version `GCC_7.0.0' not 
>> found (required by ./test_aa.exe)
>> FAIL: libphobos.aa/test_aa.d execution test
>>
>> This can be fixed by adding a few lines from 
>> libstdc++/testsuite/lib/libstdc++.exp
>> to libphobos/testsuite/lib/libphobos.exp, see attached patch.
>>
>>
>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>> Is it OK for trunk?
>
> OK.

The very same problem existed since the beginning of D language support
in GCC, so I backported this to gcc-9-branch in r277611 "[LIBPHOBOS] Fix
multi-lib RUNTESTFLAGS handling", see attached.


Grüße
 Thomas


From 2dba914d0a00623209ce8f9ceaf28e2d5e2ceb06 Mon Sep 17 00:00:00 2001
From: tschwinge 
Date: Wed, 30 Oct 2019 11:51:15 +
Subject: [PATCH] [LIBPHOBOS] Fix multi-lib RUNTESTFLAGS handling

Testing libphobos fails for multi-lib configs:

$ make check-target-libphobos RUNTESTFLAGS="--target_board=unix\{-m32,\}"

fails for every 32bit execution, because the host libgcc_s.so is used which
is not the correct version:

spawn [open ...]
./test_aa.exe: /lib/i386-linux-gnu/libgcc_s.so.1: version `GCC_7.0.0' not found (required by ./test_aa.exe)
FAIL: libphobos.aa/test_aa.d execution test

This can be fixed by adding a few lines from libstdc++/testsuite/lib/libstdc++.exp
to libphobos/testsuite/lib/libphobos.exp

Backport trunk r275332:

	libphobos/
	2019-09-03  Bernd Edlinger  

	* testsuite/lib/libphobos.exp (libphobos_init): Add multi-lib libgcc
	dirs to the ld_library_path var.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/branches/gcc-9-branch@277611 138bc75d-0d04-0410-961f-82ee72b054a4
---
 libphobos/ChangeLog   |  9 +
 libphobos/testsuite/lib/libphobos.exp | 19 +++
 2 files changed, 28 insertions(+)

diff --git a/libphobos/ChangeLog b/libphobos/ChangeLog
index 24de0ab9bd8..becd8b1ec86 100644
--- a/libphobos/ChangeLog
+++ b/libphobos/ChangeLog
@@ -1,3 +1,12 @@
+2019-10-30  Thomas Schwinge  
+
+	Backport from trunk:
+
+	2019-09-03  Bernd Edlinger  
+
+	* testsuite/lib/libphobos.exp (libphobos_init): Add multi-lib libgcc
+	dirs to the ld_library_path var.
+
 2019-08-12  Release Manager
 
 	* GCC 9.2.0 released.
diff --git a/libphobos/testsuite/lib/libphobos.exp b/libphobos/testsuite/lib/libphobos.exp
index d3fe75358c8..056e8f1d444 100644
--- a/libphobos/testsuite/lib/libphobos.exp
+++ b/libphobos/testsuite/lib/libphobos.exp
@@ -170,6 +170,25 @@ proc libphobos_init { args } {
 	append ld_library_path ":${blddir}/src/.libs"
 }
 
+# Compute what needs to be added to the existing LD_LIBRARY_PATH.
+if {$gccdir != ""} {
+	set compiler ${gccdir}/gdc
+
+	if { [is_remote host] == 0 && [which $compiler] != 0 } {
+	  foreach i "[exec $compiler --print-multi-lib]" {
+	set mldir ""
+	regexp -- "\[a-z0-9=_/\.-\]*;" $i mldir
+	set mldir [string trimright $mldir "\;@"]
+	if { "$mldir" == "." } {
+	  continue
+	}
+	if { [llength [glob -nocomplain ${gccdir}/${mldir}/libgcc_s*.so.*]] >= 1 } {
+	  append ld_library_path ":${gccdir}/${mldir}"
+	}
+	  }
+	}
+}
+
 set_ld_library_path_env_vars
 
 libphobos_maybe_build_wrapper "${objdir}/testglue.o"
-- 
2.17.1



signature.asc
Description: PGP signature

Re: [committed][AArch64] Add support for the SVE PCS

[Andreas Schwab]

This breaks boostrap.

/opt/gcc/gcc-20191030/Build/./prev-gcc/xgcc 
-B/opt/gcc/gcc-20191030/Build/./prev-gcc/ -B/usr/aarch64-suse-linux/bin/ 
-B/usr/aarch64-suse-linux/bin/ -B/usr/aarch64-suse-linux/lib/ -isystem 
/usr/aarch64-suse-linux/include -isystem /usr/aarch64-suse-linux/sys-include   
-fno-checking -c -g -O2 -fno-checking -gtoggle  -gnatpg -gnata -W -Wall 
-nostdinc -I- -I. -Iada/generated -Iada -I../../gcc/ada 
-I../../gcc/ada/gcc-interface -Iada/libgnat -I../../gcc/ada/libgnat 
../../gcc/ada/libgnat/a-charac.ads -o ada/libgnat/a-charac.o
mkdir -p ada/libgnat/
+===GNAT BUG DETECTED==+
| 10.0.0 20191030 (experimental) [trunk revision 277599] (aarch64-suse-linux) |
| Storage_Error stack overflow or erroneous memory access  |
| Error detected at system.ads:184:5   |
| Please submit a bug report; see https://gcc.gnu.org/bugs/ .  |
| Use a subject line meaningful to you and us to track the bug.|
| Include the entire contents of this bug box in the report.   |
| Include the exact command that you entered.  |
| Also include sources listed below.   |
+==+

Please include these source files with error report
Note that list may not be accurate in some cases,
so please double check that the problem can still
be reproduced with the set of files listed.
Consider also -gnatd.n switch (see debug.adb).

../../gcc/ada/libgnat/system.ads
../../gcc/ada/libgnat/a-charac.ads
../../gcc/ada/libgnat/ada.ads

compilation abandoned
make[3]: *** [../../gcc/ada/gcc-interface/Make-lang.in:144: 
ada/libgnat/a-charac.o] Error 1

Andreas.

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

Re: [PATCH] Fix PR90316

[Thomas Schwinge]

Hi!

On 2019-10-30T12:19:28+0100, Jakub Jelinek  wrote:
> On Wed, Oct 30, 2019 at 11:57:12AM +0100, Thomas Schwinge wrote:
>> ..., and when building gcc-9-branch with
>> '--enable-checking=yes,extra,rtl' (apparently I'm the only one doing
>> that, huh?), runs into the following (at least I suppose that's what's
>
> I'm testing release branches with
> ../configure --enable-languages=default,ada,obj-c++,lto,go,brig,d 
> --enable-checking=yes,rtl,extra
> too, and saw (last time I've bootstrapped/regtested gcc-9-branch
> this way was Oct 21st though):
> ../../gcc/machmode.h: In function ‘dw_loc_descr_node* mem_loc_descriptor(rtx, 
> machine_mode, machine_mode, var_init_status)’:
> ../../gcc/machmode.h:520:42: warning: ‘int_mode’ may be used uninitialized in 
> this function [-Wmaybe-uninitialized]
>   520 |? mode_size_inline (mode) : mode_size[mode]);
>   |  ^~~~
> ../../gcc/dwarf2out.c:15464:19: note: ‘int_mode’ was declared here
> 15464 |   scalar_int_mode int_mode, inner_mode, op1_mode;
>   |   ^~~~
> and no fatal error.  Are you using --enable-werror or something similar
> in addition?

Eh, you're right, of course: '--enable-werror', not '--enable-checking'
is relevant here.  (Recovering from a cold -- brain still a bit "slow".)


Grüße
 Thomas


signature.asc
Description: PGP signature

Re: introduce -fcallgraph-info option

[Richard Biener]

On Wed, 30 Oct 2019, Alexandre Oliva wrote:

> On Oct 28, 2019, Richard Biener  wrote:
> 
> > I guess you need to elaborate on 'per-file'.  With LTO as far as I
> > understand you'll get the graph per LTRANS unit (did you check
> > where the output is generated?).
> 
> Yeah, I guess this was not designed with LTO in mind; it probably even
> predates LTO.  We get per-LTRANS unit indeed, and the output is
> generated in the temporary dir, which is not desirable behavior for
> sure.  The outputs seem to be usable if you can figure out what they
> are, but I'm not sure how to go about combining the multiple .ci files,
> or how to name the combined output, since it's not generally expected
> that these files will be created at link time, rather than at compile
> time.  I'll bring this up internally and come back with some
> improvement.
> 
> > Is this mainly a debugging tool or does it serve a different purpose?
> 
> It feeds gnatstack, that's a tool to compute max stack depth and perform
> other call graph analyzes.  I don't think of it as a debugging tool.
> 
> https://www.adacore.com/gnatpro/toolsuite/gnatstack
> http://docs.adacore.com/live/wave/gnatstack/html/gnatstack_ug/index.html

Ah, thanks for clarification.  One way of operation would be to
generate the graph during the compilation step even with LTO though
it then becomes much less precise.  Note that during LTRANS you
could get at the original file via the DECL_SOURCE_LOCATION of
the TRANSLATION_UNIT_DECL each function is ultimately rooted in
so there's the vague possibility to annotate the graph accordingly
to help combining the output.  Additional arguments to
-fcallgraph-info might be used to direct the output to a specific
directory as well.

Richard.

[PATCH] PR c++/84810 - constraints on lambdas

[Jeff Chapman]

Hello,

Attached is a patch that adds parsing of the optional requires-clause in a
lambda-expression and lambda-declarator. Additionally, shorthand constraints
from the template-parameter-list are now actually applied and constrain the
synthesized operator().

Previously we were not parsing the requires clauses at all and not saving the
shorthand constraints in the place expected by grokfndecl.

The trailing requires-clause is now also used to suppress synthesis of the
conversion to function pointer for non-capturing non-generic lambdas as per
expr.prim.lambda.closure/7.

This includes a fix to template_class_depth. Previously it was computing the
wrong depth for lambdas in the initializer of a static member of a class
template, exhibited by the concepts-lambda4 test which currently fails on
trunk. The bug was causing grokfndecl to use the constraints from the template
class for the lambda.

gcc/cp/
2019-10-30  Jeff Chapman II  

PR c++/84810 - constraints on lambdas
* lambda.c (maybe_add_lambda_conv_op): Do not synthesize
conversion if the call operator does not satisfy its constraints.
* parser.c (cp_parser_lambda_declarator_opt): Parse
requires-clause on generic lambdas; combine with shorthand
constraints. Parse trailing requires-clause and attach to the
synthesized call operator.
* pt.c (template_class_depth): Only inspect
LAMBDA_TYPE_EXTRA_SCOPE if it is present. This fixes an
incorrect depth calculation for lambdas inside the initializer
of a static data member of a template class.

gcc/testsuite/
2019-10-30  Jeff Chapman II  

PR c++/84810 - constraints on lambdas
* g++.dg/cpp2a/concepts-lambda2.C: New test.
* g++.dg/cpp2a/concepts-lambda3.C: Ditto.
* g++.dg/cpp2a/concepts-lambda4.C: Ditto.
* g++.dg/cpp2a/concepts-pr84810.C: Ditto.

Bootstrapped and tested on x86_64-pc-linux-gnu.

Please let me know if there's any issues.

Thanks,
Jeff Chapman II
From d196b03e6f6924935a521a8d140d621d03ae18f2 Mon Sep 17 00:00:00 2001
From: Jeff Chapman II 
Date: Mon, 28 Oct 2019 12:57:36 -0400
Subject: [PATCH 1/1] PR c++/84810 - constraints on lambdas

gcc/cp/
2019-10-30  Jeff Chapman II  

	PR c++/84810 - constraints on lambdas
	* lambda.c (maybe_add_lambda_conv_op): Do not synthesize
	conversion if the call operator does not satisfy its constraints.
	* parser.c (cp_parser_lambda_declarator_opt): Parse
	requires-clause on generic lambdas; combine with shorthand
	constraints. Parse trailing requires-clause and attach to the
	synthesized call operator.
	* pt.c (template_class_depth): Only inspect
	LAMBDA_TYPE_EXTRA_SCOPE if it is present. This fixes an
	incorrect depth calculation for lambdas inside the initializer
	of a static data member of a template class.

gcc/testsuite/
2019-10-30  Jeff Chapman II  

	PR c++/84810 - constraints on lambdas
	* g++.dg/cpp2a/concepts-lambda2.C: New test.
	* g++.dg/cpp2a/concepts-lambda3.C: Ditto.
	* g++.dg/cpp2a/concepts-lambda4.C: Ditto.
	* g++.dg/cpp2a/concepts-pr84810.C: Ditto.
---
 gcc/cp/lambda.c   |   6 +
 gcc/cp/parser.c   |  21 ++-
 gcc/cp/pt.c   |   2 +-
 gcc/testsuite/g++.dg/cpp2a/concepts-lambda2.C | 153 ++
 gcc/testsuite/g++.dg/cpp2a/concepts-lambda3.C |  64 
 gcc/testsuite/g++.dg/cpp2a/concepts-lambda4.C |  14 ++
 gcc/testsuite/g++.dg/cpp2a/concepts-pr84810.C |  13 ++
 7 files changed, 270 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda2.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda3.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda4.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr84810.C

diff --git a/gcc/cp/lambda.c b/gcc/cp/lambda.c
index f128ed800f6..d621beca2eb 100644
--- a/gcc/cp/lambda.c
+++ b/gcc/cp/lambda.c
@@ -1046,6 +1046,12 @@ maybe_add_lambda_conv_op (tree type)
   return;
 }
 
+  /* Non-generic non-capturing lambdas only have a conversion function to
+ pointer to function when the trailing requires-clause's constraints are
+ satisfied.  */
+  if (!generic_lambda_p && !constraints_satisfied_p (callop))
+return;
+
   /* Non-template conversion operators are defined directly with build_call_a
  and using DIRECT_ARGVEC for arguments (including 'this').  Templates are
  deferred and the CALL is built in-place.  In the case of a deduced return
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 3857fe47d67..bbdf8d69077 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -10854,11 +10854,13 @@ cp_parser_lambda_introducer (cp_parser* parser, tree lambda_expr)
 
lambda-declarator:
  < template-parameter-list [opt] >
+   requires-clause [opt]
  ( parameter-declaration-clause [opt] )
attribute-specifier [opt]
decl-specifier-seq [opt]
exception-specification [opt]

Re: [PATCH] Fix PR90316

[Jakub Jelinek]

On Wed, Oct 30, 2019 at 11:57:12AM +0100, Thomas Schwinge wrote:
> ..., and when building gcc-9-branch with
> '--enable-checking=yes,extra,rtl' (apparently I'm the only one doing
> that, huh?), runs into the following (at least I suppose that's what's

I'm testing release branches with
../configure --enable-languages=default,ada,obj-c++,lto,go,brig,d 
--enable-checking=yes,rtl,extra
too, and saw (last time I've bootstrapped/regtested gcc-9-branch
this way was Oct 21st though):
../../gcc/machmode.h: In function ‘dw_loc_descr_node* mem_loc_descriptor(rtx, 
machine_mode, machine_mode, var_init_status)’:
../../gcc/machmode.h:520:42: warning: ‘int_mode’ may be used uninitialized in 
this function [-Wmaybe-uninitialized]
  520 |? mode_size_inline (mode) : mode_size[mode]);
  |  ^~~~
../../gcc/dwarf2out.c:15464:19: note: ‘int_mode’ was declared here
15464 |   scalar_int_mode int_mode, inner_mode, op1_mode;
  |   ^~~~
and no fatal error.  Are you using --enable-werror or something similar
in addition?

Jakub

Re: [Patch][Fortran] OpenMP – libgomp/testsuite – use 'stop' and 'dg-do run'

[Jakub Jelinek]

On Wed, Oct 30, 2019 at 11:45:11AM +0100, Tobias Burnus wrote:
> On 10/30/19 10:31 AM, Jakub Jelinek wrote:
> > > +++ b/libgomp/testsuite/libgomp.fortran/omp_orphan.f
> > > +++ b/libgomp/testsuite/libgomp.fortran/omp_reduction.f
> > > +++ b/libgomp/testsuite/libgomp.fortran/omp_workshare1.f
> > > --- a/libgomp/testsuite/libgomp.fortran/omp_workshare2.f
> > > +++ b/libgomp/testsuite/libgomp.fortran/omp_workshare2.f
> > Dunno, maybe, though not clear advantages of doing so.
> 
> I didn't added 'dg-do run' for those.
> Otherwise as suggested – and committed as Rev. 277606.

Thanks.

> > I'm not really happy about the uppercase STOP in all the libgomp.fortran
> > tests that are written completely in lowercase except these stops, but
> > didn't get around to changing it yet.
> 
> How about the following patch? Created by
> sed -i -e 's/STOP /stop /' in testsuite/libgomp.fortran/
> and glanced over. (Seemingly, no all-capital-letters file exists.)

> --- a/libgomp/testsuite/libgomp.fortran/lib2.f
> +++ b/libgomp/testsuite/libgomp.fortran/lib2.f
> @@ -14 +14 @@ C { dg-do run }
> -  IF (OMP_TEST_LOCK (LCK)) STOP 1
> +  IF (OMP_TEST_LOCK (LCK)) stop 1
...
> --- a/libgomp/testsuite/libgomp.fortran/lib3.f
> +++ b/libgomp/testsuite/libgomp.fortran/lib3.f
> --- a/libgomp/testsuite/libgomp.fortran/pr25162.f
> +++ b/libgomp/testsuite/libgomp.fortran/pr25162.f

Please don't change these 3 tests, they are all-capital-letters, at least on
the corresponding lines, comments and especially dg-* directives which have
to be lowercase don't count (I've looked for IF in capital letters
in your patch ;) ).

Otherwise LGTM, thanks.

Jakub

Re: [PATCH] Fix PR90316

[Thomas Schwinge]

Hi!

On 2019-05-06T11:36:22+0200, Richard Biener  wrote:
> On Sat, 4 May 2019, Richard Sandiford wrote:
>> Richard Biener  writes:
>> > On Fri, 3 May 2019, Richard Biener wrote:
>> >> I am testing the following patch [...]

... which apparently also got backported to gcc-9-branch eventually...

>> >> Note this will handle even more CFG shapes now and seems to
>> >> expose some uninit warnings in dwarf2out.c (at least).

..., and when building gcc-9-branch with
'--enable-checking=yes,extra,rtl' (apparently I'm the only one doing
that, huh?), runs into the following (at least I suppose that's what's
meant with "expose some uninit warnings in dwarf2out.c"?):

In file included from [...]/source-gcc/gcc/coretypes.h:433,
 from [...]/source-gcc/gcc/dwarf2out.c:60:
[...]/source-gcc/gcc/machmode.h: In function 'dw_loc_descr_node* 
mem_loc_descriptor(rtx, machine_mode, machine_mode, var_init_status)':
[...]/source-gcc/gcc/machmode.h:520:42: error: 'int_mode' may be used 
uninitialized in this function [-Werror=maybe-uninitialized]
  520 |? mode_size_inline (mode) : mode_size[mode]);
  |  ^~~~
[...]/source-gcc/gcc/dwarf2out.c:15464:19: note: 'int_mode' was declared 
here
15464 |   scalar_int_mode int_mode, inner_mode, op1_mode;
  |   ^~~~
cc1plus: all warnings being treated as errors
make[3]: *** [dwarf2out.o] Error 1

>> > I can't seem to find an initializer that would "trap" on use
>> > so I'm going to do
>> >
>> > Index: gcc/dwarf2out.c
>> > ===
>> > --- gcc/dwarf2out.c (revision 270849)
>> > +++ gcc/dwarf2out.c (working copy)
>> > @@ -15461,7 +15461,7 @@ mem_loc_descriptor (rtx rtl, machine_mod
>> >if (mode != GET_MODE (rtl) && GET_MODE (rtl) != VOIDmode)
>> >  return NULL;
>> >  
>> > -  scalar_int_mode int_mode, inner_mode, op1_mode;
>> > +  scalar_int_mode int_mode = SImode, inner_mode, op1_mode;
>> >switch (GET_CODE (rtl))
>> >  {
>> >  case POST_INC:
>> >
>> > unless somebody comes up with something clever over the weekend...
>> 
>> Nothing clever, but something rare like BImode is probably safer than
>> SImode, in case doing this masks real "uninitialised" uses in future.
>
> Ick, and I forgot to install this hunk when I committed it this morning.
>
> Thus fixed as obvious now, with BImode.

..., so I backported that fix (or is it rather "fix"?) to gcc-9-branch in
r277608 "Avoid '-Wmaybe-uninitialized' diagnostic in 'gcc/dwarf2out.c'",
see attached.


Grüße
 Thomas


From 3d82e409c5ef4d39908bd9798d096862af3eb4ca Mon Sep 17 00:00:00 2001
From: tschwinge 
Date: Wed, 30 Oct 2019 10:50:58 +
Subject: [PATCH] Avoid '-Wmaybe-uninitialized' diagnostic in 'gcc/dwarf2out.c'

With '--enable-checking=yes,extra,rtl':

In file included from [...]/source-gcc/gcc/coretypes.h:433,
 from [...]/source-gcc/gcc/dwarf2out.c:60:
[...]/source-gcc/gcc/machmode.h: In function 'dw_loc_descr_node* mem_loc_descriptor(rtx, machine_mode, machine_mode, var_init_status)':
[...]/source-gcc/gcc/machmode.h:520:42: error: 'int_mode' may be used uninitialized in this function [-Werror=maybe-uninitialized]
  520 |? mode_size_inline (mode) : mode_size[mode]);
  |  ^~~~
[...]/source-gcc/gcc/dwarf2out.c:15464:19: note: 'int_mode' was declared here
15464 |   scalar_int_mode int_mode, inner_mode, op1_mode;
  |   ^~~~
cc1plus: all warnings being treated as errors
make[3]: *** [dwarf2out.o] Error 1

Backport trunk r270903.

2019-05-06  Richard Biener  

	* dwarf2out.c (mem_loc_descriptor): Initialize int_mode.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/branches/gcc-9-branch@277608 138bc75d-0d04-0410-961f-82ee72b054a4
---
 gcc/ChangeLog   | 8 
 gcc/dwarf2out.c | 2 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 910887a8451..ce4630ba078 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,11 @@
+2019-10-30  Thomas Schwinge  
+
+	Backport from trunk:
+
+	2019-05-06  Richard Biener  
+
+	* dwarf2out.c (mem_loc_descriptor): Initialize int_mode.
+
 2019-10-28  Uroš Bizjak  
 
 	PR target/92225
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 55a73e12cc2..ae47387b763 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -15461,7 +15461,7 @@ mem_loc_descriptor (rtx rtl, machine_mode mode,
   if (mode != GET_MODE (rtl) && GET_MODE (rtl) != VOIDmode)
 return NULL;
 
-  scalar_int_mode int_mode, inner_mode, op1_mode;
+  scalar_int_mode int_mode = BImode, inner_mode, op1_mode;
   switch (GET_CODE (rtl))
 {
 case POST_INC:
-- 
2.17.1



signature.asc
Description: PGP signature

Re: [Patch][OpenMP] use_device_addr/use_device_ptr with Fortran allocatable/pointer arrays (= array descriptor)

[Jakub Jelinek]

On Mon, Oct 14, 2019 at 03:11:43PM +0200, Tobias Burnus wrote:
>   gcc/fortran/
>   * f95-lang.c (LANG_HOOKS_OMP_ARRAY_DATA): Set to gfc_omp_array_data.
>   * trans-array.c

Description of what has been changed and how is missing.

> --- a/gcc/langhooks.h
> +++ b/gcc/langhooks.h
> @@ -222,6 +222,10 @@ struct lang_hooks_for_decls
>/* True if this decl may be called via a sibcall.  */
>bool (*ok_for_sibcall) (const_tree);
>  
> +  /* Return a tree for of the actual data of an array descriptor - or

s/for of/for/ ?

> + NULL_TREE if original tree is not an array descriptor.  */
> +  tree (*omp_array_data) (tree);
> +

> --- a/gcc/omp-low.c
> +++ b/gcc/omp-low.c
> @@ -90,6 +90,7 @@ struct omp_context
>/* Map variables to fields in a structure that allows communication
>   between sending and receiving threads.  */
>splay_tree field_map;
> +  splay_tree array_data_map;

I was hoping you could get away without introducing yet another splay_tree
for this.  As I said on IRC, we already do have the need to record two
different fields for the same decl and do that through using
the decl address (var) once as the splay_tree_key and in another case
as _UID (var).  So can't you use the mask & 16 mode to use say
_NAME (var) and pass the decl to install_var_field rather than x?

> -  if ((mask & 8) != 0)
> +  if ((mask & 16) != 0)
> +key = (splay_tree_key) var;

That would be here.

> +  else if ((mask & 8) != 0)
>  {
>key = (splay_tree_key) _UID (var);
>gcc_checking_assert (key != (splay_tree_key) var);
> @@ -745,14 +748,17 @@ install_var_field (tree var, bool by_ref, int mask, 
> omp_context *ctx)
>else if ((mask & 3) == 1 && omp_is_reference (var))
>  type = TREE_TYPE (type);
>  
> -  field = build_decl (DECL_SOURCE_LOCATION (var),
> -   FIELD_DECL, DECL_NAME (var), type);
> +  if ((mask & 16) != 0)
> +field = build_decl (UNKNOWN_LOCATION, FIELD_DECL, NULL_TREE, type);
> +  else
> +field = build_decl (DECL_SOURCE_LOCATION (var),
> + FIELD_DECL, DECL_NAME (var), type);

And you could use the DECL_NAME as well as DECL_SOURCE_LOCATION.
It is true that for mode & 16 it would need to use the target hook to
actually find out the type of the field, so perhaps we'd need to call
the new target hook multiple times.  As it creates trees that is not the
best idea, though perhaps it could have an extra bool argument where it
would return the type of the data pointer in the descriptor (+ info whether
it is a descriptor through whether it returns non-NULL) with false for the
new arg, something that would be cheap to be called multiple times, once in
scan_sharing_clauses, then again in install_var_field for mode & 16, and
with true in the hopefully only spot where we need the actual expression
(where we gimplify it and store into the corresponding field).

> @@ -1070,7 +1078,7 @@ fixup_child_record_type (omp_context *ctx)
>  static void
>  scan_sharing_clauses (tree clauses, omp_context *ctx)
>  {
> -  tree c, decl;
> +  tree c, decl, x;
>bool scan_array_reductions = false;
>  
>for (c = clauses; c; c = OMP_CLAUSE_CHAIN (c))
> @@ -1240,10 +1248,33 @@ scan_sharing_clauses (tree clauses, omp_context *ctx)
>   case OMP_CLAUSE_USE_DEVICE_PTR:
>   case OMP_CLAUSE_USE_DEVICE_ADDR:
> decl = OMP_CLAUSE_DECL (c);
> -   if ((OMP_CLAUSE_CODE (c) == OMP_CLAUSE_USE_DEVICE_ADDR
> -&& !omp_is_reference (decl)
> -&& !omp_is_allocatable_or_ptr (decl))
> -   || TREE_CODE (TREE_TYPE (decl)) == ARRAY_TYPE)
> +  x = NULL;
> +   // Handle array descriptors

Most of omp-low.c uses /* ... */ comments, can you use them here too
(plus dot at the end + two spaces before */)?

> +   if (TREE_CODE (TREE_TYPE (decl)) == RECORD_TYPE ||

Formatting, || needs to be on the following line.

> +   (omp_is_reference (decl)
> +&& TREE_CODE (TREE_TYPE (TREE_TYPE (decl))) == RECORD_TYPE))
> + x = lang_hooks.decls.omp_array_data (decl);
> +
> +   if (x)
> + {
> +   gcc_assert (!ctx->array_data_map
> +   || !splay_tree_lookup (ctx->array_data_map,
> +(splay_tree_key) decl));
> +   if (!ctx->array_data_map)
> + ctx->array_data_map
> + = splay_tree_new (splay_tree_compare_pointers, 0, 0);

Formatting, indented too much (though, see above, I'd hope you can avoid
that).

> +
> +   splay_tree_insert (ctx->array_data_map, (splay_tree_key) decl,
> +  (splay_tree_value) x);
> +
> +   install_var_field (x, false, 19, ctx);
> +   DECL_SOURCE_LOCATION (lookup_field (x, ctx))
> + = OMP_CLAUSE_LOCATION (c);

Formatting.

> - if (OMP_CLAUSE_CODE (c) != OMP_CLAUSE_IS_DEVICE_PTR)
> +
> + // For arrays with descriptor, use the pointer to the actual data

Comment style.

Re: [Patch][Fortran/OpenMP] Don't create "alloc:" for 'target exit data'

[Jakub Jelinek]

On Fri, Oct 18, 2019 at 11:27:39AM +0200, Tobias Burnus wrote:
> Currently, one has for
>   !$omp target exit data map(delete:x)
> in the original dump:
>   #pragma omp target exit data map(delete:*x) map(alloc:x [pointer assign,
> bias: 0])
> 
> The "alloc:" not only does not make sense but also gives run-time messages
> like:
> libgomp: GOMP_target_enter_exit_data unhandled kind 0x04
> 
> [Depending on the data type, in gfc_trans_omp_clauses's OMP_LIST_MAP, add
> map clauses of type GOMP_MAP_POINTER and/or GOMP_MAP_TO_PSET.]
> 
> That's for release:/delete:. However, for 'target exit data'
> (GOMP_target_enter_exit_data) the same issue occurs for "from:"/"always,
> from:".  But "from:" implies "alloc:". – While "alloc:" does not make sense
> for "target exit data" or "update", for "target" or "target data" it surely
> matters. Hence, I only exclude "from:" for exit data and update.
> 
> See attached patch. I have additionally Fortran-fied libgomp.c/target-20.c
> to have at least one 'enter/exit target data' test case for Fortran.
> 
> Build + regtested on x86_64-gnu-linux w/o offloading. And I have tested the
> new test case with nvptx.

I believe it is easier to handle it at the same spot as we do it e.g. for
C/C++ pointer attachments (where we create the same clauses regardless of
the exact construct and then drop them later), in particular in
gimplify_scan_omp_clauses.
There we have:
case OMP_TARGET:
  break;
case OACC_DATA:
  if (TREE_CODE (TREE_TYPE (decl)) != ARRAY_TYPE)
break;
  /* FALLTHRU */
case OMP_TARGET_DATA:
case OMP_TARGET_ENTER_DATA:
case OMP_TARGET_EXIT_DATA:
case OACC_ENTER_DATA:
case OACC_EXIT_DATA:
case OACC_HOST_DATA:
  if (OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_FIRSTPRIVATE_POINTER
  || (OMP_CLAUSE_MAP_KIND (c)
  == GOMP_MAP_FIRSTPRIVATE_REFERENCE))
/* For target {,enter ,exit }data only the array slice is
   mapped, but not the pointer to it.  */
remove = true;
  break;
So, I think best would be to add
if (code == OMP_TARGET_EXIT_DATA && OMP_CLAUSE_MAP_KIND (c) ==
GOMP_MAP_WHATEVER_IS_NOT_VALID_FOR_EXIT_DATA) remove = true;
with a comment explaining that.

The testcase LGTM.

Jakub