Re: [PATCH, rs6000] PR target/89112 put branch probabilities on branches generated by inline expansion

[Segher Boessenkool]

Hi Aaron,

On Mon, Feb 04, 2019 at 01:06:57PM -0600, Aaron Sawdey wrote:
> This is the second part of the fix for 89112, fixing the conditions that 
> caused it to happen.
> This patch adds REG_BR_PROB notes to the branches generated by inline 
> expansion of memcmp
> and strncmp. This prevents any of the code from being marked as cold and 
> moved to the end
> of the function, which is what caused the long branches in 89112. With this 
> patch, the test
> case for 89112 does not have any long branches within the expansion of 
> memcmp, and the code
> for each memcmp is contiguous.

I think it is a good idea to have this in 9.  But, a few things need
fixing:

> 2019-02-04  Aaron Sawdey  
> 
>   * config/rs6000/rs6000-string.c (do_ifelse, expand_cmp_vec_sequence,
>   expand_compare_loop, expand_block_compare_gpr,
>   expand_strncmp_align_check, expand_strncmp_gpr_sequence): add branch
>   probability.

"Add", capital.  Maybe write it different, it sounds like addition of
probabilities to me :-/

> Index: gcc/config/rs6000/rs6000-string.c
> ===
> --- gcc/config/rs6000/rs6000-string.c (revision 268522)
> +++ gcc/config/rs6000/rs6000-string.c (working copy)
> @@ -35,6 +35,8 @@
>  #include "expr.h"
>  #include "output.h"
>  #include "target.h"
> +#include "profile-count.h"
> +#include "predict.h"

These should go into the changelog.  The changelog can use a few more words
in general.

>  static void
>  do_ifelse (machine_mode cmpmode, rtx_code comparison,
> -rtx a, rtx b, rtx cr, rtx true_label)
> +rtx a, rtx b, rtx cr, rtx true_label, profile_probability p)

Please call it br_prob or such, not just "p".

> @@ -1120,11 +1125,11 @@
>/* Check for > max_bytes bytes.  We want to bail out as quickly as
>possible if we have to go over to memcmp.  */
>do_ifelse (CCmode, GT, bytes_rtx, GEN_INT (max_bytes),
> -  NULL_RTX, library_call_label);
> +  NULL_RTX, library_call_label, profile_probability::even ());
> 
>/* Check for < loop_bytes bytes.  */
>do_ifelse (CCmode, LT, bytes_rtx, GEN_INT (loop_bytes),
> -  NULL_RTX, cleanup_label);
> +  NULL_RTX, cleanup_label, profile_probability::even ());

Is "even" a good guess here?  Is there no preferred direction?  "likely"
and "unlikely" are only 80% and 20% as well.

> @@ -1165,7 +1170,7 @@
>   {
> rtx lab_after = gen_label_rtx ();
> do_ifelse (CCmode, LE, bytes_rtx, GEN_INT (max_bytes),
> -  NULL_RTX, lab_after);
> +  NULL_RTX, lab_after, profile_probability::even ());

Same here, wherever that is :-)

> @@ -1363,10 +1374,12 @@
> if (bytes_is_const)
>   bytes_remaining -= load_mode_size;
> else
> - /* See if remaining length is now zero.  We previously set
> -target to 0 so we can just jump to the end.  */
> - do_ifelse (CCmode, EQ, cmp_rem, const0_rtx,
> -NULL_RTX, final_label);
> + {
> +   /* See if remaining length is now zero.  We previously set
> +  target to 0 so we can just jump to the end.  */
> +   do_ifelse (CCmode, EQ, cmp_rem, const0_rtx, NULL_RTX,
> +  final_label, profile_probability::unlikely ());
> + }

Why the extra braces?  Sometimes that helps, but it's only clutter here.

Okay for trunk and backports with those things fixed (or post it again, if
you prefer).  Thanks!


Segher

[PATCH] Fix prepare_cmp_insn BLKmode handling (PR target/89186)

[Jakub Jelinek]

Hi!

Like the other emit_*_via_libcall functions, emit_block_comp_via_libcall
expects to be called with the MEM rtxes, not their addresses.
E.g. emit_block_op_via_libcall does:
  dst_addr = copy_addr_to_reg (XEXP (dst, 0));
  dst_addr = convert_memory_address (ptr_mode, dst_addr);
  dst_tree = make_tree (ptr_type_node, dst_addr);
  
  src_addr = copy_addr_to_reg (XEXP (src, 0));
  src_addr = convert_memory_address (ptr_mode, src_addr);
  src_tree = make_tree (ptr_type_node, src_addr);
prepare_cmp_insn doesn't call it with the MEMs, but their addresses, so it
is either a wrong-code issue before (e.g. it was passing (plus (reg virtual...)
(const_int N)) and (plus (reg virtual...) (const_int M)) on the following
testcase, so it took (reg virtual...) in both cases as the address to
compare, or with the recent additions to emit_block_op_via_libcall an ICE.

Unfortunately on the following testcase where it ICEs, the BLKmode
comparison is there in dead code and so the return value doesn't matter, so
all I can show is that it doesn't ICE anymore and that it calls memcmp with
different addresses where previously it called them with the same addresses.

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

2019-02-05  Jakub Jelinek  

PR target/89186
* optabs.c (prepare_cmp_insn): Pass x and y to
emit_block_comp_via_libcall rather than XEXP (x, 0) and XEXP (y, 0).

* g++.dg/ext/vector36.C: New test.

--- gcc/optabs.c.jj 2019-01-22 10:11:00.960386744 +0100
+++ gcc/optabs.c2019-02-04 12:40:45.881211716 +0100
@@ -3917,7 +3917,7 @@ prepare_cmp_insn (rtx x, rtx y, enum rtx
goto fail;
 
   /* Otherwise call a library function.  */
-  result = emit_block_comp_via_libcall (XEXP (x, 0), XEXP (y, 0), size);
+  result = emit_block_comp_via_libcall (x, y, size);
 
   x = result;
   y = const0_rtx;
--- gcc/testsuite/g++.dg/ext/vector36.C.jj  2019-02-04 13:25:15.724616481 
+0100
+++ gcc/testsuite/g++.dg/ext/vector36.C 2019-02-04 13:26:13.099655912 +0100
@@ -0,0 +1,6 @@
+// PR target/89186
+// { dg-do compile }
+// { dg-options "-fnon-call-exceptions" }
+// { dg-additional-options "-mno-sse" { target i?86-*-* x86_64-*-* } }
+
+#include "vector27.C"

Jakub

[C++ PATCH] Clear TREE_ADDRESSABLE on non-aggregate thunk arguments (PR c++/89187)

[Jakub Jelinek]

Hi!

As the following testcase shows, for thunks the expansion code sometimes
requires that arguments with gimple reg type aren't addressable,
thunk needs to be simple passing of arguments directly to another call, not
having extra statements in between that.

In particular, for pass_by_reference arguments calls.c has if
call_from_thunk_p:
  /* We may have turned the parameter value into an SSA name.
 Go back to the original parameter so we can take the
 address.  */
  if (TREE_CODE (args[i].tree_value) == SSA_NAME)
{
  gcc_assert (SSA_NAME_IS_DEFAULT_DEF (args[i].tree_value));
  args[i].tree_value = SSA_NAME_VAR (args[i].tree_value);
  gcc_assert (TREE_CODE (args[i].tree_value) == PARM_DECL);
}
which is not the case if the PARM_DECL is TREE_ADDRESSABLE, then the def
stmt of the SSA_NAME is an assignment which loads the PARM_DECL value from
"memory".

TREE_ADDRESSABLE on the arguments is copied from the ctor from which it is
cloned (and which will be called by the thunk).  In this case the argument
needs to be addressable because vector[idx] folding takes address of vector.
In the thunk itself that doesn't matter though, it just passes the argument
to another function, so the following patch clears the TREE_ADDRESSABLE bit
there.

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

2019-02-05  Jakub Jelinek  

PR c++/89187
* optimize.c (maybe_thunk_body): Clear TREE_ADDRESSABLE on
non-aggregate PARM_DECLs of the thunk.

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

--- gcc/cp/optimize.c.jj2019-01-21 23:32:43.0 +0100
+++ gcc/cp/optimize.c   2019-02-04 16:40:21.354179933 +0100
@@ -417,6 +417,12 @@ maybe_thunk_body (tree fn, bool force)
  gcc_assert (clone_parm);
  DECL_ABSTRACT_ORIGIN (clone_parm) = NULL;
  args[parmno] = clone_parm;
+ /* Clear TREE_ADDRESSABLE on arguments with non-aggregate
+types, the thunk will not take addresses of those
+arguments, will just pass them through to another
+call.  */
+ if (!AGGREGATE_TYPE_P (TREE_TYPE (clone_parm)))
+   TREE_ADDRESSABLE (clone_parm) = 0;
  clone_parm = TREE_CHAIN (clone_parm);
}
  if (fn_parm_typelist)
--- gcc/testsuite/g++.dg/opt/pr89187.C.jj   2019-02-04 17:34:13.973847193 
+0100
+++ gcc/testsuite/g++.dg/opt/pr89187.C  2019-02-04 17:34:41.191396316 +0100
@@ -0,0 +1,23 @@
+// PR c++/89187
+// { dg-do compile { target c++11 } }
+// { dg-options "-Os -fno-tree-ccp -fno-tree-sra -fno-inline" }
+
+template  struct A {
+  typedef T __attribute__((vector_size (N))) type;
+};
+template  using B = typename A::type;
+template  using C = B;
+struct D {
+  D (C x) : d{x[3]} {}
+  D foo () { return d; }
+  C d;
+};
+extern D d;
+struct { D bar () { return d; } } l;
+struct E { void baz () const; };
+
+void
+E::baz () const
+{
+  l.bar ().foo ();
+}

Jakub

Re: [PATCH] Fix prepare_cmp_insn BLKmode handling (PR target/89186)

[Richard Biener]

On Tue, 5 Feb 2019, Jakub Jelinek wrote:

> Hi!
> 
> Like the other emit_*_via_libcall functions, emit_block_comp_via_libcall
> expects to be called with the MEM rtxes, not their addresses.
> E.g. emit_block_op_via_libcall does:
>   dst_addr = copy_addr_to_reg (XEXP (dst, 0));
>   dst_addr = convert_memory_address (ptr_mode, dst_addr);
>   dst_tree = make_tree (ptr_type_node, dst_addr);
>   
>   src_addr = copy_addr_to_reg (XEXP (src, 0));
>   src_addr = convert_memory_address (ptr_mode, src_addr);
>   src_tree = make_tree (ptr_type_node, src_addr);
> prepare_cmp_insn doesn't call it with the MEMs, but their addresses, so it
> is either a wrong-code issue before (e.g. it was passing (plus (reg 
> virtual...)
> (const_int N)) and (plus (reg virtual...) (const_int M)) on the following
> testcase, so it took (reg virtual...) in both cases as the address to
> compare, or with the recent additions to emit_block_op_via_libcall an ICE.
> 
> Unfortunately on the following testcase where it ICEs, the BLKmode
> comparison is there in dead code and so the return value doesn't matter, so
> all I can show is that it doesn't ICE anymore and that it calls memcmp with
> different addresses where previously it called them with the same addresses.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 

OK.

Richard.

> 2019-02-05  Jakub Jelinek  
> 
>   PR target/89186
>   * optabs.c (prepare_cmp_insn): Pass x and y to
>   emit_block_comp_via_libcall rather than XEXP (x, 0) and XEXP (y, 0).
> 
>   * g++.dg/ext/vector36.C: New test.
> 
> --- gcc/optabs.c.jj   2019-01-22 10:11:00.960386744 +0100
> +++ gcc/optabs.c  2019-02-04 12:40:45.881211716 +0100
> @@ -3917,7 +3917,7 @@ prepare_cmp_insn (rtx x, rtx y, enum rtx
>   goto fail;
>  
>/* Otherwise call a library function.  */
> -  result = emit_block_comp_via_libcall (XEXP (x, 0), XEXP (y, 0), size);
> +  result = emit_block_comp_via_libcall (x, y, size);
>  
>x = result;
>y = const0_rtx;
> --- gcc/testsuite/g++.dg/ext/vector36.C.jj2019-02-04 13:25:15.724616481 
> +0100
> +++ gcc/testsuite/g++.dg/ext/vector36.C   2019-02-04 13:26:13.099655912 
> +0100
> @@ -0,0 +1,6 @@
> +// PR target/89186
> +// { dg-do compile }
> +// { dg-options "-fnon-call-exceptions" }
> +// { dg-additional-options "-mno-sse" { target i?86-*-* x86_64-*-* } }
> +
> +#include "vector27.C"
> 
>   Jakub
> 
> 

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

Re: [C++ Patch] PR 88986 ("[7/8/9 Regression] ICE: tree check: expected tree that contains 'decl minimal' structure, have 'error_mark' in member_vec_binary_search, at cp/name-lookup.c:1136")

[Paolo Carlini]

Hi,

On 04/02/19 17:48, Jason Merrill wrote:

On Mon, Feb 4, 2019 at 11:00 AM Paolo Carlini  wrote:

On 04/02/19 15:47, Jason Merrill wrote:

On 2/1/19 3:52 PM, Paolo Carlini wrote:

Hi,

I think that this ICE on invalid (and valid, for c++17+) can be in
fact avoided by accepting in make_typename_type a TYPE_PACK_EXPANSION
as context, thus by not triggering the "‘T ...’ is not a class"
error. Not sure if a better fix would be something more general.
Note, anyway, that we are asserting TYPE_P (context) thus
TYPE_PACK_EXPANSIONs definitely get through beyond MAYBE_CLASS_TYPE_P.

The testcase should test that the using actually works, i.e. imports a
type from a base class.

Uhm, if I change the testcase to something like:

struct B { typedef int type; };

template struct C : T... {
using typename T::type ...;
void f() { type value; }
};

template class C;

we get a "sorry, unimplemented: use of ‘type_pack_expansion’ in
template" for value

I suspected that would happen.


which, arguably, is better than the current ICE,
but I'm not sure if we are close to completing the implementation of
this / we even want to attempt that at this Stage?!?

Well, I'd look at how we implement the equivalent for e.g. a variable:

struct A { static int i; };
template  struct B: T... {
   using T::i ...;
};
auto x = B::i; // works

and try to use that code path for the typename case as well.


Yes. Note that, AFAICS, the code you added to implement P0195R2 works 
fine for typename too. The problem happens when we actually encounter 
inside the class something like:


    void f() { type value; }

which, so to speak, appears to expand to a set of types: 'type' is a 
TYPENAME_TYPE which has a TYPE_PACK_EXPANSION as TYPE_CONTEXT. What 
should we do here, in general? Should we expand it to a set a VAR_DECLs? 
Note that in practice when the pack is tsubst-ed to more than one type 
we error out before seeing the function 'f', we say that we have a 
redeclaration:


struct B1 { typedef int type; };
struct B2 { typedef int type; };

template struct C : T... {
  using typename T::type ...;
  //void f() { type value; }
};

template class C;

we do exactly the same in the non-variadic using case, thus at least we 
are consistent.Therefore I can't imagine cases where potentially it 
would make sense to inject *a set* of VAR_DECLs and see what happens. 
See the attached for something more concrete: in hindsight it's obvious 
that if we change make_typename_type to let TYPE_PACK_EXPANSION through 
then we have to handle more than aggregates in tsubst...


Thanks, Paolo.

Index: decl.c
===
--- decl.c  (revision 268513)
+++ decl.c  (working copy)
@@ -3816,7 +3816,9 @@ make_typename_type (tree context, tree name, enum
   gcc_assert (identifier_p (name));
   gcc_assert (TYPE_P (context));
 
-  if (!MAYBE_CLASS_TYPE_P (context))
+  if (TREE_CODE (context) == TYPE_PACK_EXPANSION)
+/* This can happen for C++17 variadic using (c++/88986).  */;
+  else if (!MAYBE_CLASS_TYPE_P (context))
 {
   if (complain & tf_error)
error ("%q#T is not a class", context);
Index: pt.c
===
--- pt.c(revision 268513)
+++ pt.c(working copy)
@@ -14895,8 +14895,18 @@ tsubst (tree t, tree args, tsubst_flags_t complain
 
 case TYPENAME_TYPE:
   {
-   tree ctx = tsubst_aggr_type (TYPE_CONTEXT (t), args, complain,
-in_decl, /*entering_scope=*/1);
+   tree ctx = TYPE_CONTEXT (t);
+   if (TREE_CODE (ctx) == TYPE_PACK_EXPANSION)
+ {
+   ctx = tsubst_pack_expansion (ctx, args, complain, in_decl);
+   if (ctx == error_mark_node
+   || TREE_VEC_LENGTH (ctx) != 1)
+ return error_mark_node;
+   ctx = TREE_VEC_ELT (ctx, 0);
+ }
+   else
+ ctx = tsubst_aggr_type (ctx, args, complain, in_decl,
+ /*entering_scope=*/1);
if (ctx == error_mark_node)
  return error_mark_node;
 

[PATCH][GCC][Arm] Fix NEON REG to REG reload failures. (PR/target 88850)

[Tamar Christina]

Hi All,

We currently return cost 2 for NEON REG to REG moves, which would be incorrect
for 64 bit moves.  We currently don't have a pattern for this in the neon_move
alternatives because this is a bit of a special case.  We would almost never
want it to use this r -> r pattern unless it really has no choice.

As such we add a new neon r -> r move pattern but also hide it from being used
to determine register preferences and also disparage it during LRA.

Bootstrapped Regtested on arm-none-gnueabihf and no issues.

Ok for trunk?

Thanks,
Tamar

gcc/ChangeLog:

2019-02-05  Tamar Christina  

PR/target 88850
* config/arm/neon.md (*neon_mov): Add r -> r case.

gcc/testsuite/ChangeLog:

2019-02-05  Tamar Christina  

PR/target 88850
* gcc.target/arm/pr88850.c: New test.

-- 
diff --git a/gcc/config/arm/neon.md b/gcc/config/arm/neon.md
index f9d7ba35b137fed383f84eecbe81dd942943d216..21ff5adbd40c362aa977171bd1726b88b383a265 100644
--- a/gcc/config/arm/neon.md
+++ b/gcc/config/arm/neon.md
@@ -25,9 +25,9 @@
 
 (define_insn "*neon_mov"
   [(set (match_operand:VDX 0 "nonimmediate_operand"
-	  "=w,Un,w, w,  ?r,?w,?r, ?Us")
+	  "=w,Un,w, w,  ?r,?w,?r, ?Us,*r")
 	(match_operand:VDX 1 "general_operand"
-	  " w,w, Dn,Uni, w, r, Usi,r"))]
+	  " w,w, Dn,Uni, w, r, Usi,r,*r"))]
   "TARGET_NEON
&& (register_operand (operands[0], mode)
|| register_operand (operands[1], mode))"
@@ -62,11 +62,11 @@
 }
  [(set_attr "type" "neon_move,neon_store1_1reg,neon_move,\
 neon_load1_1reg, neon_to_gp,neon_from_gp,\
-neon_load1_2reg, neon_store1_2reg")
-  (set_attr "length" "4,4,4,4,4,4,8,8")
-  (set_attr "arm_pool_range" "*,*,*,1020,*,*,1020,*")
-  (set_attr "thumb2_pool_range" "*,*,*,1018,*,*,1018,*")
-  (set_attr "neg_pool_range" "*,*,*,1004,*,*,1004,*")])
+neon_load1_2reg, neon_store1_2reg, multiple")
+  (set_attr "length" "4,4,4,4,4,4,8,8,8")
+  (set_attr "arm_pool_range" "*,*,*,1020,*,*,1020,*,*")
+  (set_attr "thumb2_pool_range" "*,*,*,1018,*,*,1018,*,*")
+  (set_attr "neg_pool_range" "*,*,*,1004,*,*,1004,*,*")])
 
 (define_insn "*neon_mov"
   [(set (match_operand:VQXMOV 0 "nonimmediate_operand"
diff --git a/gcc/testsuite/gcc.target/arm/pr88850.c b/gcc/testsuite/gcc.target/arm/pr88850.c
new file mode 100644
index ..66d0e9e620aef4f092a33ead79521901b5d89636
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr88850.c
@@ -0,0 +1,22 @@
+/* PR target/88850 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -march=armv7-a -mfloat-abi=softfp -mfpu=neon -fdump-rtl-final" } */
+/* { dg-require-effective-target arm_neon_ok } */
+
+typedef __builtin_neon_qi int8x8_t __attribute__ ((__vector_size__ (8)));
+void bar (int8x8_t, int8x8_t);
+
+void
+foo (int8x8_t x, int8x8_t y)
+{
+ bar (y, x);
+}
+
+void foo2 (int8x8_t x)
+{
+  int8x8_t y;
+  bar (y, x);
+}
+
+/* Check that these 64-bit moves are done in SI.  */
+/* { dg-final { scan-rtl-dump "_movsi_vfp" "final" } } */

Re: [PATCH, rs6000] Fix instruction counts on powerpc64 test cases.

[Segher Boessenkool]

Hi Bill,

On Mon, Feb 04, 2019 at 03:01:24PM -0600, Bill Seurer wrote:
> [PATCH, rs6000] Fix instruction counts on powerpc64 test cases.
> 
> This patch fixes the assembler instruction counts for some test cases
> that started failing due to changes in code generation.  The targets
> were adjusted a bit as well to avoid generating BE/LE endian code on
> unsupported platforms.
> 
> Bootstrapped and tested on powerpc64le-unknown-linux-gnu (power 8 and
> power 9) and powerpc64be-unknown-linux-gnu (power 7 and power 8) with
> no regressions.  Is this ok for trunk?

> Index: gcc/testsuite/gcc.target/powerpc/vsx-vector-6.p7.c
> ===
> --- gcc/testsuite/gcc.target/powerpc/vsx-vector-6.p7.c(revision 
> 268524)
> +++ gcc/testsuite/gcc.target/powerpc/vsx-vector-6.p7.c(working copy)
> @@ -1,28 +1,21 @@
> -/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
> +/* { dg-do compile { target { powerpc64-*-* && lp64 } } } */
> +/* Note: powerpc64-*-* is BE only. */

Target powerpc64-linux does not mean we are creating 64-bit code, or BE
for that matter; nor the other way around.  It just says what the default
is.

If you want to test only when generating code for 64-bit BE targets, use

/* { dg-do compile { target { powerpc*-*-* && { lp64 && be } } } } */

But, powerpc*-*-* is guaranteed here anyway, so you can do just

/* { dg-do compile { target { lp64 && be } } } */

> -/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } 
> { "-mcpu=power7" } } */
> +/* { dg-skip-if "do not override -mcpu" { powerpc64-*-* } { "-mcpu=*" } 
> { "-mcpu=power7" } } */

Don't change this please; it was correct.  It could also be

/* { dg-skip-if "do not override -mcpu" { *-*-* } { "-mcpu=*" } { 
"-mcpu=power7" } } */

(for the same reason: everything in gcc.target/powerpc is only run for
 powerpc*-*-*).

>  /* Expected instruction counts for Power 7 */
> 
>  /* { dg-final { scan-assembler-times "xvabsdp" 1 } } */
>  /* { dg-final { scan-assembler-times "xvadddp" 1 } } */
> -/* { dg-final { scan-assembler-times "xxlnor" 8 { target le } } } */
> -/* { dg-final { scan-assembler-times "xxlnor" 7 { target be } } } */
> -/* { dg-final { scan-assembler-times "xvcmpeqdp" 5 { target le } } } */
> -/* { dg-final { scan-assembler-times "xvcmpeqdp" 6 { target be }} } */
> -/* { dg-final { scan-assembler-times "xvcmpeqdp." 5 { target le } } } */
> -/* { dg-final { scan-assembler-times "xvcmpeqdp." 6 { target be } } } */
> -/* { dg-final { scan-assembler-times "xvcmpgtdp" 9 { target le } } } */
> -/* { dg-final { scan-assembler-times "xvcmpgtdp" 8 { target be } } } */
> -/* { dg-final { scan-assembler-times "xvcmpgtdp." 9 { target le } } } */
> -/* { dg-final { scan-assembler-times "xvcmpgtdp." 8 { target be } } } */
> -/* { dg-final { scan-assembler-times "xvcmpgedp" 6 { target le } } } */
> -/* { dg-final { scan-assembler-times "xvcmpgedp" 7 { target be } } } */
> -/* { dg-final { scan-assembler-times "xvcmpgedp." 6 { target le } } } */
> -/* { dg-final { scan-assembler-times "xvcmpgedp." 7 { target be } } } */
> +/* { dg-final { scan-assembler-times "xxlnor" 5 } } */
> +/* { dg-final { scan-assembler-times "xvcmpeqdp" 6 } } */
> +/* { dg-final { scan-assembler-times "xvcmpeqdp." 6 } } */
> +/* { dg-final { scan-assembler-times "xvcmpgtdp" 7 } } */
> +/* { dg-final { scan-assembler-times "xvcmpgtdp." 7 } } */
> +/* { dg-final { scan-assembler-times "xvcmpgedp" 7 } } */
> +/* { dg-final { scan-assembler-times "xvcmpgedp." 7 } } */
>  /* { dg-final { scan-assembler-times "xvrdpim" 1 } } */
>  /* { dg-final { scan-assembler-times "xvmaddadp" 1 } } */
>  /* { dg-final { scan-assembler-times "xvmsubadp" 1 } } */

Do we not want to test this on LE anymore?  Oh, P7, heh.  Okay.  Not
enough coffee yet :-)

> +/* { dg-final { scan-assembler-times "xvcmpgedp" 6 } } */
> +/* { dg-final { scan-assembler-times "xvcmpgedp." 6 } } */

These need fixing still: both of these match
  xvcmpgedp 0,1,2
because dot means "any character", in a regexp.  Maybe

/* { dg-final { scan-assembler-times {\mxvcmpgedp\s} 6 } } */
/* { dg-final { scan-assembler-times {\mxvcmpgedp\.\s} 6 } } */

(and with the counts fixed).  (Same for all other dot insns of course).


Segher

Re: Fortran vector math header

[Martin Liška]

On 2/4/19 11:10 AM, Jakub Jelinek wrote:
> On Thu, Jan 24, 2019 at 04:25:13PM +0100, Martin Liška wrote:
>> @@ -11361,6 +11365,13 @@ gfc_match_gcc_builtin (void)
>>else if (gfc_match (" ( inbranch ) ") == MATCH_YES)
>>  clause = SIMD_INBRANCH;
>>  
>> +  if (gfc_match (" if ( '%n' ) ", target) == MATCH_YES)
> 
> Not sure if it is clean enough to have the 's in there, rather than
> matching there a string literal, requiring that it is a constant and getting
> at the string in there.  But if this is fine with the Fortran maintainers, I
> won't object.
> 
> Given the patches for aarch64, I'd think that in addition to the target
> specific multilib strings it might be worth to handle here also the generic
> lp64, ilp32 and llp64 strings (by checking the precision of the
> corresponding C types) and perhaps also big-endian, little-endian and
> pdp-endian.  This way, targets which have only those differences wouldn't
> need to come up with their hooks.

I would prefer to leave the implementation as simple as possible. I don't expect
many targets implementing the SIMD ABI in glibc. It would not require much work
on glibc side (math header file).

> 
>> +! { dg-final { scan-tree-dump "sinf.simdclone" "optimized" { target ilp32 } 
>> } } */
>> +! { dg-final { scan-tree-dump-not "sin.simdclone" "optimized" { target 
>> ilp32 } } } */
> 
> I think you need ia32 here, otherwise it will fail on x32, which is also
> ilp32 target, but not i386.

Fixed.

> 
>> +! { dg-final { scan-tree-dump "sin.simdclone" "optimized" { target lp64} } 
>> } */
>> +! { dg-final { scan-tree-dump-not "sinf.simdclone" "optimized" { target 
>> lp64 } } } */
>> diff --git a/gcc/testsuite/gfortran.dg/simd-builtins-7.h 
>> b/gcc/testsuite/gfortran.dg/simd-builtins-7.h
>> new file mode 100644
>> index 000..1c19b88f877
>> --- /dev/null
>> +++ b/gcc/testsuite/gfortran.dg/simd-builtins-7.h
>> @@ -0,0 +1,2 @@
>> +!GCC$ builtin (sin) attributes simd (notinbranch) if('x86_64')
>> +!GCC$ builtin (sinf) attributes simd (notinbranch) if('i386')
> 
> That is all from me, but I think you need a buy-in from the Fortran
> maintainers (if they are ok with such an extension from Fortran language
> POV) and from Joseph (or other glibc people).

I CCed Fortran ML, I'm attaching update patch.

Martin

> 
>   Jakub
> 

>From 74fa90f002d862582db7f613ff7aa758a0cbc5c0 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Mon, 21 Jan 2019 08:46:06 +0100
Subject: [PATCH] Support if statement in !GCC$ builtin directive.

gcc/ChangeLog:

2019-01-24  Martin Liska  

	* config/i386/i386.c (ix86_get_multilib_abi_name): New function.
	(TARGET_GET_MULTILIB_ABI_NAME): New macro defined.
	* doc/tm.texi: Document new target hook.
	* doc/tm.texi.in: Likewise.
	* target.def: Add new target macro.
	* gcc.c (find_fortran_preinclude_file): Do not search multilib
	suffixes.

gcc/fortran/ChangeLog:

2019-01-24  Martin Liska  

	* decl.c (gfc_match_gcc_builtin): Add support for filtering
	of builtin directive based on multilib ABI name.

gcc/testsuite/ChangeLog:

2019-01-24  Martin Liska  

	* gfortran.dg/simd-builtins-7.f90: New test.
	* gfortran.dg/simd-builtins-7.h: New test.
---
 gcc/config/i386/i386.c| 17 +++
 gcc/doc/tm.texi   |  4 
 gcc/doc/tm.texi.in|  2 ++
 gcc/fortran/decl.c| 21 ++-
 gcc/gcc.c | 10 -
 gcc/target.def|  6 ++
 gcc/testsuite/gfortran.dg/simd-builtins-7.f90 | 19 +
 gcc/testsuite/gfortran.dg/simd-builtins-7.h   |  2 ++
 8 files changed, 71 insertions(+), 10 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/simd-builtins-7.f90
 create mode 100644 gcc/testsuite/gfortran.dg/simd-builtins-7.h

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 789a53501ee..232a14a7de7 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -29583,6 +29583,19 @@ ix86_warn_parameter_passing_abi (cumulative_args_t cum_v, tree type)
   cum->warn_empty = false;
 }
 
