[PATCH] Adjust g++.dg/tree-ssa/pr61034.C

2016-02-11 Thread Richard Biener

I forgot to update this after adding the late DCE pass.  We now have
zero calls to free in .optimized.

Tested on x86_64-unknown-linux-gnu.

Richard.

2016-02-11  Richard Biener  

* g++.dg/tree-ssa/pr61034.C: Adjust.

Index: gcc/testsuite/g++.dg/tree-ssa/pr61034.C
===
--- gcc/testsuite/g++.dg/tree-ssa/pr61034.C (revision 233268)
+++ gcc/testsuite/g++.dg/tree-ssa/pr61034.C (working copy)
@@ -49,6 +49,5 @@ bool f(I a, I b, I c, I d) {
 // a different initial CFG and thus the final outcome is different
 
 // { dg-final { scan-tree-dump-times "free" 10 "fre3" { target x86_64-*-* 
i?86-*-* } } }
-// { dg-final { scan-tree-dump-times "free" 3 "optimized" { target x86_64-*-* 
i?86-*-* } } }
 // { dg-final { scan-tree-dump-times "free" 14 "fre3" { target aarch64-*-* 
ia64-*-* arm-*-* hppa*-*-* sparc*-*-* powerpc*-*-* alpha*-*-* } } }
-// { dg-final { scan-tree-dump-times "free" 4 "optimized" { target aarch64-*-* 
ia64-*-* arm-*-* hppa*-*-* sparc*-*-* powerpc*-*-* alpha*-*-* } } }
+// { dg-final { scan-tree-dump-times "free" 0 "optimized" } }


Re: [PATCH] PR driver/69265: improved suggestions for various misspelled options

2016-02-11 Thread Richard Biener
On Wed, Feb 10, 2016 at 5:25 PM, Bernd Schmidt  wrote:
> On 02/09/2016 09:44 PM, David Malcolm wrote:
>>
>> This is a bug in a new feature, so it isn't a regression as such, but
>> it's fairly visible, and I believe the fix is relatively low-risk
>> (error-handling of typos of command-line options).
>>
>> This also now covers PR driver/69453 (and its duplicate PR
>> driver/69642), so people *are* running into this.
>
>
> I think the patch looks reasonable (I expect it needs slight adjustment
> after an earlier sanitizer options change). Whether it's OK or not at this
> stage is something I think I'll want to ask a RM. My inclination would be
> yes.

Yes.

Richard.

> A small improvement might be calculating the candidates array only once when
> making the first suggestion and not freeing it. BTW, I've also run into a
> case of an unhelpful suggestion:
>
> ./cc1 ~/hw.c -fno-if-convert
> cc1: error: unrecognized command line option ‘-fno-if-convert’
>
> which should instead suggest fno-if-conversion.
>
>
> Bernd


Re: [PATCH] Fix unnecessary -Wmaybe-uninitialized false positive (PR target/65313)

2016-02-11 Thread Richard Biener
On Wed, 10 Feb 2016, Jakub Jelinek wrote:

> Hi!
> 
> During profiledbootstrap on ppc64 I've noticed a -Wmaybe-uninitialized
> warning in vect_schedule_slp_instance, when built with -fprofile-generate.
> While it is clearly a false positive, IMHO it is completely unnecessary
> to use here two variables, one uninitialized, another bool whether
> it is initialized.  In valid code gimple_assign_rhs_code should not
> return ERROR_MARK, so we can use ocode == ERROR_MARK for the allsame
> case and ocode != ERROR_MARK for !allsame.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Thanks,
Richard.

