Re: [PATCH] Change default of param not being smaller that min.

2017-02-21 Thread Martin Liška
On 02/21/2017 12:03 AM, Alexandre Oliva wrote:
> It's convenient to debug -fcompare-debug errors.  You set the param to a
> large number, and then non-debug insns will get the same uid in both
> debug and non-debug compilations, so it's easier to compare RTL dumps
> (using the attached gcc-diff-dump; I thought it was in contrib, but it's
> not; should I put it there?) and locate the differences.

Yes, please commit the script, I'm interested in :)

There's second version, which changes default to 0.
Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

Ready to be installed?
Martin
>From c4d196cb9ad7ace112e9013403323fea3ead69fc Mon Sep 17 00:00:00 2001
From: marxin 
Date: Fri, 17 Feb 2017 16:00:30 +0100
Subject: [PATCH] Change default of param not being smaller that min.

gcc/ChangeLog:

2017-02-17  Martin Liska  

	* params.def (PARAM_MIN_NONDEBUG_INSN_UID): Change default to 0.
---
 gcc/params.def | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/params.def b/gcc/params.def
index 023ca727648..31847efbc3b 100644
--- a/gcc/params.def
+++ b/gcc/params.def
@@ -965,7 +965,7 @@ DEFPARAM (PARAM_MAX_VARTRACK_REVERSE_OP_SIZE,
 DEFPARAM (PARAM_MIN_NONDEBUG_INSN_UID,
 	  "min-nondebug-insn-uid",
 	  "The minimum UID to be used for a nondebug insn.",
-	  0, 1, 0)
+	  0, 0, 0)
 
 DEFPARAM (PARAM_IPA_SRA_PTR_GROWTH_FACTOR,
 	  "ipa-sra-ptr-growth-factor",
-- 
2.11.0



[PATCH] Remove wrong assert about gcov_type (PR lto/79587).

2017-02-21 Thread Martin Liška
Hello.

As described in the PR, we represent unsigned long types as gcov_type in 
compiler.
That can lead to a negative value in context of GCOV_SINGLE_VALUE (coming from 
DIV/MOV expression).
Thus, we have to relax the assert.

Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. And 
Python can be bootstrapped
with the patch.

Ready to be installed?
Martin
>From ccee30401dc59b41f962330b5f5390075813be14 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Mon, 20 Feb 2017 15:55:08 +0100
Subject: [PATCH] Remove wrong assert about gcov_type (PR lto/79587).

gcc/ChangeLog:

2017-02-20  Martin Liska  

	PR lto/79587
	* data-streamer-in.c (streamer_read_gcov_count): Remove assert.
	* data-streamer-out.c (streamer_write_gcov_count_stream):
	Likewise.
	* value-prof.c (stream_out_histogram_value): Make assert more
	precise based on type of counter.

gcc/testsuite/ChangeLog:

2017-02-20  Martin Liska  

	PR lto/79587
	* gcc.dg/tree-prof/pr79587.c: New test.
---
 gcc/data-streamer-in.c   |  1 -
 gcc/data-streamer-out.c  |  1 -
 gcc/testsuite/gcc.dg/tree-prof/pr79587.c | 26 ++
 gcc/value-prof.c | 12 +++-
 4 files changed, 37 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-prof/pr79587.c

diff --git a/gcc/data-streamer-in.c b/gcc/data-streamer-in.c
index b38d91353fc..ad32ce1e99a 100644
--- a/gcc/data-streamer-in.c
+++ b/gcc/data-streamer-in.c
@@ -181,7 +181,6 @@ gcov_type
 streamer_read_gcov_count (struct lto_input_block *ib)
 {
   gcov_type ret = streamer_read_hwi (ib);
-  gcc_assert (ret >= 0);
   return ret;
 }
 
diff --git a/gcc/data-streamer-out.c b/gcc/data-streamer-out.c
index 1ee8c9f53af..1e8feb1e74a 100644
--- a/gcc/data-streamer-out.c
+++ b/gcc/data-streamer-out.c
@@ -340,7 +340,6 @@ streamer_write_hwi_stream (struct lto_output_stream *obs, HOST_WIDE_INT work)
 void
 streamer_write_gcov_count_stream (struct lto_output_stream *obs, gcov_type work)
 {
-  gcc_assert (work >= 0);
   gcc_assert ((HOST_WIDE_INT) work == work);
   streamer_write_hwi_stream (obs, work);
 }
diff --git a/gcc/testsuite/gcc.dg/tree-prof/pr79587.c b/gcc/testsuite/gcc.dg/tree-prof/pr79587.c
new file mode 100644
index 000..517e0819919
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-prof/pr79587.c
@@ -0,0 +1,26 @@
+/* { dg-require-effective-target lto } */
+/* { dg-options "-O2 -flto" } */
+
+unsigned long global = -12345;
+
+unsigned long
+__attribute__((noinline))
+test(unsigned long v, unsigned long v2)
+{
+  unsigned long x = v % v2;
+
+  return x;
+}
+
+int main(int argc, char **argv)
+{
+  unsigned long r = 0;
+
+  for (int i = 0; i < 100; i++)
+r += test(argc, global);
+
+  if (r != 100)
+__builtin_abort ();
+
+  return 0;
+}
diff --git a/gcc/value-prof.c b/gcc/value-prof.c
index 097e4094095..22dc2c9e257 100644
--- a/gcc/value-prof.c
+++ b/gcc/value-prof.c
@@ -365,7 +365,17 @@ stream_out_histogram_value (struct output_block *ob, histogram_value hist)
   break;
 }
   for (i = 0; i < hist->n_counters; i++)
-streamer_write_gcov_count (ob, hist->hvalue.counters[i]);
+{
+  /* When user uses an unsigned type with a big value, constant converted
+	 to gcov_type (a signed type) can be negative.  */
+  gcov_type value = hist->hvalue.counters[i];
+  if (hist->type == HIST_TYPE_SINGLE_VALUE && i == 0)
+	;
+  else
+	gcc_assert (value >= 0);
+
+  streamer_write_gcov_count (ob, value);
+}
   if (hist->hvalue.next)
 stream_out_histogram_value (ob, hist->hvalue.next);
 }
-- 
2.11.0



Re: [PATCH 3/5] Support WORD_REGISTER_OPERATIONS requirements in simplify_operand_subreg

2017-02-21 Thread Eric Botcazou
> The condition would look like this, What do you think?
> 
>   if (!(GET_MODE_PRECISION (mode) != GET_MODE_PRECISION (innermode)
> && GET_MODE_SIZE (mode) <= UNITS_PER_WORD
> && GET_MODE_SIZE (innermode) <= UNITS_PER_WORD
> && WORD_REGISTER_OPERATIONS)
>   && (!(MEM_ALIGN (subst) < GET_MODE_ALIGNMENT (mode)
> && SLOW_UNALIGNED_ACCESS (mode, MEM_ALIGN (subst)))
> 
>   || (MEM_ALIGN (reg) < GET_MODE_ALIGNMENT (innermode)
> 
>   && SLOW_UNALIGNED_ACCESS (innermode, MEM_ALIGN
> (reg) return true;

OK, I'm going to give it a try on SPARC.

-- 
Eric Botcazou


Re: [GIMPLE FE] add fma_expr

2017-02-21 Thread Richard Biener
On Mon, 20 Feb 2017, Kyrill Tkachov wrote:

> Hi Prathamesh,
> 
> On 16/02/17 12:47, Prathamesh Kulkarni wrote:
> > Hi Richard,
> > The attached patch handles fma_expr in gimple-fe.
> > Does it look OK ?
> > 
> > Thanks,
> > Prathamesh
> 
> I see the new test ICEing on aarch64-none-elf.

FMA_EXPR relies on either target support or library support so the 
testcase should be appropriately limited to targets supporting that.
{ target c99_runtime } maybe.

Richard.

> Thanks,
> Kyrill
> 
> --
> 
> $DIR/build-aarch64/obj/gcc2/gcc/xgcc -B$DIR/build-aarch64/obj/gcc2/gcc/
> $DIR/gcc/gcc/testsuite/gcc.dg/gimplefe-26.c -fno-diagnos
> tics-show-caret -fdiagnostics-color=never -O -fgimple -fdump-tree-ssa-gimple
> -S -specs=aem-ve.specs -o gimplefe-26.s
> $DIR/gcc/gcc/testsuite/gcc.dg/gimplefe-26.c: In function 'foo_3':
> $DIR/gcc/gcc/testsuite/gcc.dg/gimplefe-26.c:5:18: internal compiler error: in
> expand_expr_real_2, at expr.c:8705
> $DIR/gcc/gcc/testsuite/gcc.dg/gimplefe-26.c:14:1: note: in expansion of macro
> 'foo'
> 0x84e7a3 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode,
> expand_modifier)
> $DIR/gcc/gcc/expr.c:8705
> 0x838bd6 expand_expr_real_1(tree_node*, rtx_def*, machine_mode,
> expand_modifier, rtx_def**, bool)
> $DIR/gcc/gcc/expr.c:9730
> 0x83f0f5 expand_expr_real(tree_node*, rtx_def*, machine_mode, expand_modifier,
> rtx_def**, bool)
> $DIR/gcc/gcc/expr.c:8072
> 0x73ab2f expand_expr
> $DIR/gcc/gcc/expr.h:276
> 0x73ab2f expand_return
> $DIR/gcc/gcc/cfgexpand.c:3526
> 0x73ab2f expand_gimple_stmt_1
> $DIR/gcc/gcc/cfgexpand.c:3610
> 0x73ab2f expand_gimple_stmt
> $DIR/gcc/gcc/cfgexpand.c:3737
> 0x73d55b expand_gimple_basic_block
> $DIR/gcc/gcc/cfgexpand.c:5744
> 0x740b14 execute
> $DIR/gcc/gcc/cfgexpand.c:6357
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See  for instructions.
> 
> 

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


Re: [GIMPLE FE] add fma_expr

2017-02-21 Thread Kyrill Tkachov


On 21/02/17 08:43, Richard Biener wrote:

On Mon, 20 Feb 2017, Kyrill Tkachov wrote:


Hi Prathamesh,

On 16/02/17 12:47, Prathamesh Kulkarni wrote:

Hi Richard,
The attached patch handles fma_expr in gimple-fe.
Does it look OK ?

Thanks,
Prathamesh

I see the new test ICEing on aarch64-none-elf.

FMA_EXPR relies on either target support or library support so the
testcase should be appropriately limited to targets supporting that.
{ target c99_runtime } maybe.


That does skip it on aarch64-none-elf, but is ICEing the expected behaviour in 
this case?
Shouldn't the frontend give an error message of some kind?
Or is the gimple frontend considered an internal feature and any errors in 
input are expected
to cause an ICE?

Thanks,
Kyrill


Richard.


Thanks,
Kyrill

--

$DIR/build-aarch64/obj/gcc2/gcc/xgcc -B$DIR/build-aarch64/obj/gcc2/gcc/
$DIR/gcc/gcc/testsuite/gcc.dg/gimplefe-26.c -fno-diagnos
tics-show-caret -fdiagnostics-color=never -O -fgimple -fdump-tree-ssa-gimple
-S -specs=aem-ve.specs -o gimplefe-26.s
$DIR/gcc/gcc/testsuite/gcc.dg/gimplefe-26.c: In function 'foo_3':
$DIR/gcc/gcc/testsuite/gcc.dg/gimplefe-26.c:5:18: internal compiler error: in
expand_expr_real_2, at expr.c:8705
$DIR/gcc/gcc/testsuite/gcc.dg/gimplefe-26.c:14:1: note: in expansion of macro
'foo'
0x84e7a3 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode,
expand_modifier)
 $DIR/gcc/gcc/expr.c:8705
0x838bd6 expand_expr_real_1(tree_node*, rtx_def*, machine_mode,
expand_modifier, rtx_def**, bool)
 $DIR/gcc/gcc/expr.c:9730
0x83f0f5 expand_expr_real(tree_node*, rtx_def*, machine_mode, expand_modifier,
rtx_def**, bool)
 $DIR/gcc/gcc/expr.c:8072
0x73ab2f expand_expr
 $DIR/gcc/gcc/expr.h:276
0x73ab2f expand_return
 $DIR/gcc/gcc/cfgexpand.c:3526
0x73ab2f expand_gimple_stmt_1
 $DIR/gcc/gcc/cfgexpand.c:3610
0x73ab2f expand_gimple_stmt
 $DIR/gcc/gcc/cfgexpand.c:3737
0x73d55b expand_gimple_basic_block
 $DIR/gcc/gcc/cfgexpand.c:5744
0x740b14 execute
 $DIR/gcc/gcc/cfgexpand.c:6357
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See  for instructions.






Re: [GIMPLE FE] add fma_expr

2017-02-21 Thread Richard Biener
On Tue, 21 Feb 2017, Kyrill Tkachov wrote:

> 
> On 21/02/17 08:43, Richard Biener wrote:
> > On Mon, 20 Feb 2017, Kyrill Tkachov wrote:
> > 
> > > Hi Prathamesh,
> > > 
> > > On 16/02/17 12:47, Prathamesh Kulkarni wrote:
> > > > Hi Richard,
> > > > The attached patch handles fma_expr in gimple-fe.
> > > > Does it look OK ?
> > > > 
> > > > Thanks,
> > > > Prathamesh
> > > I see the new test ICEing on aarch64-none-elf.
> > FMA_EXPR relies on either target support or library support so the
> > testcase should be appropriately limited to targets supporting that.
> > { target c99_runtime } maybe.
> 
> That does skip it on aarch64-none-elf, but is ICEing the expected behaviour in
> this case?
> Shouldn't the frontend give an error message of some kind?

Well, IMHO it's not the purpose of the FE to reject all invalid IL.  We
have verifiers for this.

> Or is the gimple frontend considered an internal feature and any errors in
> input are expected
> to cause an ICE?

Yes, mostly.  The FE itself shouldn't ICE.

Richard.

> Thanks,
> Kyrill
> 
> > Richard.
> > 
> > > Thanks,
> > > Kyrill
> > > 
> > > --
> > > 
> > > $DIR/build-aarch64/obj/gcc2/gcc/xgcc -B$DIR/build-aarch64/obj/gcc2/gcc/
> > > $DIR/gcc/gcc/testsuite/gcc.dg/gimplefe-26.c -fno-diagnos
> > > tics-show-caret -fdiagnostics-color=never -O -fgimple
> > > -fdump-tree-ssa-gimple
> > > -S -specs=aem-ve.specs -o gimplefe-26.s
> > > $DIR/gcc/gcc/testsuite/gcc.dg/gimplefe-26.c: In function 'foo_3':
> > > $DIR/gcc/gcc/testsuite/gcc.dg/gimplefe-26.c:5:18: internal compiler error:
> > > in
> > > expand_expr_real_2, at expr.c:8705
> > > $DIR/gcc/gcc/testsuite/gcc.dg/gimplefe-26.c:14:1: note: in expansion of
> > > macro
> > > 'foo'
> > > 0x84e7a3 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode,
> > > expand_modifier)
> > >  $DIR/gcc/gcc/expr.c:8705
> > > 0x838bd6 expand_expr_real_1(tree_node*, rtx_def*, machine_mode,
> > > expand_modifier, rtx_def**, bool)
> > >  $DIR/gcc/gcc/expr.c:9730
> > > 0x83f0f5 expand_expr_real(tree_node*, rtx_def*, machine_mode,
> > > expand_modifier,
> > > rtx_def**, bool)
> > >  $DIR/gcc/gcc/expr.c:8072
> > > 0x73ab2f expand_expr
> > >  $DIR/gcc/gcc/expr.h:276
> > > 0x73ab2f expand_return
> > >  $DIR/gcc/gcc/cfgexpand.c:3526
> > > 0x73ab2f expand_gimple_stmt_1
> > >  $DIR/gcc/gcc/cfgexpand.c:3610
> > > 0x73ab2f expand_gimple_stmt
> > >  $DIR/gcc/gcc/cfgexpand.c:3737
> > > 0x73d55b expand_gimple_basic_block
> > >  $DIR/gcc/gcc/cfgexpand.c:5744
> > > 0x740b14 execute
> > >  $DIR/gcc/gcc/cfgexpand.c:6357
> > > Please submit a full bug report,
> > > with preprocessed source if appropriate.
> > > Please include the complete backtrace with any bug report.
> > > See  for instructions.
> > > 
> > > 
> 
> 

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


Re: [PATCH] Look through clobber stmts in uninit (PR tree-optimization/79345)

2017-02-21 Thread Richard Biener
On Mon, 20 Feb 2017, Jakub Jelinek wrote:

> Hi!
> 
> As mentioned by Jason in the PR, we've regressed on the following testcase
> since we started emitting CLOBBERs at the start of ctors (and we warn as
> before with -fno-lifetime-dse -Wuninitialized).
> With -fno-lifetime-dse, the vuse on the b.x read stmt is default def
> and thus we warn, but if there are clobbers before that, that is not the
> case and we don't warn.  The patch is quick hack to bypass the initial
> clobbers as long as there aren't really many.  If we wanted to handle
> all initial clobbers, I bet the first time we run into this we could
> recursively walk vop uses from the default def and build say a bitmap
> of vop SSA_NAMEs which are vdefs of clobbers that only have clobbers
> before it as vdef stmts.
> 
> Now, the comment says:
>   /* For memory the only cheap thing we can do is see if we
>  have a use of the default def of the virtual operand.
>  ???  Not so cheap would be to use the alias oracle via
>  walk_aliased_vdefs, if we don't find any aliasing vdef
>  warn as is-used-uninitialized, if we don't find an aliasing
>  vdef that kills our use (stmt_kills_ref_p), warn as
>  may-be-used-uninitialized.  But this walk is quadratic and
>  so must be limited which means we would miss warning
>  opportunities.  */
> I wonder if it isn't useful to walk even limited number of vdefs this way
> anyway (likely GCC8 material though), e.g. if we run into a clobber that
> must (rather than just may) portion of the read ref (and of course when
> seeing non-clobber stmt that could alias with the ref give up before that),
> we could warn even if we are very far from the start of the function.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk (or do
> you want a version with a bitmap and really ignoring all the clobbers,
> rather than just 128 of them)?

I'd rather not special-case clobbers but take this as an opportunity
to implement that ??? comment with walk_aliased_vdefs (you can do
some copy&paste programming from the use in tree-ssa-dce.c I think).
Just use a very low walking limit.

As you say, a kill from a clobber should warn (you skip them).

Richard.

> 2017-02-20  Jakub Jelinek  
> 
>   PR tree-optimization/79345
>   * tree-ssa-uninit.c (warn_uninitialized_vars): If vuse is not
>   default def, but there are only CLOBBER stmts as vdefs, handle it like
>   default def.
> 
>   * g++.dg/warn/pr79345.C: New test.
> 
> --- gcc/tree-ssa-uninit.c.jj  2017-01-01 12:45:35.0 +0100
> +++ gcc/tree-ssa-uninit.c 2017-02-20 16:43:08.244868075 +0100
> @@ -248,8 +248,7 @@ warn_uninitialized_vars (bool warn_possi
> use = gimple_vuse (stmt);
> if (use
> && gimple_assign_single_p (stmt)
> -   && !gimple_vdef (stmt)
> -   && SSA_NAME_IS_DEFAULT_DEF (use))
> +   && !gimple_vdef (stmt))
>   {
> tree rhs = gimple_assign_rhs1 (stmt);
> tree base = get_base_address (rhs);
> @@ -260,6 +259,23 @@ warn_uninitialized_vars (bool warn_possi
> || is_global_var (base))
>   continue;
>  
> +   /* Look through some CLOBBER stmts.  */
> +   for (unsigned int cnt = 0; cnt < 128 && use; cnt++)
> + {
> +   if (SSA_NAME_IS_DEFAULT_DEF (use))
> + break;
> +
> +   gimple *def_stmt = SSA_NAME_DEF_STMT (use);
> +   if (!gimple_clobber_p (def_stmt))
> + {
> +   use = NULL_TREE;
> +   break;
> + }
> +   use = gimple_vuse (def_stmt);
> + }
> +   if (use == NULL_TREE)
> + continue;
> +
> if (always_executed)
>   warn_uninit (OPT_Wuninitialized, use, gimple_assign_rhs1 (stmt),
>base, "%qE is used uninitialized in this function",
> --- gcc/testsuite/g++.dg/warn/pr79345.C.jj2017-02-20 17:19:01.138952915 
> +0100
> +++ gcc/testsuite/g++.dg/warn/pr79345.C   2017-02-20 17:18:20.0 
> +0100
> @@ -0,0 +1,22 @@
> +// PR tree-optimization/79345
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-O2 -Wuninitialized" }
> +
> +struct A {
> +  A (int);
> +};
> +
> +struct B : A {
> +  const bool x = true;
> +
> +  B () : A (x ? 3 : 7) { }   // { dg-warning "is used uninitialized in this 
> function" }
> +};
> +
> +void foo (void*);
> +
> +void
> +bar ()
> +{
> +  B b;
> +  foo (&b);
> +}
> 
>   Jakub
> 
> 

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


Re: [PATCH] Look through clobber stmts in uninit (PR tree-optimization/79345)

2017-02-21 Thread Richard Biener
On Mon, 20 Feb 2017, Marc Glisse wrote:

> On Mon, 20 Feb 2017, Jakub Jelinek wrote:
> 
> > As mentioned by Jason in the PR, we've regressed on the following testcase
> > since we started emitting CLOBBERs at the start of ctors (and we warn as
> > before with -fno-lifetime-dse -Wuninitialized).
> > With -fno-lifetime-dse, the vuse on the b.x read stmt is default def
> > and thus we warn, but if there are clobbers before that, that is not the
> > case and we don't warn.  The patch is quick hack to bypass the initial
> > clobbers as long as there aren't really many.  If we wanted to handle
> > all initial clobbers, I bet the first time we run into this we could
> > recursively walk vop uses from the default def and build say a bitmap
> > of vop SSA_NAMEs which are vdefs of clobbers that only have clobbers
> > before it as vdef stmts.
> 
>   MEM[(struct  &)&b] ={v} {CLOBBER};
>   _4 = b.x;
>   if (_4 != 0)
> 
> It would be nice (at some point in a distant future) to turn that into
> 
>   MEM[(struct  &)&b] ={v} {CLOBBER};
>   if (_4(D) != 0)
> 
> i.e. replace reads from clobbered memory with a default def (with a gimple_nop
> defining statement).
> 
> IIRC I tried to do it in FRE once, but failed.

Yeah, FRE isn't too happy about VN_TOP in the lattice (which is what
you'd use here).  I also have/had patches trying to make it (more) happy
about VN_TOP but it still broke stuff like for example tail-merging...

Need to dissect PRE from tail-merging again, and also possibly PRE
from value-numbering (run eliminate() before doing PRE).

Richard.

> > Now, the comment says:
> >  /* For memory the only cheap thing we can do is see if we
> > have a use of the default def of the virtual operand.
> > ???  Not so cheap would be to use the alias oracle via
> > walk_aliased_vdefs, if we don't find any aliasing vdef
> > warn as is-used-uninitialized, if we don't find an aliasing
> > vdef that kills our use (stmt_kills_ref_p), warn as
> > may-be-used-uninitialized.  But this walk is quadratic and
> > so must be limited which means we would miss warning
> > opportunities.  */
> > I wonder if it isn't useful to walk even limited number of vdefs this way
> 
> Seems worth a try for gcc8, there are several PRs it could help with.
> 
> > anyway (likely GCC8 material though), e.g. if we run into a clobber that
> > must (rather than just may) portion of the read ref (and of course when
> > seeing non-clobber stmt that could alias with the ref give up before that),
> > we could warn even if we are very far from the start of the function.
> 
> 

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


[PATCH] Fix typo in -masm-dialect option values.

2017-02-21 Thread Martin Liška
Hello.

The patch is obvious, however should I also replace the option in *.po files:

gcc/po/be.po:msgid "Known assembler dialects (for use with the -masm-dialect= 
option):"
gcc/po/da.po:msgid "Known assembler dialects (for use with the -masm-dialect= 
option):"
gcc/po/de.po:msgid "Known assembler dialects (for use with the -masm-dialect= 
option):"
gcc/po/de.po:msgstr "Bekannte Assemblerdialekte (für Verwendung mit Option 
-masm-dialect=):"
gcc/po/el.po:msgid "Known assembler dialects (for use with the -masm-dialect= 
option):"
gcc/po/es.po:msgid "Known assembler dialects (for use with the -masm-dialect= 
option):"
gcc/po/es.po:msgstr "Dialectos de ensamblador conocidos (para uso con la opción 
-masm-dialect=):"
...

Thanks,
Martin
>From 3fed8dfda9f63de9dce96754b09214e8c31d7fa7 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Tue, 21 Feb 2017 10:42:43 +0100
Subject: [PATCH] Fix typo in -masm-dialect option values.

gcc/ChangeLog:

2017-02-21  Martin Liska  

	* config/i386/i386.opt: Replace -masm-dialect with -masm.
---
 gcc/config/i386/i386.opt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/config/i386/i386.opt b/gcc/config/i386/i386.opt
index 71100d98708..36251abfeb5 100644
--- a/gcc/config/i386/i386.opt
+++ b/gcc/config/i386/i386.opt
@@ -258,7 +258,7 @@ Use given assembler dialect.
 
 Enum
 Name(asm_dialect) Type(enum asm_dialect)
-Known assembler dialects (for use with the -masm-dialect= option):
+Known assembler dialects (for use with the -masm= option):
 
 EnumValue
 Enum(asm_dialect) String(intel) Value(ASM_INTEL)
-- 
2.11.0



Re: [PATCH] Fix typo in -masm-dialect option values.

2017-02-21 Thread Jakub Jelinek
On Tue, Feb 21, 2017 at 10:44:40AM +0100, Martin Liška wrote:
> The patch is obvious, however should I also replace the option in *.po files:

Ok for trunk.

> gcc/po/be.po:msgid "Known assembler dialects (for use with the -masm-dialect= 
> option):"
> gcc/po/da.po:msgid "Known assembler dialects (for use with the -masm-dialect= 
> option):"
> gcc/po/de.po:msgid "Known assembler dialects (for use with the -masm-dialect= 
> option):"
> gcc/po/de.po:msgstr "Bekannte Assemblerdialekte (für Verwendung mit Option 
> -masm-dialect=):"
> gcc/po/el.po:msgid "Known assembler dialects (for use with the -masm-dialect= 
> option):"
> gcc/po/es.po:msgid "Known assembler dialects (for use with the -masm-dialect= 
> option):"
> gcc/po/es.po:msgstr "Dialectos de ensamblador conocidos (para uso con la 
> opción -masm-dialect=):"

According to Joseph, the policy is we never do that ourselves, the *.po
files are owned by the translation team.

Jakub


Re: [GIMPLE FE] add fma_expr

2017-02-21 Thread Kyrill Tkachov


On 21/02/17 08:56, Richard Biener wrote:

On Tue, 21 Feb 2017, Kyrill Tkachov wrote:


On 21/02/17 08:43, Richard Biener wrote:

On Mon, 20 Feb 2017, Kyrill Tkachov wrote:


Hi Prathamesh,

On 16/02/17 12:47, Prathamesh Kulkarni wrote:

Hi Richard,
The attached patch handles fma_expr in gimple-fe.
Does it look OK ?

Thanks,
Prathamesh

I see the new test ICEing on aarch64-none-elf.

FMA_EXPR relies on either target support or library support so the
testcase should be appropriately limited to targets supporting that.
{ target c99_runtime } maybe.

That does skip it on aarch64-none-elf, but is ICEing the expected behaviour in
this case?
Shouldn't the frontend give an error message of some kind?

Well, IMHO it's not the purpose of the FE to reject all invalid IL.  We
have verifiers for this.


Ok, here's the patch to add the target selector.
Newlib doesn't implement fmal for long double I guess (it's the long double fma 
that fails here).
Is this ok?

Thanks,
Kyrill

2016-02-21  Kyrylo Tkachov  

* gcc.dg/gimplefe-26.c: Require c99_runtime.