+/* This hook returns name of multilib ABI.  */
+
+static const char *
+ix86_get_multilib_abi_name (void)
+{
+  if (!(TARGET_64BIT_P (ix86_isa_flags)))
+return "i386";
+  else if (TARGET_X32_P (ix86_isa_flags))
+return "x32";
+  else
+return "x86_64";
+}
+
 /* Compute the alignment for a variable for Intel MCU psABI.  TYPE is
the data type, and ALIGN is the alignment that the object would
ordinarily have.  */
@@ -51815,6 +51828,10 @@ ix86_run_selftests (void)
 #undef TARGET_WARN_PARAMETER_PASSING_ABI
 #define TARGET_WARN_PARAMETER_PASSING_ABI ix86_warn_parameter_passing_abi
 
+#undef TARGET_GET_MULTILIB_ABI_NAME
+#define TARGET_GET_MULTILIB_ABI_NAME \
+  ix86_get_multilib_abi_name
+
 #if CHECKING_P
 #undef TARGET_RUN_TARGET_SELFTESTS
 #define TARGET_RUN_TARGET_SELFTESTS selftest::ix86_run_selftests
diff --git a/gcc/doc/tm.texi b/gcc/

Re: Fortran vector math header

[Martin Liška]

On 2/5/19 2:31 AM, Joseph Myers wrote:
> My main comment here is that if you go with the approach of a single 
> header shared by multilibs, you should also update the driver code so it 
> no longer uses any sort of multilib suffix when searching for this header 
> (it *should* still use the sysroot headers suffix when searching in a 
> sysroot).

Addressed in the reply I've just sent to the Jakub's email.

Martin

[PATCH][C] Reset TYPE_TRANSPARENT_AGGR on all type variants when not supported

[Richard Biener]

The following fixes an ICE in the type verifier for transparent_union
marked unions that we refuse to handle such.  
(gcc.dg/transparent-union-6.c)

Bootstrap & regtest running on x86_64-unknown-linux-gnu, OK?

Thanks,
Richard.

2019-02-05  Richard Biener  

PR c/88606
* c-decl.c (finish_struct): Reset TYPE_TRANSPARENT_AGGR on
all type variants when not supported.

Index: gcc/c/c-decl.c
===
--- gcc/c/c-decl.c  (revision 268530)
+++ gcc/c/c-decl.c  (working copy)
@@ -8394,6 +8394,16 @@ finish_struct (location_t loc, tree t, t
   }
   }
 