> 2016-02-10  Jakub Jelinek  
> 
>   PR target/65313
>   * tree-vect-slp.c (vect_schedule_slp_instance): Avoid
>   -Wmaybe-uninitialized warning.
> 
> --- gcc/tree-vect-slp.c.jj2016-01-21 13:54:19.0 +0100
> +++ gcc/tree-vect-slp.c   2016-02-09 13:40:30.280769470 +0100
> @@ -3568,20 +3568,18 @@ vect_schedule_slp_instance (slp_tree nod
>if (SLP_TREE_TWO_OPERATORS (node))
>  {
>enum tree_code code0 = gimple_assign_rhs_code (stmt);
> -  enum tree_code ocode;
> +  enum tree_code ocode = ERROR_MARK;
>gimple *ostmt;
>unsigned char *mask = XALLOCAVEC (unsigned char, group_size);
> -  bool allsame = true;
>FOR_EACH_VEC_ELT (SLP_TREE_SCALAR_STMTS (node), i, ostmt)
>   if (gimple_assign_rhs_code (ostmt) != code0)
> {
>   mask[i] = 1;
> - allsame = false;
>   ocode = gimple_assign_rhs_code (ostmt);
> }
>   else
> mask[i] = 0;
> -  if (!allsame)
> +  if (ocode != ERROR_MARK)
>   {
> vec v0;
> vec v1;
> 
>   Jakub
> 
> 

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


Re: [PATCH] Fix another ipa-split caused ICE (PR ipa/69241)

2016-02-11 Thread Richard Biener
On Wed, 10 Feb 2016, Jakub Jelinek wrote:

> Hi!
> 
> Markus has pointed out to a reduced testcase which still ICEs even with the
> PR69241 fix.  In that case the function with TREE_ADDRESSABLE return type
> does not return at all (and -Wreturn-type properly diagnoses it).
> For that case the following patch just forces the lhs on the *.part.*
> call, so that we don't ICE in assign_temp.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Um...  I wonder if it wouldn't be better to force such functions to
be noreturn by say, placing a __builtin_unreachable () at the missing
return and maybe even adjust the return type.

In the frontend I mean.

After all, what would the code below do at runtime?  Unhelpfully
write to a random memory location.  So if then I'd rather
dereference literal zero here (well, not sure what to do for
-fno-delete-null-pointer targets).  Otherwise this becomes a bigger
security issue than not initializing the not used return value?

The other option is to simply not split the function in this case.

Richard.

> 2016-02-10  Jakub Jelinek  
> 
>   PR ipa/69241
>   * ipa-split.c (split_function): If split part returns TREE_ADDRESSABLE
>   type by reference, force lhs on the call.
> 
>   * g++.dg/ipa/pr69241-4.C: New test.
> 
> --- gcc/ipa-split.c.jj2016-02-10 16:05:37.0 +0100
> +++ gcc/ipa-split.c   2016-02-10 17:18:12.553061670 +0100
> @@ -1589,7 +1589,20 @@ split_function (basic_block return_bb, s
>   }
>   }
> else
> - gsi_insert_after (&gsi, call, GSI_NEW_STMT);
> + {
> +   /* Force a lhs if the split part has to return a value.  */
> +   if (split_point->split_part_set_retval
> +   && DECL_BY_REFERENCE (DECL_RESULT (current_function_decl)))
> + {
> +   retval = DECL_RESULT (current_function_decl);
> +   if (TREE_ADDRESSABLE (TREE_TYPE (TREE_TYPE (retval
> + {
> +   retval = get_or_create_ssa_default_def (cfun, retval);
> +   gimple_call_set_lhs (call, build_simple_mem_ref (retval));
> + }
> + }
> +   gsi_insert_after (&gsi, call, GSI_NEW_STMT);
> + }
> if (tsan_func_exit_call)
>   gsi_insert_after (&gsi, tsan_func_exit_call, GSI_NEW_STMT);
>   }
> --- gcc/testsuite/g++.dg/ipa/pr69241-4.C.jj   2016-02-10 17:22:03.977866326 
> +0100
> +++ gcc/testsuite/g++.dg/ipa/pr69241-4.C  2016-02-10 17:22:00.073920229 
> +0100
> @@ -0,0 +1,55 @@
> +// PR ipa/69241
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-O2 -Wno-return-type" }
> +
> +template  class A;
> +struct B {
> +  using pointer = int *;
> +};
> +template > class basic_string {
> +  long _M_string_length;
> +  enum { _S_local_capacity = 15 } _M_local_buf[_S_local_capacity];
> +  B::pointer _M_local_data;
> +
> +public:
> +  ~basic_string();
> +};
> +template 
> +int operator<<(_Traits, basic_string<_CharT, _Alloc>);
> +class C {
> +  basic_string> _M_string;
> +};
> +class D {
> +  C _M_stringbuf;
> +};
> +class F {
> +  int stream;
> +  D stream_;
> +};
> +class G {
> +public:
> +  void operator&(int);
> +};
> +class H {
> +public:
> +  H(unsigned);
> +  H(H &&);
> +  bool m_fn1();
> +};
> +class I {
> +  void m_fn2(const int &&);
> +  static H m_fn3(const int &);
> +};
> +template  void Bind(Functor);
> +class J {
> +public:
> +  static basic_string m_fn4();
> +};
> +int a;
> +void I::m_fn2(const int &&) { Bind(m_fn3); }
> +H I::m_fn3(const int &) {
> +  !false ? (void)0 : G() & F() << J::m_fn4();
> +  H b(a);
> +  if (b.m_fn1())
> +F();
> +}
> 
>   Jakub
> 
> 

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


[SH][committed] Fix PR 69713

2016-02-11 Thread Oleg Endo
Hi,

The attached patch fixes PR 69713.  For details please see the comments
in the PR.

Tested on trunk and sh-elf with
make -k check RUNTESTFLAGS="--target_board=sh-sim\{-m2/-ml,-m2/-mb,
-m2a/-mb,-m4/-ml,-m4/-mb,-m4a/-ml,-m4a/-mb}"

Committed to trunk as r233324, 5 branch as r233326 and 4.9 branch as
r233329.

Cheers,
Oleg


gcc/ChangeLog
PR target/69713
* config/sh/sh.md (casesi_worker_0): Add T_REG use.Index: gcc/config/sh/sh.md
===
--- gcc/config/sh/sh.md	(revision 233314)
+++ gcc/config/sh/sh.md	(working copy)
@@ -11707,12 +11707,16 @@
 ;; ??? reload might clobber r0 if we use it explicitly in the RTL before
 ;; reload; using a R0_REGS pseudo reg is likely to give poor code.
 ;; So we keep the use of r0 hidden in a R0_REGS clobber until after reload.
+;;
+;; The use on the T_REG in the casesi_worker* patterns links the bounds
+;; checking insns and the table memory access.  See also PR 69713.
 (define_insn "casesi_worker_0"
   [(set (match_operand:SI 0 "register_operand" "=r,r")
 	(unspec:SI [(match_operand:SI 1 "register_operand" "0,r")
 		 (label_ref (match_operand 2 "" ""))] UNSPEC_CASESI))
(clobber (match_scratch:SI 3 "=X,1"))
-   (clobber (match_scratch:SI 4 "=&z,z"))]
+   (clobber (match_scratch:SI 4 "=&z,z"))
+   (use (reg:SI T_REG))]
   "TARGET_SH1"
   "#")
 
@@ -11721,7 +11725,8 @@
 	(unspec:SI [(match_operand:SI 1 "register_operand" "")
 		(label_ref (match_operand 2 "" ""))] UNSPEC_CASESI))
(clobber (match_scratch:SI 3 ""))
-   (clobber (match_scratch:SI 4 ""))]
+   (clobber (match_scratch:SI 4))
+   (use (reg:SI T_REG))]
   "TARGET_SH1 && ! TARGET_SH2 && reload_completed"
   [(set (reg:SI R0_REG) (unspec:SI [(label_ref (match_dup 2))] UNSPEC_MOVA))
(parallel [(set (match_dup 0)
@@ -11739,7 +11744,8 @@
 	(unspec:SI [(match_operand:SI 1 "register_operand" "")
 		(label_ref (match_operand 2 "" ""))] UNSPEC_CASESI))
(clobber (match_scratch:SI 3 ""))
-   (clobber (match_scratch:SI 4 ""))]
+   (clobber (match_scratch:SI 4))
+   (use (reg:SI T_REG))]
   "TARGET_SH2 && reload_completed"
   [(set (reg:SI R0_REG) (unspec:SI [(label_ref (match_dup 2))] UNSPEC_MOVA))
(parallel [(set (match_dup 0)


[PATCH, reload] PRE_INC with invalid hard reg

2016-02-11 Thread Alan Modra

This is PR68973 part 1, the fix for the regression of g++.dg/pr67211.C
on powerpc64-linux -mcpu=power7, which turns out to be a reload
problem.

Due to uses elsewhere in vsx instructions, reload chooses to put
psuedo 185 in fr31, which can't be used as a base register in the
following:
(set (reg/f:DI 178)
 (mem/f:DI (pre_inc:DI (reg/f:DI 185
So find_reloads_address decides that the (pre_inc:DI (reg:DI 63))
needs reloading, and pushes a reload on the entire address
expression.  This is unfortunate because the rs6000 backend reload
hooks don't look inside a pre_inc and thus don't arrange a secondary
reload.  Nor does the generic SECONDARY_MEMORY_NEEDED handling, which
is only prepared to handle regs or subregs.  So reload doesn't
allocate the stack temp needed to copy between fprs and gprs on
power7.

Now it turns out that if find_reloads_address instead pushed a reload
of just the (reg:DI 63), then SECONDARY_MEMORY_NEEDED would get a
chance to do something.  Furthermore, there is existing code in
find_reloads_address to do just that for pre_inc, but only enabled for
psuedos that don't get a hard reg.  The following patch extends this
to invalid hard regs.  I could have implemented a fix in the rs6000
backend, but it seems likely that other targets may run into this
problem.

Bootstrapped and regression tested powerpc64le-linux and
x86_64-linux.  OK to apply?

PR target/68973
* reloads.c (find_reloads_address_1): For pre/post-inc/dec
with an invalid hard reg, reload just the reg not the entire
pre/post-inc/dec address expression.

diff --git a/gcc/reload.c b/gcc/reload.c
index 6196e63..06426d9 100644
--- a/gcc/reload.c
+++ b/gcc/reload.c
@@ -5834,14 +5834,16 @@ find_reloads_address_1 (machine_mode mode, addr_space_t 
as,
   ? XEXP (x, 0)
   : reg_equiv_mem (regno));
  enum insn_code icode = optab_handler (add_optab, GET_MODE (x));
- if (insn && NONJUMP_INSN_P (insn) && equiv
- && memory_operand (equiv, GET_MODE (equiv))
+ if (insn && NONJUMP_INSN_P (insn)
 #if HAVE_cc0
  && ! sets_cc0_p (PATTERN (insn))
 #endif
- && ! (icode != CODE_FOR_nothing
-   && insn_operand_matches (icode, 0, equiv)
-   && insn_operand_matches (icode, 1, equiv))
+ && (regno < FIRST_PSEUDO_REGISTER
+ || (equiv
+ && memory_operand (equiv, GET_MODE (equiv))
+ && ! (icode != CODE_FOR_nothing
+   && insn_operand_matches (icode, 0, equiv)
+   && insn_operand_matches (icode, 1, equiv
  /* Using RELOAD_OTHER means we emit this and the reload we
 made earlier in the wrong order.  */
  && !reloaded_inner_of_autoinc)

-- 
Alan Modra
Australia Development Lab, IBM


gcc-4_9-branch backports

2016-02-11 Thread Jakub Jelinek
Hi!

I've committed following backports of my trunk commits to gcc-4_9-branch
after bootstrapping/regtesting them on x86_64-linux and i686-linux.

Jakub
2016-02-11  Jakub Jelinek  

Backported from mainline
2015-11-19  Jakub Jelinek  

PR preprocessor/60736
* include/cpplib.h (cpp_errno_filename): New prototype.
* errors.c (cpp_errno): Don't handle msgid "" specially, use
_(msgid) instead of msgid as argument to cpp_error.
(cpp_errno_filename): New function.
* files.c (read_file_guts): Use cpp_errno_filename instead of
cpp_errno.
(open_file_failed): Likewise.  Use file->name if file->path is NULL
in diagnostics.

--- libcpp/include/cpplib.h (revision 230590)
+++ libcpp/include/cpplib.h (revision 230591)
@@ -986,6 +986,9 @@ extern bool cpp_warning_syshdr (cpp_read
 /* Output a diagnostic with "MSGID: " preceding the
error string of errno.  No location is printed.  */
 extern bool cpp_errno (cpp_reader *, int, const char *msgid);
+/* Similarly, but with "FILENAME: " instead of "MSGID: ", where
+   the filename is not localized.  */
+extern bool cpp_errno_filename (cpp_reader *, int, const char *filename);
 
 /* Same as cpp_error, except additionally specifies a position as a
(translation unit) physical line and physical column.  If the line is
--- libcpp/files.c  (revision 230590)
+++ libcpp/files.c  (revision 230591)
@@ -715,7 +715,7 @@ read_file_guts (cpp_reader *pfile, _cpp_
 
   if (count < 0)
 {
-  cpp_errno (pfile, CPP_DL_ERROR, file->path);
+  cpp_errno_filename (pfile, CPP_DL_ERROR, file->path);
   free (buf);
   return false;
 }
@@ -1053,7 +1053,8 @@ open_file_failed (cpp_reader *pfile, _cp
   /* If the preprocessor output (other than dependency information) is
  being used, we must also flag an error.  */
   if (CPP_OPTION (pfile, deps.need_preprocessor_output))
-   cpp_errno (pfile, CPP_DL_FATAL, file->path);
+   cpp_errno_filename (pfile, CPP_DL_FATAL,
+   file->path ? file->path : file->name);
 }
   else
 {
@@ -1067,9 +1068,11 @@ open_file_failed (cpp_reader *pfile, _cp
   if (CPP_OPTION (pfile, deps.style) == DEPS_NONE
   || print_dep
   || CPP_OPTION (pfile, deps.need_preprocessor_output))
-   cpp_errno (pfile, CPP_DL_FATAL, file->path);
+   cpp_errno_filename (pfile, CPP_DL_FATAL,
+   file->path ? file->path : file->name);
   else
-   cpp_errno (pfile, CPP_DL_WARNING, file->path);
+   cpp_errno_filename (pfile, CPP_DL_WARNING,
+   file->path ? file->path : file->name);
 }
 }
 
--- libcpp/errors.c (revision 230590)
+++ libcpp/errors.c (revision 230591)
@@ -230,8 +230,18 @@ cpp_warning_with_line_syshdr (cpp_reader
 bool
 cpp_errno (cpp_reader *pfile, int level, const char *msgid)
 {
-  if (msgid[0] == '\0')
-msgid = _("stdout");
+  return cpp_error (pfile, level, "%s: %s", _(msgid), xstrerror (errno));
+}
+
+/* Print a warning or error, depending on the value of LEVEL.  Include
+   information from errno.  Unlike cpp_errno, the argument is a filename
+   that is not localized, but "" is replaced with localized "stdout".  */
+
+bool
+cpp_errno_filename (cpp_reader *pfile, int level, const char *filename)
+{
+  if (filename[0] == '\0')
+filename = _("stdout");
 
-  return cpp_error (pfile, level, "%s: %s", msgid, xstrerror (errno));
+  return cpp_error (pfile, level, "%s: %s", filename, xstrerror (errno));
 }
2016-02-11  Jakub Jelinek  

Backported from mainline
2015-11-19  Jakub Jelinek  

PR target/67770
* config/i386/i386.md (simple_return): Disable if
ix86_static_chain_on_stack is true.

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

--- gcc/config/i386/i386.md (revision 230592)
+++ gcc/config/i386/i386.md (revision 230593)
@@ -12203,10 +12203,14 @@ (define_expand "return"
 ;; We need to disable this for TARGET_SEH, as otherwise
 ;; shrink-wrapped prologue gets enabled too.  This might exceed
 ;; the maximum size of prologue in unwind information.
+;; Also disallow shrink-wrapping if using stack slot to pass the
+;; static chain pointer - the first instruction has to be pushl %esi
+;; and it can't be moved around, as we use alternate entry points
+;; in that case.
 
 (define_expand "simple_return"
   [(simple_return)]
-  "!TARGET_SEH"
+  "!TARGET_SEH && !ix86_static_chain_on_stack"
 {
   if (crtl->args.pops_args)
 {
--- gcc/testsuite/gcc.target/i386/pr67770.c (revision 0)
+++ gcc/testsuite/gcc.target/i386/pr67770.c (revision 230593)
@@ -0,0 +1,40 @@
+/* PR target/67770 */
+/* { dg-do run { target ia32 } } */
+/* { dg-require-effective-target trampolines } */
+/* { dg-options "-O2" } */
+
+#ifndef NO_TRAMPOLINES
+__attribute__ ((noinline)) void
+foo (int i, void (* __attribute__ ((regparm (3))) bar) (int))
+{
+  bar (i);
+}
+#endif

Re: Un-parallelized OpenACC kernels constructs with nvptx offloading: "avoid offloading"

2016-02-11 Thread Thomas Schwinge
Hi!

There are two issues here: 1. "avoid offloading" mechanism, and 2. "avoid
offloading" policy.

On Wed, 10 Feb 2016 21:07:29 +0100, Bernd Schmidt  wrote:
> On 02/10/2016 06:37 PM, Thomas Schwinge wrote:
> > On Wed, 10 Feb 2016 17:37:30 +0100, Bernd Schmidt  
> > wrote:
> >> IIUC it's also disabling offloading for parallels rather than just
> >> kernels, which we previously said shouldn't happen.
> >
> > Ah, you're talking about mixed OpenACC parallel/kernels codes -- I
> > understood the earlier discussion to apply to parallel-only codes, where
> > the "avoid offloading" flag will never be set.  In mixed parallel/kernels
> > code with one un-parallelized kernels construct, offloading would also
> > (have to be) disabled for the parallel constructs (for the same data
> > consistency reasons explained before).

The "avoid offloading" mechanism.  Owed to the non-shared-memory
offloading architecture, if the compiler/runtime decides to "avoid
offloading", then this has to apply to *all* code offloading, for data
consistency reasons.  Do we agree on that?

> > The majority of codes I've seen
> > use either parallel or kernels constructs, typically not both.
> 
> That's not something I'd want to hard-code into the compiler however. 
> Don't know how Jakub feels but to me this approach is way too 
> coarse-grained.

The "avoid offloading" policy.  I'm looking into improving that.


> > Huh?  Like, at random, discouraging users from using GCC's SIMD
> > vectorizer just because that one fails to vectorize some code that it
> > could/should vectorize?  (Of course, I'm well aware that GCC's SIMD
> > vectorizer is much more mature than the OpenACC kernels/parloops
> > handling; it's seen many more years of development.)
> 
> Your description sounded like it's not actually not optimizing, but 
> actively hurting performance for a large selection of real world codes. 

Indeed single-threaded (that is, un-parallelized OpenACC kernels
construct) offloading execution is hurting performance (data copy
overhead; kernel launch overhead; compared to a single CPU core, a single
GPU core has higher memory access latencies and is slower) -- hence the
idea to resort to host-fallback execution in such a situation.

> If I understood that correctly, we need to document this in the manual.

OK; prototyping that on .


Grüße
 Thomas


Re: [PATCH] Fix PR69726

2016-02-11 Thread Christophe Lyon
On 9 February 2016 at 13:42, Richard Biener  wrote:
>
> It turns out if-conversions poor job on
>
>  if (a)
>x[i] = ...;
>  else
>x[i] = ...;
>
> results in bogus uninit warnings of x[i] for a variety of reasons.
> First of all forwprop (aka match.pd simplification) doesn't fixup
> all of if-conversions poor job as canonicalization sometimes
> inverts the condition in [VEC_]COND_EXPRs and thus the existing
> A ? B : (A ? X : C) -> A ? B : C pattern doesn't apply.  The match.pd
> hunk fixes this (albeit in an awkward way - I don't feel like mucking
> with genmatch at this stage, nor exactly for the poor [VEC_]COND_EXPR
> IL we should rather fix).  Second, the late uninit pass is confused
> by the left-over dead code, in this case dead load feeding a dead
> VEC_COND_EXPR.  Adding a DCE pass before late uninit as the comment
> in passes.def suggests fixes this and also should avoid creating the dead
> RTL I've sometimes seen.
>
> Due to the PR69719 fix we're now over the alias-test limit for the
> testcase (well, all alias tests are bogus, see PR69732), so I upped
> that limit for the testcase.  I'm investigating the Job done there.
>
> Bootstrap and regtest is currently running on x86_64-unknown-linux-gnu.
>
Hi,

After this patch, I noticed that
  g++.dg/tree-ssa/pr61034.C  -std=gnu++11  scan-tree-dump-times
optimized "free" 4
now fails on arm* and aarch64*
The dump contains no occurrence of "free".

Is this just a matter of adjusting the test-case or an unexpected
problem with this patch?

Christophe.

> Richard.
>
> 2016-02-09  Richard Biener  
>
> PR tree-optimization/69726
> * passes.def: Add DCE pass before late uninit.
> * match.pd: Add A ? B : (!A ? C : X) -> A ? B : C patterns to
> really fixup if-conversions job.
>
> * gcc.dg/uninit-22.c: New testcase.
>
> Index: gcc/passes.def
> ===
> *** gcc/passes.def  (revision 233241)
> --- gcc/passes.def  (working copy)
> *** along with GCC; see the file COPYING3.
> *** 322,336 
> NEXT_PASS (pass_fold_builtins);
> NEXT_PASS (pass_optimize_widening_mul);
> NEXT_PASS (pass_tail_calls);
> !   /* FIXME: If DCE is not run before checking for uninitialized uses,
>  we may get false warnings (e.g., testsuite/gcc.dg/uninit-5.c).
>  However, this also causes us to misdiagnose cases that should be
> !real warnings (e.g., testsuite/gcc.dg/pr18501.c).
> !
> !To fix the false positives in uninit-5.c, we would have to
> !account for the predicates protecting the set and the use of each
> !variable.  Using a representation like Gated Single Assignment
> !may help.  */
> /* Split critical edges before late uninit warning to reduce the
>number of false positives from it.  */
> NEXT_PASS (pass_split_crit_edges);
> --- 322,332 
> NEXT_PASS (pass_fold_builtins);
> NEXT_PASS (pass_optimize_widening_mul);
> NEXT_PASS (pass_tail_calls);
> !   /* If DCE is not run before checking for uninitialized uses,
>  we may get false warnings (e.g., testsuite/gcc.dg/uninit-5.c).
>  However, this also causes us to misdiagnose cases that should be
> !real warnings (e.g., testsuite/gcc.dg/pr18501.c).  */
> !   NEXT_PASS (pass_dce);
> /* Split critical edges before late uninit warning to reduce the
>number of false positives from it.  */
> NEXT_PASS (pass_split_crit_edges);
> Index: gcc/match.pd
> ===
> *** gcc/match.pd(revision 233241)
> --- gcc/match.pd(working copy)
> *** DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> *** 1717,1722 
> --- 1717,1745 
>(simplify
> (cnd @0 @1 (cnd @0 @2 @3))
> (cnd @0 @1 @3))
> +  /* A ? B : (!A ? C : X) -> A ? B : C.  */
> +  /* ???  This matches embedded conditions open-coded because genmatch
> + would generate matching code for conditions in separate stmts only.
> + The following is still important to merge then and else arm cases
> + from if-conversion.  */
> +  (simplify
> +   (cnd @0 @1 (cnd @2 @3 @4))
> +   (if (COMPARISON_CLASS_P (@0)
> +&& COMPARISON_CLASS_P (@2)
> +&& invert_tree_comparison
> +(TREE_CODE (@0), HONOR_NANS (TREE_OPERAND (@0, 0))) == TREE_CODE 
> (@2)
> +&& operand_equal_p (TREE_OPERAND (@0, 0), TREE_OPERAND (@2, 0), 0)
> +&& operand_equal_p (TREE_OPERAND (@0, 1), TREE_OPERAND (@2, 1), 0))
> +(cnd @0 @1 @3)))
> +  (simplify
> +   (cnd @0 (cnd @1 @2 @3) @4)
> +   (if (COMPARISON_CLASS_P (@0)
> +&& COMPARISON_CLASS_P (@1)
> +&& invert_tree_comparison
> +(TREE_CODE (@0), HONOR_NANS (TREE_OPERAND (@0, 0))) == TREE_CODE 
> (@1)
> +&& operand_equal_p (TREE_OPERAND (@0, 0), TREE_OPERAND (@1, 0), 0)
> +&& operand_eq

Re: [PATCH] Fix PR69726

2016-02-11 Thread Richard Biener
On Thu, 11 Feb 2016, Christophe Lyon wrote:

> On 9 February 2016 at 13:42, Richard Biener  wrote:
> >
> > It turns out if-conversions poor job on
> >
> >  if (a)
> >x[i] = ...;
> >  else
> >x[i] = ...;
> >
> > results in bogus uninit warnings of x[i] for a variety of reasons.
> > First of all forwprop (aka match.pd simplification) doesn't fixup
> > all of if-conversions poor job as canonicalization sometimes
> > inverts the condition in [VEC_]COND_EXPRs and thus the existing
> > A ? B : (A ? X : C) -> A ? B : C pattern doesn't apply.  The match.pd
> > hunk fixes this (albeit in an awkward way - I don't feel like mucking
> > with genmatch at this stage, nor exactly for the poor [VEC_]COND_EXPR
> > IL we should rather fix).  Second, the late uninit pass is confused
> > by the left-over dead code, in this case dead load feeding a dead
> > VEC_COND_EXPR.  Adding a DCE pass before late uninit as the comment
> > in passes.def suggests fixes this and also should avoid creating the dead
> > RTL I've sometimes seen.
> >
> > Due to the PR69719 fix we're now over the alias-test limit for the
> > testcase (well, all alias tests are bogus, see PR69732), so I upped
> > that limit for the testcase.  I'm investigating the Job done there.
> >
> > Bootstrap and regtest is currently running on x86_64-unknown-linux-gnu.
> >
> Hi,
> 
> After this patch, I noticed that
>   g++.dg/tree-ssa/pr61034.C  -std=gnu++11  scan-tree-dump-times
> optimized "free" 4
> now fails on arm* and aarch64*
> The dump contains no occurrence of "free".
> 
> Is this just a matter of adjusting the test-case or an unexpected
> problem with this patch?

Yeah, I fixed the testcase up this morning.

Richard.

> Christophe.
> 
> > Richard.
> >
> > 2016-02-09  Richard Biener  
> >
> > PR tree-optimization/69726
> > * passes.def: Add DCE pass before late uninit.
> > * match.pd: Add A ? B : (!A ? C : X) -> A ? B : C patterns to
> > really fixup if-conversions job.
> >
> > * gcc.dg/uninit-22.c: New testcase.
> >
> > Index: gcc/passes.def
> > ===
> > *** gcc/passes.def  (revision 233241)
> > --- gcc/passes.def  (working copy)
> > *** along with GCC; see the file COPYING3.
> > *** 322,336 
> > NEXT_PASS (pass_fold_builtins);
> > NEXT_PASS (pass_optimize_widening_mul);
> > NEXT_PASS (pass_tail_calls);
> > !   /* FIXME: If DCE is not run before checking for uninitialized uses,
> >  we may get false warnings (e.g., testsuite/gcc.dg/uninit-5.c).
> >  However, this also causes us to misdiagnose cases that should be
> > !real warnings (e.g., testsuite/gcc.dg/pr18501.c).
> > !
> > !To fix the false positives in uninit-5.c, we would have to
> > !account for the predicates protecting the set and the use of each
> > !variable.  Using a representation like Gated Single Assignment
> > !may help.  */
> > /* Split critical edges before late uninit warning to reduce the
> >number of false positives from it.  */
> > NEXT_PASS (pass_split_crit_edges);
> > --- 322,332 
> > NEXT_PASS (pass_fold_builtins);
> > NEXT_PASS (pass_optimize_widening_mul);
> > NEXT_PASS (pass_tail_calls);
> > !   /* If DCE is not run before checking for uninitialized uses,
> >  we may get false warnings (e.g., testsuite/gcc.dg/uninit-5.c).
> >  However, this also causes us to misdiagnose cases that should be
> > !real warnings (e.g., testsuite/gcc.dg/pr18501.c).  */
> > !   NEXT_PASS (pass_dce);
> > /* Split critical edges before late uninit warning to reduce the
> >number of false positives from it.  */
> > NEXT_PASS (pass_split_crit_edges);
> > Index: gcc/match.pd
> > ===
> > *** gcc/match.pd(revision 233241)
> > --- gcc/match.pd(working copy)
> > *** DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > *** 1717,1722 
> > --- 1717,1745 
> >(simplify
> > (cnd @0 @1 (cnd @0 @2 @3))
> > (cnd @0 @1 @3))
> > +  /* A ? B : (!A ? C : X) -> A ? B : C.  */
> > +  /* ???  This matches embedded conditions open-coded because genmatch
> > + would generate matching code for conditions in separate stmts only.
> > + The following is still important to merge then and else arm cases
> > + from if-conversion.  */
> > +  (simplify
> > +   (cnd @0 @1 (cnd @2 @3 @4))
> > +   (if (COMPARISON_CLASS_P (@0)
> > +&& COMPARISON_CLASS_P (@2)
> > +&& invert_tree_comparison
> > +(TREE_CODE (@0), HONOR_NANS (TREE_OPERAND (@0, 0))) == 
> > TREE_CODE (@2)
> > +&& operand_equal_p (TREE_OPERAND (@0, 0), TREE_OPERAND (@2, 0), 0)
> > +&& operand_equal_p (TREE_OPERAND (@0, 1), TREE_OPERAND (@2, 1), 0))
> > +(cnd @0 @1 @3)))
> > +  (simplify
> > +   (cnd 

Re: [wwwdocs] Document null 'this' dereference issue in /gcc-6/porting_to.html

2016-02-11 Thread Jonathan Wakely

On 09/02/16 21:06 +, Jonathan Wakely wrote:

This adds a note to the porting document about the (shockingly
widespread) problem of calling member functions through null pointers,
which GCC 6 no longer tolerates.

Following some comments about (bool)os I'm also tweaking another part
of the doc to use the plusplusgood static_cast form.

Committed to CVS.


Tweak to conform to our spelling conventions.

Committed to CVS.


Index: htdocs/gcc-6/porting_to.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-6/porting_to.html,v
retrieving revision 1.9
diff -u -r1.9 porting_to.html
--- htdocs/gcc-6/porting_to.html	10 Feb 2016 17:21:54 -	1.9
+++ htdocs/gcc-6/porting_to.html	11 Feb 2016 10:34:43 -
@@ -248,13 +248,13 @@
 null, which is guaranteed by the language rules. Invalid programs which 
 assume it is OK to invoke a member function through a null pointer (possibly
 relying on checks like this != NULL) may crash or otherwise fail
-at run-time if null pointer checks are optimized away.
+at run time if null pointer checks are optimized away.
 With the -Wnull-dereference option the compiler tries to warn
 when it detects such invalid code.
 
 
 
-If the program cannot be fixed to remove the undefined behaviour then the
+If the program cannot be fixed to remove the undefined behavior then the
 option -fno-delete-null-pointer-checks can be used to disable
 this optimization. That option also disables other optimizations involving
 pointers, not only those involving this.


Re: [PATCH][ARM] PR target/69161: Don't ignore mode when matching comparison operator in cstore-like patterns

2016-02-11 Thread Kyrill Tkachov

Ping.

Thanks,
Kyrill
On 05/02/16 09:50, Kyrill Tkachov wrote:

Ping
https://gcc.gnu.org/ml/gcc-patches/2016-01/msg02309.html

Thanks,
Kyrill

On 29/01/16 14:27, Kyrill Tkachov wrote:

Hi all,

Similar to aarch64, the arm port also suffers from PR target/69161 when combine
tries to propagate a CCmode comparison into a vec_duplicate, creating invalid
RTL that ICEs.
Please refer to the PR and the aarch64 fix for more info.
The fix for arm is very similar.
We define a new predicate identical to arm_comparison_operator but
make it not special so that it gets the normal mode checks.
This prevents combine from matching an intermediate CCmode cstore
(where it's doing an SImode SET of a CCmode source) which it then
tries to propagate into a V4SImode vec_duplicate.

The offending patterns are the cstore patterns, so this patch updates them
to use the new predicate with mode checks. Both arm and thumb patterns
are updated.

There was no codegen difference observed on SPEC2006 for arm.
Bootstrapped and tested on arm-none-linux-gnueabihf.

Ok for trunk?

Thanks,
Kyrill

2016-01-29  Kyrylo Tkachov  

PR target/69161
* config/arm/predicates.md (arm_comparison_operator_mode):
New predicate.
* config/arm/arm.md (*mov_scc): Use arm_comparison_operator_mode
instead of arm_comparison_operator.
(*mov_negscc): Likewise.
(*mov_notscc): Likewise.
* config/arm/thumb2.md (*thumb2_mov_scc): Likewise.
(*thumb2_mov_negscc): Likewise.
(*thumb2_mov_negscc_strict_it): Likewise.
(*thumb2_mov_notscc): Likewise.
(*thumb2_mov_notscc_strict_it): Likewise.






Re: TR29124 C++ Special Maths - Make pull functions into global namespace.

2016-02-11 Thread Jonathan Wakely

On 10/02/16 22:36 -0800, Mike Stump wrote:

I’m seeing:

/home/mrs/work1/gcc/libstdc++-v3/testsuite/special_functions/18_riemann_zeta/check_value.cc:
 In function 'void test(const testcase_riemann_zeta (&)[Num], Tp)':
/home/mrs/work1/gcc/libstdc++-v3/testsuite/special_functions/18_riemann_zeta/check_value.cc:285:15:
 error: 'riemann_zeta' is not a member of 'std'
compiler exited with status 1
output is:
/home/mrs/work1/gcc/libstdc++-v3/testsuite/special_functions/18_riemann_zeta/check_value.cc:
 In function 'void test(const testcase_riemann_zeta (&)[Num], Tp)':
/home/mrs/work1/gcc/libstdc++-v3/testsuite/special_functions/18_riemann_zeta/check_value.cc:285:15:
 error: 'riemann_zeta' is not a member of 'std'

FAIL: special_functions/18_riemann_zeta/check_value.cc (test for excess errors)
Excess errors:
/home/mrs/work1/gcc/libstdc++-v3/testsuite/special_functions/18_riemann_zeta/check_value.cc:285:15:
 error: 'riemann_zeta' is not a member of 'std'

UNRESOLVED: special_functions/18_riemann_zeta/check_value.cc compilation failed 
to produce executable
extra_tool_flags are:
-D__STDCPP_WANT_MATH_SPEC_FUNCS__

on a recent trunk.  This is a typical newlib port.  Not sure if this is a real 
bug or not, but thought I’d forward it long.


This confused me at first, but I think what must be happening is that
your newlib port doesn't use c_model=c_global, so it uses a 
header that doesn't #include .

What does your $target/libstdc++-v3/config.log show instead of
c_global here?

configure:16391: "C" header strategy set to c_global

(Alternatively, just look at where the
$target/libstdc++-v3/include/cmath symlink points).




Re: [PING, patch, Fortran, pr69296, v1] [6 Regression] [F03] Problem with associate and vector subscript

2016-02-11 Thread Andre Vehreschild
PING

On Tue, 2 Feb 2016 18:37:27 +0100
Andre Vehreschild  wrote:

> Hi all,
> 
> the attached patch fixes a regression that was most likely introduced
> by one of my former patches, when in an associate() the rank of the
> associated variable could not be determined at parse time correctly.
> The patch now adds a flag to the association list indicating, that the
> rank of the associated variable has been guessed only. In the resolve
> phase the rank is corrected when the guess was wrong.
> 
> Bootstrapped and regtested ok on x86_64-linux-gnu/F23.
> 
> Ok for trunk?
> 
> Regards,
>   Andre


-- 
Andre Vehreschild * Email: vehre ad gmx dot de 
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 8441b8c..33fffd8 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -2344,6 +2344,9 @@ typedef struct gfc_association_list
  for memory handling.  */
   unsigned dangling:1;
 
+  /* True when the rank of the target expression is guessed during parsing.  */
+  unsigned rankguessed:1;
+
   char name[GFC_MAX_SYMBOL_LEN + 1];
   gfc_symtree *st; /* Symtree corresponding to name.  */
   locus where;
diff --git a/gcc/fortran/parse.c b/gcc/fortran/parse.c
index 5dcab70..7bce47f 100644
--- a/gcc/fortran/parse.c
+++ b/gcc/fortran/parse.c
@@ -4098,6 +4098,7 @@ parse_associate (void)
 	  int dim, rank = 0;
 	  if (array_ref)
 	{
+	  a->rankguessed = 1;
 	  /* Count the dimension, that have a non-scalar extend.  */
 	  for (dim = 0; dim < array_ref->dimen; ++dim)
 		if (array_ref->dimen_type[dim] != DIMEN_ELEMENT
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 8752fd4..8fb7a95 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -4777,7 +4777,7 @@ fail:
 /* Given a variable expression node, compute the rank of the expression by
examining the base symbol and any reference structures it may have.  */
 
-static void
+void
 expression_rank (gfc_expr *e)
 {
   gfc_ref *ref;
@@ -8153,16 +8153,19 @@ resolve_assoc_var (gfc_symbol* sym, bool resolve_target)
   if (target->rank != 0)
 {
   gfc_array_spec *as;
-  if (sym->ts.type != BT_CLASS && !sym->as)
+  /* The rank may be incorrectly guessed at parsing, therefore make sure
+	 it is corrected now.  */
+  if (sym->ts.type != BT_CLASS && (!sym->as || sym->assoc->rankguessed))
 	{
-	  as = gfc_get_array_spec ();
+	  if (!sym->as)
+	sym->as = gfc_get_array_spec ();
+	  as = sym->as;
 	  as->rank = target->rank;
 	  as->type = AS_DEFERRED;
 	  as->corank = gfc_get_corank (target);
 	  sym->attr.dimension = 1;
 	  if (as->corank != 0)
 	sym->attr.codimension = 1;
-	  sym->as = as;
 	}
 }
   else
diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c
index 5143c31..cb54499 100644
--- a/gcc/fortran/trans-stmt.c
+++ b/gcc/fortran/trans-stmt.c
@@ -1569,7 +1569,9 @@ trans_associate_var (gfc_symbol *sym, gfc_wrapped_block *block)
   if (sym->attr.subref_array_pointer)
 	{
 	  gcc_assert (e->expr_type == EXPR_VARIABLE);
-	  tmp = e->symtree->n.sym->backend_decl;
+	  tmp = e->symtree->n.sym->ts.type == BT_CLASS
+	  ? gfc_class_data_get (e->symtree->n.sym->backend_decl)
+	  : e->symtree->n.sym->backend_decl;
 	  tmp = gfc_get_element_type (TREE_TYPE (tmp));
 	  tmp = fold_convert (gfc_array_index_type, size_in_bytes (tmp));
 	  gfc_add_modify (&se.pre, GFC_DECL_SPAN(desc), tmp);
diff --git a/gcc/testsuite/gfortran.dg/associate_19.f03 b/gcc/testsuite/gfortran.dg/associate_19.f03
new file mode 100644
index 000..76534c5
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/associate_19.f03
@@ -0,0 +1,23 @@
+! { dg-do run }
+!
+! Contributed by mreste...@gmail.com
+! Adapated by Andre Vehreschild  
+! Test that fix for PR69296 is working.
+
+program p
+ implicit none
+
+ integer :: j, a(2,6), i(3,2)
+
+  a(1,:) = (/ ( j , j=1,6) /)
+  a(2,:) = (/ ( -10*j , j=1,6) /)
+
+  i(:,1) = (/ 1 , 3 , 5 /)
+  i(:,2) = (/ 4 , 5 , 6 /)
+
+  associate( ai => a(:,i(:,1)) )
+if (any(shape(ai) /= [2, 3])) call abort()
+if (any(reshape(ai, [6]) /= [1 , -10, 3, -30, 5, -50])) call abort()
+  end associate
+
+end program p
diff --git a/gcc/testsuite/gfortran.dg/associate_20.f03 b/gcc/testsuite/gfortran.dg/associate_20.f03
new file mode 100644
index 000..9d420ef
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/associate_20.f03
@@ -0,0 +1,31 @@
+! { dg-do run }
+!
+! Contributed by mreste...@gmail.com
+! Adapated by Andre Vehreschild  
+! Test that fix for PR69296 is working.
+
+program p
+  implicit none
+
+  type foo
+integer :: i
+  end type
+
+  integer :: j, i(3,2)
+  class(foo), allocatable :: a(:,:)
+
+  allocate (a(2,6))
+
+  a(1,:)%i = (/ ( j , j=1,6) /)
+  a(2,:)%i = (/ ( -10*j , j=1,6) /)
+
+  i(:,1) = (/ 1 , 3 , 5 /)
+  i(:,2) = (/ 4 , 5 , 6 /)
+
+  associate( ai => a(:,i(:,1))%i )
+if (any(shape(ai) /= [2, 3])) call abort()
+if (any(reshape(ai, [6]) /= [1 , -10, 3, -30, 5, -50])) call abort()
+  end associate
+
+  deallocate(a)
+end program p
gcc/f

Re: [PING, patch, Fortran, pr69296, v1] [6 Regression] [F03] Problem with associate and vector subscript

2016-02-11 Thread Paul Richard Thomas
Dear Andre,

That's very clever! OK for trunk

Thanks for the patch

Paul

On 11 February 2016 at 13:05, Andre Vehreschild  wrote:
> PING
>
> On Tue, 2 Feb 2016 18:37:27 +0100
> Andre Vehreschild  wrote:
>
>> Hi all,
>>
>> the attached patch fixes a regression that was most likely introduced
>> by one of my former patches, when in an associate() the rank of the
>> associated variable could not be determined at parse time correctly.
>> The patch now adds a flag to the association list indicating, that the
>> rank of the associated variable has been guessed only. In the resolve
>> phase the rank is corrected when the guess was wrong.
>>
>> Bootstrapped and regtested ok on x86_64-linux-gnu/F23.
>>
>> Ok for trunk?
>>
>> Regards,
>>   Andre
>
>
> --
> Andre Vehreschild * Email: vehre ad gmx dot de



-- 
The difference between genius and stupidity is; genius has its limits.

Albert Einstein


Re: [PATCH][AArch64] Only update assembler .arch directive when necessary

2016-02-11 Thread Kyrill Tkachov


On 10/02/16 10:39, James Greenhalgh wrote:

On Wed, Feb 10, 2016 at 10:32:16AM +, Kyrill Tkachov wrote:

Hi James,

On 10/02/16 10:11, James Greenhalgh wrote:

On Thu, Feb 04, 2016 at 01:50:31PM +, Kyrill Tkachov wrote:

Hi all,

As part of the target attributes and pragmas support for GCC 6 I changed the
aarch64 port to emit a .arch assembly directive for each function that
describes the architectural features used by that function.  This is a change

>from GCC 5 behaviour where we output a single .arch directive at the

beginning of the assembly file corresponding to architectural features given
on the command line.



Bootstrapped and tested on aarch64-none-linux-gnu.  With this patch I managed
to build a recent allyesconfig Linux kernel where before the build would fail
when assembling the LSE instructions.

Ok for trunk?

One comment, that I'm willing to be convinced on...


Thanks,
Kyrill

2016-02-04  Kyrylo Tkachov  

 * config/aarch64/aarch64.c (struct aarch64_output_asm_info):
 New struct definition.
 (aarch64_previous_asm_output): New variable.
 (aarch64_declare_function_name): Only output .arch assembler
 directive if it will be different from the previously output
 directive.
 (aarch64_start_file): New function.
 (TARGET_ASM_FILE_START): Define.

2016-02-04  Kyrylo Tkachov  

 * gcc.target/aarch64/assembler_arch_1.c: Add -dA to dg-options.
 Delete unneeded -save-temps.
 * gcc.target/aarch64/assembler_arch_7.c: Likewise.
 * gcc.target/aarch64/target_attr_15.c: Scan assembly for
 .arch armv8-a\n.
 * gcc.target/aarch64/assembler_arch_1.c: New test.
commit 2df0f24332e316b8d18d4571438f76726a0326e7
Author: Kyrylo Tkachov 
Date:   Wed Jan 27 12:54:54 2016 +

 [AArch64] Only update assembler .arch directive when necessary

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 5ca2ae8..0751440 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -11163,6 +11163,17 @@ aarch64_asm_preferred_eh_data_format (int code 
ATTRIBUTE_UNUSED, int global)
 return (global ? DW_EH_PE_indirect : 0) | DW_EH_PE_pcrel | type;
  }
+struct aarch64_output_asm_info
+{
+  const struct processor *arch;
+  const struct processor *cpu;
+  unsigned long isa_flags;

Why not just keep the last string you printed, and use a string compare
to decide whether to print or not? Sure we'll end up doing a bit more
work, but the logic becomes simpler to follow and we don't need to pass
around another struct...

I did do it this way to avoid a string comparison (I try to avoid
manual string manipulations where I can as they're so easy to get wrong)
though this isn't on any hot path.
We don't really pass the structure around anywhere, we just keep one
instance. We'd have to do the same with a string i.e. keep a string
object around that we'd strcpy (or C++ equivalent) a string to every time
we wanted to update it, so I thought this approach is cleaner as the
architecture features are already fully described by a pointer to
an element in the static constant all_architectures table and an
unsigned long holding the ISA flags.

If you insist I can change it to a string, but I personally don't
think it's worth it.

Had you been working on a C string I probably wouldn't have noticed. But
you're already working with C++ strings in this function, so much of what
you are concerned about is straightforward.

I'd encourage you to try it using idiomatic string manipulation in C++, the
cleanup should be worth it.


Ok, here it is using std::string.
It is a shorter patch.

Bootstrapped and tested on aarch64.

Ok?

Thanks,
Kyrill

2016-02-10  Kyrylo Tkachov  

* config/aarch64/aarch64.c (aarch64_last_printed_arch_string):
New variable.
(aarch64_last_printed_tune_string): Likewise.
(aarch64_declare_function_name): Only output .arch assembler
directive if it will be different from the previously output
directive.  Same for .tune comment but only if -dA is set.
(aarch64_start_file): New function.
(TARGET_ASM_FILE_START): Define.

2016-02-10  Kyrylo Tkachov  

* gcc.target/aarch64/target_attr_15.c: Scan assembly for
.arch armv8-a\n.  Add -dA to dg-options.
* gcc.target/aarch64/assembler_arch_1.c: New test.
* gcc.target/aarch64/target_attr_7.c: Add -dA to dg-options.


Thanks,
James



diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 59cbf81474ccdf34e59a4719ee88251bc658ef04..9055229cb31d1b6edad64efb96856610c1277161 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -11198,6 +11198,10 @@ aarch64_asm_preferred_eh_data_format (int code ATTRIBUTE_UNUSED, int global)
return (global ? DW_EH_PE_indirect : 0) | DW_EH_PE_pcrel | type;
 }
 
+/* The last .arch and .tune assembly strings that we printed.  */
+static std::string aarch64_last_printed_arch_string;
+static std::string aarch64_last_printed_tune_string;
+
 /* Implement ASM_DEC

Re: [PATCH][AArch64] Only update assembler .arch directive when necessary

2016-02-11 Thread James Greenhalgh
On Thu, Feb 11, 2016 at 01:10:33PM +, Kyrill Tkachov wrote:
> >>>Why not just keep the last string you printed, and use a string compare
> >>>to decide whether to print or not? Sure we'll end up doing a bit more
> >>>work, but the logic becomes simpler to follow and we don't need to pass
> >>>around another struct...
> >>I did do it this way to avoid a string comparison (I try to avoid
> >>manual string manipulations where I can as they're so easy to get wrong)
> >>though this isn't on any hot path.
> >>We don't really pass the structure around anywhere, we just keep one
> >>instance. We'd have to do the same with a string i.e. keep a string
> >>object around that we'd strcpy (or C++ equivalent) a string to every time
> >>we wanted to update it, so I thought this approach is cleaner as the
> >>architecture features are already fully described by a pointer to
> >>an element in the static constant all_architectures table and an
> >>unsigned long holding the ISA flags.
> >>
> >>If you insist I can change it to a string, but I personally don't
> >>think it's worth it.
> >Had you been working on a C string I probably wouldn't have noticed. But
> >you're already working with C++ strings in this function, so much of what
> >you are concerned about is straightforward.
> >
> >I'd encourage you to try it using idiomatic string manipulation in C++, the
> >cleanup should be worth it.
> 
> Ok, here it is using std::string.
> It is a shorter patch.

Looks good to me, OK for trunk.

Thanks,
James

> 2016-02-10  Kyrylo Tkachov  
> 
> * config/aarch64/aarch64.c (aarch64_last_printed_arch_string):
> New variable.
> (aarch64_last_printed_tune_string): Likewise.
> (aarch64_declare_function_name): Only output .arch assembler
> directive if it will be different from the previously output
> directive.  Same for .tune comment but only if -dA is set.
> (aarch64_start_file): New function.
> (TARGET_ASM_FILE_START): Define.
> 
> 2016-02-10  Kyrylo Tkachov  
> 
> * gcc.target/aarch64/target_attr_15.c: Scan assembly for
> .arch armv8-a\n.  Add -dA to dg-options.
> * gcc.target/aarch64/assembler_arch_1.c: New test.
> * gcc.target/aarch64/target_attr_7.c: Add -dA to dg-options.
> 



[RS6000] reload_vsx_from_gprsf splitter

2016-02-11 Thread Alan Modra
This is PR68973 part 2, the failure of a boost test, caused by a
splitter emitting an invalid move in reload_vsx_from_gprsf:
  emit_move_insn (op0_di, op2);

op0 can be any vsx reg, but the mtvsrd destination constraint in
movdi_internal64 is "wj", which only allows fp regs.  I'm not sure why
we have that constraint so left movdi_internal64 alone and used a
special p8_mtvsrd instead, similar to p8_mtvsrd_1 and p8_mtvsrd_2 used
by reload_vsx_from_gpr.  When looking at those, I noticed we're
restricted to fprs there too so fixed that as well.  (We can't use %L
in asm operands that must accept vsx.)

Bootstrapped and regression tested powerpc64le-linux.  powerpc64-linux
biarch -mcpu=power7 bootstrap still in progress.  OK to apply assuming
no regressions found?

PR target/68973
* config/rs6000/vsx.md (vsx_xscvspdpn_directmove): Delete.
* config/rs6000/rs6000.md (reload_vsx_from_gprsf): Rewrite splitter.
(p8_mtvsrd): New.
(p8_mtvsrd_1, p8_mtvsrd_2): Delete.
(reload_vsx_from_gpr): Adjust to use p8_mtvsrd.

diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index cdbf873..745293b 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -7543,29 +7543,22 @@
(set_attr "type" "three")])
 
 ;; Move 128 bit values from GPRs to VSX registers in 64-bit mode
-(define_insn "p8_mtvsrd_1"
-  [(set (match_operand:TF 0 "register_operand" "=ws")
-   (unspec:TF [(match_operand:DI 1 "register_operand" "r")]
+(define_insn "p8_mtvsrd"
+  [(set (match_operand:DF 0 "register_operand" "=ws")
+   (unspec:DF [(match_operand:DI 1 "register_operand" "r")]
   UNSPEC_P8V_MTVSRD))]
   "TARGET_POWERPC64 && TARGET_DIRECT_MOVE"
-  "mtvsrd %0,%1"
-  [(set_attr "type" "mftgpr")])
-
-(define_insn "p8_mtvsrd_2"
-  [(set (match_operand:TF 0 "register_operand" "+ws")
-   (unspec:TF [(match_dup 0)
-   (match_operand:DI 1 "register_operand" "r")]
-  UNSPEC_P8V_MTVSRD))]
-  "TARGET_POWERPC64 && TARGET_DIRECT_MOVE"
-  "mtvsrd %L0,%1"
+  "mtvsrd %x0,%1"
   [(set_attr "type" "mftgpr")])
 
 (define_insn "p8_xxpermdi_"
   [(set (match_operand:FMOVE128_GPR 0 "register_operand" "=wa")
-   (unspec:FMOVE128_GPR [(match_operand:TF 1 "register_operand" "ws")]
-UNSPEC_P8V_XXPERMDI))]
+   (unspec:FMOVE128_GPR [
+   (match_operand:DF 1 "register_operand" "ws")
+   (match_operand:DF 2 "register_operand" "ws")]
+   UNSPEC_P8V_XXPERMDI))]
   "TARGET_POWERPC64 && TARGET_DIRECT_MOVE"
-  "xxpermdi %x0,%1,%L1,0"
+  "xxpermdi %x0,%x1,%x2,0"
   [(set_attr "type" "vecperm")])
 
 (define_insn_and_split "reload_vsx_from_gpr"
@@ -7581,13 +7574,18 @@
 {
   rtx dest = operands[0];
   rtx src = operands[1];
-  rtx tmp = operands[2];
+  /* You might think that we could use op0 as one temp and a DF clobber
+ as the other, but you'd be wrong.  These secondary_reload patterns
+ don't check that the clobber is different to the destination, which
+ is probably a reload bug.  */
+  rtx tmp_hi = gen_rtx_REG (DFmode, REGNO (operands[2]));
+  rtx tmp_lo = gen_rtx_REG (DFmode, REGNO (operands[2]) + 1);
   rtx gpr_hi_reg = gen_highpart (DImode, src);
   rtx gpr_lo_reg = gen_lowpart (DImode, src);
 
-  emit_insn (gen_p8_mtvsrd_1 (tmp, gpr_hi_reg));
-  emit_insn (gen_p8_mtvsrd_2 (tmp, gpr_lo_reg));
-  emit_insn (gen_p8_xxpermdi_ (dest, tmp));
+  emit_insn (gen_p8_mtvsrd (tmp_hi, gpr_hi_reg));
+  emit_insn (gen_p8_mtvsrd (tmp_lo, gpr_lo_reg));
+  emit_insn (gen_p8_xxpermdi_ (dest, tmp_hi, tmp_lo));
   DONE;
 }
   [(set_attr "length" "12")
@@ -7622,16 +7620,18 @@
   rtx op0 = operands[0];
   rtx op1 = operands[1];
   rtx op2 = operands[2];
+
   /* Also use the destination register to hold the unconverted DImode value.
  This is conceptually a separate value from OP0, so we use gen_rtx_REG
  rather than simplify_gen_subreg.  */
-  rtx op0_di = gen_rtx_REG (DImode, REGNO (op0));
+  rtx op0_df = gen_rtx_REG (DFmode, REGNO (op0));
+  rtx op0_v4sf = gen_rtx_REG (V4SFmode, REGNO (op0));
   rtx op1_di = simplify_gen_subreg (DImode, op1, SFmode, 0);
 
   /* Move SF value to upper 32-bits for xscvspdpn.  */
   emit_insn (gen_ashldi3 (op2, op1_di, GEN_INT (32)));
-  emit_move_insn (op0_di, op2);
-  emit_insn (gen_vsx_xscvspdpn_directmove (op0, op0_di));
+  emit_insn (gen_p8_mtvsrd (op0_df, op2));
+  emit_insn (gen_vsx_xscvspdpn (op0_df, op0_v4sf));
   DONE;
 }
   [(set_attr "length" "8")
diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
index 997ff31..2d2f137 100644
--- a/gcc/config/rs6000/vsx.md
+++ b/gcc/config/rs6000/vsx.md
@@ -1518,15 +1518,6 @@
   "xscvdpspn %x0,%x1"
   [(set_attr "type" "fp")])
 
-;; Used by direct move to move a SFmode value from GPR to VSX register
-(define_insn "vsx_xscvspdpn_directmove"
-  [(set (match_operand:SF 0 "vsx_register_operand" "=wa")
-   (unspec:SF [(match_operand:DI 1 "vsx_register_operand" "wa")]
-  

[wwwdocs] Add a note about in-class initialization of static data member

2016-02-11 Thread Marek Polacek
Does this look ok?

Index: porting_to.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-6/porting_to.html,v
retrieving revision 1.9
diff -u -r1.9 porting_to.html
--- porting_to.html 10 Feb 2016 17:21:54 -  1.9
+++ porting_to.html 11 Feb 2016 14:18:59 -
@@ -269,6 +269,25 @@
 to port the code to use C++11's std::unique_ptr instead.
 
 
+'constexpr' needed for in-class initialization of static data member
+
+
+Since C++11, the constexpr keyword is needed when initializing
+a non-integral static data member in a class.  Thus the following program is
+accepted in C++03 (albeit with a -Wpedantic warning):
+
+
+
+struct X {
+  const static double i = 10;
+};
+
+
+
+While in C++11, the program above is rejected with an error.  The fix is to
+use constexpr instead of const.
+
+
 -Wmisleading-indentation
 
 A new warning -Wmisleading-indentation was added

Marek


Re: [PATCH, reload] PRE_INC with invalid hard reg

2016-02-11 Thread Bernd Schmidt

On 02/11/2016 10:45 AM, Alan Modra wrote:


Due to uses elsewhere in vsx instructions, reload chooses to put
psuedo 185 in fr31, which can't be used as a base register in the
following:


What code exactly makes the choice of fr31? I assume this is in 
reg_renumber, so it's IRA and not reload that's making that decision?




PR target/68973
* reloads.c (find_reloads_address_1): For pre/post-inc/dec
with an invalid hard reg, reload just the reg not the entire
pre/post-inc/dec address expression.


Hmm, you're patching tricky code. I'm not sure yet whether this is right 
or not. More reload dumps might help if you have them; I'll Cc myself on 
the PR.


My gut feeling is that we want to reload the inner reg before entering 
this block of code, with a new test for SECONDARY_MEMORY_NEEDED 
alongside the existing block that already sets reloaded_inner_of_autoinc.



Bernd


[SH][committed] Adjust some test cases

2016-02-11 Thread Oleg Endo
Hi,

Some SH specific test cases have started showing failures recently. 
 This one was easy.  Committed as r233346.

Cheers,
Oleg

gcc/testsuite/ChangeLog:

* gcc.target/sh/pr54089-8.c: Adjust optimization level.
Index: gcc/testsuite/gcc.target/sh/pr54089-8.c
===
--- gcc/testsuite/gcc.target/sh/pr54089-8.c	(revision 233320)
+++ gcc/testsuite/gcc.target/sh/pr54089-8.c	(working copy)
@@ -1,6 +1,6 @@
 /* Check that the rotcl instruction is generated.  */
 /* { dg-do compile }  */
-/* { dg-options "-O1" } */
+/* { dg-options "-O2" } */
 /* { dg-skip-if "" { "sh*-*-*" } { "-m5*"} { "" } }  */
 /* { dg-final { scan-assembler-times "rotcl" 28 } } */
 


[PATCH] [wwwdocs] Add a "Plugin issues" section to the GCC 6 porting guide

2016-02-11 Thread David Malcolm
I've (mostly) ported gcc-python-plugin to gcc 6.  The attached patch
for the gcc website starts a new "Plugin issues" section, and covers
the biggest issue I ran into (FWIW the suggested compatibility typedef
is the one I committed to gcc-python-plugin).

Validates.

OK to commit?Index: htdocs/gcc-6/porting_to.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-6/porting_to.html,v
retrieving revision 1.10
diff -u -p -r1.10 porting_to.html
--- htdocs/gcc-6/porting_to.html	11 Feb 2016 10:37:00 -	1.10
+++ htdocs/gcc-6/porting_to.html	11 Feb 2016 15:02:29 -
@@ -350,6 +350,63 @@ foo (void *p)
 }
 
 
+Plugin issues
+
+
+The internals of GCC have seen various improvements, and these may affect
+plugins.  Some notes on porting GCC plugins to GCC 6 follow.
+
+
+"gimple" became a struct, rather than a pointer
+
+
+Prior to GCC 6, "gimple" meant a pointer to a statement.  It was a
+typedef aliasing the type struct gimple_statement_base *:
+
+
+
+/* Excerpt from GCC 5's coretypes.h.  */
+typedef struct gimple_statement_base *gimple;
+typedef const struct gimple_statement_base *const_gimple;
+typedef gimple gimple_seq;
+
+
+
+As of GCC 6, the code above became:
+
+
+
+/* Excerpt from GCC 6's coretypes.h.  */
+struct gimple;
+typedef gimple *gimple_seq;
+
+
+"gimple" is now the statement struct itself, not a pointer.
+The "gimple" struct is now the base class of the gimple statement class
+hierarchy, and throughout gcc every gimple was changed to a
+gimple *
+(revision
+https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=42acab1cd6812e2d9e49f4132176f5505f49a0e5";>r227941
+was the commit in question).  The typedef const_gimple is no more;
+use const gimple * if you need to represent a pointer
+to a unmodifiable gimple statement.
+
+
+
+Plugins that work with gimple will need to be updated to reflect this
+change.  If you aim for compatibility between both gcc 6 and earlier
+releases of gcc, it may be cleanest to introduce a compatibility typedef
+in your plugin, such as:
+
+
+
+#if (GCC_VERSION >= 6000)
+typedef gimple *gimple_stmt_ptr;
+#else
+typedef gimple gimple_stmt_ptr;
+#end
+
+
 Links
 
 


[PATCH v4] PR48344: Fix unrecognizable insn error with -fstack-limit-register=r2

2016-02-11 Thread Kelvin Nilsen
This patch has bootstrapped and tested on powerpc64le-unknown-linux-gnu 
with no regressions.  Is this ok for the trunk?


See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48344 for the original 
problem report.  The error resulted because gcc's processing of 
command-line options within gcc initialization code originally preceded 
the processing of target-specific configuration hooks.


In the unpatched gcc implementation, the Pmode (pointer mode) variable 
has not been initialized at the time the -fstack-limit-register 
command-line option is processed.  As a consequence, the stack-limiting 
register is not assigned a proper mode.  Thus, rtl instructions that 
make use of this stack-limiting register have an unspecified mode, and 
are therefore not matched by any known instructions.


The fix represented in this patch is to defer the command-line 
processing related to command-line specification of a stack-limiting 
register until after target-specific initialization has been completed.


Some questions and issues raised in response to version 2 of this patch 
are addressed below:


1. Concerns regarding possible unintended consequences that might result 
from moving all target-specific initialization to precede the invocation 
of the handle_common_deferred_options () function are addressed by 
preserving the original initialization order and moving the relevant 
command-line processing to follow the target-specific initialization.


2. A question was raised as to whether Pmode can change with attribute 
target.  It cannot.


An issue raised in response to version 3 of this patch is addressed as 
follows:


1.  Abbreviated comments.

2. Fixed spacing following periods within comments.

3. Moved stack_limit_rtx initialization code into init_emit_once(void) 
and eliminated special initialization function that had been proposed in 
the V3 patch.



gcc/testsuite/ChangeLog:

2016-02-11  Kelvin Nilsen  

* gcc.target/powerpc/pr48344-1.c: New test.

gcc/ChangeLog:

2016-02-11  Kelvin Nilsen  

* opts-global.c (handle_common_deferred_options): Introduce and
initialize two global variables to remember command-line options
specifying a stack-limiting register.
* opts.h: Add extern declarations of the two new global variables. 
* emit-rtl.c (init_emit_once): Initialize the stack_limit_rtx
variable based on the values of the two new global variables.


Index: gcc/testsuite/gcc.target/powerpc/pr48344-1.c
===
--- gcc/testsuite/gcc.target/powerpc/pr48344-1.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr48344-1.c	(revision 232633)
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-fstack-limit-register=r2" } */
+void foo ()
+{
+  int N = 2;
+  int slots[N];
+
+}
+
Index: gcc/emit-rtl.c
===
--- gcc/emit-rtl.c	(revision 232135)
+++ gcc/emit-rtl.c	(working copy)
@@ -57,6 +57,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "builtins.h"
 #include "rtl-iter.h"
 #include "stor-layout.h"
+#include "opts.h"
 
 struct target_rtl default_target_rtl;
 #if SWITCHABLE_TARGET
@@ -5895,6 +5896,13 @@ init_emit_once (void)
 
   /* Create the unique rtx's for certain rtx codes and operand values.  */
 
+  /* Process stack-limiting command-line options.  */
+  if (opt_fstack_limit_symbol_arg != NULL)
+stack_limit_rtx 
+  = gen_rtx_SYMBOL_REF (Pmode, ggc_strdup (opt_fstack_limit_symbol_arg));
+  if (opt_fstack_limit_register_no >= 0)
+stack_limit_rtx = gen_rtx_REG (Pmode, opt_fstack_limit_register_no);
+
   /* Don't use gen_rtx_CONST_INT here since gen_rtx_CONST_INT in this case
  tries to use these variables.  */
   for (i = - MAX_SAVED_CONST_INT; i <= MAX_SAVED_CONST_INT; i++)
Index: gcc/opts.h
===
--- gcc/opts.h	(revision 232135)
+++ gcc/opts.h	(working copy)
@@ -296,6 +296,10 @@ struct cl_option_handlers
   struct cl_option_handler_func handlers[3];
 };
 
+/* Hold command-line options associated with stack limitation.  */
+extern const char *opt_fstack_limit_symbol_arg;
+extern int opt_fstack_limit_register_no;
+
 /* Input file names.  */
 
 extern const char **in_fnames;
Index: gcc/opts-global.c
===
--- gcc/opts-global.c	(revision 232135)
+++ gcc/opts-global.c	(working copy)
@@ -310,6 +310,10 @@ decode_options (struct gcc_options *opts, struct g
   finish_options (opts, opts_set, loc);
 }
 
+/* Hold command-line options associated with stack limitation.  */
+const char *opt_fstack_limit_symbol_arg = NULL;
+int opt_fstack_limit_register_no = -1;
+
 /* Process common options that have been deferred until after the
handlers have been called for all options.  */
 
@@ -417,12 +421,18 @@ handle_common_deferred_options (void)
 	if (reg < 0)
 	  error ("unrecognized register 

Re: [PATCH] Fix PR69291, RTL if-conversion bug

2016-02-11 Thread Bernd Schmidt

On 02/10/2016 03:03 PM, Richard Biener wrote:


Ok, the following is in testing now.

Ok?

Thanks,
Richard.

2016-02-10  Richard Biener  

PR rtl-optimization/69291
* ifcvt.c (noce_try_store_flag_constants): Do not allow
subexpressions affected by changing the result.


Ok if it passed.


Bernd



Re: [wwwdocs] Add a note about in-class initialization of static data member

2016-02-11 Thread Jonathan Wakely

On 11/02/16 15:20 +0100, Marek Polacek wrote:

Does this look ok?


Looks OK, although how about stressing that it was only allowed as an
extension previously, e.g. ...



Index: porting_to.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-6/porting_to.html,v
retrieving revision 1.9
diff -u -r1.9 porting_to.html
--- porting_to.html 10 Feb 2016 17:21:54 -  1.9
+++ porting_to.html 11 Feb 2016 14:18:59 -
@@ -269,6 +269,25 @@
to port the code to use C++11's std::unique_ptr instead.


+'constexpr' needed for in-class initialization of static data member
+


In C++98 in-class initialization of static data members is only
allowed for integral types, but as an extension G++ also allowed it
for floating point types:


+
+
+struct X {
+  const static double i = 10;
+};
+


The C++11 standard supports that in-class initialization using
constexpr instead, so the GNU extension is no longer
supported for C++11 or later. Programs relying on the extension will
be rejected with an error. The fix is to
use constexpr instead of const.



Re: AW: Wonly-top-basic-asm

2016-02-11 Thread Bernd Schmidt

On 02/11/2016 12:49 AM, David Wohlferd wrote:

I believe the attached patch addresses all the other outstanding comments.


Bernd Edlinger made some thorough comments; I'll just add a few more. I 
don't think this is a patch we're considering for gcc-6, at least not 
for the initial release - I imagine it could be backported from gcc-7 at 
some point.


Like the other Bernd I have a preference for just -Wbasic-asm. I'd make 
the analogy with -Wparentheses - we don't warn about every parenthesis, 
but the name of an option is not the place for detailed explanations of 
how it works. Less typing and less to remember is preferrable IMO.



+ /* Warn on basic asm used inside of functions,
+EXCEPT when in naked functions.  Also allow asm (""). */


No all-caps.


  @subsubheading Remarks
-Using extended @code{asm} typically produces smaller, safer, and more
+Using extended @code{asm} (@pxref{Extended Asm}) typically produces smaller,
+safer, and more
  efficient code, and in most cases it is a better solution than basic


Rewrap the paragraph?


-Here is an example of basic @code{asm} for i386:
+Basic @code{asm} statements do not perform an implicit "memory" clobber
+(@pxref{Clobbers}).  Also, there is no implicit clobbering of @emph{any}
+registers, so (other than in @code{naked} functions which follow the ABI
+rules) changed registers must be restored to their original value before
+exiting the @code{asm}.  While this behavior has not always been
+documented, GCC has worked this way since at least v2.95.3.

+@strong{Warning:} This "clobber nothing" behavior may be different than how
+other compilers treat basic @code{asm}, since the C standards for the
+@code{asm} statement provide no guidance regarding these semantics.  As a
+result, @code{asm} statements that work correctly on other compilers may not
+work correctly with GCC (and vice versa), even though they both compile
+without error.
+
+Future versions of GCC may change basic @code{asm} to clobber memory and
+perhaps some (or all) registers.  This change may fix subtle problems with
+existing @code{asm} statements.  However it may break or slow down ones
+that were working correctly.  To ``future-proof'' your asm against possible
+changes to basic @code{asm}'s semantics, use extended @code{asm}.
+
+You can use @option{-Wbasic-asm-in-function} to locate basic @code{asm}
+statements that may need changes, and refer to
+@uref{https://gcc.gnu.org/wiki/ConvertBasicAsmToExtended, How to convert
+from basic asm to extended asm} for information about how to perform the
+conversion.


I still think this is too verbose and would serve to confuse rather than 
enlighten in practice. (If anyone feels otherwise, feel free to overrule 
me.) I'm also no longer sure about references to the wiki.


Let's look at this existing paragraph from the manual:

  Since GCC does not parse the @var{AssemblerInstructions}, it has
  no visibility of any symbols it references. This may result in GCC
  discarding those symbols as unreferenced.

I think extending this would be better. Something like

"Since the C standards does not specify semantics for @code{asm}, it is 
a potential source of incompatibilities between compilers.  GCC does not 
parse the @var{AssemblerInstructions}, which means there is no way to 
communicate to the compiler what is happening inside them.  GCC has no 
visibility of any symbols referenced in the @code{asm} and may discard 
them as unreferenced. It also does not know about side effects that may 
occur, such as modifications of memory locations or registers. GCC 
assumes that no such side effects occur, which may not be what the user 
expected if code was written for other compilers.


Since basic @code{asm} cannot easily be used in a reliable way, 
@option{-Wbasic-asm} should be used to warn about the use of basic asm 
inside a function. See 
@uref{https://gcc.gnu.org/wiki/ConvertBasicAsmToExtended, How to convert 
from basic asm to extended asm} for information about how to convert 
code to use extended @code{asm}."


But again, if someone feels the docs patch as posted is preferrable, go 
ahead and approve it (for stage1 I assume).



Bernd


Re: [PATCH] PR driver/69265: improved suggestions for various misspelled options

2016-02-11 Thread David Malcolm
On Thu, 2016-02-11 at 10:16 +0100, Richard Biener wrote:
> On Wed, Feb 10, 2016 at 5:25 PM, Bernd Schmidt 
> wrote:
> > On 02/09/2016 09:44 PM, David Malcolm wrote:
> > > 
> > > This is a bug in a new feature, so it isn't a regression as such,
> > > but
> > > it's fairly visible, and I believe the fix is relatively low-risk
> > > (error-handling of typos of command-line options).
> > > 
> > > This also now covers PR driver/69453 (and its duplicate PR
> > > driver/69642), so people *are* running into this.
> > 
> > 
> > I think the patch looks reasonable (I expect it needs slight
> > adjustment
> > after an earlier sanitizer options change). Whether it's OK or not
> > at this
> > stage is something I think I'll want to ask a RM. My inclination
> > would be
> > yes.
> 
> Yes.

Thanks.  Were you approving the idea of fixing this bug in stage 4, or
approving the patch itself?   Note that the patch needed some changes
to apply against trunk; the latest version is at:
https://gcc.gnu.org/ml/gcc-patches/2016-02/msg00724.html

> Richard.
> 
> > A small improvement might be calculating the candidates array only
> > once when
> > making the first suggestion and not freeing it. BTW, I've also run
> > into a
> > case of an unhelpful suggestion:
> > 
> > ./cc1 ~/hw.c -fno-if-convert
> > cc1: error: unrecognized command line option ‘-fno-if-convert’
> > 
> > which should instead suggest fno-if-conversion.
> > 
> > 
> > Bernd


Re: [PATCH v4] PR48344: Fix unrecognizable insn error with -fstack-limit-register=r2

2016-02-11 Thread Bernd Schmidt

On 02/11/2016 04:12 PM, Kelvin Nilsen wrote:


* opts-global.c (handle_common_deferred_options): Introduce and
 initialize two global variables to remember command-line options
 specifying a stack-limiting register.
* opts.h: Add extern declarations of the two new global variables.
* emit-rtl.c (init_emit_once): Initialize the stack_limit_rtx
 variable based on the values of the two new global variables.


Make sure that when committed these all start with a tab character.


+void foo ()
+{
+  int N = 2;
+  int slots[N];
+
+}
+


Watch extra blank lines at the end of files.


@@ -442,3 +452,4 @@ handle_common_deferred_options (void)
}
  }
  }
+



Here too.

Ok with these fixed.


Bernd


Re: Un-parallelized OpenACC kernels constructs with nvptx offloading: "avoid offloading"

2016-02-11 Thread Bernd Schmidt

On 02/11/2016 11:01 AM, Thomas Schwinge wrote:


The "avoid offloading" mechanism.  Owed to the non-shared-memory
offloading architecture, if the compiler/runtime decides to "avoid
offloading", then this has to apply to *all* code offloading, for data
consistency reasons.  Do we agree on that?


Not necessarily, I think. It should be possible to determine whether 
some offloaded code blocks are independent from each other. (That 
doesn't mean we currently have any good way of making such decisions. 
libgomp or even the ptx compiler are probably too late and don't have 
the necessary information anymore).



Bernd


Re: AW: Wonly-top-basic-asm

2016-02-11 Thread Sandra Loosemore

On 02/11/2016 08:40 AM, Bernd Schmidt wrote:


But again, if someone feels the docs patch as posted is preferrable, go
ahead and approve it (for stage1 I assume).


TBH, I haven't looked at the documentation patch at all; I've been 
ignoring this issue because (a) I thought the technical details were 
still under discussion and (b) adding a new command-line option is stage 
1 material not suitable to commit right now anyway.


I'll take a look at the docs once the name and behavior of the new 
option have been settled upon.


-Sandra



[PATCH] Fix ipa-split handling of clobbers and debug stmts in return_bb (PR ipa/68672)

2016-02-11 Thread Jakub Jelinek
Hi!

While the cgraph versioning code is able to deal with clobber stmts that
refer to SSA_NAMEs set by basic blocks not split into the versioned
function, and similarly with debug stmts (clobbers refererring to those
are removed and debug stmts reset), on the main part that doesn't happen,
because ipa-split makes the split blocks unreachable by jumping around them
and thus if SSA_NAMEs set in the split basic blocks are referenced
in return_bb in statements we intentionally ignored for the analysis (debug
stmts and clobber stmts), we either end up with weird stuff (set debug stmts
to debug temporary that is set in the basic blocks that are removed as
dead), or for clobbers ICE.

Richard has tried to deal with this by throwing away the return_bb in
certain cases in the main part (forcing recreation of it), but that can
throw valuable statements that we could have kept (debug stmts or e.g. decl
clobbers or clobbers that don't need SSA_NAMEs from the split blocks),
and doesn't work e.g. on the 1st and 3rd testcase below anyway.

So, this patch reverts the main_part_return_p and instead does
what is needed for those ignored stmts in return_bb:
1) SSA_NAME references to retval in both debug stmts and clobber stmts
   are replaced with the lhs of the call to *.part.*
2) debug stmts referencing other SSA_NAMEs set in the split bbs are reset
3) clobber stmts referencing other SSA_NAMEs set in the split bbs are removed

The testcase hopefully cover all the interesting cases (return_bb copied
from main part (where it is kept) to split part too, and with debug stmts
and clobbers that refer to SSA_NAMEs set in main or split part or retval).

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

2016-02-11  Jakub Jelinek  

PR ipa/68672
* ipa-split.c (split_function): Don't compute/use main_part_return_p.
Compute retval and retbnd early in all cases if split_part_return_p
and return_bb is not EXIT.  Remove all clobber stmts and reset
all debug stmts that refer to SSA_NAMEs defined in split part,
except if it is retval, in that case replace the old retval with the
lhs of the call to the split part.

* g++.dg/ipa/pr68672-1.C: New test.
* g++.dg/ipa/pr68672-2.C: New test.
* g++.dg/ipa/pr68672-3.C: New test.

--- gcc/ipa-split.c.jj  2016-02-11 10:50:52.888220581 +0100
+++ gcc/ipa-split.c 2016-02-11 12:46:15.975777652 +0100
@@ -1244,28 +1244,13 @@ split_function (basic_block return_bb, s
args_to_pass.safe_push (arg);
   }
 
-  /* See if the split function or the main part will return.  */
-  bool main_part_return_p = false;
+  /* See if the split function will return.  */
   bool split_part_return_p = false;
   FOR_EACH_EDGE (e, ei, return_bb->preds)
 {
   if (bitmap_bit_p (split_point->split_bbs, e->src->index))
split_part_return_p = true;
-  else
-   main_part_return_p = true;
 }
-  /* The main part also returns if we split on a fallthru edge
- and the split part returns.  */
-  if (split_part_return_p)
-FOR_EACH_EDGE (e, ei, split_point->entry_bb->preds)
-  {
-   if (! bitmap_bit_p (split_point->split_bbs, e->src->index)
-   && single_succ_p (e->src))
- {
-   main_part_return_p = true;
-   break;
- }
-  }
 
   /* Add return block to what will become the split function.
  We do not return; no return block is needed.  */
@@ -1279,8 +1264,8 @@ split_function (basic_block return_bb, s
  FIXME: Once we are able to change return type, we should change function
  to return void instead of just outputting function with undefined return
  value.  For structures this affects quality of codegen.  */
-  else if (!split_point->split_part_set_retval
-   && (retval = find_retval (return_bb)))
+  else if ((retval = find_retval (return_bb))
+  && !split_point->split_part_set_retval)
 {
   bool redirected = true;
   basic_block new_return_bb = create_basic_block (NULL, 0, return_bb);
@@ -1308,12 +1293,10 @@ split_function (basic_block return_bb, s
 }
   /* When we pass around the value, use existing return block.  */
   else
-bitmap_set_bit (split_point->split_bbs, return_bb->index);
-
-  /* If the main part doesn't return pretend the return block wasn't
- found for all of the following.  */
-  if (! main_part_return_p)
-return_bb = EXIT_BLOCK_PTR_FOR_FN (cfun);
+{
+  bitmap_set_bit (split_point->split_bbs, return_bb->index);
+  retbnd = find_retbnd (return_bb);
+}
 
   /* If RETURN_BB has virtual operand PHIs, they must be removed and the
  virtual operand marked for renaming as we change the CFG in a way that
@@ -1382,6 +1365,44 @@ split_function (basic_block return_bb, s
   DECL_FUNCTION_CODE (node->decl) = (enum built_in_function) 0;
 }
 
+  /* If return_bb contains any clobbers that refer to SSA_NAMEs
+ set in the split part, remove them.  Also re

[PATCH] Fix another ipa-split caused ICE (PR ipa/69241)

2016-02-11 Thread Jakub Jelinek
On Thu, Feb 11, 2016 at 10:22:24AM +0100, Richard Biener wrote:
> The other option is to simply not split the function in this case.

Here is a better fix (but it needs the other patch I've sent, so that
what is return_bb stays the same).

This patch arranges for the case where there is return_bb, but does not
return a value, in function that has return type TREE_ADDRESSABLE,
to just use void return type from the split part (because we would never
set lhs of the call to *.part.* in that case anyway).

Bootstrapped/regtested on x86_64-linux and i686-linux on top of the PR68672
patch, ok for trunk?

2016-02-10  Jakub Jelinek  

PR ipa/69241
* ipa-split.c (split_function): If split part returns TREE_ADDRESSABLE
type by reference, force lhs on the call.

* g++.dg/ipa/pr69241-4.C: New test.

--- gcc/ipa-split.c.jj  2016-02-11 12:46:15.975777652 +0100
+++ gcc/ipa-split.c 2016-02-11 13:06:57.715241871 +0100
@@ -629,7 +629,18 @@ consider_split (struct split_point *curr
4) For non-SSA we need to look where the var is computed. */
   retval = find_retval (return_bb);
   if (!retval)
-current->split_part_set_retval = true;
+{
+  /* If there is a return_bb with no return value in function returning
+value by reference, also make the split part return void, otherwise
+we expansion would try to create a non-POD temporary, which is
+invalid.  */
+  if (return_bb != EXIT_BLOCK_PTR_FOR_FN (cfun)
+ && DECL_RESULT (current_function_decl)
+ && DECL_BY_REFERENCE (DECL_RESULT (current_function_decl)))
+   current->split_part_set_retval = false;
+  else
+   current->split_part_set_retval = true;
+}
   else if (is_gimple_min_invariant (retval))
 current->split_part_set_retval = false;
   /* Special case is value returned by reference we record as if it was non-ssa
--- gcc/testsuite/g++.dg/ipa/pr69241-4.C.jj 2016-02-11 13:00:04.160075417 
+0100
+++ gcc/testsuite/g++.dg/ipa/pr69241-4.C2016-02-11 13:00:04.160075417 
+0100
@@ -0,0 +1,55 @@
+// PR ipa/69241
+// { dg-do compile { target c++11 } }
+// { dg-options "-O2 -Wno-return-type" }
+
+template  class A;
+struct B {
+  using pointer = int *;
+};
+template > class basic_string {
+  long _M_string_length;
+  enum { _S_local_capacity = 15 } _M_local_buf[_S_local_capacity];
+  B::pointer _M_local_data;
+
+public:
+  ~basic_string();
+};
+template 
+int operator<<(_Traits, basic_string<_CharT, _Alloc>);
+class C {
+  basic_string> _M_string;
+};
+class D {
+  C _M_stringbuf;
+};
+class F {
+  int stream;
+  D stream_;
+};
+class G {
+public:
+  void operator&(int);
+};
+class H {
+public:
+  H(unsigned);
+  H(H &&);
+  bool m_fn1();
+};
+class I {
+  void m_fn2(const int &&);
+  static H m_fn3(const int &);
+};
+template  void Bind(Functor);
+class J {
+public:
+  static basic_string m_fn4();
+};
+int a;
+void I::m_fn2(const int &&) { Bind(m_fn3); }
+H I::m_fn3(const int &) {
+  !false ? (void)0 : G() & F() << J::m_fn4();
+  H b(a);
+  if (b.m_fn1())
+F();
+}

Jakub


[C/C++ PATCH] Fix a -Waddress regression (PR c/69768)

2016-02-11 Thread Jakub Jelinek
Hi!

Until recently, integer_zerop would STRIP_NOPS, so that change regressed
some cases in the -Waddress warning that affect some real-world code.
The following patch re-adds stripping of nops for that case (but doesn't try
to fold it further).
With this patch, for C and C++98 we get the same behavior as in 5.x,
for C++11 and C++14 we warn even about the weirdo cases where (10 - 10)
or (&e - &e) is cast to a pointer (but, e.g. for C we have been and are
warning about the latter only).  I'd say warning for such weird cases is
fine.

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

2016-02-11  Jakub Jelinek  

PR c/69768
* c-typeck.c (parser_build_binary_op): Strip nops from integer_zerop
arguments for -Waddress warning.

* typeck.c (cp_build_binary_op): Strip nops from integer_zerop
arguments for -Waddress warning.  Fix up formatting.

* c-c++-common/Waddress-1.c: New test.

--- gcc/c/c-typeck.c.jj 2016-01-27 20:30:18.0 +0100
+++ gcc/c/c-typeck.c2016-02-11 17:17:07.470222811 +0100
@@ -3597,8 +3597,10 @@ parser_build_binary_op (location_t locat
  of testing for equality or inequality of a string literal with NULL.  */
   if (code == EQ_EXPR || code == NE_EXPR)
 {
-  if ((code1 == STRING_CST && !integer_zerop (arg2.value))
- || (code2 == STRING_CST && !integer_zerop (arg1.value)))
+  if ((code1 == STRING_CST
+  && !integer_zerop (tree_strip_nop_conversions (arg2.value)))
+ || (code2 == STRING_CST
+ && !integer_zerop (tree_strip_nop_conversions (arg1.value
warning_at (location, OPT_Waddress,
"comparison with string literal results in unspecified 
behavior");
 }
--- gcc/cp/typeck.c.jj  2016-02-11 10:50:58.0 +0100
+++ gcc/cp/typeck.c 2016-02-11 17:17:07.473222771 +0100
@@ -4487,9 +4487,12 @@ cp_build_binary_op (location_t location,
warning (OPT_Wfloat_equal,
 "comparing floating point with == or != is unsafe");
   if ((complain & tf_warning)
- && ((TREE_CODE (orig_op0) == STRING_CST && !integer_zerop (op1))
- || (TREE_CODE (orig_op1) == STRING_CST && !integer_zerop (op0
-   warning (OPT_Waddress, "comparison with string literal results in 
unspecified behaviour");
+ && ((TREE_CODE (orig_op0) == STRING_CST
+  && !integer_zerop (tree_strip_nop_conversions (op1)))
+ || (TREE_CODE (orig_op1) == STRING_CST
+ && !integer_zerop (tree_strip_nop_conversions (op0)
+   warning (OPT_Waddress, "comparison with string literal results "
+  "in unspecified behaviour");
 
   build_type = boolean_type_node;
   if ((code0 == INTEGER_TYPE || code0 == REAL_TYPE
--- gcc/testsuite/c-c++-common/Waddress-1.c.jj  2016-02-11 14:30:30.677090429 
+0100
+++ gcc/testsuite/c-c++-common/Waddress-1.c 2016-02-11 17:18:13.970329341 
+0100
@@ -0,0 +1,15 @@
+/* PR c/69768 */
+/* { dg-do compile } */
+/* { dg-options "-Waddress" } */
+
+static int e;
+
+int
+foo ()
+{
+  return "foo1" != (void *) 0  /* { dg-bogus "comparison with string literal 
results in unspecified behaviou?r" } */
+&& "foo2" != (const char *) ((void *) 0)   /* { dg-bogus 
"comparison with string literal results in unspecified behaviou?r" } */
+&& "foo3" != (const char *) ((void *) (10 - 10))   /* { dg-warning 
"comparison with string literal results in unspecified behaviou?r" "" { target 
c++11 } } */
+&& "foo4" != (const char *) ((void *) (&e - &e))   /* { dg-warning 
"comparison with string literal results in unspecified behaviou?r" "" { target 
{ c || c++11 } } } */
+&& "foo5" != "foo6";   /* { dg-warning "comparison with string literal 
results in unspecified behaviou?r" } */
+}

Jakub


Re: [wwwdocs] Add a note about in-class initialization of static data member

2016-02-11 Thread Marek Polacek
On Thu, Feb 11, 2016 at 03:26:13PM +, Jonathan Wakely wrote:
> On 11/02/16 15:20 +0100, Marek Polacek wrote:
> >Does this look ok?
> 
> Looks OK, although how about stressing that it was only allowed as an
> extension previously, e.g. ...

So like this?  I've also added a note about stricter flexarr members rules.

Index: porting_to.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-6/porting_to.html,v
retrieving revision 1.9
diff -u -r1.9 porting_to.html
--- porting_to.html 10 Feb 2016 17:21:54 -  1.9
+++ porting_to.html 11 Feb 2016 16:38:38 -
@@ -269,6 +269,41 @@
 to port the code to use C++11's std::unique_ptr instead.
 
 
+'constexpr' needed for in-class initialization of static data member
+
+
+Since C++11, the constexpr keyword is needed when initializing
+a non-integral static data member in a class.  Thus the following program is
+accepted in C++03 (albeit with a -Wpedantic warning):
+
+
+
+struct X {
+  const static double i = 10;
+};
+
+
+
+The C++11 standard supports that in-class initialization using
+constexpr instead, so the GNU extension is no longer supported for
+C++11 or later.  Programs relying on the extension will be rejected with an
+error.  The fix is to use constexpr instead of const.
+
+
+Stricter flexible array member rules
+
+
+As of this release, the C++ compiler is now more strict about flexible array
+member rules.  As a consequence, the following code is no longer accepted:
+
+
+
+union U {
+  int i;
+  char a[];
+};
+
+
 -Wmisleading-indentation
 
 A new warning -Wmisleading-indentation was added

Marek


Re: [PING, patch, Fortran, pr69296, v1] [6 Regression] [F03] Problem with associate and vector subscript

2016-02-11 Thread Andre Vehreschild
Hi Paul, hi all,

thanks for the review. Committed as r233351.

Regards,
Andre

On Thu, 11 Feb 2016 13:36:44 +0100
Paul Richard Thomas  wrote:

> Dear Andre,
> 
> That's very clever! OK for trunk
> 
> Thanks for the patch
> 
> Paul
> 
> On 11 February 2016 at 13:05, Andre Vehreschild  wrote:
> > PING
> >
> > On Tue, 2 Feb 2016 18:37:27 +0100
> > Andre Vehreschild  wrote:
> >  
> >> Hi all,
> >>
> >> the attached patch fixes a regression that was most likely introduced
> >> by one of my former patches, when in an associate() the rank of the
> >> associated variable could not be determined at parse time correctly.
> >> The patch now adds a flag to the association list indicating, that the
> >> rank of the associated variable has been guessed only. In the resolve
> >> phase the rank is corrected when the guess was wrong.
> >>
> >> Bootstrapped and regtested ok on x86_64-linux-gnu/F23.
> >>
> >> Ok for trunk?
> >>
> >> Regards,
> >>   Andre  
> >
> >
> > --
> > Andre Vehreschild * Email: vehre ad gmx dot de  
> 
> 
> 


-- 
Andre Vehreschild * Email: vehre ad gmx dot de 
Index: gcc/fortran/ChangeLog
===
--- gcc/fortran/ChangeLog	(Revision 233350)
+++ gcc/fortran/ChangeLog	(Arbeitskopie)
@@ -1,3 +1,16 @@
+2016-02-11  Andre Vehreschild  
+
+	PR fortran/69296
+	* gfortran.h: Added flag to gfc_association_list indicating that
+	the rank of an associate variable has been guessed only.
+	* parse.c (parse_associate): Set the guess flag mentioned above
+	when guessing the rank of an expression.
+	* resolve.c (resolve_assoc_var): When the rank has been guessed,
+	make sure, that the guess was correct else overwrite with the actual
+	rank.
+	* trans-stmt.c (trans_associate_var): For subref_array_pointers in
+	class objects, take the span from the _data component.
+
 2016-02-07  Jerry DeLisle  
 
 	PR fortran/50555
Index: gcc/fortran/gfortran.h
===
--- gcc/fortran/gfortran.h	(Revision 233350)
+++ gcc/fortran/gfortran.h	(Arbeitskopie)
@@ -2344,6 +2344,9 @@
  for memory handling.  */
   unsigned dangling:1;
 
+  /* True when the rank of the target expression is guessed during parsing.  */
+  unsigned rankguessed:1;
+
   char name[GFC_MAX_SYMBOL_LEN + 1];
   gfc_symtree *st; /* Symtree corresponding to name.  */
   locus where;
Index: gcc/fortran/parse.c
===
--- gcc/fortran/parse.c	(Revision 233350)
+++ gcc/fortran/parse.c	(Arbeitskopie)
@@ -4098,6 +4098,7 @@
 	  int dim, rank = 0;
 	  if (array_ref)
 	{
+	  a->rankguessed = 1;
 	  /* Count the dimension, that have a non-scalar extend.  */
 	  for (dim = 0; dim < array_ref->dimen; ++dim)
 		if (array_ref->dimen_type[dim] != DIMEN_ELEMENT
Index: gcc/fortran/resolve.c
===
--- gcc/fortran/resolve.c	(Revision 233350)
+++ gcc/fortran/resolve.c	(Arbeitskopie)
@@ -4777,7 +4777,7 @@
 /* Given a variable expression node, compute the rank of the expression by
examining the base symbol and any reference structures it may have.  */
 
-static void
+void
 expression_rank (gfc_expr *e)
 {
   gfc_ref *ref;
@@ -8153,9 +8153,13 @@
   if (target->rank != 0)
 {
   gfc_array_spec *as;
-  if (sym->ts.type != BT_CLASS && !sym->as)
+  /* The rank may be incorrectly guessed at parsing, therefore make sure
+	 it is corrected now.  */
+  if (sym->ts.type != BT_CLASS && (!sym->as || sym->assoc->rankguessed))
 	{
-	  as = gfc_get_array_spec ();
+	  if (!sym->as)
+	sym->as = gfc_get_array_spec ();
+	  as = sym->as;
 	  as->rank = target->rank;
 	  as->type = AS_DEFERRED;
 	  as->corank = gfc_get_corank (target);
@@ -8162,7 +8166,6 @@
 	  sym->attr.dimension = 1;
 	  if (as->corank != 0)
 	sym->attr.codimension = 1;
-	  sym->as = as;
 	}
 }
   else
Index: gcc/fortran/trans-stmt.c
===
--- gcc/fortran/trans-stmt.c	(Revision 233350)
+++ gcc/fortran/trans-stmt.c	(Arbeitskopie)
@@ -1569,7 +1569,9 @@
   if (sym->attr.subref_array_pointer)
 	{
 	  gcc_assert (e->expr_type == EXPR_VARIABLE);
-	  tmp = e->symtree->n.sym->backend_decl;
+	  tmp = e->symtree->n.sym->ts.type == BT_CLASS
+	  ? gfc_class_data_get (e->symtree->n.sym->backend_decl)
+	  : e->symtree->n.sym->backend_decl;
 	  tmp = gfc_get_element_type (TREE_TYPE (tmp));
 	  tmp = fold_convert (gfc_array_index_type, size_in_bytes (tmp));
 	  gfc_add_modify (&se.pre, GFC_DECL_SPAN(desc), tmp);
Index: gcc/testsuite/ChangeLog
===
--- gcc/testsuite/ChangeLog	(Revision 233350)
+++ gcc/testsuite/ChangeLog	(Arbeitskopie)
@@ -1,3 +1,9 @@
+2016-02-11  Andre Vehreschild  
+
+	PR fortran/69296
+	* gfortran.dg/associate_19.f03: New test.
+	* gfortran.dg/associate_20.f03: New test.
+
 2016-02-11  Oleg Endo  
 
 	*

Re: [wwwdocs] Add a note about in-class initialization of static data member

2016-02-11 Thread Jonathan Wakely

On 11/02/16 17:39 +0100, Marek Polacek wrote:

On Thu, Feb 11, 2016 at 03:26:13PM +, Jonathan Wakely wrote:

On 11/02/16 15:20 +0100, Marek Polacek wrote:
>Does this look ok?

Looks OK, although how about stressing that it was only allowed as an
extension previously, e.g. ...


So like this?  I've also added a note about stricter flexarr members rules.


I think the first paragraph is a bit confusing. Both sentences are
true, but the first does not follow from the second, so "thus" is not
appropriate. Maybe just change "Thus" to "As a GNU extension".



Index: porting_to.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-6/porting_to.html,v
retrieving revision 1.9
diff -u -r1.9 porting_to.html
--- porting_to.html 10 Feb 2016 17:21:54 -  1.9
+++ porting_to.html 11 Feb 2016 16:38:38 -
@@ -269,6 +269,41 @@
to port the code to use C++11's std::unique_ptr instead.


+'constexpr' needed for in-class initialization of static data member
+
+
+Since C++11, the constexpr keyword is needed when initializing
+a non-integral static data member in a class.  Thus the following program is
+accepted in C++03 (albeit with a -Wpedantic warning):
+
+
+
+struct X {
+  const static double i = 10;
+};
+
+
+
+The C++11 standard supports that in-class initialization using
+constexpr instead, so the GNU extension is no longer supported for
+C++11 or later.  Programs relying on the extension will be rejected with an
+error.  The fix is to use constexpr instead of const.
+
+
+Stricter flexible array member rules
+
+
+As of this release, the C++ compiler is now more strict about flexible array
+member rules.  As a consequence, the following code is no longer accepted:
+
+
+
+union U {
+  int i;
+  char a[];
+};
+
+
-Wmisleading-indentation

A new warning -Wmisleading-indentation was added

Marek


[PATCH] combine: More distribute_notes trouble (PR69737)

2016-02-11 Thread Segher Boessenkool
PR64682 is a problem in distribute_notes, where it has trouble putting
a REG_DEAD note for a reg that is set twice in the right spot.  My fix
for that did the wrong thing for PR69567.  And then my attempted fix
for that one made PR64682 fail again.

Instead, let's just lose the note in such complicated cases, like we
already do in certain similar cases.

Tested on powerpc64-linux and x86_64-linux.  Also built Linux kernels
for some 30 supported targets; no difference in generated code was
observed.

Committing to trunk.

HJ, I tested this on GCC 5 for x86_64-linux, the failure is gone;
could you test it on your setup before I apply it there though?


Segher


2016-02-11  Segher Boessenkool  

PR rtl-optimization/64682
PR rtl-optimization/69567
PR rtl-optimization/69737
* combine.c (distribute_notes) : If the register is set
in I2 as well, just lose it.

---
 gcc/combine.c | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/gcc/combine.c b/gcc/combine.c
index 3609b94..24dcefa 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -13901,7 +13901,6 @@ distribute_notes (rtx notes, rtx_insn *from_insn, 
rtx_insn *i3, rtx_insn *i2,
tem_insn = from_insn;
  else
{
- tem_insn = i3;
  if (from_insn
  && CALL_P (from_insn)
  && find_reg_fusage (from_insn, USE, XEXP (note, 0)))
@@ -13910,14 +13909,7 @@ distribute_notes (rtx notes, rtx_insn *from_insn, 
rtx_insn *i3, rtx_insn *i2,
place = i3;
  else if (i2 != 0 && next_nonnote_nondebug_insn (i2) == i3
   && reg_referenced_p (XEXP (note, 0), PATTERN (i2)))
-   {
- place = i2;
- /* If the new I2 sets the same register that is marked dead
-in the note, the note now should not be put on I2, as the
-note refers to a previous incarnation of the reg.  */
- if (reg_set_p (XEXP (note, 0), PATTERN (i2)))
-   tem_insn = i2;
-   }
+   place = i2;
  else if ((rtx_equal_p (XEXP (note, 0), elim_i2)
&& !(i2mod
 && reg_overlap_mentioned_p (XEXP (note, 0),
@@ -13925,6 +13917,12 @@ distribute_notes (rtx notes, rtx_insn *from_insn, 
rtx_insn *i3, rtx_insn *i2,
   || rtx_equal_p (XEXP (note, 0), elim_i1)
   || rtx_equal_p (XEXP (note, 0), elim_i0))
break;
+ tem_insn = i3;
+ /* If the new I2 sets the same register that is marked dead
+in the note, we do not know where to put the note.
+Give up.  */
+ if (i2 != 0 && reg_set_p (XEXP (note, 0), PATTERN (i2)))
+   break;
}
 
  if (place == 0)
-- 
1.9.3



Re: [wwwdocs] Add a note about in-class initialization of static data member

2016-02-11 Thread Marek Polacek
On Thu, Feb 11, 2016 at 05:01:58PM +, Jonathan Wakely wrote:
> On 11/02/16 17:39 +0100, Marek Polacek wrote:
> >On Thu, Feb 11, 2016 at 03:26:13PM +, Jonathan Wakely wrote:
> >>On 11/02/16 15:20 +0100, Marek Polacek wrote:
> >>>Does this look ok?
> >>
> >>Looks OK, although how about stressing that it was only allowed as an
> >>extension previously, e.g. ...
> >
> >So like this?  I've also added a note about stricter flexarr members rules.
> 
> I think the first paragraph is a bit confusing. Both sentences are
> true, but the first does not follow from the second, so "thus" is not
> appropriate. Maybe just change "Thus" to "As a GNU extension".

Indeed, how could I not notice?  I'll go with "As a GNU extension".

Thanks much.

Marek


Re: [wwwdocs] Add a note about in-class initialization of static data member

2016-02-11 Thread Martin Sebor

On 02/11/2016 09:39 AM, Marek Polacek wrote:

On Thu, Feb 11, 2016 at 03:26:13PM +, Jonathan Wakely wrote:

On 11/02/16 15:20 +0100, Marek Polacek wrote:

Does this look ok?


Looks OK, although how about stressing that it was only allowed as an
extension previously, e.g. ...


So like this?  I've also added a note about stricter flexarr members rules.

Index: porting_to.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-6/porting_to.html,v
retrieving revision 1.9
diff -u -r1.9 porting_to.html
--- porting_to.html 10 Feb 2016 17:21:54 -  1.9
+++ porting_to.html 11 Feb 2016 16:38:38 -
@@ -269,6 +269,41 @@
  to port the code to use C++11's std::unique_ptr instead.
  

+'constexpr' needed for in-class initialization of static data member
+
+
+Since C++11, the constexpr keyword is needed when initializing
+a non-integral static data member in a class.  Thus the following program is
+accepted in C++03 (albeit with a -Wpedantic warning):
+
+
+
+struct X {
+  const static double i = 10;
+};


It''s interesting that when the example is modified to use a double
initializer it is rejected with a hard error even in C++ 03 mode
when -Wpedantic (but not -Werror) is used.  That seems like a bug.
If it isn't, it might be worth mentioning the constraint that the
initializer must be a integer in the text above.

  struct X {
const static double i = 3.14;
  };

  error: floating-point literal cannot appear in a constant-expression
const static double i = 3.14;
   ^~~~


+
+
+
+The C++11 standard supports that in-class initialization using
+constexpr instead, so the GNU extension is no longer supported for
+C++11 or later.  Programs relying on the extension will be rejected with an
+error.  The fix is to use constexpr instead of const.
+
+
+Stricter flexible array member rules
+
+
+As of this release, the C++ compiler is now more strict about flexible array
+member rules.  As a consequence, the following code is no longer accepted:


In light of bug 69550 I think it might be useful to also mention
(or show an example) that structs with a flexible array as the
only member are rejected as well.

Martin


+
+
+
+union U {
+  int i;
+  char a[];
+};
+
+
  -Wmisleading-indentation
  
  A new warning -Wmisleading-indentation was added

Marek





add check for aarch64 in check_effective_target_section_anchors()

2016-02-11 Thread Prathamesh Kulkarni
Hi,
aarch64 supports section anchors but it appears
check_effective_target_section_anchors() doesn't contain entry for it.
This patch adds for entry for aarch64.
OK for trunk ?

Thanks,
Prathamesh
diff --git a/gcc/testsuite/lib/target-supports.exp 
b/gcc/testsuite/lib/target-supports.exp
index 645981a..66fb1ea 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -5467,7 +5467,8 @@ proc check_effective_target_section_anchors { } {
 } else {
 set et_section_anchors_saved 0
 if { [istarget powerpc*-*-*]
- || [istarget arm*-*-*] } {
+ || [istarget arm*-*-*] 
+ || [istarget aarch64*-*-*] } {
set et_section_anchors_saved 1
 }
 }


ChangeLog
Description: Binary data


Re: [RS6000] reload_vsx_from_gprsf splitter

2016-02-11 Thread David Edelsohn
On Thu, Feb 11, 2016 at 6:04 AM, Alan Modra  wrote:
> This is PR68973 part 2, the failure of a boost test, caused by a
> splitter emitting an invalid move in reload_vsx_from_gprsf:
>   emit_move_insn (op0_di, op2);
>
> op0 can be any vsx reg, but the mtvsrd destination constraint in
> movdi_internal64 is "wj", which only allows fp regs.  I'm not sure why
> we have that constraint so left movdi_internal64 alone and used a
> special p8_mtvsrd instead, similar to p8_mtvsrd_1 and p8_mtvsrd_2 used
> by reload_vsx_from_gpr.  When looking at those, I noticed we're
> restricted to fprs there too so fixed that as well.  (We can't use %L
> in asm operands that must accept vsx.)

Michael, please review the "wj" constraint in these patterns.

Alan, the explanation says that it uses a special p8_mtvsrd similar to
p8_mtvsrd_[12], but does not explain why the two similar patterns are
removed.  The incorrect use of %L implies those patterns, but the
change is to p8_xxpermdi_ that is not mentioned in the
ChangeLog.

I also would appreciate Uli's comments on this direction because of
his reload expertise.

Thanks, David

>
> Bootstrapped and regression tested powerpc64le-linux.  powerpc64-linux
> biarch -mcpu=power7 bootstrap still in progress.  OK to apply assuming
> no regressions found?
>
> PR target/68973
> * config/rs6000/vsx.md (vsx_xscvspdpn_directmove): Delete.
> * config/rs6000/rs6000.md (reload_vsx_from_gprsf): Rewrite splitter.
> (p8_mtvsrd): New.
> (p8_mtvsrd_1, p8_mtvsrd_2): Delete.
> (reload_vsx_from_gpr): Adjust to use p8_mtvsrd.
>
> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> index cdbf873..745293b 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -7543,29 +7543,22 @@
> (set_attr "type" "three")])
>
>  ;; Move 128 bit values from GPRs to VSX registers in 64-bit mode
> -(define_insn "p8_mtvsrd_1"
> -  [(set (match_operand:TF 0 "register_operand" "=ws")
> -   (unspec:TF [(match_operand:DI 1 "register_operand" "r")]
> +(define_insn "p8_mtvsrd"
> +  [(set (match_operand:DF 0 "register_operand" "=ws")
> +   (unspec:DF [(match_operand:DI 1 "register_operand" "r")]
>UNSPEC_P8V_MTVSRD))]
>"TARGET_POWERPC64 && TARGET_DIRECT_MOVE"
> -  "mtvsrd %0,%1"
> -  [(set_attr "type" "mftgpr")])
> -
> -(define_insn "p8_mtvsrd_2"
> -  [(set (match_operand:TF 0 "register_operand" "+ws")
> -   (unspec:TF [(match_dup 0)
> -   (match_operand:DI 1 "register_operand" "r")]
> -  UNSPEC_P8V_MTVSRD))]
> -  "TARGET_POWERPC64 && TARGET_DIRECT_MOVE"
> -  "mtvsrd %L0,%1"
> +  "mtvsrd %x0,%1"
>[(set_attr "type" "mftgpr")])
>
>  (define_insn "p8_xxpermdi_"
>[(set (match_operand:FMOVE128_GPR 0 "register_operand" "=wa")
> -   (unspec:FMOVE128_GPR [(match_operand:TF 1 "register_operand" "ws")]
> -UNSPEC_P8V_XXPERMDI))]
> +   (unspec:FMOVE128_GPR [
> +   (match_operand:DF 1 "register_operand" "ws")
> +   (match_operand:DF 2 "register_operand" "ws")]
> +   UNSPEC_P8V_XXPERMDI))]
>"TARGET_POWERPC64 && TARGET_DIRECT_MOVE"
> -  "xxpermdi %x0,%1,%L1,0"
> +  "xxpermdi %x0,%x1,%x2,0"
>[(set_attr "type" "vecperm")])
>
>  (define_insn_and_split "reload_vsx_from_gpr"
> @@ -7581,13 +7574,18 @@
>  {
>rtx dest = operands[0];
>rtx src = operands[1];
> -  rtx tmp = operands[2];
> +  /* You might think that we could use op0 as one temp and a DF clobber
> + as the other, but you'd be wrong.  These secondary_reload patterns
> + don't check that the clobber is different to the destination, which
> + is probably a reload bug.  */
> +  rtx tmp_hi = gen_rtx_REG (DFmode, REGNO (operands[2]));
> +  rtx tmp_lo = gen_rtx_REG (DFmode, REGNO (operands[2]) + 1);
>rtx gpr_hi_reg = gen_highpart (DImode, src);
>rtx gpr_lo_reg = gen_lowpart (DImode, src);
>
> -  emit_insn (gen_p8_mtvsrd_1 (tmp, gpr_hi_reg));
> -  emit_insn (gen_p8_mtvsrd_2 (tmp, gpr_lo_reg));
> -  emit_insn (gen_p8_xxpermdi_ (dest, tmp));
> +  emit_insn (gen_p8_mtvsrd (tmp_hi, gpr_hi_reg));
> +  emit_insn (gen_p8_mtvsrd (tmp_lo, gpr_lo_reg));
> +  emit_insn (gen_p8_xxpermdi_ (dest, tmp_hi, tmp_lo));
>DONE;
>  }
>[(set_attr "length" "12")
> @@ -7622,16 +7620,18 @@
>rtx op0 = operands[0];
>rtx op1 = operands[1];
>rtx op2 = operands[2];
> +
>/* Also use the destination register to hold the unconverted DImode value.
>   This is conceptually a separate value from OP0, so we use gen_rtx_REG
>   rather than simplify_gen_subreg.  */
> -  rtx op0_di = gen_rtx_REG (DImode, REGNO (op0));
> +  rtx op0_df = gen_rtx_REG (DFmode, REGNO (op0));
> +  rtx op0_v4sf = gen_rtx_REG (V4SFmode, REGNO (op0));
>rtx op1_di = simplify_gen_subreg (DImode, op1, SFmode, 0);
>
>/* Move SF value to upper 32-bits for xscvspdpn.  */
>emit_insn (gen_ashldi3 (op2, op1_di, GEN_INT (32)));
> -  emit_move_insn (op0_d

Re: [PATCH][AArch64] Only update assembler .arch directive when necessary

2016-02-11 Thread Christophe Lyon
On 11 February 2016 at 14:10, Kyrill Tkachov
 wrote:
>
> On 10/02/16 10:39, James Greenhalgh wrote:
>>
>> On Wed, Feb 10, 2016 at 10:32:16AM +, Kyrill Tkachov wrote:
>>>
>>> Hi James,
>>>
>>> On 10/02/16 10:11, James Greenhalgh wrote:

 On Thu, Feb 04, 2016 at 01:50:31PM +, Kyrill Tkachov wrote:
>
> Hi all,
>
> As part of the target attributes and pragmas support for GCC 6 I
> changed the
> aarch64 port to emit a .arch assembly directive for each function that
> describes the architectural features used by that function.  This is a
> change

 >from GCC 5 behaviour where we output a single .arch directive at the
>
> beginning of the assembly file corresponding to architectural features
> given
> on the command line.

 
>
> Bootstrapped and tested on aarch64-none-linux-gnu.  With this patch I
> managed
> to build a recent allyesconfig Linux kernel where before the build
> would fail
> when assembling the LSE instructions.
>
> Ok for trunk?

 One comment, that I'm willing to be convinced on...

> Thanks,
> Kyrill
>
> 2016-02-04  Kyrylo Tkachov  
>
>  * config/aarch64/aarch64.c (struct aarch64_output_asm_info):
>  New struct definition.
>  (aarch64_previous_asm_output): New variable.
>  (aarch64_declare_function_name): Only output .arch assembler
>  directive if it will be different from the previously output
>  directive.
>  (aarch64_start_file): New function.
>  (TARGET_ASM_FILE_START): Define.
>
> 2016-02-04  Kyrylo Tkachov  
>
>  * gcc.target/aarch64/assembler_arch_1.c: Add -dA to dg-options.
>  Delete unneeded -save-temps.
>  * gcc.target/aarch64/assembler_arch_7.c: Likewise.
>  * gcc.target/aarch64/target_attr_15.c: Scan assembly for
>  .arch armv8-a\n.
>  * gcc.target/aarch64/assembler_arch_1.c: New test.
> commit 2df0f24332e316b8d18d4571438f76726a0326e7
> Author: Kyrylo Tkachov 
> Date:   Wed Jan 27 12:54:54 2016 +
>
>  [AArch64] Only update assembler .arch directive when necessary
>
> diff --git a/gcc/config/aarch64/aarch64.c
> b/gcc/config/aarch64/aarch64.c
> index 5ca2ae8..0751440 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -11163,6 +11163,17 @@ aarch64_asm_preferred_eh_data_format (int code
> ATTRIBUTE_UNUSED, int global)
>  return (global ? DW_EH_PE_indirect : 0) | DW_EH_PE_pcrel | type;
>   }
> +struct aarch64_output_asm_info
> +{
> +  const struct processor *arch;
> +  const struct processor *cpu;
> +  unsigned long isa_flags;

 Why not just keep the last string you printed, and use a string compare
 to decide whether to print or not? Sure we'll end up doing a bit more
 work, but the logic becomes simpler to follow and we don't need to pass
 around another struct...
>>>
>>> I did do it this way to avoid a string comparison (I try to avoid
>>> manual string manipulations where I can as they're so easy to get wrong)
>>> though this isn't on any hot path.
>>> We don't really pass the structure around anywhere, we just keep one
>>> instance. We'd have to do the same with a string i.e. keep a string
>>> object around that we'd strcpy (or C++ equivalent) a string to every time
>>> we wanted to update it, so I thought this approach is cleaner as the
>>> architecture features are already fully described by a pointer to
>>> an element in the static constant all_architectures table and an
>>> unsigned long holding the ISA flags.
>>>
>>> If you insist I can change it to a string, but I personally don't
>>> think it's worth it.
>>
>> Had you been working on a C string I probably wouldn't have noticed. But
>> you're already working with C++ strings in this function, so much of what
>> you are concerned about is straightforward.
>>
>> I'd encourage you to try it using idiomatic string manipulation in C++,
>> the
>> cleanup should be worth it.
>
>
> Ok, here it is using std::string.
> It is a shorter patch.
>
> Bootstrapped and tested on aarch64.
>
> Ok?
>
> Thanks,
> Kyrill
>
> 2016-02-10  Kyrylo Tkachov  
>
> * config/aarch64/aarch64.c (aarch64_last_printed_arch_string):
> New variable.
> (aarch64_last_printed_tune_string): Likewise.
> (aarch64_declare_function_name): Only output .arch assembler
> directive if it will be different from the previously output
> directive.  Same for .tune comment but only if -dA is set.
> (aarch64_start_file): New function.
> (TARGET_ASM_FILE_START): Define.
>
> 2016-02-10  Kyrylo Tkachov  
>
> * gcc.target/aarch64/target_attr_15.c: Scan assembly for
> .arch armv8-a\n.  Add -dA to dg-options.
> * gcc.target/aarch64/assembler_arch_1.c: New test.

Hi Kyrill,

I'm seeing this new test fail, presumably because the binutils I

Re: [PATCH PR69052]Check if loop inv can be propagated into mem ref with additional addr expr canonicalization

2016-02-11 Thread Bin.Cheng
On Thu, Feb 11, 2016 at 7:14 AM, Jeff Law  wrote:
> On 02/09/2016 04:08 AM, Bin Cheng wrote:
>>
>> Hi,
>> When counting cost for loop inv, GCC checks if a loop inv can be
>> propagated into its use site (a memory reference).  If it cannot be
>> propagated, we increase its cost so that it's expensive enough to be hoisted
>> out of loop.  Currently we simply replace loop inv register in the use site
>> with its definition expression, then call validate_changes to check if the
>> result insn is valid.  This is weak because validate_changes doesn't take
>> canonicalization into consideration.  Given below example:
>>
>>Loop inv def:
>> 69: r149:SI=r87:SI+const(unspec[`'] 1)
>>REG_DEAD r87:SI
>>Loop inv use:
>> 70: r150:SI=[r90:SI*0x4+r149:SI]
>>REG_DEAD r149:SI
>>
>> The address expression after propagation is "r90 * 0x4 + (r87 +
>> const(unspec[`']))".  Function validate_changes simply returns false to
>> it.  As a matter of fact, the propagation is feasible if we canonicalize
>> address expression into the form like "(r90 * 0x4 + r87) +
>> const(unspec[`'])".
>>
>> This patch fixes the problem by canonicalizing address expression and
>> verifying if the new addr is valid.  The canonicalization follows GCC insn
>> canonicalization rules.  The test case from bugzilla PR is also included.
>> As for the canonicalize_address interface, there is another
>> canonicalize_address in fwprop.c which only changes shift into mult.  I
>> think it would be good to factor out a common RTL interface in GCC, but
>> that's stage1 work.
>
>
> Also note there's bits in combine that will canonicalize appropriate shifts
> into mults.  Clearly there's a need for some generalized routines to take a
> fairly generic address and perform canonicalizations and simplifications on
> it.
>
>> Bootstrap and test on x86_64 and AArch64.  Is it OK?
>>
>> Thanks,
>> bin
>>
>> 2016-02-09  Bin Cheng
>>
>> PR tree-optimization/69052
>> * loop-invariant.c (canonicalize_address): New function.
>> (inv_can_prop_to_addr_use): Check validity of address expression
>> which is canonicalized by above function.
>>
>> gcc/testsuite/ChangeLog
>> 2016-02-09  Bin Cheng
>>
>> PR tree-optimization/69052
>> * gcc.target/i386/pr69052.c: New test.
>>
>>
>> pr69052-20160204.txt
>>
>>
>> diff --git a/gcc/loop-invariant.c b/gcc/loop-invariant.c
>> index 707f044..157e273 100644
>> --- a/gcc/loop-invariant.c
>> +++ b/gcc/loop-invariant.c
>> @@ -754,6 +754,74 @@ create_new_invariant (struct def *def, rtx_insn
>> *insn, bitmap depends_on,
>> return inv;
>>   }
>>
>> +/* Returns a canonical version address for X.  It identifies
>> +   addr expr in the form of A + B + C.  Following instruction
>> +   canonicalization rules, MULT operand is moved to the front,
>> +   CONST operand is moved to the end; also PLUS operators are
>> +   chained to the left.  */
>> +
>> +static rtx
>> +canonicalize_address (rtx x)
>> +{
>> +  rtx op0, op1, op2;
>> +  machine_mode mode = GET_MODE (x);
>> +  enum rtx_code code = GET_CODE (x);
>> +
>> +  if (code != PLUS)
>> +return x;
>> +
>> +  /* Extract operands from A + B (+ C).  */
>> +  if (GET_CODE (XEXP (x, 0)) == PLUS)
>> +{
>> +  op0 = XEXP (XEXP (x, 0), 0);
>> +  op1 = XEXP (XEXP (x, 0), 1);
>> +  op2 = XEXP (x, 1);
>> +}
>> +  else if (GET_CODE (XEXP (x, 1)) == PLUS)
>> +{
>> +  op0 = XEXP (x, 0);
>> +  op1 = XEXP (XEXP (x, 1), 0);
>> +  op2 = XEXP (XEXP (x, 1), 1);
>> +}
>> +  else
>> +{
>> +  op0 = XEXP (x, 0);
>> +  op1 = XEXP (x, 1);
>> +  op2 = NULL_RTX;
>> +}
>> +
>> +  /* Move MULT operand to the front.  */
>> +  if (!REG_P (op1) && !CONST_INT_P (op1))
>> +std::swap (op0, op1);
>
> This feels a bit hack-ish in the sense that you already know the form of the
> RTL you're expecting and just assume that you'll be given something of that
> form, but no more complex.
>
> ISTM you're better off walking the whole rtx, recording the tidbits as you
> go into a vec.  If you see something unexpected during that walk, you punt
> canonicalization of the whole expression.
>
> You then sort the vec.  You want to move things like MULT to the start and
> all the constants to the end I think.
>
> You then do simplifications, particularly on the constants, but there may be
> something useful to do with MULT terms as well.  You could also arrange to
> rewrite ASHIFTs into MULTs at this stage.
>
> Then you generate a new equivalent expression from the simplified operands
> in the vec.
>
> You might look at tree-ssa-reassoc for ideas on implementation details.
>
> Initially just use it in the LICM code, but I think given that kind of
> structure it'd be generally useful to replace bits of combine and fwprop
>
> If your contention is that only a few forms really matter, then I'd like to
> see those forms spelled out better in the comment and some kind of checking
> that we have reasonable inc

Re: [wwwdocs] Add a note about in-class initialization of static data member

2016-02-11 Thread Marek Polacek
On Thu, Feb 11, 2016 at 10:09:19AM -0700, Martin Sebor wrote:
> It''s interesting that when the example is modified to use a double
> initializer it is rejected with a hard error even in C++ 03 mode
> when -Wpedantic (but not -Werror) is used.  That seems like a bug.
> If it isn't, it might be worth mentioning the constraint that the
> initializer must be a integer in the text above.
> 
>   struct X {
> const static double i = 3.14;
>   };
> 
>   error: floating-point literal cannot appear in a constant-expression
> const static double i = 3.14;
>^~~~

Hm, indeed; I hadn't notice that.  Dunno if is a bug (clang++ accepts this
with a warning).  I've added ", provided the initializer is an integer",
that should be enough.

> >+Stricter flexible array member rules
> >+
> >+
> >+As of this release, the C++ compiler is now more strict about flexible array
> >+member rules.  As a consequence, the following code is no longer accepted:
> 
> In light of bug 69550 I think it might be useful to also mention
> (or show an example) that structs with a flexible array as the
> only member are rejected as well.

Somehow I knew you'd have something to add here ;).  How about this then?

Index: porting_to.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-6/porting_to.html,v
retrieving revision 1.9
diff -u -r1.9 porting_to.html
--- porting_to.html 10 Feb 2016 17:21:54 -  1.9
+++ porting_to.html 11 Feb 2016 17:58:43 -
@@ -269,6 +269,53 @@
 to port the code to use C++11's std::unique_ptr instead.
 
 
+'constexpr' needed for in-class initialization of static data member
+
+
+Since C++11, the constexpr keyword is needed when initializing a
+non-integral static data member in a class.  As a GNU extension, the following
+program is accepted in C++03 (albeit with a -Wpedantic warning),
+provided the initializer is an integer:
+
+
+
+struct X {
+  const static double i = 10;
+};
+
+
+
+The C++11 standard supports that in-class initialization using
+constexpr instead, so the GNU extension is no longer supported for
+C++11 or later.  Programs relying on the extension will be rejected with an
+error.  The fix is to use constexpr instead of const.
+
+
+Stricter flexible array member rules
+
+
+As of this release, the C++ compiler is now more strict about flexible array
+member rules.  As a consequence, the following code is no longer accepted:
+
+
+
+union U {
+  int i;
+  char a[];
+};
+
+
+
+Furthermore, the C++ compiler now rejects structures with a flexible array
+member as the only member:
+
+
+
+struct S {
+  char a[];
+};
+
+
 -Wmisleading-indentation
 
 A new warning -Wmisleading-indentation was added

Marek


Re: [PATCH][AArch64] Only update assembler .arch directive when necessary

2016-02-11 Thread Kyrill Tkachov


On 11/02/16 17:57, Christophe Lyon wrote:

On 11 February 2016 at 14:10, Kyrill Tkachov
 wrote:

On 10/02/16 10:39, James Greenhalgh wrote:

On Wed, Feb 10, 2016 at 10:32:16AM +, Kyrill Tkachov wrote:

Hi James,

On 10/02/16 10:11, James Greenhalgh wrote:

On Thu, Feb 04, 2016 at 01:50:31PM +, Kyrill Tkachov wrote:

Hi all,

As part of the target attributes and pragmas support for GCC 6 I
changed the
aarch64 port to emit a .arch assembly directive for each function that
describes the architectural features used by that function.  This is a
change

>from GCC 5 behaviour where we output a single .arch directive at the

beginning of the assembly file corresponding to architectural features
given
on the command line.



Bootstrapped and tested on aarch64-none-linux-gnu.  With this patch I
managed
to build a recent allyesconfig Linux kernel where before the build
would fail
when assembling the LSE instructions.

Ok for trunk?

One comment, that I'm willing to be convinced on...


Thanks,
Kyrill

2016-02-04  Kyrylo Tkachov  

  * config/aarch64/aarch64.c (struct aarch64_output_asm_info):
  New struct definition.
  (aarch64_previous_asm_output): New variable.
  (aarch64_declare_function_name): Only output .arch assembler
  directive if it will be different from the previously output
  directive.
  (aarch64_start_file): New function.
  (TARGET_ASM_FILE_START): Define.

2016-02-04  Kyrylo Tkachov  

  * gcc.target/aarch64/assembler_arch_1.c: Add -dA to dg-options.
  Delete unneeded -save-temps.
  * gcc.target/aarch64/assembler_arch_7.c: Likewise.
  * gcc.target/aarch64/target_attr_15.c: Scan assembly for
  .arch armv8-a\n.
  * gcc.target/aarch64/assembler_arch_1.c: New test.
commit 2df0f24332e316b8d18d4571438f76726a0326e7
Author: Kyrylo Tkachov 
Date:   Wed Jan 27 12:54:54 2016 +

  [AArch64] Only update assembler .arch directive when necessary

diff --git a/gcc/config/aarch64/aarch64.c
b/gcc/config/aarch64/aarch64.c
index 5ca2ae8..0751440 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -11163,6 +11163,17 @@ aarch64_asm_preferred_eh_data_format (int code
ATTRIBUTE_UNUSED, int global)
  return (global ? DW_EH_PE_indirect : 0) | DW_EH_PE_pcrel | type;
   }
+struct aarch64_output_asm_info
+{
+  const struct processor *arch;
+  const struct processor *cpu;
+  unsigned long isa_flags;

Why not just keep the last string you printed, and use a string compare
to decide whether to print or not? Sure we'll end up doing a bit more
work, but the logic becomes simpler to follow and we don't need to pass
around another struct...

I did do it this way to avoid a string comparison (I try to avoid
manual string manipulations where I can as they're so easy to get wrong)
though this isn't on any hot path.
We don't really pass the structure around anywhere, we just keep one
instance. We'd have to do the same with a string i.e. keep a string
object around that we'd strcpy (or C++ equivalent) a string to every time
we wanted to update it, so I thought this approach is cleaner as the
architecture features are already fully described by a pointer to
an element in the static constant all_architectures table and an
unsigned long holding the ISA flags.

If you insist I can change it to a string, but I personally don't
think it's worth it.

Had you been working on a C string I probably wouldn't have noticed. But
you're already working with C++ strings in this function, so much of what
you are concerned about is straightforward.

I'd encourage you to try it using idiomatic string manipulation in C++,
the
cleanup should be worth it.


Ok, here it is using std::string.
It is a shorter patch.

Bootstrapped and tested on aarch64.

Ok?

Thanks,
Kyrill

2016-02-10  Kyrylo Tkachov  

 * config/aarch64/aarch64.c (aarch64_last_printed_arch_string):
 New variable.
 (aarch64_last_printed_tune_string): Likewise.
 (aarch64_declare_function_name): Only output .arch assembler
 directive if it will be different from the previously output
 directive.  Same for .tune comment but only if -dA is set.
 (aarch64_start_file): New function.
 (TARGET_ASM_FILE_START): Define.

2016-02-10  Kyrylo Tkachov  

 * gcc.target/aarch64/target_attr_15.c: Scan assembly for
 .arch armv8-a\n.  Add -dA to dg-options.
 * gcc.target/aarch64/assembler_arch_1.c: New test.

Hi Kyrill,


Hi Christophe,


I'm seeing this new test fail, presumably because the binutils I use
are too old:
/cckXrDIS.s: Assembler messages:
/cckXrDIS.s:4: Error: unknown pseudo-op: `.arch_extension'
/cckXrDIS.s:20: Error: selected processor does not support `stset w0,[x2]'

Do we want to guard the test against such cases and query binutils
capabilities?


Hmmm, I'd guess it depends on the effort.
I suppose it would involve creating an effective target check
that tries to assemble a .arch_extension and then also the stset instruction?
Do we have precedent for ch

Patch ping

2016-02-11 Thread Jakub Jelinek
Hi!

I'd like to ping a C++ P1 fix for PR69658:
  http://gcc.gnu.org/ml/gcc-patches/2016-02/msg00352.html

Jakub


Re: [PATCH][AArch64] Only update assembler .arch directive when necessary

2016-02-11 Thread Christophe Lyon
On 11 February 2016 at 19:04, Kyrill Tkachov
 wrote:
>
> On 11/02/16 17:57, Christophe Lyon wrote:
>>
>> On 11 February 2016 at 14:10, Kyrill Tkachov
>>  wrote:
>>>
>>> On 10/02/16 10:39, James Greenhalgh wrote:

 On Wed, Feb 10, 2016 at 10:32:16AM +, Kyrill Tkachov wrote:
>
> Hi James,
>
> On 10/02/16 10:11, James Greenhalgh wrote:
>>
>> On Thu, Feb 04, 2016 at 01:50:31PM +, Kyrill Tkachov wrote:
>>>
>>> Hi all,
>>>
>>> As part of the target attributes and pragmas support for GCC 6 I
>>> changed the
>>> aarch64 port to emit a .arch assembly directive for each function
>>> that
>>> describes the architectural features used by that function.  This is
>>> a
>>> change
>>
>> >from GCC 5 behaviour where we output a single .arch directive at the
>>>
>>> beginning of the assembly file corresponding to architectural
>>> features
>>> given
>>> on the command line.
>>
>> 
>>>
>>> Bootstrapped and tested on aarch64-none-linux-gnu.  With this patch I
>>> managed
>>> to build a recent allyesconfig Linux kernel where before the build
>>> would fail
>>> when assembling the LSE instructions.
>>>
>>> Ok for trunk?
>>
>> One comment, that I'm willing to be convinced on...
>>
>>> Thanks,
>>> Kyrill
>>>
>>> 2016-02-04  Kyrylo Tkachov  
>>>
>>>   * config/aarch64/aarch64.c (struct aarch64_output_asm_info):
>>>   New struct definition.
>>>   (aarch64_previous_asm_output): New variable.
>>>   (aarch64_declare_function_name): Only output .arch assembler
>>>   directive if it will be different from the previously output
>>>   directive.
>>>   (aarch64_start_file): New function.
>>>   (TARGET_ASM_FILE_START): Define.
>>>
>>> 2016-02-04  Kyrylo Tkachov  
>>>
>>>   * gcc.target/aarch64/assembler_arch_1.c: Add -dA to dg-options.
>>>   Delete unneeded -save-temps.
>>>   * gcc.target/aarch64/assembler_arch_7.c: Likewise.
>>>   * gcc.target/aarch64/target_attr_15.c: Scan assembly for
>>>   .arch armv8-a\n.
>>>   * gcc.target/aarch64/assembler_arch_1.c: New test.
>>> commit 2df0f24332e316b8d18d4571438f76726a0326e7
>>> Author: Kyrylo Tkachov 
>>> Date:   Wed Jan 27 12:54:54 2016 +
>>>
>>>   [AArch64] Only update assembler .arch directive when necessary
>>>
>>> diff --git a/gcc/config/aarch64/aarch64.c
>>> b/gcc/config/aarch64/aarch64.c
>>> index 5ca2ae8..0751440 100644
>>> --- a/gcc/config/aarch64/aarch64.c
>>> +++ b/gcc/config/aarch64/aarch64.c
>>> @@ -11163,6 +11163,17 @@ aarch64_asm_preferred_eh_data_format (int
>>> code
>>> ATTRIBUTE_UNUSED, int global)
>>>   return (global ? DW_EH_PE_indirect : 0) | DW_EH_PE_pcrel |
>>> type;
>>>}
>>> +struct aarch64_output_asm_info
>>> +{
>>> +  const struct processor *arch;
>>> +  const struct processor *cpu;
>>> +  unsigned long isa_flags;
>>
>> Why not just keep the last string you printed, and use a string
>> compare
>> to decide whether to print or not? Sure we'll end up doing a bit more
>> work, but the logic becomes simpler to follow and we don't need to
>> pass
>> around another struct...
>
> I did do it this way to avoid a string comparison (I try to avoid
> manual string manipulations where I can as they're so easy to get
> wrong)
> though this isn't on any hot path.
> We don't really pass the structure around anywhere, we just keep one
> instance. We'd have to do the same with a string i.e. keep a string
> object around that we'd strcpy (or C++ equivalent) a string to every
> time
> we wanted to update it, so I thought this approach is cleaner as the
> architecture features are already fully described by a pointer to
> an element in the static constant all_architectures table and an
> unsigned long holding the ISA flags.
>
> If you insist I can change it to a string, but I personally don't
> think it's worth it.

 Had you been working on a C string I probably wouldn't have noticed. But
 you're already working with C++ strings in this function, so much of
 what
 you are concerned about is straightforward.

 I'd encourage you to try it using idiomatic string manipulation in C++,
 the
 cleanup should be worth it.
>>>
>>>
>>> Ok, here it is using std::string.
>>> It is a shorter patch.
>>>
>>> Bootstrapped and tested on aarch64.
>>>
>>> Ok?
>>>
>>> Thanks,
>>> Kyrill
>>>
>>> 2016-02-10  Kyrylo Tkachov  
>>>
>>>  * config/aarch64/aarch64.c (aarch64_last_printed_arch_string):
>>>  New variable.
>>>  (aarch64_last_printed_tune_string): Likewise.
>>>  (aarch64_declare_function_name): Only output .arch assembler
>>>  directive if it will be different f

Re: [RS6000] reload_vsx_from_gprsf splitter

2016-02-11 Thread Ulrich Weigand
David Edelsohn wrote:
> On Thu, Feb 11, 2016 at 6:04 AM, Alan Modra  wrote:
> > This is PR68973 part 2, the failure of a boost test, caused by a
> > splitter emitting an invalid move in reload_vsx_from_gprsf:
> >   emit_move_insn (op0_di, op2);
> >
> > op0 can be any vsx reg, but the mtvsrd destination constraint in
> > movdi_internal64 is "wj", which only allows fp regs.  I'm not sure why
> > we have that constraint so left movdi_internal64 alone and used a
> > special p8_mtvsrd instead, similar to p8_mtvsrd_1 and p8_mtvsrd_2 used
> > by reload_vsx_from_gpr.  When looking at those, I noticed we're
> > restricted to fprs there too so fixed that as well.  (We can't use %L
> > in asm operands that must accept vsx.)
> 
> Michael, please review the "wj" constraint in these patterns.
> 
> Alan, the explanation says that it uses a special p8_mtvsrd similar to
> p8_mtvsrd_[12], but does not explain why the two similar patterns are
> removed.  The incorrect use of %L implies those patterns, but the
> change is to p8_xxpermdi_ that is not mentioned in the
> ChangeLog.
> 
> I also would appreciate Uli's comments on this direction because of
> his reload expertise.

For the most part, this patch doesn't really change anything in the
interaction with reload as far as I can see.  The changes introduced
by the patch do make sense to me.  In particular, replacing the two
patterns p8_mtvsrd_1 and p8_mtvsrd_2 used to fill high and low parts
of a TFmode register pair with a single pattern p8_mtvsrd that just
works on any DFmode register (leaving the split into high/low to the
caller if necessary) seems to simplify things.

> > +  /* You might think that we could use op0 as one temp and a DF clobber
> > + as the other, but you'd be wrong.  These secondary_reload patterns
> > + don't check that the clobber is different to the destination, which
> > + is probably a reload bug.  */

It's not a bug, it's deliberate behavior.  The reload registers allocated
for secondary reload clobbers may overlap the destination, since in many
cases you simply move the input to the reload register, and then the
reload register to the destination (where the latter move can be a no-op
if it is possible to allocate the reload register and the destination
into the same physical register).  If you need two separate intermediate
values, you need to allocate a secondary reload register of a larger
mode (as is already done in the pattern).

> >/* Also use the destination register to hold the unconverted DImode 
> > value.
> >   This is conceptually a separate value from OP0, so we use gen_rtx_REG
> >   rather than simplify_gen_subreg.  */
> > -  rtx op0_di = gen_rtx_REG (DImode, REGNO (op0));
> > +  rtx op0_df = gen_rtx_REG (DFmode, REGNO (op0));
> > +  rtx op0_v4sf = gen_rtx_REG (V4SFmode, REGNO (op0));

While this was not introduced by this patch, I'm a little bit concerned
about the hard-coded use of REGNO here, which will crash if op0 at this
point happens to be a SUBREG instead of a REG.  This is extremely unlikely
at this point in reload, but not 100% impossible, e.g. if op0 for some
reason is one of the "magic" registers like the stack pointer.

That's why it is in general better to use simplify_gen_subreg or one of
gen_highpart/gen_lowpart, which will handle SUBREG correctly as well.
I'm not sure why it matters whether this is "conceptually a separate
value" as the comment argues ...

> >/* Move SF value to upper 32-bits for xscvspdpn.  */
> >emit_insn (gen_ashldi3 (op2, op1_di, GEN_INT (32)));
> > -  emit_move_insn (op0_di, op2);
> > -  emit_insn (gen_vsx_xscvspdpn_directmove (op0, op0_di));
> > +  emit_insn (gen_p8_mtvsrd (op0_df, op2));
> > +  emit_insn (gen_vsx_xscvspdpn (op0_df, op0_v4sf));

The sequence of modes used for op0 here is a bit weird.  First,
op0 is loaded as DFmode by mtvsrd, then it is silently re-
interpreted as V4SFmode when used as input to xscvspdpn, which
gets a DFmode output that is again silently re-interpreted as
SFmode.

This isn't really wrong as such, just maybe a bit confusing.
Maybe instead have p8_mtvsrd use DImode as output (instead of
DFmode), which shouldn't be any harder to use in the
reload_vsx_from_gpr splitter, and keep using the
vsx_xscvspdpn_directmove pattern?

[ This of course reinforces the question why we have p8_mtvsrd
in the first place, instead of just allowing this use directly
in movdi_internal64 itself.  ]

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  ulrich.weig...@de.ibm.com



Re: [patch,libgfortran] Bug 69668 - [4.9/5/6 Regression] Error reading namelist opened with DELIM='NONE'

2016-02-11 Thread Janne Blomqvist
On Wed, Feb 10, 2016 at 4:45 AM, Jerry DeLisle  wrote:
> The attached patch reverts the guilty code. We were trying to honor delim=NONE
> on namelist reads which is invalid.
>
> Test cases updated. Regression tested on x86-64.
>
> OK for trunk and back port in about a week?

For namelist_38.90, I think it would be better to open it with
status="scratch" as it was before, so that we don't leave test files
around in case the test fails.

And in the ChangeLog entry, you have misspelled the testcase names
(naMelist, not naNelist).

Ok with these changes.

>
> Regards,
>
> Jerry
>
> 2016-02-09  Jerry DeLisle  
>
> PR libgfortran/69668
> * io/list_read.c (read_character): Remove code related to DELIM_NONE.
>
>
>
> 2016-02-09 Jerry DeLisle 
>
> PR libgfortran/69668
> * gfortran.dg/nanelist_38.f90: Update test.
> * gfortran.dg/nanelist_84.f90: Update test.



-- 
Janne Blomqvist


Re: [patch,libgfortran] Bug 69668 - [4.9/5/6 Regression] Error reading namelist opened with DELIM='NONE'

2016-02-11 Thread Jerry DeLisle
On 02/09/2016 06:45 PM, Jerry DeLisle wrote:
> The attached patch reverts the guilty code. We were trying to honor delim=NONE
> on namelist reads which is invalid.
> 
> Test cases updated. Regression tested on x86-64.
> 
> OK for trunk and back port in about a week?
> 


No response yet.  Since this is a simple revert, will commit to trunk later 
today.


> Regards,
> 
> Jerry
> 
> 2016-02-09  Jerry DeLisle  
> 
>   PR libgfortran/69668
>   * io/list_read.c (read_character): Remove code related to DELIM_NONE.
> 
> 
> 
> 2016-02-09 Jerry DeLisle 
> 
>   PR libgfortran/69668
>   * gfortran.dg/nanelist_38.f90: Update test.
>   * gfortran.dg/nanelist_84.f90: Update test.
> 


Re: [wwwdocs] Add a note about in-class initialization of static data member

2016-02-11 Thread Martin Sebor

   struct X {
 const static double i = 3.14;
   };

   error: floating-point literal cannot appear in a constant-expression
 const static double i = 3.14;
^~~~


Hm, indeed; I hadn't notice that.  Dunno if is a bug (clang++ accepts this
with a warning).  I've added ", provided the initializer is an integer",
that should be enough.


I don't think documenting this as a blanket restriction is right.
The code is silently accepted without -Wpedantic and few people
use the option so the added text would be misleading as is.  But
my feeling is that this is just a bug, in which case I wouldn't
expect to see it documented in any case.



Somehow I knew you'd have something to add here ;).  How about this then?


The flexible array addition looks great to me. Thank you!

Martin



Re: [wwwdocs] Add a note about in-class initialization of static data member

2016-02-11 Thread Marek Polacek
On Thu, Feb 11, 2016 at 11:45:37AM -0700, Martin Sebor wrote:
> >>   struct X {
> >> const static double i = 3.14;
> >>   };
> >>
> >>   error: floating-point literal cannot appear in a constant-expression
> >> const static double i = 3.14;
> >>^~~~
> >
> >Hm, indeed; I hadn't notice that.  Dunno if is a bug (clang++ accepts this
> >with a warning).  I've added ", provided the initializer is an integer",
> >that should be enough.
> 
> I don't think documenting this as a blanket restriction is right.
> The code is silently accepted without -Wpedantic and few people
> use the option so the added text would be misleading as is.  But
> my feeling is that this is just a bug, in which case I wouldn't
> expect to see it documented in any case.

Ok, I'll drop that note then.
 
> The flexible array addition looks great to me. Thank you!

Great.  I'll commit the patch.

Marek


Re: [patch, Fortran] Fix PR 60526, variable name has already been declared as a type

2016-02-11 Thread Janne Blomqvist
On Sat, Feb 6, 2016 at 10:20 PM, Thomas Koenig  wrote:
> Hi Andre,
>
>> In preventing memory clutter I like to advise the use of:
>>
>> char u_name[GFC_MAX_SYMBOL_LEN + 1];
>>
>> and safe us all the dynamic memory allocation/free.
>
>
> We're really talking micro-optimizations here, but well ... ;-)

/me joins the micro-optimization bikeshed!

> In the attached patch, I have replaced this with alloca.  I was going
> to  use a VLA originally, but apparently C++ doesn't like that, at least
> not in the version that we use within C++.

IIRC VLA's were proposed for C++14, but were ultimately rejected.

The usual arguments against VLA's (and alloca, which is just another
way to spell the same thing, really) are that

- It reserves an extra register for the frame pointer.

- Functions using VLA's aren't inlined by GCC

- From a correctness perspective, unless you check the size somehow
beforehand, you can easily get a stack overflow crash.

So one solution to these correctness and performance issues is to have
a reasonably small statically sized buffer on the stack, and if the
size is bigger than that, fall back to heap allocation. E.g. something
like

void foo(..., size_t n) {
  char tmp[BUFSIZE];
  char *buf;
  if (n <= BUFSIZE)
buf = tmp;
  else
buf = xmalloc(n);
  // Use buf
 if (n > BUFSIZE)
free(buf);
}

This is roughly what std::string does with the new C++11 compatible
libstdc++ ABI. As we're supposed to be C++ nowadays, why aren't we
using that?



-- 
Janne Blomqvist


Re: [C/C++ PATCH] Fix a -Waddress regression (PR c/69768)

2016-02-11 Thread Jason Merrill

On 02/11/2016 11:25 AM, Jakub Jelinek wrote:

+  && !integer_zerop (tree_strip_nop_conversions (op1)))


Maybe cp_fold rather than tree_strip_nop_conversions?

Jason



Re: [C/C++ PATCH] Fix a -Waddress regression (PR c/69768)

2016-02-11 Thread Jakub Jelinek
Hi!

On Thu, Feb 11, 2016 at 01:56:09PM -0500, Jason Merrill wrote:
> On 02/11/2016 11:25 AM, Jakub Jelinek wrote:
> >+   && !integer_zerop (tree_strip_nop_conversions (op1)))
> 
> Maybe cp_fold rather than tree_strip_nop_conversions?

Is it safe to call cp_fully_fold (typeck.c only calls it by that name,
not cp_fold) on perhaps type or value dependent argument?
E.g. on
template 
int
bar ()
{
  return "foo1" != (void *) N
 && "foo2" != (const char *) ((void *) N)
 && "foo3" != (const char *) ((void *) (N - N))
 && "foo4" != (const char *) ((void *) (&e - &e))
 && "foo5" != "foo6";
}
op1 is NON_DEPENDENT_EXPR.  If yes, here is an alternative (that has the same
behavior on the testcase as 5.x does).  The C FE didn't use to fold it, so
I'm preserving its behavior.

2016-02-11  Jakub Jelinek  

PR c/69768
* c-typeck.c (parser_build_binary_op): Strip nops from integer_zerop
arguments for -Waddress warning.

* typeck.c (cp_build_binary_op): cp_fully_fold integer_zerop
arguments for -Waddress warning.  Fix up formatting.

* c-c++-common/Waddress-1.c: New test.

--- gcc/c/c-typeck.c.jj 2016-02-11 20:28:51.316491659 +0100
+++ gcc/c/c-typeck.c2016-02-11 20:29:50.778672554 +0100
@@ -3597,8 +3597,10 @@ parser_build_binary_op (location_t locat
  of testing for equality or inequality of a string literal with NULL.  */
   if (code == EQ_EXPR || code == NE_EXPR)
 {
-  if ((code1 == STRING_CST && !integer_zerop (arg2.value))
- || (code2 == STRING_CST && !integer_zerop (arg1.value)))
+  if ((code1 == STRING_CST
+  && !integer_zerop (tree_strip_nop_conversions (arg2.value)))
+ || (code2 == STRING_CST
+ && !integer_zerop (tree_strip_nop_conversions (arg1.value
warning_at (location, OPT_Waddress,
"comparison with string literal results in unspecified 
behavior");
 }
--- gcc/cp/typeck.c.jj  2016-02-11 20:28:51.196493312 +0100
+++ gcc/cp/typeck.c 2016-02-11 20:34:41.124672969 +0100
@@ -4487,9 +4487,12 @@ cp_build_binary_op (location_t location,
warning (OPT_Wfloat_equal,
 "comparing floating point with == or != is unsafe");
   if ((complain & tf_warning)
- && ((TREE_CODE (orig_op0) == STRING_CST && !integer_zerop (op1))
- || (TREE_CODE (orig_op1) == STRING_CST && !integer_zerop (op0
-   warning (OPT_Waddress, "comparison with string literal results in 
unspecified behaviour");
+ && ((TREE_CODE (orig_op0) == STRING_CST
+  && !integer_zerop (cp_fully_fold (op1)))
+ || (TREE_CODE (orig_op1) == STRING_CST
+ && !integer_zerop (cp_fully_fold (op0)
+   warning (OPT_Waddress, "comparison with string literal results "
+  "in unspecified behaviour");
 
   build_type = boolean_type_node;
   if ((code0 == INTEGER_TYPE || code0 == REAL_TYPE
--- gcc/testsuite/c-c++-common/Waddress-1.c.jj  2016-02-11 20:29:50.781672512 
+0100
+++ gcc/testsuite/c-c++-common/Waddress-1.c 2016-02-11 20:35:40.299857817 
+0100
@@ -0,0 +1,15 @@
+/* PR c/69768 */
+/* { dg-do compile } */
+/* { dg-options "-Waddress" } */
+
+static int e;
+
+int
+foo ()
+{
+  return "foo1" != (void *) 0  /* { dg-bogus "comparison with string literal 
results in unspecified behaviou?r" } */
+&& "foo2" != (const char *) ((void *) 0)   /* { dg-bogus 
"comparison with string literal results in unspecified behaviou?r" } */
+&& "foo3" != (const char *) ((void *) (10 - 10))   /* { dg-bogus 
"comparison with string literal results in unspecified behaviou?r" } */
+&& "foo4" != (const char *) ((void *) (&e - &e))   /* { dg-warning 
"comparison with string literal results in unspecified behaviou?r" "" { target 
c } } */
+&& "foo5" != "foo6";   /* { dg-warning "comparison with string literal 
results in unspecified behaviou?r" } */
+}


Jakub


Re: [C/C++ PATCH] Fix a -Waddress regression (PR c/69768)

2016-02-11 Thread Marek Polacek
On Thu, Feb 11, 2016 at 08:44:24PM +0100, Jakub Jelinek wrote:
> Hi!
> 
> On Thu, Feb 11, 2016 at 01:56:09PM -0500, Jason Merrill wrote:
> > On 02/11/2016 11:25 AM, Jakub Jelinek wrote:
> > >+ && !integer_zerop (tree_strip_nop_conversions (op1)))
> > 
> > Maybe cp_fold rather than tree_strip_nop_conversions?
> 
> Is it safe to call cp_fully_fold (typeck.c only calls it by that name,
> not cp_fold) on perhaps type or value dependent argument?
> E.g. on
> template 
> int
> bar ()
> {
>   return "foo1" != (void *) N
>&& "foo2" != (const char *) ((void *) N)
>&& "foo3" != (const char *) ((void *) (N - N))
>&& "foo4" != (const char *) ((void *) (&e - &e))
>&& "foo5" != "foo6";
> }
> op1 is NON_DEPENDENT_EXPR.  If yes, here is an alternative (that has the same
> behavior on the testcase as 5.x does).  The C FE didn't use to fold it, so
> I'm preserving its behavior.
> 
> 2016-02-11  Jakub Jelinek  
> 
>   PR c/69768
>   * c-typeck.c (parser_build_binary_op): Strip nops from integer_zerop
>   arguments for -Waddress warning.
> 
>   * typeck.c (cp_build_binary_op): cp_fully_fold integer_zerop
>   arguments for -Waddress warning.  Fix up formatting.
> 
>   * c-c++-common/Waddress-1.c: New test.
> 
> --- gcc/c/c-typeck.c.jj   2016-02-11 20:28:51.316491659 +0100
> +++ gcc/c/c-typeck.c  2016-02-11 20:29:50.778672554 +0100
> @@ -3597,8 +3597,10 @@ parser_build_binary_op (location_t locat
>   of testing for equality or inequality of a string literal with NULL.  */
>if (code == EQ_EXPR || code == NE_EXPR)
>  {
> -  if ((code1 == STRING_CST && !integer_zerop (arg2.value))
> -   || (code2 == STRING_CST && !integer_zerop (arg1.value)))
> +  if ((code1 == STRING_CST
> +&& !integer_zerop (tree_strip_nop_conversions (arg2.value)))
> +   || (code2 == STRING_CST
> +   && !integer_zerop (tree_strip_nop_conversions (arg1.value
>   warning_at (location, OPT_Waddress,
>   "comparison with string literal results in unspecified 
> behavior");
>  }

The C part looks ok to me.

Marek


Re: [PATCH] Fix another ipa-split caused ICE (PR ipa/69241)

2016-02-11 Thread Jeff Law

On 02/11/2016 02:22 AM, Richard Biener wrote:

On Wed, 10 Feb 2016, Jakub Jelinek wrote:


Hi!

Markus has pointed out to a reduced testcase which still ICEs even with the
PR69241 fix.  In that case the function with TREE_ADDRESSABLE return type
does not return at all (and -Wreturn-type properly diagnoses it).
For that case the following patch just forces the lhs on the *.part.*
call, so that we don't ICE in assign_temp.

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


Um...  I wonder if it wouldn't be better to force such functions to
be noreturn by say, placing a __builtin_unreachable () at the missing
return and maybe even adjust the return type.
Remember that __builtin_unreachable doesn't actually insert any real 
code, so if by some means we get to that point, we'll happily continue 
executing whatever code is there, which opens a potential security hole.


I generally prefer __builtin_trap over __builtin_unreachable.  The 
single instruction is of marginal cost and provides a level of safety if 
we do manage to get into "unreachable" code.


Jeff



[RFC] [PATCH] Add __array_size keyword

2016-02-11 Thread Stuart Brady
This patch adds an __array_size keyword for the C, C++, Objective C and
Objective C++ languages which is similar to the sizeof keyword, but
yields the size of the specified array in elements, not bytes, and will
not accept expressions of pointer type.

At the moment, I am only looking for feedback, as I appreciate that this
would have to wait for GCC 7 at the earliest, if it is deemed desirable.

I have searched and been unable to find any previous submissions similar
to this, or previous discussion of the idea.  I have discussed this with
a number of C developers and generally received positive encouragement
for submitting this patch, so it seems odd to me that this is the case,
so perhaps I have simply missed prior submissions, somehow?

The intent is that one can (perhaps conditionally) use a new definition
for the ARRAY_SIZE macro that is commonly used:

   #ifdef FOO
  #define ARRAY_SIZE(arr) __array_size(arr)
   #else
  #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof(*arr))
   #endif

The advantage to using this macro is that it will catch situations where
we attempt to take the array size of a pointer.  The error message that I
have added is as follows:

   arraysize-test.c: In function ‘foo’:
   arraysize-test.c:4:55: error: invalid application of ‘__array_size’ to 
non-array type
 printf("%i\n", __array_size(arr));
^

There may well be a better wording for this message and I would welcome
suggestions.

It should be noted that GCC's __builtin_types_compatible_p builtin allows
similar type checking to be performed with an expression that results in
evaluation of the size of a bitfield of negative width, in the case where
a pointer type is used.

However, the macro or series of macros to do this is not especially
simple, nor is the error message that is generated especially readable,
even when using appropriately named macros for the purpose[0]:

   arraysize-old.c: In function ‘main’:
   arraysize-old.c:12:20: error: negative width in bit-field ‘’
(sizeof(struct { int:-!!(e)*1234; }))
   ^
   arraysize-old.c:17:6: note: in expansion of macro ‘BUILD_BUG_ON_ZERO’
 BUILD_BUG_ON_ZERO(SAME_TYPE((a), &(*a)))
 ^
   arraysize-old.c:30:8: note: in expansion of macro ‘MUST_BE_ARRAY’
 + MUST_BE_ARRAY(a))
   ^
   arraysize-old.c:36:17: note: in expansion of macro ‘ARRAY_SIZE’
 printf("%i\n", ARRAY_SIZE(b));
^~

An additional consideration would be whether a feature test macro would
be beneficial, and whether there are any matters of consideration that
might hypothetically be relevant to ISO/IEC JTC 1/SC 22 / WG14 (although
perhaps I may be getting ahead of myself).

I present this change largely as a diagnostics improvement, rather than
as new functionality.

That said, for C++, this does introduce __array_size as a keyword that
must be handled within the name mangling scheme.  For this, I have used
the string "as", but I would appreciate advice on how appropriate this
encoding is.

Documentation and test code are currently absent from the patch, but I
will add these, should this RFC receive sufficient positive feedback to
be worth following up.

I would like advice as to what test cases I should add, i.e. should I
simply duplicate all of the tests for sizeof, or only target likely
trouble spots where __array_size behaves differently?

I appreciate that I will also need a ChangeLog entry, and to do proper
testing and include test results.  If anyone could help guide me through
this process I would be very grateful.  My patch, if deemed desirable, may
be large enough to require copyright assignment, in which case I would
very much appreciate some help in dealing with this.
-- 
Stuart Brady

[0] My own blog post with the macro that I use:
   
http://zubplot.blogspot.co.uk/2015/01/gcc-is-wonderful-better-arraysize-macro.html

 c-family/c-common.c   |   54 +++
 c-family/c-common.def |4 ++
 c-family/c-common.h   |3 +
 c/c-parser.c  |   62 
 c/c-tree.h|3 +
 c/c-typeck.c  |   69 
 cp/constexpr.c|1
 cp/cp-objcp-common.c  |1
 cp/cp-tree.h  |9 +
 cp/cxx-pretty-print.c |   11 --
 cp/error.c|6 ++-
 cp/mangle.c   |   12 ++
 cp/operators.def  |1
 cp/parser.c   |   16 -
 cp/pt.c   |   35 +++-
 cp/tree.c |8 
 cp/typeck.c   |   86 +-
 17 files changed, 360 insertions(+), 21 deletions(-)

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 3d84316..8033d14 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -445,6 +445,8 @@ const struct c_common_resword c_common_reswords[] =
   { "__PRETTY_FUNCTION__", RID

Re: [C/C++ PATCH] Fix a -Waddress regression (PR c/69768)

2016-02-11 Thread Jason Merrill

On 02/11/2016 02:44 PM, Jakub Jelinek wrote:

Hi!

On Thu, Feb 11, 2016 at 01:56:09PM -0500, Jason Merrill wrote:

On 02/11/2016 11:25 AM, Jakub Jelinek wrote:

+  && !integer_zerop (tree_strip_nop_conversions (op1)))


Maybe cp_fold rather than tree_strip_nop_conversions?


Is it safe to call cp_fully_fold (typeck.c only calls it by that name,
not cp_fold) on perhaps type or value dependent argument?


In a template cp_fold does nothing, so it's safe but won't warn until 
instantiation time.


OK either way.

Jason



Re: [wwwdocs] Add a note about in-class initialization of static data member

2016-02-11 Thread Martin Sebor

The flexible array addition looks great to me. Thank you!


Great.  I'll commit the patch.


Actually, there is one other thing that might be wort mentioning
about flexible array members.

The type and mangling of flexible array members has changed.  While
in GCC 5 and prior the type of a flexible array member is an array
of zero elements (a GCC extension), in 6 it is that of an array of
an unspecified bound (i.e., T[] as opposed to T[0]).  This is
a silent ABI change with no -fabi-version/-Wabi option.

Martin


Re: [wwwdocs] Add a note about in-class initialization of static data member

2016-02-11 Thread Marek Polacek
On Thu, Feb 11, 2016 at 01:36:49PM -0700, Martin Sebor wrote:
> >>The flexible array addition looks great to me. Thank you!
> >
> >Great.  I'll commit the patch.
> 
> Actually, there is one other thing that might be wort mentioning
> about flexible array members.
> 
> The type and mangling of flexible array members has changed.  While
> in GCC 5 and prior the type of a flexible array member is an array
> of zero elements (a GCC extension), in 6 it is that of an array of
> an unspecified bound (i.e., T[] as opposed to T[0]).  This is
> a silent ABI change with no -fabi-version/-Wabi option.

Aha.  I think you're better-suited to document this than I am.

Marek


Re: [PATCH] combine: More distribute_notes trouble (PR69737)

2016-02-11 Thread H.J. Lu
On Thu, Feb 11, 2016 at 9:04 AM, Segher Boessenkool
 wrote:
> PR64682 is a problem in distribute_notes, where it has trouble putting
> a REG_DEAD note for a reg that is set twice in the right spot.  My fix
> for that did the wrong thing for PR69567.  And then my attempted fix
> for that one made PR64682 fail again.
>
> Instead, let's just lose the note in such complicated cases, like we
> already do in certain similar cases.
>
> Tested on powerpc64-linux and x86_64-linux.  Also built Linux kernels
> for some 30 supported targets; no difference in generated code was
> observed.
>
> Committing to trunk.
>
> HJ, I tested this on GCC 5 for x86_64-linux, the failure is gone;
> could you test it on your setup before I apply it there though?
>
>
> Segher
>
>
> 2016-02-11  Segher Boessenkool  
>
> PR rtl-optimization/64682
> PR rtl-optimization/69567
> PR rtl-optimization/69737
> * combine.c (distribute_notes) : If the register is set
> in I2 as well, just lose it.
>
>

Yes, it fixed the regressions on ia32 and x96-64:

New passes:
FAIL: gcc.c-torture/execute/pr64682.c   -O2  execution test
FAIL: gcc.c-torture/execute/pr64682.c   -O2  execution test
FAIL: gcc.c-torture/execute/pr64682.c   -O2 -flto
-fno-use-linker-plugin -flto-partition=none  execution test
FAIL: gcc.c-torture/execute/pr64682.c   -O2 -flto
-fno-use-linker-plugin -flto-partition=none  execution test
FAIL: gcc.c-torture/execute/pr64682.c   -O2 -flto -fuse-linker-plugin
-fno-fat-lto-objects  execution test
FAIL: gcc.c-torture/execute/pr64682.c   -O2 -flto -fuse-linker-plugin
-fno-fat-lto-objects  execution test
FAIL: gcc.c-torture/execute/pr64682.c   -O3 -fomit-frame-pointer  execution test
FAIL: gcc.c-torture/execute/pr64682.c   -O3 -fomit-frame-pointer  execution test
FAIL: gcc.c-torture/execute/pr64682.c   -O3 -fomit-frame-pointer
-funroll-all-loops -finline-functions  execution test
FAIL: gcc.c-torture/execute/pr64682.c   -O3 -fomit-frame-pointer
-funroll-all-loops -finline-functions  execution test
FAIL: gcc.c-torture/execute/pr64682.c   -O3 -fomit-frame-pointer
-funroll-loops  execution test
FAIL: gcc.c-torture/execute/pr64682.c   -O3 -fomit-frame-pointer
-funroll-loops  execution test
FAIL: gcc.c-torture/execute/pr64682.c   -O3 -g  execution test
FAIL: gcc.c-torture/execute/pr64682.c   -O3 -g  execution test

Thanks.


-- 
H.J.


[wwwdocs] Mention flexible array type and mangling change

2016-02-11 Thread Martin Sebor

Actually, there is one other thing that might be wort mentioning
about flexible array members.

The type and mangling of flexible array members has changed.  While
in GCC 5 and prior the type of a flexible array member is an array
of zero elements (a GCC extension), in 6 it is that of an array of
an unspecified bound (i.e., T[] as opposed to T[0]).  This is
a silent ABI change with no -fabi-version/-Wabi option.


Aha.  I think you're better-suited to document this than I am.


Sure.  Here's the added text.  Please let me know if it's good to
check in.

Martin


Index: htdocs/gcc-6/porting_to.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-6/porting_to.html,v
retrieving revision 1.12
diff -u -r1.12 porting_to.html
--- htdocs/gcc-6/porting_to.html11 Feb 2016 18:56:15 -  1.12
+++ htdocs/gcc-6/porting_to.html11 Feb 2016 21:15:30 -
@@ -315,6 +315,15 @@
 };
 

+
+Finally, the type and mangling of flexible array members has changed
+from previous releases.  While in GCC 5 and prior the type of a flexible
+array member is an array of zero elements (a GCC extension), in GCC 6 it
+is that of an array of an unspecified bound (i.e., T[] as opposed
+to T[0]).  This is a silent ABI change with no corresponding
+-fabi-version or -Wabi option to disable or warn about.
+
+
 -Wmisleading-indentation
 
 A new warning -Wmisleading-indentation was added



Re: [PATCH], PR 68404 patch #3 (fix earlyclobber problem on power8 fusion)

2016-02-11 Thread Michael Meissner
After looking at Bernd Schmidt and Jakub Jelinek's suggestions, I came to
conclusion that earlyclobber was not needed in this case, and I removed it.  I
bootstrapped the compiler using profiledbootstrap and lto options and it
succeeded build and running make check.  Just to be sure, I also did a
profiledbootstrap with LTO and -O3 and it built fine.  Is it ok to install
these patches?

I decided to keep the changes to the testsuite explicitly passing the fusion
switches, rather than letting -mtune=power8/power9 set them, but I can be
persuaded to restore the 3 tests to the way they were before February 9th.

[gcc]
2016-02-11  Michael Meissner  

PR target/68404
* config/rs6000/predicates.md (fusion_gpr_addis): Revert
2016-02-09 change.

* config/rs6000/rs6000.md (fusion_gpr_load_): Remove
earlyclobber from target.  Use wF constraint for fused memory
address.
(fusion_gpr___load): Likewise.

[gcc/testsuites]
2016-02-11  Michael Meissner  

PR target/68404
* gcc.target/powerpc/fusion.c: Do not assume that -mtune=power8
sets -mpower8-fusion or -mtune=power9 sets -mpower9-fusion.
* gcc.target/powerpc/fusion2.c: Likewise.
* gcc.target/powerpc/fusion3.c: Likewise.

Since gcc 5.0 also has the earlyclobber in the pattern, I would like to apply
the same change to gcc 5.x (after testing of course), even though we haven't
yet run into the problem with GCC 5.x.  Is this ok as well?

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797
Index: gcc/config/rs6000/predicates.md
===
--- gcc/config/rs6000/predicates.md (revision 233351)
+++ gcc/config/rs6000/predicates.md (working copy)
@@ -1708,14 +1708,23 @@ (define_predicate "fusion_gpr_addis"
   (match_code "const_int,high,plus")
 {
   HOST_WIDE_INT value;
+  rtx int_const;
 
   if (GET_CODE (op) == HIGH)
 return 1;
 
-  if (!CONST_INT_P (op))
+  if (CONST_INT_P (op))
+int_const = op;
+
+  else if (GET_CODE (op) == PLUS
+  && base_reg_operand (XEXP (op, 0), Pmode)
+  && CONST_INT_P (XEXP (op, 1)))
+int_const = XEXP (op, 1);
+
+  else
 return 0;
 
-  value = INTVAL (op);
+  value = INTVAL (int_const);
   if ((value & (HOST_WIDE_INT)0x) != 0)
 return 0;
 
Index: gcc/config/rs6000/rs6000.md
===
--- gcc/config/rs6000/rs6000.md (revision 233351)
+++ gcc/config/rs6000/rs6000.md (working copy)
@@ -12915,8 +12915,8 @@ (define_peephole2
 ;; reload)
 
 (define_insn "fusion_gpr_load_"
-  [(set (match_operand:INT1 0 "base_reg_operand" "=&b")
-   (unspec:INT1 [(match_operand:INT1 1 "fusion_addis_mem_combo_load" "")]
+  [(set (match_operand:INT1 0 "base_reg_operand" "=b")
+   (unspec:INT1 [(match_operand:INT1 1 "fusion_addis_mem_combo_load" "wF")]
 UNSPEC_FUSION_GPR))]
   "TARGET_P8_FUSION"
 {
@@ -12987,7 +12987,7 @@ (define_insn "fusion_gpr__

Re: [RS6000] reload_vsx_from_gprsf splitter

2016-02-11 Thread Michael Meissner
On Thu, Feb 11, 2016 at 09:34:29AM -0800, David Edelsohn wrote:
> On Thu, Feb 11, 2016 at 6:04 AM, Alan Modra  wrote:
> > This is PR68973 part 2, the failure of a boost test, caused by a
> > splitter emitting an invalid move in reload_vsx_from_gprsf:
> >   emit_move_insn (op0_di, op2);
> >
> > op0 can be any vsx reg, but the mtvsrd destination constraint in
> > movdi_internal64 is "wj", which only allows fp regs.  I'm not sure why
> > we have that constraint so left movdi_internal64 alone and used a
> > special p8_mtvsrd instead, similar to p8_mtvsrd_1 and p8_mtvsrd_2 used
> > by reload_vsx_from_gpr.  When looking at those, I noticed we're
> > restricted to fprs there too so fixed that as well.  (We can't use %L
> > in asm operands that must accept vsx.)
> 
> Michael, please review the "wj" constraint in these patterns.
> 
> Alan, the explanation says that it uses a special p8_mtvsrd similar to
> p8_mtvsrd_[12], but does not explain why the two similar patterns are
> removed.  The incorrect use of %L implies those patterns, but the
> change is to p8_xxpermdi_ that is not mentioned in the
> ChangeLog.
> 
> I also would appreciate Uli's comments on this direction because of
> his reload expertise.

Since the mail was sent to my Lotus Notes account (mrmei...@us.ibm.com), I
originally tried to reply through that system, but I got bounce errors.

At the present time, we do not allow DImode to go into Altivec
registers. Mostly it was due to reload complications in the power8 time frame,
where we didn't have d-form addressing for the Altivec registers and
interference with the other DImode reload patterns, but also I felt I would
need to go through the main rs6000.md to look for the various DImode patterns
to see if they needed to be updated.  At some I hoped to extend this, so I
added the "wi" and "wj" constraints to be used whenever the mode is DImode.
"wi" is the constraint for the VSX register class DImode can go in
(i.e. currently FLOAT=5FREGS), and "wj" is the VSX register class DImode can go
in if we have 64-bit direct move.

The TFmode thing was a hack.  It was the only way I could see getting two
registers without a lot of hair.  Since TFmode is also restricted to FPR_REGS,
you could use %L on it.  When I was writing it, I really wished we had a reload
pattern to get more than one temporary, but we didn't, so I went for TFmode to
get 2 FPR registers.  Given the replacement pattern no longer uses a TFmode
temporary, it can go in any appropriate register.

If you are using DFmode, the right constraint is "wk" (appropriate register to
use with the double type when direct moves are available) or "ws" (appropriate
register to use for DFtype with VSX instructions).

We don't have a constraint for appropriate register class to use with SFmode
when direct moves are available, so I would suggest using "wy" for a register
class you can use ISA 2.07 (power8) ops (the float scalar ops in the upper
registers).  The "ww" constraint would be the appropriate register to use ISA
2.06 (power7) operations on SFmode.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797



Re: [RS6000] reload_vsx_from_gprsf splitter

2016-02-11 Thread Michael Meissner
On Thu, Feb 11, 2016 at 07:38:15PM +0100, Ulrich Weigand wrote:
> For the most part, this patch doesn't really change anything in the
> interaction with reload as far as I can see.  The changes introduced
> by the patch do make sense to me.  In particular, replacing the two
> patterns p8_mtvsrd_1 and p8_mtvsrd_2 used to fill high and low parts
> of a TFmode register pair with a single pattern p8_mtvsrd that just
> works on any DFmode register (leaving the split into high/low to the
> caller if necessary) seems to simplify things.
> 
> > > +  /* You might think that we could use op0 as one temp and a DF clobber
> > > + as the other, but you'd be wrong.  These secondary_reload patterns
> > > + don't check that the clobber is different to the destination, which
> > > + is probably a reload bug.  */
> 
> It's not a bug, it's deliberate behavior.  The reload registers allocated
> for secondary reload clobbers may overlap the destination, since in many
> cases you simply move the input to the reload register, and then the
> reload register to the destination (where the latter move can be a no-op
> if it is possible to allocate the reload register and the destination
> into the same physical register).  If you need two separate intermediate
> values, you need to allocate a secondary reload register of a larger
> mode (as is already done in the pattern).

This is one of the cases I wished the reload support had the ability to
allocate 2 scratch temporaries instead of 1.  As I said in my other message,
TFmode was a hack to get two registers to use.

> While this was not introduced by this patch, I'm a little bit concerned
> about the hard-coded use of REGNO here, which will crash if op0 at this
> point happens to be a SUBREG instead of a REG.  This is extremely unlikely
> at this point in reload, but not 100% impossible, e.g. if op0 for some
> reason is one of the "magic" registers like the stack pointer.
> 
> That's why it is in general better to use simplify_gen_subreg or one of
> gen_highpart/gen_lowpart, which will handle SUBREG correctly as well.
> I'm not sure why it matters whether this is "conceptually a separate
> value" as the comment argues ...

Good catch.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797



[PATCH] 19705 - -fno-branch-count-reg doesn't prevent decrement and branch instructions on a count register

2016-02-11 Thread Martin Sebor

The more than decennnial rtl-optimization/19705 - -fno-branch-count-reg
doesn't prevent decrement and branch instructions on a count register
points out that the documentation of the option leads one to expect
that it prevents the decrement and branch instruction from appearing
in the instruction stream.  This isn't the case  The option prevents
a dedicated pass from running that introduces such instructions, but
it doesn't prevent other passes from introducing it.  The attached
updates the documentation to clarify this.

Martin
PR rtl-optimization/19705 - -fno-branch-count-reg doesn't prevent decrement
	and branch instructions on a count register

gcc/ChangeLog:
2016-02-11  Martin Sebor  

	PR rtl-optimization/19705
	* doc/invoke.texi (Options That Control Optimization): Clarify
	-fno-branch-count-reg.

Index: gcc/doc/invoke.texi
===
--- gcc/doc/invoke.texi	(revision 233355)
+++ gcc/doc/invoke.texi	(working copy)
@@ -6513,11 +6513,14 @@ life-range analysis.  This option is eff
 
 @item -fno-branch-count-reg
 @opindex fno-branch-count-reg
-Do not use ``decrement and branch'' instructions on a count register,
-but instead generate a sequence of instructions that decrement a
-register, compare it against zero, then branch based upon the result.
-This option is only meaningful on architectures that support such
-instructions, which include x86, PowerPC, IA-64 and S/390.
+Avoid running a pass scanning for opportunities to use ``decrement and
+branch'' instructions on a count register instead of generating sequences
+of instructions that decrement a register, compare it against zero, and
+then branch based upon the result.  This option is only meaningful on
+architectures that support such instructions, which include x86, PowerPC,
+IA-64 and S/390.  Note that the @option{-fno-branch-count-reg} option
+doesn't remove the decrement and branch instructions from the generated
+instruction stream introduced by other optimization passes.
 
 Enabled by default at @option{-O1} and higher.
 


Re: [patch] c++/61198 backport to gcc-4.9

2016-02-11 Thread Jason Merrill

OK.

Jason


Re: [RS6000] reload_vsx_from_gprsf splitter

2016-02-11 Thread Alan Modra
On Thu, Feb 11, 2016 at 04:55:58PM -0500, Michael Meissner wrote:
> This is one of the cases I wished the reload support had the ability to
> allocate 2 scratch temporaries instead of 1.  As I said in my other message,
> TFmode was a hack to get two registers to use.

Another concern I had about this, besides using %L in asm output (what
forces TFmode to use just fprs?), is what happens when we're using
IEEE 128-bit floats?  In that case it looks like we'd get just one reg.

-- 
Alan Modra
Australia Development Lab, IBM


Re: [RFC] [PATCH] Add __array_size keyword

2016-02-11 Thread Joseph Myers
On Thu, 11 Feb 2016, Stuart Brady wrote:

> Documentation and test code are currently absent from the patch, but I

For proposed features, I find documentation and testcases of much more 
value than the rest of the implementation.  Critical issues to define and 
cover thoroughly in tests include the rules for when operands of sizeof 
are evaluated, as adapted appropriately for this keyword, and for when it 
returns various kinds of constants.  Is the rule for your keyword that the 
operand is evaluated, and the result not an integer constant, iff the 
operand is an array with a variable number of elements (as opposed to an 
array with a constant number of elements that themselves are 
variable-sized, for example)?  C11 6.5.3.4#2 (sizeof) would need testing, 
but so would 6.9#3 (rules about when declared identifiers need to be 
defined), and 6.6#6 and #8 (constant expressions) - and anything else in 
C11 that mentions sizeof.  Presumably this keyword can be applied to an 
array at function prototype scope whose size is explicitly or implicitly 
[*], though nothing useful can be done with the results, as with [*]?  
(Cf. gcc.dg/vla-5.c.)

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


Re: [RS6000] reload_vsx_from_gprsf splitter

2016-02-11 Thread Michael Meissner
On Fri, Feb 12, 2016 at 08:54:19AM +1030, Alan Modra wrote:
> On Thu, Feb 11, 2016 at 04:55:58PM -0500, Michael Meissner wrote:
> > This is one of the cases I wished the reload support had the ability to
> > allocate 2 scratch temporaries instead of 1.  As I said in my other message,
> > TFmode was a hack to get two registers to use.
> 
> Another concern I had about this, besides using %L in asm output (what
> forces TFmode to use just fprs?), is what happens when we're using
> IEEE 128-bit floats?  In that case it looks like we'd get just one reg.

The code in rs6000_hard_regno_mode_ok only allows TFmode (IBM extended double)
in GPRs and FPRs.  In theory, TFmode could go in VSX registers, just it hasn't
been done.

Good point that it breaks if the default long double (TFmode) type is IEEE
128-bit floating point.  We would need to have two patterns, one that uses
TFmode and one that uses IFmode.  I wrote the power8 direct move stuff before
going down the road of IEEE 128-bit floating point.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797



[PATCH] Spelling fixes - behaviour and neighbour

2016-02-11 Thread Jakub Jelinek
Hi!

While working on the -Waddress fix I've posted earlier today, I've noticed
that the C and C++ FE disagree on the spelling - C uses US english spelling,
while C++ uses UK english spelling.
This patch switches to US english spelling of these 2 words, in
diagnostics, documentation and comments as well.

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

2016-02-11  Jakub Jelinek  

* cgraph.c: Spelling fixes - behaviour -> behavior and
neighbour -> neighbor.
* target.def: Likewise.
* sel-sched.c: Likewise.
* config/mips/mips.c: Likewise.
* config/arc/arc.md: Likewise.
* config/arm/cortex-a57.md: Likewise.
* config/arm/arm.c: Likewise.
* config/arm/neon.md: Likewise.
* config/arm/arm-c.c: Likewise.
* config/vms/vms-c.c: Likewise.
* config/s390/s390.c: Likewise.
* config/i386/znver1.md: Likewise.
* config/i386/i386.c: Likewise.
* config/ia64/hpux-unix2003.h: Likewise.
* config/msp430/msp430.md: Likewise.
* config/rx/rx.c: Likewise.
* config/rx/rx.md: Likewise.
* config/aarch64/aarch64-simd.md: Likewise.
* config/aarch64/aarch64.c: Likewise.
* config/nvptx/nvptx.c: Likewise.
* config/bfin/bfin.c: Likewise.
* config/cris/cris.opt: Likewise.
* config/rs6000/rs6000.c: Likewise.
* target.h: Likewise.
* spellcheck.c: Likewise.
* ira-build.c: Likewise.
* tree-inline.c: Likewise.
* builtins.c: Likewise.
* lra-constraints.c: Likewise.
* explow.c: Likewise.
* hwint.h: Likewise.
* targhooks.c: Likewise.
* tree-vect-data-refs.c: Likewise.
* expr.c: Likewise.
* doc/tm.texi: Likewise.
* doc/extend.texi: Likewise.
* doc/install.texi: Likewise.
* doc/md.texi: Likewise.
* tree-ssa-tail-merge.c: Likewise.
* sched-int.h: Likewise.
* match.pd: Likewise.
* sched-ebb.c: Likewise.
* target.def (omit_struct_return_reg): Likewise.
* gimple-ssa-isolate-paths.c: Likewise.
(find_implicit_erroneous_behaviour): Renamed to...
(find_implicit_erroneous_behavior): ... this.
(find_explicit_erroneous_behaviour): Renamed to...
(find_explicit_erroneous_behavior): ... this.
(gimple_ssa_isolate_erroneous_paths): Adjust caller.
gcc/cp/
* error.c: Spelling fixes - behaviour -> behavior and
neighbour -> neighbor.
* decl.c: Likewise.
* typeck.c (cp_build_binary_op): Fix up behavior spelling in
diagnostics.
* init.c (build_delete): Likewise.
gcc/objc/
* objc-act.c: Spelling fixes - behaviour -> behavior and
neighbour -> neighbor.
* objc-map.h: Likewise.
gcc/go/
* gofrontend/lex.cc: Spelling fixes - behaviour -> behavior and
neighbour -> neighbor.
* gccgo.texi: Likewise.
gcc/ada/
* prj-tree.ads: Spelling fixes - behaviour -> behavior and
neighbour -> neighbor.
* prep.adb: Likewise.
* prj.ads: Likewise.
* prepcomp.adb: Likewise.
* g-socket.ads: Likewise.
* s-imgrea.adb: Likewise.
* a-calend.adb: Likewise.
* exp_disp.adb: Likewise.
* doc/gnat_ugn/gnat_utility_programs.rst: Likewise.
* g-socket.adb: Likewise.
* sem_ch12.adb: Likewise.
* terminals.c: Likewise.
gcc/testsuite/
* objc.dg/gnu-api-2-method.m: Spelling fixes - behaviour -> behavior
and neighbour -> neighbor.
* objc.dg/attributes/method-nonnull-1.m: Likewise.
* objc.dg/gnu-api-2-class-meta.m: Likewise.
* c-c++-common/Wvarargs.c: Likewise.
* c-c++-common/goacc/host_data-5.c: Likewise.
* obj-c++.dg/gnu-api-2-class-meta.mm: Likewise.
* obj-c++.dg/attributes/method-nonnull-1.mm: Likewise.
* obj-c++.dg/gnu-api-2-method.mm: Likewise.
* gcc.target/aarch64/pr60697.c: Likewise.
* gcc.target/aarch64/advsimd-intrinsics/vldX_lane.c: Likewise.
* gcc.target/aarch64/advsimd-intrinsics/vqshl.c: Likewise.
* gcc.target/aarch64/advsimd-intrinsics/vshuffle.inc: Likewise.
* gcc.target/aarch64/advsimd-intrinsics/vrshl.c: Likewise.
* gcc.target/aarch64/advsimd-intrinsics/vldX_dup.c: Likewise.
* gcc.target/aarch64/advsimd-intrinsics/vstX_lane.c: Likewise.
* gcc.target/aarch64/advsimd-intrinsics/vqrshl.c: Likewise.
* gcc.target/aarch64/advsimd-intrinsics/vldX.c: Likewise.
* gcc.target/aarch64/aapcs64/ice_2.c: Likewise.
* gcc.target/aarch64/aapcs64/test_23.c: Likewise.
* gcc.target/aarch64/vrnd_f64_1.c: Likewise.
* g++.dg/warn/Wconversion-real-integer-3.C: Likewise.
* g++.dg/lookup/koenig5.C: Likewise.
* g++.dg/ext/no-asm-2.C: Likewise.
* gfortran.dg/bounds_check_array_ctor_3.f90: Likewise.
* gfortran.dg/bound

Re: [Fortran, Patch] (RFC, Coarray) Implement TS18508's EVENTS

2016-02-11 Thread Alessandro Fanfarillo
Dear all,

in attachment the EVENT patch for gcc-5-branch directly back-ported
from the trunk.

Built and regtested on x86_64-pc-linux-gnu. I plan to commit this
patch this evening (Feb 12th, CET).

Cheers,

Alessandro

2015-12-17 17:19 GMT+01:00 Alessandro Fanfarillo :
> Great! Thanks.
>
> 2015-12-17 15:57 GMT+01:00 Steve Kargl :
>> On Thu, Dec 17, 2015 at 01:22:06PM +0100, Alessandro Fanfarillo wrote:
>>>
>>> I've noticed that this patch has been applied only on trunk and not on
>>> the gcc-5-branch. Is it a problem to include EVENTS in gcc-5?
>>>
>>
>> No problem.  When I applied the EVENTS patch to trunk,
>> the 5.3 release was being prepared.  I was going to
>> wait for a week or two after 5.3 came out, then apply
>> the patch.  Now that you have commit access, feel
>> free to back port the patch.  Rememer to post the
>> patch that you commit to both the fortran and gcc-patches
>> list.
>>
>> --
>> Steve
commit 9504c4c79accddeb6e2386c9bf60d651c3d8f627
Author: Alessandro Fanfarillo 
Date:   Thu Feb 11 11:24:53 2016 +

Events patch backported from gcc-trunk

diff --git ./gcc/fortran/check.c ./gcc/fortran/check.c
index 3196420..049a6fb 100644
--- ./gcc/fortran/check.c
+++ ./gcc/fortran/check.c
@@ -1157,6 +1157,59 @@ gfc_check_atomic_cas (gfc_expr *atom, gfc_expr *old, 
gfc_expr *compare,
   return true;
 }
 
+bool
+gfc_check_event_query (gfc_expr *event, gfc_expr *count, gfc_expr *stat)
+{
+  if (event->ts.type != BT_DERIVED
+  || event->ts.u.derived->from_intmod != INTMOD_ISO_FORTRAN_ENV
+  || event->ts.u.derived->intmod_sym_id != ISOFORTRAN_EVENT_TYPE)
+{
+  gfc_error ("EVENT argument at %L to the intrinsic EVENT_QUERY "
+"shall be of type EVENT_TYPE", &event->where);
+  return false;
+}
+
+  if (!scalar_check (event, 0))
+return false;
+
+  if (!gfc_check_vardef_context (count, false, false, false, NULL))
+{
+  gfc_error ("COUNT argument of the EVENT_QUERY intrinsic function at %L "
+"shall be definable", &count->where);
+  return false;
+}
+
+  if (!type_check (count, 1, BT_INTEGER))
+return false;
+
+  int i = gfc_validate_kind (BT_INTEGER, count->ts.kind, false);
+  int j = gfc_validate_kind (BT_INTEGER, gfc_default_integer_kind, false);
+
+  if (gfc_integer_kinds[i].range < gfc_integer_kinds[j].range)
+{
+  gfc_error ("COUNT argument of the EVENT_QUERY intrinsic function at %L "
+"shall have at least the range of the default integer",
+&count->where);
+  return false;
+}
+
+  if (stat != NULL)
+{
+  if (!type_check (stat, 2, BT_INTEGER))
+   return false;
+  if (!scalar_check (stat, 2))
+   return false;
+  if (!variable_check (stat, 2, false))
+   return false;
+
+  if (!gfc_notify_std (GFC_STD_F2008_TS, "STAT= argument to %s at %L",
+  gfc_current_intrinsic, &stat->where))
+   return false;
+}
+
+  return true;
+}
+
 
 bool
 gfc_check_atomic_fetch_op (gfc_expr *atom, gfc_expr *value, gfc_expr *old,
diff --git ./gcc/fortran/dump-parse-tree.c ./gcc/fortran/dump-parse-tree.c
index 83ecbaa..c886010 100644
--- ./gcc/fortran/dump-parse-tree.c
+++ ./gcc/fortran/dump-parse-tree.c
@@ -1659,6 +1659,33 @@ show_code_node (int level, gfc_code *c)
}
   break;
 
+case EXEC_EVENT_POST:
+case EXEC_EVENT_WAIT:
+  if (c->op == EXEC_EVENT_POST)
+   fputs ("EVENT POST ", dumpfile);
+  else
+   fputs ("EVENT WAIT ", dumpfile);
+
+  fputs ("event-variable=", dumpfile);
+  if (c->expr1 != NULL)
+   show_expr (c->expr1);
+  if (c->expr4 != NULL)
+   {
+ fputs (" until_count=", dumpfile);
+ show_expr (c->expr4);
+   }
+  if (c->expr2 != NULL)
+   {
+ fputs (" stat=", dumpfile);
+ show_expr (c->expr2);
+   }
+  if (c->expr3 != NULL)
+   {
+ fputs (" errmsg=", dumpfile);
+ show_expr (c->expr3);
+   }
+  break;
+
 case EXEC_LOCK:
 case EXEC_UNLOCK:
   if (c->op == EXEC_LOCK)
diff --git ./gcc/fortran/expr.c ./gcc/fortran/expr.c
index c90e823..2e74375 100644
--- ./gcc/fortran/expr.c
+++ ./gcc/fortran/expr.c
@@ -4864,6 +4864,19 @@ gfc_check_vardef_context (gfc_expr* e, bool pointer, 
bool alloc_obj,
   return false;
 }
 
+  /* TS18508, C702/C203.  */
+  if (!alloc_obj
+  && (attr.lock_comp
+ || (e->ts.type == BT_DERIVED
+ && e->ts.u.derived->from_intmod == INTMOD_ISO_FORTRAN_ENV
+ && e->ts.u.derived->intmod_sym_id == ISOFORTRAN_EVENT_TYPE)))
+{
+  if (context)
+   gfc_error ("LOCK_EVENT in variable definition context (%s) at %L",
+  context, &e->where);
+  return false;
+}
+
   /* INTENT(IN) dummy argument.  Check this, unless the object itself is the
  component of sub-component of a pointer; we need to distinguish
  assignment to a pointer component from pointer-assignment to a pointer
diff --git 

Re: [PATCH PR69052]Check if loop inv can be propagated into mem ref with additional addr expr canonicalization

2016-02-11 Thread Jeff Law

On 02/11/2016 10:59 AM, Bin.Cheng wrote:


Hi Jeff,
Thanks for detailed review.  I also think a generic canonical
interface for RTL is much better.  I will give it a try.  But with
high chance it's a next stage1 stuff.
That is, of course, fine.  However, if you do get something ready, I'd 
support using it within LICM for gcc-6, then using it in other places 
for gcc-7.


Jeff


[PATCH] Fix ICE when expanding incorrect shift counts (PR rtl-optimization/69764)

2016-02-11 Thread Jakub Jelinek
Hi!

When expanding shifts with invalid shift counts (negative or too large),
the shift count on the GIMPLE level is typically an int mode INTEGER_CST,
but when it is passed down through various layers up to
expand_binop_directly, we only have one known mode (other than operand
modes, but that is VOIDmode for CONST_INTs) - the mode of the first argument
(== result).  But, we treat it as if even the shift count has that mode,
and either keep it as is (if the expander has that mode for the shift
count), or convert_modes it to the mode of the operand.
If the CONST_INT is too large, we can have a problem though, because it
could be e.g result of expand_normal of SImode value originally, but
we then treat it as valid HImode or QImode CONST_INT, and so either crash
in convert_modes, or later on when dealing with the shift count, as it
might not be valid for the mode we are expecting.
As expand_shift_1 and expand_binop seem to use GEN_INT for the shift count
in lots of places, rather than say gen_int_mode, I think this needs to be
fixed up only at the low level - in expand_binop_directly, which this patch
does.  The common case, where the shift count is >= 0 and < bitsize,
are handled without need to call gen_int_mode.

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

2016-02-12  Jakub Jelinek  

PR rtl-optimization/69764
* optabs.c (expand_binop_directly): For shift_optab_p, if xop1
is CONST_INT above scalar int mode's GET_MODE_BITSIZE (mode),
force it into mode.

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

--- gcc/optabs.c.jj 2016-02-11 20:28:51.240492706 +0100
+++ gcc/optabs.c2016-02-12 00:11:58.951795368 +0100
@@ -1006,6 +1006,14 @@ expand_binop_directly (machine_mode mode
   xop0 = avoid_expensive_constant (xmode0, binoptab, 0, xop0, unsignedp);
   if (!shift_optab_p (binoptab))
 xop1 = avoid_expensive_constant (xmode1, binoptab, 1, xop1, unsignedp);
+  /* The mode of shift/rotate second operand is often different
+ from the mode of the operation, and for invalid shift counts xop1
+ might not be valid constant in mode, so the following convert_modes
+ might ICE on it.  Fix it up here.  */
+  else if (CONST_INT_P (xop1)
+  && SCALAR_INT_MODE_P (mode)
+  && UINTVAL (xop1) >= GET_MODE_BITSIZE (mode))
+xop1 = gen_int_mode (INTVAL (xop1), mode);
 
   /* In case the insn wants input operands in modes different from
  those of the actual operands, convert the operands.  It would
--- gcc/testsuite/c-c++-common/pr69764.c.jj 2016-02-12 00:00:54.950084697 
+0100
+++ gcc/testsuite/c-c++-common/pr69764.c2016-02-12 00:00:54.950084697 
+0100
@@ -0,0 +1,38 @@
+/* PR rtl-optimization/69764 */
+/* { dg-do compile { target int32plus } } */
+
+unsigned char
+fn1 (unsigned char a)
+{
+  return a >> ~6;  /* { dg-warning "right shift count is negative" } */
+}
+
+unsigned short
+fn2 (unsigned short a)
+{
+  return a >> ~6;  /* { dg-warning "right shift count is negative" } */
+}
+
+unsigned int
+fn3 (unsigned int a)
+{
+  return a >> ~6;  /* { dg-warning "right shift count is negative" } */
+}
+
+unsigned char
+fn4 (unsigned char a)
+{
+  return a >> 0xff03;  /* { dg-warning "right shift count >= width of type" } 
*/
+}
+
+unsigned short
+fn5 (unsigned short a)
+{
+  return a >> 0xff03;  /* { dg-warning "right shift count >= width of type" } 
*/
+}
+
+unsigned int
+fn6 (unsigned int a)
+{
+  return a >> 0xff03;  /* { dg-warning "right shift count >= width of type" } 
*/
+}

Jakub


Re: [Patch, Fortran] PR 69495: unused-label warning does not tell which flag triggered it

2016-02-11 Thread Janus Weil
ping!

2016-02-05 19:19 GMT+01:00 Janus Weil :
> Hi all,
>
> I have slightly updated the patch now to avoid string-breaking issues
> (even if it may not be a problem at all, as mentioned by Jospeh). Also
> I removed the questionable part in intrinsic.c that I was not sure
> about.
>
> This version of the patch should not be too far from obvious now. Ok for 
> trunk?
>
> Cheers,
> Janus
>
>
>
> 2016-02-03 23:27 GMT+01:00 Joseph Myers :
>> On Wed, 3 Feb 2016, Manfred Schwarb wrote:
>>
>>> There are 2 things with translation, and there is a third issue:
>>> - As you noticed, breaking things differently means translation has to be
>>>   done again.
>>> - Normally, each string is translated independently, and depending on the
>>>   language there may be lack of context (e.g. adjectives get different
>>> suffixes
>>>   depending on the noun).
>>
>> I believe gettext works fine with (compile-time) string constant
>> concatenation - that is, extracts the whole concatenated string for
>> translation, so these are non-issues.  What doesn't work includes:
>>
>> * Runtime concatenation of strings or otherwise putting English fragments
>> together at runtime.
>>
>> * String constant concatenation where one of the concatenated pieces comes
>> from a macro expansion.
>>
>> * The argument for translation being a conditional expression:
>>
>>   error (cond ? "message 1" : "message 2");
>>
>> (in this case, only one of the messages may be extracted for translation,
>> so you need to mark both of them up with appropriate macros such as G_).
>>
>> --
>> Joseph S. Myers
>> jos...@codesourcery.com


Re: [PATCH] Spelling fixes - behaviour and neighbour

2016-02-11 Thread Jeff Law

On 02/11/2016 03:57 PM, Jakub Jelinek wrote:

Hi!

While working on the -Waddress fix I've posted earlier today, I've noticed
that the C and C++ FE disagree on the spelling - C uses US english spelling,
while C++ uses UK english spelling.
This patch switches to US english spelling of these 2 words, in
diagnostics, documentation and comments as well.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Sigh.  I'm probably to blame for a lot of behaviour thingies.  I think 
at some point in the long past my editor got set to the UK English 
setting and it flagged behavior.  So I got used to using behaviour.


These days that's fixed, but muscle memory hasn't adjusted...




2016-02-11  Jakub Jelinek  

* cgraph.c: Spelling fixes - behaviour -> behavior and
neighbour -> neighbor.
* target.def: Likewise.
* sel-sched.c: Likewise.
* config/mips/mips.c: Likewise.
* config/arc/arc.md: Likewise.
* config/arm/cortex-a57.md: Likewise.
* config/arm/arm.c: Likewise.
* config/arm/neon.md: Likewise.
* config/arm/arm-c.c: Likewise.
* config/vms/vms-c.c: Likewise.
* config/s390/s390.c: Likewise.
* config/i386/znver1.md: Likewise.
* config/i386/i386.c: Likewise.
* config/ia64/hpux-unix2003.h: Likewise.
* config/msp430/msp430.md: Likewise.
* config/rx/rx.c: Likewise.
* config/rx/rx.md: Likewise.
* config/aarch64/aarch64-simd.md: Likewise.
* config/aarch64/aarch64.c: Likewise.
* config/nvptx/nvptx.c: Likewise.
* config/bfin/bfin.c: Likewise.
* config/cris/cris.opt: Likewise.
* config/rs6000/rs6000.c: Likewise.
* target.h: Likewise.
* spellcheck.c: Likewise.
* ira-build.c: Likewise.
* tree-inline.c: Likewise.
* builtins.c: Likewise.
* lra-constraints.c: Likewise.
* explow.c: Likewise.
* hwint.h: Likewise.
* targhooks.c: Likewise.
* tree-vect-data-refs.c: Likewise.
* expr.c: Likewise.
* doc/tm.texi: Likewise.
* doc/extend.texi: Likewise.
* doc/install.texi: Likewise.
* doc/md.texi: Likewise.
* tree-ssa-tail-merge.c: Likewise.
* sched-int.h: Likewise.
* match.pd: Likewise.
* sched-ebb.c: Likewise.
* target.def (omit_struct_return_reg): Likewise.
* gimple-ssa-isolate-paths.c: Likewise.
(find_implicit_erroneous_behaviour): Renamed to...
(find_implicit_erroneous_behavior): ... this.
(find_explicit_erroneous_behaviour): Renamed to...
(find_explicit_erroneous_behavior): ... this.
(gimple_ssa_isolate_erroneous_paths): Adjust caller.
gcc/cp/
* error.c: Spelling fixes - behaviour -> behavior and
neighbour -> neighbor.
* decl.c: Likewise.
* typeck.c (cp_build_binary_op): Fix up behavior spelling in
diagnostics.
* init.c (build_delete): Likewise.
gcc/objc/
* objc-act.c: Spelling fixes - behaviour -> behavior and
neighbour -> neighbor.
* objc-map.h: Likewise.
gcc/go/
* gofrontend/lex.cc: Spelling fixes - behaviour -> behavior and
neighbour -> neighbor.
* gccgo.texi: Likewise.
gcc/ada/
* prj-tree.ads: Spelling fixes - behaviour -> behavior and
neighbour -> neighbor.
* prep.adb: Likewise.
* prj.ads: Likewise.
* prepcomp.adb: Likewise.
* g-socket.ads: Likewise.
* s-imgrea.adb: Likewise.
* a-calend.adb: Likewise.
* exp_disp.adb: Likewise.
* doc/gnat_ugn/gnat_utility_programs.rst: Likewise.
* g-socket.adb: Likewise.
* sem_ch12.adb: Likewise.
* terminals.c: Likewise.
gcc/testsuite/
* objc.dg/gnu-api-2-method.m: Spelling fixes - behaviour -> behavior
and neighbour -> neighbor.
* objc.dg/attributes/method-nonnull-1.m: Likewise.
* objc.dg/gnu-api-2-class-meta.m: Likewise.
* c-c++-common/Wvarargs.c: Likewise.
* c-c++-common/goacc/host_data-5.c: Likewise.
* obj-c++.dg/gnu-api-2-class-meta.mm: Likewise.
* obj-c++.dg/attributes/method-nonnull-1.mm: Likewise.
* obj-c++.dg/gnu-api-2-method.mm: Likewise.
* gcc.target/aarch64/pr60697.c: Likewise.
* gcc.target/aarch64/advsimd-intrinsics/vldX_lane.c: Likewise.
* gcc.target/aarch64/advsimd-intrinsics/vqshl.c: Likewise.
* gcc.target/aarch64/advsimd-intrinsics/vshuffle.inc: Likewise.
* gcc.target/aarch64/advsimd-intrinsics/vrshl.c: Likewise.
* gcc.target/aarch64/advsimd-intrinsics/vldX_dup.c: Likewise.
* gcc.target/aarch64/advsimd-intrinsics/vstX_lane.c: Likewise.
* gcc.target/aarch64/advsimd-intrinsics/vqrshl.c: Likewise.
* gcc.target/aarch64/advsimd-intrinsics/vldX.c: Likewise.
* gcc.target/aarch64/aapcs64/ice_2.c: Likewise.
* gcc.target/aarch64/

[RFC] [P2] [PR tree-optimization/33562] Lowering more complex assignments.

2016-02-11 Thread Jeff Law


So I've never thought much about our Complex support and I don't have a 
lot of confidence in the coverage for our testsuite for these changes. 
So I'd really appreciate someone with more experience thinking about 
this issue for a bit.


I was looking at 33562 (P2), figuring it was DSE, which I wrote ~15 
years ago, I could probably get up-to-speed and so something about it 
without major surgery (and for Complex I'm pretty sure I can).


But while looking at the gimple code we handed off to DSE, it occurred 
to me that this would be easy to optimize if it were lowered.  Then, of 
course i remembered that we did lower complex stuff.


So this turned into a short debugging session in tree-complex.c.

The key statement in the test looks like



complex int t = 0

Where x is a complex object *and* has its address taken.  So the IL 
looks something like:


t = __complex__ (0, 0);



init_dont_simulate_again ignores this statement because the LHS is not 
an SSA_NAME (remember, t has had its address taken elsewhere in the 
sources).


So complex lowering ends up totally ignoring the function in question.

ISTM that we can and should still lower this code.  We don't want to set 
sim_again_p because the LHS is not in SSA form, so we don't really 
want/need to set up and track a lattice for this object.


So the first step was to figure out the conditions under which we ought 
to detect an assignment to/from an aggregate that is not in SSA_FORM.


I think we can do that with something like this in the GIMPLE_ASSIGN case:
 /* Simple assignments involving complex types where
 the RHS or LHS is addressable should be lowered, but
 do not inherently trigger resimulation.  */
  if (TREE_CODE (TREE_TYPE (gimple_assign_lhs (stmt)))
  == COMPLEX_TYPE)
saw_a_complex_op = true;


Essentially anytime we see a simple assignment (which would include 
initialization) we go ahead and let the complex lowering pass run, but 
we don't set sim_again_p.



Then we need to actually lower the construct.  expand_complex_move has 
99% of what we need.   If we take the code from this clause:


   else if (rhs && TREE_CODE (rhs) == SSA_NAME && !TREE_SIDE_EFFECTS (lhs))
[ ... ]

Refactor it into its own function and call it when this condition is true:

+  /* Assignment to/from a complex object that is addressable.  */
+  else if (TREE_CODE (lhs) == VAR_DECL
+  && TREE_CODE (TREE_TYPE (lhs)) == COMPLEX_TYPE
+  && (TREE_CODE (rhs) == VAR_DECL
+  || TREE_CODE (rhs) == COMPLEX_CST
+  || TREE_CODE (rhs) == COMPLEX_EXPR)
+  && TREE_CODE (TREE_TYPE (rhs)) == COMPLEX_TYPE)

Then complex-4.c and complex-5.c both work.  A variant of complex-4.c 
that I hacked up were we pass in a complex int, and assign that to a 
local addressable complex int gets lowered (and better optimized) as well.


So what am I missing here?  Is there any kind of aliasing issues I need 
to be aware of?  Any other dragons lurking?

jeff




diff --git a/gcc/tree-complex.c b/gcc/tree-complex.c
index d781a8a..64346a5 100644
--- a/gcc/tree-complex.c
+++ b/gcc/tree-complex.c
@@ -240,6 +240,14 @@ init_dont_simulate_again (void)
op0 = gimple_assign_rhs1 (stmt);
  if (gimple_num_ops (stmt) > 2)
op1 = gimple_assign_rhs2 (stmt);
+
+ /* Simple assignments involving complex types where
+the RHS or LHS is addressable should be lowered, but
+do not inherently trigger resimulation.  */
+ if (TREE_CODE (TREE_TYPE (gimple_assign_lhs (stmt)))
+ == COMPLEX_TYPE)
+   saw_a_complex_op = true;
+
  break;
 
case GIMPLE_COND:
@@ -777,6 +785,67 @@ update_phi_components (basic_block bb)
 }
 }
 
+/* Extract the real and imag parts from RHS of the statement at GSI
+   into R and I respectively.
+
+   This wouldn't be necessary except that extracting these
+   components from a COMPLEX_EXPR is different from everything
+   else.  */
+
+static void
+extract_real_and_imag_component (gimple_stmt_iterator *gsi, tree *r, tree *i)
+{
+  gimple *stmt = gsi_stmt (*gsi);
+  if (gimple_assign_rhs_code (stmt) == COMPLEX_EXPR)
+{
+  *r = gimple_assign_rhs1 (stmt);
+  *i = gimple_assign_rhs2 (stmt);
+}
+  else
+{
+  tree tmp = gimple_assign_rhs1 (stmt);
+  *r = extract_component (gsi, tmp, 0, true);
+  *i = extract_component (gsi, tmp, 1, true);
+}
+}
+
+/* Helper for expand_move_complex.  */
+
+static void
+expand_complex_move_1 (gimple_stmt_iterator *gsi, gimple *stmt,
+  tree lhs, tree inner_type)
+{
+  tree x, r, i;
+  gimple *t;
+  location_t loc = gimple_location (stmt);
+
+  extract_real_and_imag_component (gsi, &r, &i);
+  x = build1 (REALPART_EXPR, inner_type, unshare_expr (lhs));
+  t = gimple_build_assign (x, r);
+  gimple_set_location (t, loc);
+  gsi_insert_bef

Re: [PATCH], PR 68404 patch #2 (disable power8/power9 fusion on PowerPC)

2016-02-11 Thread David Edelsohn
On Wed, Feb 10, 2016 at 2:46 PM, Jakub Jelinek  wrote:
> On Wed, Feb 10, 2016 at 05:42:17PM -0500, Michael Meissner wrote:
>> This patch disables -mcpu=power8/-mtune=power8 from setting -mpower8-fusion 
>> and
>> -mcpu=power9/-mtune=power9 from setting -mpower9-fusion.  I will look at the
>> earlyclobber that Bernd Schmidt mentioned, but for now it may be safest to 
>> just
>> disable it for GCC 6.0.
>>
>> I built it on a little endian power8 system, and there were no regressions.  
>> Is
>> it ok to install?
>
> Doesn't this mean the bug is still there, just not enabled unless
> -mpower[89]-fusion (ok, perhaps mitigated by the previous workaround patch)?
> Wouldn't it be better to just forcefully clear the options (and thus ignore
> -them) for the time being if they are known to be broken?
>
>> [gcc]
>> 2016-02-10  Michael Meissner  
>>
>>   PR target/68404
>>   * config/rs6000/predicates.md (fusion_gpr_addis): Revert
>>   2016-02-09 change.
>>
>>   * config/rs6000/rs6000-cpus.def (ISA_2_7_MASKS_SERVER): Do not set
>>   power8/power9 fusion by default.
>>   (ISA_3_0_MASKS_SERVER): Likewise.
>>
>>   * config/rs6000/rs6000.c (rs6000_option_override_internal): Remove
>>   code setting -mpower8-fusion if -mtune=power8 and -mpower9-fusion
>>   if -mtune=power9.
>>
>>   * doc/invoke.texi (RS/6000 and PowerPC Options): Document that
>>   -mpower8-fusion and -mpower9-fusion are not set by default.
>>
>> [gcc/testsuites]
>> 2016-02-10  Michael Meissner  
>>
>>   PR target/68404
>>   * gcc.target/powerpc/fusion.c: Do not assume that -mtune=power8
>>   sets -mpower8-fusion or -mtune=power9 sets -mpower9-fusion.
>>   * gcc.target/powerpc/fusion2.c: Likewise.
>>   * gcc.target/powerpc/fusion3.c: Likewise.

Because of the more recent patches that should fix the cause of this
failure, this set of patches now are moot.

- David


Re: [PATCH], PR 68404 patch #3 (fix earlyclobber problem on power8 fusion)

2016-02-11 Thread David Edelsohn
On Thu, Feb 11, 2016 at 1:43 PM, Michael Meissner
 wrote:
> After looking at Bernd Schmidt and Jakub Jelinek's suggestions, I came to
> conclusion that earlyclobber was not needed in this case, and I removed it.  I
> bootstrapped the compiler using profiledbootstrap and lto options and it
> succeeded build and running make check.  Just to be sure, I also did a
> profiledbootstrap with LTO and -O3 and it built fine.  Is it ok to install
> these patches?
>
> I decided to keep the changes to the testsuite explicitly passing the fusion
> switches, rather than letting -mtune=power8/power9 set them, but I can be
> persuaded to restore the 3 tests to the way they were before February 9th.
>
> [gcc]
> 2016-02-11  Michael Meissner  
>
> PR target/68404
> * config/rs6000/predicates.md (fusion_gpr_addis): Revert
> 2016-02-09 change.
>
> * config/rs6000/rs6000.md (fusion_gpr_load_): Remove
> earlyclobber from target.  Use wF constraint for fused memory
> address.
> (fusion_gpr___load): Likewise.
>
> [gcc/testsuites]
> 2016-02-11  Michael Meissner  
>
> PR target/68404
> * gcc.target/powerpc/fusion.c: Do not assume that -mtune=power8
> sets -mpower8-fusion or -mtune=power9 sets -mpower9-fusion.
> * gcc.target/powerpc/fusion2.c: Likewise.
> * gcc.target/powerpc/fusion3.c: Likewise.
>
> Since gcc 5.0 also has the earlyclobber in the pattern, I would like to apply
> the same change to gcc 5.x (after testing of course), even though we haven't
> yet run into the problem with GCC 5.x.  Is this ok as well?

This is okay for trunk and GCC 5 branch.

Did you test the patch with the first patch reverted?  The first patch
also was correct and fixed a problem, but it also allows this
underlying bug to appear more prominently.  I want to ensure that the
patch was compared with a version of the compiler that elicited the
failure symptoms.

Thanks, David


Re: [PATCH], PR 68404 patch #3 (fix earlyclobber problem on power8 fusion)

2016-02-11 Thread Michael Meissner
On Thu, Feb 11, 2016 at 04:14:59PM -0800, David Edelsohn wrote:
> On Thu, Feb 11, 2016 at 1:43 PM, Michael Meissner
>  wrote:
> > After looking at Bernd Schmidt and Jakub Jelinek's suggestions, I came to
> > conclusion that earlyclobber was not needed in this case, and I removed it. 
> >  I
> > bootstrapped the compiler using profiledbootstrap and lto options and it
> > succeeded build and running make check.  Just to be sure, I also did a
> > profiledbootstrap with LTO and -O3 and it built fine.  Is it ok to install
> > these patches?
> >
> > I decided to keep the changes to the testsuite explicitly passing the fusion
> > switches, rather than letting -mtune=power8/power9 set them, but I can be
> > persuaded to restore the 3 tests to the way they were before February 9th.
> >
> > [gcc]
> > 2016-02-11  Michael Meissner  
> >
> > PR target/68404
> > * config/rs6000/predicates.md (fusion_gpr_addis): Revert
> > 2016-02-09 change.
> >
> > * config/rs6000/rs6000.md (fusion_gpr_load_): Remove
> > earlyclobber from target.  Use wF constraint for fused memory
> > address.
> > (fusion_gpr___load): Likewise.
> >
> > [gcc/testsuites]
> > 2016-02-11  Michael Meissner  
> >
> > PR target/68404
> > * gcc.target/powerpc/fusion.c: Do not assume that -mtune=power8
> > sets -mpower8-fusion or -mtune=power9 sets -mpower9-fusion.
> > * gcc.target/powerpc/fusion2.c: Likewise.
> > * gcc.target/powerpc/fusion3.c: Likewise.
> >
> > Since gcc 5.0 also has the earlyclobber in the pattern, I would like to 
> > apply
> > the same change to gcc 5.x (after testing of course), even though we haven't
> > yet run into the problem with GCC 5.x.  Is this ok as well?
> 
> This is okay for trunk and GCC 5 branch.
> 
> Did you test the patch with the first patch reverted?  The first patch
> also was correct and fixed a problem, but it also allows this
> underlying bug to appear more prominently.  I want to ensure that the
> patch was compared with a version of the compiler that elicited the
> failure symptoms.

The patch to predicates.md reverts the original change that I made on February
9th.  I did sync up the trunk to a newer revision, and I can go through a build
without the rs6000.md patch to show that the rs6000.md patch is the one that
fixes the problem if you prefer.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797



Re: [RS6000] reload_vsx_from_gprsf splitter

2016-02-11 Thread David Edelsohn
On Thu, Feb 11, 2016 at 10:38 AM, Ulrich Weigand  wrote:
> David Edelsohn wrote:
>> On Thu, Feb 11, 2016 at 6:04 AM, Alan Modra  wrote:
>> > This is PR68973 part 2, the failure of a boost test, caused by a
>> > splitter emitting an invalid move in reload_vsx_from_gprsf:
>> >   emit_move_insn (op0_di, op2);
>> >
>> > op0 can be any vsx reg, but the mtvsrd destination constraint in
>> > movdi_internal64 is "wj", which only allows fp regs.  I'm not sure why
>> > we have that constraint so left movdi_internal64 alone and used a
>> > special p8_mtvsrd instead, similar to p8_mtvsrd_1 and p8_mtvsrd_2 used
>> > by reload_vsx_from_gpr.  When looking at those, I noticed we're
>> > restricted to fprs there too so fixed that as well.  (We can't use %L
>> > in asm operands that must accept vsx.)
>>
>> Michael, please review the "wj" constraint in these patterns.
>>
>> Alan, the explanation says that it uses a special p8_mtvsrd similar to
>> p8_mtvsrd_[12], but does not explain why the two similar patterns are
>> removed.  The incorrect use of %L implies those patterns, but the
>> change is to p8_xxpermdi_ that is not mentioned in the
>> ChangeLog.
>>
>> I also would appreciate Uli's comments on this direction because of
>> his reload expertise.
>
> For the most part, this patch doesn't really change anything in the
> interaction with reload as far as I can see.  The changes introduced
> by the patch do make sense to me.  In particular, replacing the two
> patterns p8_mtvsrd_1 and p8_mtvsrd_2 used to fill high and low parts
> of a TFmode register pair with a single pattern p8_mtvsrd that just
> works on any DFmode register (leaving the split into high/low to the
> caller if necessary) seems to simplify things.
>
>> > +  /* You might think that we could use op0 as one temp and a DF clobber
>> > + as the other, but you'd be wrong.  These secondary_reload patterns
>> > + don't check that the clobber is different to the destination, which
>> > + is probably a reload bug.  */
>
> It's not a bug, it's deliberate behavior.  The reload registers allocated
> for secondary reload clobbers may overlap the destination, since in many
> cases you simply move the input to the reload register, and then the
> reload register to the destination (where the latter move can be a no-op
> if it is possible to allocate the reload register and the destination
> into the same physical register).  If you need two separate intermediate
> values, you need to allocate a secondary reload register of a larger
> mode (as is already done in the pattern).
>
>> >/* Also use the destination register to hold the unconverted DImode 
>> > value.
>> >   This is conceptually a separate value from OP0, so we use gen_rtx_REG
>> >   rather than simplify_gen_subreg.  */
>> > -  rtx op0_di = gen_rtx_REG (DImode, REGNO (op0));
>> > +  rtx op0_df = gen_rtx_REG (DFmode, REGNO (op0));
>> > +  rtx op0_v4sf = gen_rtx_REG (V4SFmode, REGNO (op0));
>
> While this was not introduced by this patch, I'm a little bit concerned
> about the hard-coded use of REGNO here, which will crash if op0 at this
> point happens to be a SUBREG instead of a REG.  This is extremely unlikely
> at this point in reload, but not 100% impossible, e.g. if op0 for some
> reason is one of the "magic" registers like the stack pointer.
>
> That's why it is in general better to use simplify_gen_subreg or one of
> gen_highpart/gen_lowpart, which will handle SUBREG correctly as well.
> I'm not sure why it matters whether this is "conceptually a separate
> value" as the comment argues ...
>
>> >/* Move SF value to upper 32-bits for xscvspdpn.  */
>> >emit_insn (gen_ashldi3 (op2, op1_di, GEN_INT (32)));
>> > -  emit_move_insn (op0_di, op2);
>> > -  emit_insn (gen_vsx_xscvspdpn_directmove (op0, op0_di));
>> > +  emit_insn (gen_p8_mtvsrd (op0_df, op2));
>> > +  emit_insn (gen_vsx_xscvspdpn (op0_df, op0_v4sf));
>
> The sequence of modes used for op0 here is a bit weird.  First,
> op0 is loaded as DFmode by mtvsrd, then it is silently re-
> interpreted as V4SFmode when used as input to xscvspdpn, which
> gets a DFmode output that is again silently re-interpreted as
> SFmode.
>
> This isn't really wrong as such, just maybe a bit confusing.
> Maybe instead have p8_mtvsrd use DImode as output (instead of
> DFmode), which shouldn't be any harder to use in the
> reload_vsx_from_gpr splitter, and keep using the
> vsx_xscvspdpn_directmove pattern?
>
> [ This of course reinforces the question why we have p8_mtvsrd
> in the first place, instead of just allowing this use directly
> in movdi_internal64 itself.  ]

Good question: is p8_mtvsrd really necessary if the movdi_internal64
is updated to use the correct constraints?

The patch definitely is going in the right direction.  Can we remove
more unnecessary bits?

Thanks, David


Re: [patch,libgfortran] Bug 69668 - [4.9/5/6 Regression] Error reading namelist opened with DELIM='NONE'

2016-02-11 Thread Jerry DeLisle
On 02/11/2016 10:38 AM, Janne Blomqvist wrote:
> On Wed, Feb 10, 2016 at 4:45 AM, Jerry DeLisle  wrote:
>> The attached patch reverts the guilty code. We were trying to honor 
>> delim=NONE
>> on namelist reads which is invalid.
>>
>> Test cases updated. Regression tested on x86-64.
>>
>> OK for trunk and back port in about a week?
> 
> For namelist_38.90, I think it would be better to open it with
> status="scratch" as it was before, so that we don't leave test files
> around in case the test fails.
> 
> And in the ChangeLog entry, you have misspelled the testcase names
> (naMelist, not naNelist).
> 
> Ok with these changes.
> 
Thanks Janne, will do.

Jerry


Re: [PATCH] Fix ICE when expanding incorrect shift counts (PR rtl-optimization/69764)

2016-02-11 Thread Bernd Schmidt

On 02/12/2016 12:26 AM, Jakub Jelinek wrote:

When expanding shifts with invalid shift counts (negative or too large),
the shift count on the GIMPLE level is typically an int mode INTEGER_CST,
but when it is passed down through various layers up to
expand_binop_directly, we only have one known mode (other than operand
modes, but that is VOIDmode for CONST_INTs) - the mode of the first argument
(== result).  But, we treat it as if even the shift count has that mode,
and either keep it as is (if the expander has that mode for the shift
count), or convert_modes it to the mode of the operand.
If the CONST_INT is too large, we can have a problem though, because it
could be e.g result of expand_normal of SImode value originally, but
we then treat it as valid HImode or QImode CONST_INT, and so either crash
in convert_modes, or later on when dealing with the shift count, as it
might not be valid for the mode we are expecting.
As expand_shift_1 and expand_binop seem to use GEN_INT for the shift count
in lots of places, rather than say gen_int_mode, I think this needs to be
fixed up only at the low level - in expand_binop_directly, which this patch
does.  The common case, where the shift count is >= 0 and < bitsize,
are handled without need to call gen_int_mode.


Hmm, I'm finding this explanation somewhat confusing.

Isn't the problem just that shifts are unusual as binops, in that the 
two operands can have different modes? We have the appropriate mode for 
the shift count in xmode1, so I think we should just always rebuild a 
CONST_INT in that mode. As in,


  else if (CONST_INT_P (xop1))
/* Shifts can have different modes for the shift count, and the caller
   might not have taken this into account when generating an 
integer.  */

xop1 = gen_int_mode (INTVAL (xop1), xmode1);

Ok with that change if it passes.


Bernd


Fix PR69752, insn with REG_INC being removed as equiv_init insn

2016-02-11 Thread Bernd Schmidt

This seems fairly straightforward:

(insn 213 455 216 6 (set (reg:SI 266)
(mem/u/c:SI (post_inc:SI (reg/f:SI 267)) [4  S4 A32])) 748 
{*thumb1_movsi_insn}

 (expr_list:REG_EQUAL (const_int -1044200508 [0xc1c2c3c4])
(expr_list:REG_INC (reg/f:SI 267)
(nil

We don't notice that the SET_SRC has a side effect, record the insn as 
an equivalencing one, and later remove it because we replaced the reg 
with the constant everywhere. Thus, the increment doesn't take place.


Fixed as follows. Bootstrapped and tested on x86_64-linux. Also compared 
before/after dumps for the testcase with arm-elf. Ok?



Bernd
	PR rtl-optimization/69752
	* ira.c (update_equiv_regs): When looking for more than a single SET,
	also take other side effects into account.

Index: gcc/ira.c
===
--- gcc/ira.c	(revision 233364)
+++ gcc/ira.c	(working copy)
@@ -3392,7 +3392,8 @@ update_equiv_regs (void)
 
 	  /* If this insn contains more (or less) than a single SET,
 	 only mark all destinations as having no known equivalence.  */
-	  if (set == NULL_RTX)
+	  if (set == NULL_RTX
+	  || side_effects_p (SET_SRC (set)))
 	{
 	  note_stores (PATTERN (insn), no_equiv, NULL);
 	  continue;



Re: [PATCH, reload] PRE_INC with invalid hard reg

2016-02-11 Thread Alan Modra
On Thu, Feb 11, 2016 at 03:29:05PM +0100, Bernd Schmidt wrote:
> On 02/11/2016 10:45 AM, Alan Modra wrote:
> 
> >Due to uses elsewhere in vsx instructions, reload chooses to put
> >psuedo 185 in fr31, which can't be used as a base register in the
> >following:
> 
> What code exactly makes the choice of fr31? I assume this is in
> reg_renumber, so it's IRA and not reload that's making that decision?

Yes, sorry, I shouldn't have said reload chooses the reg.

> > PR target/68973
> > * reloads.c (find_reloads_address_1): For pre/post-inc/dec
> > with an invalid hard reg, reload just the reg not the entire
> > pre/post-inc/dec address expression.
> 
> Hmm, you're patching tricky code.

Thanks for being willing to review!  :)

> I'm not sure yet whether this is right or
> not. More reload dumps might help if you have them; I'll Cc myself on the
> PR.

I've attached rtl dumps to the PR.

> My gut feeling is that we want to reload the inner reg before entering this
> block of code,

Yes, my first quick hack did just that, then I noticed there was
existing code to reload the inner reg..

> with a new test for SECONDARY_MEMORY_NEEDED alongside the
> existing block that already sets reloaded_inner_of_autoinc.

I don't understand this comment.  If we're pushing a reload of the
inner reg, then the SECONDARY_MEMORY_NEEDED code in push_reload will
fire.  Why then should there be any need to do anything special in
find_reloads_address_1 regarding secondary memory?

-- 
Alan Modra
Australia Development Lab, IBM


[PATCH] Add PR c++/11814 test case to testsuite

2016-02-11 Thread Patrick Palka
Hi Jason,

Your recent fix for PR c++/10200 seems to have fixed this longstanding
PR too.  Should I add its test case to the testsuite and close the PR?

gcc/testsuite/ChangeLog:

PR c++/11814
* g++.dg/lookup/template4.C: New test.
---
 gcc/testsuite/g++.dg/lookup/template4.C | 16 
 1 file changed, 16 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/lookup/template4.C

diff --git a/gcc/testsuite/g++.dg/lookup/template4.C 
b/gcc/testsuite/g++.dg/lookup/template4.C
new file mode 100644
index 000..53030d6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lookup/template4.C
@@ -0,0 +1,16 @@
+// PR c++/11814
+
+template  struct A
+{
+template  void foo();
+};
+
+template  struct B
+{
+template  void foo()
+{
+A* p;
+p->foo();  // { dg-error "" }
+}
+};
+
-- 
2.7.1.257.g925a48d



[PATCH] PR driver/69779: fix bogus cleanup code used by libgccjit affecting s390x

2016-02-11 Thread David Malcolm
In r227188 I introduced driver::finalize () which resets
all state within gcc.c, allowing the driver code to embedded
inside libgccjit and run repeatedly in-process.

Running this on s390x revealed a bug in this cleanup:
I was cleaning up "specs" via:
XDELETEVEC (specs);
and this was crashing on s390x with:
  free(): invalid pointer: 0x03fffdf30568 ***

Closer investigation revealed that specs == static_specs,
a statically allocated array.

What's happening is that set_specs sets "specs" to static_specs
the first time it's called, and any calls to set_specs () with
a name that isn't in static_specs cause a new spec_list node
to be dynamically-allocated and put at the front of the list; "specs"
then equals that.

All of my testing had been on x86_64, which inserts exactly one spec
with a name not in the static array, hence the:
XDELETEVEC (specs);
happened to work.

On s390x, the only names in the "specs" file are those in the static
array, so specs == static_specs, and the bogus cleanup code attempts
to free a statically-allocated array.

The following patch reworks this part of the cleanup, walking any list
of specs, freeing the dynamically-allocated nodes until the
statically-allocated ones are reached.

driver::finalize is only called by libgccjit, not by the main driver
program, so this should be relatively low-risk.

Verified directly by running under valgrind on x86_64 and s390x.

With the patch, jit.sum on s390x-ibm-linux-gnu (RHEL 7) goes from:
  # of expected passes1799
  # of unexpected failures61
  # of unresolved testcases   2
to:
  # of expected passes8514

Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.

OK for trunk?

gcc/ChangeLog:
PR driver/69779
* gcc.c (driver::finalize): Fix cleanup of "specs".
---
 gcc/gcc.c | 16 ++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/gcc/gcc.c b/gcc/gcc.c
index 683b30f..ba1ee41 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -9898,8 +9898,20 @@ driver::finalize ()
   multilib_os_dir = 0;
   multiarch_dir = 0;
 
-  XDELETEVEC (specs);
-  specs = 0;
+  /* Free any specs dynamically-allocated by set_spec.
+ These will be at the head of the list, before the
+ statically-allocated ones.  */
+  if (specs)
+{
+  while (specs != static_specs)
+   {
+ spec_list *next = specs->next;
+ free (const_cast  (specs->name));
+ XDELETE (specs);
+ specs = next;
+   }
+  specs = 0;
+}
   for (unsigned i = 0; i < ARRAY_SIZE (static_specs); i++)
 {
   spec_list *sl = &static_specs[i];
-- 
1.8.5.3



Re: [PR66726] Fixe regression caused by Factor conversion out of COND_EXPR

2016-02-11 Thread Markus Trippelsdorf
On 2016.02.08 at 09:49 -0700, Jeff Law wrote:
> On 01/18/2016 08:52 PM, Kugan wrote:
> >
> >2016-01-19  Kugan Vivekanandarajah  
> >
> > PR middle-end/66726
> > * tree-ssa-reassoc.c (optimize_range_tests): Handle tcc_compare stmt
> > whose result is used in PHI.
> > (maybe_optimize_range_tests): Likewise.
> > (final_range_test_p): Lokweise.
> >
> Otherwise this looks OK for the trunk.  It really hasn't changed much since
> the version from July.  And while the PR is not marked as such, this is a
> code quality regression fix for targets with a BRANCH_COST > 1.

This causes LTO/PGO bootstrap on ppc64le to ICE all over the place:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69781

-- 
Markus


Re: [PR66726] Fixe regression caused by Factor conversion out of COND_EXPR

2016-02-11 Thread kugan



On 12/02/16 17:18, Markus Trippelsdorf wrote:

On 2016.02.08 at 09:49 -0700, Jeff Law wrote:

On 01/18/2016 08:52 PM, Kugan wrote:


2016-01-19  Kugan Vivekanandarajah  

PR middle-end/66726
* tree-ssa-reassoc.c (optimize_range_tests): Handle tcc_compare stmt
whose result is used in PHI.
(maybe_optimize_range_tests): Likewise.
(final_range_test_p): Lokweise.


Otherwise this looks OK for the trunk.  It really hasn't changed much since
the version from July.  And while the PR is not marked as such, this is a
code quality regression fix for targets with a BRANCH_COST > 1.


This causes LTO/PGO bootstrap on ppc64le to ICE all over the place:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69781



Sorry for the breakage. I will revert the patch while I investigate this.

Thanks,
Kugan


Re: AW: AW: Wonly-top-basic-asm

2016-02-11 Thread David Wohlferd

why not simply -Wbasic-asm ?


Since both you and Bernd favor this shorter name, I have changed it.


Indentation wrong here. The whole block must be indented by 2 spaces.


Fixed.


Comments should end with dot space space */


Fixed.


the DECL_ATTRIBUTES should be at the same column as the "naked".


Fixed.


Comments should end with dot space space */


Fixed.


the DECL_ATTRIBUTES should be at the same column as the "naked".


Fixed.


C++, isn't it always upper case?


Fixed.


ChangeLog lines begin with TAB.


Hmm.  Thunderbird changed them to spaces.  I've tried something 
different this time.  Hopefully fixed.



Please split the ChangeLog
use relative file names.


Fixed (I think).


Please add the function name where you changed in brackets.


Fixed. == ChangeLog: 
2016-02-11 David Wohlferd  * doc/extend.texi: Doc 
basic asm behavior and new -Wbasic-asm option. * doc/invoke.texi: Doc 
new -Wbasic-asm option. ChangeLog: 2016-02-11 David Wohlferd 
 * c.opt: Define -Wbasic-asm. ChangeLog: 
2016-02-11 David Wohlferd  * c-parser.c 
(c_parser_asm_statement): Implement -Wbasic-asm for C. ChangeLog: 
2016-02-11 David Wohlferd  * parser.c 
(cp_parser_asm_definition): Implement -Wbasic-asm for C++. ChangeLog: 
2016-02-11 David Wohlferd  * 
c-c++-common/Wbasic-asm.c: New tests for -Wbasic-asm. * 
c-c++-common/Wbasic-asm-2.c: Ditto. New patch is attached. Note that 
Bernd Schmidt and I are still discussing changes to the docs (see next 
message). dw


Index: gcc/c-family/c.opt
===
--- gcc/c-family/c.opt	(revision 233367)
+++ gcc/c-family/c.opt	(working copy)
@@ -585,6 +585,10 @@
 C++ ObjC++ Var(warn_namespaces) Warning
 Warn on namespace definition.
 
+Wbasic-asm
+C ObjC ObjC++ C++ Var(warn_basic_asm) Warning
+Warn on unsafe uses of basic asm.
+
 Wsized-deallocation
 C++ ObjC++ Var(warn_sized_deallocation) Warning EnabledBy(Wextra)
 Warn about missing sized deallocation functions.
Index: gcc/c/c-parser.c
===
--- gcc/c/c-parser.c	(revision 233367)
+++ gcc/c/c-parser.c	(working copy)
@@ -5978,8 +5978,19 @@
   labels = NULL_TREE;
 
   if (c_parser_next_token_is (parser, CPP_CLOSE_PAREN) && !is_goto)
-goto done_asm;
+{
+  /* Warn on basic asm used inside of functions,
+	 except when in naked functions.  Also allow asm ("").  */
+  if (warn_basic_asm && TREE_STRING_LENGTH (str) != 1)
+	if (lookup_attribute ("naked",
+			  DECL_ATTRIBUTES (current_function_decl))
+	== NULL_TREE)
+	  warning_at (asm_loc, OPT_Wbasic_asm,
+		  "asm statement in function does not use extended syntax");
 
+  goto done_asm;
+}
+
   /* Parse each colon-delimited section of operands.  */
   nsections = 3 + is_goto;
   for (section = 0; section < nsections; ++section)
Index: gcc/cp/parser.c
===
--- gcc/cp/parser.c	(revision 233367)
+++ gcc/cp/parser.c	(working copy)
@@ -18041,6 +18041,8 @@
   bool goto_p = false;
   required_token missing = RT_NONE;
 
+  location_t asm_loc = cp_lexer_peek_token (parser->lexer)->location;
+
   /* Look for the `asm' keyword.  */
   cp_parser_require_keyword (parser, RID_ASM, RT_ASM);
 
@@ -18199,6 +18201,17 @@
 	  /* If the extended syntax was not used, mark the ASM_EXPR.  */
 	  if (!extended_p)
 	{
+	  /* Warn on basic asm used inside of functions,
+		 EXCEPT when in naked functions.  Also allow asm ("").  */
+	  if (warn_basic_asm
+		  && TREE_STRING_LENGTH (string) != 1)
+		if (lookup_attribute ("naked",
+  DECL_ATTRIBUTES (current_function_decl))
+		== NULL_TREE)
+		  warning_at (asm_loc, OPT_Wbasic_asm,
+			  "asm statement in function does not use extended"
+			  " syntax");
+
 	  tree temp = asm_stmt;
 	  if (TREE_CODE (temp) == CLEANUP_POINT_EXPR)
 		temp = TREE_OPERAND (temp, 0);
Index: gcc/doc/extend.texi
===
--- gcc/doc/extend.texi	(revision 233367)
+++ gcc/doc/extend.texi	(working copy)
@@ -7458,7 +7458,8 @@
 @end table
 
 @subsubheading Remarks
-Using extended @code{asm} typically produces smaller, safer, and more
+Using extended @code{asm} (@pxref{Extended Asm}) typically produces smaller,
+safer, and more
 efficient code, and in most cases it is a better solution than basic
 @code{asm}.  However, there are two situations where only basic @code{asm}
 can be used:
@@ -7516,11 +7517,51 @@
 Basic @code{asm} provides no
 mechanism to provide different assembler strings for different dialects.
 
-Here is an example of basic @code{asm} for i386:
+Basic @code{asm} statements do not perform an implicit "memory" clobber
+(@pxref{Clobbers}).  Also, there is no implicit clobbering of @emph{any}
+registers, so (other than in @code{naked} functions which follow the ABI
+rules) changed registers must be restored to their original va

Re: AW: Wonly-top-basic-asm

2016-02-11 Thread David Wohlferd


I don't think this is a patch we're considering for gcc-6, at least 
not for the initial release - I imagine it could be backported from 
gcc-7 at some point.


Actually, it was my intent that this apply to v6.  It's not like there 
is a significant change here.  We're documenting long-time behavior, and 
adding a (disabled) warning.


The reasons I think this is needed for v6 are:

1) We have become aware of places where basic asm's existing behavior is 
a problem.  This patch provides a way for users to locate these issues 
with a minimal, non-intrusive change.
2) There is a significant change to this behavior being proposed for 
v7.  When this happens, having a way to locate affected statements with 
features from a stable release seems desirable.


Like the other Bernd I have a preference for just -Wbasic-asm. I'd 
make the analogy with -Wparentheses - we don't warn about every 
parenthesis, but the name of an option is not the place for detailed 
explanations of how it works. Less typing and less to remember is 
preferrable IMO.


While I still prefer Wbasic-asm-in-function, I really don't have strong 
feelings here.  Since both you and Bernd prefer this, I have changed it.



+  /* Warn on basic asm used inside of functions,
+ EXCEPT when in naked functions.  Also allow asm (""). */


No all-caps.


Fixed.


  @subsubheading Remarks
-Using extended @code{asm} typically produces smaller, safer, and more
+Using extended @code{asm} (@pxref{Extended Asm}) typically produces 
smaller,

+safer, and more
  efficient code, and in most cases it is a better solution than basic


Rewrap the paragraph?


I could, but then people get cranky about how hard the patch is to review.


-Here is an example of basic @code{asm} for i386:
+Basic @code{asm} statements do not perform an implicit "memory" clobber
+(@pxref{Clobbers}).  Also, there is no implicit clobbering of 
@emph{any}
+registers, so (other than in @code{naked} functions which follow the 
ABI
+rules) changed registers must be restored to their original value 
before

+exiting the @code{asm}.  While this behavior has not always been
+documented, GCC has worked this way since at least v2.95.3.

+@strong{Warning:} This "clobber nothing" behavior may be different 
than how

+other compilers treat basic @code{asm}, since the C standards for the
+@code{asm} statement provide no guidance regarding these semantics.  
As a
+result, @code{asm} statements that work correctly on other compilers 
may not

+work correctly with GCC (and vice versa), even though they both compile
+without error.
+
+Future versions of GCC may change basic @code{asm} to clobber memory 
and
+perhaps some (or all) registers.  This change may fix subtle 
problems with

+existing @code{asm} statements.  However it may break or slow down ones
+that were working correctly.  To ``future-proof'' your asm against 
possible

+changes to basic @code{asm}'s semantics, use extended @code{asm}.
+
+You can use @option{-Wbasic-asm-in-function} to locate basic @code{asm}
+statements that may need changes, and refer to
+@uref{https://gcc.gnu.org/wiki/ConvertBasicAsmToExtended, How to 
convert
+from basic asm to extended asm} for information about how to perform 
the

+conversion.


I still think this is too verbose and would serve to confuse rather 
than enlighten in practice. (If anyone feels otherwise, feel free to 
overrule me.) 


I assume you mean someone besides me...


I'm also no longer sure about references to the wiki.


This is not the first reference to the gcc wiki in the docs (looks like 
the 6th for gcc, plus 10 for other wikis).



Let's look at this existing paragraph from the manual:

  Since GCC does not parse the @var{AssemblerInstructions}, it has
  no visibility of any symbols it references. This may result in GCC
  discarding those symbols as unreferenced.

I think extending this would be better. Something like

"Since the C standards does not specify semantics for @code{asm}, it 
is a potential source of incompatibilities between compilers. GCC does 
not parse the @var{AssemblerInstructions}, which means there is no way 
to communicate to the compiler what is happening inside them.  GCC has 
no visibility of any symbols referenced in the @code{asm} and may 
discard them as unreferenced. It also does not know about side effects 
that may occur, such as modifications of memory locations or 
registers. GCC assumes that no such side effects occur, which may not 
be what the user expected if code was written for other compilers.


Since basic @code{asm} cannot easily be used in a reliable way, 
@option{-Wbasic-asm} should be used to warn about the use of basic asm 
inside a function. See 
@uref{https://gcc.gnu.org/wiki/ConvertBasicAsmToExtended, How to 
convert from basic asm to extended asm} for information about how to 
convert code to use extended @code{asm}."


Hmm.  Yes, that's better.  But there are some things that got lost here 
that I think are important.  How abo

Re: AW: Wonly-top-basic-asm

2016-02-11 Thread David Wohlferd

On 2/11/2016 8:03 AM, Sandra Loosemore wrote:

On 02/11/2016 08:40 AM, Bernd Schmidt wrote:


But again, if someone feels the docs patch as posted is preferrable, go
ahead and approve it (for stage1 I assume).


TBH, I haven't looked at the documentation patch at all; I've been 
ignoring this issue because (a) I thought the technical details were 
still under discussion and (b) adding a new command-line option is 
stage 1 material not suitable to commit right now anyway.


I'll take a look at the docs once the name and behavior of the new 
option have been settled upon.


So far, no one has suggested any changes to the behavior.  And we seem 
to have settled on a name.


dw


Re: Fix PR69752, insn with REG_INC being removed as equiv_init insn

2016-02-11 Thread Jeff Law

On 02/11/2016 06:28 PM, Bernd Schmidt wrote:

This seems fairly straightforward:

(insn 213 455 216 6 (set (reg:SI 266)
 (mem/u/c:SI (post_inc:SI (reg/f:SI 267)) [4  S4 A32])) 748
{*thumb1_movsi_insn}
  (expr_list:REG_EQUAL (const_int -1044200508 [0xc1c2c3c4])
 (expr_list:REG_INC (reg/f:SI 267)
 (nil

We don't notice that the SET_SRC has a side effect, record the insn as
an equivalencing one, and later remove it because we replaced the reg
with the constant everywhere. Thus, the increment doesn't take place.

Fixed as follows. Bootstrapped and tested on x86_64-linux. Also compared
before/after dumps for the testcase with arm-elf. Ok?


Bernd

equiv-inc.diff


PR rtl-optimization/69752
* ira.c (update_equiv_regs): When looking for more than a single SET,
also take other side effects into account.
Also note that the reporter says gcc-4.9 didn't have this problem, so 
there's a reasonable chance this is a latent regression exposed by 
codegen changes prior to IRA.



OK for the trunk.

Jeff



Re: [PATCH] PR driver/69779: fix bogus cleanup code used by libgccjit affecting s390x

2016-02-11 Thread Jeff Law

On 02/11/2016 10:12 PM, David Malcolm wrote:

In r227188 I introduced driver::finalize () which resets
all state within gcc.c, allowing the driver code to embedded
inside libgccjit and run repeatedly in-process.

Running this on s390x revealed a bug in this cleanup:
I was cleaning up "specs" via:
 XDELETEVEC (specs);
and this was crashing on s390x with:
   free(): invalid pointer: 0x03fffdf30568 ***

Closer investigation revealed that specs == static_specs,
a statically allocated array.

What's happening is that set_specs sets "specs" to static_specs
the first time it's called, and any calls to set_specs () with
a name that isn't in static_specs cause a new spec_list node
to be dynamically-allocated and put at the front of the list; "specs"
then equals that.

All of my testing had been on x86_64, which inserts exactly one spec
with a name not in the static array, hence the:
 XDELETEVEC (specs);
happened to work.

On s390x, the only names in the "specs" file are those in the static
array, so specs == static_specs, and the bogus cleanup code attempts
to free a statically-allocated array.

The following patch reworks this part of the cleanup, walking any list
of specs, freeing the dynamically-allocated nodes until the
statically-allocated ones are reached.

driver::finalize is only called by libgccjit, not by the main driver
program, so this should be relatively low-risk.

Verified directly by running under valgrind on x86_64 and s390x.

With the patch, jit.sum on s390x-ibm-linux-gnu (RHEL 7) goes from:
   # of expected passes1799
   # of unexpected failures61
   # of unresolved testcases   2
to:
   # of expected passes8514

Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.

OK for trunk?

gcc/ChangeLog:
PR driver/69779
* gcc.c (driver::finalize): Fix cleanup of "specs".

Can't you just "free (specs->name) rather than messing with the cast?

OK with that twiddle, assuming it works.
jeff