Or is the gimple frontend considered an internal feature and any errors in
input are expected
to cause an ICE?

Yes, mostly.  The FE itself shouldn't ICE.

Richard.


Thanks,
Kyrill


Richard.


Thanks,
Kyrill

--

$DIR/build-aarch64/obj/gcc2/gcc/xgcc -B$DIR/build-aarch64/obj/gcc2/gcc/
$DIR/gcc/gcc/testsuite/gcc.dg/gimplefe-26.c -fno-diagnos
tics-show-caret -fdiagnostics-color=never -O -fgimple
-fdump-tree-ssa-gimple
-S -specs=aem-ve.specs -o gimplefe-26.s
$DIR/gcc/gcc/testsuite/gcc.dg/gimplefe-26.c: In function 'foo_3':
$DIR/gcc/gcc/testsuite/gcc.dg/gimplefe-26.c:5:18: internal compiler error:
in
expand_expr_real_2, at expr.c:8705
$DIR/gcc/gcc/testsuite/gcc.dg/gimplefe-26.c:14:1: note: in expansion of
macro
'foo'
0x84e7a3 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode,
expand_modifier)
  $DIR/gcc/gcc/expr.c:8705
0x838bd6 expand_expr_real_1(tree_node*, rtx_def*, machine_mode,
expand_modifier, rtx_def**, bool)
  $DIR/gcc/gcc/expr.c:9730
0x83f0f5 expand_expr_real(tree_node*, rtx_def*, machine_mode,
expand_modifier,
rtx_def**, bool)
  $DIR/gcc/gcc/expr.c:8072
0x73ab2f expand_expr
  $DIR/gcc/gcc/expr.h:276
0x73ab2f expand_return
  $DIR/gcc/gcc/cfgexpand.c:3526
0x73ab2f expand_gimple_stmt_1
  $DIR/gcc/gcc/cfgexpand.c:3610
0x73ab2f expand_gimple_stmt
  $DIR/gcc/gcc/cfgexpand.c:3737
0x73d55b expand_gimple_basic_block
  $DIR/gcc/gcc/cfgexpand.c:5744
0x740b14 execute
  $DIR/gcc/gcc/cfgexpand.c:6357
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See  for instructions.






diff --git a/gcc/testsuite/gcc.dg/gimplefe-26.c b/gcc/testsuite/gcc.dg/gimplefe-26.c
index 55a4624..bc2f3b1 100644
--- a/gcc/testsuite/gcc.dg/gimplefe-26.c
+++ b/gcc/testsuite/gcc.dg/gimplefe-26.c
@@ -1,4 +1,4 @@
-/* { dg-do compile } */
+/* { dg-do compile { target c99_runtime } } */
 /* { dg-options "-O -fgimple -fdump-tree-ssa-gimple" } */
 
 #define foo(type, num) \


Re: [GIMPLE FE] add fma_expr

2017-02-21 Thread Richard Biener
On Tue, 21 Feb 2017, Kyrill Tkachov wrote:

> 
> On 21/02/17 08:56, Richard Biener wrote:
> > On Tue, 21 Feb 2017, Kyrill Tkachov wrote:
> > 
> > > On 21/02/17 08:43, Richard Biener wrote:
> > > > On Mon, 20 Feb 2017, Kyrill Tkachov wrote:
> > > > 
> > > > > Hi Prathamesh,
> > > > > 
> > > > > On 16/02/17 12:47, Prathamesh Kulkarni wrote:
> > > > > > Hi Richard,
> > > > > > The attached patch handles fma_expr in gimple-fe.
> > > > > > Does it look OK ?
> > > > > > 
> > > > > > Thanks,
> > > > > > Prathamesh
> > > > > I see the new test ICEing on aarch64-none-elf.
> > > > FMA_EXPR relies on either target support or library support so the
> > > > testcase should be appropriately limited to targets supporting that.
> > > > { target c99_runtime } maybe.
> > > That does skip it on aarch64-none-elf, but is ICEing the expected
> > > behaviour in
> > > this case?
> > > Shouldn't the frontend give an error message of some kind?
> > Well, IMHO it's not the purpose of the FE to reject all invalid IL.  We
> > have verifiers for this.
> 
> Ok, here's the patch to add the target selector.
> Newlib doesn't implement fmal for long double I guess (it's the long double
> fma that fails here).
> Is this ok?

Yes.

Thanks,
Richard.

> Thanks,
> Kyrill
> 
> 2016-02-21  Kyrylo Tkachov  
> 
> * gcc.dg/gimplefe-26.c: Require c99_runtime.
> 
> 
> > > Or is the gimple frontend considered an internal feature and any errors in
> > > input are expected
> > > to cause an ICE?
> > Yes, mostly.  The FE itself shouldn't ICE.
> > 
> > Richard.
> > 
> > > Thanks,
> > > Kyrill
> > > 
> > > > Richard.
> > > > 
> > > > > Thanks,
> > > > > Kyrill
> > > > > 
> > > > > --
> > > > > 
> > > > > $DIR/build-aarch64/obj/gcc2/gcc/xgcc
> > > > > -B$DIR/build-aarch64/obj/gcc2/gcc/
> > > > > $DIR/gcc/gcc/testsuite/gcc.dg/gimplefe-26.c -fno-diagnos
> > > > > tics-show-caret -fdiagnostics-color=never -O -fgimple
> > > > > -fdump-tree-ssa-gimple
> > > > > -S -specs=aem-ve.specs -o gimplefe-26.s
> > > > > $DIR/gcc/gcc/testsuite/gcc.dg/gimplefe-26.c: In function 'foo_3':
> > > > > $DIR/gcc/gcc/testsuite/gcc.dg/gimplefe-26.c:5:18: internal compiler
> > > > > error:
> > > > > in
> > > > > expand_expr_real_2, at expr.c:8705
> > > > > $DIR/gcc/gcc/testsuite/gcc.dg/gimplefe-26.c:14:1: note: in expansion
> > > > > of
> > > > > macro
> > > > > 'foo'
> > > > > 0x84e7a3 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode,
> > > > > expand_modifier)
> > > > >   $DIR/gcc/gcc/expr.c:8705
> > > > > 0x838bd6 expand_expr_real_1(tree_node*, rtx_def*, machine_mode,
> > > > > expand_modifier, rtx_def**, bool)
> > > > >   $DIR/gcc/gcc/expr.c:9730
> > > > > 0x83f0f5 expand_expr_real(tree_node*, rtx_def*, machine_mode,
> > > > > expand_modifier,
> > > > > rtx_def**, bool)
> > > > >   $DIR/gcc/gcc/expr.c:8072
> > > > > 0x73ab2f expand_expr
> > > > >   $DIR/gcc/gcc/expr.h:276
> > > > > 0x73ab2f expand_return
> > > > >   $DIR/gcc/gcc/cfgexpand.c:3526
> > > > > 0x73ab2f expand_gimple_stmt_1
> > > > >   $DIR/gcc/gcc/cfgexpand.c:3610
> > > > > 0x73ab2f expand_gimple_stmt
> > > > >   $DIR/gcc/gcc/cfgexpand.c:3737
> > > > > 0x73d55b expand_gimple_basic_block
> > > > >   $DIR/gcc/gcc/cfgexpand.c:5744
> > > > > 0x740b14 execute
> > > > >   $DIR/gcc/gcc/cfgexpand.c:6357
> > > > > Please submit a full bug report,
> > > > > with preprocessed source if appropriate.
> > > > > Please include the complete backtrace with any bug report.
> > > > > See  for instructions.
> > > > > 
> > > > > 
> > > 
> 
> 

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


loop versioning doc changes

2017-02-21 Thread Aldy Hernandez
Found these while poking at loop versioning.  Most importantly, we no 
longer have tree_ssa_loop_version, so I removed it from the docs.


OK?
gcc/

* doc/loop.texi (Loop manipulation): Remove nonexistent
tree_ssa_loop_version from the documentation.
* cfgloopmanip.c (loop_version): Document CONDITION_BB argument.

diff --git a/gcc/cfgloopmanip.c b/gcc/cfgloopmanip.c
index a0e3e30..3e34aad 100644
--- a/gcc/cfgloopmanip.c
+++ b/gcc/cfgloopmanip.c
@@ -1689,10 +1689,13 @@ lv_adjust_loop_entry_edge (basic_block first_head, 
basic_block second_head,
This transformation given a condition and a loop, creates
-if (condition) { loop_copy1 } else { loop_copy2 },
where loop_copy1 is the loop transformed in one way, and loop_copy2
-   is the loop transformed in another way (or unchanged). 'condition'
+   is the loop transformed in another way (or unchanged). COND_EXPR
may be a run time test for things that were not resolved by static
analysis (overlapping ranges (anti-aliasing), alignment, etc.).
 
+   If non-NULL, CONDITION_BB is set to the basic block containing the
+   condition.
+
THEN_PROB is the probability of the then edge of the if.  THEN_SCALE
is the ratio by that the frequencies in the original loop should
be scaled.  ELSE_SCALE is the ratio by that the frequencies in the
diff --git a/gcc/doc/loop.texi b/gcc/doc/loop.texi
index b607e95..8535769 100644
--- a/gcc/doc/loop.texi
+++ b/gcc/doc/loop.texi
@@ -258,12 +258,11 @@ one of the edges entering loop header, thus performing 
either loop
 unrolling or loop peeling.  @code{can_duplicate_loop_p}
 (@code{can_unroll_loop_p} on GIMPLE) must be true for the duplicated
 loop.
-@item @code{loop_version}, @code{tree_ssa_loop_version}: These function
-create a copy of a loop, and a branch before them that selects one of
-them depending on the prescribed condition.  This is useful for
-optimizations that need to verify some assumptions in runtime (one of
-the copies of the loop is usually left unchanged, while the other one is
-transformed in some way).
+@item @code{loop_version}: This function creates a copy of a loop, and
+a branch before them that selects one of them depending on the
+prescribed condition.  This is useful for optimizations that need to
+verify some assumptions in runtime (one of the copies of the loop is
+usually left unchanged, while the other one is transformed in some way).
 @item @code{tree_unroll_loop}: Unrolls the loop, including peeling the
 extra iterations to make the number of iterations divisible by unroll
 factor, updating the exit condition, and removing the exits that now


RE: [PATCH, MIPS] Calling convention differs depending on the presence of MSA

2017-02-21 Thread Matthew Fortune
Sameera Deshpande  writes:
> Hi Matthew,
> 
> Please find attached updated patch as per our offline discussion.
> 
> I have disabled return in registers for all vector float types, and
> updated the test case accordingly.
> 
> Ok for trunk?

Hi Sameera,

Sorry for the slow response.  I'd like to explain my thinking here
as I believe your change is right but you are actually ending up
fixing more issues than you expected!

There are 3 potential vector modes that are affected by your change
rather than just 2.  V2SF (paired single), V4SF and V2DF (MSA).

The V4SF and V2DF changes are the ones needed for MSA as the test
case shows.  These become valid modes with -mmsa so the float
vector types no longer get assigned BLKmode. (As an aside I think
it is a legacy bug that this function was ever defined in terms of
modes for o32).

The V2SF is an interesting one as it would not historically have
been a valid mode for o32 until o32 FP64 was re-invented.  Now that
o32 FP64 is a compatible ABI extension we(I) have in fact allowed an
incompatible ABI change through as a vector of 2 floats will be
returned in registers for o32 FP64 and not the other o32 variants.

As an aside... The integer vector types up to 16-bytes get returned
in registers also potentially by chance because...

1) We have TImode support in the backend
2) TImode is 16-bytes and hence the type->mode logic will fall back
   to looking for a wider integer mode for integer vectors and find
   TImode
3) The mips_hard_regno_mode_ok_p logic does not limit the size of
   a mode allowed in GPRs as long as the first register is even.
4) TImode is not BLKmode so is not forced to memory here.

I'm doubtful this is by design but it is what it is.  Let it
hereby be part of the MIPS o32 ABI definition!

Are you sure you need the dg-skip-if on the test case? Perhaps just
-O0. The test does not need vectorization as it is explicitly
Vectorized but it may break the scan-assembler at -O0.  Also please
can you apply the normal code style to the tests?

v4f32
fcmpOeqVector4 (v4f32 a, v4f32 b)
{
  return a + b;
}

As this is an ABI fix, just wait a day or so in case Catherine has
any comment otherwise OK to commit.

Thanks,
Matthew

> 
> - Thanks and regards,
>   Sameera D.
> 
> From: Sameera Deshpande
> Sent: 08 February 2017 14:10:52
> To: Matthew Fortune
> Cc: gcc-patches@gcc.gnu.org
> Subject: [PATCH, MIPS] Calling convention differs depending on the
> presence of MSA
> 
> Hi Matthew,
> 
> Please find attached the patch to fix the calling convention issue,
> where argument and result passing convention differed for MSA and non-
> MSA variants.
> 
> The implementation of TARGET_RETURN_IN_MEMORY is altered to block V4SF
> to be returned in registers.
> 
> Ok for trunk?
> 
> - Thanks and regards,
>   Sameera D.
> 
> 
> Changelog:
> gcc/
> * config/mips/mips.c (mips_return_in_memory) : Restrict V4SFmode to
> be returned in registers.
> 
> gcc/testsuite/
> * gcc.target/mips/msa-fp-cc.c : New testcase.


[PATCH] Use -Wnornalized=[options] instead of -Wnormalized=

2017-02-21 Thread Martin Liška
Hello.

To make documentation and output of --help consistent, I suggest to replace 
inequality
signs with square brackets.

Ready to be installed?
Martin
>From 2b1ae19cf76271790b585e3fd90ba76b37166ed0 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Tue, 21 Feb 2017 11:39:24 +0100
Subject: [PATCH] Use -Wnornalized=[options] instead of -Wnormalized=

gcc/ChangeLog:

2017-02-21  Martin Liska  

	* doc/invoke.texi: Replace inequality signs with square brackets
	for -Wnornalized.

gcc/c-family/ChangeLog:

2017-02-21  Martin Liska  

	* c.opt: Replace inequality signs with square brackets
	for -Wnornalized.
---
 gcc/c-family/c.opt  | 2 +-
 gcc/doc/invoke.texi | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index a1b3ae5c0b0..a863471da7f 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -806,7 +806,7 @@ C ObjC C++ ObjC++ Warning Alias(Wnormalized=,nfc,none)
 
 Wnormalized=
 C ObjC C++ ObjC++ RejectNegative Joined Warning CPP(warn_normalize) CppReason(CPP_W_NORMALIZE) Init(normalized_C) Var(cpp_warn_normalize) Enum(cpp_normalize_level)
--Wnormalized=	Warn about non-normalized Unicode strings.
+-Wnormalized=[none|id|nfc|nfkc]	Warn about non-normalized Unicode strings.
 
 ; Required for these enum values.
 SourceInclude
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 26bc146fa65..ea42dac528f 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -6218,7 +6218,7 @@ Do not warn if a multicharacter constant (@samp{'FOOF'}) is used.
 Usually they indicate a typo in the user's code, as they have
 implementation-defined values, and should not be used in portable code.
 
-@item -Wnormalized@r{[}=@r{<}none@r{|}id@r{|}nfc@r{|}nfkc@r{>]}
+@item -Wnormalized=@r{[}none@r{|}id@r{|}nfc@r{|}nfkc@r{]}
 @opindex Wnormalized=
 @opindex Wnormalized
 @opindex Wno-normalized
-- 
2.11.0



Re: [PATCH] Fix typo in -masm-dialect option values.

2017-02-21 Thread Martin Liška
On 02/21/2017 10:52 AM, Jakub Jelinek wrote:
> On Tue, Feb 21, 2017 at 10:44:40AM +0100, Martin Liška wrote:
>> The patch is obvious, however should I also replace the option in *.po files:
> 
> Ok for trunk.
> 
>> gcc/po/be.po:msgid "Known assembler dialects (for use with the 
>> -masm-dialect= option):"
>> gcc/po/da.po:msgid "Known assembler dialects (for use with the 
>> -masm-dialect= option):"
>> gcc/po/de.po:msgid "Known assembler dialects (for use with the 
>> -masm-dialect= option):"
>> gcc/po/de.po:msgstr "Bekannte Assemblerdialekte (für Verwendung mit Option 
>> -masm-dialect=):"
>> gcc/po/el.po:msgid "Known assembler dialects (for use with the 
>> -masm-dialect= option):"
>> gcc/po/es.po:msgid "Known assembler dialects (for use with the 
>> -masm-dialect= option):"
>> gcc/po/es.po:msgstr "Dialectos de ensamblador conocidos (para uso con la 
>> opción -masm-dialect=):"
> 
> According to Joseph, the policy is we never do that ourselves, the *.po
> files are owned by the translation team.

Makes sense, installed as r245624.

Martin

> 
>   Jakub
> 



[PR translation/79638] "%ntid.y" confuses gcc.pot generation (was: [PATCH] Fix exgettext to handle multi-line help texts from *.opt files (PR translation/78745))

2017-02-21 Thread Thomas Schwinge
Hi!

On Thu, 16 Feb 2017 23:33:54 +, Joseph Myers  
wrote:
> [...] So I 
> think one of the local fixes to avoid this particular case being 
> misdetected as a spec string should be preferred.

Committed to trunk in r245623 (but I have not regenerated the gcc.pot
file):

commit 74a4a36d1369f295a6f0ce8c2fc2547246fb15fa
Author: tschwinge 
Date:   Tue Feb 21 10:42:07 2017 +

[PR translation/79638] "%ntid.y" confuses gcc.pot generation

gcc/
* config/nvptx/nvptx.c (ENTRY_TEMPLATE): Single out "%ntid.y".

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@245623 
138bc75d-0d04-0410-961f-82ee72b054a4
---
 gcc/ChangeLog| 5 +
 gcc/config/nvptx/nvptx.c | 5 -
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git gcc/ChangeLog gcc/ChangeLog
index d23fcbf..fe9018f 100644
--- gcc/ChangeLog
+++ gcc/ChangeLog
@@ -1,3 +1,8 @@
+2017-02-21  Thomas Schwinge  
+
+   PR translation/79638
+   * config/nvptx/nvptx.c (ENTRY_TEMPLATE): Single out "%ntid.y".
+
 2017-02-21  Eric Botcazou  
 
PR ada/67205
diff --git gcc/config/nvptx/nvptx.c gcc/config/nvptx/nvptx.c
index 98278d3..c52b090 100644
--- gcc/config/nvptx/nvptx.c
+++ gcc/config/nvptx/nvptx.c
@@ -1123,13 +1123,15 @@ write_omp_entry (FILE *file, const char *name, const 
char *orig)
   func_decls << ".extern .func gomp_nvptx_main (.param.u" << POINTER_SIZE
 << " %in_ar1, .param.u" << POINTER_SIZE << " %in_ar2);\n";
 }