+  /* If this was supposed to be a transparent union, but we can't
+ make it one, warn and turn off the flag.  */
+  if (TREE_CODE (t) == UNION_TYPE
+  && TYPE_TRANSPARENT_AGGR (t)
+  && (!TYPE_FIELDS (t) || TYPE_MODE (t) != DECL_MODE (TYPE_FIELDS (t
+{
+  TYPE_TRANSPARENT_AGGR (t) = 0;
+  warning_at (loc, 0, "union cannot be made transparent");
+}
+
   /* Note: C_TYPE_INCOMPLETE_VARS overloads TYPE_VFIELD which is used
  in dwarf2out via rest_of_decl_compilation below and means
  something totally different.  Since we will be clearing
@@ -8406,22 +8416,13 @@ finish_struct (location_t loc, tree t, t
 {
   TYPE_FIELDS (x) = TYPE_FIELDS (t);
   TYPE_LANG_SPECIFIC (x) = TYPE_LANG_SPECIFIC (t);
+  TYPE_TRANSPARENT_AGGR (x) = TYPE_TRANSPARENT_AGGR (t);
   C_TYPE_FIELDS_READONLY (x) = C_TYPE_FIELDS_READONLY (t);
   C_TYPE_FIELDS_VOLATILE (x) = C_TYPE_FIELDS_VOLATILE (t);
   C_TYPE_VARIABLE_SIZE (x) = C_TYPE_VARIABLE_SIZE (t);
   C_TYPE_INCOMPLETE_VARS (x) = NULL_TREE;
 }
 
-  /* If this was supposed to be a transparent union, but we can't
- make it one, warn and turn off the flag.  */
-  if (TREE_CODE (t) == UNION_TYPE
-  && TYPE_TRANSPARENT_AGGR (t)
-  && (!TYPE_FIELDS (t) || TYPE_MODE (t) != DECL_MODE (TYPE_FIELDS (t
-{
-  TYPE_TRANSPARENT_AGGR (t) = 0;
-  warning_at (loc, 0, "union cannot be made transparent");
-}
-
   /* Update type location to the one of the definition, instead of e.g.
  a forward declaration.  */
   if (TYPE_STUB_DECL (t))

Re: [PATCH][C] Reset TYPE_TRANSPARENT_AGGR on all type variants when not supported

[Jakub Jelinek]

On Tue, Feb 05, 2019 at 02:15:30PM +0100, Richard Biener wrote:
> 
> The following fixes an ICE in the type verifier for transparent_union
> marked unions that we refuse to handle such.  
> (gcc.dg/transparent-union-6.c)
> 
> Bootstrap & regtest running on x86_64-unknown-linux-gnu, OK?
> 
> Thanks,
> Richard.
> 
> 2019-02-05  Richard Biener  
> 
>   PR c/88606
>   * c-decl.c (finish_struct): Reset TYPE_TRANSPARENT_AGGR on
>   all type variants when not supported.

Ok, thanks.

Jakub

Re: Make clear, when contributions will be ignored

[Дилян Палаузов]

Hello,

the current way to come forward is to send biweekly manual reminders.

Will it help, if bugzilla is tweaked to send reminders every two weeks for 
ready-patches?  This also has the advantage,
that people will not have to once update a patch in BZ and then send it over 
gcc-patches.

Regards
  Дилян

On Fri, 2018-12-07 at 10:55 +, Дилян Палаузов wrote:
> Hello,
> 
> will it help, if Bugzilla is reprogrammed to send automatically weekly
> reminders on all patches, that are not integrated yet?
> 
> Will lt help, if I hire myself to integrate the patch, or shall I
> rather hire somebody to send reminders?
> 
> If something can be done after sending a reminder, then it can be
> arranged also without reminders.  In particular, dealing with reminders
> is avoidable extra work.
> 
> Whether people are paid or not, does not change on the subject very
> much.  I have experienced organizations, where people are not paid and
> they manage to tackle everything.  I have seen organizations where
> people are paid and they do not get the management right.
> 
> I am not speaking about having some strict time to get a response, but
> rather to ensure an answer in reasonable time.  No answer in reasonable
> time is the same as ignorance — the subject of this thread.
> 
> The patch I proposed on 27th Oct was first submitted towards GDB and
> then I was told to send it to GCC.  Here I was told to sent it to GDB. 
> What shall happen to quit the loop?
> 
> In any case, if the common aim is to have a system where contributions
> do not get lost, then I’m sure the workflows can be adjusted to achieve
> this aim.
> 
> Regards
>   Дилян
> 
> 
> On Wed, 2018-12-05 at 17:37 +, Joseph Myers wrote:
> > On Wed, 5 Dec 2018, Segher Boessenkool wrote:
> > 
> > > Patches are usually ignored because everyone thinks someone else will
> > > handle it.
> > 
> > And in this case, it looks like this patch would best be reviewed first in 
> > the GDB context - then once committed to binutils-gdb, the committer could 
> > post to gcc-patches (CC:ing build system maintainers) requesting a commit 
> > to GCC if they don't have write access to GCC themselves.  I consider 
> > synchronizing changes to such top-level files in either direction to be 
> > obvious and not to need a separate review.
> > 

[Patch] [arm] Fix 88714, Arm LDRD/STRD peepholes

[Matthew Malcomson]

These peepholes match a pair of SImode loads or stores that can be
implemented with a single LDRD or STRD instruction.
When compiling for TARGET_ARM, these peepholes originally created a set
pattern in DI mode to be caught by movdi patterns.

This approach failed to take into account the possibility that the two
matched insns operated on memory with different aliasing information.
The peepholes lost the aliasing information on one of the insns, which
could then cause the scheduler to make an invalid transformation.

This patch changes the peepholes so they generate a PARALLEL expression
of the two relevant loads or stores, which means the aliasing
information of both is kept.  Such a PARALLEL pattern is what the
peepholes currently produce for TARGET_THUMB2.

In order to match these new insn patterns, we add two new define_insn's.  These
define_insn's use the same checks as the peepholes to find valid insns.

Note that the patterns now created by the peepholes for LDRD and STRD
are very similar to those created by the peepholes for LDM and STM.
Many patterns could be matched by the LDM and STM define_insns, which
means we rely on the order the define_insn patterns are defined in the
machine description, with those for LDRD/STRD defined before those for
LDM/STM.

The difference between the peepholes for LDRD/STRD and those for LDM/STM
are mainly that those for LDRD/STRD have some logic to ensure that the
two registers are consecutive and the first one is even.

Bootstrapped and regtested on arm-none-linux-gnu.
Demonstrated fix of bug 88714 by bootstrapping on armv7l.


gcc/ChangeLog:

2019-02-05  Matthew Malcomson  

PR bootstrap/88714
* config/arm/arm-protos.h (valid_operands_ldrd_strd,
arm_count_ldrdstrd_insns): New declarations.
* config/arm/arm.c (mem_ok_for_ldrd_strd): Remove broken handling of
MINUS.
(valid_operands_ldrd_strd): New function.
(arm_count_ldrdstrd_insns): New function.
* config/arm/ldrdstrd.md: Change peepholes to generate PARALLEL SImode
sets instead of single DImode set and define new insns to match this.

gcc/testsuite/ChangeLog:

2019-02-05  Matthew Malcomson  

* gcc.c-torture/execute/pr88714.c: New test.
* gcc.dg/rtl/arm/ldrd-peepholes.c: New test.



### Attachment also inlined for ease of reply###


diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
index 
79ede0db174fcce87abe8b4d18893550d4c7e2f6..485bc68a618d6ae4a1640368ccb025fe2c9e1420
 100644
--- a/gcc/config/arm/arm-protos.h
+++ b/gcc/config/arm/arm-protos.h
@@ -125,6 +125,7 @@ extern rtx arm_gen_store_multiple (int *, int, rtx, int, 
rtx, HOST_WIDE_INT *);
 extern bool offset_ok_for_ldrd_strd (HOST_WIDE_INT);
 extern bool operands_ok_ldrd_strd (rtx, rtx, rtx, HOST_WIDE_INT, bool, bool);
 extern bool gen_operands_ldrd_strd (rtx *, bool, bool, bool);
+extern bool valid_operands_ldrd_strd (rtx *, bool);
 extern int arm_gen_movmemqi (rtx *);
 extern bool gen_movmem_ldrd_strd (rtx *);
 extern machine_mode arm_select_cc_mode (RTX_CODE, rtx, rtx);
@@ -146,6 +147,7 @@ extern const char *output_mov_long_double_arm_from_arm (rtx 
*);
 extern const char *output_move_double (rtx *, bool, int *count);
 extern const char *output_move_quad (rtx *);
 extern int arm_count_output_move_double_insns (rtx *);
+extern int arm_count_ldrdstrd_insns (rtx *, bool);
 extern const char *output_move_vfp (rtx *operands);
 extern const char *output_move_neon (rtx *operands);
 extern int arm_attr_length_move_neon (rtx_insn *);
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 
73cb8df9af1ec9d680091bb8691bcd925a1be1d3..1c336da9e5b0948ef1058c46966364510dc1ca38
 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -15556,7 +15556,7 @@ mem_ok_for_ldrd_strd (rtx mem, rtx *base, rtx *offset, 
HOST_WIDE_INT *align)
   *base = addr;
   return true;
 }
-  else if (GET_CODE (addr) == PLUS || GET_CODE (addr) == MINUS)
+  else if (GET_CODE (addr) == PLUS)
 {
   *base = XEXP (addr, 0);
   *offset = XEXP (addr, 1);
@@ -15721,7 +15721,7 @@ gen_operands_ldrd_strd (rtx *operands, bool load,
 }
 
   /* Make sure accesses are to consecutive memory locations.  */
-  if (gap != 4)
+  if (gap != GET_MODE_SIZE (SImode))
 return false;
 
   if (!align_ok_ldrd_strd (align[0], offset))
@@ -15802,6 +15802,55 @@ gen_operands_ldrd_strd (rtx *operands, bool load,
 }
 
 
+/* Return true if parallel execution of the two word-size accesses provided
+   could be satisfied with a single LDRD/STRD instruction.  Two word-size
+   accesses are represented by the OPERANDS array, where OPERANDS[0,1] are
+   register operands and OPERANDS[2,3] are the corresponding memory operands.
+   */
+bool
+valid_operands_ldrd_strd (rtx *operands, bool load)
+{
+  int nops = 2;
+  HOST_WIDE_INT offsets[2], offset, align[2];
+  rtx base = NULL_RTX;
+  rtx cur_base, cur_offset;
+  int i, gap;
+
+  /* Check that the me

[PATCH] libstdc++/89130 and libstdc++/89090 fixes for vector relocation

[Jonathan Wakely]

This fixes two PRs, one trivial (don't use C++17 features in C++11
mode) and one more serious (don't require MoveInsertable when we
should only need CopyInsertable).

It would be nice to rely on if-constexpr in C++11 mode, but it causes
clang warnings, complicates testcase bisection/reduction, and causes
users to file bogus bug reports. So let's just avoid it.

Tested powerpc64le-linux, committed to trunk.


commit 51908e56bd32b5f89bc909193c3da957de01c3e0
Author: Jonathan Wakely 
Date:   Tue Feb 5 11:50:18 2019 +

PR libstdc++/89130 restore support for non-MoveConstructible types

The changes to "relocate" std::vector elements can lead to new errors
outside the immediate context, because moving the elements to new
storage no longer makes use of the move-if-noexcept utilities. This
means that types with deleted moves no longer degenerate to copies, but
are just ill-formed. The errors happen while instantiating the
noexcept-specifier for __relocate_object_a, when deciding whether to try
to relocate.

This patch introduces indirections to avoid the ill-formed
instantiations of std::__relocate_object_a. In order to avoid using
if-constexpr prior to C++17 this is done by tag dispatching. After this
patch all uses of std::__relocate_a are guarded by checks that will
support sensible code (i.e. code not using custom allocators that fool
the new checks).

PR libstdc++/89130
* include/bits/alloc_traits.h (__is_copy_insertable_impl): Rename to
__is_alloc_insertable_impl. Replace single type member with two
members, one for each of copy and move insertable.
(__is_move_insertable): New trait for internal use.
* include/bits/stl_vector.h (vector::_S_nothrow_relocate(true_type))
(vector::_S_nothrow_relocate(true_type)): New functions to
conditionally check if __relocate_a can throw.
(vector::_S_use_relocate()): Dispatch to _S_nothrow_relocate based
on __is_move_insertable.
(vector::_S_do_relocate): New overloaded functions to conditionally
call __relocate_a.
(vector::_S_relocate): New function that dispatches to _S_do_relocate
based on _S_use_relocate.
* include/bits/vector.tcc (vector::reserve, vector::_M_realloc_insert)
(vector::_M_default_append): Call _S_relocate instead of __relocate_a.
* testsuite/23_containers/vector/modifiers/push_back/89130.cc: New.

diff --git a/libstdc++-v3/include/bits/alloc_traits.h b/libstdc++-v3/include/bits/alloc_traits.h
index ed61ce845f8..3b0c16fbf64 100644
--- a/libstdc++-v3/include/bits/alloc_traits.h
+++ b/libstdc++-v3/include/bits/alloc_traits.h
@@ -577,14 +577,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 }
 
   template
-class __is_copy_insertable_impl
+class __is_alloc_insertable_impl
 {
-  typedef allocator_traits<_Alloc> _Traits;
+  using _Traits = allocator_traits<_Alloc>;
+  using value_type = typename _Traits::value_type;
 
-  template,
+	   typename
 	   = decltype(_Traits::construct(std::declval<_Alloc&>(),
-	 std::declval<_Up*>(),
-	 std::declval()))>
+	 std::declval<_Tp*>(),
+	 std::declval<_Up>()))>
 	static true_type
 	_M_select(int);
 
@@ -593,13 +595,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	_M_select(...);
 
 public:
-  typedef decltype(_M_select(0)) type;
+  using copy = decltype(_M_select(0));
+  using move = decltype(_M_select(0));
 };
 
   // true if _Alloc::value_type is CopyInsertable into containers using _Alloc
   template
 struct __is_copy_insertable
-: __is_copy_insertable_impl<_Alloc>::type
+: __is_alloc_insertable_impl<_Alloc>::copy
 { };
 
   // std::allocator<_Tp> just requires CopyConstructible
@@ -608,6 +611,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 : is_copy_constructible<_Tp>
 { };
 
+  // true if _Alloc::value_type is MoveInsertable into containers using _Alloc
+  template
+struct __is_move_insertable
+: __is_alloc_insertable_impl<_Alloc>::move
+{ };
+
+  // std::allocator<_Tp> just requires MoveConstructible
+  template
+struct __is_move_insertable>
+: is_move_constructible<_Tp>
+{ };
+
   // Trait to detect Allocator-like types.
   template
 struct __is_allocator : false_type { };
diff --git a/libstdc++-v3/include/bits/stl_vector.h b/libstdc++-v3/include/bits/stl_vector.h
index 43debda54f1..10bf4fac62e 100644
--- a/libstdc++-v3/include/bits/stl_vector.h
+++ b/libstdc++-v3/include/bits/stl_vector.h
@@ -425,14 +425,47 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 private:
 #if __cplusplus >= 201103L
   static constexpr bool
-  _S_use_relocate()
+  _S_nothrow_relocate(true_type)
   {
 	return noexcept(std::__relocate_a(std::declval(),
 	  std::declval(),
 	  std::declval(),
 	  std::declval<_Tp_alloc_type&>()));
  

Re: [PATCH] libstdc++/89130 and libstdc++/89090 fixes for vector relocation

[Jonathan Wakely]

On 05/02/19 14:45 +, Jonathan Wakely wrote:

This fixes two PRs, one trivial (don't use C++17 features in C++11
mode) and one more serious (don't require MoveInsertable when we
should only need CopyInsertable).

It would be nice to rely on if-constexpr in C++11 mode, but it causes
clang warnings, complicates testcase bisection/reduction, and causes
users to file bogus bug reports. So let's just avoid it.


Oh, and the test change can be reverted now that types with deleted
moves work again.

Tested x86_64-linux, committed to trunk.


commit 3cd6139661801ab6bd88558e336a8ba12d1ed640
Author: Jonathan Wakely 
Date:   Tue Feb 5 14:50:58 2019 +

Restore previous behaviour of test

Go back to using CopyConsOnlyType as before r265485, because it works
again now. Add test using DelAnyAssign for completeness and additional
coverage.

* testsuite/23_containers/vector/modifiers/push_back/49836.cc: Restore
use of CopyConsOnlyType, but also test DelAnyAssign for completeness.

diff --git a/libstdc++-v3/testsuite/23_containers/vector/modifiers/push_back/49836.cc b/libstdc++-v3/testsuite/23_containers/vector/modifiers/push_back/49836.cc
index 1a7d8729718..3e59781e7cc 100644
--- a/libstdc++-v3/testsuite/23_containers/vector/modifiers/push_back/49836.cc
+++ b/libstdc++-v3/testsuite/23_containers/vector/modifiers/push_back/49836.cc
@@ -24,11 +24,12 @@
 // libstdc++/49836
 void test01()
 {
-  using __gnu_test::assign::DelAnyAssign;
+  using __gnu_test::CopyConsOnlyType;
   using __gnu_test::MoveConsOnlyType;
+  using __gnu_test::assign::DelAnyAssign;
 
-  std::vector v1;
-  DelAnyAssign t1;
+  std::vector v1;
+  CopyConsOnlyType t1(1);
   v1.push_back(t1);
   v1.push_back(t1);
   v1.push_back(t1);
@@ -40,6 +41,14 @@ void test01()
   v2.push_back(std::move(t2));
   v2.push_back(std::move(t2));
   VERIFY( v2.size() == 3 );
+
+  std::vector v3;
+  DelAnyAssign t3;
+  v3.push_back(t3);
+  v3.push_back(t3);
+  v3.push_back(t3);
+  VERIFY( v3.size() == 3 );
+
 }
 
 int main()

Re: [PATCH, rs6000] Fix instruction counts on powerpc64 test cases.

[Bill Seurer]

On 02/05/19 05:31, Segher Boessenkool wrote:

Do we not want to test this on LE anymore?  Oh, P7, heh.  Okay.  Not
enough coffee yet :-)


It will work if I specify -mbig-endian -mabi=elfv1 but yeah, we don't 
really care about P7 generating LE code.



+/* { dg-final { scan-assembler-times "xvcmpgedp" 6 } } */
+/* { dg-final { scan-assembler-times "xvcmpgedp." 6 } } */

These need fixing still: both of these match
   xvcmpgedp 0,1,2
because dot means "any character", in a regexp.  Maybe

/* { dg-final { scan-assembler-times {\mxvcmpgedp\s} 6 } } */
/* { dg-final { scan-assembler-times {\mxvcmpgedp\.\s} 6 } } */

(and with the counts fixed).  (Same for all other dot insns of course).



A lot of these test cases (as in power-specific opcode tests) test for 
both OPCODE and OPCODE. which as you noted matches both.  Many others 
just test for OPCODE (which will also match OPCODE.).  I meant to ask if 
that was actually useful as they always matched the same thing either way.


I will do the updates you recommended.  Should I post the updated diffs 
or is this enough to submit the patch?

--

-Bill Seurer

[PATCH] Fix not 8-byte aligned ldrd/strd on ARMv5

[Bernd Edlinger]

Hi,

due to the AAPCS parameter passing of 8-byte aligned structures, which happen to
be 8-byte aligned or only 4-byte aligned in the test case, ldrd instructions
are generated that may access 4-byte aligned stack slots, which will trap on 
ARMv5 and
ARMv6 according to the following document:


http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0473m/dom1361290002364.html
says:

"In ARMv5TE, or in ARMv6 when SCTLR.U is 0, LDRD and STRD doubleword data 
transfers must be
eight-byte aligned.  Use ALIGN 8 before memory allocation directives such as 
DCQ if the data
is to be accessed using LDRD or STRD.  This is not required in ARMv6 when 
SCTLR.U is 1, or in
ARMv7, because in these versions, doubleword data transfers can be 
word-aligned."


The reason why the ldrd instruction is generated seems to be a missing 
alignment check in the
function output_move_double.  But when that is fixed, it turns out that if the 
parameter happens
to be 8-byte aligned by chance, they still have MEM_ALIGN = 4, which prevents 
the ldrd completely.

The reason for that is in function.c (assign_parm_find_stack_rtl), where values 
that happen to be
aligned to STACK_BOUNDARY, are only  aligned to PARM_BOUNDARY.


Bootstrapped and reg-tested on x86_64-pc-linux-gnu and arm-linux-gnueabihf with 
all languages.
Is it OK for trunk?


Thanks
Bernd.
2019-02-05  Bernd Edlinger  

	* config/arm/arm.c (output_move_double): Check required memory
	alignment for ldrd/strd instructions.
	* function.c (assign_parm_find_stack_rtl): Use larger alignment
	when possible.

testsuite:
2019-02-05  Bernd Edlinger  

	* gcc.target/arm/unaligned-argument-1.c: New test.
	* gcc.target/arm/unaligned-argument-2.c: New test.

Index: gcc/config/arm/arm.c
===
--- gcc/config/arm/arm.c	(revision 268337)
+++ gcc/config/arm/arm.c	(working copy)
@@ -18303,6 +18303,8 @@ output_move_double (rtx *operands, bool emit, int
   otherops[0] = gen_rtx_REG (SImode, 1 + reg0);
 
   gcc_assert (code1 == MEM);  /* Constraints should ensure this.  */
+  bool allow_ldrd = TARGET_LDRD
+			&& align_ok_ldrd_strd (MEM_ALIGN (operands[1]), 0);
 
   switch (GET_CODE (XEXP (operands[1], 0)))
 	{
@@ -18310,8 +18312,8 @@ output_move_double (rtx *operands, bool emit, int
 
 	  if (emit)
 	{
-	  if (TARGET_LDRD
-		  && !(fix_cm3_ldrd && reg0 == REGNO(XEXP (operands[1], 0
+	  if (allow_ldrd
+		  && !(fix_cm3_ldrd && reg0 == REGNO (XEXP (operands[1], 0
 		output_asm_insn ("ldrd%?\t%0, [%m1]", operands);
 	  else
 		output_asm_insn ("ldmia%?\t%m1, %M0", operands);
@@ -18319,7 +18321,7 @@ output_move_double (rtx *operands, bool emit, int
 	  break;
 
 	case PRE_INC:
-	  gcc_assert (TARGET_LDRD);
+	  gcc_assert (allow_ldrd);
 	  if (emit)
 	output_asm_insn ("ldrd%?\t%0, [%m1, #8]!", operands);
 	  break;
@@ -18327,7 +18329,7 @@ output_move_double (rtx *operands, bool emit, int
 	case PRE_DEC:
 	  if (emit)
 	{
-	  if (TARGET_LDRD)
+	  if (allow_ldrd)
 		output_asm_insn ("ldrd%?\t%0, [%m1, #-8]!", operands);
 	  else
 		output_asm_insn ("ldmdb%?\t%m1!, %M0", operands);
@@ -18337,7 +18339,7 @@ output_move_double (rtx *operands, bool emit, int
 	case POST_INC:
 	  if (emit)
 	{
-	  if (TARGET_LDRD)
+	  if (allow_ldrd)
 		output_asm_insn ("ldrd%?\t%0, [%m1], #8", operands);
 	  else
 		output_asm_insn ("ldmia%?\t%m1!, %M0", operands);
@@ -18345,7 +18347,7 @@ output_move_double (rtx *operands, bool emit, int
 	  break;
 
 	case POST_DEC:
-	  gcc_assert (TARGET_LDRD);
+	  gcc_assert (allow_ldrd);
 	  if (emit)
 	output_asm_insn ("ldrd%?\t%0, [%m1], #-8", operands);
 	  break;
@@ -18483,7 +18485,7 @@ output_move_double (rtx *operands, bool emit, int
 		}
 		  otherops[0] = gen_rtx_REG(SImode, REGNO(operands[0]) + 1);
 		  operands[1] = otherops[0];
-		  if (TARGET_LDRD
+		  if (allow_ldrd
 		  && (REG_P (otherops[2])
 			  || TARGET_THUMB2
 			  || (CONST_INT_P (otherops[2])
@@ -18544,7 +18546,7 @@ output_move_double (rtx *operands, bool emit, int
 	  if (count)
 		*count = 2;
 
-	  if (TARGET_LDRD)
+	  if (allow_ldrd)
 		return "ldrd%?\t%0, [%1]";
 
 	  return "ldmia%?\t%1, %M0";
@@ -18589,7 +18591,8 @@ output_move_double (rtx *operands, bool emit, int
 	 values but user assembly constraints can force an odd
 	 starting register.  */
   bool allow_strd = TARGET_LDRD
-			 && !(TARGET_ARM && (REGNO (operands[1]) & 1) == 1);
+			 && !(TARGET_ARM && (REGNO (operands[1]) & 1) == 1)
+			 && align_ok_ldrd_strd (MEM_ALIGN (operands[0]), 0);
   switch (GET_CODE (XEXP (operands[0], 0)))
 {
 	case REG:
Index: gcc/function.c
===
--- gcc/function.c	(revision 268337)
+++ gcc/function.c	(working copy)
@@ -2698,8 +2698,20 @@ assign_parm_find_stack_rtl (tree parm, struct assi
  intentionally forcing upward padding.  Otherwise we have to come
  up with a gues

[PATCH] Fix RTL DCE (PR target/89188)

[Jakub Jelinek]

Hi!

RTL DCE is called in different modes, sometimes it is allowed to alter cfg,
in other cases it is not.  The following testcase ICEs because when it is
not allowed to alter cfg (combiner's df_analyze) we remove a noop move that
was considered to potentially throw and that made a bb unreachable (which
the pass didn't like), but even removing those unreachable bbs would ICE
anyway, as the algorithm is for !can_alter_cfg not really prepared to see
bbs and edges being removed.

So, the following patch does 3 things:
1) doesn't delete noop moves if they can
   throw and we can't alter cfg or delete dead exceptions
2) instead of setting must_clean blindly for any CALL_INSNs, it instead
   sets it accurately from delete_insn_and_edges return value
3) it asserts that if !can_alter_cfg we don't actually have must_clean set

Bootstrapped/regtested on powerpc64{,le}-linux, ok for trunk?

2019-02-05  Jakub Jelinek  

PR target/89188
* dce.c (delete_unmarked_insns): Don't remove no-op moves if they
can throw, non-call exceptions are enabled and we can't delete
dead exceptions or alter cfg.  Set must_clean if
delete_insn_and_edges returns true, don't set it blindly for calls.
Assert that delete_unreachable_blocks is called only if can_alter_cfg.

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

--- gcc/dce.c.jj2019-02-05 10:04:16.984062429 +0100
+++ gcc/dce.c   2019-02-05 10:53:47.655584503 +0100
@@ -584,7 +584,12 @@ delete_unmarked_insns (void)
  rtx turn_into_use = NULL_RTX;
 
  /* Always delete no-op moves.  */
- if (noop_move_p (insn))
+ if (noop_move_p (insn)
+ /* Unless the no-op move can throw and we are not allowed
+to alter cfg.  */
+ && (!cfun->can_throw_non_call_exceptions
+ || (cfun->can_delete_dead_exceptions && can_alter_cfg)
+ || insn_nothrow_p (insn)))
{
  if (RTX_FRAME_RELATED_P (insn))
turn_into_use
@@ -627,12 +632,6 @@ delete_unmarked_insns (void)
 for the destination regs in order to avoid dangling notes.  */
  remove_reg_equal_equiv_notes_for_defs (insn);
 
- /* If a pure or const call is deleted, this may make the cfg
-have unreachable blocks.  We rememeber this and call
-delete_unreachable_blocks at the end.  */
- if (CALL_P (insn))
-   must_clean = true;
-
  if (turn_into_use)
{
  /* Don't remove frame related noop moves if they cary
@@ -645,12 +644,15 @@ delete_unmarked_insns (void)
}
  else
/* Now delete the insn.  */
-   delete_insn_and_edges (insn);
+   must_clean |= delete_insn_and_edges (insn);
}
 
   /* Deleted a pure or const call.  */
   if (must_clean)
-delete_unreachable_blocks ();
+{
+  gcc_assert (can_alter_cfg);
+  delete_unreachable_blocks ();
+}
 }
 
 
--- gcc/testsuite/g++.dg/opt/pr89188.C.jj   2019-02-04 14:20:57.714851393 
+0100
+++ gcc/testsuite/g++.dg/opt/pr89188.C  2019-02-04 14:52:09.988782531 +0100
@@ -0,0 +1,5 @@
+// PR target/89188
+// { dg-do compile { target c++11 } }
+// { dg-options "-Og -flive-range-shrinkage -fnon-call-exceptions" }
+
+#include "../torture/pr88861.C"

Jakub

[committed] Fix combiner make_extraction (PR rtl-optimization/89195)

[Jakub Jelinek]

Hi!

The following patch ensures we don't access bytes beyond the original MEM,
those might not be mapped etc., or for big endian the offset computation
computes the offset incorrectly.

Bootstrapped/regtested on powerpc64{,-le}-linux, Wilco has tested it on
aarch64_be-*, preapproved by Segher in the PR, committed to trunk.

2019-02-05  Jakub Jelinek  

PR rtl-optimization/89195
* combine.c (make_extraction): For MEMs, don't extract bytes outside
of the original MEM.

* gcc.c-torture/execute/pr89195.c: New test.

--- gcc/combine.c.jj2019-02-01 23:52:24.701025057 +0100
+++ gcc/combine.c   2019-02-05 12:51:17.813658725 +0100
@@ -7687,6 +7687,7 @@ make_extraction (machine_mode mode, rtx
  /* We can't do this if we are widening INNER_MODE (it
 may not be aligned, for one thing).  */
  && !paradoxical_subreg_p (tmode, inner_mode)
+ && known_le (pos + len, GET_MODE_PRECISION (is_mode))
  && (inner_mode == tmode
  || (! mode_dependent_address_p (XEXP (inner, 0),
  MEM_ADDR_SPACE (inner))
--- gcc/testsuite/gcc.c-torture/execute/pr89195.c.jj2019-02-05 
12:55:40.856362136 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr89195.c   2019-02-05 
12:55:09.920853996 +0100
@@ -0,0 +1,22 @@
+/* PR rtl-optimization/89195 */
+/* { dg-require-effective-target int32plus } */
+
+struct S { unsigned i : 24; };
+
+volatile unsigned char x;
+
+__attribute__((noipa)) int
+foo (struct S d) 
+{
+  return d.i & x;
+}
+
+int
+main ()
+{
+  struct S d = { 0x123456 };
+  x = 0x75;
+  if (foo (d) != (0x56 & 0x75))
+__builtin_abort ();
+  return 0;
+}

Jakub

[PATCH] PR libstdc++/89194 untangle is_convertible and is_nothrow_convertible

[Jonathan Wakely]

The additional logic added to __is_convertible_helper in order to
support is_nothrow_convertible makes some uses of is_convertible
ill-formed. This appears to be due to PR c++/87603, but can be avoided
just by defining a separate helper for is_nothrow_convertible. The same
problems are likely to still exist for is_nothrow_convertible, but that
is new and so won't cause regressions for existing users of
is_convertible.

PR libstdc++/89194
* include/std/type_traits (__is_convertible_helper)
(__is_convertible_helper<_From, _To, false>): Revert changes to
support is_nothrow_convertible.
(__is_nt_convertible_helper): New helper.
(is_nothrow_convertible): Use __is_nt_convertible_helper.

Tested powerpc64le-linux, committed to trunk.


commit 0fd1d35fad359e5406deb936688e228f96447ee9
Author: Jonathan Wakely 
Date:   Tue Feb 5 15:16:09 2019 +

PR libstdc++/89194 untangle is_convertible and is_nothrow_convertible

The additional logic added to __is_convertible_helper in order to
support is_nothrow_convertible makes some uses of is_convertible
ill-formed. This appears to be due to PR c++/87603, but can be avoided
just by defining a separate helper for is_nothrow_convertible. The same
problems are likely to still exist for is_nothrow_convertible, but that
is new and so won't cause regressions for existing users of
is_convertible.

PR libstdc++/89194
* include/std/type_traits (__is_convertible_helper)
(__is_convertible_helper<_From, _To, false>): Revert changes to
support is_nothrow_convertible.
(__is_nt_convertible_helper): New helper.
(is_nothrow_convertible): Use __is_nt_convertible_helper.

diff --git a/libstdc++-v3/include/std/type_traits 
b/libstdc++-v3/include/std/type_traits
index 7c7aeeb3b32..f05a583cb04 100644
--- a/libstdc++-v3/include/std/type_traits
+++ b/libstdc++-v3/include/std/type_traits
@@ -1344,9 +1344,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 struct __is_convertible_helper
 {
   typedef typename is_void<_To>::type type;
-#if __cplusplus > 201703L
-  typedef type __is_nothrow_type;
-#endif
 };
 
   template
@@ -1364,23 +1361,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
static false_type
__test(...);
 
-#if __cplusplus > 201703L
-  template(std::declval<_From1>()))>
-   static __bool_constant<_NoEx>
-   __test_nothrow(int);
-
-  template
-   static false_type
-   __test_nothrow(...);
-#endif
-
 public:
   typedef decltype(__test<_From, _To>(0)) type;
-
-#if __cplusplus > 201703L
-  typedef decltype(__test_nothrow<_From, _To>(0)) __is_nothrow_type;
-#endif
 };
 
 
@@ -1391,10 +1373,35 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 { };
 
 #if __cplusplus > 201703L
+template, is_function<_To>,
+is_array<_To>>::value>
+struct __is_nt_convertible_helper
+: is_void<_To>
+{ };
+
+  template
+class __is_nt_convertible_helper<_From, _To, false>
+{
+  template
+   static void __test_aux(_To1) noexcept;
+
+  template
+   static bool_constant(std::declval<_From1>()))>
+   __test(int);
+
+  template
+   static false_type
+   __test(...);
+
+public:
+  using type = decltype(__test<_From, _To>(0));
+};
+
   /// is_nothrow_convertible
   template
 struct is_nothrow_convertible
-: public __is_convertible_helper<_From, _To>::__is_nothrow_type
+: public __is_nt_convertible_helper<_From, _To>::type
 { };
 
   /// is_nothrow_convertible_v

Re: C++ PATCH for c++/88325 - ICE with invalid out-of-line template member definition

[Jason Merrill]

On Mon, Feb 4, 2019 at 4:52 PM Marek Polacek  wrote:
> On Mon, Feb 04, 2019 at 09:44:01AM -0500, Jason Merrill wrote:
> > On 2/1/19 2:55 PM, Marek Polacek wrote:
> > > On Fri, Feb 01, 2019 at 12:02:44PM -0500, Jason Merrill wrote:
> > > > On 2/1/19 11:26 AM, Marek Polacek wrote:
> > > > > On Wed, Jan 30, 2019 at 01:39:11PM -0500, Jason Merrill wrote:
> > > > > > On 1/28/19 9:46 PM, Marek Polacek wrote:
> > > > > > > This patch fixes an ICE-on-invalid (becase out-of-line 
> > > > > > > constructors can't have
> > > > > > > template arguments and also because function templates can't be 
> > > > > > > partially
> > > > > > > specialized) in C++2a: when we're parsing
> > > > > > >
> > > > > > >  template template A::A ()
> > > > > > >
> > > > > > > in the attached test we end up parsing "A::A" as a type 
> > > > > > > name, and first we
> > > > > > > try a class-name.  First we process "A::" as the nested name 
> > > > > > > specifier and then
> > > > > > > we parse "A".  In this test that results in a BASELINK.  
> > > > > > > Because in this context
> > > > > > > we're supposed to treat it as a typename ([temp.res]/6), we call 
> > > > > > > make_typename_type,
> > > > > > > but that crashes.
> > > > > >
> > > > > > Hmm.  If we've done an actual lookup (that gave us a BASELINK), we 
> > > > > > aren't
> > > > > > dealing with a member of an unknown specialization anymore, so we 
> > > > > > should
> > > > > > just use the result of the lookup rather than speculate about what 
> > > > > > the name
> > > > > > might mean.  Why are we still trying to treat it as a typename?
> > > > >
> > > > > Good point.  It's because cp_parser_class_name has:
> > > > >
> > > > > 23095   /* Any name names a type if we're following the `typename' 
> > > > > keyword
> > > > > 23096  in a qualified name where the enclosing scope is 
> > > > > type-dependent.  */
> > > > > 23097   typename_p = (typename_keyword_p && scope && TYPE_P (scope)
> > > > > 23098 && dependent_type_p (scope));
> > > > >
> > > > > and scope is in this case "A" which is dependent.  Then there's 
> > > > > this
> > > > > "identifier, but not template-id" case which only performs name 
> > > > > lookup when
> > > > > typename_p is false.  But we're parsing "A" so we call
> > > > > cp_parser_template_id.  It sees CPP_TEMPLATE_ID -- an already parsed
> > > > > template-id, so it just returns it, which is a BASELINK.  So even 
> > > > > messing
> > > > > with tag_type wouldn't help.
> > > > >
> > > > > Does this answer your question?
> > > >
> > > > Mostly, though I'm still curious how we got the previous parse of the
> > > > template-id and yet continued to try to parse it as a class-name.
> > >
> > > So we have
> > > template template
> > > A::A ()
> > >
> > > and we're in cp_parser_single_declaration.  We're trying to parse the
> > > decl-specifier-seq, and that first tries to parse variou RID_* keywords
> > > and if that doesn't work it tries to parse a constructor:
> > >
> > >/* Constructors are a special case.  The `S' in `S()' is not a
> > >  decl-specifier; it is the beginning of the declarator.  */
> > >constructor_p
> > > = (!found_decl_spec
> > >&& constructor_possible_p
> > >&& (cp_parser_constructor_declarator_p
> > >(parser, decl_spec_seq_has_spec_p (decl_specs, ds_friend;
> > >
> > > cp_parser_constructor_declarator_p calls 
> > > cp_parser_nested_name_specifier_opt
> > > and that's where we parse the two template-ids, A and A.  But
> > > cp_parser_constructor_declarator_p isn't successful so we go to parsing
> > > A::A as a type-specifier, and that's where the above happens.
> >
> > Aha.  That seems like a bug in cp_parser_constructor_declarator.
>
> Sorry, what specifically, that cp_parser_constructor_declarator_p thinks it's
> not a ctor?  I thought it was an invalid ctor, since "out-of-line constructor
> for 'A' cannot have template arguments".

True, good point, though it is still more a constructor declarator
than anything else.  It would be nice for the diagnostic to suggest
removing the redundant template arguments aren't allowed, not just the
current confusing message about a partial specialization.  The
diagnostic for a similar declaration of a nested class template is
"partial specialization ‘struct A::B’ does not specialize any
template arguments; to define the primary template, remove the
template argument list".

We also shouldn't mention the internal name "__ct".

> Once again, we're parsing "A::A ()".  "A::" is parsed as a nested-
> name-specifier = struct A.  Then:
>
> 27377   /* If we have a class scope, this is easy; DR 147 says that S::S 
> always
> 27378  names the constructor, and no other qualified name could.  */
> 27379   if (constructor_p && nested_name_specifier
> 27380   && CLASS_TYPE_P (nested_name_specifier))
> 27381 {
> 27382   tree id = cp_parser_unqualified_id (parser,
> 27383   /*

Re: [PATCH, rs6000] Fix instruction counts on powerpc64 test cases.

[Segher Boessenkool]

On Tue, Feb 05, 2019 at 08:58:44AM -0600, Bill Seurer wrote:
> On 02/05/19 05:31, Segher Boessenkool wrote:
> >Do we not want to test this on LE anymore?  Oh, P7, heh.  Okay.  Not
> >enough coffee yet :-)
> 
> It will work if I specify -mbig-endian -mabi=elfv1 but yeah, we don't 
> really care about P7 generating LE code.
> 
> >>+/* { dg-final { scan-assembler-times "xvcmpgedp" 6 } } */
> >>+/* { dg-final { scan-assembler-times "xvcmpgedp." 6 } } */
> >These need fixing still: both of these match
> >   xvcmpgedp 0,1,2
> >because dot means "any character", in a regexp.  Maybe
> >
> >/* { dg-final { scan-assembler-times {\mxvcmpgedp\s} 6 } } */
> >/* { dg-final { scan-assembler-times {\mxvcmpgedp\.\s} 6 } } */
> >
> >(and with the counts fixed).  (Same for all other dot insns of course).
> >
> 
> A lot of these test cases (as in power-specific opcode tests) test for 
> both OPCODE and OPCODE. which as you noted matches both.  Many others 
> just test for OPCODE (which will also match OPCODE.).  I meant to ask if 
> that was actually useful as they always matched the same thing either way.
> 
> I will do the updates you recommended.  Should I post the updated diffs 
> or is this enough to submit the patch?

Please repost.  Do the "dot" things in a separate patch please, if you
have those already anyway.


Segher

[PATCH, rs6000] Further vec-extract test fixes

[Jakub Jelinek]

On Sat, Feb 02, 2019 at 05:09:37PM -0600, Segher Boessenkool wrote:
> > 2019-02-01  Kelvin Nilsen  
> > 
> > * gcc.target/powerpc/vec-extract-slong-1.c: Require p8 execution
> > hardware.
> > * gcc.target/powerpc/vec-extract-schar-1.c: Likewise.
> > * gcc.target/powerpc/vec-extract-sint128-1.c: Likewise.
> > * gcc.target/powerpc/vec-extract-sshort-1.c: Likewise.
> > * gcc.target/powerpc/vec-extract-ulong-1.c: Likewise.
> > * gcc.target/powerpc/vec-extract-uchar-1.c: Likewise.
> > * gcc.target/powerpc/vec-extract-sint-1.c: Likewise.
> > * gcc.target/powerpc/vec-extract-uint128-1.c: Likewise.
> > * gcc.target/powerpc/vec-extract-ushort-1.c: Likewise.
> > * gcc.target/powerpc/vec-extract-uint-1.c: Likewise.

I don't see how the vec-extract-?int128-1.c tests could pass on big endian
power8 with -m32, __int128 type is only supported for 64-bit targets.

Ok for trunk?

2019-02-05  Jakub Jelinek  

* gcc.target/powerpc/vec-extract-sint128-1.c: Require int128 effective
target.
* gcc.target/powerpc/vec-extract-uint128-1.c: Likewise.

--- gcc/testsuite/gcc.target/powerpc/vec-extract-sint128-1.c.jj 2019-02-05 
16:34:42.955573498 +0100
+++ gcc/testsuite/gcc.target/powerpc/vec-extract-sint128-1.c2019-02-05 
16:54:11.18806 +0100
@@ -1,6 +1,6 @@
 /* Test to verify that the vec_extract from a vector of
signed __int128s remains signed.  */
-/* { dg-do run } */
+/* { dg-do run { target int128 } } */
 /* { dg-options "-ansi -mcpu=power8 " } */
 /* { dg-require-effective-target p8vector_hw } */
 /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { 
"-mcpu=power8" } } */
--- gcc/testsuite/gcc.target/powerpc/vec-extract-uint128-1.c.jj 2019-02-05 
16:34:42.957573464 +0100
+++ gcc/testsuite/gcc.target/powerpc/vec-extract-uint128-1.c2019-02-05 
16:54:23.503906762 +0100
@@ -1,6 +1,6 @@
 /* Test to verify that the vec_extract from a vector of
unsigned __int128s remains unsigned.  */
-/* { dg-do run } */
+/* { dg-do run { target int128 } } */
 /* { dg-options "-ansi -mcpu=power8 " } */
 /* { dg-require-effective-target p8vector_hw } */
 /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { 
"-mcpu=power8" } } */


Jakub

Re: [C++PATCH] [PR86379] do not use TREE_TYPE for USING_DECL_SCOPE

[Jason Merrill]

On Tue, Feb 5, 2019 at 1:37 AM Alexandre Oliva  wrote:
> On Jan 31, 2019, Jason Merrill  wrote:
>
> > Let's use strip_using_decl instead
>
> Aah, nice!  Thanks, I'll make the changes, test them, and post a new patch.
>
>
> >> @@ -13288,7 +13295,8 @@ grok_special_member_properties (tree decl)
> >> {
> >> tree class_type;
> >> -  if (!DECL_NONSTATIC_MEMBER_FUNCTION_P (decl))
> >> +  if (TREE_CODE (decl) != USING_DECL
> >> +  && !DECL_NONSTATIC_MEMBER_FUNCTION_P (decl))
> >> return;
>
> > Is there a reason not to use it here, as well?
>
> The using decl will take us to a member of a different class, and this
> function takes the DECL_CONTEXT of decl and adjusts the properties of
> that class.  If we followed USING_DECLs in decl that early, we'd adjust
> (again?) the member properties of USING_DECL_SCOPE(original using decl),
> rather than of DECL_CONTEXT (original using decl) as intended.

But a little further in, copy_fn_p will also check
DECL_NONSTATIC_MEMBER_FUNCTION_P and crash.

This function used to do nothing for USING_DECL (since its TREE_TYPE
wasn't METHOD_TYPE), right?  It should continue to do nothing.

Come to think of it, how about fixing DECL_NONSTATIC_MEMBER_FUNCTION_P
to be false for non-functions rather than messing with all these
places that use it?

Jason

Re: [C++ PATCH] Clear TREE_ADDRESSABLE on non-aggregate thunk arguments (PR c++/89187)

[Jason Merrill]

On 2/5/19 4:01 AM, Jakub Jelinek wrote:

Hi!

As the following testcase shows, for thunks the expansion code sometimes
requires that arguments with gimple reg type aren't addressable,
thunk needs to be simple passing of arguments directly to another call, not
having extra statements in between that.

In particular, for pass_by_reference arguments calls.c has if
call_from_thunk_p:
   /* We may have turned the parameter value into an SSA name.
  Go back to the original parameter so we can take the
  address.  */
   if (TREE_CODE (args[i].tree_value) == SSA_NAME)
 {
   gcc_assert (SSA_NAME_IS_DEFAULT_DEF (args[i].tree_value));
   args[i].tree_value = SSA_NAME_VAR (args[i].tree_value);
   gcc_assert (TREE_CODE (args[i].tree_value) == PARM_DECL);
 }
which is not the case if the PARM_DECL is TREE_ADDRESSABLE, then the def
stmt of the SSA_NAME is an assignment which loads the PARM_DECL value from
"memory".

TREE_ADDRESSABLE on the arguments is copied from the ctor from which it is
cloned (and which will be called by the thunk).  In this case the argument
needs to be addressable because vector[idx] folding takes address of vector.
In the thunk itself that doesn't matter though, it just passes the argument
to another function, so the following patch clears the TREE_ADDRESSABLE bit
there.

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

2019-02-05  Jakub Jelinek  

PR c++/89187
* optimize.c (maybe_thunk_body): Clear TREE_ADDRESSABLE on
non-aggregate PARM_DECLs of the thunk.

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

--- gcc/cp/optimize.c.jj2019-01-21 23:32:43.0 +0100
+++ gcc/cp/optimize.c   2019-02-04 16:40:21.354179933 +0100
@@ -417,6 +417,12 @@ maybe_thunk_body (tree fn, bool force)
  gcc_assert (clone_parm);
  DECL_ABSTRACT_ORIGIN (clone_parm) = NULL;
  args[parmno] = clone_parm;
+ /* Clear TREE_ADDRESSABLE on arguments with non-aggregate
+types, the thunk will not take addresses of those
+arguments, will just pass them through to another
+call.  */
+ if (!AGGREGATE_TYPE_P (TREE_TYPE (clone_parm)))
+   TREE_ADDRESSABLE (clone_parm) = 0;


We probably want to do this in maybe_add_lambda_conv_op, as well (in the 
loop over fn_args that sets DECL_CONTEXT).


I notice that use_thunk clears TREE_ADDRESSABLE unconditionally, is it 
important to handle aggregates differently here?


use_thunk also clears DECL_RTL and DECL_HAS_VALUE_EXPR_P, I don't know 
if that's important.


Jason

Re: [PATCH] Fix RTL DCE (PR target/89188)

[Richard Biener]

On February 5, 2019 4:32:29 PM GMT+01:00, Jakub Jelinek  
wrote:
>Hi!
>
>RTL DCE is called in different modes, sometimes it is allowed to alter
>cfg,
>in other cases it is not.  The following testcase ICEs because when it
>is
>not allowed to alter cfg (combiner's df_analyze) we remove a noop move
>that
>was considered to potentially throw and that made a bb unreachable
>(which
>the pass didn't like), but even removing those unreachable bbs would
>ICE
>anyway, as the algorithm is for !can_alter_cfg not really prepared to
>see
>bbs and edges being removed.
>
>So, the following patch does 3 things:
>1) doesn't delete noop moves if they can
>   throw and we can't alter cfg or delete dead exceptions
>2) instead of setting must_clean blindly for any CALL_INSNs, it instead
>   sets it accurately from delete_insn_and_edges return value
>3) it asserts that if !can_alter_cfg we don't actually have must_clean
>set
>
>Bootstrapped/regtested on powerpc64{,le}-linux, ok for trunk?

OK. 

Richard. 

>2019-02-05  Jakub Jelinek  
>
>   PR target/89188
>   * dce.c (delete_unmarked_insns): Don't remove no-op moves if they
>   can throw, non-call exceptions are enabled and we can't delete
>   dead exceptions or alter cfg.  Set must_clean if
>   delete_insn_and_edges returns true, don't set it blindly for calls.
>   Assert that delete_unreachable_blocks is called only if can_alter_cfg.
>
>   * g++.dg/opt/pr89188.C: New test.
>
>--- gcc/dce.c.jj   2019-02-05 10:04:16.984062429 +0100
>+++ gcc/dce.c  2019-02-05 10:53:47.655584503 +0100
>@@ -584,7 +584,12 @@ delete_unmarked_insns (void)
> rtx turn_into_use = NULL_RTX;
> 
> /* Always delete no-op moves.  */
>-if (noop_move_p (insn))
>+if (noop_move_p (insn)
>+/* Unless the no-op move can throw and we are not allowed
>+   to alter cfg.  */
>+&& (!cfun->can_throw_non_call_exceptions
>+|| (cfun->can_delete_dead_exceptions && can_alter_cfg)
>+|| insn_nothrow_p (insn)))
>   {
> if (RTX_FRAME_RELATED_P (insn))
>   turn_into_use
>@@ -627,12 +632,6 @@ delete_unmarked_insns (void)
>for the destination regs in order to avoid dangling notes.  */
> remove_reg_equal_equiv_notes_for_defs (insn);
> 
>-/* If a pure or const call is deleted, this may make the cfg
>-   have unreachable blocks.  We rememeber this and call
>-   delete_unreachable_blocks at the end.  */
>-if (CALL_P (insn))
>-  must_clean = true;
>-
> if (turn_into_use)
>   {
> /* Don't remove frame related noop moves if they cary
>@@ -645,12 +644,15 @@ delete_unmarked_insns (void)
>   }
> else
>   /* Now delete the insn.  */
>-  delete_insn_and_edges (insn);
>+  must_clean |= delete_insn_and_edges (insn);
>   }
> 
>   /* Deleted a pure or const call.  */
>   if (must_clean)
>-delete_unreachable_blocks ();
>+{
>+  gcc_assert (can_alter_cfg);
>+  delete_unreachable_blocks ();
>+}
> }
> 
> 
>--- gcc/testsuite/g++.dg/opt/pr89188.C.jj  2019-02-04 14:20:57.714851393
>+0100
>+++ gcc/testsuite/g++.dg/opt/pr89188.C 2019-02-04 14:52:09.988782531
>+0100
>@@ -0,0 +1,5 @@
>+// PR target/89188
>+// { dg-do compile { target c++11 } }
>+// { dg-options "-Og -flive-range-shrinkage -fnon-call-exceptions" }
>+
>+#include "../torture/pr88861.C"
>
>   Jakub

Re: [PATCH, rs6000] Further vec-extract test fixes

[Segher Boessenkool]

On Tue, Feb 05, 2019 at 05:04:03PM +0100, Jakub Jelinek wrote:
> On Sat, Feb 02, 2019 at 05:09:37PM -0600, Segher Boessenkool wrote:
> > > 2019-02-01  Kelvin Nilsen  
> > > 
> > >   * gcc.target/powerpc/vec-extract-slong-1.c: Require p8 execution
> > >   hardware.
> > >   * gcc.target/powerpc/vec-extract-schar-1.c: Likewise.
> > >   * gcc.target/powerpc/vec-extract-sint128-1.c: Likewise.
> > >   * gcc.target/powerpc/vec-extract-sshort-1.c: Likewise.
> > >   * gcc.target/powerpc/vec-extract-ulong-1.c: Likewise.
> > >   * gcc.target/powerpc/vec-extract-uchar-1.c: Likewise.
> > >   * gcc.target/powerpc/vec-extract-sint-1.c: Likewise.
> > >   * gcc.target/powerpc/vec-extract-uint128-1.c: Likewise.
> > >   * gcc.target/powerpc/vec-extract-ushort-1.c: Likewise.
> > >   * gcc.target/powerpc/vec-extract-uint-1.c: Likewise.
> 
> I don't see how the vec-extract-?int128-1.c tests could pass on big endian
> power8 with -m32, __int128 type is only supported for 64-bit targets.

This is fine, okay for trunk, thanks!


Segher


> 2019-02-05  Jakub Jelinek  
> 
>   * gcc.target/powerpc/vec-extract-sint128-1.c: Require int128 effective
>   target.
>   * gcc.target/powerpc/vec-extract-uint128-1.c: Likewise.

Re: C++ PATCH for c++/89158 - by-value capture of constexpr variable broken

[Jason Merrill]

On 2/4/19 3:48 PM, Marek Polacek wrote:

In this test we have a user-defined conversion converting const int & to
T, and we're binding a const int to const int & -- the parameter of the
converting ctor.  We call Func with "VIEW_CONVERT_EXPR(Val)"
as an argument.  I like to use a diagram for the conversion like this:

ck_rvalue   <-   ck_user   <-   ck_identity
TT  const int
 V_C_E(Val)

when processing the ck_user part we call __ct_comp, whose ICS for its
argument is

ck_ref_bind   <-   ck_identity
const int &   <-   const int
V_C_E(Val)

this ICS comes from reference_binding, and at that time we still had "Val"
so need_temporary_p is 0.  But when processing the ck_user conversion
before calling build_over_call for the __ct_comp, we call
  7009 expr = mark_rvalue_use (expr);
for the argument which turns the V_C_E to NON_LVALUE_EXPR<42>.  Binding
"42" to const int & is possible, but requires a temporary, which we don't
create precisely because need_temporary_p is 0.  I.e. the original expr
of the conversion changed behind reference_binding's back.

Now, the call to mark_rvalue_use was added in r159096 to handle
-Wunused-but-set-variable.  Since r261121, we're actually able to turn a V_C_E
into an rvalue.  But it seems we shouldn't be trying to turn the argument into
an rvalue in the ck_user case; mark_lvalue_use should do the job (= mark is as
used) without changing an lvalue into an rvalue, breaking the ICS as above.

I'm not sure if I'm describing the issue clearly enough, but I hope this
will do.


Yes, thanks, mark_rvalue_use is definitely wrong here.  But 
mark_lvalue_use might be wrong as well; we don't know here how the 
expression is used by the inner conversions for the user-defined 
conversion.  Can we remove the call entirely?  It doesn't seem to break 
any Wunused* tests.


Jason

Re: C++ PATCH for c++/89158 - by-value capture of constexpr variable broken

[Jakub Jelinek]

On Tue, Feb 05, 2019 at 11:24:14AM -0500, Jason Merrill wrote:
> Yes, thanks, mark_rvalue_use is definitely wrong here.  But mark_lvalue_use
> might be wrong as well; we don't know here how the expression is used by the
> inner conversions for the user-defined conversion.  Can we remove the call
> entirely?  It doesn't seem to break any Wunused* tests.

It was added in r159096 for -Wunused-but-set*.  It is surely possible it is
now covered by other mark_*_use calls.  Or could we call there just
mark_exp_read instead?

Jakub

Re: [C++ PATCH] Clear TREE_ADDRESSABLE on non-aggregate thunk arguments (PR c++/89187)

[Jakub Jelinek]

On Tue, Feb 05, 2019 at 11:16:04AM -0500, Jason Merrill wrote:
> > --- gcc/cp/optimize.c.jj2019-01-21 23:32:43.0 +0100
> > +++ gcc/cp/optimize.c   2019-02-04 16:40:21.354179933 +0100
> > @@ -417,6 +417,12 @@ maybe_thunk_body (tree fn, bool force)
> >   gcc_assert (clone_parm);
> >   DECL_ABSTRACT_ORIGIN (clone_parm) = NULL;
> >   args[parmno] = clone_parm;
> > + /* Clear TREE_ADDRESSABLE on arguments with non-aggregate
> > +types, the thunk will not take addresses of those
> > +arguments, will just pass them through to another
> > +call.  */
> > + if (!AGGREGATE_TYPE_P (TREE_TYPE (clone_parm)))
> > +   TREE_ADDRESSABLE (clone_parm) = 0;
> 
> We probably want to do this in maybe_add_lambda_conv_op, as well (in the
> loop over fn_args that sets DECL_CONTEXT).

Like below?

> I notice that use_thunk clears TREE_ADDRESSABLE unconditionally, is it
> important to handle aggregates differently here?

No, I was just trying to be too careful.  If use_thunk does that
unconditionally, I think we can as well.

> use_thunk also clears DECL_RTL and DECL_HAS_VALUE_EXPR_P, I don't know if
> that's important.

I can't imagine how DECL_RTL could be set (these days) and have no idea why
DECL_VALUE_EXPR would be used either.

I'll bootstrap/regtest following patch then:

2019-02-05  Jakub Jelinek  

PR c++/89187
* optimize.c (maybe_thunk_body): Clear TREE_ADDRESSABLE on
PARM_DECLs of the thunk.
* lambda.c (maybe_add_lambda_conv_op): Likewise.

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

--- gcc/cp/optimize.c.jj2019-02-05 10:04:17.089060672 +0100
+++ gcc/cp/optimize.c   2019-02-05 17:34:37.644762690 +0100
@@ -417,6 +417,8 @@ maybe_thunk_body (tree fn, bool force)
  gcc_assert (clone_parm);
  DECL_ABSTRACT_ORIGIN (clone_parm) = NULL;
  args[parmno] = clone_parm;
+ /* Clear TREE_ADDRESSABLE on thunk arguments.  */
+ TREE_ADDRESSABLE (clone_parm) = 0;
  clone_parm = TREE_CHAIN (clone_parm);
}
  if (fn_parm_typelist)
--- gcc/cp/lambda.c.jj  2019-02-02 11:07:01.217345765 +0100
+++ gcc/cp/lambda.c 2019-02-05 17:37:00.573387272 +0100
@@ -1130,6 +1130,9 @@ maybe_add_lambda_conv_op (tree type)
   {
tree new_node = copy_node (src);
 
+   /* Clear TREE_ADDRESSABLE on thunk arguments.  */
+   TREE_ADDRESSABLE (new_node) = 0;
+
if (!fn_args)
  fn_args = tgt = new_node;
else
--- gcc/testsuite/g++.dg/opt/pr89187.C.jj   2019-02-05 17:33:44.230650417 
+0100
+++ gcc/testsuite/g++.dg/opt/pr89187.C  2019-02-05 17:33:44.230650417 +0100
@@ -0,0 +1,23 @@
+// PR c++/89187
+// { dg-do compile { target c++11 } }
+// { dg-options "-Os -fno-tree-ccp -fno-tree-sra -fno-inline" }
+
+template  struct A {
+  typedef T __attribute__((vector_size (N))) type;
+};
+template  using B = typename A::type;
+template  using C = B;
+struct D {
+  D (C x) : d{x[3]} {}
+  D foo () { return d; }
+  C d;
+};
+extern D d;
+struct { D bar () { return d; } } l;
+struct E { void baz () const; };
+
+void
+E::baz () const
+{
+  l.bar ().foo ();
+}


Jakub

Re: Move -Wmaybe-uninitialized to -Wextra

[Jeff Law]

On 2/4/19 11:24 PM, Marc Glisse wrote:
> On Mon, 4 Feb 2019, Martin Jambor wrote:
> 
>>> Looking for "optional" and "-Wmaybe-uninitialized" shows
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78044
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635
>>>
>>> Google also gives
>>> https://www.boost.org/doc/libs/1_69_0/libs/optional/doc/html/boost_optional/tutorial/gotchas/false_positive_with__wmaybe_uninitialized.html
>>>
>>> https://sourceware.org/ml/gdb-patches/2017-05/msg00130.html
>>> etc
>>>
>>> And that's just for using a type called 'optional' (3 implementations of
>>> it).
>>
>> from my very quick reading of the first googled testcase, I assume the
>> instance of the optional class got SRAed and a warning was generated for
>> what originally was a class member, which indeed is not easy to
>> initialize on its own in order to avoid the warning.
> 
> Hmm, SRA and -Wmaybe-uninitialized, I saw that recently in
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66459
> 
>> Would it perhaps make sense to split the -Wmaybe-uninitialized warning
>> into two, one for scalars that are scalars in the original code and one
>> for SRA-created scalars and move only the latter to -Wextra?
> 
> If SRA is really the main source of false positives (I don't know about
> that), maybe. But I am afraid we will also miss a significant proportion
> of the already rare true positives that -Wmaybe-uninitialized currently
> finds. I really have no idea if such a split would work great or badly
> (sorry, I am not being very useful).
I've had some discussions with Kees and others in this space.  The
general consensus is that for pure scalars that everyone does a pretty
good job at generating good maybe-uninitialized warnings.  LLVM and GCC
take different approaches, both are valid and both have been useful at
giving developers actionable warnings.

However, the general consensus is that for aggregates or anything living
in memory is that most compilers aren't doing a particularly good job
across the board in this space.  Not enough objects are being thoroughly
analyzed and those that are analyzed give too many false positives, etc.
 On the Microsoft side, they've chosen to focus on improving DSE and
just initializing everything in memory (presumably flag controlled).
It's not clear where LLVM is going to go in this space.  ISTM that most
of the analysis to do a good job at DSE-ing away the redundant
initializer code should play directly into doing a good job at
maybe-uninit warnings for objects in memory.  However, I don't really
have the time to explore here.

I would generally support some experimentation in the overall space,
that might include allowing for different default settings for uninit
warnings of pure scalars vs aggregates/addressables.

FWIW, I've been doing Fedora rawhide builds with gcc-9 since early Jan.
 Not everything builds with -Wall -Werror, but lots of packages do.
I've only seen one maybe-uninit warning cause problems -- it was
spurious and for a memory object.  I didn't dig into it at all.

In contrast things like the missing NUL termination warnings, buffer
overflow warnings, NULL strings to *printf* warnings, etc all caught
numerous (dozens) of real bugs.  I mention it because it's one of the
ways I know packages are building with -Wall -Werror :-)

Jeff

[PATCH][AArch64] Use neon_dot_q type for 128-bit [US]DOT instructions where appropriate

[Kyrill Tkachov]

Hi all,

For the Dot Product instructions we have the scheduling types neon_dot and 
neon_dot_q for the 128-bit versions.
It seems that we're only using the former though, not assigning the neon_dot_q 
type anywhere.

This patch fixes that by adding the  mode attribute suffix to the type, 
similar to how we do it for other
types in aarch64-simd.md.

Bootstrapped and tested on aarch64-none-linux-gnu.

Ok for trunk?

Thanks,
Kyrill

2019-05-02  Kyrylo Tkachov  

* config/aarch64/aarch64-simd.md (aarch64_dot): Use 
neon_dot for type.
(aarch64_dot_lane): Likewise.
(aarch64_dot_laneq): Likewise.
diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
index 76e947e36f5b874cdd070e060d8988fec4875959..64a26ff923e3573a316a41742ea6b43f44475740 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -503,7 +503,7 @@ (define_insn "aarch64_dot"
 		DOTPROD)))]
   "TARGET_DOTPROD"
   "dot\\t%0., %2., %3."
-  [(set_attr "type" "neon_dot")]
+  [(set_attr "type" "neon_dot")]
 )
 
 ;; These expands map to the Dot Product optab the vectorizer checks for.
@@ -555,7 +555,7 @@ (define_insn "aarch64_dot_lane"
 operands[4] = aarch64_endian_lane_rtx (V8QImode, INTVAL (operands[4]));
 return "dot\\t%0., %2., %3.4b[%4]";
   }
-  [(set_attr "type" "neon_dot")]
+  [(set_attr "type" "neon_dot")]
 )
 
 (define_insn "aarch64_dot_laneq"
@@ -570,7 +570,7 @@ (define_insn "aarch64_dot_laneq"
 operands[4] = aarch64_endian_lane_rtx (V16QImode, INTVAL (operands[4]));
 return "dot\\t%0., %2., %3.4b[%4]";
   }
-  [(set_attr "type" "neon_dot")]
+  [(set_attr "type" "neon_dot")]
 )
 
 (define_expand "copysign3"

[PATCH] Fix pr84711.c testcase

[Segher Boessenkool]

On powerpc64-linux, this testcase complains the ABI for vector args
has changed, making the testcase fail (excess output).  This patch
shuts up that warning.

Committing as obvious.


Segher


2019-02-05  Segher Boessenkool  

* gcc.dg/vect/pr84711.c: Use -Wno-psabi.

---
 gcc/testsuite/gcc.dg/vect/pr84711.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.dg/vect/pr84711.c 
b/gcc/testsuite/gcc.dg/vect/pr84711.c
index b0f685d..12e9f60 100644
--- a/gcc/testsuite/gcc.dg/vect/pr84711.c
+++ b/gcc/testsuite/gcc.dg/vect/pr84711.c
@@ -1,6 +1,6 @@
 /* { dg-do compile } */
 /* { dg-require-effective-target vect_int } */
-/* { dg-options "-O2" } */
+/* { dg-options "-O2 -Wno-psabi" } */
 /* { dg-additional-options "-msse" { target i?86-*-* x86_64-*-* } } */
 
 typedef int v4si
-- 
1.8.3.1

New German PO file for 'cpplib' (version 9.1-b20190203)

[Translation Project Robot]

Hello, gentle maintainer.

This is a message from the Translation Project robot.

A revised PO file for textual domain 'cpplib' has been submitted
by the German team of translators.  The file is available at:

https://translationproject.org/latest/cpplib/de.po

(This file, 'cpplib-9.1-b20190203.de.po', has just now been sent to you in
a separate email.)

All other PO files for your package are available in:

https://translationproject.org/latest/cpplib/

Please consider including all of these in your next release, whether
official or a pretest.

Whenever you have a new distribution with a new version number ready,
containing a newer POT file, please send the URL of that distribution
tarball to the address below.  The tarball may be just a pretest or a
snapshot, it does not even have to compile.  It is just used by the
translators when they need some extra translation context.

The following HTML page has been updated:

https://translationproject.org/domain/cpplib.html

If any question arises, please contact the translation coordinator.

Thank you for all your work,

The Translation Project robot, in the
name of your translation coordinator.

Contents of PO file 'cpplib-9.1-b20190203.de.po'

[Translation Project Robot]

cpplib-9.1-b20190203.de.po.gz
Description: Binary data
The Translation Project robot, in the
name of your translation coordinator.

New template for 'cpplib' made available

[Translation Project Robot]

Hello, gentle maintainer.

This is a message from the Translation Project robot.  (If you have
any questions, send them to .)

A new POT file for textual domain 'cpplib' has been made available
to the language teams for translation.  It is archived as:

https://translationproject.org/POT-files/cpplib-9.1-b20190203.pot

Whenever you have a new distribution with a new version number ready,
containing a newer POT file, please send the URL of that distribution
tarball to the address below.  The tarball may be just a pretest or a
snapshot, it does not even have to compile.  It is just used by the
translators when they need some extra translation context.

Below is the URL which has been provided to the translators of your
package.  Please inform the translation coordinator, at the address
at the bottom, if this information is not current:

https://gcc.gnu.org/pub/gcc/snapshots/9-20190203/gcc-9-20190203.tar.xz

Translated PO files will later be automatically e-mailed to you.

Thank you for all your work,

The Translation Project robot, in the
name of your translation coordinator.

New Ukrainian PO file for 'cpplib' (version 9.1-b20190203)

[Translation Project Robot]

Hello, gentle maintainer.

This is a message from the Translation Project robot.

A revised PO file for textual domain 'cpplib' has been submitted
by the Ukrainian team of translators.  The file is available at:

https://translationproject.org/latest/cpplib/uk.po

(This file, 'cpplib-9.1-b20190203.uk.po', has just now been sent to you in
a separate email.)

All other PO files for your package are available in:

https://translationproject.org/latest/cpplib/

Please consider including all of these in your next release, whether
official or a pretest.

Whenever you have a new distribution with a new version number ready,
containing a newer POT file, please send the URL of that distribution
tarball to the address below.  The tarball may be just a pretest or a
snapshot, it does not even have to compile.  It is just used by the
translators when they need some extra translation context.

The following HTML page has been updated:

https://translationproject.org/domain/cpplib.html

If any question arises, please contact the translation coordinator.

Thank you for all your work,

The Translation Project robot, in the
name of your translation coordinator.

Contents of PO file 'cpplib-9.1-b20190203.uk.po'

[Translation Project Robot]

cpplib-9.1-b20190203.uk.po.gz
Description: Binary data
The Translation Project robot, in the
name of your translation coordinator.

Re: [C++ PATCH] Clear TREE_ADDRESSABLE on non-aggregate thunk arguments (PR c++/89187)

[Jason Merrill]

On 2/5/19 11:40 AM, Jakub Jelinek wrote:

On Tue, Feb 05, 2019 at 11:16:04AM -0500, Jason Merrill wrote:

--- gcc/cp/optimize.c.jj2019-01-21 23:32:43.0 +0100
+++ gcc/cp/optimize.c   2019-02-04 16:40:21.354179933 +0100
@@ -417,6 +417,12 @@ maybe_thunk_body (tree fn, bool force)
  gcc_assert (clone_parm);
  DECL_ABSTRACT_ORIGIN (clone_parm) = NULL;
  args[parmno] = clone_parm;
+ /* Clear TREE_ADDRESSABLE on arguments with non-aggregate
+types, the thunk will not take addresses of those
+arguments, will just pass them through to another
+call.  */
+ if (!AGGREGATE_TYPE_P (TREE_TYPE (clone_parm)))
+   TREE_ADDRESSABLE (clone_parm) = 0;


We probably want to do this in maybe_add_lambda_conv_op, as well (in the
loop over fn_args that sets DECL_CONTEXT).


Like below?


I notice that use_thunk clears TREE_ADDRESSABLE unconditionally, is it
important to handle aggregates differently here?


No, I was just trying to be too careful.  If use_thunk does that
unconditionally, I think we can as well.


use_thunk also clears DECL_RTL and DECL_HAS_VALUE_EXPR_P, I don't know if
that's important.


I can't imagine how DECL_RTL could be set (these days) and have no idea why
DECL_VALUE_EXPR would be used either.

I'll bootstrap/regtest following patch then:

2019-02-05  Jakub Jelinek  

PR c++/89187
* optimize.c (maybe_thunk_body): Clear TREE_ADDRESSABLE on
PARM_DECLs of the thunk.
* lambda.c (maybe_add_lambda_conv_op): Likewise.


OK.

Jason

[Committed] S/390: Remove load and test fp splitter

[Andreas Krebbel]

Splitters are not allowed to use data flow routines depending on
REG_DEAD notes since these are not recomputed by the split pass and
might therefore be outdated.  The splitter we have for load and test
FP turns a potentially stale REG_DEAD note into a clobber of that
register.  This then leads to wrong code being generated.  This patch
just removes that splitter until I can come up with a better solution.

Regression tested on GCC 8 and mainline.

The 2 load and test FP testcases fail now due to that change:
load-and-test-fp-1.c and load-and-test-fp-2.c

I'll leave it that way to remind me to fix that.

Committed to mainline, GCC 8, and GCC 7.

Andreas

gcc/ChangeLog:

2019-02-05  Andreas Krebbel  

PR target/88856
* config/s390/s390.md: Remove load and test FP splitter.
---
 gcc/config/s390/s390.md | 21 +++--
 1 file changed, 3 insertions(+), 18 deletions(-)

diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md
index 722d924..9033672 100644
--- a/gcc/config/s390/s390.md
+++ b/gcc/config/s390/s390.md
@@ -1357,10 +1357,11 @@
 ; (TF|DF|SF|TD|DD|SD) instructions
 
 
-; load and test instructions turn SNaN into QNaN what is not
+; FIXME: load and test instructions turn SNaN into QNaN what is not
 ; acceptable if the target will be used afterwards.  On the other hand
 ; they are quite convenient for implementing comparisons with 0.0. So
-; try to enable them via splitter if the value isn't needed anymore.
+; try to enable them via splitter/peephole if the value isn't needed anymore.
+; See testcases: load-and-test-fp-1.c and load-and-test-fp-2.c
 
 ; ltxbr, ltdbr, ltebr, ltxtr, ltdtr
 (define_insn "*cmp_ccs_0"
@@ -1373,22 +1374,6 @@
[(set_attr "op_type" "RRE")
 (set_attr "type"  "fsimp")])
 
-(define_split
-  [(set (match_operand 0 "cc_reg_operand")
-   (compare (match_operand:FP 1 "register_operand")
-(match_operand:FP 2 "const0_operand")))]
-  "TARGET_HARD_FLOAT && REG_P (operands[1]) && dead_or_set_p (insn, 
operands[1])"
-  [(parallel
-[(set (match_dup 0) (match_dup 3))
- (clobber (match_dup 1))])]
- {
-   /* s390_match_ccmode requires the compare to have the same CC mode
-  as the CC destination register.  */
-   operands[3] = gen_rtx_COMPARE (GET_MODE (operands[0]),
- operands[1], operands[2]);
- })
-
-
 ; VX: TFmode in FPR pairs: use cxbr instead of wfcxb
 ; cxtr, cdtr, cxbr, cdbr, cebr, cdb, ceb, wfcsb, wfcdb
 (define_insn "*cmp_ccs"
-- 
2.7.4

Go patch committed: Check duplicate implicit indexes in slices/arrays

[Ian Lance Taylor]

This patch by Ben Shi fixes the Go frontend to check duplicate
implicit indexes in slices/array composite literals.  This fixes
https://golang.org/issue/28186.  Bootstrapped and ran Go testsuite on
x86_64-pc-linux-gnu.  Committed to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 268465)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-047b0aa6a29d46fde99b3e5823339ac8866f797c
+347628daf153baf3034b61b2abb4ec39e2ab37c8
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/expressions.cc
===
--- gcc/go/gofrontend/expressions.cc(revision 268369)
+++ gcc/go/gofrontend/expressions.cc(working copy)
@@ -14244,6 +14244,13 @@ Composite_literal_expression::lower_arra
 
   if (index_expr == NULL)
{
+ if (std::find(indexes->begin(), indexes->end(), index)
+ != indexes->end())
+   {
+ go_error_at(val->location(),
+ "duplicate value for index %lu", index);
+ return Expression::make_error(location);
+   }
  if (!indexes->empty())
indexes->push_back(index);
}

Re: C++ PATCH for c++/89158 - by-value capture of constexpr variable broken

[Jason Merrill]

On 2/5/19 11:31 AM, Jakub Jelinek wrote:

On Tue, Feb 05, 2019 at 11:24:14AM -0500, Jason Merrill wrote:

Yes, thanks, mark_rvalue_use is definitely wrong here.  But mark_lvalue_use
might be wrong as well; we don't know here how the expression is used by the
inner conversions for the user-defined conversion.  Can we remove the call
entirely?  It doesn't seem to break any Wunused* tests.


It was added in r159096 for -Wunused-but-set*.  It is surely possible it is
now covered by other mark_*_use calls.  Or could we call there just
mark_exp_read instead?


That would be fine too.

Jason

New template for 'gcc' made available

[Translation Project Robot]

Hello, gentle maintainer.

This is a message from the Translation Project robot.  (If you have
any questions, send them to .)

A new POT file for textual domain 'gcc' has been made available
to the language teams for translation.  It is archived as:

https://translationproject.org/POT-files/gcc-9.1-b20190203.pot

Whenever you have a new distribution with a new version number ready,
containing a newer POT file, please send the URL of that distribution
tarball to the address below.  The tarball may be just a pretest or a
snapshot, it does not even have to compile.  It is just used by the
translators when they need some extra translation context.

Below is the URL which has been provided to the translators of your
package.  Please inform the translation coordinator, at the address
at the bottom, if this information is not current:

https://gcc.gnu.org/pub/gcc/snapshots/9-20190203/gcc-9-20190203.tar.xz

Translated PO files will later be automatically e-mailed to you.

Thank you for all your work,

The Translation Project robot, in the
name of your translation coordinator.

Re: Late-breaking jit features (was Re: [PATCH][gcc] libgccjit: introduce gcc_jit_context_add_driver_option)

[David Malcolm]

On Sat, 2019-02-02 at 16:34 +0100, Jakub Jelinek wrote:
> On Sat, Feb 02, 2019 at 10:18:43AM -0500, David Malcolm wrote:
> > > > Alternatively, should these patches go into a branch of queued
> > > > jit
> > > > changes for gcc 10?
> > > 
> > > Is there anything like an ABI involved? If so we should avoid
> > > breaking it all the time. Otherwise JIT is not release critical
> > > and
> > > thus if you break it in the wrong moment it's your own fault. 
> > 
> > The two patches each add a new API entrypoint, but libgccjit uses
> > symbol-versioning to extend the ABI, without bumping the SONAME:
> >   https://gcc.gnu.org/onlinedocs/jit/topics/compatibility.html
> > So it's not an ABI break as such.
> 
> I'd say it depends on how quickly the copyright paperwork can be
> done, the
> patch can't be added until that is resolved.  While gccjit is not
> release
> critical, it would be nice not to break it late, so say if it can be
> committed by end of February/mid March, I guess it is fine, given the
> assumption we'd like to release mid April to end of April, if it
> can't be
> done by then, might be better to postpone to GCC 10.
> 
>   Jakub

Jakub and Richard: thanks.

I've double-checked the gcc_jit_context_add_driver_option patch and it
looks good (it's a different patch that we're waiting on paperwork
for).

Andrea: are you able to commit this, or should I do this on your
behalf?

Dave

Re: [PATCH] print correct array sizes in errors (PR 87996)

[Martin Sebor]

On 2/1/19 7:41 AM, Jason Merrill wrote:

On 1/31/19 5:49 PM, Martin Sebor wrote:

On 1/30/19 3:15 PM, Jason Merrill wrote:

On 1/29/19 7:15 PM, Martin Sebor wrote:

+  /* Try to convert the original SIZE to a ssizetype.  */
+  if (orig_size != error_mark_node
+  && !TYPE_UNSIGNED (TREE_TYPE (orig_size)))
+    {
+  if (TREE_CODE (size) == INTEGER_CST
+  && tree_int_cst_sign_bit (size))
+    diagsize = build_converted_constant_expr (ssizetype, size,
+  tsubst_flags_t ());
+  else if (size == error_mark_node
+   && TREE_CODE (orig_size) == INTEGER_CST
+   && tree_int_cst_sign_bit (orig_size))
+    diagsize = build_converted_constant_expr (ssizetype, 
orig_size,

+  tsubst_flags_t ());
+    }


Using build_converted_constant_expr here looks odd; that's a 
language-level notion, and we're dealing with compiler internals. 
fold_convert seems more appropriate.


Done.




+  if (TREE_CONSTANT (size))
+    {
+  if (!diagsize && TREE_CODE (size) == INTEGER_CST)
+    diagsize = size;
+    }
+  else
 size = osize;
 }

@@ -9732,15 +9758,12 @@ compute_array_index_type_loc (location_t 
name_loc,

   if (TREE_CODE (size) == INTEGER_CST)
 {
   /* An array must have a positive number of elements.  */
-  if (!valid_constant_size_p (size))
+  if (!diagsize)
+    diagsize = size;


It seems like the earlier hunk here is unnecessary; if size is an 
INTEGER_CST, it will be unchanged, and so be used for diagsize in the 
latter hunk without any changes to the earlier location.  Actually, 
why not do all of the diagsize logic down here?  There doesn't seem 
to be anything above that relies on information we will have lost at 
this point.


Sure.  Done in the attached revision.



-  tree osize = size;
+  /* The original numeric size as seen in the source code after
+ any substitution and before conversion to size_t.  */
+  tree origsize = NULL_TREE;


Can't you use osize?  instantiate_non_dependent_expr doesn't do any 
actual substitution, it shouldn't change the type of the expression or 
affect whether it's an INTEGER_CST.


I went ahead and reused osize but kept the new name origsize (I assume
avoiding introducing a new variable is what you meant).  The longer
name is more descriptive and also has a comment explaining what it's
for (which is why I didn't touch osize initially -- I didn't know
enough about what it was for or how it might change).

Martin
PR c++/87996 - size of array is negative error when SIZE_MAX/2 < sizeof(array) <= SIZE_MAX

gcc/ChangeLog:

	PR c++/87996
	* builtins.c (max_object_size): Move from here...
	* builtins.h (max_object_size): ...and here...
	* tree.c (max_object_size): ...to here...
	* tree.h (max_object_size): ...and here.

gcc/c-family/ChangeLog:

	PR c++/87996
	* c-common.c (invalid_array_size_error): New function.
	(valid_array_size_p): Call it.  Handle size as well as type.
	* c-common.h (valid_constant_size_p): New function.
	(enum cst_size_error): New type.

gcc/cp/ChangeLog:

	PR c++/87996
	* decl.c (compute_array_index_type_loc): Preserve signed sizes
	for diagnostics.  Call valid_array_size_p instead of error.
	* init.c (build_new_1): Compute size for diagnostic.  Call
	invalid_array_size_error
	(build_new): Call valid_array_size_p instead of error.

gcc/testsuite/ChangeLog:

	PR c++/87996
	* c-c++-common/array-5.c: New test.
	* c-c++-common/pr68107.c: Adjust text of diagnostics.
	* g++.dg/init/new38.C: Same.
	* g++.dg/init/new43.C: Same.
	* g++.dg/init/new44.C: Same.
	* g++.dg/init/new46.C: Same.
	* g++.dg/other/large-size-array.C: Same.
	* g++.dg/other/new-size-type.C: Same.
	* g++.dg/template/array30.C: Same.
	* g++.dg/template/array32.C: New test.
	* g++.dg/template/dependent-name3.C: Adjust.
	* gcc.dg/large-size-array-3.c: Same.
	* gcc.dg/large-size-array-5.c: Same.
	* gcc.dg/large-size-array.c: Same.
	* g++.old-deja/g++.brendan/array1.C: Same.
	* g++.old-deja/g++.mike/p6149.C: Same.

Index: gcc/builtins.c
===
--- gcc/builtins.c	(revision 268547)
+++ gcc/builtins.c	(working copy)
@@ -11210,12 +11210,3 @@ target_char_cst_p (tree t, char *p)
   *p = (char)tree_to_uhwi (t);
   return true;
 }
-
-/* Return the maximum object size.  */
-
-tree
-max_object_size (void)
-{
-  /* To do: Make this a configurable parameter.  */
-  return TYPE_MAX_VALUE (ptrdiff_type_node);
-}
Index: gcc/builtins.h
===
--- gcc/builtins.h	(revision 268547)
+++ gcc/builtins.h	(working copy)
@@ -150,6 +150,5 @@ extern internal_fn replacement_internal_fn (gcall
 
 extern void warn_string_no_nul (location_t, const char *, tree, tree);
 extern tree unterminated_array (tree, tree * = NULL, bool * = NULL);
-extern tree max_object_size ();
 
 #endif /* GCC_BUILTINS_H */
Index: gcc/c-family/c-common.c
=

New Finnish PO file for 'cpplib' (version 9.1-b20190203)

[Translation Project Robot]

Hello, gentle maintainer.

This is a message from the Translation Project robot.

A revised PO file for textual domain 'cpplib' has been submitted
by the Finnish team of translators.  The file is available at:

https://translationproject.org/latest/cpplib/fi.po

(This file, 'cpplib-9.1-b20190203.fi.po', has just now been sent to you in
a separate email.)

All other PO files for your package are available in:

https://translationproject.org/latest/cpplib/

Please consider including all of these in your next release, whether
official or a pretest.

Whenever you have a new distribution with a new version number ready,
containing a newer POT file, please send the URL of that distribution
tarball to the address below.  The tarball may be just a pretest or a
snapshot, it does not even have to compile.  It is just used by the
translators when they need some extra translation context.

The following HTML page has been updated:

https://translationproject.org/domain/cpplib.html

If any question arises, please contact the translation coordinator.

Thank you for all your work,

The Translation Project robot, in the
name of your translation coordinator.

Contents of PO file 'cpplib-9.1-b20190203.fi.po'

[Translation Project Robot]

cpplib-9.1-b20190203.fi.po.gz
Description: Binary data
The Translation Project robot, in the
name of your translation coordinator.

Re: [PATCH] Fix an ICE when compiling Go with LTO (PR go/89019)

[Nikhil Benesch]

Ian—is there anything preventing this from getting merged? (I don't have
write access.)

On Thu, Jan 24, 2019 at 11:31 AM Nikhil Benesch 
wrote:

> On Thu, Jan 24, 2019 at 10:15 AM Richard Biener
>  wrote:
> >
> > Ah, I missed that pt is probably a pointer type as well, then the code
> > simply aligns the pointed-to type (note dependent on how pt was built
> > this seems prone to wreck the TYPE_POINTER_TO/TYPE_NEXT_PTR_TO
> > chains which link pointer types to a type.
>
> Right. In fact, set_placeholder_pointer_type asserts that both pt and tt
> are pointer types.
>
> It's true that, after a call to set_placeholder_pointer_type, pt becomes
> a distinct pointer type to T, yet is not part of the
> TYPE_POINTER_TO/TYPE_NEXT_PTR_TO chain for T. As far as I can tell,
> that's fine. The chain for T remains intact, as placeholder pointer
> types are careful to always point to a distinct dummy object (see the
> Gcc_backend::placeholder_pointer_type method) that nothing cares about.
> The only consequence I see is the increased memory usage of having
> multiple distinct but semantically identical pointer types to T.
>

Re: [PR86218] handle ck_aggr in compare_ics in both and either conversion

[Jason Merrill]

On 1/30/19 10:56 AM, Alexandre Oliva wrote:

Because of rank compares, and checks for ck_list, we know that if we
see user_conv_p or ck_list in ics1, we'll also see it in ics2.  This
reasoning does not extend to ck_aggr, however, so we might have
ck_aggr conversions starting both ics1 and ics2, which we handle
correctly, or either, which we likely handle by crashing on whatever
path we take depending on whether ck_aggr is in ics1 or ics2.

We crash because, as we search the conversion sequences, we may very
well fail to find what we are looking for, and reach the end of the
sequence, which is unexpected in all paths.

This patch arranges for us to take the same path when ck_aggr is in
ics2 only that we would if it was in ics1 (regardless of ics2), and it
deals with not finding the kind of conversion we look for there.

I've changed the type of the literal constant in the testcase, so as
to hopefully make it well-formed.  We'd fail to reject the narrowing
conversion in the original testcase, but that's a separate bug.

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


for  gcc/cp/ChangeLog

PR c++/86218
* call.c (compare_ics): Deal with ck_aggr in either cs.


OK.

Jason

Re: [PATCH] print correct array sizes in errors (PR 87996)

[Jason Merrill]

On 2/5/19 1:46 PM, Martin Sebor wrote:

On 2/1/19 7:41 AM, Jason Merrill wrote:

On 1/31/19 5:49 PM, Martin Sebor wrote:

On 1/30/19 3:15 PM, Jason Merrill wrote:

On 1/29/19 7:15 PM, Martin Sebor wrote:

+  /* Try to convert the original SIZE to a ssizetype.  */
+  if (orig_size != error_mark_node
+  && !TYPE_UNSIGNED (TREE_TYPE (orig_size)))
+    {
+  if (TREE_CODE (size) == INTEGER_CST
+  && tree_int_cst_sign_bit (size))
+    diagsize = build_converted_constant_expr (ssizetype, size,
+  tsubst_flags_t ());
+  else if (size == error_mark_node
+   && TREE_CODE (orig_size) == INTEGER_CST
+   && tree_int_cst_sign_bit (orig_size))
+    diagsize = build_converted_constant_expr (ssizetype, 
orig_size,

+  tsubst_flags_t ());
+    }


Using build_converted_constant_expr here looks odd; that's a 
language-level notion, and we're dealing with compiler internals. 
fold_convert seems more appropriate.


Done.




+  if (TREE_CONSTANT (size))
+    {
+  if (!diagsize && TREE_CODE (size) == INTEGER_CST)
+    diagsize = size;
+    }
+  else
 size = osize;
 }

@@ -9732,15 +9758,12 @@ compute_array_index_type_loc (location_t 
name_loc,

   if (TREE_CODE (size) == INTEGER_CST)
 {
   /* An array must have a positive number of elements.  */
-  if (!valid_constant_size_p (size))
+  if (!diagsize)
+    diagsize = size;


It seems like the earlier hunk here is unnecessary; if size is an 
INTEGER_CST, it will be unchanged, and so be used for diagsize in 
the latter hunk without any changes to the earlier location.  
Actually, why not do all of the diagsize logic down here?  There 
doesn't seem to be anything above that relies on information we will 
have lost at this point.


Sure.  Done in the attached revision.



-  tree osize = size;
+  /* The original numeric size as seen in the source code after
+ any substitution and before conversion to size_t.  */
+  tree origsize = NULL_TREE;


Can't you use osize?  instantiate_non_dependent_expr doesn't do any 
actual substitution, it shouldn't change the type of the expression or 
affect whether it's an INTEGER_CST.


I went ahead and reused osize but kept the new name origsize (I assume
avoiding introducing a new variable is what you meant).  The longer
name is more descriptive and also has a comment explaining what it's
for (which is why I didn't touch osize initially -- I didn't know
enough about what it was for or how it might change).


Yep, it was saving the original "size" before conversions.  I guess the 
name change is OK, but please restore the initialization from "size", 
and drop the added comment on the call to mark_rvalue_use (which is to 
possibly replace a lambda capture with its constant value).


How about passing origsize into valid_array_size_p rather than than 
always computing diagsize in the caller?


Jason

Contents of PO file 'cpplib-9.1-b20190203.fr.po'

[Translation Project Robot]

cpplib-9.1-b20190203.fr.po.gz
Description: Binary data
The Translation Project robot, in the
name of your translation coordinator.

New French PO file for 'cpplib' (version 9.1-b20190203)

[Translation Project Robot]

Hello, gentle maintainer.

This is a message from the Translation Project robot.

A revised PO file for textual domain 'cpplib' has been submitted
by the French team of translators.  The file is available at:

https://translationproject.org/latest/cpplib/fr.po

(This file, 'cpplib-9.1-b20190203.fr.po', has just now been sent to you in
a separate email.)

All other PO files for your package are available in:

https://translationproject.org/latest/cpplib/

Please consider including all of these in your next release, whether
official or a pretest.

Whenever you have a new distribution with a new version number ready,
containing a newer POT file, please send the URL of that distribution
tarball to the address below.  The tarball may be just a pretest or a
snapshot, it does not even have to compile.  It is just used by the
translators when they need some extra translation context.

The following HTML page has been updated:

https://translationproject.org/domain/cpplib.html

If any question arises, please contact the translation coordinator.

Thank you for all your work,

The Translation Project robot, in the
name of your translation coordinator.

New Finnish PO file for 'gcc' (version 9.1-b20190203)

[Translation Project Robot]

Hello, gentle maintainer.

This is a message from the Translation Project robot.

A revised PO file for textual domain 'gcc' has been submitted
by the Finnish team of translators.  The file is available at:

https://translationproject.org/latest/gcc/fi.po

(This file, 'gcc-9.1-b20190203.fi.po', has just now been sent to you in
a separate email.)

All other PO files for your package are available in:

https://translationproject.org/latest/gcc/

Please consider including all of these in your next release, whether
official or a pretest.

Whenever you have a new distribution with a new version number ready,
containing a newer POT file, please send the URL of that distribution
tarball to the address below.  The tarball may be just a pretest or a
snapshot, it does not even have to compile.  It is just used by the
translators when they need some extra translation context.

The following HTML page has been updated:

https://translationproject.org/domain/gcc.html

If any question arises, please contact the translation coordinator.

Thank you for all your work,

The Translation Project robot, in the
name of your translation coordinator.

[PATCH, rs6000] Fix instruction counts on powerpc64 test cases. (take 2)

[Bill Seurer]

[PATCH, rs6000] Fix instruction counts on powerpc64 test cases.

This patch fixes the assembler instruction counts for some test cases
that started failing due to changes in code generation.  The targets
were adjusted a bit as well to avoid generating BE/LE endian code on
unsupported platforms.

Bootstrapped and tested on powerpc64le-unknown-linux-gnu (power 8 and
power 9) and powerpc64be-unknown-linux-gnu (power 7 and power 8) with
no regressions.  Is this ok for trunk?


2019-02-04  Bill Seurer  

* gcc/testsuite/gcc.target/powerpc/vsx-vector-6.p7.c: Update 
instruction counts and target.
* gcc/testsuite/gcc.target/powerpc/vsx-vector-6.p8.c: Update 
instruction counts and target.
* gcc/testsuite/gcc.target/powerpc/vsx-vector-6.p9.c: Update 
instruction counts and target.

Index: gcc/testsuite/gcc.target/powerpc/vsx-vector-6.p7.c
===
--- gcc/testsuite/gcc.target/powerpc/vsx-vector-6.p7.c  (revision 268524)
+++ gcc/testsuite/gcc.target/powerpc/vsx-vector-6.p7.c  (working copy)
@@ -1,28 +1,20 @@
-/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
+/* { dg-do compile { target { lp64 && be } } } */
 /* { dg-skip-if "" { powerpc*-*-darwin* } } */
 /* { dg-require-effective-target powerpc_vsx_ok } */
 /* { dg-options "-mvsx -O2 -mcpu=power7 -dp" } */
 /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { 
"-mcpu=power7" } } */
 
-
 /* Expected instruction counts for Power 7 */
 
 /* { dg-final { scan-assembler-times "xvabsdp" 1 } } */
 /* { dg-final { scan-assembler-times "xvadddp" 1 } } */
-/* { dg-final { scan-assembler-times "xxlnor" 8 { target le } } } */
-/* { dg-final { scan-assembler-times "xxlnor" 7 { target be } } } */
-/* { dg-final { scan-assembler-times "xvcmpeqdp" 5 { target le } } } */
-/* { dg-final { scan-assembler-times "xvcmpeqdp" 6 { target be }} } */
-/* { dg-final { scan-assembler-times "xvcmpeqdp." 5 { target le } } } */
-/* { dg-final { scan-assembler-times "xvcmpeqdp." 6 { target be } } } */
-/* { dg-final { scan-assembler-times "xvcmpgtdp" 9 { target le } } } */
-/* { dg-final { scan-assembler-times "xvcmpgtdp" 8 { target be } } } */
-/* { dg-final { scan-assembler-times "xvcmpgtdp." 9 { target le } } } */
-/* { dg-final { scan-assembler-times "xvcmpgtdp." 8 { target be } } } */
-/* { dg-final { scan-assembler-times "xvcmpgedp" 6 { target le } } } */
-/* { dg-final { scan-assembler-times "xvcmpgedp" 7 { target be } } } */
-/* { dg-final { scan-assembler-times "xvcmpgedp." 6 { target le } } } */
-/* { dg-final { scan-assembler-times "xvcmpgedp." 7 { target be } } } */
+/* { dg-final { scan-assembler-times "xxlnor" 5 } } */
+/* { dg-final { scan-assembler-times {\mxvcmpeqdp\s} 1 } } */
+/* { dg-final { scan-assembler-times {\mxvcmpeqdp\.\s} 5 } } */
+/* { dg-final { scan-assembler-times {\mxvcmpgtdp\s} 2 } } */
+/* { dg-final { scan-assembler-times {\mxvcmpgtdp\.\s} 5 } } */
+/* { dg-final { scan-assembler-times {\mxvcmpgedp\s} 1 } } */
+/* { dg-final { scan-assembler-times {\mxvcmpgedp\.\s} 6 } } */
 /* { dg-final { scan-assembler-times "xvrdpim" 1 } } */
 /* { dg-final { scan-assembler-times "xvmaddadp" 1 } } */
 /* { dg-final { scan-assembler-times "xvmsubadp" 1 } } */
Index: gcc/testsuite/gcc.target/powerpc/vsx-vector-6.p8.c
===
--- gcc/testsuite/gcc.target/powerpc/vsx-vector-6.p8.c  (revision 268524)
+++ gcc/testsuite/gcc.target/powerpc/vsx-vector-6.p8.c  (working copy)
@@ -1,16 +1,15 @@
-/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
+/* { dg-do compile { target { lp64 } } } */
 /* { dg-skip-if "" { powerpc*-*-darwin* } } */
 /* { dg-require-effective-target powerpc_vsx_ok } */
 /* { dg-options "-mvsx -O2 -mcpu=power8" } */
 /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { 
"-mcpu=power8" } } */
 
-
 /* Expected instruction counts for Power 8.  */
 
 /* { dg-final { scan-assembler-times "xvabsdp" 1 } } */
 /* { dg-final { scan-assembler-times "xvadddp" 1 } } */
-/* { dg-final { scan-assembler-times "xxlnor" 8 { target le } } } */
-/* { dg-final { scan-assembler-times "xxlnor" 7 { target be } } } */
+/* { dg-final { scan-assembler-times "xxlnor" 6 { target le } } } */
+/* { dg-final { scan-assembler-times "xxlnor" 5 { target be } } } */
 
 /* We generate xxlor instructions for many reasons other than or'ing vector
operands or calling __builtin_vec_or(), which  means we cannot rely on
@@ -18,16 +17,16 @@
xxlor instruction was generated.  */
 /* { dg-final { scan-assembler "xxlor" } } */
 
-/* { dg-final { scan-assembler-times "xvcmpeqdp" 4 { target le } } } */
-/* { dg-final { scan-assembler-times "xvcmpeqdp" 6 { target be } } } */
-/* { dg-final { scan-assembler-times "xvcmpeqdp." 4 { target le } } } */
-/* { dg-final { scan-assembler-times "xvcmpeqdp." 6 { target be } } } */
-/* { dg-final { scan-assembler-times "xvcmpgtdp" 7 { target le 

Re: [PATCH] Fix PR89150, GC of tree-form bitmaps

[Jeff Law]

On 2/4/19 9:07 AM, Jeff Law wrote:
> On 2/4/19 6:15 AM, Richard Biener wrote:
>>
>> When I introduced tree-form bitmaps I forgot to think about GC.
>> The following drops the chain_prev annotation to make the marker
>> work for trees.  I've also maked the obstack member GTY skip
>> (and prevent bitmap_obstack from gengtype processing) because
>> the obstack isn't used for GC allocated bitmaps.
>>
>> Bootstrap & regtest running on x86_64-unknown-linux-gnu.
>>
>> Richard.
>>
>> 2019-02-04  Richard Biener  
>>
>>  PR middle-end/89150
>>  * bitmap.h (struct bitmap_obstack): Do not mark GTY.
>>  (struct bitmap_element): Drop chain_prev so we properly recurse on
>>  the prev member, supporting tree views.
>>  (struct bitmap_head): GTY skip the obstack member.
> Was there a particular failure mode you observed or was this discovered
> by inspection.
> 
> The reason I ask is my tester is showing occasional failures
> bootstrapping hppa-linux-gnu with a segfault in ICF.
> 
> I'd rather not have to dig into it if it can be avoided :-)
> Bootstrapping hppa via qemu is something like 6 hours...
And just to follow-up.  I'm really starting to wonder if the hppa issue
is a qemu bug of some kind.

The faulting instruction, ACAICT, shouldn't be faulting.  It appears to
be loading from a valid address, it's a byte load (so no alignment
issues).  Furthermore, I can instruction-step over it in gdb.

Anyway, I think we can reasonably conclude the PA issue is unrelated to
your change.

jeff

Re: C++ PATCH for c++/89158 - by-value capture of constexpr variable broken

[Marek Polacek]

On Tue, Feb 05, 2019 at 01:24:15PM -0500, Jason Merrill wrote:
> On 2/5/19 11:31 AM, Jakub Jelinek wrote:
> > On Tue, Feb 05, 2019 at 11:24:14AM -0500, Jason Merrill wrote:
> > > Yes, thanks, mark_rvalue_use is definitely wrong here.  But 
> > > mark_lvalue_use
> > > might be wrong as well; we don't know here how the expression is used by 
> > > the
> > > inner conversions for the user-defined conversion.  Can we remove the call
> > > entirely?  It doesn't seem to break any Wunused* tests.
> > 
> > It was added in r159096 for -Wunused-but-set*.  It is surely possible it is
> > now covered by other mark_*_use calls.  Or could we call there just
> > mark_exp_read instead?
> 
> That would be fine too.

So how about this if it passes testing?

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

2019-02-05  Marek Polacek  

PR c++/89158 - by-value capture of constexpr variable broken.
* call.c (convert_like_real) : Call mark_exp_read
instead of mark_rvalue_use.

* g++.dg/cpp0x/lambda/lambda-89158.C: New test.

diff --git gcc/cp/call.c gcc/cp/call.c
index c74d1b4ebdf..020a095dade 100644
--- gcc/cp/call.c
+++ gcc/cp/call.c
@@ -7006,7 +7006,9 @@ convert_like_real (conversion *convs, tree expr, tree fn, 
int argnum,
return expr;
  }
 
-   expr = mark_rvalue_use (expr);
+   /* Calling mark_rvalue_use could turn a decl into a constant, breaking
+  e.g. the need_temporary_p assumption in the ICS.  */
+   mark_exp_read (expr);
 
/* Pass LOOKUP_NO_CONVERSION so rvalue/base handling knows not to allow
   any more UDCs.  */
diff --git gcc/testsuite/g++.dg/cpp0x/lambda/lambda-89158.C 
gcc/testsuite/g++.dg/cpp0x/lambda/lambda-89158.C
new file mode 100644
index 000..15f15b46875
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/lambda/lambda-89158.C
@@ -0,0 +1,11 @@
+// PR c++/89158
+// { dg-do compile { target c++11 } }
+
+struct T { T(const int&); };
+void Func(T);
+
+void test()
+{
+  constexpr int Val = 42;
+  [Val]() { Func(Val); };
+}

Re: [Patch] PR rtl-optimization/87763 - generate more bfi instructions on aarch64

[Wilco Dijkstra]

Hi Steve,

Thanks for looking at this. A few comments on the patch:

+bool
+aarch64_masks_and_shift_for_bfi_p (scalar_int_mode mode,
+  unsigned HOST_WIDE_INT mask1,
+  unsigned HOST_WIDE_INT shft_amnt,
+  unsigned HOST_WIDE_INT mask2)
+{
+  unsigned HOST_WIDE_INT t;
+
+  /* Verify that there is no overlap in what bits are set in the two masks.  */
+  if ((mask1 + mask2) != HOST_WIDE_INT_M1U)
+return false;
+  if ((mask1 & mask2) != 0)
+return false;

Why not check mask1 == ~mask2? That's much simpler...

+  /* Verify that the shift amount is less than the mode size.  */
+  if (shft_amnt >= GET_MODE_BITSIZE (mode))
+return false;

The md pattern already guarantees this (this could be an assert). We need
to check that  shift+width <= mode size. Given immediates are limited to
the mode size, the easiest way is to check mask2 is not 0 or M1.

+  /* Verify that the mask being shifted is contiguous and would be in the
+ least significant bits after shifting by creating a mask 't' based on
+ the number of bits set in mask2 and the shift amount for mask2 and
+ comparing that to the actual mask2.  */
+  t = popcount_hwi (mask2);
+  t = (HOST_WIDE_INT_1U << t) - 1;
+  t = t << shft_amnt;
+  if (t != mask2)
+return false;
+  
+  return true;

This would return true if mask2 == 0 or M1 (I think this can't happen with
current md patterns, but this would emit an illegal bfi).

After special cases you could do something like t = mask2 + (HWI_1U << shift);
return t == (t & -t) to check for a valid bfi.

+  "bfi\t%0, %1, 0, %P2"

This could emit a width that may be 32 too large in SImode if bit 31 is set
(there is another use of %P in aarch64.md which may have the same issue).

Finally from some quick testing it seems one case is not yet supported:

int f1(int x, int y)
{
  return (y & 0xfff) | (((x <<28) & 0xf000));
}

This just has a shift, rather than an AND plus a shift.

Cheers,
Wilco

Re: C++ PATCH for c++/89158 - by-value capture of constexpr variable broken

[Jason Merrill]

On 2/5/19 3:43 PM, Marek Polacek wrote:

On Tue, Feb 05, 2019 at 01:24:15PM -0500, Jason Merrill wrote:

On 2/5/19 11:31 AM, Jakub Jelinek wrote:

On Tue, Feb 05, 2019 at 11:24:14AM -0500, Jason Merrill wrote:

Yes, thanks, mark_rvalue_use is definitely wrong here.  But mark_lvalue_use
might be wrong as well; we don't know here how the expression is used by the
inner conversions for the user-defined conversion.  Can we remove the call
entirely?  It doesn't seem to break any Wunused* tests.


It was added in r159096 for -Wunused-but-set*.  It is surely possible it is
now covered by other mark_*_use calls.  Or could we call there just
mark_exp_read instead?


That would be fine too.


So how about this if it passes testing?

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

2019-02-05  Marek Polacek  

PR c++/89158 - by-value capture of constexpr variable broken.
* call.c (convert_like_real) : Call mark_exp_read
instead of mark_rvalue_use.

* g++.dg/cpp0x/lambda/lambda-89158.C: New test.

diff --git gcc/cp/call.c gcc/cp/call.c
index c74d1b4ebdf..020a095dade 100644
--- gcc/cp/call.c
+++ gcc/cp/call.c
@@ -7006,7 +7006,9 @@ convert_like_real (conversion *convs, tree expr, tree fn, 
int argnum,
return expr;
  }
  
-	expr = mark_rvalue_use (expr);

+   /* Calling mark_rvalue_use could turn a decl into a constant, breaking
+  e.g. the need_temporary_p assumption in the ICS.  */
+   mark_exp_read (expr);


I'd describe the issue at a higher level, e.g. "we don't know here 
whether expr is being used as an lvalue or rvalue".  mark_rvalue_use is 
wrong for this testcase because expr is actually used as an lvalue.  OK 
with that change.


Jason

Re: C++ PATCH for c++/89158 - by-value capture of constexpr variable broken

[Marek Polacek]

On Tue, Feb 05, 2019 at 04:17:48PM -0500, Jason Merrill wrote:
> On 2/5/19 3:43 PM, Marek Polacek wrote:
> > On Tue, Feb 05, 2019 at 01:24:15PM -0500, Jason Merrill wrote:
> > > On 2/5/19 11:31 AM, Jakub Jelinek wrote:
> > > > On Tue, Feb 05, 2019 at 11:24:14AM -0500, Jason Merrill wrote:
> > > > > Yes, thanks, mark_rvalue_use is definitely wrong here.  But 
> > > > > mark_lvalue_use
> > > > > might be wrong as well; we don't know here how the expression is used 
> > > > > by the
> > > > > inner conversions for the user-defined conversion.  Can we remove the 
> > > > > call
> > > > > entirely?  It doesn't seem to break any Wunused* tests.
> > > > 
> > > > It was added in r159096 for -Wunused-but-set*.  It is surely possible 
> > > > it is
> > > > now covered by other mark_*_use calls.  Or could we call there just
> > > > mark_exp_read instead?
> > > 
> > > That would be fine too.
> > 
> > So how about this if it passes testing?
> > 
> > Bootstrapped/regtested running on x86_64-linux, ok for trunk/8?
> > 
> > 2019-02-05  Marek Polacek  
> > 
> > PR c++/89158 - by-value capture of constexpr variable broken.
> > * call.c (convert_like_real) : Call mark_exp_read
> > instead of mark_rvalue_use.
> > 
> > * g++.dg/cpp0x/lambda/lambda-89158.C: New test.
> > 
> > diff --git gcc/cp/call.c gcc/cp/call.c
> > index c74d1b4ebdf..020a095dade 100644
> > --- gcc/cp/call.c
> > +++ gcc/cp/call.c
> > @@ -7006,7 +7006,9 @@ convert_like_real (conversion *convs, tree expr, tree 
> > fn, int argnum,
> > return expr;
> >   }
> > -   expr = mark_rvalue_use (expr);
> > +   /* Calling mark_rvalue_use could turn a decl into a constant, breaking
> > +  e.g. the need_temporary_p assumption in the ICS.  */
> > +   mark_exp_read (expr);
> 
> I'd describe the issue at a higher level, e.g. "we don't know here whether
> expr is being used as an lvalue or rvalue".  mark_rvalue_use is wrong for
> this testcase because expr is actually used as an lvalue.  OK with that
> change.

Will do, thanks!

Marek

New Ukrainian PO file for 'gcc' (version 9.1-b20190203)

[Translation Project Robot]

Hello, gentle maintainer.

This is a message from the Translation Project robot.

A revised PO file for textual domain 'gcc' has been submitted
by the Ukrainian team of translators.  The file is available at:

https://translationproject.org/latest/gcc/uk.po

(This file, 'gcc-9.1-b20190203.uk.po', has just now been sent to you in
a separate email.)

All other PO files for your package are available in:

https://translationproject.org/latest/gcc/

Please consider including all of these in your next release, whether
official or a pretest.

Whenever you have a new distribution with a new version number ready,
containing a newer POT file, please send the URL of that distribution
tarball to the address below.  The tarball may be just a pretest or a
snapshot, it does not even have to compile.  It is just used by the
translators when they need some extra translation context.

The following HTML page has been updated:

https://translationproject.org/domain/gcc.html

If any question arises, please contact the translation coordinator.

Thank you for all your work,

The Translation Project robot, in the
name of your translation coordinator.

Re: Late-breaking jit features (was Re: [PATCH][gcc] libgccjit: introduce gcc_jit_context_add_driver_option)

[Andrea Corallo]

David Malcolm writes:

> On Sat, 2019-02-02 at 16:34 +0100, Jakub Jelinek wrote:
>> On Sat, Feb 02, 2019 at 10:18:43AM -0500, David Malcolm wrote:
>> > > > Alternatively, should these patches go into a branch of queued
>> > > > jit
>> > > > changes for gcc 10?
>> > >
>> > > Is there anything like an ABI involved? If so we should avoid
>> > > breaking it all the time. Otherwise JIT is not release critical
>> > > and
>> > > thus if you break it in the wrong moment it's your own fault.
>> >
>> > The two patches each add a new API entrypoint, but libgccjit uses
>> > symbol-versioning to extend the ABI, without bumping the SONAME:
>> >   https://gcc.gnu.org/onlinedocs/jit/topics/compatibility.html
>> > So it's not an ABI break as such.
>>
>> I'd say it depends on how quickly the copyright paperwork can be
>> done, the
>> patch can't be added until that is resolved.  While gccjit is not
>> release
>> critical, it would be nice not to break it late, so say if it can be
>> committed by end of February/mid March, I guess it is fine, given the
>> assumption we'd like to release mid April to end of April, if it
>> can't be
>> done by then, might be better to postpone to GCC 10.
>>
>>  Jakub
>
> Jakub and Richard: thanks.
>
> I've double-checked the gcc_jit_context_add_driver_option patch and it
> looks good (it's a different patch that we're waiting on paperwork
> for).
>
> Andrea: are you able to commit this, or should I do this on your
> behalf?
>
> Dave

Hi David,
I have no repo write access so if you could push it that would be great.

Thanks a lot

  Andrea

Re: [PATCH] Fix up gcc.target/i386/call-1.c testcase (PR rtl-optimization/11304)

[Mike Stump]

On Feb 2, 2019, at 2:32 AM, Jakub Jelinek  wrote:
> 
> Regardless of the PR87485 decision, I think we should fix this testcase.

> ok for trunk?

Ok.

> 2019-02-02  Jakub Jelinek  
> 
>   PR rtl-optimization/11304
>   * gcc.target/i386/call-1.c (set_eax): Add "eax" clobber.
>   * gcc.target/i386/call-2.c: New test.
> 
> void set_eax(int val)
> {
> -  __asm__ __volatile__ ("mov %0, %%eax" : : "m" (val));
> +  __asm__ __volatile__ ("mov %0, %%eax" : : "m" (val) : "eax");
> }

Re: [PATCH] print correct array sizes in errors (PR 87996)

[Martin Sebor]

On 2/5/19 12:14 PM, Jason Merrill wrote:

On 2/5/19 1:46 PM, Martin Sebor wrote:

On 2/1/19 7:41 AM, Jason Merrill wrote:

On 1/31/19 5:49 PM, Martin Sebor wrote:

On 1/30/19 3:15 PM, Jason Merrill wrote:

On 1/29/19 7:15 PM, Martin Sebor wrote:

+  /* Try to convert the original SIZE to a ssizetype.  */
+  if (orig_size != error_mark_node
+  && !TYPE_UNSIGNED (TREE_TYPE (orig_size)))
+    {
+  if (TREE_CODE (size) == INTEGER_CST
+  && tree_int_cst_sign_bit (size))
+    diagsize = build_converted_constant_expr (ssizetype, size,
+  tsubst_flags_t ());
+  else if (size == error_mark_node
+   && TREE_CODE (orig_size) == INTEGER_CST
+   && tree_int_cst_sign_bit (orig_size))
+    diagsize = build_converted_constant_expr (ssizetype, 
orig_size,

+  tsubst_flags_t ());
+    }


Using build_converted_constant_expr here looks odd; that's a 
language-level notion, and we're dealing with compiler internals. 
fold_convert seems more appropriate.


Done.




+  if (TREE_CONSTANT (size))
+    {
+  if (!diagsize && TREE_CODE (size) == INTEGER_CST)
+    diagsize = size;
+    }
+  else
 size = osize;
 }

@@ -9732,15 +9758,12 @@ compute_array_index_type_loc (location_t 
name_loc,

   if (TREE_CODE (size) == INTEGER_CST)
 {
   /* An array must have a positive number of elements.  */
-  if (!valid_constant_size_p (size))
+  if (!diagsize)
+    diagsize = size;


It seems like the earlier hunk here is unnecessary; if size is an 
INTEGER_CST, it will be unchanged, and so be used for diagsize in 
the latter hunk without any changes to the earlier location. 
Actually, why not do all of the diagsize logic down here?  There 
doesn't seem to be anything above that relies on information we 
will have lost at this point.


Sure.  Done in the attached revision.



-  tree osize = size;
+  /* The original numeric size as seen in the source code after
+ any substitution and before conversion to size_t.  */
+  tree origsize = NULL_TREE;


Can't you use osize?  instantiate_non_dependent_expr doesn't do any 
actual substitution, it shouldn't change the type of the expression 
or affect whether it's an INTEGER_CST.


I went ahead and reused osize but kept the new name origsize (I assume
avoiding introducing a new variable is what you meant).  The longer
name is more descriptive and also has a comment explaining what it's
for (which is why I didn't touch osize initially -- I didn't know
enough about what it was for or how it might change).


Yep, it was saving the original "size" before conversions.  I guess the 
name change is OK, but please restore the initialization from "size", 
and drop the added comment on the call to mark_rvalue_use (which is to 
possibly replace a lambda capture with its constant value).


How about passing origsize into valid_array_size_p rather than than 
always computing diagsize in the caller?


I'm not sure I understand what you're asking for here.  diagsize
is computed from either size or origsize but valid_array_size_p
takes just one size (or type) argument.  What part do you want
to move to valid_array_size_p (and why)?  Or are you asking to
call fold_convert() in valid_array_size_p instead here?

Martin

[v3 PATCH, RFC] Rewrite variant. Also PR libstdc++/85517

[Ville Voutilainen]

Okay then. This patch takes the hopefully biggest steps towards
a std::variant rewrite. The problem we have with the current
approach is that we'd really like to write fairly straightforward
code for variant's special member functions, but can't, because
the specification suggests fairly straightforward compile-time-property
queries and selection of behavior based on those, but variant's selected
index is a run-time property. This leads to difficulties implementing
what the standard requires, like the differences of handling throwing and
non-throwing move/copy operations of the types held in variant.

Well. We apply the hammer of Pattern Matching. variant's visit()
can get us into code that can do the compile-time queries despite
variant's selected index being a run-time property. We modify
the visitation mechanism to not throw an exception if invoked
with a special kind of visitor that can handle valueless variants,
and then use polylambdas as such visitors, and do if-constexprs
in those polylambdas.

We can now get rid of the function-pointer tables that are used for
similar kind of dispatching in the original approach. Visitation
generates similar tables, so we keep the O(1) indexing into the right
function based on the current selected index, and there's the same
amount of indirect calls through a pointer-to-function, one.
Our code for e.g. variant's copy assignment is now straightforward;
it visits both the this-variant and the rhs-variant at the same time,
and does the right thing based on the compile-time properties of the
selected type,
and whether either variant is valueless. Modulo the polylambda wrapping
and the fact that all ifs in it are if-constexprs, it looks like it's just
doing conditional code based on what the nothrow-properties, for example,
of the type of the object held by the variant are.

This patch also partially gets rid of some of the cases where an object
is self-destroyed before it's reconstructed. We shouldn't do that.
We should be doing _M_reset followed by _M_construct, so that we only
ever destroy a variant member of a union object held in the variant's
storage, and then try to reconstruct that (or some other variant member),
and we should never self-destroy any object in the inheritance chain of
variant. The rest of that is work-in-progress, as is changing the rest
of the special member functions to use visitation.

Thoughts?

2019-02-05  Ville Voutilainen  

Rewrite variant.
Also PR libstdc++/85517
* include/std/variant (__do_visit): New.
(__variant_cast): Likewise.
(__variant_cookie): Likewise.
(__erased_dtor): Remove.
(_Variant_storage::_S_vtable): Likewise.
(_Variant_storage::__M_reset_impl): Adjust to use __do_visit.
(_Variant_storage::__M_reset): Adjust.
(_Move_ctor_base::__M_destructive_copy): New.
(_Copy_assign_base::operator=): Adjust to use __do_visit.
(_Copy_assign_alias): Adjust to check both copy assignment
and copy construction for triviality.
(_Multi_array): Add support for visitors that accept and return
a __variant_cookie.
(__gen_vtable_impl::_S_apply_all_alts): Likewise.
(__gen_vtable_impl::_S_apply_single_alt): Likewise.
(__gen_vtable_impl::__element_by_index_or_cookie): New. Generate
a __variant_cookie temporary for a variant that is valueless and..
(__gen_vtable_impl::__visit_invoke): ..adjust here.
(__gen_vtable::_Array_type): Conditionally make space for
the __variant_cookie visitor case.
(variant): Add __variant_cast as a friend.
(variant::emplace): Use _M_reset() instead of self-destruction.
(visit): Reimplement in terms of __do_visit.
* testsuite/20_util/variant/compile.cc: Adjust.
* testsuite/20_util/variant/run.cc: Likewise.
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index 89deb14..c2f82a7 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -138,6 +138,19 @@ namespace __variant
 constexpr variant_alternative_t<_Np, variant<_Types...>> const&&
 get(const variant<_Types...>&&);
 
+  template
+constexpr decltype(auto)
+__do_visit(_Visitor&& __visitor, _Variants&&... __variants);
+
+  template 
+decltype(auto) __variant_cast(_Tp&& __rhs)
+{
+  if constexpr (is_const_v>)
+return static_cast&>(__rhs);
+  else
+return static_cast&>(__rhs);
+}
+
 namespace __detail
 {
 namespace __variant
@@ -155,6 +168,9 @@ namespace __variant
   std::integral_constant
 	? 0 : __index_of_v<_Tp, _Rest...> + 1> {};
 
+  // used for raw visitation
+  struct __variant_cookie {};
+
   // _Uninitialized is guaranteed to be a literal type, even if T is not.
   // We have to do this, because [basic.types]p10.5.3 (n4606) is not implemented
   // yet. When it's implemented, _Uninitialized can be changed to the alias
@@ -246,11 +262,6 @@ namespace __variant
   ::new (__lhs) _Type(__variant::__ref_cast<_Rhs>(__rhs));
 }
 
-  template
-

Re: Late-breaking jit features (was Re: [PATCH][gcc] libgccjit: introduce gcc_jit_context_add_driver_option)

[David Malcolm]

On Tue, 2019-02-05 at 21:40 +, Andrea Corallo wrote:
> David Malcolm writes:
> 
> > On Sat, 2019-02-02 at 16:34 +0100, Jakub Jelinek wrote:
> > > On Sat, Feb 02, 2019 at 10:18:43AM -0500, David Malcolm wrote:
> > > > > > Alternatively, should these patches go into a branch of
> > > > > > queued
> > > > > > jit
> > > > > > changes for gcc 10?
> > > > > 
> > > > > Is there anything like an ABI involved? If so we should avoid
> > > > > breaking it all the time. Otherwise JIT is not release
> > > > > critical
> > > > > and
> > > > > thus if you break it in the wrong moment it's your own fault.
> > > > 
> > > > The two patches each add a new API entrypoint, but libgccjit
> > > > uses
> > > > symbol-versioning to extend the ABI, without bumping the
> > > > SONAME:
> > > >   https://gcc.gnu.org/onlinedocs/jit/topics/compatibility.html
> > > > So it's not an ABI break as such.
> > > 
> > > I'd say it depends on how quickly the copyright paperwork can be
> > > done, the
> > > patch can't be added until that is resolved.  While gccjit is not
> > > release
> > > critical, it would be nice not to break it late, so say if it can
> > > be
> > > committed by end of February/mid March, I guess it is fine, given
> > > the
> > > assumption we'd like to release mid April to end of April, if it
> > > can't be
> > > done by then, might be better to postpone to GCC 10.
> > > 
> > >   Jakub
> > 
> > Jakub and Richard: thanks.
> > 
> > I've double-checked the gcc_jit_context_add_driver_option patch and
> > it
> > looks good (it's a different patch that we're waiting on paperwork
> > for).
> > 
> > Andrea: are you able to commit this, or should I do this on your
> > behalf?
> > 
> > Dave
> 
> Hi David,
> I have no repo write access so if you could push it that would be
> great.
> 
> Thanks a lot

I've committed it to trunk as r268563.

Dave

[PATCH] Fix fold_const_vec_convert (PR middle-end/89210)

[Jakub Jelinek]

Hi!

Apparently VECTOR_CSTs shouldn't be stepped if they contain floating
elements and also widening conversions can be problematic if there is
wrapping in the narrower type.  On the following testcase, we create a
stepped VECTOR_CST with REAL_CST elts and ICE whenever we try to print it or
when we try to expand it.  The following patch follows what fold_convert_const
does.

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

2019-02-05  Jakub Jelinek  

PR middle-end/89210
* fold-const-call.c (fold_const_vec_convert): Pass true as last
operand to new_unary_operation only if both element types are integral
and it isn't a widening conversion.  Return NULL_TREE if
new_unary_operation failed.

* c-c++-common/builtin-convertvector-2.c: New test.

--- gcc/fold-const-call.c.jj2019-01-07 09:47:33.046517940 +0100
+++ gcc/fold-const-call.c   2019-02-05 19:15:44.111209138 +0100
@@ -665,8 +665,17 @@ fold_const_vec_convert (tree ret_type, t
   && SCALAR_FLOAT_TYPE_P (TREE_TYPE (ret_type)))
 code = FLOAT_EXPR;
 
+  /* We can't handle steps directly when extending, since the
+ values need to wrap at the original precision first.  */
+  bool step_ok_p
+= (INTEGRAL_TYPE_P (TREE_TYPE (ret_type))
+   && INTEGRAL_TYPE_P (TREE_TYPE (arg_type))
+   && (TYPE_PRECISION (TREE_TYPE (ret_type))
+  <= TYPE_PRECISION (TREE_TYPE (arg_type;
   tree_vector_builder elts;
-  elts.new_unary_operation (ret_type, arg, true);
+  if (!elts.new_unary_operation (ret_type, arg, step_ok_p))
+return NULL_TREE;
+
   unsigned int count = elts.encoded_nelts ();
   for (unsigned int i = 0; i < count; ++i)
 {
--- gcc/testsuite/c-c++-common/builtin-convertvector-2.c.jj 2019-02-05 
19:19:16.981705824 +0100
+++ gcc/testsuite/c-c++-common/builtin-convertvector-2.c2019-02-05 
19:17:20.217627468 +0100
@@ -0,0 +1,12 @@
+/* PR middle-end/89210 */
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+typedef int v4si __attribute__((vector_size (4 * sizeof (int;
+typedef double v4df __attribute__((vector_size (4 * sizeof (double;
+void
+foo (v4df *x)
+{
+  v4si a = { 1, 2, 3, 4 };
+  *x = __builtin_convertvector (a, v4df);
+}

Jakub

[C PATCH] Fix C ICE with K&R definitions (PR c/89211)

[Jakub Jelinek]

Hi!

The r253411 change to improve diagnostics added code to set DECL_ARGUMENTS
to the declarator->u.arg_info->parms.  My understanding is that this was
meant for function prototypes, so that we can emit better diagnostics for
those.  Unfortunately, start_decl doesn't always return a new decl, but
returns what pushdecl returned, which could be the new prototype, but could
be some earlier prototyped or defined function.  The expansion etc. is very
unhappy if a function definition changes in the middle from having no
arguments into one with DECL_ARGUMENTs, especially if those arguments are
invalid.  So, this patch limits that change to prototypes only, doesn't try
to modify a function definition.

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

2019-02-05  Jakub Jelinek  

PR c/89211
* c-parser.c (c_parser_declaration_or_fndef): Don't update
DECL_ARGUMENTS of d if it has been defined already.  Use a single if
instead of 3 nested ifs.

* gcc.dg/pr89211.c: New test.

--- gcc/c/c-parser.c.jj 2019-01-09 19:55:01.071371422 +0100
+++ gcc/c/c-parser.c2019-02-05 20:57:14.460652556 +0100
@@ -2154,10 +2154,12 @@ c_parser_declaration_or_fndef (c_parser
  tree d = start_decl (declarator, specs, false,
   chainon (postfix_attrs,
all_prefix_attrs));
- if (d && TREE_CODE (d) == FUNCTION_DECL)
-   if (declarator->kind == cdk_function)
- if (DECL_ARGUMENTS (d) == NULL_TREE)
-   DECL_ARGUMENTS (d) = declarator->u.arg_info->parms;
+ if (d
+ && TREE_CODE (d) == FUNCTION_DECL
+ && declarator->kind == cdk_function
+ && DECL_ARGUMENTS (d) == NULL_TREE
+ && DECL_INITIAL (d) == NULL_TREE)
+   DECL_ARGUMENTS (d) = declarator->u.arg_info->parms;
  if (omp_declare_simd_clauses.exists ())
{
  tree parms = NULL_TREE;
--- gcc/testsuite/gcc.dg/pr89211.c.jj   2019-02-05 20:59:45.113159822 +0100
+++ gcc/testsuite/gcc.dg/pr89211.c  2019-02-05 20:59:40.331238946 +0100
@@ -0,0 +1,8 @@
+/* PR c/89211 */
+/* { dg-do compile } */
+
+void foo ();
+void foo ()
+{
+  void foo (struct S); /* { dg-warning "declared inside parameter list" } */
+}

Jakub

Contents of PO file 'cpplib-9.1-b20190203.pt_BR.po'

[Translation Project Robot]

cpplib-9.1-b20190203.pt_BR.po.gz
Description: Binary data
The Translation Project robot, in the
name of your translation coordinator.

New Brazilian Portuguese PO file for 'cpplib' (version 9.1-b20190203)

[Translation Project Robot]

Hello, gentle maintainer.

This is a message from the Translation Project robot.

A revised PO file for textual domain 'cpplib' has been submitted
by the Brazilian Portuguese team of translators.  The file is available at:

https://translationproject.org/latest/cpplib/pt_BR.po

(This file, 'cpplib-9.1-b20190203.pt_BR.po', has just now been sent to you in
a separate email.)

All other PO files for your package are available in:

https://translationproject.org/latest/cpplib/

Please consider including all of these in your next release, whether
official or a pretest.

Whenever you have a new distribution with a new version number ready,
containing a newer POT file, please send the URL of that distribution
tarball to the address below.  The tarball may be just a pretest or a
snapshot, it does not even have to compile.  It is just used by the
translators when they need some extra translation context.

The following HTML page has been updated:

https://translationproject.org/domain/cpplib.html

If any question arises, please contact the translation coordinator.

Thank you for all your work,

The Translation Project robot, in the
name of your translation coordinator.

[PATCH] PR libstdc++/89128 add deduction guides for container adaptors

[Jonathan Wakely]

PR libstdc++/89128
* include/bits/stl_queue.h (queue, priority_queue): Add deduction
guides.
* include/bits/stl_stack.h (stack): Likewise.
* testsuite/23_containers/priority_queue/deduction.cc: New test.
* testsuite/23_containers/queue/deduction.cc: New test.
* testsuite/23_containers/stack/deduction.cc: New test.

Tested powerpc64le-linux, committed to trunk.

commit 398e393995ca03cae4de07643aa4d5765da0d278
Author: Jonathan Wakely 
Date:   Tue Feb 5 22:33:49 2019 +

PR libstdc++/89128 add deduction guides for container adaptors

PR libstdc++/89128
* include/bits/stl_queue.h (queue, priority_queue): Add deduction
guides.
* include/bits/stl_stack.h (stack): Likewise.
* testsuite/23_containers/priority_queue/deduction.cc: New test.
* testsuite/23_containers/queue/deduction.cc: New test.
* testsuite/23_containers/stack/deduction.cc: New test.

diff --git a/libstdc++-v3/include/bits/stl_queue.h 
b/libstdc++-v3/include/bits/stl_queue.h
index aa8dd02ed6e..6d092c9bbfe 100644
--- a/libstdc++-v3/include/bits/stl_queue.h
+++ b/libstdc++-v3/include/bits/stl_queue.h
@@ -302,6 +302,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #endif // __cplusplus >= 201103L
 };
 
+#if __cpp_deduction_guides >= 201606
+  template::value>>
+queue(_Container) -> queue;
+
+  template::value>,
+  typename = enable_if_t<__is_allocator<_Allocator>::value>>
+queue(_Container, _Allocator)
+-> queue;
+#endif
+
   /**
*  @brief  Queue equality comparison.
*  @param  __x  A %queue.
@@ -653,6 +665,32 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #endif // __cplusplus >= 201103L
 };
 
+#if __cpp_deduction_guides >= 201606
+  template::value>,
+  typename = enable_if_t::value>>
+priority_queue(_Compare, _Container)
+-> priority_queue;
+
+  template::value_type,
+  typename _Compare = less<_ValT>,
+  typename _Container = vector<_ValT>,
+  typename = _RequireInputIter<_InputIterator>,
+  typename = enable_if_t::value>,
+  typename = enable_if_t::value>>
+priority_queue(_InputIterator, _InputIterator, _Compare = _Compare(),
+  _Container = _Container())
+-> priority_queue<_ValT, _Container, _Compare>;
+
+  template::value>,
+  typename = enable_if_t::value>,
+  typename = enable_if_t<__is_allocator<_Allocator>::value>>
+priority_queue(_Compare, _Container, _Allocator)
+-> priority_queue;
+#endif
+
   // No equality/comparison operators are provided for priority_queue.
 
 #if __cplusplus >= 201103L
diff --git a/libstdc++-v3/include/bits/stl_stack.h 
b/libstdc++-v3/include/bits/stl_stack.h
index 8581dce210c..e8443a78a05 100644
--- a/libstdc++-v3/include/bits/stl_stack.h
+++ b/libstdc++-v3/include/bits/stl_stack.h
@@ -276,6 +276,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #endif // __cplusplus >= 201103L
 };
 
+#if __cpp_deduction_guides >= 201606
+  template::value>>
+stack(_Container) -> stack;
+
+  template::value>,
+  typename = enable_if_t<__is_allocator<_Allocator>::value>>
+stack(_Container, _Allocator)
+-> stack;
+#endif
+
   /**
*  @brief  Stack equality comparison.
*  @param  __x  A %stack.
diff --git a/libstdc++-v3/testsuite/23_containers/priority_queue/deduction.cc 
b/libstdc++-v3/testsuite/23_containers/priority_queue/deduction.cc
new file mode 100644
index 000..4630cbb101c
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/priority_queue/deduction.cc
@@ -0,0 +1,119 @@
+// 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++17" }
+// { dg-do compile { target c++17 } }
+
+#include 
+#include 
+#include 
+#include 
+
+template struct require_same;
+template struct require_same { using type = void; };
+
+template
+  typename require_same::type
+  check_type(U&) { }
+
+void
+test01()
+{
+  std::priority_queue s0;
+
+  std::priority_queue s1 = s0;
+  check_type>(s1);
+
+  std::priority_queue s2 = std::move(s0);
+  check_type>(s2);
+
+  const std::priority_queue s3 = s0;
+  check_type>(s3);
+
+  const std::priority_queue s4 = s3;
+  check_type>(s4);
+
+  std::allocator a;
+  std::priority_queue s5(s

Re: Make clear, when contributions will be ignored

[Joseph Myers]

On Tue, 5 Feb 2019, Дилян Палаузов wrote:

> Will it help, if bugzilla is tweaked to send reminders every two weeks 
> for ready-patches?  This also has the advantage, that people will not 
> have to once update a patch in BZ and then send it over gcc-patches.

For any proposed changes to patch submission / review processes to be 
helpful, they need to work with the existing development community, which 
means they need to be designed based on a deep understanding of what works 
for developers and reviewers, and of the issues likely to lead to lack of 
response on a submission (which can be that it doesn't fall clearly into 
any one maintainer's area, but can also be that the submission has 
deficiencies meaning it would take much longer to review than a 
well-formed submission, such as inadequate explanation, lack of testcases, 
lack of documentation, poor or missing comments, failure to follow the GNU 
Coding Standards, lack of ChangeLog entries, etc., and so is likely to be 
dropped unless a reviewer has more time than usual at the time the patch 
is posted).  A discussion at a future GNU Tools Cauldron would be better 
than on the gcc-patches list (which is for concrete discussion of 
individual patches, not meta-discussion of patch review processes).  I 
think an ongoing commitment from someone with sufficient experience with 
the community to maintain and develop any new tool used would also be 
required, as any existing tool in this area is unlikely to do well without 
significant customization.

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

Re: [C PATCH] Fix C ICE with K&R definitions (PR c/89211)

[Joseph Myers]

On Tue, 5 Feb 2019, Jakub Jelinek wrote:

> Hi!
> 
> The r253411 change to improve diagnostics added code to set DECL_ARGUMENTS
> to the declarator->u.arg_info->parms.  My understanding is that this was
> meant for function prototypes, so that we can emit better diagnostics for
> those.  Unfortunately, start_decl doesn't always return a new decl, but
> returns what pushdecl returned, which could be the new prototype, but could
> be some earlier prototyped or defined function.  The expansion etc. is very
> unhappy if a function definition changes in the middle from having no
> arguments into one with DECL_ARGUMENTs, especially if those arguments are
> invalid.  So, this patch limits that change to prototypes only, doesn't try
> to modify a function definition.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

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

Fix type of extern array declared in inner scope with outer initialization shadowed (PR c/88584)

[Joseph Myers]

As reported in bug 88584, if you have a file-scope array with external
linkage, initialized at file scope, and that array is shadowed at
block scope, and is declared again with external linkage and an
incomplete type in an inner scope, it is wrongly given a complete type
in that inner scope when the correct C semantics give it an incomplete
type (only the visible declarations contribute to the type in a given
scope).

In general, issues with the types of external linkage declarations
being different in different scopes were addressed by my fixes for bug
13801, for GCC 4.0.  In this case, however, the code in pushdecl
dealing with giving declarations the right type in each scope works
fine, and the type is subsequently modified by complete_array_type
called from finish_decl: finish_decl is trying to complete an array
type based on an initializer, but that's only correct for the original
initialization at file scope, not for such a declaration in an inner
scope (it's harmless but unnecessary in the case where the original
declaration is still visible in the inner scope).  Thus, this patch
changes finish_decl to stop this logic applying for such an external
declaration in an inner scope.  (An erroneous attempt to include an
initializer for an extern variable in an inner scope is diagnosed
elsewhere.)

This is a regression from GCC 3.4, which properly rejected the code in
question (quite likely by accident).

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

gcc/c:
2019-02-06  Joseph Myers  

PR c/88584
* c-decl.c (finish_decl): Do not complete array types for arrays
with external linkage not at file scope.

gcc/testsuite:
2019-02-06  Joseph Myers  

PR c/88584
* gcc.dg/redecl-18.c: New test.

Index: gcc/c/c-decl.c
===
--- gcc/c/c-decl.c  (revision 268567)
+++ gcc/c/c-decl.c  (working copy)
@@ -5099,10 +5099,15 @@ finish_decl (tree decl, location_t init_loc, tree
 
   type = TREE_TYPE (decl);
 
-  /* Deduce size of array from initialization, if not already known.  */
+  /* Deduce size of array from initialization, if not already known.
+ This is only needed for an initialization in the current scope;
+ it must not be done for a file-scope initialization of a
+ declaration with external linkage, redeclared in an inner scope
+ with the outer declaration shadowed in an intermediate scope.  */
   if (TREE_CODE (type) == ARRAY_TYPE
   && TYPE_DOMAIN (type) == NULL_TREE
-  && TREE_CODE (decl) != TYPE_DECL)
+  && TREE_CODE (decl) != TYPE_DECL
+  && !(TREE_PUBLIC (decl) && current_scope != file_scope))
 {
   bool do_default
= (TREE_STATIC (decl)
Index: gcc/testsuite/gcc.dg/redecl-18.c
===
--- gcc/testsuite/gcc.dg/redecl-18.c(nonexistent)
+++ gcc/testsuite/gcc.dg/redecl-18.c(working copy)
@@ -0,0 +1,17 @@
+/* Test redeclaration in an inner scope, with an incomplete type, of a
+   file-scope initialized array shadowed in an intermediate scope (bug
+   88584).  */
+/* { dg-do compile } */
+/* { dg-options "" } */
+
+int a[1] = { 0 };
+
+void
+f (void)
+{
+  int a;
+  {
+extern int a[];
+sizeof (a); /* { dg-error "incomplete" } */
+  }
+}

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

Re: [C++PATCH] [PR86379] do not use TREE_TYPE for USING_DECL_SCOPE

[Alexandre Oliva]

On Feb  5, 2019, Jason Merrill  wrote:

> On Tue, Feb 5, 2019 at 1:37 AM Alexandre Oliva  wrote:
>> On Jan 31, 2019, Jason Merrill  wrote:
>> 
>> > Let's use strip_using_decl instead
>> 
>> Aah, nice!  Thanks, I'll make the changes, test them, and post a new patch.
>> 
>> 
>> >> @@ -13288,7 +13295,8 @@ grok_special_member_properties (tree decl)
>> >> {
>> >> tree class_type;
>> >> -  if (!DECL_NONSTATIC_MEMBER_FUNCTION_P (decl))
>> >> +  if (TREE_CODE (decl) != USING_DECL
>> >> +  && !DECL_NONSTATIC_MEMBER_FUNCTION_P (decl))
>> >> return;
>> 
>> > Is there a reason not to use it here, as well?
>> 
>> The using decl will take us to a member of a different class, and this
>> function takes the DECL_CONTEXT of decl and adjusts the properties of
>> that class.  If we followed USING_DECLs in decl that early, we'd adjust
>> (again?) the member properties of USING_DECL_SCOPE(original using decl),
>> rather than of DECL_CONTEXT (original using decl) as intended.

> But a little further in, copy_fn_p will also check
> DECL_NONSTATIC_MEMBER_FUNCTION_P and crash.

It would crash if I hadn't adjusted it to test for USING_DECLs, yes.

> This function used to do nothing for USING_DECL (since its TREE_TYPE
> wasn't METHOD_TYPE), right?  It should continue to do nothing.

I thought I was fixing bugs making certain functions follow USING_DECLs
rather than fail to do what it should, because the test on TREE_TYPE
happened to not pass.  This one I was not so sure about, for the reasons
above.  I guess it makes sense to return early if it really shouldn't do
anything for a USING_DECL, even if it happens to resolve to a method
that it would otherwise handle if declared directly in the class.

> Come to think of it, how about fixing DECL_NONSTATIC_MEMBER_FUNCTION_P
> to be false for non-functions rather than messing with all these
> places that use it?

I considered that at first, but it seemed to me that this would not
bring about the desired behavior in the first functions I touched
(lookup_member vs shared_member), and I thought we'd be better off
retaining some means to catch incorrect uses of this and other macros on
USING_DECLs, so that we can analyze and fix the uses instead of getting
accidental behavior, correct or not.


Here's what I'm testing now.


[PR86379] do not use TREE_TYPE for USING_DECL_SCOPE

From: Alexandre Oliva 

It's too risk to reuse the type field for USING_DECL_SCOPE.
Language-independent parts of the compiler, such as location and
non-lvalue wrappers, happily take the TREE_TYPE of a USING_DECL as if
it was a type rather than an unrelated scope.

For better or worse, USING_DECLs use the non-common struct so we can
use the otherwise unused result field.  Adjust fallout, from uses of
TREE_TYPE that were supposed to be USING_DECL_SCOPE, to other
accidental uses of TREE_TYPE of a USING_DECL.


for  gcc/cp/ChangeLog

PR c++/86379
* cp-tree.h (USING_DECL_SCOPE): Use result rather than type.
* name-lookup.c (strip_using_decl): Use USING_DECL_SCOPE.
* search.c (protected_accessible_p): Follow USING_DECL_DECLS.
(shared_member_p): Likewise.
(lookup_member): Likewise.
* decl.c (grok_special_member_properties): Skip USING_DECLs.
* semantics.c (finish_omp_declare_simd_methods): Likewise.

for  gcc/testsuite/ChangeLog

PR c++/86379
* g++.dg/cpp0x/pr86379.C: New.
---
 gcc/cp/cp-tree.h |2 
 gcc/cp/decl.c|3 
 gcc/cp/name-lookup.c |2 
 gcc/cp/search.c  |   17 ++-
 gcc/cp/semantics.c   |3 
 gcc/testsuite/g++.dg/cpp0x/pr86379.C |  207 ++
 6 files changed, 227 insertions(+), 7 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr86379.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index dada3a6aa410..44a3620a539f 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -3293,7 +3293,7 @@ struct GTY(()) lang_decl {
 #define DECL_DEPENDENT_P(NODE) DECL_LANG_FLAG_0 (USING_DECL_CHECK (NODE))
 
 /* The scope named in a using decl.  */
-#define USING_DECL_SCOPE(NODE) TREE_TYPE (USING_DECL_CHECK (NODE))
+#define USING_DECL_SCOPE(NODE) DECL_RESULT_FLD (USING_DECL_CHECK (NODE))
 
 /* The decls named by a using decl.  */
 #define USING_DECL_DECLS(NODE) DECL_INITIAL (USING_DECL_CHECK (NODE))
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 65ba812deb67..373b79b844dc 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -13297,7 +13297,8 @@ grok_special_member_properties (tree decl)
 {
   tree class_type;
 
-  if (!DECL_NONSTATIC_MEMBER_FUNCTION_P (decl))
+  if (TREE_CODE (decl) == USING_DECL
+  || !DECL_NONSTATIC_MEMBER_FUNCTION_P (decl))
 return;
 
   class_type = DECL_CONTEXT (decl);
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index d7b9029b0a3a..959f43b02384 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -2100,7 +2100,7 @@ strip_using_decl (tree decl)

Re: [PATCH] Fix an ICE when compiling Go with LTO (PR go/89019)

[Ian Lance Taylor]

On Tue, Feb 5, 2019 at 10:46 AM Nikhil Benesch  wrote:
>
> Ian—is there anything preventing this from getting merged? (I don't have
> write access.)

Thanks, committed now.

Ian

> On Thu, Jan 24, 2019 at 11:31 AM Nikhil Benesch 
> wrote:
>
> > On Thu, Jan 24, 2019 at 10:15 AM Richard Biener
> >  wrote:
> > >
> > > Ah, I missed that pt is probably a pointer type as well, then the code
> > > simply aligns the pointed-to type (note dependent on how pt was built
> > > this seems prone to wreck the TYPE_POINTER_TO/TYPE_NEXT_PTR_TO
> > > chains which link pointer types to a type.
> >
> > Right. In fact, set_placeholder_pointer_type asserts that both pt and tt
> > are pointer types.
> >
> > It's true that, after a call to set_placeholder_pointer_type, pt becomes
> > a distinct pointer type to T, yet is not part of the
> > TYPE_POINTER_TO/TYPE_NEXT_PTR_TO chain for T. As far as I can tell,
> > that's fine. The chain for T remains intact, as placeholder pointer
> > types are careful to always point to a distinct dummy object (see the
> > Gcc_backend::placeholder_pointer_type method) that nothing cares about.
> > The only consequence I see is the increased memory usage of having
> > multiple distinct but semantically identical pointer types to T.
> >

[RS6000] Set uses_pic_offset_table on sysv secure-plt calls and tls.

[Alan Modra]

Segher, you'll recognize these as your patches from pr88343.  All I've
done here is give you a testcase for the legitimize_tls_address change
(which you said you had no idea what the patch was for!)

Fixes lack of r30 save/restore on powerpc-linux.

// -m32 -fpic -ftls-model=initial-exec
__thread char* p;
char** f1 (void) { return &p; }

and

// -m32 -fpic -msecure-plt
extern int foo (int);
int f1 (int x) { return foo (x); }

PR target/88343
* config/rs6000/rs6000.c (rs6000_legitimize_tls_address),
(rs6000_call_sysv): Set uses_pic_offset_table.

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 883361cabbe..ab01dee9b68 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -8705,7 +8705,10 @@ rs6000_legitimize_tls_address (rtx addr, enum tls_model 
model)
   else
{
  if (flag_pic == 1)
-   got = gen_rtx_REG (Pmode, RS6000_PIC_OFFSET_TABLE_REGNUM);
+   {
+ crtl->uses_pic_offset_table = 1;
+ got = gen_rtx_REG (Pmode, RS6000_PIC_OFFSET_TABLE_REGNUM);
+   }
  else
{
  rtx gsym = rs6000_got_sym ();
@@ -38068,7 +38071,10 @@ rs6000_call_sysv (rtx value, rtx func_desc, rtx 
tlsarg, rtx cookie)
   && (!SYMBOL_REF_LOCAL_P (func_addr)
  || SYMBOL_REF_EXTERNAL_P (func_addr)
  || SYMBOL_REF_WEAK (func_addr)))
-call[n++] = gen_rtx_USE (VOIDmode, pic_offset_table_rtx);
+{
+  crtl->uses_pic_offset_table = 1;
+  call[n++] = gen_rtx_USE (VOIDmode, pic_offset_table_rtx);
+}
 
   call[n++] = gen_hard_reg_clobber (Pmode, LR_REGNO);
 

-- 
Alan Modra
Australia Development Lab, IBM