+  /* PR79332.  Single out this string; it confuses gcc.pot generation.  */
+#define NTID_Y "%ntid.y"
 #define ENTRY_TEMPLATE(PS, PS_BYTES, MAD_PS_32) "\
  (.param.u" PS " %arg, .param.u" PS " %stack, .param.u" PS " %sz)\n\
 {\n\
.reg.u32 %r<3>;\n\
.reg.u" PS " %R<4>;\n\
mov.u32 %r0, %tid.y;\n\
-   mov.u32 %r1, %ntid.y;\n\
+   mov.u32 %r1, " NTID_Y ";\n\
mov.u32 %r2, %ctaid.x;\n\
cvt.u" PS ".u32 %R1, %r0;\n\
" MAD_PS_32 " %R1, %r1, %r2, %R1;\n\
@@ -1157,6 +1159,7 @@ write_omp_entry (FILE *file, const char *name, const char 
*orig)
   static const char entry64[] = ENTRY_TEMPLATE ("64", "8", "mad.wide.u32");
   static const char entry32[] = ENTRY_TEMPLATE ("32", "4", "mad.lo.u32  ");
 #undef ENTRY_TEMPLATE
+#undef NTID_Y
   const char *entry_1 = TARGET_ABI64 ? entry64 : entry32;
   /* Position ENTRY_2 after the embedded nul using strlen of the prefix.  */
   const char *entry_2 = entry_1 + strlen (entry64) + 1;


Grüße
 Thomas


Re: [PATCH 1/5] Handle WORD_REGISTER_OPERATIONS when reloading (subreg(reg))

2017-02-21 Thread Christophe Lyon
Hi,

On 20 February 2017 at 13:08, Matthew Fortune
 wrote:
> Vladimir Makarov  writes:
>> On 02/07/2017 09:08 AM, Matthew Fortune wrote:
>> > Hi,
>> >
>> > This change deals with reloading a subreg(reg) using the inner mode to
>> > prevent partial spilling of data like in the case described here:
>> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78660#c8
>> >
>> > No test case for now but I am investigating a targeted test using the
>> > RTL frontend for later submission.
>> >
>> >
>> > gcc/
>> > PR target/78660
>> > * lra-constraints.c (curr_insn_transform): Handle
>> > WORD_REGISTER_OPERATIONS requirements when reloading SUBREGs.
>> >
>> The patch is OK.  So all 5 patches can be committed to the trunk.  I
>> hope Eric's test of the patches on SPARC will be successful.  Thank you
>> again for your efforts.
>
> Committed as r245598.
>

This patch is causing ICEs on arm-none-linux-gnueabi
FAIL: gcc.c-torture/execute/simd-2.c   -O1  (internal compiler error)
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.c-torture/execute/simd-2.c:
In function 'main':
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.c-torture/execute/simd-2.c:72:1:
error: unable to find a register to spill
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.c-torture/execute/simd-2.c:72:1:
error: this is the insn:
(insn 276 2123 2129 2 (set (subreg:SI (reg:TI 886 [362]) 0)
(and:SI (subreg:SI (reg:TI 567 [orig:111 j.1_2 ] [111]) 0)
(subreg:SI (reg:TI 568 [orig:110 i.0_1 ] [110]) 0)))
"/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.c-torture/execute/simd-2.c":47
82 {*arm_andsi3_insn}
 (expr_list:REG_DEAD (reg:TI 568 [orig:110 i.0_1 ] [110])
(expr_list:REG_DEAD (reg:TI 567 [orig:111 j.1_2 ] [111])
(nil
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.c-torture/execute/simd-2.c:72:1:
internal compiler error: in assign_by_spills, at lra-assigns.c:1457
0xacc9d3 _fatal_insn(char const*, rtx_def const*, char const*, int, char const*)
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/rtl-error.c:108
0x9a6123 assign_by_spills
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/lra-assigns.c:1457
0x9a7506 lra_assign()
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/lra-assigns.c:1643
0x9a16d4 lra(_IO_FILE*)
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/lra.c:2451
0x957669 do_reload
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira.c:5452
0x957669 execute
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira.c:5636



I've also noticed that --target arm-none-eabi with default --with-mode
and --with-cpu
fails to build libgcc (it may be easier to reproduce):
/tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/fixed-bit.c:
In function '__gnu_mulhelperudq':
/tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/fixed-bit.c:371:1:
error: unable to find a register to spill
 }
 ^
/tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/fixed-bit.c:371:1:
error: this is the insn:
(insn 286 296 287 2 (set (reg:SI 232)
(neg:SI (ltu:SI (subreg:SI (reg:DI 238 [orig:119 _10 ] [119]) 4)
(subreg:SI (reg/v:DI 149 [ temp1 ]) 4
"/tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/fixed-bit.c":314
849 {cstor
esi_nltu_thumb1}
 (expr_list:REG_DEAD (reg:DI 238 [orig:119 _10 ] [119])
(nil)))
/tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/fixed-bit.c:371:1:
internal compiler error: in assign_by_spills, at lra-assigns.c:1457
0xacc9c3 _fatal_insn(char const*, rtx_def const*, char const*, int, char const*)

/tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/rtl-error.c:108
0x9a6113 assign_by_spills

/tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/lra-assigns.c:1457
0x9a74f6 lra_assign()

/tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/lra-assigns.c:1643
0x9a16c4 lra(_IO_FILE*)
/tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/lra.c:2451
0x957659 do_reload
/tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira.c:5452
0x957659 execute
/tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira.c:5636
make[4]: *** [_mulhelperUDQ.o] Error 1

Thanks,

Christophe


> Thanks,
> Matthew


Re: [PATCH 1/5] Handle WORD_REGISTER_OPERATIONS when reloading (subreg(reg))

2017-02-21 Thread Kyrill Tkachov


On 21/02/17 10:54, Christophe Lyon wrote:

Hi,

On 20 February 2017 at 13:08, Matthew Fortune
 wrote:

Vladimir Makarov  writes:

On 02/07/2017 09:08 AM, Matthew Fortune wrote:

Hi,

This change deals with reloading a subreg(reg) using the inner mode to
prevent partial spilling of data like in the case described here:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78660#c8

No test case for now but I am investigating a targeted test using the
RTL frontend for later submission.


gcc/
 PR target/78660
 * lra-constraints.c (curr_insn_transform): Handle
 WORD_REGISTER_OPERATIONS requirements when reloading SUBREGs.


The patch is OK.  So all 5 patches can be committed to the trunk.  I
hope Eric's test of the patches on SPARC will be successful.  Thank you
again for your efforts.

Committed as r245598.


This patch is causing ICEs on arm-none-linux-gnueabi
FAIL: gcc.c-torture/execute/simd-2.c   -O1  (internal compiler error)
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.c-torture/execute/simd-2.c:
In function 'main':
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.c-torture/execute/simd-2.c:72:1:
error: unable to find a register to spill
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.c-torture/execute/simd-2.c:72:1:
error: this is the insn:
(insn 276 2123 2129 2 (set (subreg:SI (reg:TI 886 [362]) 0)
 (and:SI (subreg:SI (reg:TI 567 [orig:111 j.1_2 ] [111]) 0)
 (subreg:SI (reg:TI 568 [orig:110 i.0_1 ] [110]) 0)))
"/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.c-torture/execute/simd-2.c":47
82 {*arm_andsi3_insn}
  (expr_list:REG_DEAD (reg:TI 568 [orig:110 i.0_1 ] [110])
 (expr_list:REG_DEAD (reg:TI 567 [orig:111 j.1_2 ] [111])
 (nil
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.c-torture/execute/simd-2.c:72:1:
internal compiler error: in assign_by_spills, at lra-assigns.c:1457
0xacc9d3 _fatal_insn(char const*, rtx_def const*, char const*, int, char const*)
 /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/rtl-error.c:108
0x9a6123 assign_by_spills
 /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/lra-assigns.c:1457
0x9a7506 lra_assign()
 /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/lra-assigns.c:1643
0x9a16d4 lra(_IO_FILE*)
 /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/lra.c:2451
0x957669 do_reload
 /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira.c:5452
0x957669 execute
 /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira.c:5636



I've also noticed that --target arm-none-eabi with default --with-mode
and --with-cpu
fails to build libgcc (it may be easier to reproduce):
/tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/fixed-bit.c:
In function '__gnu_mulhelperudq':
/tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/fixed-bit.c:371:1:
error: unable to find a register to spill
  }
  ^
/tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/fixed-bit.c:371:1:
error: this is the insn:
(insn 286 296 287 2 (set (reg:SI 232)
 (neg:SI (ltu:SI (subreg:SI (reg:DI 238 [orig:119 _10 ] [119]) 4)
 (subreg:SI (reg/v:DI 149 [ temp1 ]) 4
"/tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/fixed-bit.c":314
849 {cstor
esi_nltu_thumb1}
  (expr_list:REG_DEAD (reg:DI 238 [orig:119 _10 ] [119])
 (nil)))


This looks like PR 79660 that Richard just filed.
Kyrill


/tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/fixed-bit.c:371:1:
internal compiler error: in assign_by_spills, at lra-assigns.c:1457
0xacc9c3 _fatal_insn(char const*, rtx_def const*, char const*, int, char const*)
 
/tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/rtl-error.c:108
0x9a6113 assign_by_spills
 
/tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/lra-assigns.c:1457
0x9a74f6 lra_assign()
 
/tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/lra-assigns.c:1643
0x9a16c4 lra(_IO_FILE*)
 /tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/lra.c:2451
0x957659 do_reload
 /tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira.c:5452
0x957659 execute
 /tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira.c:5636
make[4]: *** [_mulhelperUDQ.o] Error 1

Thanks,

Christophe



Thanks,
Matthew




Re: [PATCH 1/5] Handle WORD_REGISTER_OPERATIONS when reloading (subreg(reg))

2017-02-21 Thread Christophe Lyon
On 21 February 2017 at 11:59, Kyrill Tkachov
 wrote:
>
> On 21/02/17 10:54, Christophe Lyon wrote:
>>
>> Hi,
>>
>> On 20 February 2017 at 13:08, Matthew Fortune
>>  wrote:
>>>
>>> Vladimir Makarov  writes:

 On 02/07/2017 09:08 AM, Matthew Fortune wrote:
>
> Hi,
>
> This change deals with reloading a subreg(reg) using the inner mode to
> prevent partial spilling of data like in the case described here:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78660#c8
>
> No test case for now but I am investigating a targeted test using the
> RTL frontend for later submission.
>
>
> gcc/
>  PR target/78660
>  * lra-constraints.c (curr_insn_transform): Handle
>  WORD_REGISTER_OPERATIONS requirements when reloading SUBREGs.
>
 The patch is OK.  So all 5 patches can be committed to the trunk.  I
 hope Eric's test of the patches on SPARC will be successful.  Thank you
 again for your efforts.
>>>
>>> Committed as r245598.
>>>
>> This patch is causing ICEs on arm-none-linux-gnueabi
>> FAIL: gcc.c-torture/execute/simd-2.c   -O1  (internal compiler error)
>>
>> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.c-torture/execute/simd-2.c:
>> In function 'main':
>>
>> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.c-torture/execute/simd-2.c:72:1:
>> error: unable to find a register to spill
>>
>> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.c-torture/execute/simd-2.c:72:1:
>> error: this is the insn:
>> (insn 276 2123 2129 2 (set (subreg:SI (reg:TI 886 [362]) 0)
>>  (and:SI (subreg:SI (reg:TI 567 [orig:111 j.1_2 ] [111]) 0)
>>  (subreg:SI (reg:TI 568 [orig:110 i.0_1 ] [110]) 0)))
>>
>> "/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.c-torture/execute/simd-2.c":47
>> 82 {*arm_andsi3_insn}
>>   (expr_list:REG_DEAD (reg:TI 568 [orig:110 i.0_1 ] [110])
>>  (expr_list:REG_DEAD (reg:TI 567 [orig:111 j.1_2 ] [111])
>>  (nil
>>
>> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.c-torture/execute/simd-2.c:72:1:
>> internal compiler error: in assign_by_spills, at lra-assigns.c:1457
>> 0xacc9d3 _fatal_insn(char const*, rtx_def const*, char const*, int, char
>> const*)
>>  /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/rtl-error.c:108
>> 0x9a6123 assign_by_spills
>>  /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/lra-assigns.c:1457
>> 0x9a7506 lra_assign()
>>  /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/lra-assigns.c:1643
>> 0x9a16d4 lra(_IO_FILE*)
>>  /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/lra.c:2451
>> 0x957669 do_reload
>>  /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira.c:5452
>> 0x957669 execute
>>  /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira.c:5636
>>
>>
>>
>> I've also noticed that --target arm-none-eabi with default --with-mode
>> and --with-cpu
>> fails to build libgcc (it may be easier to reproduce):
>>
>> /tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/fixed-bit.c:
>> In function '__gnu_mulhelperudq':
>>
>> /tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/fixed-bit.c:371:1:
>> error: unable to find a register to spill
>>   }
>>   ^
>>
>> /tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/fixed-bit.c:371:1:
>> error: this is the insn:
>> (insn 286 296 287 2 (set (reg:SI 232)
>>  (neg:SI (ltu:SI (subreg:SI (reg:DI 238 [orig:119 _10 ] [119]) 4)
>>  (subreg:SI (reg/v:DI 149 [ temp1 ]) 4
>>
>> "/tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/fixed-bit.c":314
>> 849 {cstor
>> esi_nltu_thumb1}
>>   (expr_list:REG_DEAD (reg:DI 238 [orig:119 _10 ] [119])
>>  (nil)))
>
>
> This looks like PR 79660 that Richard just filed.

Indeed, thanks.

> Kyrill
>
>
>>
>> /tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/fixed-bit.c:371:1:
>> internal compiler error: in assign_by_spills, at lra-assigns.c:1457
>> 0xacc9c3 _fatal_insn(char const*, rtx_def const*, char const*, int, char
>> const*)
>>
>> /tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/rtl-error.c:108
>> 0x9a6113 assign_by_spills
>>
>> /tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/lra-assigns.c:1457
>> 0x9a74f6 lra_assign()
>>
>> /tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/lra-assigns.c:1643
>> 0x9a16c4 lra(_IO_FILE*)
>>
>> /tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/lra.c:2451
>> 0x957659 do_reload
>>
>> /tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira.c:5452
>> 0x957659 execute
>>
>> /tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira.c:5636
>> make[4]: *** [_mulhelperUDQ.o] Error 1
>>
>> Thanks,
>>
>> Christophe
>>
>>
>>> Thanks,
>>> Matthew
>
>


RE: [PATCH 1/5] Handle WORD_REGISTER_OPERATIONS when reloading (subreg(reg))

2017-02-21 Thread Matthew Fortune
Christophe Lyon  writes:
> On 21 February 2017 at 11:59, Kyrill Tkachov
>  wrote:
> >
> > On 21/02/17 10:54, Christophe Lyon wrote:
> >>
> >> Hi,
> >>
> >> On 20 February 2017 at 13:08, Matthew Fortune
> >>  wrote:
> >>>
> >>> Vladimir Makarov  writes:
> 
>  On 02/07/2017 09:08 AM, Matthew Fortune wrote:
> >
> > Hi,
> >
> > This change deals with reloading a subreg(reg) using the inner
> > mode to prevent partial spilling of data like in the case
> described here:
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78660#c8
> >
> > No test case for now but I am investigating a targeted test using
> > the RTL frontend for later submission.
> >
> >
> > gcc/
> >  PR target/78660
> >  * lra-constraints.c (curr_insn_transform): Handle
> >  WORD_REGISTER_OPERATIONS requirements when reloading SUBREGs.
> >
>  The patch is OK.  So all 5 patches can be committed to the trunk.
>  I hope Eric's test of the patches on SPARC will be successful.
>  Thank you again for your efforts.
> >>>
> >>> Committed as r245598.
> >>>
> >> This patch is causing ICEs on arm-none-linux-gnueabi
> >> FAIL: gcc.c-torture/execute/simd-2.c   -O1  (internal compiler error)
> >>
> >> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.c-
> torture/execute/simd-2.c:
> >> In function 'main':
> >>
> >> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.c-
> torture/execute/simd-2.c:72:1:
> >> error: unable to find a register to spill
> >>
> >> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.c-
> torture/execute/simd-2.c:72:1:
> >> error: this is the insn:
> >> (insn 276 2123 2129 2 (set (subreg:SI (reg:TI 886 [362]) 0)
> >>  (and:SI (subreg:SI (reg:TI 567 [orig:111 j.1_2 ] [111]) 0)
> >>  (subreg:SI (reg:TI 568 [orig:110 i.0_1 ] [110]) 0)))
> >>
> >> "/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.c-torture/exec
> >> ute/simd-2.c":47
> >> 82 {*arm_andsi3_insn}
> >>   (expr_list:REG_DEAD (reg:TI 568 [orig:110 i.0_1 ] [110])
> >>  (expr_list:REG_DEAD (reg:TI 567 [orig:111 j.1_2 ] [111])
> >>  (nil
> >>
> >> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.c-
> torture/execute/simd-2.c:72:1:
> >> internal compiler error: in assign_by_spills, at lra-assigns.c:1457
> >> 0xacc9d3 _fatal_insn(char const*, rtx_def const*, char const*, int,
> >> char
> >> const*)
> >>  /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/rtl-error.c:108
> >> 0x9a6123 assign_by_spills
> >>  /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/lra-assigns.c:1457
> >> 0x9a7506 lra_assign()
> >>  /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/lra-assigns.c:1643
> >> 0x9a16d4 lra(_IO_FILE*)
> >>  /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/lra.c:2451
> >> 0x957669 do_reload
> >>  /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira.c:5452
> >> 0x957669 execute
> >>  /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ira.c:5636
> >>
> >>
> >>
> >> I've also noticed that --target arm-none-eabi with default
> >> --with-mode and --with-cpu fails to build libgcc (it may be easier to
> >> reproduce):
> >>
> >> /tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-
> fsf/gccsrc/libgcc/fixed-bit.c:
> >> In function '__gnu_mulhelperudq':
> >>
> >> /tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-
> fsf/gccsrc/libgcc/fixed-bit.c:371:1:
> >> error: unable to find a register to spill
> >>   }
> >>   ^
> >>
> >> /tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-
> fsf/gccsrc/libgcc/fixed-bit.c:371:1:
> >> error: this is the insn:
> >> (insn 286 296 287 2 (set (reg:SI 232)
> >>  (neg:SI (ltu:SI (subreg:SI (reg:DI 238 [orig:119 _10 ]
> [119]) 4)
> >>  (subreg:SI (reg/v:DI 149 [ temp1 ]) 4
> >>
> >> "/tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/fixe
> >> d-bit.c":314
> >> 849 {cstor
> >> esi_nltu_thumb1}
> >>   (expr_list:REG_DEAD (reg:DI 238 [orig:119 _10 ] [119])
> >>  (nil)))
> >
> >
> > This looks like PR 79660 that Richard just filed.
> 
> Indeed, thanks.

Thanks for reporting. I'll take a brief look first but revert if the
issue isn't something that vaguely makes sense to me.

Sorry for the hassle.

Matthew


> 
> > Kyrill
> >
> >
> >>
> >> /tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-
> fsf/gccsrc/libgcc/fixed-bit.c:371:1:
> >> internal compiler error: in assign_by_spills, at lra-assigns.c:1457
> >> 0xacc9c3 _fatal_insn(char const*, rtx_def const*, char const*, int,
> >> char
> >> const*)
> >>
> >> /tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/rtl-erro
> >> r.c:108
> >> 0x9a6113 assign_by_spills
> >>
> >> /tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/lra-assi
> >> gns.c:1457
> >> 0x9a74f6 lra_assign()
> >>
> >> /tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/lra-assi
> >> gns.c:1643
> >> 0x9a16c4 lra(_IO_FILE*)
> >>
> >> /tmp/870921_29.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/lra.c:24
> >> 51
> >> 0x957659 do_reload
> >>
> >> /tmp/870921_29.

Re: [PATCH 1/5] Handle WORD_REGISTER_OPERATIONS when reloading (subreg(reg))

2017-02-21 Thread Eric Botcazou
> Thanks for reporting. I'll take a brief look first but revert if the
> issue isn't something that vaguely makes sense to me.

You need to restrict any WORD_REGISTER_OPERATIONS test to subword registers.

-- 
Eric Botcazou


Re: [PATCH 3/5] Support WORD_REGISTER_OPERATIONS requirements in simplify_operand_subreg

2017-02-21 Thread Eric Botcazou
> The condition would look like this, What do you think?
> 
>   if (!(GET_MODE_PRECISION (mode) != GET_MODE_PRECISION (innermode)
> && GET_MODE_SIZE (mode) <= UNITS_PER_WORD
> && GET_MODE_SIZE (innermode) <= UNITS_PER_WORD
> && WORD_REGISTER_OPERATIONS)
>   && (!(MEM_ALIGN (subst) < GET_MODE_ALIGNMENT (mode)
> && SLOW_UNALIGNED_ACCESS (mode, MEM_ALIGN (subst)))
> 
>   || (MEM_ALIGN (reg) < GET_MODE_ALIGNMENT (innermode)
> 
>   && SLOW_UNALIGNED_ACCESS (innermode, MEM_ALIGN
> (reg) return true;

No regressions on SPARC.

-- 
Eric Botcazou


[PATCH] Add -Wno-psabi to diagnostic-test-expressions-1.c options

2017-02-21 Thread Segher Boessenkool
Without this the test fails on 32-bit PowerPC.

Is this okay for trunk?


Segher


2017-02-21  Segher Boessenkool  

gcc/testsuite/
* gcc.dg/plugin/diagnostic-test-expressions-1.c: Add -Wno-psabi
to dg-options.

---
 gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c 
b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c
index afbe0f7..b0dbc05 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O -fdiagnostics-show-caret" } */
+/* { dg-options "-O -fdiagnostics-show-caret -Wno-psabi" } */
 
 /* This is a collection of unittests to verify that we're correctly
capturing the source code ranges of various kinds of expression.
-- 
1.9.3



Re: [PATCH] Add -Wno-psabi to diagnostic-test-expressions-1.c options

2017-02-21 Thread Jakub Jelinek
On Tue, Feb 21, 2017 at 12:00:03PM +, Segher Boessenkool wrote:
> Without this the test fails on 32-bit PowerPC.
> 
> Is this okay for trunk?
> 
> 
> Segher
> 
> 
> 2017-02-21  Segher Boessenkool  
> 
> gcc/testsuite/
>   * gcc.dg/plugin/diagnostic-test-expressions-1.c: Add -Wno-psabi
>   to dg-options.

Ok, thanks.

Jakub


Re: [PATCH] Properly deprecate -fipa-cp-alignment

2017-02-21 Thread Martin Jambor
Hi,

On Fri, Feb 17, 2017 at 10:34:21AM +0100, Jakub Jelinek wrote:
> On Fri, Feb 17, 2017 at 10:31:16AM +0100, Martin Jambor wrote:
> > @@ -8066,12 +8065,8 @@ This flag is enabled by default at @option{-O3}.
> >  
> >  @item -fipa-cp-alignment
> >  @opindex -fipa-cp-alignment
> > -When enabled, this optimization propagates alignment of function
> > -parameters to support better vectorization and string operations.
> > -
> > -This flag is enabled by default at @option{-O2} and @option{-Os}.  It
> > -requires that @option{-fipa-cp} is enabled.
> > -@option{-fipa-cp-alignment} is obsolete, use @option{-fipa-bit-cp} instead.
> > +This option has been superseded by @option{-fipa-bit-cp} and is now
> > +deprecated.  Use @option{-fipa-bit-cp} instead.
> 
> I'd just remove the whole documentation about -fipa-cp-alignment, I think
> that is what we do for other removed options.
> Or at least say and is now ignored. rather than deprecated, to make it clear
> how we handle it (not at all).
> Ok with either of those changes.
> 

I see, I suppose we should be consistent, I will commit the following,
which I have included in a bootstrap and test run on x86_64-linux.

Thanks,

Martin


2017-02-20  Martin Jambor  

* common.opt (-fipa-cp-alignment): Mark as ignored and preserved
for backward compatibility only.
* doc/invoke.texi (Option Summary): Remove all references to
-fipa-cp-alignment.

---
 gcc/common.opt  |  4 ++--
 gcc/doc/invoke.texi | 12 +---
 2 files changed, 3 insertions(+), 13 deletions(-)

diff --git a/gcc/common.opt b/gcc/common.opt
index 6defe713c0b..e5ae364abd9 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1612,8 +1612,8 @@ Common Report Var(flag_ipa_cp_clone) Optimization
 Perform cloning to make Interprocedural constant propagation stronger.
 
 fipa-cp-alignment
-Common Report Var(flag_ipa_cp_alignment) Optimization
-Perform alignment discovery and propagation to make Interprocedural constant 
propagation stronger.
+Common Ignore
+Does nothing.  Preserved for backward compatibility.
 
 fipa-bit-cp
 Common Report Var(flag_ipa_bit_cp) Optimization
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 6e219dab4d3..8fae09d1a5e 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -372,7 +372,7 @@ Objective-C and Objective-C++ Dialects}.
 -fif-conversion2  -findirect-inlining @gol
 -finline-functions  -finline-functions-called-once  -finline-limit=@var{n} @gol
 -finline-small-functions  -fipa-cp  -fipa-cp-clone @gol
--fipa-cp-alignment  -fipa-bit-cp @gol
+-fipa-bit-cp @gol
 -fipa-pta  -fipa-profile  -fipa-pure-const  -fipa-reference  -fipa-icf @gol
 -fira-algorithm=@var{algorithm} @gol
 -fira-region=@var{region}  -fira-hoist-pressure @gol
@@ -7063,7 +7063,6 @@ also turns on the following optimization flags:
 -finline-small-functions @gol
 -findirect-inlining @gol
 -fipa-cp @gol
--fipa-cp-alignment @gol
 -fipa-bit-cp @gol
 -fipa-sra @gol
 -fipa-icf @gol
@@ -8073,15 +8072,6 @@ it may significantly increase code size
 (see @option{--param ipcp-unit-growth=@var{value}}).
 This flag is enabled by default at @option{-O3}.
 
-@item -fipa-cp-alignment
-@opindex -fipa-cp-alignment
-When enabled, this optimization propagates alignment of function
-parameters to support better vectorization and string operations.
-
-This flag is enabled by default at @option{-O2} and @option{-Os}.  It
-requires that @option{-fipa-cp} is enabled.
-@option{-fipa-cp-alignment} is obsolete, use @option{-fipa-bit-cp} instead.
-
 @item -fipa-bit-cp
 @opindex -fipa-bit-cp
 When enabled, perform ipa bitwise constant propagation. This flag is
-- 
2.11.0



[PR 79579] Avoid segfault on NULL ipa_edge_args_vector

2017-02-21 Thread Martin Jambor
Hi,

in, PR 79579, early inliner creates ipa_node_params_sum which is then
tested by ipa_prop_write_jump_functions to figure out whether there
has been anything to stream out when there is not.

The following patch improves the test - when there are no jump
function, there is no point in streaming jump functions or any ipa-cp
info in general.

Bootstrapped and tested on x86_64-linux, I will commit it momentarily
as obvious.

Thanks,

Martin


2017-02-20  Martin Jambor  

PR lto/79579
* ipa-prop.c (ipa_prop_write_jump_functions): Bail out if no edges
have been analyzed.
---
 gcc/ipa-prop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index e4e44ce20c6..33503d4befc 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -5040,7 +5040,7 @@ ipa_prop_write_jump_functions (void)
   lto_symtab_encoder_iterator lsei;
   lto_symtab_encoder_t encoder;
 
-  if (!ipa_node_params_sum)
+  if (!ipa_node_params_sum || !ipa_edge_args_vector)
 return;
 
   ob = create_output_block (LTO_section_jump_functions);
-- 
2.11.0



[PATCH] Avoid uninit warnings with PR79345 fix

2017-02-21 Thread Richard Biener

The following fixes a bit-load.c bug as well as avoids the warnings
for two other cases.

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

Ok?

Thanks,
Richard.

2017-02-21  Richard Biener  

* bt-load.c (compute_defs_uses_and_gen): Clear call_saved.
* fixed-value.c (fixed_from_string): Use wi::ulow and wi::uhigh.
(fixed_convert_from_real): Likewise.
* ipa-cp.c (ipcp_store_vr_results): Do not uselessly initialize
VR_VARYING min/max.

Index: gcc/bt-load.c
===
--- gcc/bt-load.c   (revision 245620)
+++ gcc/bt-load.c   (working copy)
@@ -543,6 +543,7 @@ compute_defs_uses_and_gen (btr_heap_t *a
  int i;
 
  /* Check for sibcall.  */
+ CLEAR_HARD_REG_SET (call_saved);
  if (GET_CODE (pat) == PARALLEL)
for (i = XVECLEN (pat, 0) - 1; i >= 0; i--)
  if (ANY_RETURN_P (XVECEXP (pat, 0, i)))
Index: gcc/fixed-value.c
===
--- gcc/fixed-value.c   (revision 245620)
+++ gcc/fixed-value.c   (working copy)
@@ -130,8 +130,8 @@ fixed_from_string (FIXED_VALUE_TYPE *f,
   real_arithmetic (&fixed_value, MULT_EXPR, &real_value, &base_value);
   wide_int w = real_to_integer (&fixed_value, &fail,
GET_MODE_PRECISION (mode));
-  f->data.low = w.elt (0);
-  f->data.high = w.elt (1);
+  f->data.low = w.ulow ();
+  f->data.high = w.uhigh ();
 
   if (temp == FIXED_MAX_EPS && ALL_FRACT_MODE_P (f->mode))
 {
@@ -1049,8 +1049,8 @@ fixed_convert_from_real (FIXED_VALUE_TYP
 
   wide_int w = real_to_integer (&fixed_value, &fail,
GET_MODE_PRECISION (mode));
-  f->data.low = w.elt (0);
-  f->data.high = w.elt (1);
+  f->data.low = w.ulow ();
+  f->data.high = w.uhigh ();
   temp = check_real_for_fixed_mode (&real_value, mode);
   if (temp == FIXED_UNDERFLOW) /* Minimum.  */
 {
Index: gcc/ipa-cp.c
===
--- gcc/ipa-cp.c(revision 245620)
+++ gcc/ipa-cp.c(working copy)
@@ -4959,7 +4959,6 @@ ipcp_store_vr_results (void)
{
  vr.known = false;
  vr.type = VR_VARYING;
- vr.min = vr.max = wi::zero (INT_TYPE_SIZE);
}
  ts->m_vr->quick_push (vr);
}


Re: [driver, doc] Support escaping special characters in specs

2017-02-21 Thread Rainer Orth
Hi Joseph,

> On Fri, 13 Jan 2017, Rainer Orth wrote:
>
>> I'm unsure if the patch is large enough to need a copyright assignment
>> (in which case it's almost certainly too late for GCC 7), and even if
>> not if it's appropriate at this point in the release cycle.
>
> I think it's big enough to need an assignment.

Jeff informed me that he'd received confirmation from the FSF that his
assignment has been processed.

> Note missing spaces before '(' in calls to free, and after ')' in a cast.

Fixed in the following revised patch, which also incorporates Sandra's
review comments.

Bootstrapped without regressions on i386-pc-solaris2.12 and
x86_64-pc-linux-gnu, in a tree which also contained the patch for PR
target/40411 which needs this feature.

The doc parts have been checked separately with

$ make doc/gcc.info doc/gcc.pdf

and visual inspection of the output.

Ok for mainline either now or when GCC 8 stage 1 opens?

Thanks.
Rainer

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


2017-01-10  Jeff Downs  
Rainer Orth  

* gcc.c (handle_braces): Support escaping in switch matching
text.
* doc/invoke.texi (Spec Files): Document it.
Remove superfluous @code markup in items.

# HG changeset patch
# Parent  12dd51d3fddd52cbb3925fe2a7950cb8df0ad230
Support escaping special characters in specs

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -26207,7 +26207,7 @@ Substitute the variable part of a matche
 Note that each comma in the substituted string is replaced by
 a single space.
 
-@item %<@code{S}
+@item %

Re: [driver, doc] Support escaping special characters in specs

2017-02-21 Thread Rainer Orth
Hi Sandra,

> On 01/16/2017 03:54 AM, Rainer Orth wrote:
>> Hi Sandra,
>>
>>> On 01/13/2017 05:59 AM, Rainer Orth wrote:
 diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
 --- a/gcc/doc/invoke.texi
 +++ b/gcc/doc/invoke.texi
 @@ -26391,6 +26391,13 @@ be as many clauses as you need.  This ma

   @end table

 +The switch matching text @code{S} in a %@{@code{S}@},
 +%@{@code{S}:@code{X}@} or similar construct can use a backslash to
 +ignore the special meaning of the character following it, thus allowing
 +literal matching of a character that is otherwise specially treated.
 +For example, %@{@code{std=iso9899\:1999}:@code{X}@} would substitute
 +@code{X} if the @option{-std=iso9899:1999} option were given.
 +
>>>
>>> I see this "%@{@code{..." markup appears in the paragraph just before this,
>>> but it's wrong.  The whole thing needs to be wrapped in @samp and the
>>> nested @codes removed, like
>>>
>>> s/%@{@code{S}:@code{X}@}/@samp{%@{S:X@}}/
>>>
>>> etc.
>>
>> I see, fixed.  I assume this applies to the uses inside @item, too, and
>> irrespective of %{S:X} or %{S}?
>
> It looked to me like this table environment uses
>
> @table @code
>
> so there should be no need for additional @code markup within the @item
> tags.
>
> I'm not asking you to repair existing bad markup elsewhere in this section,
> just not propagate it to your new paragraph of text.

perhaps not, but it makes sense to fix this while I'm at this section.
There are still more instances of the same problems (e.g. in the table
listing the spec functions), but I haven't touched those.

Should be all in the revised patch just submitted.

Rainer

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


Re: loop versioning doc changes

2017-02-21 Thread Richard Biener
On Tue, Feb 21, 2017 at 11:09 AM, Aldy Hernandez  wrote:
> Found these while poking at loop versioning.  Most importantly, we no longer
> have tree_ssa_loop_version, so I removed it from the docs.
>
> OK?

Ok.

Richard.


Re: [PATCH] Avoid uninit warnings with PR79345 fix

2017-02-21 Thread Jakub Jelinek
On Tue, Feb 21, 2017 at 02:53:32PM +0100, Richard Biener wrote:
> 
> The following fixes a bit-load.c bug as well as avoids the warnings
> for two other cases.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
> 
> Ok?
> 
> Thanks,
> Richard.
> 
> 2017-02-21  Richard Biener  
> 
>   * bt-load.c (compute_defs_uses_and_gen): Clear call_saved.
>   * fixed-value.c (fixed_from_string): Use wi::ulow and wi::uhigh.
>   (fixed_convert_from_real): Likewise.
>   * ipa-cp.c (ipcp_store_vr_results): Do not uselessly initialize
>   VR_VARYING min/max.
> 
> Index: gcc/bt-load.c
> ===
> --- gcc/bt-load.c (revision 245620)
> +++ gcc/bt-load.c (working copy)
> @@ -543,6 +543,7 @@ compute_defs_uses_and_gen (btr_heap_t *a
> int i;
>  
> /* Check for sibcall.  */
> +   CLEAR_HARD_REG_SET (call_saved);
> if (GET_CODE (pat) == PARALLEL)
>   for (i = XVECLEN (pat, 0) - 1; i >= 0; i--)
> if (ANY_RETURN_P (XVECEXP (pat, 0, i)))

Why do we warn here?
  HARD_REG_SET *clobbered = &call_used_reg_set;
  HARD_REG_SET call_saved;
  rtx pat = PATTERN (insn);
  int i;

  /* Check for sibcall.  */
  if (GET_CODE (pat) == PARALLEL)
for (i = XVECLEN (pat, 0) - 1; i >= 0; i--)
  if (ANY_RETURN_P (XVECEXP (pat, 0, i)))
{
  COMPL_HARD_REG_SET (call_saved,
  call_used_reg_set);
  clobbered = &call_saved;
}

  for (regno = first_btr; regno <= last_btr; regno++)
if (TEST_HARD_REG_BIT (*clobbered, regno))
  note_btr_set (regno_reg_rtx[regno], NULL_RTX, &info);
COMPL_HARD_REG_SET should always overwrite all of call_saved (it is
call_saved = ~call_used_reg_set).

> --- gcc/fixed-value.c (revision 245620)
> +++ gcc/fixed-value.c (working copy)
> @@ -130,8 +130,8 @@ fixed_from_string (FIXED_VALUE_TYPE *f,
>real_arithmetic (&fixed_value, MULT_EXPR, &real_value, &base_value);
>wide_int w = real_to_integer (&fixed_value, &fail,
>   GET_MODE_PRECISION (mode));
> -  f->data.low = w.elt (0);
> -  f->data.high = w.elt (1);
> +  f->data.low = w.ulow ();
> +  f->data.high = w.uhigh ();

Is this for the case when the wide_int has only a single uhwi (or more than
two)?

> --- gcc/ipa-cp.c  (revision 245620)
> +++ gcc/ipa-cp.c  (working copy)
> @@ -4959,7 +4959,6 @@ ipcp_store_vr_results (void)
>   {
> vr.known = false;
> vr.type = VR_VARYING;
> -   vr.min = vr.max = wi::zero (INT_TYPE_SIZE);
>   }
> ts->m_vr->quick_push (vr);
>   }

It is strange to see removing initialization of something as a work-around
to uninitialized warning.  Is that because of the vr.min = vr.max assignment
and that wi::zero doesn't initialize everything?

Jakub


Re: [PATCH] Avoid uninit warnings with PR79345 fix

2017-02-21 Thread Richard Biener
On Tue, 21 Feb 2017, Jakub Jelinek wrote:

> On Tue, Feb 21, 2017 at 02:53:32PM +0100, Richard Biener wrote:
> > 
> > The following fixes a bit-load.c bug as well as avoids the warnings
> > for two other cases.
> > 
> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> > 
> > Ok?
> > 
> > Thanks,
> > Richard.
> > 
> > 2017-02-21  Richard Biener  
> > 
> > * bt-load.c (compute_defs_uses_and_gen): Clear call_saved.
> > * fixed-value.c (fixed_from_string): Use wi::ulow and wi::uhigh.
> > (fixed_convert_from_real): Likewise.
> > * ipa-cp.c (ipcp_store_vr_results): Do not uselessly initialize
> > VR_VARYING min/max.
> > 
> > Index: gcc/bt-load.c
> > ===
> > --- gcc/bt-load.c   (revision 245620)
> > +++ gcc/bt-load.c   (working copy)
> > @@ -543,6 +543,7 @@ compute_defs_uses_and_gen (btr_heap_t *a
> >   int i;
> >  
> >   /* Check for sibcall.  */
> > + CLEAR_HARD_REG_SET (call_saved);
> >   if (GET_CODE (pat) == PARALLEL)
> > for (i = XVECLEN (pat, 0) - 1; i >= 0; i--)
> >   if (ANY_RETURN_P (XVECEXP (pat, 0, i)))
> 
> Why do we warn here?
>   HARD_REG_SET *clobbered = &call_used_reg_set;
>   HARD_REG_SET call_saved;
>   rtx pat = PATTERN (insn);
>   int i;
> 
>   /* Check for sibcall.  */
>   if (GET_CODE (pat) == PARALLEL)
> for (i = XVECLEN (pat, 0) - 1; i >= 0; i--)
>   if (ANY_RETURN_P (XVECEXP (pat, 0, i)))
> {
>   COMPL_HARD_REG_SET (call_saved,
>   call_used_reg_set);
>   clobbered = &call_saved;
> }
> 
>   for (regno = first_btr; regno <= last_btr; regno++)
> if (TEST_HARD_REG_BIT (*clobbered, regno))
>   note_btr_set (regno_reg_rtx[regno], NULL_RTX, 
> &info);
> COMPL_HARD_REG_SET should always overwrite all of call_saved (it is
> call_saved = ~call_used_reg_set).

Not sure why we warn, didn't fully investigate.

> > --- gcc/fixed-value.c   (revision 245620)
> > +++ gcc/fixed-value.c   (working copy)
> > @@ -130,8 +130,8 @@ fixed_from_string (FIXED_VALUE_TYPE *f,
> >real_arithmetic (&fixed_value, MULT_EXPR, &real_value, &base_value);
> >wide_int w = real_to_integer (&fixed_value, &fail,
> > GET_MODE_PRECISION (mode));
> > -  f->data.low = w.elt (0);
> > -  f->data.high = w.elt (1);
> > +  f->data.low = w.ulow ();
> > +  f->data.high = w.uhigh ();
> 
> Is this for the case when the wide_int has only a single uhwi (or more than
> two)?

This is for the case where elt (0) does

template 
inline HOST_WIDE_INT
generic_wide_int ::elt (unsigned int i) const
{
  if (i >= this->get_len ())
return sign_mask ();

turning that into 0 == this->get_len () and sign_mask has

inline HOST_WIDE_INT
generic_wide_int ::sign_mask () const
{
  unsigned int len = this->get_len ();
  unsigned HOST_WIDE_INT high = this->get_val ()[len - 1];

so we propagate len == 0 here and access this out-of-bound (and nothing
knows len is never zero).

> > --- gcc/ipa-cp.c(revision 245620)
> > +++ gcc/ipa-cp.c(working copy)
> > @@ -4959,7 +4959,6 @@ ipcp_store_vr_results (void)
> > {
> >   vr.known = false;
> >   vr.type = VR_VARYING;
> > - vr.min = vr.max = wi::zero (INT_TYPE_SIZE);
> > }
> >   ts->m_vr->quick_push (vr);
> > }
> 
> It is strange to see removing initialization of something as a work-around
> to uninitialized warning.  Is that because of the vr.min = vr.max assignment
> and that wi::zero doesn't initialize everything?

didn't fully track this down but it looks like the assignment from
wi::hwi_with_prec somehow accesses uninitialized memory.

I can only spend more time with this later this week (or next week).
Changing the warning patch to add a %K helps (you get at least an idea
of the inline stack):

+ /* We didn't find any may-defs so on all paths either
+reached function entry or a killing clobber.  */
  if (always_executed)
-   warn_uninit (OPT_Wuninitialized, use, gimple_assign_rhs1 
(stmt),
-base, "%qE is used uninitialized in this 
function",
-stmt, UNKNOWN_LOCATION);
+   warning_at (gimple_location (stmt), OPT_Wuninitialized,
+   "%K%qE is used uninitialized in this 
function",
+   rhs, rhs);
  else if (warn_possibly_uninitialized)
-   warn_uninit (OPT_Wmaybe_uninitialized, use,
-gimple_assign_rhs1 (stmt), base,
-   

Re: [PATCH PR77536]Generate correct profiling information for vectorized loop

2017-02-21 Thread Bin.Cheng
On Mon, Feb 20, 2017 at 4:05 PM, Jan Hubicka  wrote:
>> BTW, if we use gcov_type in calculation from 
>> expected_loop_iterations_unbounded,
>> how should we adjust the number for calling scale_loop_frequencies
>> which has int type?  In extreme case, gcov_type could be out of int's
>> range, we have to cap the value anyway.  But yes, 1 in
>> expect_loop_iterations is too small.
>
> What I usually do is to use fixed point math in this case (based on 
> REG_BR_PROB_BASE).
> Just pass REG_BR_PROB_BASE as den and calculate the adjustment in gcov_type 
> converting
> to int. Because you should be just decreasing the counts, it won't overflow 
> and because
> the decarese will be in range, say 2...256 times, it should also be 
> sufficiently
> precise.
>
> Be careful to avoid overflow of gcov type - it is not safe to multiply two
> counts in 64bit math because each count can be more than 2^32.  (next stage1 I
> plan to switch most of this to sreals that will make this easier)
>
>> >> > But I guess here it is sort of safe because vectorized loops are simple.
>> >> > You can't just scale down the existing counts/frequencies by vf, 
>> >> > because the
>> >> > entry edge frequency was adjusted.
>> >> I am not 100% follow here, it looks the code avoids changing frequency
>> >> counter for preheader/exit edge, otherwise we would need to change all
>> >> counters dominated by them?
>> >
>> > I was just wondering what is wrong with taking the existing 
>> > frequencies/counts
>> > the loop body has and dividing them all by the unroll factor.  This is 
>> > correct
>> > if you ignore the versioning. With versioning I guess you want to scale by
>> > the unroll factor and also subtract frequencies/counts that was acocunted 
>> > to
>> > the other versions of the loop, right?
>> IIUC, for (vectorized) loop header, it's frequency is calculated by:
>>   freq_header = freq_preheader + freq_latch
>> and freq_latch = (niter * freq_preheader).  Simply scaling it by VF gives:
>>   freq_header = (freq_preheader + freq_latch) / VF
>> which is wrong.  Especially if the loop is vectorized by large VF
>> (=16) and we normally assume niter (=10) without profiling
>> information, it results not only mismatch, but also
>> (loop->header->frequency < loop->preheader->frequency).  In fact, if
>> we have accurate niter information, the counter should be:
>>   freq_header = freq_preheader + niter * freq_preheader
>
> You are right. We need to compensate for the change of probability of the loop
> exit edge.
>>
>> >> >
>> >> > Also niter_for_unrolled_loop depends on sanity of the profile, so 
>> >> > perhaps you
>> >> > need to compute it before you start chanigng the CFG by peeling 
>> >> > proplogue?
>> >> Peeling for prologue doesn't change profiling information of
>> >> vect_loop, it is the skip edge from before loop to preferred epilogue
>> >> loop that will change profile counters.  I guess here exists a dilemma
>> >> that niter_for_unrolled_loop is for loop after peeling for prologue?
>> >
>> > expected_loop_iterations_unbounded calculates number of iteations by 
>> > computing
>> > sum of frequencies of edges entering the loop and comparing it to the 
>> > frequency
>> > of loop header.  While peeling the prologue, you split the preheader edge 
>> > and
>> > adjust frequency of the new preheader BB of the loop to be vectorized.  I 
>> > think
>> > that will adjust the #of iterations estimate.
>> It's not the case now I think.  one motivation of new vect_do_peeling
>> is to avoid niter checks between prologue and vector loop.  Once
>> prologue loop is entered or checked, the vector loop must be executed
>> unconditionally.  So the preheaderof vector loop has consistent
>> frequency counters now.  The niter check on whether vector loop should
>> be executed is now merged with cost check before prologue, and in the
>> future I think this can be further merged if loop versioning is
>> needed.
>
> Originally you have
>
>   loop_preheader
>|
>v
>loop_header
>
> and the ratio of the two BB frequencies is the loop iteration count. Then you
> do something like:
>
>   orig_loop_preheader
>|
>v
>loop_prologue -> scalar_version_of_loop
>|
>v
>  new_loop_preheader
>|
>v
>loop_header
>
> At some point, you need to update new_loop_preheader frequency/count
> to reflect the fact that with some probability the loop_prologue avoids
> the vectorized loop.  Once you do it and if you don't scale frequency of
> loop_header you will make expect_loop_iterations to return higher value
> than previously.
>
> So at the time you are calling it, you need to be sure that the loop_header
> and its preheader frequences was both adjusted by same factor.  Or you need
> to call it early before you start hacking on the CFG and its profile.
>
> Pehaps currently it is safe, because your peeling code is also scaling
> the loop profiles.
Hi Honza,
Attachment is the up

[PATCH 2/6] cris: Fix for RTL checking

2017-02-21 Thread Segher Boessenkool
2017-02-21  Segher Boessenkool  

* config/cris/cris.md: Use correct operand in a define_peephole2.

---
 gcc/config/cris/cris.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/config/cris/cris.md b/gcc/config/cris/cris.md
index 6ba3772..472aec7 100644
--- a/gcc/config/cris/cris.md
+++ b/gcc/config/cris/cris.md
@@ -5034,7 +5034,7 @@ (define_peephole2 ; andqu (casesi+46)
   [(set (match_dup 0) (match_dup 3))
(set (match_dup 0) (and:SI (match_dup 0) (match_dup 4)))]
 {
-  machine_mode zmode = INTVAL (operands[2]) <= 255 ? QImode : HImode;
+  machine_mode zmode = INTVAL (operands[1]) <= 255 ? QImode : HImode;
   rtx op1
 = (REG_S_P (operands[2])
? gen_rtx_REG (zmode, REGNO (operands[2]))
-- 
1.9.3



[PATCH 4/6] nios2: Fixes for RTL checking

2017-02-21 Thread Segher Boessenkool
You cannot call REGNO on something that isn't a REG, and you cannot
call INTVAL on something that isn't a CONST_INT.

The way I fixed nios2_alternate_compare_const is admittedly a bit lame.


2017-02-21  Segher Boessenkool  

* config/nios2/nios2.c (nios2_simple_const_p): Returns false if the
argument isn't a CONST_INT.
(nios2_alternate_compare_const): Set *alt_code and *alt_op to code
and op if op is not a CONST_INT.
(nios2_valid_compare_const_p): Return false if the argument isn't
a CONST_INT.
(ldstwm_operation_p): Return false if first_base is not a REG or
if first_offset is not a CONST_INT.

---
 gcc/config/nios2/nios2.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/gcc/config/nios2/nios2.c b/gcc/config/nios2/nios2.c
index e1b0372..753 100644
--- a/gcc/config/nios2/nios2.c
+++ b/gcc/config/nios2/nios2.c
@@ -1416,6 +1416,8 @@ nios2_option_override (void)
 static bool
 nios2_simple_const_p (const_rtx cst)
 {
+  if (!CONST_INT_P (cst))
+return false;
   HOST_WIDE_INT val = INTVAL (cst);
   return SMALL_INT (val) || SMALL_INT_UNSIGNED (val) || UPPER16_INT (val);
 }
@@ -1753,6 +1755,13 @@ nios2_alternate_compare_const (enum rtx_code code, rtx 
op,
   enum rtx_code *alt_code, rtx *alt_op,
   machine_mode mode)
 {
+  if (!CONST_INT_P (op))
+{
+  *alt_code = code;
+  *alt_op = op;
+  return;
+}
+
   HOST_WIDE_INT opval = INTVAL (op);
   enum rtx_code scode = signed_condition (code);
   bool dec_p = (scode == LT || scode == GE);
@@ -1788,6 +1797,9 @@ nios2_alternate_compare_const (enum rtx_code code, rtx op,
 static bool
 nios2_valid_compare_const_p (enum rtx_code code, rtx op)
 {
+  if (!CONST_INT_P (op))
+return false;
+
   switch (code)
 {
 case EQ: case NE: case GE: case LT:
@@ -4558,6 +4570,8 @@ ldstwm_operation_p (rtx op, bool load_p)
  if (!split_mem_address (XEXP (mem, 0),
  &first_base, &first_offset))
return false;
+ if (!REG_P (first_base) || !CONST_INT_P (first_offset))
+   return false;
  base_reg = first_base;
  inc_p = INTVAL (first_offset) >= 0;
}
-- 
1.9.3



[PATCH 3/6] microblaze: Fixes for RTL checking

2017-02-21 Thread Segher Boessenkool
REGNO can only be called on REGs, not SUBREGs; and INTVAL does not work
on REGs.

2017-02-21  Segher Boessenkool  

* config/microblaze/microblaze.c (microblaze_expand_shift): Do not
test for register moves to themselves.
* config/microblaze/microblaze.md (*ashlsi3_byone, *ashrsi3_byone,
*lshrsi3_byone): Test for const1_rtx instead of calling INTVAL on
something that isn't necessarily a CONST_INT.

---
 gcc/config/microblaze/microblaze.c  | 5 ++---
 gcc/config/microblaze/microblaze.md | 6 +++---
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/gcc/config/microblaze/microblaze.c 
b/gcc/config/microblaze/microblaze.c
index 746bef1..4850e85 100644
--- a/gcc/config/microblaze/microblaze.c
+++ b/gcc/config/microblaze/microblaze.c
@@ -3322,11 +3322,10 @@ microblaze_expand_shift (rtx operands[])
  || (GET_CODE (operands[1]) == REG)
  || (GET_CODE (operands[1]) == SUBREG));
 
-  /* Shift by zero -- copy regs if necessary.  */
+  /* Shift by zero -- copy regs.  */
   if ((GET_CODE (operands[2]) == CONST_INT) && (INTVAL (operands[2]) == 0))
 {
-  if (REGNO (operands[0]) != REGNO (operands[1]))
-   emit_insn (gen_movsi (operands[0], operands[1]));
+  emit_insn (gen_movsi (operands[0], operands[1]));
   return 1;
 }
 
diff --git a/gcc/config/microblaze/microblaze.md 
b/gcc/config/microblaze/microblaze.md
index 66ebc1e..9cf83f5 100644
--- a/gcc/config/microblaze/microblaze.md
+++ b/gcc/config/microblaze/microblaze.md
@@ -1321,7 +1321,7 @@ (define_insn "*ashlsi3_byone"
   [(set (match_operand:SI 0 "register_operand" "=d")
(ashift:SI (match_operand:SI 1 "register_operand" "d")
(match_operand:SI 2 "arith_operand""I")))] 
-  "(INTVAL (operands[2]) == 1)"
+  "operands[2] == const1_rtx"
   "addk\t%0,%1,%1"
   [(set_attr "type""arith")
(set_attr "mode""SI")
@@ -1482,7 +1482,7 @@ (define_insn "*ashrsi3_byone"
   [(set (match_operand:SI 0 "register_operand" "=d")
(ashiftrt:SI (match_operand:SI 1 "register_operand" "d")
  (match_operand:SI 2 "arith_operand""I")))] 
-  "(INTVAL (operands[2]) == 1)"
+  "operands[2] == const1_rtx"
   "sra\t%0,%1"
   [(set_attr "type""arith")
(set_attr "mode""SI")
@@ -1571,7 +1571,7 @@ (define_insn "*lshrsi3_byone"
   [(set (match_operand:SI 0 "register_operand" "=d")
(lshiftrt:SI (match_operand:SI 1 "register_operand" "d")
  (match_operand:SI 2 "arith_operand""I")))] 
-  "(INTVAL (operands[2]) == 1)"
+  "operands[2] == const1_rtx"
   "srl\t%0,%1"
   [(set_attr "type""arith")
(set_attr "mode""SI")
-- 
1.9.3



[PATCH 6/6] sh: Fixes for RTL checking

2017-02-21 Thread Segher Boessenkool
2017-02-21  Segher Boessenkool  

* config/sh/sh.md (tstsi_t): If operands[0] is a SUBREG instead of
a REG, look at the REG it is a SUBREG of.
(splitter for cmpeqsi_t): Ditto.

---
 gcc/config/sh/sh.md | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/gcc/config/sh/sh.md b/gcc/config/sh/sh.md
index 2645fca..e19e977 100644
--- a/gcc/config/sh/sh.md
+++ b/gcc/config/sh/sh.md
@@ -561,8 +561,12 @@ (define_insn_and_split "tstsi_t"
   gcc_assert (CONST_INT_P (operands[1]));
 
   HOST_WIDE_INT op1val = INTVAL (operands[1]);
+  rtx reg = operands[0];
+  if (SUBREG_P (reg))
+reg = SUBREG_REG (reg);
+  gcc_assert (REG_P (reg));
   bool op0_dead_after_this =
-   sh_reg_dead_or_unused_after_insn (curr_insn, REGNO (operands[0]));
+   sh_reg_dead_or_unused_after_insn (curr_insn, REGNO (reg));
 
   if (optimize)
 {
@@ -834,7 +838,11 @@ (define_split
   /* If the tested reg is not dead after this insn, it's probably used by
  something else after the comparison.  It's probably better to leave
  it as it is.  */
-  if (find_regno_note (curr_insn, REG_DEAD, REGNO (operands[0])) == NULL_RTX)
+  rtx reg = operands[0];
+  if (SUBREG_P (reg))
+reg = SUBREG_REG (reg);
+  gcc_assert (REG_P (reg));
+  if (find_regno_note (curr_insn, REG_DEAD, REGNO (reg)) != NULL_RTX)
 FAIL;
 
   /* FIXME: Maybe also search the predecessor basic blocks to catch
-- 
1.9.3



[PATCH 0/6] Fallout from RTL checking

2017-02-21 Thread Segher Boessenkool
Hi!

I tested building a cross compiler to all Linux targets (well, those
that are supported by GCC), and building Linux with that.  I used
--enable-checking=yes,rtl,tree and there was some fallout; all in
ports, and all with RTL checking.

c6x and pa illegally share RTL.  The rest of the failures are calling
REGNO on something that isn't a REG, and INTVAL on something that isn't
a CONST_INT.

These patches were tested as described (not bootstrapped and/or
regression checked).  Are they okay to commit, or do you want to handle
things differently?


Segher


Segher Boessenkool (6):
  c6x: Fix for RTL checking
  cris: Fix for RTL checking
  microblaze: Fixes for RTL checking
  nios2: Fixes for RTL checking
  pa: Fixes for RTL checking
  sh: Fixes for RTL checking

 gcc/config/c6x/c6x.c|  1 +
 gcc/config/cris/cris.md |  2 +-
 gcc/config/microblaze/microblaze.c  |  5 ++---
 gcc/config/microblaze/microblaze.md |  6 +++---
 gcc/config/nios2/nios2.c| 14 ++
 gcc/config/pa/pa.c  | 26 --
 gcc/config/sh/sh.md | 12 ++--
 7 files changed, 43 insertions(+), 23 deletions(-)

-- 
1.9.3


[PATCH 5/6] pa: Fixes for RTL checking

2017-02-21 Thread Segher Boessenkool
I do not know what the USEs are really for, so I do not know if using
just the pattern here is correct.  Using the full insn is probably not
correct either though.


2017-02-21  Segher Boessenkool  

* config/pa/pa.c (pa_combine_instructions): Do not share RTL.  Make
the special USEs with the pattern of the insn, not the insn itself.

---
 gcc/config/pa/pa.c | 26 --
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/gcc/config/pa/pa.c b/gcc/config/pa/pa.c
index b0b3311..3f7b2c7 100644
--- a/gcc/config/pa/pa.c
+++ b/gcc/config/pa/pa.c
@@ -9178,17 +9178,17 @@ pa_combine_instructions (void)
  || anchor_attr == PA_COMBINE_TYPE_FMPY))
{
  /* Emit the new instruction and delete the old anchor.  */
- emit_insn_before (gen_rtx_PARALLEL
-   (VOIDmode,
-gen_rtvec (2, PATTERN (anchor),
-   PATTERN (floater))),
-   anchor);
+ rtvec vtemp = gen_rtvec (2, copy_rtx (PATTERN (anchor)),
+  copy_rtx (PATTERN (floater)));
+ rtx temp = gen_rtx_PARALLEL (VOIDmode, vtemp);
+ emit_insn_before (temp, anchor);
 
  SET_INSN_DELETED (anchor);
 
  /* Emit a special USE insn for FLOATER, then delete
 the floating insn.  */
- emit_insn_before (gen_rtx_USE (VOIDmode, floater), floater);
+ temp = copy_rtx (PATTERN (floater));
+ emit_insn_before (gen_rtx_USE (VOIDmode, temp), floater);
  delete_insn (floater);
 
  continue;
@@ -9196,21 +9196,19 @@ pa_combine_instructions (void)
  else if (floater
   && anchor_attr == PA_COMBINE_TYPE_UNCOND_BRANCH)
{
- rtx temp;
  /* Emit the new_jump instruction and delete the old anchor.  */
- temp
-   = emit_jump_insn_before (gen_rtx_PARALLEL
-(VOIDmode,
- gen_rtvec (2, PATTERN (anchor),
-PATTERN (floater))),
-anchor);
+ rtvec vtemp = gen_rtvec (2, copy_rtx (PATTERN (anchor)),
+  copy_rtx (PATTERN (floater)));
+ rtx temp = gen_rtx_PARALLEL (VOIDmode, vtemp);
+ temp = emit_jump_insn_before (temp, anchor);
 
  JUMP_LABEL (temp) = JUMP_LABEL (anchor);
  SET_INSN_DELETED (anchor);
 
  /* Emit a special USE insn for FLOATER, then delete
 the floating insn.  */
- emit_insn_before (gen_rtx_USE (VOIDmode, floater), floater);
+ temp = copy_rtx (PATTERN (floater));
+ emit_insn_before (gen_rtx_USE (VOIDmode, temp), floater);
  delete_insn (floater);
  continue;
}
-- 
1.9.3



Re: [PATCH 2/6] cris: Fix for RTL checking

2017-02-21 Thread Hans-Peter Nilsson
> From: Segher Boessenkool 
> Date: Tue, 21 Feb 2017 14:48:14 +

> 2017-02-21  Segher Boessenkool  
> 
>   * config/cris/cris.md: Use correct operand in a define_peephole2.

Obviously ok, thanks (INTVAL on "const_int_operand"
vs. "nonimmediate_operand").

brgds, H-P


[PATCH 1/6] c6x: Fix for RTL checking

2017-02-21 Thread Segher Boessenkool
2017-02-21  Segher Boessenkool  

* config/c6x/c6x.c (predicate_insn): Do not incorrectly share RTL.

---
 gcc/config/c6x/c6x.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gcc/config/c6x/c6x.c b/gcc/config/c6x/c6x.c
index 84bfdfa..42b773b 100644
--- a/gcc/config/c6x/c6x.c
+++ b/gcc/config/c6x/c6x.c
@@ -3800,6 +3800,7 @@ predicate_insn (rtx_insn *insn, rtx cond, bool doit)
 {
   if (doit)
{
+ cond = copy_rtx (cond);
  rtx newpat = gen_rtx_COND_EXEC (VOIDmode, cond, PATTERN (insn));
  PATTERN (insn) = newpat;
  INSN_CODE (insn) = -1;
-- 
1.9.3



Re: [PATCH 1/6] c6x: Fix for RTL checking

2017-02-21 Thread Bernd Schmidt

On 02/21/2017 03:48 PM, Segher Boessenkool wrote:

2017-02-21  Segher Boessenkool  

* config/c6x/c6x.c (predicate_insn): Do not incorrectly share RTL.


Ok, thanks.


Bernd



[PATCH] arc: Fixes for RTL checking

2017-02-21 Thread Segher Boessenkool
Whoops, I forgot one.  Here it is.  Joern, please see
.


2017-02-21  Segher Boessenkool  

* config/arc/arc.c (arc_ccfsm_advance): Only take the PATTERN of
this_insn if it is an INSN or JUMP_INSN.
(force_offsettable): Look at base, not at addr.
* config/arc/predicates.md (brcc_nolimm_operator): Don't call INTVAL
on things that aren' necessarily CONST_INTs.

---
 gcc/config/arc/arc.c | 8 +---
 gcc/config/arc/predicates.md | 2 ++
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
index 8a838f9..4c99f1d 100644
--- a/gcc/config/arc/arc.c
+++ b/gcc/config/arc/arc.c
@@ -3832,8 +3832,6 @@ arc_ccfsm_advance (rtx_insn *insn, struct arc_ccfsm 
*state)
  break;
}
 
- scanbody = PATTERN (this_insn);
-
  switch (GET_CODE (this_insn))
{
case CODE_LABEL:
@@ -3868,6 +3866,8 @@ arc_ccfsm_advance (rtx_insn *insn, struct arc_ccfsm 
*state)
  break;
 
case JUMP_INSN:
+ scanbody = PATTERN (this_insn);
+
  /* If this is an unconditional branch to the same label, succeed.
 If it is to another label, do nothing.  If it is conditional,
 fail.  */
@@ -3902,6 +3902,8 @@ arc_ccfsm_advance (rtx_insn *insn, struct arc_ccfsm 
*state)
  break;
 
case INSN:
+ scanbody = PATTERN (this_insn);
+
  /* We can only do this with insns that can use the condition
 codes (and don't set them).  */
  if (GET_CODE (scanbody) == SET
@@ -7401,7 +7403,7 @@ force_offsettable (rtx addr, HOST_WIDE_INT size, bool 
reuse)
 }
   if (!REG_P (base)
   || (REGNO (base) != STACK_POINTER_REGNUM
- && REGNO_PTR_FRAME_P (REGNO (addr)))
+ && REGNO_PTR_FRAME_P (REGNO (base)))
   || !CONST_INT_P (offs) || !SMALL_INT (INTVAL (offs))
   || !SMALL_INT (INTVAL (offs) + size))
 {
diff --git a/gcc/config/arc/predicates.md b/gcc/config/arc/predicates.md
index 159a6b4..0dec736 100644
--- a/gcc/config/arc/predicates.md
+++ b/gcc/config/arc/predicates.md
@@ -458,8 +458,10 @@ (define_predicate "ge_lt_comparison_operator"
 (define_predicate "brcc_nolimm_operator"
   (ior (match_test "REG_P (XEXP (op, 1))")
(and (match_code "eq, ne, lt, ge, ltu, geu")
+   (match_test "CONST_INT_P (XEXP (op, 1))")
(match_test "u6_immediate_operand (XEXP (op, 1), SImode)"))
(and (match_code "le, gt, leu, gtu")
+   (match_test "CONST_INT_P (XEXP (op, 1))")
(match_test "UNSIGNED_INT6 (INTVAL (XEXP (op, 1)) + 1)"
 
 ;; Return TRUE if this is the condition code register, if we aren't given
-- 
1.9.3



Re: [PATCH 6/6] sh: Fixes for RTL checking

2017-02-21 Thread Oleg Endo
On Tue, 2017-02-21 at 14:48 +, Segher Boessenkool wrote:
> 2017-02-21  Segher Boessenkool  
> 
>   * config/sh/sh.md (tstsi_t): If operands[0] is a SUBREG instead of
>   a REG, look at the REG it is a SUBREG of.
>   (splitter for cmpeqsi_t): Ditto.

> 
> ---
>  gcc/config/sh/sh.md | 12 ++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/gcc/config/sh/sh.md b/gcc/config/sh/sh.md
> index 2645fca..e19e977 100644
> --- a/gcc/config/sh/sh.md
> +++ b/gcc/config/sh/sh.md
> @@ -561,8 +561,12 @@ (define_insn_and_split "tstsi_t"
>    gcc_assert (CONST_INT_P (operands[1]));
>  
>    HOST_WIDE_INT op1val = INTVAL (operands[1]);
> +  rtx reg = operands[0];
> +  if (SUBREG_P (reg))
> +reg = SUBREG_REG (reg);
> +  gcc_assert (REG_P (reg));
>    bool op0_dead_after_this =
> - sh_reg_dead_or_unused_after_insn (curr_insn, REGNO
> (operands[0]));
> + sh_reg_dead_or_unused_after_insn (curr_insn, REGNO (reg));
>  

Thanks for dealing with those.

That SUBREG vs. REG stuff is annoying.  Isn't there a simple function
that just does the right thing which can be used instead of manually
open-coding these checks over and over again?

Cheers,
Oleg


Re: [PATCH] Remove wrong assert about gcov_type (PR lto/79587).

2017-02-21 Thread Jan Hubicka
> Hello.
> 
> As described in the PR, we represent unsigned long types as gcov_type in 
> compiler.
> That can lead to a negative value in context of GCOV_SINGLE_VALUE (coming 
> from DIV/MOV expression).
> Thus, we have to relax the assert.
> 
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. 
> And Python can be bootstrapped
> with the patch.
> 
> Ready to be installed?
OK,
thanks!
Honza


Re: [PATCH PR77536]Generate correct profiling information for vectorized loop

2017-02-21 Thread Jan Hubicka
> 2017-02-21  Bin Cheng  
> 
> PR tree-optimization/77536
> * tree-ssa-loop-manip.c (niter_for_unrolled_loop): New function.
> (tree_transform_and_unroll_loop): Use above function to compute the
> estimated niter of unrolled loop and use it when scaling profile.
> * tree-ssa-loop-manip.h niter_for_unrolled_loop(): New declaration.
> * tree-vect-loop.c (scale_profile_for_vect_loop): New function.
> (vect_transform_loop): Call above function.
> 
> gcc/testsuite/ChangeLog
> 2017-02-21  Bin Cheng  
> 
> PR tree-optimization/77536
> * gcc.dg/vect/pr79347.c: Revise testing string.
> @@ -1329,7 +1339,12 @@ tree_transform_and_unroll_loop (struct loop *loop, 
> unsigned factor,
>freq_h = loop->header->frequency;
>freq_e = EDGE_FREQUENCY (loop_preheader_edge (loop));
>if (freq_h != 0)
> -scale_loop_frequencies (loop, freq_e * (new_est_niter + 1), freq_h);
> +{
> +  gcov_type scale;
> +  /* This should not overflow.  */
> +  scale = GCOV_COMPUTE_SCALE (freq_e * (new_est_niter + 1), freq_h);
> +  scale_loop_frequencies (loop, scale, REG_BR_PROB_BASE);

You need to use counts counts when new_est_niter is derrived from profile 
feedback.
This is because frequencies are capped to 1, so if loop iterates very many 
times,
new_est_niter will be large, freq_h will be 1 and freq_e will be 0.

Also watch the case when freq_e==loop_preheader_edge (loop)->count==0 and freq_h
is non-zero.  Just do MAX (freq_e, 1). This will not drop the loop body profile 
to 0.

> +/* Scale profiling counters by estimation for LOOP which is vectorized
> +   by factor VF.  */
> +
> +static void
> +scale_profile_for_vect_loop (struct loop *loop, unsigned vf)
> +{
> +  edge preheader = loop_preheader_edge (loop);
> +  unsigned freq_h = loop->header->frequency;
> +  unsigned freq_e = EDGE_FREQUENCY (preheader);
> +  /* Reduce loop iterations by the vectorization factor.  */
> +  gcov_type new_est_niter = niter_for_unrolled_loop (loop, vf);
> +
> +  /* Use profiling count information if frequencies are zero.  */
> +  if (freq_h == 0 || freq_e == 0)
> +{
> +  freq_e = preheader->count;
> +  freq_h = loop->header->count;
> +}
> +
> +  if (freq_h != 0)
> +{
> +  gcov_type scale;
> +  /* This should not overflow.  */
> +  scale = GCOV_COMPUTE_SCALE (freq_e * (new_est_niter + 1), freq_h);
> +  scale_loop_frequencies (loop, scale, REG_BR_PROB_BASE);
> +}

Similarly here. Use counts when they are non-zero and use MAX (freq_e, 1).
freq_e/freq_h needs to be gcov_type in that case.

Patch is OK with these changes.  Thanks a lot!
Honza
> +
> +  basic_block exit_bb = single_pred (loop->latch);
> +  edge exit_e = single_exit (loop);
> +  exit_e->count = loop_preheader_edge (loop)->count;
> +  exit_e->probability = REG_BR_PROB_BASE / (new_est_niter + 1);
> +
> +  edge exit_l = single_pred_edge (loop->latch);
> +  int prob = exit_l->probability;
> +  exit_l->probability = REG_BR_PROB_BASE - exit_e->probability;
> +  exit_l->count = exit_bb->count - exit_e->count;
> +  if (exit_l->count < 0)
> +exit_l->count = 0;
> +  if (prob > 0)
> +scale_bbs_frequencies_int (&loop->latch, 1, exit_l->probability, prob);
> +}
> +
>  /* Function vect_transform_loop.
>  
> The analysis phase has determined that the loop is vectorizable.
> @@ -6743,16 +6785,10 @@ vect_transform_loop (loop_vec_info loop_vinfo)
>bool transform_pattern_stmt = false;
>bool check_profitability = false;
>int th;
> -  /* Record number of iterations before we started tampering with the 
> profile. */
> -  gcov_type expected_iterations = expected_loop_iterations_unbounded (loop);
>  
>if (dump_enabled_p ())
>  dump_printf_loc (MSG_NOTE, vect_location, "=== vec_transform_loop 
> ===\n");
>  
> -  /* If profile is inprecise, we have chance to fix it up.  */
> -  if (LOOP_VINFO_NITERS_KNOWN_P (loop_vinfo))
> -expected_iterations = LOOP_VINFO_INT_NITERS (loop_vinfo);
> -
>/* Use the more conservative vectorization threshold.  If the number
>   of iterations is constant assume the cost check has been performed
>   by our caller.  If the threshold makes all loops profitable that
> @@ -7068,9 +7104,8 @@ vect_transform_loop (loop_vec_info loop_vinfo)
>  
>slpeel_make_loop_iterate_ntimes (loop, niters_vector);
>  
> -  /* Reduce loop iterations by the vectorization factor.  */
> -  scale_loop_profile (loop, GCOV_COMPUTE_SCALE (1, vf),
> -   expected_iterations / vf);
> +  scale_profile_for_vect_loop (loop, vf);
> +
>/* The minimum number of iterations performed by the epilogue.  This
>   is 1 when peeling for gaps because we always need a final scalar
>   iteration.  */



Re: [PATCH 6/6] sh: Fixes for RTL checking

2017-02-21 Thread Segher Boessenkool
On Wed, Feb 22, 2017 at 12:16:43AM +0900, Oleg Endo wrote:
> That SUBREG vs. REG stuff is annoying.  Isn't there a simple function
> that just does the right thing which can be used instead of manually
> open-coding these checks over and over again?

There is reg_or_subregno, which would even work here :-)


Segher


[committed] Avoid pattern-recognizing memset/memcpy/memmove for loops with non-generic address spaces (PR tree-optimization/79649)

2017-02-21 Thread Jakub Jelinek
Hi!

At least the library routines for these builtins can't deal with non-generic
address spaces, so we shouldn't transform loops setting or copying to and/or
from such address spaces into the builtins.

Bootstrapped/regtested on x86_64-linux and i686-linux, preapproved by
Richard in the PR, committed to trunk.

2017-02-21  Jakub Jelinek  

PR tree-optimization/79649
* tree-loop-distribution.c (classify_partition): Give up on
non-generic address space loads/stores.

* gcc.target/i386/pr79649.c: New test.

--- gcc/tree-loop-distribution.c.jj 2017-01-30 09:31:47.0 +0100
+++ gcc/tree-loop-distribution.c2017-02-21 09:31:52.484838050 +0100
@@ -1072,6 +1072,13 @@ classify_partition (loop_p loop, struct
   /* But exactly one store and/or load.  */
   for (j = 0; RDG_DATAREFS (rdg, i).iterate (j, &dr); ++j)
{
+ tree type = TREE_TYPE (DR_REF (dr));
+
+ /* The memset, memcpy and memmove library calls are only
+able to deal with generic address space.  */
+ if (!ADDR_SPACE_GENERIC_P (TYPE_ADDR_SPACE (type)))
+   return;
+
  if (DR_IS_READ (dr))
{
  if (single_load != NULL)
--- gcc/testsuite/gcc.target/i386/pr79649.c.jj  2017-02-21 09:49:51.404547952 
+0100
+++ gcc/testsuite/gcc.target/i386/pr79649.c 2017-02-21 09:49:01.0 
+0100
@@ -0,0 +1,53 @@
+/* PR tree-optimization/79649 */
+/* { dg-do compile } */
+/* { dg-options "-O3 -fdump-tree-optimized" } */
+/* { dg-final { scan-tree-dump-not "__builtin_memset" "optimized" } } */
+/* { dg-final { scan-tree-dump-not "__builtin_memcpy" "optimized" } } */
+/* { dg-final { scan-tree-dump-not "__builtin_memmove" "optimized" } } */
+
+typedef __SIZE_TYPE__ size_t;
+
+void
+f1 (unsigned char __seg_gs *s, size_t n)
+{
+  for (size_t i = 0; i < n; ++i)
+s[i] = 0;
+}
+
+void
+f2 (unsigned char __seg_gs *__restrict d, unsigned char __seg_gs *__restrict 
s, size_t n)
+{
+  for (size_t i = 0; i < n; ++i)
+d[i] = s[i];
+}
+
+void
+f3 (unsigned char __seg_gs *__restrict d, unsigned char *__restrict s, size_t 
n)
+{
+  for (size_t i = 0; i < n; ++i)
+d[i] = s[i];
+}
+
+void
+f4 (unsigned char *__restrict d, unsigned char __seg_gs *__restrict s, size_t 
n)
+{
+  for (size_t i = 0; i < n; ++i)
+d[i] = s[i];
+}
+
+void
+f5 (unsigned char __seg_gs *__restrict d, unsigned char __seg_fs *__restrict 
s, size_t n)
+{
+  for (size_t i = 0; i < n; ++i)
+d[i] = s[i];
+}
+
+struct A { int a; char b[1024]; };
+extern struct A __seg_gs a;
+
+void
+f6 (size_t n)
+{
+  for (size_t i = 0; i < n; ++i)
+a.b[i] = 0;
+}

Jakub


[C++ PATCH] Fix decomp error recovery (PR c++/79654)

2017-02-21 Thread Jakub Jelinek
Hi!

This patch undoes part of my r245218 change, there is nothing wrong with
the decomposition type, it is better to handle it in tsubst_decomp_names.
Plus Paolo's decl2.c change that also fixes the ICE.

I think we want both, there could be other spots that aren't happy about
error_mark_node type, on the other side type might be error_mark_node
for other reasons.

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

2017-02-21  Jakub Jelinek  
Paolo Carlini  

PR c++/79654
* decl.c (cp_finish_decomp): Don't set decl's type to error_mark_node
on error.
* pt.c (tsubst_decomp_names): Return error_mark_node if the first
decl after the decomposition artificial decl has error_mark_node.
* decl2.c (prune_vars_needing_no_initialization): Use error_operand_p
instead of just == error_mark_node comparison.

* g++.dg/cpp1z/decomp26.C: New test.

--- gcc/cp/decl.c.jj2017-02-20 13:43:21.0 +0100
+++ gcc/cp/decl.c   2017-02-21 11:42:31.832447757 +0100
@@ -7385,7 +7385,6 @@ cp_finish_decomp (tree decl, tree first,
}
  first = DECL_CHAIN (first);
}
-  TREE_TYPE (decl) = error_mark_node;
   if (DECL_P (decl) && DECL_NAMESPACE_SCOPE_P (decl))
SET_DECL_ASSEMBLER_NAME (decl, get_identifier (""));
   return;
--- gcc/cp/pt.c.jj  2017-02-20 13:43:21.0 +0100
+++ gcc/cp/pt.c 2017-02-21 11:45:48.736860963 +0100
@@ -15610,6 +15610,11 @@ tsubst_decomp_names (tree decl, tree pat
&& DECL_NAME (decl2);
decl2 = DECL_CHAIN (decl2))
 {
+  if (TREE_TYPE (decl2) == error_mark_node && *cnt == 0)
+   {
+ gcc_assert (errorcount);
+ return error_mark_node;
+   }
   (*cnt)++;
   gcc_assert (DECL_HAS_VALUE_EXPR_P (decl2));
   tree v = DECL_VALUE_EXPR (decl2);
--- gcc/cp/decl2.c.jj   2017-02-13 20:30:18.0 +0100
+++ gcc/cp/decl2.c  2017-02-21 15:32:38.327611223 +0100
@@ -3879,7 +3879,7 @@ prune_vars_needing_no_initialization (tr
   tree init = TREE_PURPOSE (t);
 
   /* Deal gracefully with error.  */
-  if (decl == error_mark_node)
+  if (error_operand_p (decl))
{
  var = &TREE_CHAIN (t);
  continue;
--- gcc/testsuite/g++.dg/cpp1z/decomp26.C.jj2017-02-21 11:48:20.356261173 
+0100
+++ gcc/testsuite/g++.dg/cpp1z/decomp26.C   2017-02-21 11:47:56.0 
+0100
@@ -0,0 +1,6 @@
+// PR c++/79654
+// { dg-do compile { target c++11 } }
+// { dg-options "" }
+
+template T &make();// { dg-warning "decomposition 
declaration only available with" "" { target c++14_down } .+1 }
+auto [d1, d2] = make();   // { dg-error "cannot decompose non-array 
non-class type" }

Jakub


[PATCH] Fix ICE with -fcheck-pointer-bounds -mmpx (PR target/79633)

2017-02-21 Thread Jakub Jelinek
Hi!

This function accesses arguments of builtin call without checking
the right arguments are actually provided.  Fixed thusly,
bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

The is_gimple_call in there is meant as a performance thing, we could
call gimple_call_builtin_p first and then only check
gimple_call_with_bounds_p, but that would unnecessarily test compatibility
of arguments even for calls without bounds.

2017-02-21  Jakub Jelinek  

PR target/79633
* tree-chkp-opt.c (chkp_optimize_string_function_calls): Use
is_gimple_call instead of comparing gimple_code with GIMPLE_CALL.
Use gimple_call_builtin_p.

* gcc.target/i386/mpx/pr79633.c: New test.

--- gcc/tree-chkp-opt.c.jj  2017-01-01 12:45:37.0 +0100
+++ gcc/tree-chkp-opt.c 2017-02-21 12:06:44.163025698 +0100
@@ -964,15 +964,12 @@ chkp_optimize_string_function_calls (voi
  gimple *stmt = gsi_stmt (i);
  tree fndecl;
 
- if (gimple_code (stmt) != GIMPLE_CALL
- || !gimple_call_with_bounds_p (stmt))
+ if (!is_gimple_call (stmt)
+ || !gimple_call_with_bounds_p (stmt)
+ || !gimple_call_builtin_p (stmt, BUILT_IN_NORMAL))
continue;
 
  fndecl = gimple_call_fndecl (stmt);
-
- if (!fndecl || DECL_BUILT_IN_CLASS (fndecl) != BUILT_IN_NORMAL)
-   continue;
-
  if (DECL_FUNCTION_CODE (fndecl) == BUILT_IN_MEMCPY_CHKP
  || DECL_FUNCTION_CODE (fndecl) == BUILT_IN_MEMPCPY_CHKP
  || DECL_FUNCTION_CODE (fndecl) == BUILT_IN_MEMMOVE_CHKP
--- gcc/testsuite/gcc.target/i386/mpx/pr79633.c.jj  2017-02-21 
12:09:54.034537817 +0100
+++ gcc/testsuite/gcc.target/i386/mpx/pr79633.c 2017-02-21 12:09:29.0 
+0100
@@ -0,0 +1,11 @@
+/* PR target/79633 */
+/* { dg-do compile } */
+/* { dg-options "-fcheck-pointer-bounds -mmpx -w -O2" } */
+
+extern void *memcpy ();
+
+void
+foo ()
+{
+  memcpy ();
+}

Jakub


[C/C++ PATCH] Fix mode attribute handling (PR c++/79641)

2017-02-21 Thread Jakub Jelinek
Hi!

On the following testcase, we have TYPE_READONLY integer type
before handling the mode attribute and transform it into
a signed char type (without TYPE_READONLY), which causes ICE in the FE,
because it isn't const anymore.

Fixed by making sure we have the same quals as before applying the mode
attribute.  Bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

2017-02-21  Jakub Jelinek  

PR c++/79641
* c-attribs.c (handle_mode_attribute): Use build_qualified_type to
preserve quals.

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

--- gcc/c-family/c-attribs.c.jj 2017-01-01 12:45:46.0 +0100
+++ gcc/c-family/c-attribs.c2017-02-21 12:38:10.105547005 +0100
@@ -1430,7 +1430,7 @@ handle_mode_attribute (tree *node, tree
  return NULL_TREE;
}
 
-  *node = typefm;
+  *node = build_qualified_type (typefm, TYPE_QUALS (type));
 }
 
   return NULL_TREE;
--- gcc/testsuite/c-c++-common/pr79641.c.jj 2017-02-21 12:43:24.246466684 
+0100
+++ gcc/testsuite/c-c++-common/pr79641.c2017-02-21 12:43:18.132546096 
+0100
@@ -0,0 +1,4 @@
+/* PR c++/79641 */
+/* { dg-do compile } */
+
+const int __attribute__((__mode__ (__QI__))) i = 0;

Jakub


[C++ PATCH] Diagnose constexpr store to ARRAY_REF with negative index (PR c++/79655)

2017-02-21 Thread Jakub Jelinek
Hi!

The following patch fixes ICE where we try to store a[-1].  The problem
is that we don't check the bounds first and then later on we assert it
is not negative.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

2017-02-21  Jakub Jelinek  

PR c++/79655
* constexpr.c (cxx_eval_array_reference): Diagnose negative subscript.

* g++.dg/cpp1y/constexpr-79655.C: New test.

--- gcc/cp/constexpr.c.jj   2017-02-17 18:29:21.0 +0100
+++ gcc/cp/constexpr.c  2017-02-21 13:49:06.530261975 +0100
@@ -2263,9 +2263,10 @@ cxx_eval_array_reference (const constexp
   nelts = cxx_eval_constant_expression (ctx, nelts, false, non_constant_p,
overflow_p);
   VERIFY_CONSTANT (nelts);
-  if (lval
-  ? !tree_int_cst_le (index, nelts)
-  : !tree_int_cst_lt (index, nelts))
+  if ((lval
+   ? !tree_int_cst_le (index, nelts)
+   : !tree_int_cst_lt (index, nelts))
+  || tree_int_cst_sgn (index) < 0)
 {
   diag_array_subscript (ctx, ary, index);
   *non_constant_p = true;
--- gcc/testsuite/g++.dg/cpp1y/constexpr-79655.C.jj 2017-02-21 
14:14:37.245395108 +0100
+++ gcc/testsuite/g++.dg/cpp1y/constexpr-79655.C2017-02-21 
14:14:07.0 +0100
@@ -0,0 +1,18 @@
+// PR c++/79655
+// { dg-do compile { target c++14 } }
+
+constexpr int
+foo (int x, int y)
+{
+  int a[6] = { 1, 2, 3, 4, 5, 6 };
+  a[x] = 0;
+  return a[y];
+}
+
+constexpr int b = foo (0, -1); // { dg-error "is outside the bounds" }
+constexpr int c = foo (0, 6);  // { dg-error "is outside the bounds" }
+constexpr int d = foo (6, 0);  // { dg-error "is outside the bounds" }
+constexpr int e = foo (-1, 0); // { dg-error "is outside the bounds" }
+static_assert (foo (5, 5) == 0, "");
+static_assert (foo (4, 5) == 6, "");
+static_assert (foo (5, 4) == 5, "");
 
Jakub


[C++ PATCH] Fix ICE with constexpr store to pointer to method (PR c++/79639)

2017-02-21 Thread Jakub Jelinek
Hi!

Apparently we can end up trying to store into a pointer-to-member
that has a PTRMEM_CST as its current value.  Later code in
cxx_eval_store_expression is upset that it isn't a CONSTRUCTOR when
the type is actually aggregate.

The following patch fixes that, bootstrapped/regtested on x86_64-linux
and i686-linux, though I admit I'm not really sure if this is the best fix.

2017-02-21  Jakub Jelinek  

PR c++/79639
* constexpr.c (cxx_eval_store_expression): If *valp is a PTRMEM_CST,
call cplus_expand_constant on it first.

* g++.dg/cpp1y/constexpr-79639.C: New test. 

--- gcc/cp/constexpr.c.jj   2017-02-21 13:49:06.0 +0100
+++ gcc/cp/constexpr.c  2017-02-21 14:57:30.290440638 +0100
@@ -3518,11 +3518,12 @@ cxx_eval_store_expression (const constex
 wants to modify it.  */
   if (*valp == NULL_TREE)
{
- *valp = new_ctx.ctor = build_constructor (type, NULL);
- CONSTRUCTOR_NO_IMPLICIT_ZERO (new_ctx.ctor) = no_zero_init;
+ *valp = build_constructor (type, NULL);
+ CONSTRUCTOR_NO_IMPLICIT_ZERO (*valp) = no_zero_init;
}
-  else
-   new_ctx.ctor = *valp;
+  else if (TREE_CODE (*valp) == PTRMEM_CST)
+   *valp = cplus_expand_constant (*valp);
+  new_ctx.ctor = *valp;
   new_ctx.object = target;
 }
 
--- gcc/testsuite/g++.dg/cpp1y/constexpr-79639.C.jj 2017-02-21 
15:06:42.625475572 +0100
+++ gcc/testsuite/g++.dg/cpp1y/constexpr-79639.C2017-02-21 
15:06:07.0 +0100
@@ -0,0 +1,27 @@
+// PR c++/79639
+// { dg-do compile { target c++14 } }
+
+struct A
+{
+  void foo () {}
+  void bar () {}
+};
+typedef void (A::*T) ();
+
+constexpr T
+foo (T f)
+{
+  f = 0;
+  return f;
+}
+
+constexpr T
+bar (T f)
+{
+  f = &A::bar;
+  return f;
+}
+
+constexpr T a = foo (&A::foo);
+constexpr T b = foo (&A::foo);
+static_assert (a == nullptr, "");

Jakub


[PATCH] Fix ICE with selective scheduling and VTA (PR target/79570)

2017-02-21 Thread Jakub Jelinek
Hi!

We ICE in the following hunk which has been added by Alexandre back
in VTA merge.  A fairly recent change started clearing BLOCK_FOR_INSN
for instructions that are temporarily removed from the IL (but those
would never previously satisfy the condition - they would never be
heads of their block).  While it isn't well understood why the condition
is in there, the patch pretty much restores the previous behavior for
such DEBUG_INSNs.  Bootstrapped/regtested on x86_64-linux and i686-linux,
ok for trunk?

2017-02-21  Jakub Jelinek  

PR target/79570
* sel-sched.c (moveup_expr_cached): Don't call sel_bb_head
on temporarily removed DEBUG_INSNs.

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

--- gcc/sel-sched.c.jj  2017-01-01 12:45:38.0 +0100
+++ gcc/sel-sched.c 2017-02-17 14:14:06.493525368 +0100
@@ -2529,6 +2529,7 @@ moveup_expr_cached (expr_t expr, insn_t
 }
 
   if (DEBUG_INSN_P (EXPR_INSN_RTX (expr))
+  && BLOCK_FOR_INSN (EXPR_INSN_RTX (expr))
   && (sel_bb_head (BLOCK_FOR_INSN (EXPR_INSN_RTX (expr)))
  == EXPR_INSN_RTX (expr)))
 /* Don't use cached information for debug insns that are heads of
--- gcc/testsuite/gcc.dg/pr79570.c.jj   2017-02-17 14:37:15.183672738 +0100
+++ gcc/testsuite/gcc.dg/pr79570.c  2017-02-17 14:37:10.306738336 +0100
@@ -0,0 +1,6 @@
+/* PR target/79570 */
+/* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
+/* { dg-options "-O2 -fselective-scheduling2 -fvar-tracking-assignments" } */
+/* { dg-warning "changes selective scheduling" "" { target *-*-* } 0 } */
+
+#include "pr69956.c"

Jakub


Re: [PATCH] Fix ICE with selective scheduling and VTA (PR target/79570)

2017-02-21 Thread Jeff Law

On 02/21/2017 09:49 AM, Jakub Jelinek wrote:

Hi!

We ICE in the following hunk which has been added by Alexandre back
in VTA merge.  A fairly recent change started clearing BLOCK_FOR_INSN
for instructions that are temporarily removed from the IL (but those
would never previously satisfy the condition - they would never be
heads of their block).  While it isn't well understood why the condition
is in there, the patch pretty much restores the previous behavior for
such DEBUG_INSNs.  Bootstrapped/regtested on x86_64-linux and i686-linux,
ok for trunk?

2017-02-21  Jakub Jelinek  

PR target/79570
* sel-sched.c (moveup_expr_cached): Don't call sel_bb_head
on temporarily removed DEBUG_INSNs.

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

OK.
jeff



Re: [C/C++ PATCH] Fix mode attribute handling (PR c++/79641)

2017-02-21 Thread Jeff Law

On 02/21/2017 09:40 AM, Jakub Jelinek wrote:

Hi!

On the following testcase, we have TYPE_READONLY integer type
before handling the mode attribute and transform it into
a signed char type (without TYPE_READONLY), which causes ICE in the FE,
because it isn't const anymore.

Fixed by making sure we have the same quals as before applying the mode
attribute.  Bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

2017-02-21  Jakub Jelinek  

PR c++/79641
* c-attribs.c (handle_mode_attribute): Use build_qualified_type to
preserve quals.

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

OK.
jeff



[PATCH 0/2] [ARM] PR61551 addressing mode costs

2017-02-21 Thread charles . baylis
From: Charles Baylis 

Hi Ramana,

This patch set continues previous work on fixing the cost calculations for MEMs
which use different addressing modes. It implements the approach we discussed
at Linaro Connect BKK16.

I have included some notes on the patch set as follows:


Background:

The motivating problem is that this function:
  char *f(char *p, int8x8x4_t v, int r) { vst4_s8(p, v); p+=32; return p; }
compiles to:
mov r3, r0
addsr0, r0, #32
vst4.8  {d0-d3}, [r3]
bx  lr
but we would like to get:
vst4.8  {d0-d3}, [r0]!
bx  lr

Although the ARM back end contains patterns for the write-back forms of these
instructions, they are not currently generated. The reason for this is that the
auto-inc-dec phase does not perform this optimisation because arm_rtx_costs
incorrectly calculates the cost of "vst4.8  {d0-d3}, [r0]!" as much higher than
"vst4.8  {d0-d3}, [r3]". For that reason, it considers the POST_INC form to be
worse than the initial sequence of vst4/add and does not perform the
transformation.

In fact, GCC6 has regressions compared to GCC5 in this area, and no longer
does post-indexed addressing for int64_t or 64 bit vector types.


Solution:

Change cost calculation for MEMs so that the cost of the memory access
is computed separately from the cost of the addressing mode. A new
table-driven mechanism is introduced for the costs of the addressing modes.

The first patch in the series implements the calculation of the cost of
the memory access.

The second patch adds the table-driven model of the extra cost of the
selected addressing mode. I don't have access to a lot of CPU pipeline
information, so most CPUs use the generic cost table, with the exception of
Cortex-A57.


Testing:

I did "make check" on arm-linux-gnueabihf with qemu. This patch fixes one test
failure in lp1243022.c.


Benchmarking:

On Cortex-A15, SPEC2006 and a popular suite of embedded benchmarks perform the
same as before this patch is applied.  This is expected, the expected gain is
in code quality for hand-written NEON intrinsics code.



Charles Baylis (2):
  [ARM] Refactor costs calculation for MEM.
  [ARM] Add table of costs for AAarch32 addressing modes.

 gcc/config/arm/aarch-common-protos.h |  16 +
 gcc/config/arm/aarch-cost-tables.h   |  54 ++--
 gcc/config/arm/arm.c | 120 ++-
 3 files changed, 154 insertions(+), 36 deletions(-)

-- 
2.7.4



[PATCH 1/2] [ARM] Refactor costs calculation for MEM.

2017-02-21 Thread charles . baylis
From: Charles Baylis 

This patch moves the calculation of costs for MEM into a
separate function, and reforms the calculation into two
parts. Firstly any additional cost of the addressing mode
is calculated, and then the cost of the memory access itself
is added.

In this patch, the calculation of the cost of the addressing
mode is left as a placeholder, to be added in a subsequent
patch.

gcc/ChangeLog:

  Charles Baylis  

* config/arm/arm.c (arm_mem_costs): New function.
(arm_rtx_costs_internal): Use arm_mem_costs.

Change-Id: I99e93406ea39ee31f71c7bf428ad3e127b7a618e
---
 gcc/config/arm/arm.c | 66 +---
 1 file changed, 42 insertions(+), 24 deletions(-)

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 6cae178..7f002f1 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -9072,6 +9072,47 @@ arm_unspec_cost (rtx x, enum rtx_code /* outer_code */, 
bool speed_p, int *cost)
  } \
while (0);
 
+/* Helper function for arm_rtx_costs_internal. Calculates the cost of a MEM,
+   considering the costs of the addressing mode and memory access
+   separately.  */
+static bool
+arm_mem_costs (rtx x, const struct cpu_cost_table *extra_cost,
+  int *cost, bool speed_p)
+{
+  machine_mode mode = GET_MODE (x);
+  if (flag_pic
+  && GET_CODE (XEXP (x, 0)) == PLUS
+  && will_be_in_index_register (XEXP (XEXP (x, 0), 1)))
+/* This will be split into two instructions.  Add the cost of the
+   additional instruction here.  The cost of the memory access is computed
+   below.  See arm.md:calculate_pic_address.  */
+*cost = COSTS_N_INSNS (1);
+  else
+*cost = 0;
+
+  /* Calculate cost of the addressing mode.  */
+  if (speed_p)
+  {
+/* TODO: Add table-driven costs for addressing modes.  */
+  }
+
+  /* cost of memory access */
+  if (speed_p)
+  {
+/* data transfer is transfer size divided by bus width.  */
+int bus_width = arm_arch7 ? 8 : 4;
+*cost += COSTS_N_INSNS((GET_MODE_SIZE (mode) + bus_width - 1) / bus_width);
+*cost += extra_cost->ldst.load;
+  } else {
+*cost += COSTS_N_INSNS (1);
+  }
+
+  return true;
+}
+/* Convert fron bytes to ints.  */
+#define ARM_NUM_INTS(X) (((X) + UNITS_PER_WORD - 1) / UNITS_PER_WORD)
+
+
 /* RTX costs.  Make an estimate of the cost of executing the operation
X, which is contained with an operation with code OUTER_CODE.
SPEED_P indicates whether the cost desired is the performance cost,
@@ -9152,30 +9193,7 @@ arm_rtx_costs_internal (rtx x, enum rtx_code code, enum 
rtx_code outer_code,
   return false;
 
 case MEM:
-  /* A memory access costs 1 insn if the mode is small, or the address is
-a single register, otherwise it costs one insn per word.  */
-  if (REG_P (XEXP (x, 0)))
-   *cost = COSTS_N_INSNS (1);
-  else if (flag_pic
-  && GET_CODE (XEXP (x, 0)) == PLUS
-  && will_be_in_index_register (XEXP (XEXP (x, 0), 1)))
-   /* This will be split into two instructions.
-  See arm.md:calculate_pic_address.  */
-   *cost = COSTS_N_INSNS (2);
-  else
-   *cost = COSTS_N_INSNS (ARM_NUM_REGS (mode));
-
-  /* For speed optimizations, add the costs of the address and
-accessing memory.  */
-  if (speed_p)
-#ifdef NOT_YET
-   *cost += (extra_cost->ldst.load
- + arm_address_cost (XEXP (x, 0), mode,
- ADDR_SPACE_GENERIC, speed_p));
-#else
-*cost += extra_cost->ldst.load;
-#endif
-  return true;
+  return arm_mem_costs (x, extra_cost, cost, speed_p);
 
 case PARALLEL:
 {
-- 
2.7.4



[PATCH 2/2] [ARM] Add table of costs for AAarch32 addressing modes.

2017-02-21 Thread charles . baylis
From: Charles Baylis 

This patch adds support for modelling the varying costs of
different addressing modes. The generic cost table treats
all addressing modes as having equal cost. The cost table
for Cortex-A57 is derived from 
http://infocenter.arm.com/help/topic/com.arm.doc.uan0015b/Cortex_A57_Software_Optimization_Guide_external.pdf
and treats addressing modes with write-back as having a
cost equal to one additional instruction.

gcc/ChangeLog:

  Charles Baylis  

* config/arm/aarch-common-protos.h (enum arm_addr_mode_op): New.
(struct addr_mode_cost_table): New.
(struct cpu_cost_table): Add pointer to an addr_mode_cost_table.
* config/arm/aarch-cost-tables.h: (generic_addr_mode_costs): New.
(generic_extra_costs) Initialise aarch32_addr_mode.
(cortexa53_extra_costs) Likewise.
(addr_mode_costs_cortexa57) New.
(cortexa57_extra_costs) Initialise aarch32_addr_mode.
(exynosm1_extra_costs) Likewise.
(xgene1_extra_costs) Likewise.
(qdf24xx_extra_costs) Likewise.
* config/arm/arm.c (cortexa9_extra_costs) Initialise aarch32_addr_mode.
(cortexa9_extra_costs) Likewise.
(cortexa8_extra_costs) Likewise.
(cortexa5_extra_costs) Likewise.
(cortexa7_extra_costs) Likewise.
(cortexa12_extra_costs) Likewise.
(cortexv7m_extra_costs) Likewise.
(arm_mem_costs): Use table lookup to calculate cost of addressing
mode.

Change-Id: If71bd7c4f4bb876c5ed82dc28791130efb8bf89e
---
 gcc/config/arm/aarch-common-protos.h | 16 +++
 gcc/config/arm/aarch-cost-tables.h   | 54 ++
 gcc/config/arm/arm.c | 56 ++--
 3 files changed, 113 insertions(+), 13 deletions(-)

diff --git a/gcc/config/arm/aarch-common-protos.h 
b/gcc/config/arm/aarch-common-protos.h
index 7c2bb4c..f6fcc94 100644
--- a/gcc/config/arm/aarch-common-protos.h
+++ b/gcc/config/arm/aarch-common-protos.h
@@ -130,6 +130,21 @@ struct vector_cost_table
   const int alu;
 };
 
+enum arm_addr_mode_op
+{
+   AMO_DEFAULT,
+   AMO_NO_WB,
+   AMO_WB,
+   AMO_MAX /* for array size */
+};
+
+struct addr_mode_cost_table
+{
+   const int integer[AMO_MAX];
+   const int fp[AMO_MAX];
+   const int vector[AMO_MAX];
+};
+
 struct cpu_cost_table
 {
   const struct alu_cost_table alu;
@@ -137,6 +152,7 @@ struct cpu_cost_table
   const struct mem_cost_table ldst;
   const struct fp_cost_table fp[2]; /* SFmode and DFmode.  */
   const struct vector_cost_table vect;
+  const struct addr_mode_cost_table *aarch32_addr_mode;
 };
 
 
diff --git a/gcc/config/arm/aarch-cost-tables.h 
b/gcc/config/arm/aarch-cost-tables.h
index 68f84b0..e3d257f 100644
--- a/gcc/config/arm/aarch-cost-tables.h
+++ b/gcc/config/arm/aarch-cost-tables.h
@@ -22,6 +22,16 @@
 #ifndef GCC_AARCH_COST_TABLES_H
 #define GCC_AARCH_COST_TABLES_H
 
+const struct addr_mode_cost_table generic_addr_mode_costs =
+{
+  /* int */
+  { 0, 0, 0 },
+  /* float */
+  { 0, 0, 0 },
+  /* vector */
+  { 0, 0, 0 }
+};
+
 const struct cpu_cost_table generic_extra_costs =
 {
   /* ALU */
@@ -122,7 +132,9 @@ const struct cpu_cost_table generic_extra_costs =
   /* Vector */
   {
 COSTS_N_INSNS (1)  /* alu.  */
-  }
+  },
+  /* Addressing mode */
+  &generic_addr_mode_costs
 };
 
 const struct cpu_cost_table cortexa53_extra_costs =
@@ -225,6 +237,30 @@ const struct cpu_cost_table cortexa53_extra_costs =
   /* Vector */
   {
 COSTS_N_INSNS (1)  /* alu.  */
+  },
+  /* Addressing mode */
+  &generic_addr_mode_costs
+};
+
+const struct addr_mode_cost_table addr_mode_costs_cortexa57 =
+{
+  /* int */
+  {
+0,
+0,
+COSTS_N_INSNS (1),
+  },
+  /* float */
+  {
+0,
+0,
+COSTS_N_INSNS (1),
+  },
+  /* vector */
+  {
+0,
+0,
+COSTS_N_INSNS (1),
   }
 };
 
@@ -328,7 +364,9 @@ const struct cpu_cost_table cortexa57_extra_costs =
   /* Vector */
   {
 COSTS_N_INSNS (1)  /* alu.  */
-  }
+  },
+  /* Addressing mode */
+  &addr_mode_costs_cortexa57
 };
 
 const struct cpu_cost_table exynosm1_extra_costs =
@@ -431,7 +469,9 @@ const struct cpu_cost_table exynosm1_extra_costs =
   /* Vector */
   {
 COSTS_N_INSNS (0)  /* alu.  */
-  }
+  },
+  /* Addressing mode */
+  &generic_addr_mode_costs
 };
 
 const struct cpu_cost_table xgene1_extra_costs =
@@ -534,7 +574,9 @@ const struct cpu_cost_table xgene1_extra_costs =
   /* Vector */
   {
 COSTS_N_INSNS (2)  /* alu.  */
-  }
+  },
+  /* Addressing mode */
+  &generic_addr_mode_costs
 };
 
 const struct cpu_cost_table qdf24xx_extra_costs =
@@ -637,7 +679,9 @@ const struct cpu_cost_table qdf24xx_extra_costs =
   /* Vector */
   {
 COSTS_N_INSNS (1)  /* alu.  */
-  }
+  },
+  /* Addressing mode */
+  &generic_addr_mode_costs
 };
 
 #endif /* GCC_AARCH_COST_TABLES_H */
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 7f002f1..fe4cd73 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -1099,7 +1099,9 @@ 

RE: [PATCH 1/5] Handle WORD_REGISTER_OPERATIONS when reloading (subreg(reg))

2017-02-21 Thread Matthew Fortune
Eric Botcazou  writes:
> > Thanks for reporting. I'll take a brief look first but revert if the
> > issue isn't something that vaguely makes sense to me.
> 
> You need to restrict any WORD_REGISTER_OPERATIONS test to subword
> registers.

I've reverted this. I haven't been able to quickly find a change that
I both feel is right and works. I am wondering if I actually don't
need this change and that 'patch 3' (the change to
simplify_operand_subreg) is the actual fix for both issues I have seen.

I'll test my other change against an ARM build and wait a day to see
that ARM is at least working on trunk before committing the other
fix to this area of code.

Thanks,
Matthew 


Re: [PATCH 1/5] Handle WORD_REGISTER_OPERATIONS when reloading (subreg(reg))

2017-02-21 Thread Kyrill Tkachov


On 21/02/17 16:58, Matthew Fortune wrote:

Eric Botcazou  writes:

Thanks for reporting. I'll take a brief look first but revert if the
issue isn't something that vaguely makes sense to me.

You need to restrict any WORD_REGISTER_OPERATIONS test to subword
registers.

I've reverted this. I haven't been able to quickly find a change that
I both feel is right and works. I am wondering if I actually don't
need this change and that 'patch 3' (the change to
simplify_operand_subreg) is the actual fix for both issues I have seen.

I'll test my other change against an ARM build and wait a day to see
that ARM is at least working on trunk before committing the other
fix to this area of code.


Thanks Matthew.

Kyrill


Thanks,
Matthew




Re: [PATCH 1/5] Handle WORD_REGISTER_OPERATIONS when reloading (subreg(reg))

2017-02-21 Thread Richard Sandiford
Matthew Fortune  writes:
> Eric Botcazou  writes:
>> > Thanks for reporting. I'll take a brief look first but revert if the
>> > issue isn't something that vaguely makes sense to me.
>> 
>> You need to restrict any WORD_REGISTER_OPERATIONS test to subword
>> registers.
>
> I've reverted this. I haven't been able to quickly find a change that
> I both feel is right and works. I am wondering if I actually don't
> need this change and that 'patch 3' (the change to
> simplify_operand_subreg) is the actual fix for both issues I have seen.

I think that patch might have a similar problem though, in that it
applies WORD_REGISTER_OPERATIONS semantics to things that might be
vectors.

Thanks,
Richard


RE: [PATCH 1/5] Handle WORD_REGISTER_OPERATIONS when reloading (subreg(reg))

2017-02-21 Thread Matthew Fortune
Richard Sandiford  writes:
> Matthew Fortune  writes:
> > Eric Botcazou  writes:
> >> > Thanks for reporting. I'll take a brief look first but revert if
> >> > the issue isn't something that vaguely makes sense to me.
> >>
> >> You need to restrict any WORD_REGISTER_OPERATIONS test to subword
> >> registers.
> >
> > I've reverted this. I haven't been able to quickly find a change that
> > I both feel is right and works. I am wondering if I actually don't
> > need this change and that 'patch 3' (the change to
> > simplify_operand_subreg) is the actual fix for both issues I have
> seen.
> 
> I think that patch might have a similar problem though, in that it
> applies WORD_REGISTER_OPERATIONS semantics to things that might be
> vectors.

There is an amendment done as part of SPARC testing that limits the
change to modes that are no wider than a word. But, given that assumptions
coming from WORD_REGISTER_OPERATIONS can only be applied to integer
modes then it should also check both modes are MODE_INT as well.

All that said... I don't entirely follow why any target should be
reliant on subreg(mem) being simplified to use the outer mode. It is an
optimization certainly for some cases but I don't understand what rule
or guarantee we have that means reloading the inner MEM is illegal.

The latter point is not appropriate to debate for GCC 7 though.

Matthew


Re: [PATCH] Fix ICE with -fcheck-pointer-bounds -mmpx (PR target/79633)

2017-02-21 Thread Jeff Law

On 02/21/2017 09:37 AM, Jakub Jelinek wrote:

Hi!

This function accesses arguments of builtin call without checking
the right arguments are actually provided.  Fixed thusly,
bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

The is_gimple_call in there is meant as a performance thing, we could
call gimple_call_builtin_p first and then only check
gimple_call_with_bounds_p, but that would unnecessarily test compatibility
of arguments even for calls without bounds.

2017-02-21  Jakub Jelinek  

PR target/79633
* tree-chkp-opt.c (chkp_optimize_string_function_calls): Use
is_gimple_call instead of comparing gimple_code with GIMPLE_CALL.
Use gimple_call_builtin_p.

* gcc.target/i386/mpx/pr79633.c: New test.

OK.
jeff



Re: [PATCH][libgcc, fuchsia]

2017-02-21 Thread Josh Conner via gcc-patches

Ping^2?


On 2/2/17 11:22 AM, Josh Conner wrote:

Ping?


On 1/17/17 10:40 AM, Josh Conner wrote:

The attached patch adds fuchsia support to libgcc.

OK for trunk?

Thanks -

Josh

2017-01-17  Joshua Conner  

* config/arm/unwind-arm.h (_Unwind_decode_typeinfo_ptr): Use
pc-relative indirect handling for fuchsia.
* config/t-slibgcc-fuchsia: New file.
* config.host (*-*-fuchsia*, aarch64*-*-fuchsia*, arm*-*-fuchsia*,
x86_64-*-fuchsia*): Add definitions.







Re: [C++ PATCH] Fix ICE with constexpr store to pointer to method (PR c++/79639)

2017-02-21 Thread Jason Merrill
OK.

On Tue, Feb 21, 2017 at 8:44 AM, Jakub Jelinek  wrote:
> Hi!
>
> Apparently we can end up trying to store into a pointer-to-member
> that has a PTRMEM_CST as its current value.  Later code in
> cxx_eval_store_expression is upset that it isn't a CONSTRUCTOR when
> the type is actually aggregate.
>
> The following patch fixes that, bootstrapped/regtested on x86_64-linux
> and i686-linux, though I admit I'm not really sure if this is the best fix.
>
> 2017-02-21  Jakub Jelinek  
>
> PR c++/79639
> * constexpr.c (cxx_eval_store_expression): If *valp is a PTRMEM_CST,
> call cplus_expand_constant on it first.
>
> * g++.dg/cpp1y/constexpr-79639.C: New test.
>
> --- gcc/cp/constexpr.c.jj   2017-02-21 13:49:06.0 +0100
> +++ gcc/cp/constexpr.c  2017-02-21 14:57:30.290440638 +0100
> @@ -3518,11 +3518,12 @@ cxx_eval_store_expression (const constex
>  wants to modify it.  */
>if (*valp == NULL_TREE)
> {
> - *valp = new_ctx.ctor = build_constructor (type, NULL);
> - CONSTRUCTOR_NO_IMPLICIT_ZERO (new_ctx.ctor) = no_zero_init;
> + *valp = build_constructor (type, NULL);
> + CONSTRUCTOR_NO_IMPLICIT_ZERO (*valp) = no_zero_init;
> }
> -  else
> -   new_ctx.ctor = *valp;
> +  else if (TREE_CODE (*valp) == PTRMEM_CST)
> +   *valp = cplus_expand_constant (*valp);
> +  new_ctx.ctor = *valp;
>new_ctx.object = target;
>  }
>
> --- gcc/testsuite/g++.dg/cpp1y/constexpr-79639.C.jj 2017-02-21 
> 15:06:42.625475572 +0100
> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-79639.C2017-02-21 
> 15:06:07.0 +0100
> @@ -0,0 +1,27 @@
> +// PR c++/79639
> +// { dg-do compile { target c++14 } }
> +
> +struct A
> +{
> +  void foo () {}
> +  void bar () {}
> +};
> +typedef void (A::*T) ();
> +
> +constexpr T
> +foo (T f)
> +{
> +  f = 0;
> +  return f;
> +}
> +
> +constexpr T
> +bar (T f)
> +{
> +  f = &A::bar;
> +  return f;
> +}
> +
> +constexpr T a = foo (&A::foo);
> +constexpr T b = foo (&A::foo);
> +static_assert (a == nullptr, "");
>
> Jakub


Re: [C++ PATCH] Diagnose constexpr store to ARRAY_REF with negative index (PR c++/79655)

2017-02-21 Thread Jason Merrill
OK.

On Tue, Feb 21, 2017 at 8:42 AM, Jakub Jelinek  wrote:
> Hi!
>
> The following patch fixes ICE where we try to store a[-1].  The problem
> is that we don't check the bounds first and then later on we assert it
> is not negative.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
>
> 2017-02-21  Jakub Jelinek  
>
> PR c++/79655
> * constexpr.c (cxx_eval_array_reference): Diagnose negative subscript.
>
> * g++.dg/cpp1y/constexpr-79655.C: New test.
>
> --- gcc/cp/constexpr.c.jj   2017-02-17 18:29:21.0 +0100
> +++ gcc/cp/constexpr.c  2017-02-21 13:49:06.530261975 +0100
> @@ -2263,9 +2263,10 @@ cxx_eval_array_reference (const constexp
>nelts = cxx_eval_constant_expression (ctx, nelts, false, non_constant_p,
> overflow_p);
>VERIFY_CONSTANT (nelts);
> -  if (lval
> -  ? !tree_int_cst_le (index, nelts)
> -  : !tree_int_cst_lt (index, nelts))
> +  if ((lval
> +   ? !tree_int_cst_le (index, nelts)
> +   : !tree_int_cst_lt (index, nelts))
> +  || tree_int_cst_sgn (index) < 0)
>  {
>diag_array_subscript (ctx, ary, index);
>*non_constant_p = true;
> --- gcc/testsuite/g++.dg/cpp1y/constexpr-79655.C.jj 2017-02-21 
> 14:14:37.245395108 +0100
> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-79655.C2017-02-21 
> 14:14:07.0 +0100
> @@ -0,0 +1,18 @@
> +// PR c++/79655
> +// { dg-do compile { target c++14 } }
> +
> +constexpr int
> +foo (int x, int y)
> +{
> +  int a[6] = { 1, 2, 3, 4, 5, 6 };
> +  a[x] = 0;
> +  return a[y];
> +}
> +
> +constexpr int b = foo (0, -1); // { dg-error "is outside the bounds" }
> +constexpr int c = foo (0, 6);  // { dg-error "is outside the bounds" }
> +constexpr int d = foo (6, 0);  // { dg-error "is outside the bounds" }
> +constexpr int e = foo (-1, 0); // { dg-error "is outside the bounds" }
> +static_assert (foo (5, 5) == 0, "");
> +static_assert (foo (4, 5) == 6, "");
> +static_assert (foo (5, 4) == 5, "");
>
> Jakub


Re: [C++ PATCH] Fix decomp error recovery (PR c++/79654)

2017-02-21 Thread Jason Merrill
OK.

On Tue, Feb 21, 2017 at 8:35 AM, Jakub Jelinek  wrote:
> Hi!
>
> This patch undoes part of my r245218 change, there is nothing wrong with
> the decomposition type, it is better to handle it in tsubst_decomp_names.
> Plus Paolo's decl2.c change that also fixes the ICE.
>
> I think we want both, there could be other spots that aren't happy about
> error_mark_node type, on the other side type might be error_mark_node
> for other reasons.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2017-02-21  Jakub Jelinek  
> Paolo Carlini  
>
> PR c++/79654
> * decl.c (cp_finish_decomp): Don't set decl's type to error_mark_node
> on error.
> * pt.c (tsubst_decomp_names): Return error_mark_node if the first
> decl after the decomposition artificial decl has error_mark_node.
> * decl2.c (prune_vars_needing_no_initialization): Use error_operand_p
> instead of just == error_mark_node comparison.
>
> * g++.dg/cpp1z/decomp26.C: New test.
>
> --- gcc/cp/decl.c.jj2017-02-20 13:43:21.0 +0100
> +++ gcc/cp/decl.c   2017-02-21 11:42:31.832447757 +0100
> @@ -7385,7 +7385,6 @@ cp_finish_decomp (tree decl, tree first,
> }
>   first = DECL_CHAIN (first);
> }
> -  TREE_TYPE (decl) = error_mark_node;
>if (DECL_P (decl) && DECL_NAMESPACE_SCOPE_P (decl))
> SET_DECL_ASSEMBLER_NAME (decl, get_identifier (""));
>return;
> --- gcc/cp/pt.c.jj  2017-02-20 13:43:21.0 +0100
> +++ gcc/cp/pt.c 2017-02-21 11:45:48.736860963 +0100
> @@ -15610,6 +15610,11 @@ tsubst_decomp_names (tree decl, tree pat
> && DECL_NAME (decl2);
> decl2 = DECL_CHAIN (decl2))
>  {
> +  if (TREE_TYPE (decl2) == error_mark_node && *cnt == 0)
> +   {
> + gcc_assert (errorcount);
> + return error_mark_node;
> +   }
>(*cnt)++;
>gcc_assert (DECL_HAS_VALUE_EXPR_P (decl2));
>tree v = DECL_VALUE_EXPR (decl2);
> --- gcc/cp/decl2.c.jj   2017-02-13 20:30:18.0 +0100
> +++ gcc/cp/decl2.c  2017-02-21 15:32:38.327611223 +0100
> @@ -3879,7 +3879,7 @@ prune_vars_needing_no_initialization (tr
>tree init = TREE_PURPOSE (t);
>
>/* Deal gracefully with error.  */
> -  if (decl == error_mark_node)
> +  if (error_operand_p (decl))
> {
>   var = &TREE_CHAIN (t);
>   continue;
> --- gcc/testsuite/g++.dg/cpp1z/decomp26.C.jj2017-02-21 11:48:20.356261173 
> +0100
> +++ gcc/testsuite/g++.dg/cpp1z/decomp26.C   2017-02-21 11:47:56.0 
> +0100
> @@ -0,0 +1,6 @@
> +// PR c++/79654
> +// { dg-do compile { target c++11 } }
> +// { dg-options "" }
> +
> +template T &make();// { dg-warning "decomposition 
> declaration only available with" "" { target c++14_down } .+1 }
> +auto [d1, d2] = make();   // { dg-error "cannot decompose non-array 
> non-class type" }
>
> Jakub


Re: [C++ PATCH] Fix ICE with decomp gimplification (PR sanitizer/79589)

2017-02-21 Thread Jason Merrill
OK.

On Mon, Feb 20, 2017 at 12:38 PM, Jakub Jelinek  wrote:
> Hi!
>
> When generic is unshared, we generally don't unshare DECL_VALUE_EXPRs,
> so those are assumed to be not shared, otherwise as in the testcase
> we can clear first argument of a COMPOUND_EXPR after it has been gimplified
> in one use and by that clobbering all others.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2017-02-20  Jakub Jelinek  
>
> PR sanitizer/79589
> * decl.c: Include gimplify.h.
> (cp_finish_decomp): Make sure there is no sharing of trees
> in between DECL_VALUE_EXPR of decomposition decls.
>
> * g++.dg/ubsan/pr79589.C: New test.
>
> --- gcc/cp/decl.c.jj2017-02-17 18:29:21.0 +0100
> +++ gcc/cp/decl.c   2017-02-20 11:45:55.281636544 +0100
> @@ -50,6 +50,7 @@ along with GCC; see the file COPYING3.
>  #include "plugin.h"
>  #include "cilk.h"
>  #include "builtins.h"
> +#include "gimplify.h"
>
>  /* Possible cases of bad specifiers type used by bad_specifiers. */
>  enum bad_spec_place {
> @@ -7467,7 +7468,7 @@ cp_finish_decomp (tree decl, tree first,
> {
>   TREE_TYPE (v[i]) = eltype;
>   layout_decl (v[i], 0);
> - tree t = dexp;
> + tree t = unshare_expr (dexp);
>   t = build4_loc (DECL_SOURCE_LOCATION (v[i]), ARRAY_REF,
>   eltype, t, size_int (i), NULL_TREE,
>   NULL_TREE);
> @@ -7486,7 +7487,7 @@ cp_finish_decomp (tree decl, tree first,
> {
>   TREE_TYPE (v[i]) = eltype;
>   layout_decl (v[i], 0);
> - tree t = dexp;
> + tree t = unshare_expr (dexp);
>   t = build1_loc (DECL_SOURCE_LOCATION (v[i]),
>   i ? IMAGPART_EXPR : REALPART_EXPR, eltype,
>   t);
> @@ -7504,7 +7505,7 @@ cp_finish_decomp (tree decl, tree first,
> {
>   TREE_TYPE (v[i]) = eltype;
>   layout_decl (v[i], 0);
> - tree t = dexp;
> + tree t = unshare_expr (dexp);
>   convert_vector_to_array_for_subscript (DECL_SOURCE_LOCATION (v[i]),
>  &t, size_int (i));
>   t = build4_loc (DECL_SOURCE_LOCATION (v[i]), ARRAY_REF,
> @@ -7603,7 +7604,8 @@ cp_finish_decomp (tree decl, tree first,
>   continue;
> else
>   {
> -   tree tt = finish_non_static_data_member (field, t, NULL_TREE);
> +   tree tt = finish_non_static_data_member (field, unshare_expr (t),
> +NULL_TREE);
> if (REFERENCE_REF_P (tt))
>   tt = TREE_OPERAND (tt, 0);
> TREE_TYPE (v[i]) = TREE_TYPE (tt);
> --- gcc/testsuite/g++.dg/ubsan/pr79589.C.jj 2017-02-20 11:51:36.732130221 
> +0100
> +++ gcc/testsuite/g++.dg/ubsan/pr79589.C2017-02-20 11:52:09.012704729 
> +0100
> @@ -0,0 +1,13 @@
> +// PR sanitizer/79589
> +// { dg-do compile }
> +// { dg-options "-fsanitize=undefined -std=c++1z" }
> +
> +struct A { char a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, q, r; } 
> a[64];
> +
> +void
> +foo ()
> +{
> +  int z = 0;
> +  for (auto & [ b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, q, r, s ] : a)
> +z += b + c + d + e + f + g + h + i + j + k + l + m + n + o + p + q + r + 
> s;
> +}
>
> Jakub


[PATCH][PR tree-optimization/79621] Avoid path isolation on block with edge to itself

2017-02-21 Thread Jeff Law


Erroneous path isolation inherently duplicates blocks with erroneous 
behavior when reached via certain paths.  This allows us to modify one 
copy without affecting the other.


Block duplication can cause re-allocation of a PHI nodes in successor 
blocks and thus referencing the old PHI references stale data.


This is what's happening in this BZ.  We duplicate the block with an 
edge to itself.  This causes PHIs to be reallocated which 
gimple-ssa-isolate-paths can't handle.


This can only occur in a block which itself as a direct successor and 
which also has some other path that triggers erroneous behavior.


Fixed by avoiding isolation if the block with erroneous behaviour can 
reach itself.  This avoids the ICE, but does leave a missed optimization 
in the IL.  So I'll update the BZ rather than closing.


Bootstrapped and regression tested on x86_64.  Installing on the trunk.

Jeff
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index a58a516..a7b0b49 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2017-02-21 Jeff Law  
+
+   PR tree-optimization/79621
+   * gimple-ssa-isolate-paths.c (find_implicit_erroneous_behavior): Ignore
+   blocks with edges to themselves.
+
 2017-02-21  Jakub Jelinek  
 
PR target/79570
diff --git a/gcc/gimple-ssa-isolate-paths.c b/gcc/gimple-ssa-isolate-paths.c
index 25e8c8a..7babe09 100644
--- a/gcc/gimple-ssa-isolate-paths.c
+++ b/gcc/gimple-ssa-isolate-paths.c
@@ -35,6 +35,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree-ssa.h"
 #include "cfgloop.h"
 #include "tree-cfg.h"
+#include "cfganal.h"
 #include "intl.h"
 
 
@@ -352,6 +353,16 @@ find_implicit_erroneous_behavior (void)
   if (has_abnormal_or_eh_outgoing_edge_p (bb))
continue;
 
+
+  /* If BB has an edge to itself, then duplication of BB below
+could result in reallocation of BB's PHI nodes.   If that happens
+then the loop below over the PHIs would use the old PHI and
+thus invalid information.  We don't have a good way to know
+if a PHI has been reallocated, so just avoid isolation in
+this case.  */
+  if (find_edge (bb, bb))
+   continue;
+
   /* First look for a PHI which sets a pointer to NULL and which
 is then dereferenced within BB.  This is somewhat overly
 conservative, but probably catches most of the interesting
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index f6f2e37..b164483 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2017-02-21  Jeff Law  
+
+   PR tree-optimization/79621
+   * gcc.c-torture/compile/pr79621.c: New test.
+
 2017-02-21  Jakub Jelinek  
 
PR target/79570
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr79621.c 
b/gcc/testsuite/gcc.c-torture/compile/pr79621.c
new file mode 100644
index 000..f115c07
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr79621.c
@@ -0,0 +1,18 @@
+int b5;
+
+void
+h6 (int zb, int e7)
+{
+  while (b5 > 0)
+{
+  int gv;
+
+  for (gv = 1; gv < 4; ++gv)
+{
+  ((zb != 0) ? b5 : gv) && (b5 /= e7);
+  zb = 0;
+}
+  e7 = 0;
+}
+}
+


Re: [PATCH] restore -Wunused-variable on a typedef'd variable in a function template (PR 79548)

2017-02-21 Thread Jason Merrill

On 02/17/2017 05:07 PM, Martin Sebor wrote:

* decl.c (poplevel): Avoid diagnosing entities declared with
attribute unused.


This change is OK.


(initialize_local_var): Do not consider the type of a variable
when determining whether or not it's used.


This is not; the documentation for attribute unused says,

When attached to a type (including a @code{union} or a @code{struct}),
this attribute means that variables of that type are meant to appear
possibly unused.  GCC does not produce a warning for any variables of
that type, even if the variable appears to do nothing.  This is often
the case with lock or thread classes, which are usually defined and then
not referenced, but contain constructors and destructors that have
nontrivial bookkeeping functions.

So a TREE_USED type should imply TREE_USED on variables of that type.

Jason



Re: [C++ PATCH] Fix default TLS model for non-inline static data members (PR c++/79288)

2017-02-21 Thread Jason Merrill
OK.

On Mon, Jan 30, 2017 at 12:49 PM, Jakub Jelinek  wrote:
> Hi!
>
> The inline variable changes broke the default TLS model of non-inline static
> data members.  The decl_default_tls_model call needs DECL_EXTERNAL for the
> !processing_template_decl be already set appropriately.  The following patch
> moves the thread_p processing a few lines below, so that it is already set
> there.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, additionally
> on both tested with make check-c++-all, ok for trunk?
>
> 2017-01-30  Jakub Jelinek  
>
> PR c++/79288
> * decl.c (grokdeclarator): For static data members, handle thread_p
> only after handling inline.
>
> * g++.dg/tls/pr79288.C: New test.
>
> --- gcc/cp/decl.c.jj2017-01-26 09:14:24.0 +0100
> +++ gcc/cp/decl.c   2017-01-30 18:49:38.544438710 +0100
> @@ -12049,14 +12049,6 @@ grokdeclarator (const cp_declarator *dec
> : input_location,
> VAR_DECL, unqualified_id, type);
> set_linkage_for_static_data_member (decl);
> -   if (thread_p)
> - {
> -   CP_DECL_THREAD_LOCAL_P (decl) = true;
> -   if (!processing_template_decl)
> - set_decl_tls_model (decl, decl_default_tls_model 
> (decl));
> -   if (declspecs->gnu_thread_keyword_p)
> - SET_DECL_GNU_TLS_P (decl);
> - }
> if (concept_p)
> error ("static data member %qE declared %",
>unqualified_id);
> @@ -12077,6 +12069,15 @@ grokdeclarator (const cp_declarator *dec
>  definition is provided, unless this is an inline
>  variable.  */
>   DECL_EXTERNAL (decl) = 1;
> +
> +   if (thread_p)
> + {
> +   CP_DECL_THREAD_LOCAL_P (decl) = true;
> +   if (!processing_template_decl)
> + set_decl_tls_model (decl, decl_default_tls_model 
> (decl));
> +   if (declspecs->gnu_thread_keyword_p)
> + SET_DECL_GNU_TLS_P (decl);
> + }
>   }
> else
>   {
> --- gcc/testsuite/g++.dg/tls/pr79288.C.jj   2017-01-30 18:55:05.754282818 
> +0100
> +++ gcc/testsuite/g++.dg/tls/pr79288.C  2017-01-30 18:54:52.0 +0100
> @@ -0,0 +1,28 @@
> +// PR c++/79288
> +// { dg-do compile { target nonpic } }
> +// { dg-require-effective-target tls }
> +// { dg-options "-O2" }
> +// { dg-final { scan-assembler-not "@tpoff" { target i?86-*-* x86_64-*-* } } 
> }
> +
> +struct S
> +{
> +  static __thread int *p;
> +};
> +
> +template 
> +struct T
> +{
> +  static __thread int *p;
> +};
> +
> +int *
> +foo ()
> +{
> +  return S::p;
> +}
> +
> +int *
> +bar ()
> +{
> +  return T<0>::p;
> +}
>
> Jakub


Re: [PATCH 1/5] Handle WORD_REGISTER_OPERATIONS when reloading (subreg(reg))

2017-02-21 Thread Richard Sandiford
Matthew Fortune  writes:
> Richard Sandiford  writes:
>> Matthew Fortune  writes:
>> > Eric Botcazou  writes:
>> >> > Thanks for reporting. I'll take a brief look first but revert if
>> >> > the issue isn't something that vaguely makes sense to me.
>> >>
>> >> You need to restrict any WORD_REGISTER_OPERATIONS test to subword
>> >> registers.
>> >
>> > I've reverted this. I haven't been able to quickly find a change that
>> > I both feel is right and works. I am wondering if I actually don't
>> > need this change and that 'patch 3' (the change to
>> > simplify_operand_subreg) is the actual fix for both issues I have
>> seen.
>> 
>> I think that patch might have a similar problem though, in that it
>> applies WORD_REGISTER_OPERATIONS semantics to things that might be
>> vectors.
>
> There is an amendment done as part of SPARC testing that limits the
> change to modes that are no wider than a word. But, given that assumptions
> coming from WORD_REGISTER_OPERATIONS can only be applied to integer
> modes then it should also check both modes are MODE_INT as well.

Oops, sorry, I should have checked.

> All that said... I don't entirely follow why any target should be
> reliant on subreg(mem) being simplified to use the outer mode. It is an
> optimization certainly for some cases but I don't understand what rule
> or guarantee we have that means reloading the inner MEM is illegal.

Agreed.  I don't think things like WORD_MODE_OPERATIONS should change
rtl semantics, just optimisation decisions.  And using the smallest
possible spill size is often good even for RISCy targets.

> The latter point is not appropriate to debate for GCC 7 though.

Yeah.

Thanks,
Richard


Re: [PATCH 1/5] Handle WORD_REGISTER_OPERATIONS when reloading (subreg(reg))

2017-02-21 Thread Eric Botcazou
> Agreed.  I don't think things like WORD_MODE_OPERATIONS should change
> rtl semantics, just optimisation decisions.  And using the smallest
> possible spill size is often good even for RISCy targets.

I don't think that's correct, WORD_MODE_OPERATIONS does change RTL semantics 
for SUBREGs smaller than a word since it can make all bits defined, even if 
only the lower part is assigned.  For example (SUBREG:SI (MEM:QI)) has the 
higher bits defined according to LOAD_EXTEND_OP if WORD_MODE_OPERATIONS.

LRA simply needs to preserve the semantics, just as reload does.

-- 
Eric Botcazou


Re: [driver, doc] Support escaping special characters in specs

2017-02-21 Thread Sandra Loosemore

On 02/21/2017 06:54 AM, Rainer Orth wrote:

Hi Joseph,


On Fri, 13 Jan 2017, Rainer Orth wrote:


I'm unsure if the patch is large enough to need a copyright assignment
(in which case it's almost certainly too late for GCC 7), and even if
not if it's appropriate at this point in the release cycle.


I think it's big enough to need an assignment.


Jeff informed me that he'd received confirmation from the FSF that his
assignment has been processed.


Note missing spaces before '(' in calls to free, and after ')' in a cast.


Fixed in the following revised patch, which also incorporates Sandra's
review comments.

Bootstrapped without regressions on i386-pc-solaris2.12 and
x86_64-pc-linux-gnu, in a tree which also contained the patch for PR
target/40411 which needs this feature.

The doc parts have been checked separately with

$ make doc/gcc.info doc/gcc.pdf

and visual inspection of the output.

Ok for mainline either now or when GCC 8 stage 1 opens?


The doc parts of the patch look OK for whenever the code change can go in.

-Sandra



[PATCH, i386]: Fix PR79593: Poor/Worse code generation for FPU on versions after 6

2017-02-21 Thread Uros Bizjak
Hello!

Attached patch fixes oversight in standard_x87sse_constant_load
splitter and its float-extend counterpart, where a FP reg-reg move
insn RTX can be tagged with REG_EQUIV or REG_EQUAL const_double RTX.

find_constant_src and ix86_standard_x87sse_constant_load_p predicate
are able to handle this situation, and patched splitters emit direct
constant load instead of a reg-reg move. This also lowers regstack
register pressure, as evident from the testcase:

--- pr79593.s_  2017-02-21 19:41:36.615740647 +0100
+++ pr79593.s   2017-02-21 19:41:47.251622966 +0100
@@ -15,21 +15,16 @@
fldz
 .L2:
fld1
-   fld %st(0)
-   fcomp   %st(2)
+   fcomp   %st(1)
fnstsw  %ax
sahf
-   jnb .L5
-   fstp%st(1)
-   jmp .L3
-   .p2align 4,,10
-   .p2align 3
-.L5:
+   jnb .L3
fstp%st(0)
+   fld1
 .L3:
rep ret
.cfi_endproc
 .LFE2:
.size   bar, .-bar
-   .ident  "GCC: (GNU) 7.0.0 20170117 (experimental) [trunk
revision 244540]"
+   .ident  "GCC: (GNU) 7.0.1 20170221 (experimental) [trunk
revision 245630]"
.section.note.GNU-stack,"",@progbits

Patched compiler also removed a jump to a BB where only compensating
regstack pop was emitted.

2017-02-21  Uros Bizjak  

PR target/79593
* config/i386/i386.md (standard_x87sse_constant_load splitter):
Use nonimmediate_operand instead of memory_operand for operand 1.
(float-extend standard_x87sse_constant_load splitter): Ditto.

testsuite/ChangeLog:

2017-02-21  Uros Bizjak  

PR target/79593
* gcc.target/i386/pr79593.c: New test.

Patch was bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

Committed to mainline SVN.

Uros.


Re: [PATCH] restore -Wunused-variable on a typedef'd variable in a function template (PR 79548)

2017-02-21 Thread Martin Sebor

On 02/21/2017 11:08 AM, Jason Merrill wrote:

On 02/17/2017 05:07 PM, Martin Sebor wrote:

* decl.c (poplevel): Avoid diagnosing entities declared with
attribute unused.


This change is OK.


(initialize_local_var): Do not consider the type of a variable
when determining whether or not it's used.


This is not; the documentation for attribute unused says,

When attached to a type (including a @code{union} or a @code{struct}),
this attribute means that variables of that type are meant to appear
possibly unused.  GCC does not produce a warning for any variables of
that type, even if the variable appears to do nothing.  This is often
the case with lock or thread classes, which are usually defined and then
not referenced, but contain constructors and destructors that have
nontrivial bookkeeping functions.

So a TREE_USED type should imply TREE_USED on variables of that type.


Yes, I'm familiar with the effect of the attribute on types but
in my testing this change doesn't affect how it works (i.e., it
passes a full bootstrap and regression tests and I haven't been
able to construct a failing test case.)  It looks like that's
because TREE_USED(decl) is already set here for a decl whose
type is declared attribute used.

While trying to come up with a test case to exercise the scenario
you describe I see that current trunk doesn't handle it correctly
but the patch just happens to fix that too.  For example:

template 
void f ()
{
  T t;   // trunk warns for f (good)

  typedef T U;
  U u;   // trunk doesn't warn for f (bug 79548)
}

template void f();

struct __attribute__ ((unused)) S { };

template void f();   // no warnings here (good)

void g ()
{
  S s;

  typedef S T;
  T t;   // trunk warns here (new bug), doesn't with the patch
}

In the test case above, TREE_USED (TREE_TYPE (decl)) is set for
t in g() so trunk sets it on t too and warbs.  The patch doesn't
and so it doesn't warn as expected.

But it's entirely possible I'm missing a case.  Do you have
a suggestion for a test that trunk handles correctly but that
fails with the patch?

Thanks
Martin



Re: [PATCH 5/6] pa: Fixes for RTL checking

2017-02-21 Thread Jeff Law

On 02/21/2017 07:48 AM, Segher Boessenkool wrote:

I do not know what the USEs are really for, so I do not know if using
just the pattern here is correct.  Using the full insn is probably not
correct either though.


2017-02-21  Segher Boessenkool  

* config/pa/pa.c (pa_combine_instructions): Do not share RTL.  Make
the special USEs with the pattern of the insn, not the insn itself.
OK.  Or alternately, remove all that code :-)  The optimization it was 
doing was only useful on pa7xx/pa7xxx models which are beyond ancient.


Jeff



Re: [PATCH] arc: Fixes for RTL checking

2017-02-21 Thread Jeff Law

On 02/21/2017 08:08 AM, Segher Boessenkool wrote:

Whoops, I forgot one.  Here it is.  Joern, please see
.


2017-02-21  Segher Boessenkool  

* config/arc/arc.c (arc_ccfsm_advance): Only take the PATTERN of
this_insn if it is an INSN or JUMP_INSN.
(force_offsettable): Look at base, not at addr.
* config/arc/predicates.md (brcc_nolimm_operator): Don't call INTVAL
on things that aren' necessarily CONST_INTs.

OK.
jeff



Re: [PATCH 3/6] microblaze: Fixes for RTL checking

2017-02-21 Thread Jeff Law

On 02/21/2017 07:48 AM, Segher Boessenkool wrote:

REGNO can only be called on REGs, not SUBREGs; and INTVAL does not work
on REGs.

2017-02-21  Segher Boessenkool  

* config/microblaze/microblaze.c (microblaze_expand_shift): Do not
test for register moves to themselves.
* config/microblaze/microblaze.md (*ashlsi3_byone, *ashrsi3_byone,
*lshrsi3_byone): Test for const1_rtx instead of calling INTVAL on
something that isn't necessarily a CONST_INT.
So I wanted to make sure that the avoidance of a nop-move resulting from 
a nop-shift wasn't because the ISA couldn't encode a nop-move (and yes, 
I've run into such ISAs).


The microblaze doesn't seem to have any trouble encoding a nop move, so 
if we were to generate one due to this change it won't cause a problem.



OK for the trunk.

Jeff



Re: [PATCH 4/6] nios2: Fixes for RTL checking

2017-02-21 Thread Jeff Law

On 02/21/2017 07:48 AM, Segher Boessenkool wrote:

You cannot call REGNO on something that isn't a REG, and you cannot
call INTVAL on something that isn't a CONST_INT.

The way I fixed nios2_alternate_compare_const is admittedly a bit lame.


2017-02-21  Segher Boessenkool  

* config/nios2/nios2.c (nios2_simple_const_p): Returns false if the
argument isn't a CONST_INT.
(nios2_alternate_compare_const): Set *alt_code and *alt_op to code
and op if op is not a CONST_INT.
(nios2_valid_compare_const_p): Return false if the argument isn't
a CONST_INT.
(ldstwm_operation_p): Return false if first_base is not a REG or
if first_offset is not a CONST_INT.

OK.
jeff



Re: [PATCH] Look through clobber stmts in uninit (PR tree-optimization/79345)

2017-02-21 Thread Jeff Law

On 02/21/2017 02:01 AM, Richard Biener wrote:

On Mon, 20 Feb 2017, Marc Glisse wrote:


On Mon, 20 Feb 2017, Jakub Jelinek wrote:


As mentioned by Jason in the PR, we've regressed on the following testcase
since we started emitting CLOBBERs at the start of ctors (and we warn as
before with -fno-lifetime-dse -Wuninitialized).
With -fno-lifetime-dse, the vuse on the b.x read stmt is default def
and thus we warn, but if there are clobbers before that, that is not the
case and we don't warn.  The patch is quick hack to bypass the initial
clobbers as long as there aren't really many.  If we wanted to handle
all initial clobbers, I bet the first time we run into this we could
recursively walk vop uses from the default def and build say a bitmap
of vop SSA_NAMEs which are vdefs of clobbers that only have clobbers
before it as vdef stmts.


  MEM[(struct  &)&b] ={v} {CLOBBER};
  _4 = b.x;
  if (_4 != 0)

It would be nice (at some point in a distant future) to turn that into

  MEM[(struct  &)&b] ={v} {CLOBBER};
  if (_4(D) != 0)

i.e. replace reads from clobbered memory with a default def (with a gimple_nop
defining statement).

IIRC I tried to do it in FRE once, but failed.


Yeah, FRE isn't too happy about VN_TOP in the lattice (which is what
you'd use here).  I also have/had patches trying to make it (more) happy
about VN_TOP but it still broke stuff like for example tail-merging...

Need to dissect PRE from tail-merging again, and also possibly PRE
from value-numbering (run eliminate() before doing PRE).
Couldn't this be easily modeled as redundant load elimination?  Pretend 
the clobber is a read, which would make the read of b.x redundant.


I haven't prototyped it, but that'd be the first approach I'd try.

jeff



Re: [PATCH 4/6] nios2: Fixes for RTL checking

2017-02-21 Thread Sandra Loosemore

On 02/21/2017 07:48 AM, Segher Boessenkool wrote:

You cannot call REGNO on something that isn't a REG, and you cannot
call INTVAL on something that isn't a CONST_INT.

The way I fixed nios2_alternate_compare_const is admittedly a bit lame.


Yeah.  :-P


2017-02-21  Segher Boessenkool  

* config/nios2/nios2.c (nios2_simple_const_p): Returns false if the
argument isn't a CONST_INT.
(nios2_alternate_compare_const): Set *alt_code and *alt_op to code
and op if op is not a CONST_INT.
(nios2_valid_compare_const_p): Return false if the argument isn't
a CONST_INT.
(ldstwm_operation_p): Return false if first_base is not a REG or
if first_offset is not a CONST_INT.


Give me a couple days to fiddle with this a bit and run regression tests.

-Sandra



RE: [PATCH 1/5] Handle WORD_REGISTER_OPERATIONS when reloading (subreg(reg))

2017-02-21 Thread Matthew Fortune
Eric Botcazou  writes:
> > Agreed.  I don't think things like WORD_MODE_OPERATIONS should change
> > rtl semantics, just optimisation decisions.

Sorry, I did cover two topics in one email.  My point was about whether
simplifying:

(subreg:OUTER (mem:INNER ...))
To:
(mem:OUTER ...)

Should ever be a requirement for successful reloading rather than just an
optimisation that 'could' be applied. There are a few cases it seems that
require this simplification to happen which I find odd.

> > And using the smallest
> > possible spill size is often good even for RISCy targets.

Yes, if (subreg(mem)) simplification only ever happened when it was reducing
the size of the load/store I would understand the code more too but actually
it will currently happily simplify to a wider mode. Using a wider mode may
well be beneficial in other situations where a further reload would be needed
due to insn constraints I guess. It all feels a bit like magic still.

> I don't think that's correct, WORD_MODE_OPERATIONS does change RTL semantics
> for SUBREGs smaller than a word since it can make all bits defined, even if
> only the lower part is assigned.  For example (SUBREG:SI (MEM:QI)) has the
> higher bits defined according to LOAD_EXTEND_OP if WORD_MODE_OPERATIONS.

It would almost be simpler if we had another variant of subreg (like we have
strict_low_part) to explicitly represent the WORD_REGISTER_OPERATIONS behaviour
with the mode change. I'll stop myself and hold this for later though!

> LRA simply needs to preserve the semantics, just as reload does.

I will keep working on this code post GCC 7 as the topic is bugging me now :-)

Thanks,
Matthew


Re: [PATCH 3/6] microblaze: Fixes for RTL checking

2017-02-21 Thread Michael Eager

On 02/21/2017 06:48 AM, Segher Boessenkool wrote:

REGNO can only be called on REGs, not SUBREGs; and INTVAL does not work
on REGs.

2017-02-21  Segher Boessenkool  

* config/microblaze/microblaze.c (microblaze_expand_shift): Do not
test for register moves to themselves.
* config/microblaze/microblaze.md (*ashlsi3_byone, *ashrsi3_byone,
*lshrsi3_byone): Test for const1_rtx instead of calling INTVAL on
something that isn't necessarily a CONST_INT.

---
  gcc/config/microblaze/microblaze.c  | 5 ++---
  gcc/config/microblaze/microblaze.md | 6 +++---
  2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/gcc/config/microblaze/microblaze.c 
b/gcc/config/microblaze/microblaze.c
index 746bef1..4850e85 100644
--- a/gcc/config/microblaze/microblaze.c
+++ b/gcc/config/microblaze/microblaze.c
@@ -3322,11 +3322,10 @@ microblaze_expand_shift (rtx operands[])
  || (GET_CODE (operands[1]) == REG)
  || (GET_CODE (operands[1]) == SUBREG));

-  /* Shift by zero -- copy regs if necessary.  */
+  /* Shift by zero -- copy regs.  */
if ((GET_CODE (operands[2]) == CONST_INT) && (INTVAL (operands[2]) == 0))
  {
-  if (REGNO (operands[0]) != REGNO (operands[1]))
-   emit_insn (gen_movsi (operands[0], operands[1]));
+  emit_insn (gen_movsi (operands[0], operands[1]));
return 1;
  }



Why generate an unnecessary NOP?


--
Michael Eagerea...@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077


C++ PATCH to fix ICE with NSDMI and flexible array member (PR c++/79535)

2017-02-21 Thread Marek Polacek
Jason suggested that the way forward with this PR, where we ICE because we have
a default mem-initializer for a flexible array, and verify_ctor_sanity crashes
on that, is to reject such code.  This patch attempts to do that; I'm not
entirely happy about the placement of the maybe_reject_flexarray_init call,
but it probably couldn't be in digest_nsdmi_init, becase that could result
in multiple error messages for the same thing.

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

2017-02-21  Marek Polacek  

PR c++/79535
* cp-tree.h (maybe_reject_flexarray_init): Declare.
* init.c (maybe_reject_flexarray_init): No longer static.
Add check for current_function_decl.
* parser.c (cp_parser_late_parse_one_default_arg): Reject
a default mem-initializer for a flexible array.

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

diff --git gcc/cp/cp-tree.h gcc/cp/cp-tree.h
index 6675ee5..f53f744 100644
--- gcc/cp/cp-tree.h
+++ gcc/cp/cp-tree.h
@@ -6069,6 +6069,7 @@ extern tree scalar_constant_value (tree);
 extern tree decl_really_constant_value (tree);
 extern int diagnose_uninitialized_cst_or_ref_member (tree, bool, bool);
 extern tree build_vtbl_address  (tree);
+extern bool maybe_reject_flexarray_init(tree, tree);
 
 /* in lex.c */
 extern void cxx_dup_lang_specific_decl (tree);
diff --git gcc/cp/init.c gcc/cp/init.c
index fa74226..13ade8a 100644
--- gcc/cp/init.c
+++ gcc/cp/init.c
@@ -600,7 +600,7 @@ get_nsdmi (tree member, bool in_ctor)
 /* Diagnose the flexible array MEMBER if its INITializer is non-null
and return true if so.  Otherwise return false.  */
 
-static bool
+bool
 maybe_reject_flexarray_init (tree member, tree init)
 {
   tree type = TREE_TYPE (member);
@@ -615,6 +615,7 @@ maybe_reject_flexarray_init (tree member, tree init)
  initializer list.  */
   location_t loc;
   if (DECL_INITIAL (member) == init
+  || !current_function_decl
   || DECL_DEFAULTED_FN (current_function_decl))
 loc = DECL_SOURCE_LOCATION (member);
   else
diff --git gcc/cp/parser.c gcc/cp/parser.c
index 0146596..15e09f7 100644
--- gcc/cp/parser.c
+++ gcc/cp/parser.c
@@ -27228,6 +27228,8 @@ cp_parser_late_parse_one_default_arg (cp_parser 
*parser, tree decl,
   if (TREE_CODE (decl) == PARM_DECL)
parsed_arg = check_default_argument (parmtype, parsed_arg,
 tf_warning_or_error);
+  else if (maybe_reject_flexarray_init (decl, parsed_arg))
+   parsed_arg = error_mark_node;
   else
parsed_arg = digest_nsdmi_init (decl, parsed_arg);
 }
diff --git gcc/testsuite/g++.dg/ext/flexary23.C 
gcc/testsuite/g++.dg/ext/flexary23.C
index e69de29..099e7fd 100644
--- gcc/testsuite/g++.dg/ext/flexary23.C
+++ gcc/testsuite/g++.dg/ext/flexary23.C
@@ -0,0 +1,11 @@
+// PR c++/79535 - ICE with NSDMI and array
+// { dg-do compile { target c++14 } }
+// { dg-options -Wno-pedantic }
+
+struct A
+{
+  int b = 1;
+  int c = 2;
+  int x[] = { c, 3 }; // { dg-error "initializer for flexible array member" }
+};
+A a = { 4, 5 };

Marek


Re: [PATCH 3/6] microblaze: Fixes for RTL checking

2017-02-21 Thread Jakub Jelinek
On Tue, Feb 21, 2017 at 02:48:15PM +, Segher Boessenkool wrote:
> -  /* Shift by zero -- copy regs if necessary.  */
> +  /* Shift by zero -- copy regs.  */
>if ((GET_CODE (operands[2]) == CONST_INT) && (INTVAL (operands[2]) == 0))

You could have changed this line to
  if (operands[2] == const0_rtx)
as well.

Jakub


Re: C++ PATCH to fix ICE with NSDMI and flexible array member (PR c++/79535)

2017-02-21 Thread Jason Merrill
OK.

On Tue, Feb 21, 2017 at 12:11 PM, Marek Polacek  wrote:
> Jason suggested that the way forward with this PR, where we ICE because we 
> have
> a default mem-initializer for a flexible array, and verify_ctor_sanity crashes
> on that, is to reject such code.  This patch attempts to do that; I'm not
> entirely happy about the placement of the maybe_reject_flexarray_init call,
> but it probably couldn't be in digest_nsdmi_init, becase that could result
> in multiple error messages for the same thing.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2017-02-21  Marek Polacek  
>
> PR c++/79535
> * cp-tree.h (maybe_reject_flexarray_init): Declare.
> * init.c (maybe_reject_flexarray_init): No longer static.
> Add check for current_function_decl.
> * parser.c (cp_parser_late_parse_one_default_arg): Reject
> a default mem-initializer for a flexible array.
>
> * g++.dg/ext/flexary23.C: New test.
>
> diff --git gcc/cp/cp-tree.h gcc/cp/cp-tree.h
> index 6675ee5..f53f744 100644
> --- gcc/cp/cp-tree.h
> +++ gcc/cp/cp-tree.h
> @@ -6069,6 +6069,7 @@ extern tree scalar_constant_value (tree);
>  extern tree decl_really_constant_value (tree);
>  extern int diagnose_uninitialized_cst_or_ref_member (tree, bool, bool);
>  extern tree build_vtbl_address  (tree);
> +extern bool maybe_reject_flexarray_init(tree, tree);
>
>  /* in lex.c */
>  extern void cxx_dup_lang_specific_decl (tree);
> diff --git gcc/cp/init.c gcc/cp/init.c
> index fa74226..13ade8a 100644
> --- gcc/cp/init.c
> +++ gcc/cp/init.c
> @@ -600,7 +600,7 @@ get_nsdmi (tree member, bool in_ctor)
>  /* Diagnose the flexible array MEMBER if its INITializer is non-null
> and return true if so.  Otherwise return false.  */
>
> -static bool
> +bool
>  maybe_reject_flexarray_init (tree member, tree init)
>  {
>tree type = TREE_TYPE (member);
> @@ -615,6 +615,7 @@ maybe_reject_flexarray_init (tree member, tree init)
>   initializer list.  */
>location_t loc;
>if (DECL_INITIAL (member) == init
> +  || !current_function_decl
>|| DECL_DEFAULTED_FN (current_function_decl))
>  loc = DECL_SOURCE_LOCATION (member);
>else
> diff --git gcc/cp/parser.c gcc/cp/parser.c
> index 0146596..15e09f7 100644
> --- gcc/cp/parser.c
> +++ gcc/cp/parser.c
> @@ -27228,6 +27228,8 @@ cp_parser_late_parse_one_default_arg (cp_parser 
> *parser, tree decl,
>if (TREE_CODE (decl) == PARM_DECL)
> parsed_arg = check_default_argument (parmtype, parsed_arg,
>  tf_warning_or_error);
> +  else if (maybe_reject_flexarray_init (decl, parsed_arg))
> +   parsed_arg = error_mark_node;
>else
> parsed_arg = digest_nsdmi_init (decl, parsed_arg);
>  }
> diff --git gcc/testsuite/g++.dg/ext/flexary23.C 
> gcc/testsuite/g++.dg/ext/flexary23.C
> index e69de29..099e7fd 100644
> --- gcc/testsuite/g++.dg/ext/flexary23.C
> +++ gcc/testsuite/g++.dg/ext/flexary23.C
> @@ -0,0 +1,11 @@
> +// PR c++/79535 - ICE with NSDMI and array
> +// { dg-do compile { target c++14 } }
> +// { dg-options -Wno-pedantic }
> +
> +struct A
> +{
> +  int b = 1;
> +  int c = 2;
> +  int x[] = { c, 3 }; // { dg-error "initializer for flexible array member" }
> +};
> +A a = { 4, 5 };
>
> Marek


Re: [PATCH 3/6] microblaze: Fixes for RTL checking

2017-02-21 Thread Segher Boessenkool
On Tue, Feb 21, 2017 at 12:08:34PM -0800, Michael Eager wrote:
> >-  /* Shift by zero -- copy regs if necessary.  */
> >+  /* Shift by zero -- copy regs.  */
> >if ((GET_CODE (operands[2]) == CONST_INT) && (INTVAL (operands[2]) == 
> >0))
> >  {
> >-  if (REGNO (operands[0]) != REGNO (operands[1]))
> >-emit_insn (gen_movsi (operands[0], operands[1]));
> >+  emit_insn (gen_movsi (operands[0], operands[1]));
> >return 1;
> >  }
> 
> Why generate an unnecessary NOP?

Why not?  It will be optimised away anyway, and the code to get at the
subregs is hairy...  But could optimise away the useless move here
already if both ops are a reg and both are the same, if you prefer that?
Or rtx_equal_p (operands[0], operands[1])?


Segher


Re: [PATCH] restore -Wunused-variable on a typedef'd variable in a function template (PR 79548)

2017-02-21 Thread Jason Merrill
On Tue, Feb 21, 2017 at 11:00 AM, Martin Sebor  wrote:
> On 02/21/2017 11:08 AM, Jason Merrill wrote:
>>
>> On 02/17/2017 05:07 PM, Martin Sebor wrote:
>>>
>>> * decl.c (poplevel): Avoid diagnosing entities declared with
>>> attribute unused.
>>
>>
>> This change is OK.
>>
>>> (initialize_local_var): Do not consider the type of a variable
>>> when determining whether or not it's used.
>>
>>
>> This is not; the documentation for attribute unused says,
>>
>> When attached to a type (including a @code{union} or a @code{struct}),
>> this attribute means that variables of that type are meant to appear
>> possibly unused.  GCC does not produce a warning for any variables of
>> that type, even if the variable appears to do nothing.  This is often
>> the case with lock or thread classes, which are usually defined and then
>> not referenced, but contain constructors and destructors that have
>> nontrivial bookkeeping functions.
>>
>> So a TREE_USED type should imply TREE_USED on variables of that type.
>
> Yes, I'm familiar with the effect of the attribute on types but
> in my testing this change doesn't affect how it works (i.e., it
> passes a full bootstrap and regression tests and I haven't been
> able to construct a failing test case.)  It looks like that's
> because TREE_USED(decl) is already set here for a decl whose
> type is declared attribute used.
>
> While trying to come up with a test case to exercise the scenario
> you describe I see that current trunk doesn't handle it correctly
> but the patch just happens to fix that too.  For example:
>
> template 
> void f ()
> {
>   T t;   // trunk warns for f (good)
>
>   typedef T U;
>   U u;   // trunk doesn't warn for f (bug 79548)
> }
>
> template void f();
>
> struct __attribute__ ((unused)) S { };
>
> template void f();   // no warnings here (good)
>
> void g ()
> {
>   S s;
>
>   typedef S T;
>   T t;   // trunk warns here (new bug), doesn't with the patch
> }
>
> In the test case above, TREE_USED (TREE_TYPE (decl)) is set for
> t in g() so trunk sets it on t too and warbs.  The patch doesn't
> and so it doesn't warn as expected.
>
> But it's entirely possible I'm missing a case.  Do you have
> a suggestion for a test that trunk handles correctly but that
> fails with the patch?

Ah, I see, your patch changes attribute unused handling for local
variables from tracking TREE_USED to lookup_attribute.  I'm not
opposed to this change, but I'd like to understand why the TREE_USED
handling wasn't working.

Jason


Re: [PATCH 3/6] microblaze: Fixes for RTL checking

2017-02-21 Thread Michael Eager

On 02/21/2017 12:15 PM, Jakub Jelinek wrote:

On Tue, Feb 21, 2017 at 02:48:15PM +, Segher Boessenkool wrote:

-  /* Shift by zero -- copy regs if necessary.  */
+  /* Shift by zero -- copy regs.  */
if ((GET_CODE (operands[2]) == CONST_INT) && (INTVAL (operands[2]) == 0))


You could have changed this line to
   if (operands[2] == const0_rtx)
as well.


And this would not change the generated code.

--
Michael Eagerea...@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077


Re: [C++ PATCH] For -gdwarf-5 emit DW_TAG_variable instead of DW_TAG_member for C++ static data members

2017-02-21 Thread Jason Merrill
On Sat, Feb 18, 2017 at 12:17 AM, Jakub Jelinek  wrote:
> On Fri, Feb 17, 2017 at 09:37:09PM -0500, Jason Merrill wrote:
>> On Fri, Feb 17, 2017 at 1:52 PM, Jakub Jelinek  wrote:
>> > -  && die->die_tag != DW_TAG_member)
>> > +  && die->die_tag != DW_TAG_member
>> > +  && (die->die_tag != DW_TAG_variable || !class_scope_p 
>> > (die->die_parent)))
>>
>> How about we only check class_scope_p (die->die_parent), and don't
>> consider the TAG at all?  DW_TAG_member should only appear at class
>> scope.
>
> That wouldn't work, because that would avoid adding linkage attributes to
> methods.  I could use:
>   && (die->die_tag == DW_TAG_subprogram || !class_scope_p 
> (die->die_parent))
> (not 100% sure if some other tag than those can make it here)
> or
>   && ((die->die_tag != DW_TAG_member || die->die_tag != DW_TAG_variable)
>   || !class_scope_p (die->die_parent))
> or just use
>   && (!VAR_P (decl) || !class_scope_p (die->die_parent))
> or similar.
>
>> > - if (old_die->die_tag == DW_TAG_member)
>> > + if (old_die->die_tag == DW_TAG_member
>> > + || (dwarf_version >= 5 && class_scope_p 
>> > (old_die->die_parent)))
>>
>> Likewise here.
>
> This spot probably can be changed as you wrote, it is in gen_variable_die,
> so methods shouldn't appear there.

Hmm, let's think about the behavior we want here.  I don't see any
reason not to put AT_linkage_name on a member DW_TAG_variable; I think
the old behavior avoided putting it on DW_TAG_member just because it
isn't defined for DW_TAG_member.  So it's not clear to me that we need
any changes in add_linkage_name or its call site.

Jason


Re: [PATCH 3/6] microblaze: Fixes for RTL checking

2017-02-21 Thread Jakub Jelinek
On Tue, Feb 21, 2017 at 12:35:17PM -0800, Michael Eager wrote:
> On 02/21/2017 12:15 PM, Jakub Jelinek wrote:
> > On Tue, Feb 21, 2017 at 02:48:15PM +, Segher Boessenkool wrote:
> > > -  /* Shift by zero -- copy regs if necessary.  */
> > > +  /* Shift by zero -- copy regs.  */
> > > if ((GET_CODE (operands[2]) == CONST_INT) && (INTVAL (operands[2]) == 
> > > 0))
> > 
> > You could have changed this line to
> >if (operands[2] == const0_rtx)
> > as well.
> 
> And this would not change the generated code.

Sure, and it doesn't need to be done for GCC7, but would be a nice cleanup
for stage1 (e.g. remove redundant parens in the backend, cases like this
where because of the uniqueness of CONST_INT values a pointer comparison
is enough, or using macros where available (e.g. GET_CODE (operands[2]) == 
CONST_INT
can be written as CONST_INT_P (operands[2]), etc.).

Jakub


Re: [PATCH] Move -Wrestrict warning later in the FEs and fix some issues in it (PR c++/79588)

2017-02-21 Thread Jason Merrill
OK if Joseph doesn't object in the next couple of days.

On Mon, Feb 20, 2017 at 12:35 PM, Jakub Jelinek  wrote:
> Hi!
>
> As mentioned in the PR, -Wrestrict warning is done way too early, where
> e.g. default arguments aren't filled up yet (reason for ICE on first
> testcase) or where arguments in templates aren't instantiated yet (reason
> why we don't diagnose anything on the second testcase).
>
> This patch moves it later where e.g. -Wformat is diagnosed and fixes
> some issues I found while looking at the code.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2017-02-20  Jakub Jelinek  
>
> PR c++/79588
> c-family/
> * c-common.c (check_function_arguments): Add FNDECL argument.
> Handle -Wrestrict here.
> * c-warn.c (warn_for_restrict): Remove ARGS argument, add ARGARRAY
> and NARGS.  Use auto_vec for ARG_POSITIONS, simplify.
> * c-common.h (check_function_arguments): Add FNDECL argument.
> (warn_for_restrict): Remove ARGS argument, add ARGARRAY and NARGS.
> c/
> * c-parser.c (c_parser_postfix_expression_after_primary): Don't
> handle -Wrestrict here.
> * c-typeck.c (build_function_call_vec): Adjust
> check_function_arguments caller.
> cp/
> * call.c (build_over_call): Call check_function_arguments even for
> -Wrestrict, adjust check_function_arguments caller.
> * parser.c (cp_parser_postfix_expression): Don't handle -Wrestrict
> here.
> * typeck.c (cp_build_function_call_vec): Adjust
> check_function_arguments caller.
> testsuite/
> * g++.dg/warn/Wrestrict-1.C: New test.
> * g++.dg/warn/Wrestrict-2.C: New test.
>
> --- gcc/c-family/c-common.c.jj  2017-01-24 23:29:05.0 +0100
> +++ gcc/c-family/c-common.c 2017-02-20 13:17:06.601211847 +0100
> @@ -5605,8 +5605,8 @@ attribute_fallthrough_p (tree attr)
> There are NARGS arguments in the array ARGARRAY.  LOC should be used for
> diagnostics.  Return true if -Wnonnull warning has been diagnosed.  */
>  bool
> -check_function_arguments (location_t loc, const_tree fntype, int nargs,
> - tree *argarray)
> +check_function_arguments (location_t loc, const_tree fndecl, const_tree 
> fntype,
> + int nargs, tree *argarray)
>  {
>bool warned_p = false;
>
> @@ -5624,6 +5624,44 @@ check_function_arguments (location_t loc
>
>if (warn_format)
>  check_function_sentinel (fntype, nargs, argarray);
> +
> +  if (warn_restrict)
> +{
> +  int i;
> +  tree parms;
> +
> +  if (fndecl
> + && TREE_CODE (fndecl) == FUNCTION_DECL
> + && DECL_ARGUMENTS (fndecl))
> +   parms = DECL_ARGUMENTS (fndecl);
> +  else
> +   parms = TYPE_ARG_TYPES (fntype);
> +
> +  for (i = 0; i < nargs; i++)
> +TREE_VISITED (argarray[i]) = 0;
> +
> +  for (i = 0; i < nargs && parms && parms != void_list_node; i++)
> +   {
> + tree type;
> + if (TREE_CODE (parms) == PARM_DECL)
> +   {
> + type = TREE_TYPE (parms);
> + parms = DECL_CHAIN (parms);
> +   }
> + else
> +   {
> + type = TREE_VALUE (parms);
> + parms = TREE_CHAIN (parms);
> +   }
> + if (POINTER_TYPE_P (type)
> + && TYPE_RESTRICT (type)
> + && !TYPE_READONLY (TREE_TYPE (type)))
> +   warn_for_restrict (i, argarray, nargs);
> +   }
> +
> +  for (i = 0; i < nargs; i++)
> +TREE_VISITED (argarray[i]) = 0;
> +}
>return warned_p;
>  }
>
> --- gcc/c-family/c-warn.c.jj2017-02-15 18:06:19.0 +0100
> +++ gcc/c-family/c-warn.c   2017-02-20 12:36:29.008455672 +0100
> @@ -2170,55 +2170,49 @@ maybe_warn_bool_compare (location_t loc,
> restrict-qualified param, and it aliases with another argument.  */
>
>  void
> -warn_for_restrict (unsigned param_pos, vec *args)
> +warn_for_restrict (unsigned param_pos, tree *argarray, unsigned nargs)
>  {
> -  tree arg = (*args)[param_pos];
> -  if (TREE_VISITED (arg) || operand_equal_p (arg, null_pointer_node, 0))
> +  tree arg = argarray[param_pos];
> +  if (TREE_VISITED (arg) || integer_zerop (arg))
>  return;
>
>location_t loc = EXPR_LOC_OR_LOC (arg, input_location);
>gcc_rich_location richloc (loc);
>
>unsigned i;
> -  tree current_arg;
> -  int *arg_positions = XNEWVEC (int, args->length ());
> -  unsigned arg_positions_len = 0;
> +  auto_vec arg_positions;
>
> -  FOR_EACH_VEC_ELT (*args, i, current_arg)
> +  for (i = 0; i < nargs; i++)
>  {
>if (i == param_pos)
> continue;
>
> -  tree current_arg = (*args)[i];
> +  tree current_arg = argarray[i];
>if (operand_equal_p (arg, current_arg, 0))
> {
>   TREE_VISITED (current_arg) = 1;
> - arg_positions[arg_positions_len++] = (i + 1);
> + arg_positions.safe_push (i + 1);
> }
>  }

Re: [v3 PATCH] Implement LWG 2825, LWG 2756 breaks class template argument deduction for optional.

2017-02-21 Thread Jason Merrill
On Mon, Jan 30, 2017 at 4:48 PM, Ville Voutilainen
 wrote:
> On 31 January 2017 at 00:41, Ville Voutilainen
>  wrote:
>> On 31 January 2017 at 00:06, Tim Song  wrote:
>>> On Mon, Jan 30, 2017 at 9:36 PM Jonathan Wakely  wrote:

 On 30/01/17 13:28 +, Jonathan Wakely wrote:
 >On 30/01/17 13:47 +0200, Ville Voutilainen wrote:
 >>Tested on Linux-x64.
 >
 >OK, thanks.

 To be clear: this isn't approved by LWG yet, but I think we can be a
 bit adventurous with deduction guides and add them for experimental
 C++17 features. Getting more usage experience before we standardise
 these things will be good, and deduction guides are very new and
 untried. If we find problems we can remove them again, and will have
 invaluable feedback for the standards committee.

>>>
>>> My brain compiler says that this may cause problems with
>>>
>>> std::optional o1;
>>> std::optional o2 = o1; // wanted optional, deduced 
>>> optional>
>>>
>>> Trunk GCC deduces optional, but I don't think it implements
>>> P0512R0 yet, which prefers explicit guides to implicit ones before
>>> considering partial ordering. This example is very similar to the
>>> example in https://timsong-cpp.github.io/cppwp/over.match.best#1.6.
>>
>>
>> I'll see about constraining the guide tomorrow.
>
> I don't actually need to constrain it, I could just add a guide like
>
> template  optional(optional<_Tp>) -> optional<_Tp>;
>
> However, I'm not convinced I need to. The preference to an explicit guide is, 
> at least based
> on that paper, a tie-breaker rule. If the copy/move constructors are better 
> matches than the guide,
> those should be picked over a guide. Jason?

Currently G++ first tries deduction directly from the intializer like
in a call to a function template; only if that fails does it consider
deduction guides.  How to handle class deduction WRT
implicitly-declared constructors is still very much under discussion.

Jason


C++ PATCH for c++/17729, c++/50308, deprecated warning issues

2017-02-21 Thread Jason Merrill
Both of these PRs are problems with the attribute deprecated warning
in cases when there are no other declarations of the name in scope.
[basic.def.odr] says that a function is odr-used if it is the unique
result of name lookup, and also if it is selected by overload
resolution; in 17729 that results in a duplicate warning, and in 50308
in a bogus warning because a different function is chosen by
arg-dependent lookup.

The solution for these is to avoid calling mark_used on the lookup
when we know we're building a call.

Tested x86_64-pc-linux-gnu, applying to trunk.
commit b823b69a6cec77c5e96d0383b919a8da14b13a72
Author: Jason Merrill 
Date:   Tue Feb 21 09:45:19 2017 -0800

PR c++/50308 - wrong deprecated warning with ADL

PR c++/17729 - duplicate deprecated warning
* semantics.c (finish_id_expression): Only call mark_used on a
function if we aren't building a call.

diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 6a47476..6ba7c13 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -3743,7 +3743,15 @@ finish_id_expression (tree id_expression,
  if (TREE_CODE (first_fn) == TEMPLATE_DECL)
first_fn = DECL_TEMPLATE_RESULT (first_fn);
 
- if (!really_overloaded_fn (decl)
+ /* [basic.def.odr]: "A function whose name appears as a
+potentially-evaluated expression is odr-used if it is the unique
+lookup result".
+
+But only mark it if it's a complete postfix-expression; in a call,
+ADL might select a different function, and we'll call mark_used in
+build_over_call.  */
+ if (done
+ && !really_overloaded_fn (decl)
  && !mark_used (first_fn))
return error_mark_node;
 
diff --git a/gcc/testsuite/c-c++-common/pr69558.c 
b/gcc/testsuite/c-c++-common/pr69558.c
index 102d72c..4c6d498 100644
--- a/gcc/testsuite/c-c++-common/pr69558.c
+++ b/gcc/testsuite/c-c++-common/pr69558.c
@@ -16,4 +16,4 @@
 
 __attribute__((deprecated)) void foo (void); /* { dg-bogus "declared here" "" 
{ xfail { c++ } } } */
 
-C (foo) /* { dg-bogus "is deprecated"  "" { xfail { c++ } } } */
+C (foo) /* { dg-bogus "is deprecated" } */
diff --git a/gcc/testsuite/g++.dg/warn/deprecated-12.C 
b/gcc/testsuite/g++.dg/warn/deprecated-12.C
new file mode 100644
index 000..df5c76f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/deprecated-12.C
@@ -0,0 +1,20 @@
+// PR c++/50308
+
+void A( int ) __attribute__((deprecated));
+
+namespace B {
+
+  struct C {};
+
+  void A(C) {}
+
+}
+
+int main ()
+{
+  B::C x;
+
+  // ADL correctly identifies the non-deprecated B::A, but a warning about the
+  // global A is generated anyway
+  A( x );  // { dg-bogus "deprecated" }
+}


C++ PATCH for c++/41727, ICE with partial specialization of member of instantiation

2017-02-21 Thread Jason Merrill
This was a regression from rejects-valid to ice-on-valid.  This patch
fixes the ICE, but not the underlying bug.

Tested x86_64-pc-linux-gnu, applying to trunk.
commit 6788ad9769bfce33d478c426aafbebccdbb5f795
Author: Jason Merrill 
Date:   Tue Feb 21 08:21:55 2017 -0800

PR c++/41727 - ICE with partial spec of partial instantiation

* pt.c (process_partial_specialization): For now, don't check more
specialized if there is more than one level of args.

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 2cac24f..475ac1f 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -4619,6 +4619,9 @@ process_partial_specialization (tree decl)
 
   /* If we aren't in a dependent class, we can actually try deduction.  */
   else if (tpd.level == 1
+  /* FIXME we should be able to handle a partial specialization of a
+ partial instantiation, but currently we can't (c++/41727).  */
+  && TMPL_ARGS_DEPTH (specargs) == 1
   && !get_partial_spec_bindings (maintmpl, maintmpl, specargs))
 {
   if (permerror (input_location, "partial specialization %qD is not "
diff --git a/gcc/testsuite/g++.dg/template/partial-specialization5.C 
b/gcc/testsuite/g++.dg/template/partial-specialization5.C
new file mode 100644
index 000..7a8db5a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/partial-specialization5.C
@@ -0,0 +1,22 @@
+// PR c++/41727
+
+struct tag0;
+
+template < class Tag > struct outer
+{
+  template < typename Arg0, typename Arg1 > struct inner;
+};
+
+template < int Value > struct value_wrap { };
+
+template 
+template < typename Arg0, int Arg1 >
+struct outer ::inner < Arg0, value_wrap < Arg1 > >
+{
+  typedef Arg0 type;
+};
+
+typedef outer < tag0 >
+::inner < tag0, value_wrap < 999 > >
+::type // { dg-bogus "incomplete" "" { xfail *-*-* } }
+  outer_inner_type;


Re: [PATCH 3/6] microblaze: Fixes for RTL checking

2017-02-21 Thread Michael Eager

On 02/21/2017 12:25 PM, Segher Boessenkool wrote:

On Tue, Feb 21, 2017 at 12:08:34PM -0800, Michael Eager wrote:

-  /* Shift by zero -- copy regs if necessary.  */
+  /* Shift by zero -- copy regs.  */
if ((GET_CODE (operands[2]) == CONST_INT) && (INTVAL (operands[2]) ==
0))
  {
-  if (REGNO (operands[0]) != REGNO (operands[1]))
-   emit_insn (gen_movsi (operands[0], operands[1]));
+  emit_insn (gen_movsi (operands[0], operands[1]));
return 1;
  }


Why generate an unnecessary NOP?


Why not?  It will be optimised away anyway, and the code to get at the
subregs is hairy...  But could optimise away the useless move here
already if both ops are a reg and both are the same, if you prefer that?
Or rtx_equal_p (operands[0], operands[1])?


It's optimized away only if optimization is on.

The existing code checks and does not generate the unneeded move.
Whatever you change to clean up code should maintain the same behavior.

if ((operands[2] == const0_rtx) && !rtx_equal_p (operands[0], operands[1]))
  {
emit_insn (gen_movsi (operands[0], operands[1]));
  }


--
Michael Eagerea...@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077


Re: [PATCH 3/6] microblaze: Fixes for RTL checking

2017-02-21 Thread Segher Boessenkool
On Tue, Feb 21, 2017 at 12:35:17PM -0800, Michael Eager wrote:
> On 02/21/2017 12:15 PM, Jakub Jelinek wrote:
> >On Tue, Feb 21, 2017 at 02:48:15PM +, Segher Boessenkool wrote:
> >>-  /* Shift by zero -- copy regs if necessary.  */
> >>+  /* Shift by zero -- copy regs.  */
> >>if ((GET_CODE (operands[2]) == CONST_INT) && (INTVAL (operands[2]) == 
> >>0))
> >
> >You could have changed this line to
> >   if (operands[2] == const0_rtx)
> >as well.
> 
> And this would not change the generated code.

Of course, but you still need the other changes.

I did not make this random cleanup because a) it is a random cleanup; and
b) there are at least three occurrences of this in both microblaze.c and
microblaze.md .

Michael, will you make a fix yourself?  --enable-checking=yes,rtl will
reproduce the problem.


Segher


  1   2   >