Re: [PATCHv3, resent] Add a warning for suspicious use of conditional expressions in boolean context

[Eric Gallager]

On 9/20/16, Trevor Saunders  wrote:
> On Wed, Sep 21, 2016 at 01:13:41AM +0200, Bernd Schmidt wrote:
>> On 09/21/2016 01:09 AM, Trevor Saunders wrote:
>> >
>> > I thought I remember discussing this macro with you, but see what was
>> > checked in I'll believe I'm thinking of something similar but
>> > different.
>>
>> I think this here was an earlier patch and the one we were discussing
>> recently was the other macro with a similar name.
>>
>> > Any way sorry about the dumb bug
>>
>> Stuff like this happens, no worries. But I've seen it happen a lot over the
>> years, and maybe you can see in this an explanation of why I'm often not the
>> most enthusiastic supporter of pure cleanup patches (those not motivated by
>> more substantial patches depending on them).
>
> yeah, there's always some risk, though I also believe if you define
> something as cleaning up then it has some value compared to pointless
> permutation.  Ironically I think one of the big motivating reasons to
> remove ifdefs is to remove a source of bustage.
>
> Trev
>


This is kinda changing the topic a bit, but if removing ifdefs is to
remove bustage, maybe GCC should start compiling with -Wundef to
ensure that the ifdef removal doesn't actually introduce any new
bustage? Glibc started using -Wundef for that reason:

https://sourceware.org/ml/libc-alpha/2014-02/msg00828.html
https://www.sourceware.org/ml/libc-alpha/2015-08/msg00751.html

Re: [PATCH] - improve sprintf buffer overflow detection (middle-end/49905)

[Martin Sebor]

On 09/20/2016 01:38 PM, David Malcolm wrote:

On Tue, 2016-09-20 at 13:35 -0600, Martin Sebor wrote:

[...]


+#if 0

...

+#else

...

+#endif

Please remove the #if 0'd code.


Sorry about that.  This was left over from my effort to persuade
the substring_loc class to underline just the last quote of the
format string.  I thought David had made some changes to make it
possible but I may have misremembered.  Let me raise a separate
bug for it.


I don't think the substring_loc code supports such a feature yet;
please open a bug for it and CC me on it.


Done in bug 77672 - wrong rich location in warning: writing
a terminating nul past the end.

Thanks
Martin

Re: [PATCHv3, resent] Add a warning for suspicious use of conditional expressions in boolean context

[Trevor Saunders]

On Wed, Sep 21, 2016 at 01:13:41AM +0200, Bernd Schmidt wrote:
> On 09/21/2016 01:09 AM, Trevor Saunders wrote:
> > 
> > I thought I remember discussing this macro with you, but see what was
> > checked in I'll believe I'm thinking of something similar but different.
> 
> I think this here was an earlier patch and the one we were discussing
> recently was the other macro with a similar name.
> 
> > Any way sorry about the dumb bug
> 
> Stuff like this happens, no worries. But I've seen it happen a lot over the
> years, and maybe you can see in this an explanation of why I'm often not the
> most enthusiastic supporter of pure cleanup patches (those not motivated by
> more substantial patches depending on them).

yeah, there's always some risk, though I also believe if you define
something as cleaning up then it has some value compared to pointless
permutation.  Ironically I think one of the big motivating reasons to
remove ifdefs is to remove a source of bustage.

Trev

> 
> 
> Bernd

Re: [PATCH] Optimise the fpclassify builtin to perform integer operations when possible

[Joseph Myers]

On Tue, 20 Sep 2016, Michael Meissner wrote:

> It would be better to have a fpclassify2 pattern, and if it isn't
> defined, then do the machine independent processing.  That is the way it is
> done elsewhere.

But note:

* The __builtin_fpclassify function takes arguments for all the possible 
FP_* results, so the insn pattern would need to map the results to the 
arguments passed to __builtin_fpclassify.  (They are documented as needing 
to be constants, of type int.)

* Then you want that mapping step to get optimized away in the case of a 
comparison fpclassify (...) == FP_SUBNORMAL (for example), or a switch 
over possible results.  Will the RTL optimizers do that given the insns 
structured appropriately?

(For that matter, I don't know if the GIMPLE optimizers will optimize away 
such a mapping either, but they clearly should.  I've wondered what the 
right approach would be for making FLT_ROUNDS properly depend on the 
rounding mode - bug 30569, 
 - where the same issues 
apply.  For boolean operations such as isnan you don't have such 
complications.)

* If flag_signaling_nans, then any pattern should work for signaling NaNs.

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

[PATCH], PR 77670, Fix PowerPC ISA 3.0 -mpower9-minmax code generation

[Michael Meissner]

There are some instructions in PowerPC ISA 3.0 that are not currently enabled
by default when you use -mcpu=power9.  These instructions are currently added
with the -mpower9-minmax switch.

I recently built Spec 2006 with the current trunk compiler, and I discovered
that three of the benchmarks (gamess, povray, and soplex) do not build when you
use the -mpower9-minmiax option.

In particular, ISA 3.0 adds 3 new instructions: XSCMPEQDP, XSCMPGEDP, and
XSCMPGTDP that are similar to the vector instructions in that they set the
scalar part of the vector register to all 0's or all 1's so that the XXSEL
instruction can be used to do a floating point conditional move.

The code did not provide an inverted operation that moves the registers if the
condition is false instead of true.  I added the an insn splitter for the
inverted operation to fix the problem.

In testing it, I discovered that when I implemented the XXSEL operation for
scalar, I had the registers backwards.  I provided a fix for this as well.

I rebuilt Spec 2016, and it all builds now.  I also tested a small program on
the simulator and it generated the correct output.

Since this is in ISA 3.0 code only, I did not do the full bootstrap and make
check sequence.  I can do this if you prefer.

Is the patch ok to put into the trunk?

2016-09-20  Michael Meissner  

PR target/77670
* config/rs6000/predicates.md (invert_fpmask_comparison_operator):
New predicate that matches the ISA 3.0 XSCMP{EQ,GT,GE}DP
instructions when you want to invert the test.
* config/rs6000/rs6000.md (fpmask): Use the arguments in the
correct order for XXSEL.
(movcc_invert_p9): Define the inverted test
for using XSCMP{EQ,GT,GE}DP.

-- 
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 240283)
+++ gcc/config/rs6000/predicates.md (working copy)
@@ -1172,6 +1172,12 @@ (define_predicate "scc_rev_comparison_op
 (define_predicate "fpmask_comparison_operator"
   (match_code "eq,gt,ge"))
 
+;; Return 1 if OP is a comparison operator suitable for vector/scalar
+;; comparisons that generate a 0/-1 mask (i.e. the inverse of
+;; fpmask_comparison_operator).
+(define_predicate "invert_fpmask_comparison_operator"
+  (match_code "ne,unlt,unle"))
+
 ;; Return 1 if OP is a comparison operation that is valid for a branch
 ;; insn, which is true if the corresponding bit in the CC register is set.
 (define_predicate "branch_positive_comparison_operator"
Index: gcc/config/rs6000/rs6000.md
===
--- gcc/config/rs6000/rs6000.md (revision 240283)
+++ gcc/config/rs6000/rs6000.md (working copy)
@@ -4882,6 +4882,43 @@ (define_insn_and_split "*mov<
  [(set_attr "length" "8")
   (set_attr "type" "vecperm")])
 
+;; Handle inverting the fpmask comparisons.
+(define_insn_and_split "*movcc_invert_p9"
+  [(set (match_operand:SFDF 0 "vsx_register_operand" "=&,")
+   (if_then_else:SFDF
+(match_operator:CCFP 1 "invert_fpmask_comparison_operator"
+   [(match_operand:SFDF2 2 "vsx_register_operand" 
",")
+(match_operand:SFDF2 3 "vsx_register_operand" 
",")])
+(match_operand:SFDF 4 "vsx_register_operand" ",")
+(match_operand:SFDF 5 "vsx_register_operand" ",")))
+   (clobber (match_scratch:V2DI 6 "=0,"))]
+  "TARGET_P9_MINMAX"
+  "#"
+  ""
+  [(set (match_dup 6)
+   (if_then_else:V2DI (match_dup 9)
+  (match_dup 7)
+  (match_dup 8)))
+   (set (match_dup 0)
+   (if_then_else:SFDF (ne (match_dup 6)
+  (match_dup 8))
+  (match_dup 5)
+  (match_dup 4)))]
+{
+  rtx op1 = operands[1];
+  enum rtx_code cond = reverse_condition_maybe_unordered (GET_CODE (op1));
+
+  if (GET_CODE (operands[6]) == SCRATCH)
+operands[6] = gen_reg_rtx (V2DImode);
+
+  operands[7] = CONSTM1_RTX (V2DImode);
+  operands[8] = CONST0_RTX (V2DImode);
+
+  operands[9] = gen_rtx_fmt_ee (cond, CCFPmode, operands[2], operands[3]);
+}
+ [(set_attr "length" "8")
+  (set_attr "type" "vecperm")])
+
 (define_insn "*fpmask"
   [(set (match_operand:V2DI 0 "vsx_register_operand" "=wa")
(if_then_else:V2DI
@@ -4901,7 +4938,7 @@ (define_insn "*xxsel"
   (match_operand:SFDF 3 "vsx_register_operand" "")
   (match_operand:SFDF 4 "vsx_register_operand" 
"")))]
   "TARGET_P9_MINMAX"
-  "xxsel %x0,%x1,%x3,%x4"
+  "xxsel %x0,%x4,%x3,%x1"
   [(set_attr "type" "vecmove")])
 
 

Re: [PATCHv3, resent] Add a warning for suspicious use of conditional expressions in boolean context

[Bernd Schmidt]

On 09/21/2016 01:09 AM, Trevor Saunders wrote:


I thought I remember discussing this macro with you, but see what was
checked in I'll believe I'm thinking of something similar but different.


I think this here was an earlier patch and the one we were discussing 
recently was the other macro with a similar name.



Any way sorry about the dumb bug


Stuff like this happens, no worries. But I've seen it happen a lot over 
the years, and maybe you can see in this an explanation of why I'm often 
not the most enthusiastic supporter of pure cleanup patches (those not 
motivated by more substantial patches depending on them).



Bernd

Re: [PATCHv3, resent] Add a warning for suspicious use of conditional expressions in boolean context

[Trevor Saunders]

On Tue, Sep 20, 2016 at 05:30:36PM +0200, Bernd Schmidt wrote:
> On 09/20/2016 05:18 PM, Jeff Law wrote:
> > > I assume HARD_FRAME_POINTER_REGNUM is never zero.
> > It could be zero.  It's just a hard register number.  No target has the
> > property that its hard frame pointer register is 0 though :-)
> 
> git blame to the rescue. The current state comes from one of tbsaunde's
> cleanup patches:
> 
> > diff --git a/gcc/regrename.c b/gcc/regrename.c
> index 174d3b5..e5248a5 100644
> --- a/gcc/regrename.c
> +++ b/gcc/regrename.c
> @@ -442,12 +442,10 @@ rename_chains (void)
> continue;
> 
>if (fixed_regs[reg] || global_regs[reg]
> -#if !HARD_FRAME_POINTER_IS_FRAME_POINTER
> - || (frame_pointer_needed && reg == HARD_FRAME_POINTER_REGNUM)
> -#else
> - || (frame_pointer_needed && reg == FRAME_POINTER_REGNUM)
> -#endif
> - )
> + || (!HARD_FRAME_POINTER_IS_FRAME_POINTER && frame_pointer_needed
> + && reg == HARD_FRAME_POINTER_REGNUM)
> + || (HARD_FRAME_POINTER_REGNUM && frame_pointer_needed
> + && reg == FRAME_POINTER_REGNUM))
> continue;
> 
>COPY_HARD_REG_SET (this_unavailable, unavailable);
> 
> Looks like it never got reviewed and was committed as preapproved.

I thought I remember discussing this macro with you, but see what was
checked in I'll believe I'm thinking of something similar but different.
Any way sorry about the dumb bug, though in my defense, or perhaps in
further proof I'm not good at detail I missed the
HARD_FRAME_POINTER_REGNUM instead of HARD_FRAME_POINTER_IS_FRAME_POINTER
the first time I looked at it here too :"(

> The #ifdef we had before looks odd too. Maybe this should just read
> 
>  if (fixed_regs[reg] || global_regs[reg]
>  || (frame_pointer_needed && reg == HARD_FRAME_POINTER_REGNUM))

I'd agree that sounds reasonable.  Though I'd want to check for a target
where HARD_FRAME_POINTER_IS_FRAME_POINTER is true, but
HARD_FRAME_POINTER_REGNUM != FRAME_POINTER_REGNUM, though that could
only be arm or mips I believe and it sounds pretty bogus.

Thanks for fixing that up for me!

Trev

> 
> 
> Bernd

RE: [PATCH] Fix PR tree-optimization/77654

[Doug Gilmore]

It looks like the original message was dropped, resending.

Doug

From: Doug Gilmore
Sent: Tuesday, September 20, 2016 2:12 PM
To: gcc-patches@gcc.gnu.org; rgue...@gcc.gnu.org
Subject: [PATCH] Fix PR tree-optimization/77654

From:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77654

Richard Biener wrote:
> Looks good though addr_base should always be a pointer but it might
> not be an SSA name so better check that...

I took a look at other situations where duplicate_ssa_name_ptr_info()
is called and found that there are no checks for the SSA name since
that check is done in duplicate_ssa_name_ptr_info().  Do you still
want the additional check added?

Also does it make sense to make a test case for this?

I was thinking of making the following change to:

diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
index 8051a66..b799c43 100644
--- a/gcc/tree-ssa-alias.c
+++ b/gcc/tree-ssa-alias.c
@@ -296,7 +296,16 @@ ptr_derefs_may_alias_p (tree ptr1, tree ptr2)
   pi1 = SSA_NAME_PTR_INFO (ptr1);
   pi2 = SSA_NAME_PTR_INFO (ptr2);
   if (!pi1 || !pi2)
-return true;
+{
+  if (dump_file)
+   {
+ if (! pi1)
+   fprintf (dump_file, "%s pi1 is NULL\n", __FUNCTION__);
+ if (! pi2)
+   fprintf (dump_file, "%s pi2 is NULL\n", __FUNCTION__);
+   }
+  return true;
+}

Then when compiling the test case, we could scan for the RE
"pi. is NULL" in the dump file created by compiling with -fdump-rtl-sched2.

I attached the original patch.

Thanks,

Doug

gcc/
PR tree-optimization/77654
* tree-ssa-alias.c (issue_prefetch_ref): Add call
to duplicate_ssa_name_ptr_info.

RE: [PATCH] Fix PR tree-optimization/77654

[Doug Gilmore]

Add missing attachment.

Doug

gcc/
PR tree-optimization/77654
* tree-ssa-alias.c (issue_prefetch_ref): Add call
to duplicate_ssa_name_ptr_info.
From ec9069b7b7b07d5fda9c04aaa9b385fba89a6e16 Mon Sep 17 00:00:00 2001
From: Doug Gilmore 
Date: Tue, 6 Sep 2016 10:18:42 -0700
Subject: [PATCH 2/2] Ensure points-to information is maintained for prefetch
 addresses.

---
 gcc/tree-ssa-loop-prefetch.c |   11 +++
 1 file changed, 11 insertions(+)

diff --git a/gcc/tree-ssa-loop-prefetch.c b/gcc/tree-ssa-loop-prefetch.c
index 26cf0a0..10ade186 100644
--- a/gcc/tree-ssa-loop-prefetch.c
+++ b/gcc/tree-ssa-loop-prefetch.c
@@ -39,6 +39,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree-ssa-loop-manip.h"
 #include "tree-ssa-loop-niter.h"
 #include "tree-ssa-loop.h"
+#include "ssa.h"
 #include "tree-into-ssa.h"
 #include "cfgloop.h"
 #include "tree-scalar-evolution.h"
@@ -1160,6 +1161,16 @@ issue_prefetch_ref (struct mem_ref *ref, unsigned unroll_factor, unsigned ahead)
   addr = force_gimple_operand_gsi (, unshare_expr (addr), true,
 	   NULL, true, GSI_SAME_STMT);
   }
+
+  if (POINTER_TYPE_P (TREE_TYPE (addr_base)))
+	{
+	  duplicate_ssa_name_ptr_info (addr, SSA_NAME_PTR_INFO (addr_base));
+	  /* As this isn't a plain copy we have to reset alignment
+	 information.  */
+	  if (SSA_NAME_PTR_INFO (addr))
+	mark_ptr_info_alignment_unknown (SSA_NAME_PTR_INFO (addr));
+	}
+
   /* Create the prefetch instruction.  */
   prefetch = gimple_build_call (builtin_decl_explicit (BUILT_IN_PREFETCH),
 3, addr, write_p, local);
-- 
1.7.9.5

[PATCH] PR68212, Correct frequencies/counts when unrolling

[Pat Haugen]

The following patch corrects frequency/count values computed when generating 
the switch blocks/peeled loop copies before the loop. The two main problem 
areas were for the peeled copies duplicate_loop_to_header_edge was not using 
the preheader frequency as part of the scale factor when peeling a copy of the 
loop to the preheader edge, and the second was that the switch block generation 
was just totally lacking code to compute correct freq/count values. Verified by 
comparing freq/count values in the unroller dump before/after.

Bootstrap/regtest on powerpc64le with no new regressions. Ok for trunk?

-Pat



2016-09-20  Pat Haugen  

PR rtl-optimization/68212
* cfgloopmanip.c (duplicate_loop_to_header_edge): Use preheader edge
frequency when computing scale factor for peeled copies.
* loop-unroll.c (unroll_loop_runtime_iterations): Fix freq/count
values for switch/peel blocks/edges.


Index: gcc/cfgloopmanip.c
===
--- gcc/cfgloopmanip.c	(revision 240167)
+++ gcc/cfgloopmanip.c	(working copy)
@@ -1276,10 +1276,13 @@ duplicate_loop_to_header_edge (struct lo
 	}
   else
 	{
+	  int preheader_freq = EDGE_FREQUENCY (e);
 	  scale_main = REG_BR_PROB_BASE;
 	  for (i = 0; i < ndupl; i++)
 	scale_main = combine_probabilities (scale_main, scale_step[i]);
-	  scale_act = REG_BR_PROB_BASE - prob_pass_thru;
+	  if (preheader_freq > freq_in)
+	preheader_freq = freq_in;
+	  scale_act = GCOV_COMPUTE_SCALE (preheader_freq, freq_in);
 	}
   for (i = 0; i < ndupl; i++)
 	gcc_assert (scale_step[i] >= 0 && scale_step[i] <= REG_BR_PROB_BASE);
Index: gcc/loop-unroll.c
===
--- gcc/loop-unroll.c	(revision 240167)
+++ gcc/loop-unroll.c	(working copy)
@@ -858,7 +858,8 @@ unroll_loop_runtime_iterations (struct l
   rtx_insn *init_code, *branch_code;
   unsigned i, j, p;
   basic_block preheader, *body, swtch, ezc_swtch;
-  int may_exit_copy;
+  int may_exit_copy, iter_freq, new_freq;
+  gcov_type iter_count, new_count;
   unsigned n_peel;
   edge e;
   bool extra_zero_check, last_may_exit;
@@ -952,6 +953,15 @@ unroll_loop_runtime_iterations (struct l
   /* Record the place where switch will be built for preconditioning.  */
   swtch = split_edge (loop_preheader_edge (loop));
 
+  /* Compute frequency/count increments for each switch block and initialize
+ innermost switch block.  Switch blocks and peeled loop copies are built
+ from innermost outward.  */
+  iter_freq = new_freq = swtch->frequency / (max_unroll + 1);
+  iter_count = new_count = swtch->count / (max_unroll + 1);
+  swtch->frequency = new_freq;
+  swtch->count = new_count;
+  single_succ_edge (swtch)->count = new_count;
+
   for (i = 0; i < n_peel; i++)
 {
   /* Peel the copy.  */
@@ -969,6 +979,10 @@ unroll_loop_runtime_iterations (struct l
   p = REG_BR_PROB_BASE / (i + 2);
 
   preheader = split_edge (loop_preheader_edge (loop));
+  /* Add in frequency/count of edge from switch block.  */
+  preheader->frequency += iter_freq;
+  preheader->count += iter_count;
+  single_succ_edge (preheader)->count = preheader->count;
   branch_code = compare_and_jump_seq (copy_rtx (niter), GEN_INT (j), EQ,
 	  block_label (preheader), p,
 	  NULL);
@@ -980,9 +994,14 @@ unroll_loop_runtime_iterations (struct l
   swtch = split_edge_and_insert (single_pred_edge (swtch), branch_code);
   set_immediate_dominator (CDI_DOMINATORS, preheader, swtch);
   single_succ_edge (swtch)->probability = REG_BR_PROB_BASE - p;
+  single_succ_edge (swtch)->count = new_count;
+  new_freq += iter_freq;
+  new_count += iter_count;
+  swtch->frequency = new_freq;
+  swtch->count = new_count;
   e = make_edge (swtch, preheader,
 		 single_succ_edge (swtch)->flags & EDGE_IRREDUCIBLE_LOOP);
-  e->count = RDIV (preheader->count * REG_BR_PROB_BASE, p);
+  e->count = iter_count;
   e->probability = p;
 }
 
@@ -992,6 +1011,14 @@ unroll_loop_runtime_iterations (struct l
   p = REG_BR_PROB_BASE / (max_unroll + 1);
   swtch = ezc_swtch;
   preheader = split_edge (loop_preheader_edge (loop));
+  /* Recompute frequency/count adjustments since initial peel copy may
+	 have exited and reduced those values that were computed above.  */
+  iter_freq = swtch->frequency / (max_unroll + 1);
+  iter_count = swtch->count / (max_unroll + 1);
+  /* Add in frequency/count of edge from switch block.  */
+  preheader->frequency += iter_freq;
+  preheader->count += iter_count;
+  single_succ_edge (preheader)->count = preheader->count;
   branch_code = compare_and_jump_seq (copy_rtx (niter), const0_rtx, EQ,
 	  block_label (preheader), p,
 	  NULL);
@@ -1000,9 +1027,10 @@ unroll_loop_runtime_iterations (struct l
   swtch = split_edge_and_insert (single_succ_edge 

Re: [Patch, fortran] PR77657 - link error with implementation of user-defined derived type input/output (UD-DTIO) in child extending abstract parent

[Jerry DeLisle]

On 09/20/2016 01:15 PM, Paul Richard Thomas wrote:
> Dear All,
> 
> Once found, the fix for this PR is trivial. The generic name is only
> to be found in the parent derived type. Since this is over-ridden in
> the type extension, not only is the wrong symbol selected for the dtio
> procedure but, being abstract, the procedure does not exist. The
> mechanism is borrowed from resolve.c(resolve_typebound_generic_call).
> This goes back with the generic procedure name and looks again for the
> procedure in the type extension. The testcase is a dejagnuified
> version of the original.
> 
> Bootstraps and regtests on x86_64/FC21 - OK for trunk?
> 
>

OK, thanks Paul.

Jerry

Re: [PATCH] Optimise the fpclassify builtin to perform integer operations when possible

[Michael Meissner]

On Tue, Sep 20, 2016 at 01:19:07PM +0100, Tamar Christina wrote:
> On 19/09/16 23:16, Michael Meissner wrote:
> >On Mon, Sep 12, 2016 at 04:19:32PM +, Tamar Christina wrote:
> >>Hi All,
> >>
> >>This patch adds an optimized route to the fpclassify builtin
> >>for floating point numbers which are similar to IEEE-754 in format.
> >>
> >>The goal is to make it faster by:
> >>1. Trying to determine the most common case first
> >>(e.g. the float is a Normal number) and then the
> >>rest. The amount of code generated at -O2 are
> >>about the same +/- 1 instruction, but the code
> >>is much better.
> >>2. Using integer operation in the optimized path.
> >>
> >>At a high level, the optimized path uses integer operations
> >>to perform the following:
> >>
> >>   if (exponent bits aren't all set or unset)
> >>  return Normal;
> >>   else if (no bits are set on the number after masking out
> >>sign bits then)
> >>  return Zero;
> >>   else if (exponent has no bits set)
> >>  return Subnormal;
> >>   else if (mantissa has no bits set)
> >>  return Infinite;
> >>   else
> >>  return NaN;
> >I haven't looked at fpclassify.  I assume we can define a backend insn to do
> >the right thing?  One of the things we've noticed over the years with the
> >PowerPC is that it can be rather expensive to move things from the floating
> >point/vector unit to the integer registers and vice versa.  This is
> >particularly true if you having to do the transfer via the memory unit via
> >stores and loads of different sizes.
> >
> Hmm, what do you mean with the right thing? Do you mean never to use the
> integer version?

The forthcoming PowerPC with ISA 3.0 (power9), we have different ways to do
classification within the floating point unit.

For example, there is the XSTSTDCDP instruction that can set a condition code
register to whether the value is 0, NaN, Infinity, Denormal.  We might come up
with a clever set of tests to use 4 of these instructions to return the
appropriate FP_.

Even if we want to do it by looking at the exponent, ISA 3.0 defines
instructions like XSXEXPDP that extracts the exponent from a double precision
value and returns it in a GPR register.

> If so then no, it currently determines it based on the format.
> I could potentially add a hook to allow backends to opt-in/out if
> there's a concern this might be slower.

It would be better to have a fpclassify2 pattern, and if it isn't
defined, then do the machine independent processing.  That is the way it is
done elsewhere.

> Though is the move that much slower that it negates the benefits we
> should get from not having to do
> 4 branches in the normal case?

It depends.  We have a lot of other stuff for ISA 3.0 on our plates, and
truthfully, we won't be able to answer the question about performance until we
get real hardware, but I would prefer not to be locked into an existing
implementation.

-- 
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: Document OpenACC status for GCC 6

[Gerald Pfeifer]

[ Old e-mail alert ]

On Fri, 22 Apr 2016, Thomas Schwinge wrote:
> Thanks for the review; OK to commit as follows?  And then, should
> something be added to the "News" section on 
> itself, too?  (I don't know the policy for that.  We didn't suggest that
> for GCC 5, because at that time we described the support as a
> "preliminary implementation of the OpenACC 2.0a specification"; now 
> it's much more complete and usable.)

Yes, definitely.  Sorry for not picking this up earlier, but this
definitely is a strong News item.  If you feel that particular item
is not suitable any longer, perhaps there is another one (or there
are other ones, plural)?

As a rule of thumb, when in doubt, propose a News item.  As a group 
we have been way too conservative on that front.

Gerald

[Patch, fortran] PR77657 - link error with implementation of user-defined derived type input/output (UD-DTIO) in child extending abstract parent

[Paul Richard Thomas]

Dear All,

Once found, the fix for this PR is trivial. The generic name is only
to be found in the parent derived type. Since this is over-ridden in
the type extension, not only is the wrong symbol selected for the dtio
procedure but, being abstract, the procedure does not exist. The
mechanism is borrowed from resolve.c(resolve_typebound_generic_call).
This goes back with the generic procedure name and looks again for the
procedure in the type extension. The testcase is a dejagnuified
version of the original.

Bootstraps and regtests on x86_64/FC21 - OK for trunk?

Paul

2016-09-20  Paul Thomas  

PR fortran/77657

* interface.c (gfc_find_specific_dtio_proc): Borrow trick from
resolve_typebound_generic_call to find dtio procedures that
over-ride those in the declared type.

2016-09-20  Paul Thomas  

PR fortran/77657
* gfortran.dg/dtio_12.f90: New test.
Index: gcc/fortran/interface.c
===
*** gcc/fortran/interface.c (revision 240270)
--- gcc/fortran/interface.c (working copy)
*** gfc_find_specific_dtio_proc (gfc_symbol
*** 4792,4797 
--- 4792,4800 
  
if (tb_io_st != NULL)
  {
+   const char *genname;
+   gfc_symtree *st;
+ 
tb_io_proc = tb_io_st->n.tb;
gcc_assert (tb_io_proc != NULL);
gcc_assert (tb_io_proc->is_generic);
*** gfc_find_specific_dtio_proc (gfc_symbol
*** 4800,4806 
specific_proc = tb_io_proc->u.generic->specific;
gcc_assert (!specific_proc->is_generic);
  
!   dtio_sub = specific_proc->u.specific->n.sym;
  }
  
if (tb_io_st != NULL)
--- 4803,4818 
specific_proc = tb_io_proc->u.generic->specific;
gcc_assert (!specific_proc->is_generic);
  
!   /* Go back and make sure that we have the right specific procedure.
!Here we most likely have a procedure from the parent type, which
!can be overridden in extensions.  */
!   genname = tb_io_proc->u.generic->specific_st->name;
!   st = gfc_find_typebound_proc (derived, NULL, genname,
!   true, _io_proc->where);
!   if (st)
!   dtio_sub = st->n.tb->u.specific->n.sym;
!   else
!   dtio_sub = specific_proc->u.specific->n.sym;
  }
  
if (tb_io_st != NULL)
Index: gcc/testsuite/gfortran.dg/dtio_12.f90
===
*** gcc/testsuite/gfortran.dg/dtio_12.f90   (revision 0)
--- gcc/testsuite/gfortran.dg/dtio_12.f90   (working copy)
***
*** 0 
--- 1,74 
+ ! { dg-do run }
+ !
+ ! Test the fix for PR77657 in which the DTIO subroutine was not found,
+ ! which led to an error in attempting to link to the abstract interface.
+ !
+ ! Contributed by Damian Rouson  
+ !
+ MODULE abstract_parent
+   implicit none
+ 
+   type, abstract :: parent
+   contains
+ procedure(write_formatted_interface), deferred :: write_formatted
+ generic :: write(formatted) => write_formatted
+   end type parent
+ 
+   abstract interface
+ subroutine write_formatted_interface(this,unit,iotype,vlist,iostat,iomsg)
+   import parent
+   class(parent), intent(in) :: this
+   integer, intent(in) :: unit
+   character (len=*), intent(in) :: iotype
+   integer, intent(in) :: vlist(:)
+   integer, intent(out) :: iostat
+   character (len=*), intent(inout) :: iomsg
+ end subroutine
+   end interface
+ 
+ end module
+ 
+ module child_module
+   use abstract_parent, only : parent
+   implicit none
+ 
+   type, extends(parent) :: child
+ integer :: i = 99
+   contains
+ procedure :: write_formatted
+   end type
+ contains
+   subroutine write_formatted(this,unit,iotype,vlist,iostat,iomsg)
+ class(child), intent(in) :: this
+ integer, intent(in) :: unit
+ character (len=*), intent(in) :: iotype
+ integer, intent(in) :: vlist(:)
+ integer, intent(out) :: iostat
+ character (len=*), intent(inout) :: iomsg
+ write (unit, "(i4)") this%i
+   end subroutine
+ end module
+ 
+   use child_module, only : child
+   implicit none
+   type (child) :: baby
+   integer :: v(1), istat
+   character(20) :: msg
+   open (10, status = "scratch")
+   call baby%write_formatted(10, "abcd", v, istat, msg) ! Call the dtio proc 
directly
+   rewind (10)
+   read (10, *) msg
+   if (trim (msg) .ne. "99") call abort
+   rewind (10)
+   baby%i = 42
+   write (10,"(DT)") baby   ! Call the dtio proc 
via the library
+   rewind (10)
+   read (10, *) msg
+   if (trim (msg) .ne. "42") call abort
+   rewind (10)
+   write (10,"(DT)") child (77) ! The original testcase
+   rewind (10)
+   read (10, *) msg
+   if (trim (msg) .ne. "77") call abort
+   close(10)
+ end

Re: [PATCH] - improve sprintf buffer overflow detection (middle-end/49905)

[David Malcolm]

On Tue, 2016-09-20 at 13:35 -0600, Martin Sebor wrote:

[...]

> > > +#if 0
> ...
> > > +#else
> ...
> > > +#endif
> > Please remove the #if 0'd code.
> 
> Sorry about that.  This was left over from my effort to persuade
> the substring_loc class to underline just the last quote of the
> format string.  I thought David had made some changes to make it
> possible but I may have misremembered.  Let me raise a separate
> bug for it.

I don't think the substring_loc code supports such a feature yet;
please open a bug for it and CC me on it.

[...]

Re: [PATCH] - improve sprintf buffer overflow detection (middle-end/49905)

[Martin Sebor]

+  /* The minimum and maximum length.  The MAXLEN pointer stays unchanged
+ but MINLEN may be cleared during the execution of the function.  */
+  tree *minlen = length;
+  tree* const maxlen = length + 1;

Check formatting here.  I'm not sure if the formatting standards
explicitly state how to handle the "*" when there's a qualifier between
the type and the object.  But please check.


If my regular expressions were right, most code puts the star right
before the const (379 occurrences in .c and .h files under the gcc/
directory not counting the test suite).  For example:

  gcc/cp/decl.c:  const char *const name;

There's quite a bit of code that also puts a space after it (219
occurrences).  For example:

  gcc/cgraph.h:extern const char * const tls_model_names[];

There are 28 occurrences of the style I used, mostly in function
declarations.  For instance:

gcc/cp/mangle.c:decl_is_template_id (const tree decl, tree* const 
template_info)


I changed my code to follow with the dominant style.




+inform (info.fmtloc,
+"using the range [%qE, %qE] for directive argument",
+fmtres.argmin, fmtres.argmax);

Don't you need G_ for these messages?

Can you please check the calls to fmtwarn which pass in the string as an
argument and add G_ as needed?  I think the cases where the string is
computed into a variable are all handled correctly, it's jut those where
the string appears as a call argument that need double checking.  I
think there's ~3 of them.


I found an error() call in c-family/c-format.c with the string split
across two lines like here (see below) and the error message appears
correctly concatenated in the gcc.pot file so I take that to mean
that the G_ isn't necessary here.

  error ("found a %<%s%> reference but the format argument should"
 " be a string", format_name (gcc_objc_string_format_type));

Other than that, though, since it's an easy mistake to make, it
would be helpful if problems like the one you were worried about
could be detected by some tool run during a build.  I wonder how
hard it would be to write an awk scirpt...


+#if 0

...

+#else

...

+#endif

Please remove the #if 0'd code.


Sorry about that.  This was left over from my effort to persuade
the substring_loc class to underline just the last quote of the
format string.  I thought David had made some changes to make it
possible but I may have misremembered.  Let me raise a separate
bug for it.


I think with the nits above fixed, this is OK for the trunk.

Thanks for your patience.


Great, thank you!

Martin

Re: [gofrontend-dev] Go patch committed: Avoid warning by using a local var for std::ofstream

[Florian Weimer]

* Ian Lance Taylor:

> GCC PR 77625 is about a warning while compiling the Go frontend.  The
> Go frontend called `new std::ofstream()`, and the warning is "error:
> ‘new’ of type ‘std::ofstream {aka std::basic_ofstream}’ with
> extended alignment 16".  Frankly I'm not sure how this supposed to
> work: shouldn't it be possible to write new std::ofstream()?  But the
> problem is easy to avoid, since in this case the std::ofstream can be
> a local variable, and that is a better approach anyhow.  This patch
> was bootstrapped on x86_64-pc-linux-gnu.  Committed to mainline.

This happens only on hppa, right?  It looks as if libstdc++ is
seriously broken there.

(The old code looks like it has a memory leak, so the stack allocation
is reasonable anyway.)

Re: Register numbers in RTL dumps (was Re: [PATCH 0/9] RFC: selftests based on RTL dumps)

[David Malcolm]

On Tue, 2016-09-20 at 09:20 -0600, Jeff Law wrote:
> On 09/20/2016 08:34 AM, Bernd Schmidt wrote:
> > On 09/20/2016 04:32 PM, David Malcolm wrote:
> > > 
> > > To summarize so far: you want every pseudo to have its regno
> > > dumped
> > > with a 'p' prefix, and renumber them on dump (and then on load).
> > > OK.
> > 
> > Renumbering is not helpful because it interferes with the view you
> > have
> > in the debugger. So, IMO just a prefix, and maybe
> Yea, I guess it does since we want the numbers in the dump to be the 
> same that we used to access the arrays.  So prefixing in the dump
> with 
> adjustment of the number in the reader.

To check I understand: am I right in thinking you want:
(A) the numbers in the dump to be unmodified when dumping, so that we
can easily look up values in arrays without confusion, and
(B) regnums in the dump gain a 'p' prefix for values >=
 FIRST_PSEUDO_REGISTER, so that humans and parsers can easily see when
the regs are pseudos, and that
(C) the parser will detect if a 'p'-prefixed regno actually has the
same number as a hard reg (which can happen e.g. when a .md file
changes, or when sharing .rtl dumps between targets), and remap the
values on load accordingly

?

(in which case we do need the regno_remapper class, or something like
it)

> > 
> > >   (reg/f:DI v1 virtual-stack-vars)
> > 
> > this.
> Doesn't the same apply to the number of virtual stack regs?  Those
> are 
> in the same array as pseudos.  So ISTM we prefix in the dump, but do 
> adjustment of the number in the reader?

Presumably we could use "v" rather than "p" as the prefix for the first
5 pseudos (up to LAST_VIRTUAL_REGISTER), doing any adjustment at load
time, rather than at dump time.  So the above example would look like:

   (reg/f:DI v82 virtual-stack-vars)

i.e. the 82 for x86_64's virtual-stack-vars would be prefixed with a
'v', and the loader would adjust it to be the current target's value
for VIRTUAL_STACK_VARS_REGNUM.

Do you like the idea of prefixing regnums of hardregs with 'h'? (so
that all regnos get a one-char prefix) e.g.
  (reg/i:SI h0 ax)
  (reg/i:SF h21 xmm0)


Dave

[PATCH] Fix JUMP_LABEL documentation

[Segher Boessenkool]

On Tue, Sep 20, 2016 at 12:11:45PM -0500, Segher Boessenkool wrote:
> I'm just pointing out the documentation is out-of-date here.  I'll do
> a patch.

Like this.  Okay for trunk, 6, 5?

Thanks,


Segher


2016-09-20  Segher Boessenkool  

* doc/rtl.texi (JUMP_LABEL): Document RETURN and SIMPLE_RETURN values.

---
 gcc/doc/rtl.texi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/doc/rtl.texi b/gcc/doc/rtl.texi
index 1b3f47e..692d9b5 100644
--- a/gcc/doc/rtl.texi
+++ b/gcc/doc/rtl.texi
@@ -3525,8 +3525,8 @@ and @code{addr_diff_vec}, where @code{JUMP_LABEL} is 
@code{NULL_RTX}
 and the only way to find the labels is to scan the entire body of the
 insn.
 
-Return insns count as jumps, but since they do not refer to any
-labels, their @code{JUMP_LABEL} is @code{NULL_RTX}.
+Return insns count as jumps, but their @code{JUMP_LABEL} is @code{RETURN}
+or @code{SIMPLE_RETURN}.
 
 @findex call_insn
 @item call_insn
-- 
1.9.3

[committed] Adjust fall through comment

[Marek Polacek]

This patch adjusts a fall through comment so that our -Wimplicit-fallthrough
comment parsing handles this well.

Applying to trunk.

2016-09-20  Marek Polacek  

* trans-intrinsic.c (conv_expr_ref_to_caf_ref): Adjust fall through
comment.

diff --git gcc/fortran/trans-intrinsic.c gcc/fortran/trans-intrinsic.c
index d6453c5..c6883dc 100644
--- gcc/fortran/trans-intrinsic.c
+++ gcc/fortran/trans-intrinsic.c
@@ -1365,7 +1365,7 @@ conv_expr_ref_to_caf_ref (stmtblock_t *block, gfc_expr 
*expr)
   handling easier.  */
stride = gfc_index_one_node;
 
- /* Intentionally fall through.  */
+ /* Fall through.  */
case DIMEN_ELEMENT:
  if (ref->u.ar.start[i])
{

Marek

Re: [PATCHv3, resent] Add a warning for suspicious use of conditional expressions in boolean context

[Bernd Edlinger]

On 09/20/16 16:51, Jason Merrill wrote:
> On Tue, Sep 20, 2016 at 10:11 AM, Bernd Edlinger
>  wrote:
>> On 09/20/16 13:29, Kyrill Tkachov wrote:
>>>
>>> arm bootstrap is now failing:
>>> $SRC/gcc/config/arm/arm.h:2229:40: error: ?: using integer constants in
>>> boolean context [-Werror=int-in-bool-context]
>>>   : (TARGET_VFP_DOUBLE ? (TARGET_FP16 ? 14 : 12) : 0)) \
>>>  ~^~
>>> $SRC/gcc/config/arm/arm-c.c:133:7: note: in expansion of macro
>>> 'TARGET_ARM_FP'
>>>  if (TARGET_ARM_FP)
>>>
>>>
>>> The full definition of TARGET_ARM_FP is:
>>> #define TARGET_ARM_FP\
>>> (!TARGET_SOFT_FLOAT ? (TARGET_VFP_SINGLE ? 4\
>>>   : (TARGET_VFP_DOUBLE ? (TARGET_FP16 ? 14 : 12) : 0)) \
>>> : 0)
>>>
>>> We want it set to 0 when there's no FP but when FP is available we set
>>> it to a bitmask
>>> to suggest the level that is available. That seems like a legitimate use
>>> to me.
>>>
>>
>> Ok, I see, sorry for that.
>>
>> I think I will have to suppress the warning if the conditional is in
>> a macro somehow.
>
> from_macro_expansion_at will help with that.
>
> Though it seems to me that the issue here is more that (TARGET_FP16 ?
> 14 : 12) is not in a boolean context, it's in an integer context; only
> the outer ?: is in a boolean context.
>
> I also still think the warning message should be changed.
>

I try this:

Index: c-common.c
===
--- c-common.c  (revision 240268)
+++ c-common.c  (working copy)
@@ -4652,7 +4652,8 @@
   TREE_OPERAND (expr, 0));

  case COND_EXPR:
-  if (warn_int_in_bool_context)
+  if (warn_int_in_bool_context
+ && !from_macro_expansion_at (EXPR_LOCATION (expr)))
{
  tree val1 = fold_for_warn (TREE_OPERAND (expr, 1));
  tree val2 = fold_for_warn (TREE_OPERAND (expr, 2));
@@ -4663,7 +4664,8 @@
  && (!integer_onep (val1)
  || !integer_onep (val2)))
warning_at (EXPR_LOCATION (expr), OPT_Wint_in_bool_context,
-   "?: using integer constants in boolean context");
+   "?: using integer constants in boolean context, "
+   "the expression will always evaluate to %");
}
/* Distribute the conversion into the arms of a COND_EXPR.  */
if (c_dialect_cxx ())


That will fix "if (TARGET_ARM_FP)" which is fine.

But this seems to suppress all such warnings from an assert
macro too.  Like for instance "assert(a?1:2)".

Sure, we would not be interested in a ?: that is part of the assert
macro itself, but the expression that is evaluated by the macro should
be checked, but that is no longer done, because the macro parameter is
now also from the macro expansion.  But it is initially from the
macro invocation point.  Ideas?


Thanks
Bernd.

Re: Fix libgo syscall test on Solaris

[Ian Lance Taylor]

On Mon, Sep 12, 2016 at 1:07 AM, Rainer Orth
 wrote:
> The libgo syscall test has been failing on Solaris for quite some time:
>
> exec_unix_test.go:174:19: error: reference to undefined identifier 
> 'syscall.Ioctl'
>   errno := syscall.Ioctl(tty.Fd(), syscall.TIOCGPGRP, 
> uintptr(unsafe.Pointer()))
>^
> exec_unix_test.go:209:18: error: reference to undefined identifier 
> 'syscall.Ioctl'
>   errno = syscall.Ioctl(tty.Fd(), syscall.TIOCSPGRP, 
> uintptr(unsafe.Pointer()))
>   ^
> FAIL: syscall
>
> The following patch fixes it, tested across the whole {i386-pc,
> sparc-sun}-solaris2.1[012] range.

Thanks.  Committed to mainline.

Ian

Re: [PATCH] libstdc++/77641 fix std::variant for literal types

[Tim Shen]

On Mon, Sep 19, 2016 at 1:06 PM, Tim Shen  wrote:
> I believe that it's a "typo" from me - it should be
> is_trivially_destructible, not is_default_constructible (so that we
> can avoid using aligned_storage in the corresponding specialization).
> is_literal_type works, since literal types are trivially destructible.

Sorry I misunderstood your patch.

The underlying problem is that _Variant_storage wasn't always default
constructible, but it should be.

Jon, your fix doesn't fix the following constexpr variation of your test case:
  struct X {
constexpr X() { }
  };

  constexpr std::variant v1 = X{};

So I have a patch here to make _Variant_storage's internal union
default constructible.

Tested on x86_64-linux-gnu.


-- 
Regards,
Tim Shen


a.diff
Description: Binary data

Re: [PATCH] - improve sprintf buffer overflow detection (middle-end/49905)

[Martin Sebor]

David, in the way of feedback, I spent hours debugging the simple
multiline test I added.  It seems that DejaGnu is exquisitely
sensitive to whitespaces in the multiline output.  I appreciate
that spacing is essential to lining up the caret and the tildes
with the text of the program but tests fail even when all the
expected output is lined up right in the multiline directive but
off by a space (or tab) with respect to the actual output.  That
DejaGnu output doesn't make this very clear at all makes debugging
these types of issues a pain.  I wonder if there's a better to do
this.


Gah; I'm sorry this was such a pain to do.

I tend to just copy the stuff I want to quote directly from
Emacs's compilation buffer.


Yes, I did start with that.  The problem is that I have a script
that I run to help massage my patches to the format required by
the GCC Coding Conventions.  The script mainly replaces blocks
of spaces with tabs. So while my initial patch worked, the final
one didn't because of the tabs, and it took me hours to figure
out why.  Because I couldn't see the difference I thought it was
a bug in DejaGnu or some other tool that was different between
the two machines I was using for testing the two patches.  Until
it occurred to me to diff the test itself... (FWIW, I think this
style convention is bad and should be abolished in favor of using
only plain spaces.)



However a nit, which I think is why you had problems...


...


You have two pairs of dg-begin/end-multiline-output, but I think it's
better to have four; use a single begin/end pair for each call to
diagnostic_show_locus.  Hopefully doing that ought to make things a bit
less sensitive to whitespace, and easier to debug: each begin/end can
be handled by itself, whereas by combining them it's harder for
multiline.exp to detect them.  If combined, they can only be detected
if all of the dg-warning/dg-messages work (and are thus pruned by
prune.exp), if any aren't pruned, the multiline outputs will also fail.
 Maybe this exacerbated the problem?


Maybe.  I remember experimenting with the multiline directives when
I first added the test and was struggling to get it to work.  I think
the problems I was having were unrelated to tabs but probably had
something to do with the exact number of leading spaces.

FWIW, to make the multiline directives less prone to this type of
a problem it seems that instead of counting each leading space and
tab character and treating them as ordinary they could verify that
the first non-whitespace character on each line of the actual output
lines up with the first non-whitespace character of the expected
output with some constant offset.  That way spaces vs tabs shouldn't
matter as long as they were used consistently (or they could be made
not to matter at all if a tab was treated as a single space).  What
do you think about that?



So something like this, grouping the 4 diagnostics together with their
diagnostic_show_locus output by opening and closing comments (line
numbers will need adjusting):


Thanks.  Let me apply it and see how it works.



Another nit, if I may: FWIW I'm not in love with the wording of the messages.  
Sorry to bikeshed, but how about:
  warning: buffer overflow will occur when writing terminating NUL
and:
  note: formatted output of 2 bytes into a destination of size 1
or somesuch.


I won't claim the text of the messages is perfect but I did spend
a lot of time tweaking them.  That's not to say they can't be
improved but changing them means quite a bit of work adjusting
the tests.  At this point, I'd like to focus on getting the patch
committed.  After that, if there's still time, I'm happy to take
feedback and tweak the diagnostics based on it.

Thanks again for your help and the suggestions above!

Martin

[PATCH, i386]: Use pow2p_hwi some more

[Uros Bizjak]

2016-09-20  Uros Bizjak  

* config/i386/i386.md (mult->ashift peephole2s): Use pow2p_hwi
instead of exact_log2.

Bootstrapped and regression tested on x86_64-linux-gnu {,-m32} ,
committed to mainline SVN.

Uros.
Index: config/i386/i386.md
===
--- config/i386/i386.md (revision 240276)
+++ config/i386/i386.md (working copy)
@@ -18304,7 +18304,7 @@
   [(set (match_operand:SWI48 0 "register_operand")
(mult:SWI48 (match_dup 0)
(match_operand:SWI48 1 "const_int_operand")))]
-  "exact_log2 (INTVAL (operands[1])) >= 0
+  "pow2p_hwi (INTVAL (operands[1]))
&& peep2_regno_dead_p (0, FLAGS_REG)"
   [(parallel [(set (match_dup 0) (ashift:SWI48 (match_dup 0) (match_dup 1)))
  (clobber (reg:CC FLAGS_REG))])]
@@ -18316,7 +18316,7 @@
  (mult:SI (match_operand:SI 1 "register_operand")
   (match_operand:SI 2 "const_int_operand"]
   "TARGET_64BIT
-   && exact_log2 (INTVAL (operands[2])) >= 0
+   && pow2p_hwi (INTVAL (operands[2]))
&& REGNO (operands[0]) == REGNO (operands[1])
&& peep2_regno_dead_p (0, FLAGS_REG)"
   [(parallel [(set (match_dup 0)

Re: [PATCH][ARM][v2] Use snprintf rather than sprintf

[Pedro Alves]

On 09/20/2016 05:49 PM, Kyrill Tkachov wrote:

> Ok, I'm proposing a new function defined in system.h
> (though I'm open to suggestions for other location).
> It would be something like:
> 
> static inline int ATTRIBUTE_PRINTF_3
> gcc_snprintf (char *str, size_t size, const char *format, ...)
> {
>   va_list ap;
>   va_start(ap, format);
>   size_t res = vsnprintf (str, size, format, ap);
>   va_end (ap);
>   /* vsnprintf returns >= size if input was truncated.  */
>   gcc_assert (res < size);
>   return res;
> }
> 
> Would that be acceptable?

gdb has had exactly that for eons:

int
xsnprintf (char *str, size_t size, const char *format, ...)
{
  va_list args;
  int ret;

  va_start (args, format);
  ret = vsnprintf (str, size, format, args);
  gdb_assert (ret < size);
  va_end (args);

  return ret;
}

Maybe reuse the same name?  It follows the naming scheme of
xmalloc, etc., with x meaning it never fails.

Even better would be to put this in libiberty.

And perhaps even better would be to get rid of the hardcoded
buffer sizes and use libiberty's existing xasprintf instead.

Go patch committed: Avoid warning by using a local var for std::ofstream

[Ian Lance Taylor]

GCC PR 77625 is about a warning while compiling the Go frontend.  The
Go frontend called `new std::ofstream()`, and the warning is "error:
‘new’ of type ‘std::ofstream {aka std::basic_ofstream}’ with
extended alignment 16".  Frankly I'm not sure how this supposed to
work: shouldn't it be possible to write new std::ofstream()?  But the
problem is easy to avoid, since in this case the std::ofstream can be
a local variable, and that is a better approach anyhow.  This patch
was bootstrapped on x86_64-pc-linux-gnu.  Committed to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 240275)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-80720773ac1a3433b7de59ffa5c04744123247c3
+57d120d75be87c2a0da67e750f16929891f1b8f4
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/ast-dump.cc
===
--- gcc/go/gofrontend/ast-dump.cc   (revision 240053)
+++ gcc/go/gofrontend/ast-dump.cc   (working copy)
@@ -166,24 +166,24 @@ const char* kAstDumpFileExtension = ".du
 void
 Ast_dump_context::dump(Gogo* gogo, const char* basename)
 {
-  std::ofstream* out = new std::ofstream();
+  std::ofstream out;
   std::string dumpname(basename);
   dumpname += ".dump.ast";
-  out->open(dumpname.c_str());
+  out.open(dumpname.c_str());
 
-  if (out->fail())
+  if (out.fail())
 {
   error("cannot open %s:%m, -fgo-dump-ast ignored", dumpname.c_str());
   return;
 }
 
   this->gogo_ = gogo;
-  this->ostream_ = out;
+  this->ostream_ = 
 
   Ast_dump_traverse_blocks_and_functions adtbf(this);
   gogo->traverse();
 
-  out->close();
+  out.close();
 }
 
 // Dump a textual representation of a type to the

Re: [PATCH 0/8] make next_*_insn take rtx_insn * arguments

[Jeff Law]

On 09/20/2016 11:11 AM, Segher Boessenkool wrote:

On Tue, Sep 20, 2016 at 08:52:46AM -0600, Jeff Law wrote:

- JUMP_LABEL can be a return which is not an insn.  That also seems
 like something that should be improved at some point.

The JUMP_LABEL field within a JUMP_INSN?  That sounds like a bug.


yes, see the comment before the declaration of
rtx_jump_insn::jump_label ()
it can be either a possibly deleted label, or a RETURN or SIMPLE_RETURN
expression.  Memory usage concerns aside its a tempting design to
change, but at the moment itts definitely intended to work this way,
there's plenty of code that checks ANY_RETURN_P (JUMP_LABEL (x)).


Yes.  But rtl.texi still says:

 Return insns count as jumps, but since they do not refer to any
 labels, their @code{JUMP_LABEL} is @code{NULL_RTX}.


And the comment at the top of jump.c:

  Each CODE_LABEL has a count of the times it is used
  stored in the LABEL_NUSES internal field, and each JUMP_INSN
  has one label that it refers to stored in the
  JUMP_LABEL internal field.  With this we can detect labels that
  become unused because of the deletion of all the jumps that
  formerly used them.  The JUMP_LABEL info is sometimes looked
  at by later passes.  For return insns, it contains either a
  RETURN or a SIMPLE_RETURN rtx.

It's intentional, and really there's nothing wrong with it, or with
operands[] containing CODE_LABELs. The problem is trying to force a


I'm just pointing out the documentation is out-of-date here.  I'll do
a patch.


static type system onto a dynamically typed data structure.

But what this doesn't answer is why we stuff RETURN/SIMPLE_RETURN into
the JUMP_LABEL field of a return.  I simply can't remember any rationale
behind that.


It is very inconvenient to parse the whole rtx to see if this is a
RETURN vs. a SIMPLE_RETURN (it can be inside a conditional or inside
a PARALLEL, etc.)
So it's similar to how we use JUMP_LABEL to find the jump target without 
having to dig through the whole rtx.





Ideally we would not have RETURN at all anymore, just SIMPLE_RETURN,
but that isn't going to happen any time soon.

And it wouldn't totally resolve this anyway.




I suspect that if we dig further, we'll find that we can accomplish
whatever the goal was behind that decision in a way that easily works in
a world where we have separated rtx objects from objects that are part
of the insn chain.


We cannot easily extract the JUMP_LABEL of a normal jump from its rtx
pattern either (if at all).  So it seems the extra field of a JUMP_INSN
is here to stay.  We'll have to figure out how to nicely do INSN vs.
JUMP_INSN in an rtx_insn world.

Both INSN and JUMP_INSNs are rtx_insn *s.

The problem is we're stuffing a (return) or (simple_return) rtx into a 
field that most of the time has an rtx_insn * (CODE_LABEL).


A hack-ish way to address this would be to create special CODE_LABELs 
that represent (return) and (simple_return) and stuff that into the 
JUMP_LABEL field.  At which point the JUMP_LABEL field should turn into 
an rtx_insn * and the as_a casts related to this wart go away.


There may be cleaner ways, but there's certainly ways to move forward here.




Just to be clear, I don't see us going to a world where everything we
have as an rtx today is a statically typed object. But there are things
that I think make sense to carve out, the most obvious being objects
which are part of the insn chain.


Agreed on both.  I have nightmares about the first, so please don't even
talk about going there :-)
While I could see some value in static typing enough to allow static 
checking things like we don't try to access out-of-range operands (say 
XEXP (x, 1)) where X is a unary code.  I don't see the level of pain to 
get there being worth it.


It's much like what we see in the tree world.  There's value in 
separating _DECL, _TYPE and _EXPR nodes.  But going deeper into in the 
_EXPR nodes seems like a huge level of pain.


jeff

Re: [PATCH] Optimise the fpclassify builtin to perform integer operations when possible

[Joseph Myers]

On Tue, 20 Sep 2016, Jeff Law wrote:

> Could we defer lowering in the case where the object is not addressable until
> gimple->rtl expansion time?  That's the best time to introduce target
> dependencies into the code we generate.

If we do that (remembering that -fsignaling-nans always wants the integer 
path), we need to make sure there are tests of fpclassify that reliably 
exercise both paths

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

Re: C/C++ PATCH to implement -Wbool-operation (PR c/77490)

[Marek Polacek]

On Mon, Sep 19, 2016 at 02:41:04PM -0400, Jason Merrill wrote:
> On 09/16/2016 05:01 AM, Marek Polacek wrote:
> > @@ -5853,7 +5853,16 @@ cp_build_unary_op (enum tree_code code, tree xarg, 
> > bool noconvert,
> >arg, true)))
> > errstring = _("wrong type argument to bit-complement");
> >else if (!noconvert && CP_INTEGRAL_TYPE_P (TREE_TYPE (arg)))
> > -   arg = cp_perform_integral_promotions (arg, complain);
> > +   {
> > + /* Warn if the expression has boolean value.  */
> > + location_t location = EXPR_LOC_OR_LOC (arg, input_location);
> 
> Let's move this variable to the beginning of the function; hopefully it will
> become a parameter soon.
 
Done.

> > + if ((TREE_CODE (TREE_TYPE (arg)) == BOOLEAN_TYPE
> > +  || truth_value_p (TREE_CODE (arg)))
> 
> You shouldn't need to check truth_value_p; in C++ all truth operations have
> type bool.

Oops, force of habit...

> > + && warning_at (location, OPT_Wbool_operation,
> > +"%<~%> on a boolean expression"))
> 
> And let's refer to "expression of type bool" rather than "boolean
> expression".

Adjusted (and in the C FE too).

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

2016-09-20  Marek Polacek  

PR c/77490
* c.opt (Wbool-operation): New.

* c-typeck.c (build_unary_op): Warn about bit not on expressions that
have boolean value.  Warn about ++/-- on booleans.

* typeck.c (cp_build_unary_op): Warn about bit not on expressions that
have boolean value.

* doc/invoke.texi: Document -Wbool-operation.

* c-c++-common/Wbool-operation-1.c: New test.
* gcc.dg/Wbool-operation-1.c: New test.

diff --git gcc/c-family/c.opt gcc/c-family/c.opt
index 6cf915d..0e8d68a 100644
--- gcc/c-family/c.opt
+++ gcc/c-family/c.opt
@@ -315,6 +315,10 @@ Wbool-compare
 C ObjC C++ ObjC++ Var(warn_bool_compare) Warning LangEnabledBy(C ObjC C++ 
ObjC++,Wall)
 Warn about boolean expression compared with an integer value different from 
true/false.
 
+Wbool-operation
+C ObjC C++ ObjC++ Var(warn_bool_op) Warning LangEnabledBy(C ObjC C++ 
ObjC++,Wall)
+Warn about certain operations on boolean expressions.
+
 Wframe-address
 C ObjC C++ ObjC++ Var(warn_frame_address) Warning LangEnabledBy(C ObjC C++ 
ObjC++,Wall)
 Warn when __builtin_frame_address or __builtin_return_address is used unsafely.
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 059ad1f..4006b1a 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -4196,6 +4196,22 @@ build_unary_op (location_t location, enum tree_code 
code, tree xarg,
  || (typecode == VECTOR_TYPE
  && !VECTOR_FLOAT_TYPE_P (TREE_TYPE (arg
{
+ tree e = arg;
+
+ /* Warn if the expression has boolean value.  */
+ while (TREE_CODE (e) == COMPOUND_EXPR)
+   e = TREE_OPERAND (e, 1);
+
+ if ((TREE_CODE (TREE_TYPE (arg)) == BOOLEAN_TYPE
+  || truth_value_p (TREE_CODE (e)))
+ && warning_at (location, OPT_Wbool_operation,
+"%<~%> on an expression of type bool"))
+   {
+ gcc_rich_location richloc (location);
+ richloc.add_fixit_insert_before (location, "!");
+ inform_at_rich_loc (, "did you mean to use logical "
+ "not?");
+   }
  if (!noconvert)
arg = default_conversion (arg);
}
@@ -4306,6 +4322,16 @@ build_unary_op (location_t location, enum tree_code 
code, tree xarg,
"decrement of enumeration value is invalid in C++");
}
 
+  if (TREE_CODE (TREE_TYPE (arg)) == BOOLEAN_TYPE)
+   {
+ if (code == PREINCREMENT_EXPR || code == POSTINCREMENT_EXPR)
+   warning_at (location, OPT_Wbool_operation,
+   "increment of an expression of type bool");
+ else
+   warning_at (location, OPT_Wbool_operation,
+   "decrement of an expression of type bool");
+   }
+
   /* Ensure the argument is fully folded inside any SAVE_EXPR.  */
   arg = c_fully_fold (arg, false, NULL);
 
diff --git gcc/cp/typeck.c gcc/cp/typeck.c
index c53a85a..dee17b3 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -5792,6 +5792,7 @@ cp_build_unary_op (enum tree_code code, tree xarg, bool 
noconvert,
 {
   /* No default_conversion here.  It causes trouble for ADDR_EXPR.  */
   tree arg = xarg;
+  location_t location = EXPR_LOC_OR_LOC (arg, input_location);
   tree argtype = 0;
   const char *errstring = NULL;
   tree val;
@@ -5853,7 +5854,14 @@ cp_build_unary_op (enum tree_code code, tree xarg, bool 
noconvert,
   arg, true)))
errstring = _("wrong type argument to bit-complement");
   else if (!noconvert && CP_INTEGRAL_TYPE_P (TREE_TYPE (arg)))
-   arg = 

Re: [patch,avr] Fix PR77326: CC_NONE might clobber condition code

[Denis Chertykov]

2016-09-20 13:07 GMT+03:00 Georg-Johann Lay :
> This fixes the case of CC_NONE (insn attribute for cc is "none"):
>
> Even in cases where the instructions of an insn do not change the condition
> code of the machine, they still might change some registers by clobber, set,
> etc.  If one such register overlaps an expression stored in
> cc_status.value1/2 we must reset cc_status as the value stored there no more
> represents the state of cc0.
>
> Ok to apply and backport?

Ok.
Please apply.

>
>
> Johann
>
> gcc/
> PR target/77326
> * config/avr/avr.c (avr_notice_update_cc) [CC_NONE]: If insn
> touches some regs mentioned in cc_status, do CC_STATUS_INIT.
>
> gcc/testsuite/
> PR target/77326
> * gcc.target/avr/torture/pr77326.c: New test.

Re: [PATCH 0/8] make next_*_insn take rtx_insn * arguments

[Segher Boessenkool]

On Tue, Sep 20, 2016 at 08:52:46AM -0600, Jeff Law wrote:
> >- JUMP_LABEL can be a return which is not an insn.  That also seems
> >  like something that should be improved at some point.
> The JUMP_LABEL field within a JUMP_INSN?  That sounds like a bug.
> >>>
> >>>yes, see the comment before the declaration of
> >>>rtx_jump_insn::jump_label ()
> >>>it can be either a possibly deleted label, or a RETURN or SIMPLE_RETURN
> >>>expression.  Memory usage concerns aside its a tempting design to
> >>>change, but at the moment itts definitely intended to work this way,
> >>>there's plenty of code that checks ANY_RETURN_P (JUMP_LABEL (x)).
> >>
> >>Yes.  But rtl.texi still says:
> >>
> >>  Return insns count as jumps, but since they do not refer to any
> >>  labels, their @code{JUMP_LABEL} is @code{NULL_RTX}.
> >
> >And the comment at the top of jump.c:
> >
> >   Each CODE_LABEL has a count of the times it is used
> >   stored in the LABEL_NUSES internal field, and each JUMP_INSN
> >   has one label that it refers to stored in the
> >   JUMP_LABEL internal field.  With this we can detect labels that
> >   become unused because of the deletion of all the jumps that
> >   formerly used them.  The JUMP_LABEL info is sometimes looked
> >   at by later passes.  For return insns, it contains either a
> >   RETURN or a SIMPLE_RETURN rtx.
> >
> >It's intentional, and really there's nothing wrong with it, or with
> >operands[] containing CODE_LABELs. The problem is trying to force a

I'm just pointing out the documentation is out-of-date here.  I'll do
a patch.

> >static type system onto a dynamically typed data structure.
> But what this doesn't answer is why we stuff RETURN/SIMPLE_RETURN into 
> the JUMP_LABEL field of a return.  I simply can't remember any rationale 
> behind that.

It is very inconvenient to parse the whole rtx to see if this is a
RETURN vs. a SIMPLE_RETURN (it can be inside a conditional or inside
a PARALLEL, etc.)

Ideally we would not have RETURN at all anymore, just SIMPLE_RETURN,
but that isn't going to happen any time soon.

> I suspect that if we dig further, we'll find that we can accomplish 
> whatever the goal was behind that decision in a way that easily works in 
> a world where we have separated rtx objects from objects that are part 
> of the insn chain.

We cannot easily extract the JUMP_LABEL of a normal jump from its rtx
pattern either (if at all).  So it seems the extra field of a JUMP_INSN
is here to stay.  We'll have to figure out how to nicely do INSN vs.
JUMP_INSN in an rtx_insn world.

> Just to be clear, I don't see us going to a world where everything we 
> have as an rtx today is a statically typed object. But there are things 
> that I think make sense to carve out, the most obvious being objects 
> which are part of the insn chain.

Agreed on both.  I have nightmares about the first, so please don't even
talk about going there :-)


Segher

Re: [PATCH] accept flexible arrays in struct in unions (c++/71912 - [6/7 regression])

[Martin Sebor]

On 09/16/2016 12:19 PM, Jason Merrill wrote:

On 09/14/2016 01:03 PM, Martin Sebor wrote:

Attached is an updated patch that does away with the "overlapping"
warning and instead issues the same warnings as the C front end
(though with more context).

In struct flexmems_t I've retained the NEXT array even though only
the first element is used to diagnose problems.  The second element
is used by the find_flexarrays function to differentiate structs
with flexible array members in unions (which are valid) from those
in structs (which are not).

FWIW, I think this C restriction on structs is unnecessary and will
propose to remove it so that's valid to define a struct that contains
another struct (possibly recursively) with a flexible array member.
I.e., I think this should be made valid in C (and accepted without
the pedantic warning that GCC, and with this patch also G++, issues):


Agreed.


This is now N2083:
  http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2083.htm




+  /* Is FLD a typedef for an anonymous struct?  */
+  bool anon_type_p
+   = (TREE_CODE (fld) == TYPE_DECL
+  && DECL_IMPLICIT_TYPEDEF_P (fld)
+  && anon_aggrname_p (DECL_NAME (fld)));


We talked earlier about handling typedefs in
cp_parser_{simple,single}_declaration, so that we catch

typedef struct { int a[]; } B;

or at least adding a FIXME comment here explaining that looking at
typedefs is a temporary hack.


I've added a FIXME comment.




+  /* Type of the member.  */
+  tree fldtype = TREE_CODE (fld) == FIELD_DECL ? TREE_TYPE (fld)
: fld;


Why set "fldtype" to be a TYPE_DECL rather than its type?


I'm not sure I understand the question but (IIRC) the purpose of
this code is to detect invalid uses of flexible array members in
typedefs such as this:

   struct S { typedef struct { int i, a[]; } X2 [2]; };

Sadly, it doesn't do anything for

   struct X { int i, a[]; };
   struct S { typedef struct X X2 [2]; };


+  /* Determine the type of the array element or object referenced
+by the member so that it can be checked for flexible array
+members if it hasn't been yet.  */
+  tree eltype = TREE_CODE (fld) == FIELD_DECL ? fldtype :
TREE_TYPE (fld);


Given the above, this seems equivalent to

tree eltype = TREE_TYPE (fld);


Yes.




+  if (RECORD_OR_UNION_TYPE_P (eltype))
+   {

...

+ if (fmem->array && !fmem->after[bool (pun)])
+   {
+ /* Once the member after the flexible array has been found
+we're done.  */
+ fmem->after[bool (pun)] = fld;
+ break;
+   }

...

  if (field_nonempty_p (fld))
{

...

  /* Remember the first non-static data member after the flexible
 array member, if one has been found, or the zero-length
array
 if it has been found.  */
  if (fmem->array && !fmem->after[bool (pun)])
fmem->after[bool (pun)] = fld;
}


Can we do this in one place rather than two?


You mean merge the two assignments to fmem->after[bool (pun)] into
one?  I'm not sure.  There's quite a bit of conditional logic in
between them, including a recursive call that might set fmem.  What
would be gained by rewriting it all to merge the two assignments?
If it could be done I worry that I'd just end up duplicating the
conditions instead.




+ if (eltype == fldtype || TYPE_UNNAMED_P (eltype))


This is excluding arrays, so we don't diagnose, say,


struct A
{
  int i;
  int ar[];
};

struct B {
  A a[2];
};


Should we complain elsewhere about an array of a class with a flexible
array member?


Yes, there are outstanding gaps/bugs that remain to be fixed.  I tried
to limit this fix to not much more than the reported regression.  I did
end up tightening other things up as I found more gaps during testing
(though I probably should have resisted).  Sometimes I find it hard to
know where to stop but I feel like I need to draw the line somewhere.
I of course agree that the outstanding bugs should be fixed as well
but I'd prefer to do it in a separate change.


+ /* Does the field represent an anonymous struct?  */
+ bool anon_p = !DECL_NAME (fld) && ANON_AGGR_TYPE_P
(eltype);


You don't need to check DECL_NAME; ANON_AGGR_TYPE_P by itself indicates
that we're dealing with an anonymous struct/union.


Thanks, I've removed the variable.




   Similarly, PSTR is set to the outermost struct of which T is a member
   if one such struct exists, otherwise to NULL.  */

...

  find_flexarrays (eltype, fmem, false, pun,
   !pstr && TREE_CODE (t) == RECORD_TYPE ?
fld : pstr);

...

+diagnose_invalid_flexarray (const flexmems_t *fmem)
+{
+  if (fmem->array && fmem->enclosing
+  && pedwarn (location_of (fmem->enclosing), OPT_Wpedantic,
+ TYPE_DOMAIN (TREE_TYPE (fmem->array))
+ ? G_("invalid use of %q#T with a zero-size array "
+   

[PATCH, i386]: Fix PR77621 (target part), handle Atom V2DFmode tuning through vector cost infrastructure

[Uros Bizjak]

Hello!

Attached patch improves Atom V2DFmode vectorization by penalizing V2DF
mode insns through vector cost infrastructure. Looking at Agner Fog's
instruction tables, it is evident that only V2DFmode arithmetic insns
(e.g. addpd, maxpd, sqrtpd, ...) have increased latencies, and we
shouldn't fully disable V2DFmode vectorization with a "big hammer"
approach.

As suggested in the PR, we now increase the cost of problematic insns
in ix86_add_stmt_cost. This is enough to prevent vectorization of
V2DFmode loops, but we still allow cases where a single problematic
insn would prevent vectorization of a complex, mostly integer loop.

(BTW: The factor 5 is arbitrary, but based on the factor between
latencies of V2DFmode  and DFmode insns).

2016-09-20  Uros Bizjak  

PR target/77621
* config/i386/i386.c (ix86_preferred_simd_mode) :
Don't return word_mode for !TARGET_VECTORIZE_DOUBLE.
(ix86_add_stmt_cost): Penalize DFmode vector operations
for !TARGET_VECTORIZE_DOUBLE.

testsuite/ChangeLog:

2016-09-20  Uros Bizjak  

PR target/77621
* gcc.target/i386/pr77621.c: New test.
* gcc.target/i386/vect-double-2.c: Update scan-tree-dump-times
pattern, loop should vectorize with -mtune=atom.

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

Committed to mainline SVN.

Uros.
Index: config/i386/i386.c
===
--- config/i386/i386.c  (revision 240263)
+++ config/i386/i386.c  (working copy)
@@ -49554,9 +49554,7 @@ ix86_preferred_simd_mode (machine_mode mode)
return V4SFmode;
 
 case DFmode:
-  if (!TARGET_VECTORIZE_DOUBLE)
-   return word_mode;
-  else if (TARGET_AVX512F)
+  if (TARGET_AVX512F)
return V8DFmode;
   else if (TARGET_AVX && !TARGET_PREFER_AVX128)
return V4DFmode;
@@ -49647,9 +49645,14 @@ ix86_add_stmt_cost (void *data, int count, enum ve
   tree vectype = stmt_info ? stmt_vectype (stmt_info) : NULL_TREE;
   int stmt_cost = ix86_builtin_vectorization_cost (kind, vectype, misalign);
 
+  /* Penalize DFmode vector operations for !TARGET_VECTORIZE_DOUBLE.  */
+  if (kind == vector_stmt && !TARGET_VECTORIZE_DOUBLE
+  && vectype && GET_MODE_INNER (TYPE_MODE (vectype)) == DFmode)
+stmt_cost *= 5;  /* FIXME: The value here is arbitrary.  */
+
   /* Statements in an inner loop relative to the loop being
  vectorized are weighted more heavily.  The value here is
-  arbitrary and could potentially be improved with analysis.  */
+ arbitrary and could potentially be improved with analysis.  */
   if (where == vect_body && stmt_info && stmt_in_inner_loop_p (stmt_info))
 count *= 50;  /* FIXME.  */
 
Index: testsuite/gcc.target/i386/pr77621.c
===
--- testsuite/gcc.target/i386/pr77621.c (nonexistent)
+++ testsuite/gcc.target/i386/pr77621.c (working copy)
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -mtune=atom -msse2 -fdump-tree-vect-stats" } */
+
+void
+foo (double *x, int *y)
+{
+  int i;
+  for (i = 0; i < 8; i++)
+x[i] -= y[i] * x[i + 1];
+}
+
+/* { dg-final { scan-tree-dump-not "Vectorized loops: 1" "vect" } } */
Index: testsuite/gcc.target/i386/vect-double-2.c
===
--- testsuite/gcc.target/i386/vect-double-2.c   (revision 240263)
+++ testsuite/gcc.target/i386/vect-double-2.c   (working copy)
@@ -31,4 +31,4 @@ sse2_test (void)
 }
 }
 
-/* { dg-final { scan-tree-dump-not "vectorized 1 loops" "vect" } } */
+/* { dg-final { scan-tree-dump-times "Vectorized loops: 1" 1 "vect" } } */

Re: [PATCH][ARM][v2] Use snprintf rather than sprintf

[Richard Earnshaw (lists)]

On 20/09/16 17:49, Kyrill Tkachov wrote:
> 
> On 20/09/16 15:13, Richard Earnshaw (lists) wrote:
>> On 08/09/16 12:00, Kyrill Tkachov wrote:
>>> Hi all,
>>>
>>> This is a rebase and slight respin of [1] that I sent out last November
>>> to change all uses of
>>> sprintf to snprintf in the arm backend.
>>>
>>> Bootstrapped and tested on arm-none-linux-gnueabihf.
>>> Ok for trunk?
>>>
>>> Thanks,
>>> Kyrill
>>>
>>> [1] https://gcc.gnu.org/ml/gcc-patches/2015-11/msg00937.html
>>>
>>> 2016-09-08  Kyrylo Tkachov  
>>>
>>>  * config/arm/arm.c (arm_set_fixed_optab_libfunc): Use snprintf
>>>  rather than sprintf.
>>>  (arm_set_fixed_conv_libfunc): Likewise.
>>>  (arm_option_override): Likewise.
>>>  (neon_output_logic_immediate): Likewise.
>>>  (neon_output_shift_immediate): Likewise.
>>>  (arm_output_multireg_pop): Likewise.
>>>  (vfp_output_vstmd): Likewise.
>>>  (output_move_vfp): Likewise.
>>>  (output_move_neon): Likewise.
>>>  (output_return_instruction): Likewise.
>>>  (arm_elf_asm_cdtor): Likewise.
>>>  (arm_output_shift): Likewise.
>>>  (arm_output_iwmmxt_shift_immediate): Likewise.
>>>  (arm_output_iwmmxt_tinsr): Likewise.
>>>  * config/arm/neon.md (*neon_mov, VDX): Likewise.
>>>  (*neon_mov, VQXMOV): Likewise.
>>>  (neon_vc_insn): Likewise.
>>>  (neon_vc_insn_unspec): Likewise.
>> Most of these should assert that truncation did not occur (it's an
>> internal error if the buffers aren't large enough).  In a few cases we
>> should call output_operand_lossage on failure since it might be due to a
>> user writing inline assembly and overflowing a buffer.
>>
>> I don't think we should silently accept a truncated write.
> 
> Ok, I'm proposing a new function defined in system.h
> (though I'm open to suggestions for other location).
> It would be something like:
> 
> static inline int ATTRIBUTE_PRINTF_3
> gcc_snprintf (char *str, size_t size, const char *format, ...)
> {
>   va_list ap;
>   va_start(ap, format);
>   size_t res = vsnprintf (str, size, format, ap);
>   va_end (ap);
>   /* vsnprintf returns >= size if input was truncated.  */
>   gcc_assert (res < size);
>   return res;
> }
> 
> Would that be acceptable?
> 

Yes, that's sort of what I had in mind.

Not sure if it needs to be inline, though; *printf are not generally
performance-critical functions and this bloats code somewhat with two
function calls for each invocation.

R.

> Thanks,
> Kyrill
> 
>>
>> R.
>>
>>> arm-snprintf.patch
>>>
>>>
>>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
>>> index
>>> 946f308ca84e232af8af6eca4813464914cbd59c..8ff6c0e18f7c2a2eed72e56402ed582c9f692d2d
>>> 100644
>>> --- a/gcc/config/arm/arm.c
>>> +++ b/gcc/config/arm/arm.c
>>> @@ -2388,9 +2388,10 @@ arm_set_fixed_optab_libfunc (optab optable,
>>> machine_mode mode,
>>> char buffer[50];
>>>   if (num_suffix == 0)
>>> -sprintf (buffer, "__gnu_%s%s", funcname, modename);
>>> +snprintf (buffer, sizeof (buffer), "__gnu_%s%s", funcname,
>>> modename);
>>> else
>>> -sprintf (buffer, "__gnu_%s%s%d", funcname, modename, num_suffix);
>>> +snprintf (buffer, sizeof (buffer), "__gnu_%s%s%d", funcname,
>>> +  modename, num_suffix);
>>>   set_optab_libfunc (optable, mode, buffer);
>>>   }
>>> @@ -2409,8 +2410,8 @@ arm_set_fixed_conv_libfunc (convert_optab
>>> optable, machine_mode to,
>>> && ALL_FRACT_MODE_P (from) == ALL_FRACT_MODE_P (to))
>>>   maybe_suffix_2 = "2";
>>>   -  sprintf (buffer, "__gnu_%s%s%s%s", funcname, fromname, toname,
>>> -   maybe_suffix_2);
>>> +  snprintf (buffer, sizeof (buffer), "__gnu_%s%s%s%s", funcname,
>>> +fromname, toname, maybe_suffix_2);
>>>   set_conv_libfunc (optable, to, from, buffer);
>>>   }
>>> @@ -3163,7 +3164,8 @@ arm_option_override (void)
>>> if (!arm_selected_tune)
>>>   arm_selected_tune = _cores[arm_selected_cpu->core];
>>>   -  sprintf (arm_arch_name, "__ARM_ARCH_%s__", arm_selected_cpu->arch);
>>> +  snprintf (arm_arch_name, sizeof (arm_arch_name),
>>> +"__ARM_ARCH_%s__", arm_selected_cpu->arch);
>>> insn_flags = arm_selected_cpu->flags;
>>> arm_base_arch = arm_selected_cpu->base_arch;
>>>   @@ -12735,9 +12737,11 @@ neon_output_logic_immediate (const char
>>> *mnem, rtx *op2, machine_mode mode,
>>> gcc_assert (is_valid != 0);
>>>   if (quad)
>>> -sprintf (templ, "%s.i%d\t%%q0, %%2", mnem, width);
>>> +snprintf (templ, sizeof (templ), "%s.i%d\t%%q0, %%2",
>>> +  mnem, width);
>>> else
>>> -sprintf (templ, "%s.i%d\t%%P0, %%2", mnem, width);
>>> +snprintf (templ, sizeof (templ), "%s.i%d\t%%P0, %%2",
>>> +  mnem, width);
>>>   return templ;
>>>   }
>>> @@ -12757,9 +12761,11 @@ neon_output_shift_immediate (const char
>>> *mnem, char sign, rtx *op2,
>>> gcc_assert (is_valid != 0);
>>>   if (quad)
>>> -sprintf (templ, "%s.%c%d\t%%q0, %%q1, %%2", mnem, sign, width);
>>> +

Re: [PATCH][ARM][v2] Use snprintf rather than sprintf

[Kyrill Tkachov]

On 20/09/16 15:13, Richard Earnshaw (lists) wrote:

On 08/09/16 12:00, Kyrill Tkachov wrote:

Hi all,

This is a rebase and slight respin of [1] that I sent out last November
to change all uses of
sprintf to snprintf in the arm backend.

Bootstrapped and tested on arm-none-linux-gnueabihf.
Ok for trunk?

Thanks,
Kyrill

[1] https://gcc.gnu.org/ml/gcc-patches/2015-11/msg00937.html

2016-09-08  Kyrylo Tkachov  

 * config/arm/arm.c (arm_set_fixed_optab_libfunc): Use snprintf
 rather than sprintf.
 (arm_set_fixed_conv_libfunc): Likewise.
 (arm_option_override): Likewise.
 (neon_output_logic_immediate): Likewise.
 (neon_output_shift_immediate): Likewise.
 (arm_output_multireg_pop): Likewise.
 (vfp_output_vstmd): Likewise.
 (output_move_vfp): Likewise.
 (output_move_neon): Likewise.
 (output_return_instruction): Likewise.
 (arm_elf_asm_cdtor): Likewise.
 (arm_output_shift): Likewise.
 (arm_output_iwmmxt_shift_immediate): Likewise.
 (arm_output_iwmmxt_tinsr): Likewise.
 * config/arm/neon.md (*neon_mov, VDX): Likewise.
 (*neon_mov, VQXMOV): Likewise.
 (neon_vc_insn): Likewise.
 (neon_vc_insn_unspec): Likewise.

Most of these should assert that truncation did not occur (it's an
internal error if the buffers aren't large enough).  In a few cases we
should call output_operand_lossage on failure since it might be due to a
user writing inline assembly and overflowing a buffer.

I don't think we should silently accept a truncated write.


Ok, I'm proposing a new function defined in system.h
(though I'm open to suggestions for other location).
It would be something like:

static inline int ATTRIBUTE_PRINTF_3
gcc_snprintf (char *str, size_t size, const char *format, ...)
{
  va_list ap;
  va_start(ap, format);
  size_t res = vsnprintf (str, size, format, ap);
  va_end (ap);
  /* vsnprintf returns >= size if input was truncated.  */
  gcc_assert (res < size);
  return res;
}

Would that be acceptable?

Thanks,
Kyrill



R.


arm-snprintf.patch


diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 
946f308ca84e232af8af6eca4813464914cbd59c..8ff6c0e18f7c2a2eed72e56402ed582c9f692d2d
 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -2388,9 +2388,10 @@ arm_set_fixed_optab_libfunc (optab optable, machine_mode 
mode,
char buffer[50];
  
if (num_suffix == 0)

-sprintf (buffer, "__gnu_%s%s", funcname, modename);
+snprintf (buffer, sizeof (buffer), "__gnu_%s%s", funcname, modename);
else
-sprintf (buffer, "__gnu_%s%s%d", funcname, modename, num_suffix);
+snprintf (buffer, sizeof (buffer), "__gnu_%s%s%d", funcname,
+ modename, num_suffix);
  
set_optab_libfunc (optable, mode, buffer);

  }
@@ -2409,8 +2410,8 @@ arm_set_fixed_conv_libfunc (convert_optab optable, 
machine_mode to,
&& ALL_FRACT_MODE_P (from) == ALL_FRACT_MODE_P (to))
  maybe_suffix_2 = "2";
  
-  sprintf (buffer, "__gnu_%s%s%s%s", funcname, fromname, toname,

-  maybe_suffix_2);
+  snprintf (buffer, sizeof (buffer), "__gnu_%s%s%s%s", funcname,
+   fromname, toname, maybe_suffix_2);
  
set_conv_libfunc (optable, to, from, buffer);

  }
@@ -3163,7 +3164,8 @@ arm_option_override (void)
if (!arm_selected_tune)
  arm_selected_tune = _cores[arm_selected_cpu->core];
  
-  sprintf (arm_arch_name, "__ARM_ARCH_%s__", arm_selected_cpu->arch);

+  snprintf (arm_arch_name, sizeof (arm_arch_name),
+   "__ARM_ARCH_%s__", arm_selected_cpu->arch);
insn_flags = arm_selected_cpu->flags;
arm_base_arch = arm_selected_cpu->base_arch;
  
@@ -12735,9 +12737,11 @@ neon_output_logic_immediate (const char *mnem, rtx *op2, machine_mode mode,

gcc_assert (is_valid != 0);
  
if (quad)

-sprintf (templ, "%s.i%d\t%%q0, %%2", mnem, width);
+snprintf (templ, sizeof (templ), "%s.i%d\t%%q0, %%2",
+ mnem, width);
else
-sprintf (templ, "%s.i%d\t%%P0, %%2", mnem, width);
+snprintf (templ, sizeof (templ), "%s.i%d\t%%P0, %%2",
+ mnem, width);
  
return templ;

  }
@@ -12757,9 +12761,11 @@ neon_output_shift_immediate (const char *mnem, char 
sign, rtx *op2,
gcc_assert (is_valid != 0);
  
if (quad)

-sprintf (templ, "%s.%c%d\t%%q0, %%q1, %%2", mnem, sign, width);
+snprintf (templ, sizeof (templ),
+ "%s.%c%d\t%%q0, %%q1, %%2", mnem, sign, width);
else
-sprintf (templ, "%s.%c%d\t%%P0, %%P1, %%2", mnem, sign, width);
+snprintf (templ, sizeof (templ),
+ "%s.%c%d\t%%P0, %%P1, %%2", mnem, sign, width);
  
return templ;

  }
@@ -17858,17 +17864,17 @@ arm_output_multireg_pop (rtx *operands, bool 
return_pc, rtx cond, bool reverse,
conditional = reverse ? "%?%D0" : "%?%d0";
/* Can't use POP if returning from an interrupt.  */
if ((regno_base == SP_REGNUM) && update && !(interrupt_p && return_pc))
-sprintf (pattern, "pop%s\t{", conditional);
+snprintf (pattern, 

libgo patch committed: Fix type passed to __splitstack_find

[Ian Lance Taylor]

PR 77642 points out that since the change to generate runtime types
from Go libgo is passing the wrong type to __splitstack_find: it is
passing uintptr* to a function that expects size_t*.  This breaks on
s390.  This patch, based on one from Andreas Krebbel in the PR, fixes
the problem.  Bootstrapped and ran Go testsuite on
x86_64-pc-linux-gnu.  Committed to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 240146)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-b34c93bf00ec4f2ad043ec89ff96989e0d1b26aa
+80720773ac1a3433b7de59ffa5c04744123247c3
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/runtime/proc.c
===
--- libgo/runtime/proc.c(revision 240053)
+++ libgo/runtime/proc.c(working copy)
@@ -2052,9 +2052,13 @@ doentersyscall()
 
// Leave SP around for GC and traceback.
 #ifdef USING_SPLIT_STACK
-   g->gcstack = __splitstack_find(nil, nil, >gcstacksize,
-  >gcnextsegment, >gcnextsp,
-  >gcinitialsp);
+   {
+ size_t gcstacksize;
+ g->gcstack = __splitstack_find(nil, nil, ,
+>gcnextsegment, >gcnextsp,
+>gcinitialsp);
+ g->gcstacksize = (uintptr)gcstacksize;
+   }
 #else
{
void *v;
@@ -2099,9 +2103,13 @@ runtime_entersyscallblock(void)
 
// Leave SP around for GC and traceback.
 #ifdef USING_SPLIT_STACK
-   g->gcstack = __splitstack_find(nil, nil, >gcstacksize,
-  >gcnextsegment, >gcnextsp,
-  >gcinitialsp);
+   {
+ size_t gcstacksize;
+ g->gcstack = __splitstack_find(nil, nil, ,
+>gcnextsegment, >gcnextsp,
+>gcinitialsp);
+ g->gcstacksize = (uintptr)gcstacksize;
+   }
 #else
g->gcnextsp = (byte *) 
 #endif

Re: [PATCHv3, resent] Add a warning for suspicious use of conditional expressions in boolean context

[Bernd Edlinger]

On 09/20/16 18:29, Kyrill Tkachov wrote:
>>
>> I'll try bootstrapping and testing this change on
>> arm-none-linux-gnueabihf.
>>
>
> ... and I'm hitting the same issue in sel-sched.c:
> if (fixed_regs[regno]
> || global_regs[regno]
> || (!HARD_FRAME_POINTER_IS_FRAME_POINTER && frame_pointer_needed
> && regno == HARD_FRAME_POINTER_REGNUM)
> || (HARD_FRAME_POINTER_REGNUM && frame_pointer_needed
> && regno == FRAME_POINTER_REGNUM)
>
> I think the condition was copied from regrename.c (or the other way
> around),
> so I'll fix it the same way.
>
> Kyrill
>
>

Interesting that this warning shakes so many bugs out of the gcc-tree,
but hit zero times for the whole linux and even openssl.  Even for the
more aggressive variant I had before.

I would have expected to see at least some false positives there,
but there were none.


Bernd.

Re: [patch, libgomp, OpenACC] Additional enter/exit data map handling

[Cesar Philippidis]

On 08/29/2016 12:46 AM, Chung-Lin Tang wrote:

> Index: oacc-parallel.c
> ===
> --- oacc-parallel.c   (revision 239814)
> +++ oacc-parallel.c   (working copy)
> @@ -38,15 +38,23 @@
>  #include 
>  #include 
>  
> +/* Returns the number of mappings associated with the pointer or pset. PSET
> +   have three mappings, whereas pointer have two.  */
> +
>  static int
> -find_pset (int pos, size_t mapnum, unsigned short *kinds)
> +find_pointer (int pos, size_t mapnum, unsigned short *kinds)
>  {
>if (pos + 1 >= mapnum)
>  return 0;
>  
>unsigned char kind = kinds[pos+1] & 0xff;
>  
> -  return kind == GOMP_MAP_TO_PSET;
> +  if (kind == GOMP_MAP_TO_PSET)
> +return 3;
> +  else if (kind == GOMP_MAP_POINTER)
> +return 2;
> +
> +  return 0;
>  }

Is this still necessary with the firstprivatization of subarrays
pointers? Well, it might be for fortran. Conceptually, the gimplifier
should prune out those unnecessary firstprivate pointer clauses for
executable constructs such as enter/exit data and update.

Actually, this is one area in the spec where the intent of enter/exit
data conflicts with what it describes. If you look at the runtime
documentation for, say, acc_create, it states that

  acc_create (pvar, n*sizeof(var))

is equivalent to

  acc enter data create (pvar[n])

And to free acc_create, you use acc_delete. So in theory, you should be
able to

  #pragma acc enter data create (pvar[n])
  acc_free (pvar)

but this may result in a memory leak if the pointer mapping isn't freed.

Fortran is somewhat special because of the pointer sets. I'm not sure if
its possible to make the OpenACC runtime API compatible with enter/exit
data.

>  static void goacc_wait (int async, int num_waits, va_list *ap);
> @@ -298,7 +306,9 @@ GOACC_enter_exit_data (int device, size_t mapnum,
>  
>if (kind == GOMP_MAP_FORCE_ALLOC
> || kind == GOMP_MAP_FORCE_PRESENT
> -   || kind == GOMP_MAP_FORCE_TO)
> +   || kind == GOMP_MAP_FORCE_TO
> +   || kind == GOMP_MAP_TO
> +   || kind == GOMP_MAP_ALLOC)
>   {
> data_enter = true;
> break;
> @@ -312,31 +322,39 @@ GOACC_enter_exit_data (int device, size_t mapnum,
> kind);
>  }
>  
> +  /* In c, non-pointers and arrays are represented by a single data clause.
> + Dynamically allocated arrays and subarrays are represented by a data
> + clause followed by an internal GOMP_MAP_POINTER.
> +
> + In fortran, scalars and not allocated arrays are represented by a
> + single data clause. Allocated arrays and subarrays have three mappings:
> + 1) the original data clause, 2) a PSET 3) a pointer to the array data.
> +  */
> +
>if (data_enter)
>  {
>for (i = 0; i < mapnum; i++)
>   {
> unsigned char kind = kinds[i] & 0xff;
>  
> -   /* Scan for PSETs.  */
> -   int psets = find_pset (i, mapnum, kinds);
> +   /* Scan for pointers and PSETs.  */
> +   int pointer = find_pointer (i, mapnum, kinds);
>  
> -   if (!psets)
> +   if (!pointer)
>   {
> switch (kind)
>   {
> - case GOMP_MAP_POINTER:
> -   gomp_acc_insert_pointer (1, [i], [i],
> - [i]);
> + case GOMP_MAP_ALLOC:
> +   acc_present_or_create (hostaddrs[i], sizes[i]);
> break;
>   case GOMP_MAP_FORCE_ALLOC:
> acc_create (hostaddrs[i], sizes[i]);
> break;
> - case GOMP_MAP_FORCE_PRESENT:
> + case GOMP_MAP_TO:
> acc_present_or_copyin (hostaddrs[i], sizes[i]);
> break;
>   case GOMP_MAP_FORCE_TO:
> -   acc_present_or_copyin (hostaddrs[i], sizes[i]);
> +   acc_copyin (hostaddrs[i], sizes[i]);
> break;

Thanks for correcting that. I had some of those data mappings wrong.

>   default:
> gomp_fatal (" GOACC_enter_exit_data UNHANDLED kind 
> 0x%.2x",
> @@ -346,12 +364,16 @@ GOACC_enter_exit_data (int device, size_t mapnum,
>   }
> else
>   {
> -   gomp_acc_insert_pointer (3, [i], [i], [i]);
> +   if (!acc_is_present (hostaddrs[i], sizes[i]))
> + {
> +   gomp_acc_insert_pointer (pointer, [i],
> +[i], [i]);
> + }
> /* Increment 'i' by two because OpenACC requires fortran
>arrays to be contiguous, so each PSET is associated with
>one of MAP_FORCE_ALLOC/MAP_FORCE_PRESET/MAP_FORCE_TO, and
>one MAP_POINTER.  */
> -   i += 2;
> +   i += pointer - 1;
>   }
>   }
>  }
> @@ -360,19 +382,15 @@ GOACC_enter_exit_data (int device, size_t mapnum,
>{
>   unsigned char kind = kinds[i] & 0xff;
>  
> - int psets = find_pset 

Re: [PATCH, docs] invoke.texi: random copy-editing

[Gerald Pfeifer]

On Mon, 5 Sep 2016, Sandra Loosemore wrote:
> Adjective phrases immediately before the noun they modify are 
> hyphenated. This is the same reason why we write "floating-point 
> arithmetic" but "floating point", unhyphenated, as a noun.

Thanks for the explanation / background, Sandra.

Below is the patch I just committed based on your other feedback.

Gerald

2016-09-20  Gerald Pfeifer  

* doc/invoke.texi (Warning Options): Simplify language.
(Optimize Options): Complete sentence.

Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 240270)
+++ doc/invoke.texi (working copy)
@@ -3752,8 +3752,8 @@
 @code{register}.
 
 @item
-(C++ only) A base class is not initialized in a derived class's copy
-constructor.
+(C++ only) A base class is not initialized in the copy constructor
+of a derived class.
 
 @end itemize
 
@@ -9128,9 +9128,9 @@
 optimizers.
 
 When profile feedback is available (see @option{-fprofile-generate}) the actual
-recursion depth can be guessed from probability that function recurses via a
-given call expression.  This parameter limits inlining only to call expressions
-whose probability exceeds the given threshold (in percents).
+recursion depth can be guessed from the probability that function recurses
+via a given call expression.  This parameter limits inlining only to call
+expressions whose probability exceeds the given threshold (in percents).
 The default value is 10.
 
 @item early-inlining-insns

Re: [PATCH] More trivial bits from early LTO debug merge

[David Edelsohn]

dbxout.c has two definitions of the structure: dbx and xcoff.  The
patch forgot to update the xcoff one (lucky AIX :-).

* dbxout.c (xcoff_debug_hooks):  Add filename parameter to early_finish hook.

Thanks, David

Index: dbxout.c
===
--- dbxout.c(revision 240270)
+++ dbxout.c(working copy)
@@ -388,8 +388,8 @@ const struct gcc_debug_hooks xcoff_debug_hooks =
 {
   dbxout_init,
   dbxout_finish,
+  debug_nothing_charstar,
   debug_nothing_void,
-  debug_nothing_void,
   debug_nothing_int_charstar,
   debug_nothing_int_charstar,
   dbxout_start_source_file,

Re: [PATCHv3, resent] Add a warning for suspicious use of conditional expressions in boolean context

[Kyrill Tkachov]

On 20/09/16 17:12, Kyrill Tkachov wrote:


On 20/09/16 16:30, Bernd Schmidt wrote:

On 09/20/2016 05:18 PM, Jeff Law wrote:

I assume HARD_FRAME_POINTER_REGNUM is never zero.

It could be zero.  It's just a hard register number.  No target has the
property that its hard frame pointer register is 0 though :-)


git blame to the rescue. The current state comes from one of tbsaunde's cleanup 
patches:

> diff --git a/gcc/regrename.c b/gcc/regrename.c
index 174d3b5..e5248a5 100644
--- a/gcc/regrename.c
+++ b/gcc/regrename.c
@@ -442,12 +442,10 @@ rename_chains (void)
continue;

   if (fixed_regs[reg] || global_regs[reg]
-#if !HARD_FRAME_POINTER_IS_FRAME_POINTER
- || (frame_pointer_needed && reg == HARD_FRAME_POINTER_REGNUM)
-#else
- || (frame_pointer_needed && reg == FRAME_POINTER_REGNUM)
-#endif
- )
+ || (!HARD_FRAME_POINTER_IS_FRAME_POINTER && frame_pointer_needed
+ && reg == HARD_FRAME_POINTER_REGNUM)
+ || (HARD_FRAME_POINTER_REGNUM && frame_pointer_needed
+ && reg == FRAME_POINTER_REGNUM))
continue;

   COPY_HARD_REG_SET (this_unavailable, unavailable);

Looks like it never got reviewed and was committed as preapproved.

The #ifdef we had before looks odd too. Maybe this should just read

 if (fixed_regs[reg] || global_regs[reg]
 || (frame_pointer_needed && reg == HARD_FRAME_POINTER_REGNUM))



I'll try bootstrapping and testing this change on arm-none-linux-gnueabihf.



... and I'm hitting the same issue in sel-sched.c:
   if (fixed_regs[regno]
   || global_regs[regno]
   || (!HARD_FRAME_POINTER_IS_FRAME_POINTER && frame_pointer_needed
   && regno == HARD_FRAME_POINTER_REGNUM)
   || (HARD_FRAME_POINTER_REGNUM && frame_pointer_needed
   && regno == FRAME_POINTER_REGNUM)

I think the condition was copied from regrename.c (or the other way around),
so I'll fix it the same way.

Kyrill



Thanks,
Kyrill



Bernd

Re: [PATCHv3, resent] Add a warning for suspicious use of conditional expressions in boolean context

[Bernd Edlinger]

On 09/20/16 18:12, Kyrill Tkachov wrote:
>
> On 20/09/16 16:30, Bernd Schmidt wrote:
>> On 09/20/2016 05:18 PM, Jeff Law wrote:
 I assume HARD_FRAME_POINTER_REGNUM is never zero.
>>> It could be zero.  It's just a hard register number.  No target has the
>>> property that its hard frame pointer register is 0 though :-)
>>
>> git blame to the rescue. The current state comes from one of
>> tbsaunde's cleanup patches:
>>
>> > diff --git a/gcc/regrename.c b/gcc/regrename.c
>> index 174d3b5..e5248a5 100644
>> --- a/gcc/regrename.c
>> +++ b/gcc/regrename.c
>> @@ -442,12 +442,10 @@ rename_chains (void)
>> continue;
>>
>>if (fixed_regs[reg] || global_regs[reg]
>> -#if !HARD_FRAME_POINTER_IS_FRAME_POINTER
>> - || (frame_pointer_needed && reg == HARD_FRAME_POINTER_REGNUM)
>> -#else
>> - || (frame_pointer_needed && reg == FRAME_POINTER_REGNUM)
>> -#endif
>> - )
>> + || (!HARD_FRAME_POINTER_IS_FRAME_POINTER &&
>> frame_pointer_needed
>> + && reg == HARD_FRAME_POINTER_REGNUM)
>> + || (HARD_FRAME_POINTER_REGNUM && frame_pointer_needed
>> + && reg == FRAME_POINTER_REGNUM))
>> continue;
>>
>>COPY_HARD_REG_SET (this_unavailable, unavailable);
>>
>> Looks like it never got reviewed and was committed as preapproved.
>>
>> The #ifdef we had before looks odd too. Maybe this should just read
>>
>>  if (fixed_regs[reg] || global_regs[reg]
>>  || (frame_pointer_needed && reg == HARD_FRAME_POINTER_REGNUM))
>>

I think this breaks the symmetry with the statement above.

how about this (similar to what Jeff suggested):

Index: regrename.c
===
--- regrename.c (revision 240268)
+++ regrename.c (working copy)
@@ -464,8 +464,7 @@
if (frame_pointer_needed)
  {
add_to_hard_reg_set (, Pmode, FRAME_POINTER_REGNUM);
-  if (!HARD_FRAME_POINTER_IS_FRAME_POINTER)
-   add_to_hard_reg_set (, Pmode, 
HARD_FRAME_POINTER_REGNUM);
+  add_to_hard_reg_set (, Pmode, HARD_FRAME_POINTER_REGNUM);
  }

FOR_EACH_VEC_ELT (id_to_chain, i, this_head)
@@ -479,10 +478,9 @@
 continue;

if (fixed_regs[reg] || global_regs[reg]
- || (!HARD_FRAME_POINTER_IS_FRAME_POINTER && frame_pointer_needed
- && reg == HARD_FRAME_POINTER_REGNUM)
- || (HARD_FRAME_POINTER_REGNUM && frame_pointer_needed
- && reg == FRAME_POINTER_REGNUM))
+ || (frame_pointer_needed
+ && (reg == HARD_FRAME_POINTER_REGNUM
+ || reg == FRAME_POINTER_REGNUMi)))
 continue;

COPY_HARD_REG_SET (this_unavailable, unavailable);


FRAME_POINTER_REGNUM is always special, and it
may be identical with HFP.  But it is not necessary
to use HARD_FRAME_POINTER_IS_FRAME_POINTER at all.

Bernd.

>
> I'll try bootstrapping and testing this change on arm-none-linux-gnueabihf.
>
> Thanks,
> Kyrill
>
>>
>> Bernd
>

Re: Implement -Wimplicit-fallthrough (version 9)

[Jason Merrill]

On 09/20/2016 11:33 AM, Marek Polacek wrote:

@@ -5135,6 +5135,30 @@ cp_parser_primary_expression (cp_parser *parser,
case RID_AT_SELECTOR:
  return cp_parser_objc_expression (parser);

+   case RID_ATTRIBUTE:


Attribute handling doesn't belong in this function; we don't want to 
allow attributes anywhere we see a primary-expression.  This should be 
either in cp_parser_expression_statement or cp_parser_statement.


Jason

[C++ PATCH] Fix constexpr switch handling (PR c++/77467, take 2)

[Jakub Jelinek]

On Mon, Sep 19, 2016 at 02:34:17PM -0400, Jason Merrill wrote:
> > For STATEMENT_LIST that is called by cxx_eval_constant_expression,
> > for BIND_EXPR if we are lucky enough that BIND_EXPR_BODY is a STATEMENT_LIST
> > too (otherwise I assume even my patch doesn't fix it, it would need to
> > verify that).  If body is some other statement, then it really should be
> > skipped, but it isn't, because cxx_eval_constant_expression ignores it.
> 
> > I wonder if we e.g. cxx_eval_constant_expression couldn't early in the
> > function for if (*jump_target) return immediately unless code is something
> > like STATEMENT_LIST or BIND_EXPR with BIND_EXPR_BODY being STATEMENT_LIST,
> > or perhaps in the future other construct containing other stmts.
> 
> We might assert !jump_target before the call to
> cxx_eval_store_expression, to make sure we don't accidentally evaluate
> one when we're trying to jump.

I've done that.

> > I've beeing thinking about TRY block, but at least on the testcases I've
> > tried it has been rejected in constexpr functions, I think one can't branch
> > into statement expressions, so that should be fine, OpenMP/OpenACC
> > constructs are hopefully also rejected in constexpr, what else?
> 
> LOOP_EXPR, COND_EXPR?

Looking at those and adding further testcases revealed lots of other issues,
which the following patch attempts to deal with.

One is that handling the CASE_LABEL_EXPR or LABEL_EXPR only inside of
cxx_eval_statement_list doesn't really work well, because as the testcase
shows, for labeled null statements inside say if then block or else block
there is no STATEMENT_LIST wrapping it up.

Another is that the default: label can be arbitrarily nested (in inner
STATEMENT_LIST, or LOOP_EXPR body, or COND_EXPR then or else block, so
trying to handle it in cxx_eval_statement_list also works only in the
simplest switch case, we really need to process (with initial skipping mode)
the whole SWITCH_EXPR body and if we get through everything and (as
optimization) determine there has been a default: label, restart processing
it again with a flag to satisfy switches (jump_target) with the default:
label of the current switch.

Another thing is that the skipping of statements in cxx_eval_statement_list
didn't really work correctly for COND_EXPR/LOOP_EXPR, so the patch pretty
much removes all the skip/label/etc. handling from cxx_eval_statement_list.

And, lastly the handling of LOOP_EXPR and COND_EXPR when initially skipping
had to be changed.  For COND_EXPR, we don't want to evaluate the condition
in that case (one can't goto into the condition expression), and need to
process then block with skipping and, if it isn't skipped at its end, not
process else block, otherwise process it.  And for LOOP_EXPR, if still
skipping we'd need not to iterate again on the body, as the whole loop has
been bypassed in that case.

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

2016-09-20  Jakub Jelinek  

PR c++/77467
* constexpr.c (enum constexpr_switch_state): New.
(struct constexpr_ctx): Add css_state field.
(label_matches): Add CTX and STMT arguments, remove I and
DEFAULT_LABEL.  For CASE_LABEL_EXPR assert ctx->css_state != NULL,
handle default labels according to css_state.
(cxx_eval_statement_list): Remove statement skipping, label_matches
and default_label handling code.
(cxx_eval_loop_expr): Exit after first iteration even if
switches (jump_target).
(cxx_eval_switch_expr): Set up css_state field in ctx, if default
label has been seen in the body, but no cases matched, evaluate
the body second time.
(cxx_eval_constant_expression): Handle stmt skipping and label_matches
here.  Handle PREDICT_EXPR.  For MODIFY_EXPR or INIT_EXPR, assert
statement is not skipped.  For COND_EXPR during skipping, don't
evaluate condition, just the then block and if still skipping at the
end also the else block.
(cxx_eval_outermost_constant_expr): Adjust constexpr_ctx initializer.
(is_sub_constant_expr): Likewise.

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

--- gcc/cp/constexpr.c.jj   2016-09-19 16:35:32.835574406 +0200
+++ gcc/cp/constexpr.c  2016-09-20 12:51:48.270305879 +0200
@@ -900,6 +900,18 @@ struct constexpr_call_hasher : ggc_ptr_h
   static bool equal (constexpr_call *, constexpr_call *);
 };
 
+enum constexpr_switch_state {
+  /* Used when processing a switch for the first time by cxx_eval_switch_expr
+ and default: label for that switch has not been seen yet.  */
+  css_default_not_seen,
+  /* Used when processing a switch for the first time by cxx_eval_switch_expr
+ and default: label for that switch has been seen already.  */
+  css_default_seen,
+  /* Used when processing a switch for the second time by
+ cxx_eval_switch_expr, where default: label should match.  */
+  css_default_processing
+};

Re: [PATCHv3, resent] Add a warning for suspicious use of conditional expressions in boolean context

[Kyrill Tkachov]

On 20/09/16 16:30, Bernd Schmidt wrote:

On 09/20/2016 05:18 PM, Jeff Law wrote:

I assume HARD_FRAME_POINTER_REGNUM is never zero.

It could be zero.  It's just a hard register number.  No target has the
property that its hard frame pointer register is 0 though :-)


git blame to the rescue. The current state comes from one of tbsaunde's cleanup 
patches:

> diff --git a/gcc/regrename.c b/gcc/regrename.c
index 174d3b5..e5248a5 100644
--- a/gcc/regrename.c
+++ b/gcc/regrename.c
@@ -442,12 +442,10 @@ rename_chains (void)
continue;

   if (fixed_regs[reg] || global_regs[reg]
-#if !HARD_FRAME_POINTER_IS_FRAME_POINTER
- || (frame_pointer_needed && reg == HARD_FRAME_POINTER_REGNUM)
-#else
- || (frame_pointer_needed && reg == FRAME_POINTER_REGNUM)
-#endif
- )
+ || (!HARD_FRAME_POINTER_IS_FRAME_POINTER && frame_pointer_needed
+ && reg == HARD_FRAME_POINTER_REGNUM)
+ || (HARD_FRAME_POINTER_REGNUM && frame_pointer_needed
+ && reg == FRAME_POINTER_REGNUM))
continue;

   COPY_HARD_REG_SET (this_unavailable, unavailable);

Looks like it never got reviewed and was committed as preapproved.

The #ifdef we had before looks odd too. Maybe this should just read

 if (fixed_regs[reg] || global_regs[reg]
 || (frame_pointer_needed && reg == HARD_FRAME_POINTER_REGNUM))



I'll try bootstrapping and testing this change on arm-none-linux-gnueabihf.

Thanks,
Kyrill



Bernd

Re: [PATCH] [AArch64] Add missing attributes to arm_neon.h

[James Greenhalgh]

On Tue, Sep 20, 2016 at 04:37:42PM +0100, Tamar Christina wrote:
> Hi All,
> 
> This patch fixes the regression failures introduced by r240256.
> It adds some missing attributes to the functions:
> 
>  * vst2_s64
>  * vst2_u64
>  * vst2_f64
>  * vst2_s8
>  * vst3_s64
>  * vst3_u64
>  * vst3_f64
>  * vst3_s8
>  * vst4_s64
>  * vst4_u64
>  * vst4_f64
>  * vst4_s8
> 
> These were always missing but the change of marking the
> functions extern instead of static exposed them.
> 
> The patch has been pre-approved to be checked in but I don't
> have commit rights.
> https://gcc.gnu.org/ml/gcc-patches/2016-09/msg01293.html
> 
> Can someone check it in for me?

Done. r240271.

> gcc/
> 2016-09-20  Tamar Christina  
> 
>   * config/aarch64/arm_neon.h
>   (vst2_s64, vst2_u64, vst2_f64, vst2_s8): Add missing attributes.
>   (vst3_s64, vst3_u64, vst3_f64, vst3_s8): Likewise.
>   (vst4_s64, vst4_u64, vst4_f64, vst4_s8): Likewise.
> 

Thanks for the prompt fix,
James

Re: [PATCH] Fix PR tree-optimization/77550

[Bernd Edlinger]

On 09/20/16 09:41, Richard Biener wrote:
> On Mon, 19 Sep 2016, Bernd Edlinger wrote:
>
>> On 09/19/16 11:25, Richard Biener wrote:
>>> On Sun, 18 Sep 2016, Bernd Edlinger wrote:
>>>
 Hi,

 this PR shows that in vectorizable_store and vectorizable_load
 as well, the vector access always uses the first dr as the alias
 type for the whole access.  But that is not right, if they are
 different types, like in this example.

 So I tried to replace all reference_alias_ptr_type (DR_REF (first_dr))
 by an alias type that is correct for all references in the whole
 access group.  With this patch we fall back to ptr_type_node, which
 can alias anything, if the group consists of different alias sets.


 Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
 Is it OK for trunk and gcc-6-branch?
>>>
>>> +/* Function get_group_alias_ptr_type.
>>> +
>>> +   Return the alias type for the group starting at FIRST_STMT
>>> +   containing GROUP_SIZE elements.  */
>>> +
>>> +static tree
>>> +get_group_alias_ptr_type (gimple *first_stmt, int group_size)
>>> +{
>>> +  struct data_reference *first_dr, *next_dr;
>>> +  gimple *next_stmt;
>>> +  int i;
>>> +
>>> +  first_dr = STMT_VINFO_DATA_REF (vinfo_for_stmt (first_stmt));
>>> +  next_stmt = GROUP_NEXT_ELEMENT (vinfo_for_stmt (first_stmt));
>>> +  for (i = 1; i < group_size && next_stmt; i++)
>>> +{
>>>
>>>
>>> there is no need to pass in group_size, it's enough to walk
>>> GROUP_NEXT_ELEMENT until it becomes NULL.
>>>
>>> Ok with removing the redundant arg.
>>>
>>> Thanks,
>>> Richard.
>>>
>>
>> Hmmm, I'm afraid this needs one more iteration.
>>
>>
>> I tried first to check if there are no stmts after the group_size
>> and noticed there are cases when group_size is 0,
>> for instance in gcc.dg/torture/pr36244.c.
>>
>> I think there is a bug in vectorizable_load, here:
>>
>>if (grouped_load)
>>  {
>>first_stmt = GROUP_FIRST_ELEMENT (stmt_info);
>> /* For SLP vectorization we directly vectorize a subchain
>>without permutation.  */
>> if (slp && ! SLP_TREE_LOAD_PERMUTATION (slp_node).exists ())
>>   first_stmt = ;
>>
>> group_size = GROUP_SIZE (vinfo_for_stmt (first_stmt));
>>
>> = 0, and even worse:
>>
>> group_gap_adj = vf * group_size - nunits * vec_num;
>>
>> = -4 !
>>
>> apparently GROUP_SIZE is only valid on the GROUP_FIRST_ELEMENT,
>
> Yes.  I'm not sure group_size or group_gap_adj are used in the
> slp && ! SLP_TREE_LOAD_PERMUTATION (slp_node).exists () case but moving
> the computation up before we re-set first_stmt is probably a good idea.
>
>> while it may be 0 on SLP_TREE_SCALAR_STMTS (slp_node)[0]
>>
>> moving the GROUP_SIZE up before first_stmt is overwritten
>> results in no different code
>
> See above - it's eventually unused.  The load/store vectorization code
> is quite twisted ;)
>

Agreed.

Here is the new version of the patch:

Moved the goups_size up, and everything works fine.

Removed the parameter from get_group_alias_ptr_type.

I think in the case, where first_stmt is not set to
GROUP_FIRST_ELEMENT (stmt_info) but directly to stmt,
it is likely somewhere in a list, so it is not necessary
to walk the GROUP_NEXT_ELEMENT, so I would like to call
reference_alias_ptr_type directly in that case.


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


Thanks
Bernd.
gcc:
2016-09-18  Bernd Edlinger  

PR tree-optimization/77550
* tree-vect-stmts.c (create_array_ref): Change parameters.
(get_group_alias_ptr_type): New function.
(vectorizable_store, vectorizable_load): Use get_group_alias_ptr_type.

testsuite:
2016-09-18  Bernd Edlinger  

PR tree-optimization/77550
* g++.dg/pr77550.C: New test.
Index: gcc/tree-vect-stmts.c
===
--- gcc/tree-vect-stmts.c	(revision 240251)
+++ gcc/tree-vect-stmts.c	(working copy)
@@ -170,11 +170,10 @@ write_vector_array (gimple *stmt, gimple_stmt_iter
(and its group).  */
 
 static tree
-create_array_ref (tree type, tree ptr, struct data_reference *first_dr)
+create_array_ref (tree type, tree ptr, tree alias_ptr_type)
 {
-  tree mem_ref, alias_ptr_type;
+  tree mem_ref;
 
-  alias_ptr_type = reference_alias_ptr_type (DR_REF (first_dr));
   mem_ref = build2 (MEM_REF, type, ptr, build_int_cst (alias_ptr_type, 0));
   /* Arrays have the same alignment as their type.  */
   set_ptr_info_alignment (get_ptr_info (ptr), TYPE_ALIGN_UNIT (type), 0);
@@ -5432,6 +5431,35 @@ ensure_base_align (stmt_vec_info stmt_info, struct
 }
 
 
+/* Function get_group_alias_ptr_type.
+
+   Return the alias type for the group starting at FIRST_STMT.  */
+
+static tree
+get_group_alias_ptr_type (gimple *first_stmt)
+{
+  struct data_reference *first_dr, *next_dr;
+  gimple *next_stmt;
+
+  first_dr = 

[PATCH] [AArch64] Add missing attributes to arm_neon.h

[Tamar Christina]

Hi All,

This patch fixes the regression failures introduced by r240256.
It adds some missing attributes to the functions:

 * vst2_s64
 * vst2_u64
 * vst2_f64
 * vst2_s8
 * vst3_s64
 * vst3_u64
 * vst3_f64
 * vst3_s8
 * vst4_s64
 * vst4_u64
 * vst4_f64
 * vst4_s8

These were always missing but the change of marking the
functions extern instead of static exposed them.

The patch has been pre-approved to be checked in but I don't
have commit rights.
https://gcc.gnu.org/ml/gcc-patches/2016-09/msg01293.html

Can someone check it in for me?

Fixes failures:

gcc.target/aarch64/vect_int32x2x4_1.c
gcc.target/aarch64/vdup_lane_2.c
gcc.target/aarch64/vect_combine_zeroes_1.c

gcc/
2016-09-20  Tamar Christina  

* config/aarch64/arm_neon.h
(vst2_s64, vst2_u64, vst2_f64, vst2_s8): Add missing attributes.
(vst3_s64, vst3_u64, vst3_f64, vst3_s8): Likewise.
(vst4_s64, vst4_u64, vst4_f64, vst4_s8): Likewise.

Thanks,
Tamardiff --git a/gcc/config/aarch64/arm_neon.h b/gcc/config/aarch64/arm_neon.h
index b4db87b0bc60d22306da810381b62920646aac9b..c463e3b698a47b9b5c5a04e0fb7fff1f71817af1 100644
--- a/gcc/config/aarch64/arm_neon.h
+++ b/gcc/config/aarch64/arm_neon.h
@@ -26102,6 +26102,7 @@ vst1q_lane_u64 (uint64_t *__a, uint64x2_t __b, const int __lane)
 /* vstn */
 
 __extension__ extern __inline void
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
 vst2_s64 (int64_t * __a, int64x1x2_t val)
 {
   __builtin_aarch64_simd_oi __o;
@@ -26114,6 +26115,7 @@ vst2_s64 (int64_t * __a, int64x1x2_t val)
 }
 
 __extension__ extern __inline void
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
 vst2_u64 (uint64_t * __a, uint64x1x2_t val)
 {
   __builtin_aarch64_simd_oi __o;
@@ -26126,6 +26128,7 @@ vst2_u64 (uint64_t * __a, uint64x1x2_t val)
 }
 
 __extension__ extern __inline void
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
 vst2_f64 (float64_t * __a, float64x1x2_t val)
 {
   __builtin_aarch64_simd_oi __o;
@@ -26138,6 +26141,7 @@ vst2_f64 (float64_t * __a, float64x1x2_t val)
 }
 
 __extension__ extern __inline void
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
 vst2_s8 (int8_t * __a, int8x8x2_t val)
 {
   __builtin_aarch64_simd_oi __o;
@@ -26397,6 +26401,7 @@ vst2q_f64 (float64_t * __a, float64x2x2_t val)
 }
 
 __extension__ extern __inline void
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
 vst3_s64 (int64_t * __a, int64x1x3_t val)
 {
   __builtin_aarch64_simd_ci __o;
@@ -26411,6 +26416,7 @@ vst3_s64 (int64_t * __a, int64x1x3_t val)
 }
 
 __extension__ extern __inline void
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
 vst3_u64 (uint64_t * __a, uint64x1x3_t val)
 {
   __builtin_aarch64_simd_ci __o;
@@ -26425,6 +26431,7 @@ vst3_u64 (uint64_t * __a, uint64x1x3_t val)
 }
 
 __extension__ extern __inline void
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
 vst3_f64 (float64_t * __a, float64x1x3_t val)
 {
   __builtin_aarch64_simd_ci __o;
@@ -26439,6 +26446,7 @@ vst3_f64 (float64_t * __a, float64x1x3_t val)
 }
 
 __extension__ extern __inline void
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
 vst3_s8 (int8_t * __a, int8x8x3_t val)
 {
   __builtin_aarch64_simd_ci __o;
@@ -26731,6 +26739,7 @@ vst3q_f64 (float64_t * __a, float64x2x3_t val)
 }
 
 __extension__ extern __inline void
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
 vst4_s64 (int64_t * __a, int64x1x4_t val)
 {
   __builtin_aarch64_simd_xi __o;
@@ -26747,6 +26756,7 @@ vst4_s64 (int64_t * __a, int64x1x4_t val)
 }
 
 __extension__ extern __inline void
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
 vst4_u64 (uint64_t * __a, uint64x1x4_t val)
 {
   __builtin_aarch64_simd_xi __o;
@@ -26763,6 +26773,7 @@ vst4_u64 (uint64_t * __a, uint64x1x4_t val)
 }
 
 __extension__ extern __inline void
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
 vst4_f64 (float64_t * __a, float64x1x4_t val)
 {
   __builtin_aarch64_simd_xi __o;
@@ -26779,6 +26790,7 @@ vst4_f64 (float64_t * __a, float64x1x4_t val)
 }
 
 __extension__ extern __inline void
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
 vst4_s8 (int8_t * __a, int8x8x4_t val)
 {
   __builtin_aarch64_simd_xi __o;

Implement -Wimplicit-fallthrough (version 9)

[Marek Polacek]

Time for another latest & greatest version of -Wimplicit-fallthrough.
Hope this time it's near its end.

Changes from last time:

* I addressed Jakub's comment, for details see
  
* in the C FE:
  case 0:
__attribute)) ((fallthrough));
  case 1:
...
  now works
* we've discussed cases like
  [[fallthrough]]; lab1: case 1:;
  and
  [[fallthrough]]; lab2:; case 2:;

  This was discussed on the C++ committee reflector and it was concluded that
  the former should work and the latter should not.  So I implemented that.
  But there's no way to tell the difference between "lab1: case 1:;" and
  "lab2:; case 2:;" so the null statements are ignored.  Another problem is
  what to do with "[[fallthrough]]; lab1: (void) 0; case 2:".  Currently it's
  not rejected because we optimize "(void) 0;" out in gimplify_expr.  And I
  couldn't figure out how to circumvent this (in gimplify_expr you can't see
  what the next statement is).  Though this seems more like a pedantic issue
  and probably not a real-world issue.  (E.g.
  "[[fallthrough]]; lab1: if (0) { ... } case 2:"
  should be rejected.)
 
Jason, would you mind looking at the C++ FE parts again?

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

2016-09-20  Marek Polacek  
Jakub Jelinek  

PR c/7652
gcc/
* common.opt (Wimplicit-fallthrough): New option.
* doc/extend.texi: Document statement attributes and the fallthrough
attribute.
* doc/invoke.texi: Document -Wimplicit-fallthrough.
* gimple.h (gimple_call_internal_p): New function.
* gimplify.c (struct gimplify_ctx): Add in_switch_expr.
(struct label_entry): New struct.
(find_label_entry): New function.
(case_label_p): New function.
(collect_fallthrough_labels): New function.
(last_stmt_in_scope): New function.
(should_warn_for_implicit_fallthrough): New function.
(warn_implicit_fallthrough_r): New function.
(maybe_warn_implicit_fallthrough): New function.
(expand_FALLTHROUGH_r): New function.
(expand_FALLTHROUGH): New function.
(gimplify_switch_expr): Call maybe_warn_implicit_fallthrough and
expand_FALLTHROUGH for the innermost GIMPLE_SWITCH.
(gimplify_label_expr): New function.
(gimplify_case_label_expr): Set location.
(gimplify_expr): Call gimplify_label_expr.
* internal-fn.c (expand_FALLTHROUGH): New function.
* internal-fn.def (FALLTHROUGH): New internal function.
* langhooks.c (lang_GNU_OBJC): New function.
* langhooks.h (lang_GNU_OBJC): Declare.
* system.h (gcc_fallthrough): Define.
* tree-core.h: Add FALLTHROUGH_LABEL_P comment.
* tree.h (FALLTHROUGH_LABEL_P): Define.
gcc/c-family/
* c-common.c (c_common_attribute_table): Add fallthrough attribute.
(handle_fallthrough_attribute): New function.
(maybe_attribute_fallthrough_p): New function.
* c-common.h (maybe_attribute_fallthrough_p): Declare.
gcc/c/
* c-parser.c (struct c_token): Add flags field.
(c_lex_one_token): Pass it to c_lex_with_flags.
(c_parser_declaration_or_fndef): Turn __attribute__((fallthrough));
into IFN_FALLTHROUGH.
(c_parser_label): Set FALLTHROUGH_LABEL_P on labels.  Handle
attribute fallthrough after a case label or default label.
(c_parser_statement_after_labels): Handle RID_ATTRIBUTE.
gcc/cp/
* constexpr.c (cxx_eval_internal_function): Handle IFN_FALLTHROUGH.
(potential_constant_expression_1): Likewise.
* constraint.cc (function_concept_check_p): Check fn for null.
* parser.c (cp_parser_primary_expression): Handle RID_ATTRIBUTE.
(cp_parser_statement): Handle fallthrough attribute.
(cp_parser_label_for_labeled_statement): Set FALLTHROUGH_LABEL_P on
labels.
(cp_parser_std_attribute): Handle fallthrough attribute.
(cp_parser_check_std_attribute): Detect duplicated fallthrough
attribute.
* pt.c (tsubst_copy_and_build): Handle internal functions.
(instantiation_dependent_scope_ref_p): Return if the expression is
null.
gcc/testsuite/
* c-c++-common/Wimplicit-fallthrough-1.c: New test.
* c-c++-common/Wimplicit-fallthrough-10.c: New test.
* c-c++-common/Wimplicit-fallthrough-11.c: New test.
* c-c++-common/Wimplicit-fallthrough-12.c: New test.
* c-c++-common/Wimplicit-fallthrough-13.c: New test.
* c-c++-common/Wimplicit-fallthrough-14.c: New test.
* c-c++-common/Wimplicit-fallthrough-15.c: New test.
* c-c++-common/Wimplicit-fallthrough-16.c: New test.
* c-c++-common/Wimplicit-fallthrough-17.c: New test.
* c-c++-common/Wimplicit-fallthrough-18.c: New test.
* c-c++-common/Wimplicit-fallthrough-19.c: New 

Re: Register numbers in RTL dumps (was Re: [PATCH 0/9] RFC: selftests based on RTL dumps)

[Bernd Schmidt]

On 09/20/2016 05:20 PM, Jeff Law wrote:

On 09/20/2016 08:34 AM, Bernd Schmidt wrote:



  (reg/f:DI v1 virtual-stack-vars)


this.

Doesn't the same apply to the number of virtual stack regs?  Those are
in the same array as pseudos.  So ISTM we prefix in the dump, but do
adjustment of the number in the reader?


I meant the virtual-stack-vars name. But maybe we're already doing that, 
I can't remember.



Bernd

Re: [PATCHv3, resent] Add a warning for suspicious use of conditional expressions in boolean context

[Bernd Schmidt]

On 09/20/2016 05:18 PM, Jeff Law wrote:

I assume HARD_FRAME_POINTER_REGNUM is never zero.

It could be zero.  It's just a hard register number.  No target has the
property that its hard frame pointer register is 0 though :-)


git blame to the rescue. The current state comes from one of tbsaunde's 
cleanup patches:


> diff --git a/gcc/regrename.c b/gcc/regrename.c
index 174d3b5..e5248a5 100644
--- a/gcc/regrename.c
+++ b/gcc/regrename.c
@@ -442,12 +442,10 @@ rename_chains (void)
continue;

   if (fixed_regs[reg] || global_regs[reg]
-#if !HARD_FRAME_POINTER_IS_FRAME_POINTER
- || (frame_pointer_needed && reg == HARD_FRAME_POINTER_REGNUM)
-#else
- || (frame_pointer_needed && reg == FRAME_POINTER_REGNUM)
-#endif
- )
+ || (!HARD_FRAME_POINTER_IS_FRAME_POINTER && frame_pointer_needed
+ && reg == HARD_FRAME_POINTER_REGNUM)
+ || (HARD_FRAME_POINTER_REGNUM && frame_pointer_needed
+ && reg == FRAME_POINTER_REGNUM))
continue;

   COPY_HARD_REG_SET (this_unavailable, unavailable);

Looks like it never got reviewed and was committed as preapproved.

The #ifdef we had before looks odd too. Maybe this should just read

 if (fixed_regs[reg] || global_regs[reg]
 || (frame_pointer_needed && reg == HARD_FRAME_POINTER_REGNUM))


Bernd

Re: [PATCH] Tree-level fix for PR 69526

[Jeff Law]

On 09/20/2016 06:31 AM, Robin Dapp wrote:

Hi,


I meant to do sth like

Index: tree-ssa-propagate.c
===
--- tree-ssa-propagate.c(revision 240133)
+++ tree-ssa-propagate.c(working copy)
@@ -1105,10 +1105,10 @@ substitute_and_fold_dom_walker::before_d
   /* Replace real uses in the statement.  */
   did_replace |= replace_uses_in (stmt, get_value_fn);

-  /* If we made a replacement, fold the statement.  */
-  if (did_replace)
+  /* Fold the statement.  */
+  if (fold_stmt (, follow_single_use_edges))
{
- fold_stmt (, follow_single_use_edges);
+ did_replace = true;
  stmt = gsi_stmt (i);
}

this would need compile-time cost evaluation (and avoid the tree-vrp.c
folding part
of your patch).


Using this causes the simplifications to be performed in ccp1 instead of
fwprop1. I noticed two tests failing that rely on propagation being
performed in fwprop. Should these be adapted or rather the patch be changed?
ISTM this is an indication that something changed the IL without folding 
prior to ccp & forwprop.  That's not in and of itself bad, but may be 
worth investigating to see if whatever prior pass that made this change 
ought to be adjusted.


That investigation would likely guide you with what to do with the 
testcase.  If you find that the earlier pass should have folded, then 
you fix it and change the testcase to verify the earlier pass folded 
properly.  Else you change the testcase to verify ccp folds the statement.


I'm going to let Richi own the review on this.  Just thought I'd chime 
in on that one topic.

jeff

[PATCH GCC][v2]Simplify alias check code generation in vectorizer

[Bin Cheng]

Hi,
I originally posted a patch improving code generation for alias check in 
vectorizer at https://gcc.gnu.org/ml/gcc-patches/2016-06/msg00929.html.  Here 
it's the 2nd version (better) patch.  It detects data reference pair in which 
the two references are only different to each other wrto index.  In this case 
the patch generates intersect range checks based on index of address of 
reference, rather than the address of reference.  Take example from patch's 
comment, given below two data references:

   DR_A   DR_B
  data-ref arr[i] arr[j]
  base_object  arrarr
  index{i_0, +, 1}_loop   {j_0, +, 1}_loop

The addresses and their index can be depicted as:

|<- ADDR_A->|  |<- ADDR_B->|
 --->
|   |   |   |   |  |   |   |   |   |
 --->
i_0 ... i_0+4  j_0 ... j_0+4

We can create expression based on index rather than address:  (i_0 + 4 < j_0 || 
j_0 + 4 < i_0).

Also take example from spec2k/200.sixtrack/DACOP, gcc needs to generate alias 
check for three pairs:
//pair 1
dr_a:
(Data Ref:
  bb: 8
  stmt: _92 = da.cc[_27];
  ref: da.cc[_27];
)
dr_b:
(Data Ref:
  bb: 8
  stmt: da.cc[_93] = _92;
  ref: da.cc[_93];
)
//pair 2
dr_a:
(Data Ref:
  bb: 8
  stmt: pretmp_29 = da.i2[_27];
  ref: da.i2[_27];
)
dr_b:
(Data Ref:
  bb: 8
  stmt: da.i2[_93] = pretmp_29;
  ref: da.i2[_93];
)
//pair 3
dr_a:
(Data Ref:
  bb: 8
  stmt: pretmp_28 = da.i1[_27];
  ref: da.i1[_27];
)
dr_b:
(Data Ref:
  bb: 8
  stmt: da.i1[_93] = pretmp_28;
  ref: da.i1[_93];
)

With this patch, code can be improved to:

  :
  _37 = (unsigned int) _6;
  _106 = (unsigned int) _52;
  _107 = _37 - _106;
  _108 = _107 > 7;
  _109 = (integer(kind=8)) _2;
  _110 = _109 + 3;
  _111 = (integer(kind=8)) _52;
  _112 = _111 + -1;
  _113 = _110 < _112;
  _114 = (integer(kind=8)) _52;
  _115 = _114 + 2;
  _116 = (integer(kind=8)) _2;
  _117 = _115 < _116;
  _118 = _113 | _117;
  _119 = _108 & _118;
  _120 = (integer(kind=8)) _2;
  _121 = _120 + 3;
  _122 = (integer(kind=8)) _52;
  _123 = _122 + -1;
  _124 = _121 < _123;
  _125 = (integer(kind=8)) _52;
  _126 = _125 + 2;
  _127 = (integer(kind=8)) _2;
  _128 = _126 < _127;
  _129 = _124 | _128;
  _130 = _119 & _129;
  _131 = (integer(kind=8)) _2;
  _132 = _131 + 3;
  _133 = (integer(kind=8)) _52;
  _134 = _133 + -1;
  _135 = _132 < _134;
  _136 = (integer(kind=8)) _52;
  _137 = _136 + 2;
  _138 = (integer(kind=8)) _2;
  _139 = _137 < _138;
  _140 = _135 | _139;
  _141 = _130 & _140;
  if (_141 != 0)
goto ;
  else
goto ;

Note this patch doesn't do local CSE, and common sub-expressions will be 
optimized by later passes.  I think this is OK because the redundancy is far 
away from loops thus won't have bad effect (there is an opposite example/PR).
Bootstrap and test on x86_64 and AArch64, is it OK?

Thanks,
bin

2016-09-19  Bin Cheng  

* tree-vect-loop-manip.c (create_intersect_range_checks_index): New.
(create_intersect_range_checks): New.
(vect_create_cond_for_alias_checks): Call above function.diff --git a/gcc/tree-vect-loop-manip.c b/gcc/tree-vect-loop-manip.c
index 01d6bb1..c7d130e 100644
--- a/gcc/tree-vect-loop-manip.c
+++ b/gcc/tree-vect-loop-manip.c
@@ -2263,6 +2263,179 @@ vect_create_cond_for_align_checks (loop_vec_info 
loop_vinfo,
 *cond_expr = part_cond_expr;
 }
 
+/* Given two data references and segment lengths described by DR_A and DR_B,
+   create expression checking if the two addresses ranges intersect with
+   each other based on index of the two addresses.  This can only be done
+   if DR_A and DR_B referring to the same (array) object and the index is
+   the only difference.  For example:
+
+   DR_A   DR_B
+  data-ref arr[i] arr[j]
+  base_object  arrarr
+  index{i_0, +, 1}_loop   {j_0, +, 1}_loop
+
+   The addresses and their index are like:
+
+|<- ADDR_A->|  |<- ADDR_B->|
+ --->
+|   |   |   |   |  |   |   |   |   |
+ --->
+i_0 ... i_0+4  j_0 ... j_0+4
+
+   We can create expression based on index rather than address:
+ (i_0 + 4 < j_0 || j_0 + 4 < i_0).  */
+
+static bool
+create_intersect_range_checks_index (loop_vec_info loop_vinfo, tree *cond_expr,
+const dr_with_seg_len& dr_a,
+const dr_with_seg_len& dr_b)
+{
+  if (integer_zerop (DR_STEP (dr_a.dr))
+  || integer_zerop (DR_STEP (dr_b.dr))
+  || DR_NUM_DIMENSIONS 

Re: Register numbers in RTL dumps (was Re: [PATCH 0/9] RFC: selftests based on RTL dumps)

[Jeff Law]

On 09/20/2016 08:34 AM, Bernd Schmidt wrote:

On 09/20/2016 04:32 PM, David Malcolm wrote:


To summarize so far: you want every pseudo to have its regno dumped
with a 'p' prefix, and renumber them on dump (and then on load).
OK.


Renumbering is not helpful because it interferes with the view you have
in the debugger. So, IMO just a prefix, and maybe
Yea, I guess it does since we want the numbers in the dump to be the 
same that we used to access the arrays.  So prefixing in the dump with 
adjustment of the number in the reader.





  (reg/f:DI v1 virtual-stack-vars)


this.
Doesn't the same apply to the number of virtual stack regs?  Those are 
in the same array as pseudos.  So ISTM we prefix in the dump, but do 
adjustment of the number in the reader?


jeff

Re: [PATCHv3, resent] Add a warning for suspicious use of conditional expressions in boolean context

[Jeff Law]

On 09/20/2016 08:38 AM, Bernd Edlinger wrote:

On 09/20/16 16:20, Kyrill Tkachov wrote:


On 20/09/16 15:11, Bernd Edlinger wrote:

On 09/20/16 13:29, Kyrill Tkachov wrote:

arm bootstrap is now failing:
$SRC/gcc/config/arm/arm.h:2229:40: error: ?: using integer constants in
boolean context [-Werror=int-in-bool-context]
  : (TARGET_VFP_DOUBLE ? (TARGET_FP16 ? 14 : 12) : 0)) \
 ~^~
$SRC/gcc/config/arm/arm-c.c:133:7: note: in expansion of macro
'TARGET_ARM_FP'
 if (TARGET_ARM_FP)


The full definition of TARGET_ARM_FP is:
#define TARGET_ARM_FP\
(!TARGET_SOFT_FLOAT ? (TARGET_VFP_SINGLE ? 4\
  : (TARGET_VFP_DOUBLE ? (TARGET_FP16 ? 14 : 12) : 0)) \
: 0)

We want it set to 0 when there's no FP but when FP is available we set
it to a bitmask
to suggest the level that is available. That seems like a legitimate use
to me.


Ok, I see, sorry for that.

I think I will have to suppress the warning if the conditional is in
a macro somehow.

Can you work around that for a few days?


I changed to:
  if (TARGET_ARM_FP != 0)
in my tree to allow the build to proceed but encountered another
bootstrap failure:
In file included from ./tm.h:38:0,
  from $SRC/gcc/backend.h:28,
  from $SRC/gcc/regrename.c:23:
$SRC/gcc/regrename.c: In function 'void rename_chains()':
$SRC/gcc/config/arm/arm.h:915:4: error: ?: using integer constants in
boolean context [-Werror=int-in-bool-context]
(TARGET_ARM \
~
 ? ARM_HARD_FRAME_POINTER_REGNUM  \
 ^~
 : THUMB_HARD_FRAME_POINTER_REGNUM)
 ~~
$SRC/gcc/regrename.c:484:8: note: in expansion of macro
'HARD_FRAME_POINTER_REGNUM'
 || (HARD_FRAME_POINTER_REGNUM && frame_pointer_needed
 ^
That looks like a legitimate bug in regrename.c ?
The full condition is:
if (fixed_regs[reg] || global_regs[reg]
|| (!HARD_FRAME_POINTER_IS_FRAME_POINTER &&
frame_pointer_needed
&& reg == HARD_FRAME_POINTER_REGNUM)
|| (HARD_FRAME_POINTER_REGNUM && frame_pointer_needed
&& reg == FRAME_POINTER_REGNUM))

The condition in the second || looks bogus (what use testing if a
register is 'non-zero').

So this looks like a useful catch by the warning, though I'm not sure at
first glance how
to fix it.
Kyrill



I assume HARD_FRAME_POINTER_REGNUM is never zero.
It could be zero.  It's just a hard register number.  No target has the 
property that its hard frame pointer register is 0 though :-)




That looks like it should be

  || (frame_pointer_needed && reg == FRAME_POINTER_REGNUM)

I'm not even entirely sure why this is two conditions.

if (fixed_regs[regs] || global_regs[reg]
|| (frame_pointer_needed
&& (reg == HARD_FRAME_POINTER_REGNUM
|| reg == FRAME_POINTER_REGNUM

?


Bernd Schmidt ought to be able to guide us.

jeff

Re: [C++ PATCH] Fix parser ICE with [[...]] and [[unused,...]] (PR c++/77637)

[Jason Merrill]

OK.

On Mon, Sep 19, 2016 at 5:50 PM, Jakub Jelinek  wrote:
> Hi!
>
> As the testcase shows, while the grammar has
> attribute-list:
>   attribute[opt]
>   attribute-list , attribute[opt]
>   attribute ...
>   attribute-list , attribute ...
> we are actually parsing it as if the last 2 lines were
>   attribute[opt] ...
>   attribute-list , attribute[opt] ...
> and ICEing when there is no attribute.
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk/6.3?
>
> 2016-09-19  Jakub Jelinek  
>
> PR c++/77637
> * parser.c (cp_parser_std_attribute_list): Reject ... without
> preceding attribute.
>
> * g++.dg/cpp0x/gen-attrs-62.C: New test.
>
> --- gcc/cp/parser.c.jj  2016-09-16 22:19:39.0 +0200
> +++ gcc/cp/parser.c 2016-09-19 10:33:51.481904739 +0200
> @@ -24220,8 +24220,12 @@ cp_parser_std_attribute_list (cp_parser
>if (token->type == CPP_ELLIPSIS)
> {
>   cp_lexer_consume_token (parser->lexer);
> - TREE_VALUE (attribute)
> -   = make_pack_expansion (TREE_VALUE (attribute));
> + if (attribute == NULL_TREE)
> +   error_at (token->location,
> + "expected attribute before %<...%>");
> + else
> +   TREE_VALUE (attribute)
> + = make_pack_expansion (TREE_VALUE (attribute));
>   token = cp_lexer_peek_token (parser->lexer);
> }
>if (token->type != CPP_COMMA)
> --- gcc/testsuite/g++.dg/cpp0x/gen-attrs-62.C.jj2016-09-19 
> 10:37:59.414772726 +0200
> +++ gcc/testsuite/g++.dg/cpp0x/gen-attrs-62.C   2016-09-19 10:37:28.0 
> +0200
> @@ -0,0 +1,5 @@
> +// PR c++/77637
> +// { dg-do compile { target c++11 } }
> +
> +int [[...]] a; // { dg-error "expected attribute before '...'" }
> +int [[,,...]] b;   // { dg-error "expected attribute before '...'" }
>
> Jakub

Re: [C++ PATCH] Fix ICE on operator"" template (PR c++/77638)

[Jason Merrill]

OK.

On Mon, Sep 19, 2016 at 5:54 PM, Jakub Jelinek  wrote:
> Hi!
>
> As the testcase shows for C++14 and up, for 1 argument template we don't ICE
> even if the template argument is invalid, because it checks
>   if (TREE_TYPE (parm) != char_type_node
>   || !TEMPLATE_PARM_PARAMETER_PACK (DECL_INITIAL (parm)))
> and if parm is error_mark_node, then it doesn't have char_type_node type.
> But, for 2 argument template, if both the template arguments have
> error_mark_node type the type test succeeds and we ICE, because DEC_INITIAL
> on error_mark_node is not valid.
>
> The following testcase fixes the ICE and results in the same diagnostics
> that used to be emitted for C++11 already before the patch.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2016-09-19  Jakub Jelinek  
>
> PR c++/77638
> * parser.c (cp_parser_template_declaration_after_parameter): For 2
> argument operator"" template set ok to false for
> parm == error_mark_node.
>
> * g++.dg/cpp0x/udlit-tmpl-arg-neg2.C: New test.
>
> --- gcc/cp/parser.c.jj  2016-09-19 10:33:51.0 +0200
> +++ gcc/cp/parser.c 2016-09-19 11:29:25.724937375 +0200
> @@ -25722,7 +25722,8 @@ cp_parser_template_declaration_after_par
>   tree type = INNERMOST_TEMPLATE_PARMS (parm_type);
>   tree parm_list = TREE_VEC_ELT (parameter_list, 1);
>   tree parm = INNERMOST_TEMPLATE_PARMS (parm_list);
> - if (TREE_TYPE (parm) != TREE_TYPE (type)
> + if (parm == error_mark_node
> + || TREE_TYPE (parm) != TREE_TYPE (type)
>   || !TEMPLATE_PARM_PARAMETER_PACK (DECL_INITIAL (parm)))
> ok = false;
> }
> --- gcc/testsuite/g++.dg/cpp0x/udlit-tmpl-arg-neg2.C.jj 2016-09-19 
> 11:34:58.680674929 +0200
> +++ gcc/testsuite/g++.dg/cpp0x/udlit-tmpl-arg-neg2.C2016-09-19 
> 11:33:54.0 +0200
> @@ -0,0 +1,7 @@
> +// PR c++/77638
> +// { dg-do compile { target c++11 } }
> +
> +template    // { dg-error "'T' has not been declared" }
> +int operator"" _foo ();// { dg-error "has invalid parameter 
> list" }
> +template   // { dg-error "'T' has not been declared" }
> +int operator"" _bar ();// { dg-error "has invalid parameter 
> list" }
>
> Jakub

Re: [C++ PATCH] Fix ICE with class fields with invalid types (PR c++/77626)

[Jason Merrill]

OK.

On Mon, Sep 19, 2016 at 5:57 PM, Jakub Jelinek  wrote:
> Hi!
>
> layout_class_type for FIELD_DECLs with error_mark_node skips further
> processing, so such fields don't have DECL_FIELD_OFFSET and
> DECL_FIELD_BIT_OFFSET, thus byte_position ICEs on it.
>
> So, when we walk in constexpr handling all FIELD_DECLs, this patch fixes it
> by skipping over fields with error_mark_node types.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2016-09-19  Jakub Jelinek  
>
> PR c++/77626
> * constexpr.c (cxx_fold_indirect_ref): Don't call byte_position on
> FIELD_DECLs with error_mark_node type.  Remove useless break; after
> return.
>
> * g++.dg/other/pr77626.C: New test.
>
> --- gcc/cp/constexpr.c.jj   2016-09-16 22:22:00.0 +0200
> +++ gcc/cp/constexpr.c  2016-09-19 13:00:02.542599407 +0200
> @@ -2894,13 +2894,11 @@ cxx_fold_indirect_ref (location_t loc, t
>   tree field = TYPE_FIELDS (optype);
>   for (; field; field = DECL_CHAIN (field))
> if (TREE_CODE (field) == FIELD_DECL
> +   && TREE_TYPE (field) != error_mark_node
> && integer_zerop (byte_position (field))
> && (same_type_ignoring_top_level_qualifiers_p
> (TREE_TYPE (field), type)))
> - {
> -   return fold_build3 (COMPONENT_REF, type, op, field, 
> NULL_TREE);
> -   break;
> - }
> + return fold_build3 (COMPONENT_REF, type, op, field, NULL_TREE);
> }
>  }
>else if (TREE_CODE (sub) == POINTER_PLUS_EXPR
> @@ -2972,14 +2970,12 @@ cxx_fold_indirect_ref (location_t loc, t
>   tree field = TYPE_FIELDS (op00type);
>   for (; field; field = DECL_CHAIN (field))
> if (TREE_CODE (field) == FIELD_DECL
> +   && TREE_TYPE (field) != error_mark_node
> && tree_int_cst_equal (byte_position (field), op01)
> && (same_type_ignoring_top_level_qualifiers_p
> (TREE_TYPE (field), type)))
> - {
> -   return fold_build3 (COMPONENT_REF, type, op00,
> -   field, NULL_TREE);
> -   break;
> - }
> + return fold_build3 (COMPONENT_REF, type, op00,
> + field, NULL_TREE);
> }
> }
>  }
> --- gcc/testsuite/g++.dg/other/pr77626.C.jj 2016-09-19 13:22:57.895418630 
> +0200
> +++ gcc/testsuite/g++.dg/other/pr77626.C2016-09-19 13:22:21.0 
> +0200
> @@ -0,0 +1,13 @@
> +// PR c++/77626
> +// { dg-do compile }
> +
> +struct B;  // { dg-message "forward declaration of" }
> +struct A { struct B b; };  // { dg-error "has incomplete type" }
> +void bar (int);
> +
> +void
> +foo ()
> +{
> +  A a;
> +  bar ((int &) a);
> +}
>
> Jakub

Re: Report DejaGnu ERROR messages in dg-extract-results

[Jeff Law]

On 09/20/2016 03:26 AM, Christophe Lyon wrote:

Hello,

We recently faced a problem where a DejaGnu error went un-noticed
(https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01879.html).

To help identify these problems earlier, here is a patch for
dg-extract-results that makes such error obvious in the final summary:

=== gcc Summary ===

# of DejaGnu errors 1
# of expected passes104885
# of unexpected failures43
# of unexpected successes   2
# of expected failures  276
# of unsupported tests  2284


OK?

OK.
jeff

Re: [C++ PATCH] Fix C++ violation in g++.jason/init3.C (PR testsuite/63299)

[Jeff Law]

On 09/19/2016 04:04 PM, Jakub Jelinek wrote:

Hi!

As valgrind reports, this testcase allocates the string with new [], but
deallocates with delete str.

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

2016-09-19  Maxim Ostapenko  
Jakub Jelinek  

PR testsuite/63299
* g++.old-deja/g++.jason/init3.C (My_string::~My_string): Use delete[]
instead of delete.

OK.
jeff

Re: [BUILDROBOT] vax-netbsdelf / vax-linux: ‘ELIMINABLE_REGS’ was not declared in this scope

[Jeff Law]

On 09/19/2016 04:24 PM, Bernd Edlinger wrote:

On 09/19/16 23:51, Jeff Law wrote:

On 09/17/2016 05:29 PM, Bernd Edlinger wrote:

On 09/17/16 22:29, Jan-Benedict Glaw wrote:

On Fri, 2016-09-09 21:40:38 +, Bernd Edlinger
 wrote:

Hi,

I think it is time to remove support for
INITIAL_FRAME_POINTER_OFFSET, which is no longer
used by any target today.  This removes a bunch of conditional code,
and fixes a few bits
in the documentation.  I'd say that part of the documentation is
quite out of sync, but I just
have to stop somewhere.


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


The vax backend doesn't yet define ELIMINABLE_REGS.



Oh, yes.  I see.  What a hack.

Then we should define it.

But simply returning zero for the fp to sp offset is not ok,
and even if the offset is not used for register eliminations
it should still be correct for rtx_addr_can_trap_p
to know the safe stack frame offset ranges.

I would assume a small performance improvement, when
rtx_addr_can_trap_p has correct data available.

How about this patch, it should at least fix the bootstrap.
Is it OK for trunk?

OK.
jeff


Jeff,  I am sorry.

But I think I got the sign of the elimination offset wrong,
because I peeked at a stack-grows-upward target (hppa).
And the doc/tm.texi is not a big help either.

Funny, I almost called that out as something to double check :-)




With my limited testing this did not make any difference.

Actually I have not touched any vax since I was a student.

And that is already quite a while ago...

I hope you don't mind when I commit it this way:
+#define INITIAL_ELIMINATION_OFFSET(FROM, TO, OFFSET) \
+  ((OFFSET) = get_frame_size ())

That's fine.

jeff

Re: Report DejaGnu ERROR messages in compare_tests

[Jeff Law]

On 09/20/2016 03:26 AM, Christophe Lyon wrote:

Hello,

We recently faced a problem where a DejaGnu error went un-noticed
(https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01879.html).

To help identify these problems earlier, here is a patch for
compare_tests that will report such cases as:

New tests that FAIL:

arm-sim: (DejaGnu) proc "scan-dump-tree-not fail_test optimized" does not exist.


OK?

OK.
jeff

Re: [PATCH 0/8] make next_*_insn take rtx_insn * arguments

[Jeff Law]

On 09/20/2016 04:38 AM, Bernd Schmidt wrote:



On 09/20/2016 01:16 AM, Segher Boessenkool wrote:

On Mon, Sep 19, 2016 at 06:07:46PM -0400, Trevor Saunders wrote:

- JUMP_LABEL can be a return which is not an insn.  That also seems
  like something that should be improved at some point.

The JUMP_LABEL field within a JUMP_INSN?  That sounds like a bug.


yes, see the comment before the declaration of
rtx_jump_insn::jump_label ()
it can be either a possibly deleted label, or a RETURN or SIMPLE_RETURN
expression.  Memory usage concerns aside its a tempting design to
change, but at the moment itts definitely intended to work this way,
there's plenty of code that checks ANY_RETURN_P (JUMP_LABEL (x)).


Yes.  But rtl.texi still says:

  Return insns count as jumps, but since they do not refer to any
  labels, their @code{JUMP_LABEL} is @code{NULL_RTX}.


And the comment at the top of jump.c:

   Each CODE_LABEL has a count of the times it is used
   stored in the LABEL_NUSES internal field, and each JUMP_INSN
   has one label that it refers to stored in the
   JUMP_LABEL internal field.  With this we can detect labels that
   become unused because of the deletion of all the jumps that
   formerly used them.  The JUMP_LABEL info is sometimes looked
   at by later passes.  For return insns, it contains either a
   RETURN or a SIMPLE_RETURN rtx.

It's intentional, and really there's nothing wrong with it, or with
operands[] containing CODE_LABELs. The problem is trying to force a
static type system onto a dynamically typed data structure.
But what this doesn't answer is why we stuff RETURN/SIMPLE_RETURN into 
the JUMP_LABEL field of a return.  I simply can't remember any rationale 
behind that.


I suspect that if we dig further, we'll find that we can accomplish 
whatever the goal was behind that decision in a way that easily works in 
a world where we have separated rtx objects from objects that are part 
of the insn chain.


Just to be clear, I don't see us going to a world where everything we 
have as an rtx today is a statically typed object. But there are things 
that I think make sense to carve out, the most obvious being objects 
which are part of the insn chain.


jeff

Re: [PATCHv3, resent] Add a warning for suspicious use of conditional expressions in boolean context

[Jason Merrill]

On Tue, Sep 20, 2016 at 10:11 AM, Bernd Edlinger
 wrote:
> On 09/20/16 13:29, Kyrill Tkachov wrote:
>>
>> arm bootstrap is now failing:
>> $SRC/gcc/config/arm/arm.h:2229:40: error: ?: using integer constants in
>> boolean context [-Werror=int-in-bool-context]
>>  : (TARGET_VFP_DOUBLE ? (TARGET_FP16 ? 14 : 12) : 0)) \
>> ~^~
>> $SRC/gcc/config/arm/arm-c.c:133:7: note: in expansion of macro
>> 'TARGET_ARM_FP'
>> if (TARGET_ARM_FP)
>>
>>
>> The full definition of TARGET_ARM_FP is:
>> #define TARGET_ARM_FP\
>>(!TARGET_SOFT_FLOAT ? (TARGET_VFP_SINGLE ? 4\
>>  : (TARGET_VFP_DOUBLE ? (TARGET_FP16 ? 14 : 12) : 0)) \
>>: 0)
>>
>> We want it set to 0 when there's no FP but when FP is available we set
>> it to a bitmask
>> to suggest the level that is available. That seems like a legitimate use
>> to me.
>>
>
> Ok, I see, sorry for that.
>
> I think I will have to suppress the warning if the conditional is in
> a macro somehow.

from_macro_expansion_at will help with that.

Though it seems to me that the issue here is more that (TARGET_FP16 ?
14 : 12) is not in a boolean context, it's in an integer context; only
the outer ?: is in a boolean context.

I also still think the warning message should be changed.

Jason

Re: [PATCH] Optimise the fpclassify builtin to perform integer operations when possible

[Jeff Law]

On 09/20/2016 06:00 AM, Tamar Christina wrote:



On 16/09/16 20:49, Jeff Law wrote:

On 09/12/2016 10:19 AM, Tamar Christina wrote:

Hi All,
+
+  /* Re-interpret the float as an unsigned integer type
+ with equal precision.  */
+  int_arg_type = build_nonstandard_integer_type (TYPE_PRECISION
(type), 0);
+  int_arg = fold_build1_loc (loc, INDIRECT_REF, int_arg_type,
+  fold_build1_loc (loc, NOP_EXPR,
+   build_pointer_type (int_arg_type),
+fold_build1_loc (loc, ADDR_EXPR,
+ build_pointer_type (type), arg)));

Doesn't this make ARG addressable?  Which in turn means ARG won't be
exposed to the gimple/ssa optimizers.Or is it the case that when
fpclassify is used its argument is already in memory (and thus
addressable?)


I believe that it is the case that when fpclassify is use the argument
is already addressable, but I am not 100% certain. I may be able to do
this differently so I'll come back to you on this one.
The more I think about it, the more I suspect ARG is only going to 
already be marked as addressable if it has already had its address taken.



But I think we can look at this as an opportunity.  If ARG is already 
addressable, then it's most likely going to be living in memory (there 
are exceptions).  If ARG is most likely going to be living in memory, 
then we clearly want to use your fast integer path, regardless of the 
target.


If ARG is not addressable, then it's not as clear as the object is 
likely going to be assigned into an FP register.  Integer operations on 
the an FP register likely will force a sequence where we dump the 
register into memory, load from memory into a GPR, then bit test on the 
GPR.  That gets very expensive on some architectures.


Could we defer lowering in the case where the object is not addressable 
until gimple->rtl expansion time?  That's the best time to introduce 
target dependencies into the code we generate.



Jeff

Re: [PATCHv3, resent] Add a warning for suspicious use of conditional expressions in boolean context

[Bernd Edlinger]

On 09/20/16 16:20, Kyrill Tkachov wrote:
>
> On 20/09/16 15:11, Bernd Edlinger wrote:
>> On 09/20/16 13:29, Kyrill Tkachov wrote:
>>> arm bootstrap is now failing:
>>> $SRC/gcc/config/arm/arm.h:2229:40: error: ?: using integer constants in
>>> boolean context [-Werror=int-in-bool-context]
>>>   : (TARGET_VFP_DOUBLE ? (TARGET_FP16 ? 14 : 12) : 0)) \
>>>  ~^~
>>> $SRC/gcc/config/arm/arm-c.c:133:7: note: in expansion of macro
>>> 'TARGET_ARM_FP'
>>>  if (TARGET_ARM_FP)
>>>
>>>
>>> The full definition of TARGET_ARM_FP is:
>>> #define TARGET_ARM_FP\
>>> (!TARGET_SOFT_FLOAT ? (TARGET_VFP_SINGLE ? 4\
>>>   : (TARGET_VFP_DOUBLE ? (TARGET_FP16 ? 14 : 12) : 0)) \
>>> : 0)
>>>
>>> We want it set to 0 when there's no FP but when FP is available we set
>>> it to a bitmask
>>> to suggest the level that is available. That seems like a legitimate use
>>> to me.
>>>
>> Ok, I see, sorry for that.
>>
>> I think I will have to suppress the warning if the conditional is in
>> a macro somehow.
>>
>> Can you work around that for a few days?
>
> I changed to:
>   if (TARGET_ARM_FP != 0)
> in my tree to allow the build to proceed but encountered another
> bootstrap failure:
> In file included from ./tm.h:38:0,
>   from $SRC/gcc/backend.h:28,
>   from $SRC/gcc/regrename.c:23:
> $SRC/gcc/regrename.c: In function 'void rename_chains()':
> $SRC/gcc/config/arm/arm.h:915:4: error: ?: using integer constants in
> boolean context [-Werror=int-in-bool-context]
> (TARGET_ARM \
> ~
>  ? ARM_HARD_FRAME_POINTER_REGNUM  \
>  ^~
>  : THUMB_HARD_FRAME_POINTER_REGNUM)
>  ~~
> $SRC/gcc/regrename.c:484:8: note: in expansion of macro
> 'HARD_FRAME_POINTER_REGNUM'
>  || (HARD_FRAME_POINTER_REGNUM && frame_pointer_needed
>  ^
> That looks like a legitimate bug in regrename.c ?
> The full condition is:
> if (fixed_regs[reg] || global_regs[reg]
> || (!HARD_FRAME_POINTER_IS_FRAME_POINTER &&
> frame_pointer_needed
> && reg == HARD_FRAME_POINTER_REGNUM)
> || (HARD_FRAME_POINTER_REGNUM && frame_pointer_needed
> && reg == FRAME_POINTER_REGNUM))
>
> The condition in the second || looks bogus (what use testing if a
> register is 'non-zero').
>
> So this looks like a useful catch by the warning, though I'm not sure at
> first glance how
> to fix it.
> Kyrill
>

I assume HARD_FRAME_POINTER_REGNUM is never zero.

That looks like it should be

  || (frame_pointer_needed && reg == FRAME_POINTER_REGNUM)


Bernd.

Re: Implement -Wimplicit-fallthrough (version 8)

[Jakub Jelinek]

On Tue, Sep 20, 2016 at 04:24:35PM +0200, Marek Polacek wrote:
> > (to skip over the koenig_p etc. cases) and fallthrough into the argument
> > handling and add another if (function == NULL_TREE) handling after that,
> > which would just build another internal call with the tsubsted arguments.
>  
> I added the assert here, but I spent time implementing this, and it didn't
> really work, because without a testcase it was hard to say whether what I
> had was correct.  E.g., how to determine the return type of the internal
> function, etc.

I agree that without the testcase it is hard and that assert is good enough
for now.  For return type of the internal function, I'd just tsubst the
TREE_TYPE of the CALL_EXPR though.

Jakub

Re: Register numbers in RTL dumps (was Re: [PATCH 0/9] RFC: selftests based on RTL dumps)

[Bernd Schmidt]

On 09/20/2016 04:32 PM, David Malcolm wrote:


To summarize so far: you want every pseudo to have its regno dumped
with a 'p' prefix, and renumber them on dump (and then on load).
OK.


Renumbering is not helpful because it interferes with the view you have 
in the debugger. So, IMO just a prefix, and maybe



  (reg/f:DI v1 virtual-stack-vars)


this.


Bernd

Register numbers in RTL dumps (was Re: [PATCH 0/9] RFC: selftests based on RTL dumps)

[David Malcolm]

On Mon, 2016-09-19 at 11:35 -0600, Jeff Law wrote:
> On 09/16/2016 03:27 PM, David Malcolm wrote:
> > > We should also twiddle how we represent registers in the dumps.
> > > Identifying hard regs by name (so we can map back to a hard reg
> > > if
> > > the
> > > hard regs change), identifying pseudos by number that isn't
> > > affected
> > > if
> > > the hard register set changes ie, p0, p1, p2, p3 where the number
> > > is
> > > REGNO (x) - FIRST_PSEUDO_REGISTER. identifying the virtual
> > > registers,
> > > etc.
> > 
> > Good idea.
> > 
> > I don't know if you saw this yet, but the patch has logic from
> > renumbering pseudos on load (see class regno_remapper), but dumping
> > them as p0, p1 etc and reloading them as such seems much easier for
> > everyone.
> And just an FYI, I think we should do this unconditionally in our
> dumps.

To summarize so far: you want every pseudo to have its regno dumped
with a 'p' prefix, and renumber them on dump (and then on load).
OK.

When dumping regnos, would you want another distinction between
virtuals and non-virtuals in the dump?  For example, given that
LAST_VIRTUAL_POINTER_REGISTER is ((FIRST_VIRTUAL_REGISTER) + 4), this
could mean:

  v0, v1, ..., v4  for the virtual regnos

and, for pseudos that aren't virtual regnos:
  p0, p1, ...
or
  p5, p6, ...
depending on whether we want to start numbering the pseudos at p0 for
LAST_VIRTUAL_REGISTER + 1, or whether we regard v0 as the first pseduo,
and hence p5 is the first non-virtual pseudo.   FWIW I think starting
at p5 is the better approach, given that:
  #define FIRST_VIRTUAL_REGISTER(FIRST_PSEUDO_REGISTER)

Either way, this would give things like:
  (reg/f:DI v1 virtual-stack-vars)
  (reg:DI p78)

In a similar vein, how about adding a "h" prefix for the hard regnos?
That way every regno would have a one-char prefix telling you what kind
of reg it is (and you can directly see whether or not you need to
offset the number by FIRST_PSEUDO_REGISTER to get at the "real" regno).

This would give things (for x86_64) like:
  (reg/i:SI h0 ax)
  (reg/i:SF h21 xmm0)

> > 
> > > The key being rather than put a ton of smarts/hacks in a reader,
> > > we
> > > should work to have the RTL writer give us something more useful.
> > >  That
> > > may mean simple changes to the output, or some conditional
> > > changes
> > > (like
> > > not emitting the INSN_CODE or its name).
> > 
> > That would make the reader a lot less grim; it has a lot of warts
> > for
> > dealing with all the special cases in the current output format.
> That's the idea.
> > 
> > But maybe it is useful to be able to deal with the current output
> > format.  For example, I was able to look at PR 71779 and find some
> > fragmentary RTL dumps there (comment #2) and use them.  I *did*
> > need to
> > edit them a little, so maybe it's OK to require a little editing
> > (especially with older dumps... to what extent has the format
> > changed
> > over the years?)
> It's changed, but not in radical ways.
> 
> I think the case we want to cater to is dumping something from the 
> current compiler rather than picking up some crusty RTL and creating
> a 
> testcase from that.  By "current" I explicitly want the ability to 
> refine the output to make the reader easier to implement.
> 
> Jeff

Re: Implement -Wimplicit-fallthrough (version 8)

[Marek Polacek]

On Fri, Sep 09, 2016 at 06:44:44PM +0200, Jakub Jelinek wrote:
> Hi!
 
Finally getting back to this...

> On Thu, Sep 01, 2016 at 03:40:49PM +0200, Marek Polacek wrote:
> > @@ -1749,6 +1758,16 @@ c_parser_declaration_or_fndef (c_parser *parser, 
> > bool fndef_ok,
> >  {
> >if (auto_type_p)
> > error_at (here, "%<__auto_type%> in empty declaration");
> > +  else if (specs->typespec_kind == ctsk_none && specs->attrs
> > +  && is_attribute_p ("fallthrough",
> > + get_attribute_name (specs->attrs)))
> > +   {
> > + if (fallthru_attr_p != NULL)
> > +   *fallthru_attr_p = true;
> 
> What happens if there are more than one attribute in specs->attrs?
> Either fallthrough not being the first one, or there are other attributes
> afterwards?
> Like:
>   __attribute__((unused, fallthrough)) ;
> or
>   __attribute__((fallthrough, unused)) ;
 
I only accepted __attribute__((fallthrough)); or __attribute__((fallthrough,
unused));, but that is probably wrong.  Fixed in another version of the patch,
where I introduced maybe_attribute_fallthrough_p that detects various valid
and invalid cases.

> > + tree fn = build_call_expr_internal_loc (here, IFN_FALLTHROUGH,
> > + void_type_node, 0);
> > + add_stmt (fn);
> > +   }
> >else if (empty_ok)
> > shadow_tag (specs);
> >else
> > @@ -4845,9 +4864,11 @@ c_parser_compound_statement_nostart (c_parser 
> > *parser)
> > {
> >   last_label = false;
> >   mark_valid_location_for_stdc_pragma (false);
> > + bool fallthru_attr_p = false;
> >   c_parser_declaration_or_fndef (parser, true, true, true, true,
> > -true, NULL, vNULL);
> > - if (last_stmt)
> > +true, NULL, vNULL, NULL,
> > +_attr_p);
> > + if (last_stmt && !fallthru_attr_p)
> > pedwarn_c90 (loc, OPT_Wdeclaration_after_statement,
> >  "ISO C90 forbids mixed declarations and code");
> >   last_stmt = false;
> 
> So, for fallthru_attr_p here you have a guarantee it has been
> __attribute__((fallthrough)) ; which is actualy not a declaration, but
> (empty) statement?  If so, I'd say that for fallthru_attr_p in that case you
> don't want to clear last_stmt, but want to set it (or let it be unchanged?).
> I mean:
>   int a;
>   a = 5;
>   __attribute__((fallthrough)) ;
>   int b;
> would for -std=c99 -pedantic-errors not error out on the declaration of b
> being after the statements (a = 5 and empty stmt).
> What about
>   int a;
>   __attribute__((fallthrough)) ;
>   int b;
> ?  Shall we error on that too?  Pedantically it is also a declaration after
> a statement.
 
Yeah, fixed now.

> > @@ -5327,6 +5360,27 @@ c_parser_statement_after_labels (c_parser *parser, 
> > bool *if_p,
> >   gcc_assert (c_dialect_objc ());
> >   c_parser_objc_synchronized_statement (parser);
> >   break;
> > +   case RID_ATTRIBUTE:
> > + {
> > +   /* Allow '__attribute__((fallthrough));'.  */
> > +   tree attrs = c_parser_attributes (parser);
> > +   if (attrs != NULL_TREE
> > +   && is_attribute_p ("fallthrough", get_attribute_name (attrs)))
> 
> See above.
 
Fixed with maybe_attribute_fallthrough_p.

> > --- gcc/gcc/cp/parser.c
> > +++ gcc/gcc/cp/parser.c
> > @@ -5135,6 +5135,31 @@ cp_parser_primary_expression (cp_parser *parser,
> > case RID_AT_SELECTOR:
> >   return cp_parser_objc_expression (parser);
> >  
> > +   case RID_ATTRIBUTE:
> > + {
> > +   /* This might be __attribute__((fallthrough));.  */
> > +   tree attr = cp_parser_gnu_attributes_opt (parser);
> > +   if (attr != NULL_TREE
> > +   && is_attribute_p ("fallthrough", get_attribute_name (attr)))
> 
> Again.

Fixed with maybe_attribute_fallthrough_p.

> > @@ -10585,14 +10610,25 @@ cp_parser_statement (cp_parser* parser, tree 
> > in_statement_expr,
> > }
> >/* Look for an expression-statement instead.  */
> >statement = cp_parser_expression_statement (parser, 
> > in_statement_expr);
> > +
> > +  /* Handle [[fallthrough]];.  */
> > +  if (std_attrs != NULL_TREE
> > + && is_attribute_p ("fallthrough", get_attribute_name (std_attrs))
> > + && statement == NULL_TREE)
> 
> And again.
 
Fixed with maybe_attribute_fallthrough_p.

> > +   {
> > + statement = build_call_expr_internal_loc (statement_location,
> > +   IFN_FALLTHROUGH,
> > +   void_type_node, 0);
> > + finish_expr_stmt (statement);
> > + std_attrs = NULL_TREE;
> > +   }
> >  }
> >  
> > diff --git gcc/gcc/cp/pt.c gcc/gcc/cp/pt.c
> > index b0f0664..60c5d43 100644
> > --- gcc/gcc/cp/pt.c
> > +++ gcc/gcc/cp/pt.c
> > @@ -16556,7 +16556,9 @@ tsubst_copy_and_build (tree t,
> > tree ret;
> >  
> >

Re: PR35503 - warn for restrict pointer

[Joseph Myers]

On Tue, 20 Sep 2016, Martin Sebor wrote:

> > That's much harder to support in format checking (which expects one length
> > modifier, not a combination like that).
> 
> I haven't looked into it detail but since the format checker supports
> one-to-two character sequences of length modifiers (h or hh, etc) it
> should be possible to extend it to handle one-to-three character
> sequences (h, hV, or hhV, etc.)  The V character would not be a new
> length modifier on its own but instead be recognized as part of
> a longer length modifier sequence.  Do you see a problem with that?

You could arrange for length modifiers to have extra variants like that 
(at present they have single and double variants listed), but it's clearly 
more complicated than a modifier that's used on its own.

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

Re: PR35503 - warn for restrict pointer

[Martin Sebor]

On 09/20/2016 07:00 AM, Joseph Myers wrote:

On Tue, 20 Sep 2016, Martin Sebor wrote:


That said, since this specifier formats a vec, it seems that
it might be useful to be able to format vectors of other elements,
such as short and long.  With that in mind, would adding a new V
length modifier instead be a more regular way to extend the pretty
printer to these types?  The V length modifier would have to be
accepted in conjunction with other length modifiers (h, l, etc
and type specifiers (d, i, o, etc.).  E.g, "%hVu" would accept


That's much harder to support in format checking (which expects one length
modifier, not a combination like that).


I haven't looked into it detail but since the format checker supports
one-to-two character sequences of length modifiers (h or hh, etc) it
should be possible to extend it to handle one-to-three character
sequences (h, hV, or hhV, etc.)  The V character would not be a new
length modifier on its own but instead be recognized as part of
a longer length modifier sequence.  Do you see a problem with that?

Martin

Re: [PATCHv3, resent] Add a warning for suspicious use of conditional expressions in boolean context

[Kyrill Tkachov]

On 20/09/16 15:11, Bernd Edlinger wrote:

On 09/20/16 13:29, Kyrill Tkachov wrote:

arm bootstrap is now failing:
$SRC/gcc/config/arm/arm.h:2229:40: error: ?: using integer constants in
boolean context [-Werror=int-in-bool-context]
  : (TARGET_VFP_DOUBLE ? (TARGET_FP16 ? 14 : 12) : 0)) \
 ~^~
$SRC/gcc/config/arm/arm-c.c:133:7: note: in expansion of macro
'TARGET_ARM_FP'
 if (TARGET_ARM_FP)


The full definition of TARGET_ARM_FP is:
#define TARGET_ARM_FP\
(!TARGET_SOFT_FLOAT ? (TARGET_VFP_SINGLE ? 4\
  : (TARGET_VFP_DOUBLE ? (TARGET_FP16 ? 14 : 12) : 0)) \
: 0)

We want it set to 0 when there's no FP but when FP is available we set
it to a bitmask
to suggest the level that is available. That seems like a legitimate use
to me.


Ok, I see, sorry for that.

I think I will have to suppress the warning if the conditional is in
a macro somehow.

Can you work around that for a few days?


I changed to:
 if (TARGET_ARM_FP != 0)
in my tree to allow the build to proceed but encountered another bootstrap 
failure:
In file included from ./tm.h:38:0,
 from $SRC/gcc/backend.h:28,
 from $SRC/gcc/regrename.c:23:
$SRC/gcc/regrename.c: In function 'void rename_chains()':
$SRC/gcc/config/arm/arm.h:915:4: error: ?: using integer constants in boolean 
context [-Werror=int-in-bool-context]
   (TARGET_ARM \
   ~
? ARM_HARD_FRAME_POINTER_REGNUM  \
^~
: THUMB_HARD_FRAME_POINTER_REGNUM)
~~
$SRC/gcc/regrename.c:484:8: note: in expansion of macro 
'HARD_FRAME_POINTER_REGNUM'
|| (HARD_FRAME_POINTER_REGNUM && frame_pointer_needed
^
That looks like a legitimate bug in regrename.c ?
The full condition is:
   if (fixed_regs[reg] || global_regs[reg]
   || (!HARD_FRAME_POINTER_IS_FRAME_POINTER && frame_pointer_needed
   && reg == HARD_FRAME_POINTER_REGNUM)
   || (HARD_FRAME_POINTER_REGNUM && frame_pointer_needed
   && reg == FRAME_POINTER_REGNUM))

The condition in the second || looks bogus (what use testing if a register is 
'non-zero').

So this looks like a useful catch by the warning, though I'm not sure at first 
glance how
to fix it.
Kyrill



Bernd.

Re: [PATCH][ARM][v2] Use snprintf rather than sprintf

[Richard Earnshaw (lists)]

On 08/09/16 12:00, Kyrill Tkachov wrote:
> Hi all,
> 
> This is a rebase and slight respin of [1] that I sent out last November
> to change all uses of
> sprintf to snprintf in the arm backend.
> 
> Bootstrapped and tested on arm-none-linux-gnueabihf.
> Ok for trunk?
> 
> Thanks,
> Kyrill
> 
> [1] https://gcc.gnu.org/ml/gcc-patches/2015-11/msg00937.html
> 
> 2016-09-08  Kyrylo Tkachov  
> 
> * config/arm/arm.c (arm_set_fixed_optab_libfunc): Use snprintf
> rather than sprintf.
> (arm_set_fixed_conv_libfunc): Likewise.
> (arm_option_override): Likewise.
> (neon_output_logic_immediate): Likewise.
> (neon_output_shift_immediate): Likewise.
> (arm_output_multireg_pop): Likewise.
> (vfp_output_vstmd): Likewise.
> (output_move_vfp): Likewise.
> (output_move_neon): Likewise.
> (output_return_instruction): Likewise.
> (arm_elf_asm_cdtor): Likewise.
> (arm_output_shift): Likewise.
> (arm_output_iwmmxt_shift_immediate): Likewise.
> (arm_output_iwmmxt_tinsr): Likewise.
> * config/arm/neon.md (*neon_mov, VDX): Likewise.
> (*neon_mov, VQXMOV): Likewise.
> (neon_vc_insn): Likewise.
> (neon_vc_insn_unspec): Likewise.

Most of these should assert that truncation did not occur (it's an
internal error if the buffers aren't large enough).  In a few cases we
should call output_operand_lossage on failure since it might be due to a
user writing inline assembly and overflowing a buffer.

I don't think we should silently accept a truncated write.

R.

> 
> arm-snprintf.patch
> 
> 
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index 
> 946f308ca84e232af8af6eca4813464914cbd59c..8ff6c0e18f7c2a2eed72e56402ed582c9f692d2d
>  100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -2388,9 +2388,10 @@ arm_set_fixed_optab_libfunc (optab optable, 
> machine_mode mode,
>char buffer[50];
>  
>if (num_suffix == 0)
> -sprintf (buffer, "__gnu_%s%s", funcname, modename);
> +snprintf (buffer, sizeof (buffer), "__gnu_%s%s", funcname, modename);
>else
> -sprintf (buffer, "__gnu_%s%s%d", funcname, modename, num_suffix);
> +snprintf (buffer, sizeof (buffer), "__gnu_%s%s%d", funcname,
> +   modename, num_suffix);
>  
>set_optab_libfunc (optable, mode, buffer);
>  }
> @@ -2409,8 +2410,8 @@ arm_set_fixed_conv_libfunc (convert_optab optable, 
> machine_mode to,
>&& ALL_FRACT_MODE_P (from) == ALL_FRACT_MODE_P (to))
>  maybe_suffix_2 = "2";
>  
> -  sprintf (buffer, "__gnu_%s%s%s%s", funcname, fromname, toname,
> -maybe_suffix_2);
> +  snprintf (buffer, sizeof (buffer), "__gnu_%s%s%s%s", funcname,
> + fromname, toname, maybe_suffix_2);
>  
>set_conv_libfunc (optable, to, from, buffer);
>  }
> @@ -3163,7 +3164,8 @@ arm_option_override (void)
>if (!arm_selected_tune)
>  arm_selected_tune = _cores[arm_selected_cpu->core];
>  
> -  sprintf (arm_arch_name, "__ARM_ARCH_%s__", arm_selected_cpu->arch);
> +  snprintf (arm_arch_name, sizeof (arm_arch_name),
> + "__ARM_ARCH_%s__", arm_selected_cpu->arch);
>insn_flags = arm_selected_cpu->flags;
>arm_base_arch = arm_selected_cpu->base_arch;
>  
> @@ -12735,9 +12737,11 @@ neon_output_logic_immediate (const char *mnem, rtx 
> *op2, machine_mode mode,
>gcc_assert (is_valid != 0);
>  
>if (quad)
> -sprintf (templ, "%s.i%d\t%%q0, %%2", mnem, width);
> +snprintf (templ, sizeof (templ), "%s.i%d\t%%q0, %%2",
> +   mnem, width);
>else
> -sprintf (templ, "%s.i%d\t%%P0, %%2", mnem, width);
> +snprintf (templ, sizeof (templ), "%s.i%d\t%%P0, %%2",
> +   mnem, width);
>  
>return templ;
>  }
> @@ -12757,9 +12761,11 @@ neon_output_shift_immediate (const char *mnem, char 
> sign, rtx *op2,
>gcc_assert (is_valid != 0);
>  
>if (quad)
> -sprintf (templ, "%s.%c%d\t%%q0, %%q1, %%2", mnem, sign, width);
> +snprintf (templ, sizeof (templ),
> +   "%s.%c%d\t%%q0, %%q1, %%2", mnem, sign, width);
>else
> -sprintf (templ, "%s.%c%d\t%%P0, %%P1, %%2", mnem, sign, width);
> +snprintf (templ, sizeof (templ),
> +   "%s.%c%d\t%%P0, %%P1, %%2", mnem, sign, width);
>  
>return templ;
>  }
> @@ -17858,17 +17864,17 @@ arm_output_multireg_pop (rtx *operands, bool 
> return_pc, rtx cond, bool reverse,
>conditional = reverse ? "%?%D0" : "%?%d0";
>/* Can't use POP if returning from an interrupt.  */
>if ((regno_base == SP_REGNUM) && update && !(interrupt_p && return_pc))
> -sprintf (pattern, "pop%s\t{", conditional);
> +snprintf (pattern, sizeof (pattern), "pop%s\t{", conditional);
>else
>  {
>/* Output ldmfd when the base register is SP, otherwise output ldmia.
>   It's just a convention, their semantics are identical.  */
>if (regno_base == SP_REGNUM)
> - sprintf (pattern, "ldmfd%s\t", conditional);
> + snprintf (pattern, sizeof (pattern), "ldmfd%s\t", conditional);
>else if 

Re: [PATCHv3, resent] Add a warning for suspicious use of conditional expressions in boolean context

[Bernd Edlinger]

On 09/20/16 13:29, Kyrill Tkachov wrote:
>
> arm bootstrap is now failing:
> $SRC/gcc/config/arm/arm.h:2229:40: error: ?: using integer constants in
> boolean context [-Werror=int-in-bool-context]
>  : (TARGET_VFP_DOUBLE ? (TARGET_FP16 ? 14 : 12) : 0)) \
> ~^~
> $SRC/gcc/config/arm/arm-c.c:133:7: note: in expansion of macro
> 'TARGET_ARM_FP'
> if (TARGET_ARM_FP)
>
>
> The full definition of TARGET_ARM_FP is:
> #define TARGET_ARM_FP\
>(!TARGET_SOFT_FLOAT ? (TARGET_VFP_SINGLE ? 4\
>  : (TARGET_VFP_DOUBLE ? (TARGET_FP16 ? 14 : 12) : 0)) \
>: 0)
>
> We want it set to 0 when there's no FP but when FP is available we set
> it to a bitmask
> to suggest the level that is available. That seems like a legitimate use
> to me.
>

Ok, I see, sorry for that.

I think I will have to suppress the warning if the conditional is in
a macro somehow.

Can you work around that for a few days?


Bernd.

Re: [v3 PATCH] PR libstdc++/77619

[Jonathan Wakely]

On 18/09/16 21:07 +0300, Ville Voutilainen wrote:

diff --git a/libstdc++-v3/include/bits/stl_construct.h 
b/libstdc++-v3/include/bits/stl_construct.h
index 3d12628..c7ca1f8 100644
--- a/libstdc++-v3/include/bits/stl_construct.h
+++ b/libstdc++-v3/include/bits/stl_construct.h
@@ -83,6 +83,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
  ::new(static_cast(__p)) _T1(__value);
}
#endif


Blank line here please.


+  template
+inline void
+_Construct_novalue(_T1* __p)
+{ ::new(static_cast(__p)) _T1; }

  /**
   * Destroy the object pointed to by a pointer type.




diff --git a/libstdc++-v3/include/bits/stl_uninitialized.h 
b/libstdc++-v3/include/bits/stl_uninitialized.h
index c5c81fb..b4213d5 100644
--- a/libstdc++-v3/include/bits/stl_uninitialized.h
+++ b/libstdc++-v3/include/bits/stl_uninitialized.h
@@ -640,6 +644,104 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
allocator<_Tp>&)
{ return std::__uninitialized_default_n(__first, __n); }

+  template
+struct __uninitialized_default_novalue_1
+{
+  template
+static void
+__uninit_default_novalue(_ForwardIterator __first,
+_ForwardIterator __last)
+{

 ...

+   }


The left brace is indented with spaces and the right one with a tab,
might as well make them the same.


+};
+
+  template<>
+struct __uninitialized_default_novalue_1
+{
+  template
+static void
+__uninit_default_novalue(_ForwardIterator __first,
+_ForwardIterator __last)
+{
+   }


Ditto.


+};
+
+  template
+struct __uninitialized_default_novalue_n_1
+{
+  template
+static _ForwardIterator
+__uninit_default_novalue_n(_ForwardIterator __first, _Size __n)
+{
+ _ForwardIterator __cur = __first;
+ __try
+   {
+ for (; __n > 0; --__n, ++__cur)
+   std::_Construct_novalue(std::__addressof(*__cur));
+ return __cur;
+   }
+ __catch(...)
+   {
+ std::_Destroy(__first, __cur);
+ __throw_exception_again;
+   }
+   }


Ditto.


+};
+
+  template<>
+struct __uninitialized_default_novalue_n_1
+{
+  template
+static _ForwardIterator
+__uninit_default_novalue_n(_ForwardIterator __first, _Size __n)
+{
+   }


Ditto.


+};
+
+  // __uninitialized_default_novalue
+  // Fills [first, last) with std::distance(first, last) default-initialized
+  // value_types(s).
+  template
+inline void
+__uninitialized_default_novalue(_ForwardIterator __first,
+   _ForwardIterator __last)
+{
+  typedef typename iterator_traits<_ForwardIterator>::value_type
+   _ValueType;
+  // trivial types can have deleted assignment
+  const bool __assignable = is_copy_assignable<_ValueType>::value;


Aha! A non-whitespace comment ... __assignable isn't used. If it's not
needed it can be removed.


+  std::__uninitialized_default_novalue_1<
+   is_trivially_default_constructible<_ValueType>::value>::
+   __uninit_default_novalue(__first, __last);
+}
+
+  // __uninitialized_default_n
+  // Fills [first, first + n) with n default-initialized value_type(s).
+  template
+inline _ForwardIterator
+__uninitialized_default_novalue_n(_ForwardIterator __first, _Size __n)
+{
+  typedef typename iterator_traits<_ForwardIterator>::value_type
+   _ValueType;
+  // trivial types can have deleted assignment
+  const bool __assignable = is_copy_assignable<_ValueType>::value;


Ditto.


+  return __uninitialized_default_novalue_n_1<
+   is_trivially_default_constructible<_ValueType>::value>::
+   __uninit_default_novalue_n(__first, __n);
+}

  template
@@ -669,6 +771,38 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   random_access_iterator_tag)
{ return std::uninitialized_copy(__first, __first + __n, __result); }

+  template
+pair<_InputIterator, _ForwardIterator>
+__uninitialized_copy_n_pair(_InputIterator __first, _Size __n,
+  _ForwardIterator __result, input_iterator_tag)
+{
+  _ForwardIterator __cur = __result;
+  __try
+   {
+ for (; __n > 0; --__n, ++__first, ++__cur)
+   std::_Construct(std::__addressof(*__cur), *__first);
+ return {__first, __cur};
+   }
+  __catch(...)
+   {
+ std::_Destroy(__result, __cur);
+ __throw_exception_again;
+   }
+}
+
+  template
+inline pair<_RandomAccessIterator, _ForwardIterator>
+__uninitialized_copy_n_pair(_RandomAccessIterator __first, _Size __n,
+  _ForwardIterator __result,
+  random_access_iterator_tag)
+{
+  auto second = uninitialized_copy(__first, __first + __n, __result);
+  auto first = 

Re: PR77661 - Fix: --enable-maintainer-mode causes in-tree-build of MPC to fail

[Richard Biener]

On Tue, Sep 20, 2016 at 3:21 PM, Tobias Burnus
 wrote:
> Currently, --enable-maintainer-mode causes an in-tree build MPC to fail (cf. 
> PR).
> The attached patch fixes this for --enable-maintainer-mode.
>
> OK for the trunk?

Ok if nobody objects within 48 hours.

Richard.

> Cheers,
>
> Tobias
>
> PS: Thanks to Richard for the suggestion.

PR77661 - Fix: --enable-maintainer-mode causes in-tree-build of MPC to fail

[Tobias Burnus]

Currently, --enable-maintainer-mode causes an in-tree build MPC to fail (cf. 
PR).
The attached patch fixes this for --enable-maintainer-mode.

OK for the trunk?

Cheers,

Tobias

PS: Thanks to Richard for the suggestion.
	PR boot/77661
	* Makefile.def: Don't pass --enable-maintainer-mode on to an
	in-tree build MPC.
	* Makefile.in: Regenerate.

diff --git a/Makefile.def b/Makefile.def
index 05316a4..3b3f3ea 100644
--- a/Makefile.def
+++ b/Makefile.def
@@ -65,3 +65,3 @@ host_modules= { module= mpfr; lib_path=src/.libs; bootstrap=true;
 host_modules= { module= mpc; lib_path=src/.libs; bootstrap=true;
-		extra_configure_flags='--disable-shared @extra_mpc_gmp_configure_flags@ @extra_mpc_mpfr_configure_flags@';
+		extra_configure_flags='--disable-shared @extra_mpc_gmp_configure_flags@ @extra_mpc_mpfr_configure_flags@ --disable-maintainer-mode';
 		no_install= true; };
diff --git a/Makefile.in b/Makefile.in
index 117fbf5..9005405 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -14101,3 +14101,3 @@ configure-mpc:
 	  $(HOST_CONFIGARGS) --build=${build_alias} --host=${host_alias} \
-	  --target=${target_alias} --disable-shared @extra_mpc_gmp_configure_flags@ @extra_mpc_mpfr_configure_flags@ \
+	  --target=${target_alias} --disable-shared @extra_mpc_gmp_configure_flags@ @extra_mpc_mpfr_configure_flags@ --disable-maintainer-mode \
 	  || exit 1
@@ -14137,3 +14137,3 @@ configure-stage1-mpc:
 	  $(STAGE1_CONFIGURE_FLAGS) \
-	  --disable-shared @extra_mpc_gmp_configure_flags@ @extra_mpc_mpfr_configure_flags@
+	  --disable-shared @extra_mpc_gmp_configure_flags@ @extra_mpc_mpfr_configure_flags@ --disable-maintainer-mode
 @endif mpc-bootstrap
@@ -14171,3 +14171,3 @@ configure-stage2-mpc:
 	  $(STAGE2_CONFIGURE_FLAGS) \
-	  --disable-shared @extra_mpc_gmp_configure_flags@ @extra_mpc_mpfr_configure_flags@
+	  --disable-shared @extra_mpc_gmp_configure_flags@ @extra_mpc_mpfr_configure_flags@ --disable-maintainer-mode
 @endif mpc-bootstrap
@@ -14205,3 +14205,3 @@ configure-stage3-mpc:
 	  $(STAGE3_CONFIGURE_FLAGS) \
-	  --disable-shared @extra_mpc_gmp_configure_flags@ @extra_mpc_mpfr_configure_flags@
+	  --disable-shared @extra_mpc_gmp_configure_flags@ @extra_mpc_mpfr_configure_flags@ --disable-maintainer-mode
 @endif mpc-bootstrap
@@ -14239,3 +14239,3 @@ configure-stage4-mpc:
 	  $(STAGE4_CONFIGURE_FLAGS) \
-	  --disable-shared @extra_mpc_gmp_configure_flags@ @extra_mpc_mpfr_configure_flags@
+	  --disable-shared @extra_mpc_gmp_configure_flags@ @extra_mpc_mpfr_configure_flags@ --disable-maintainer-mode
 @endif mpc-bootstrap
@@ -14273,3 +14273,3 @@ configure-stageprofile-mpc:
 	  $(STAGEprofile_CONFIGURE_FLAGS) \
-	  --disable-shared @extra_mpc_gmp_configure_flags@ @extra_mpc_mpfr_configure_flags@
+	  --disable-shared @extra_mpc_gmp_configure_flags@ @extra_mpc_mpfr_configure_flags@ --disable-maintainer-mode
 @endif mpc-bootstrap
@@ -14307,3 +14307,3 @@ configure-stagefeedback-mpc:
 	  $(STAGEfeedback_CONFIGURE_FLAGS) \
-	  --disable-shared @extra_mpc_gmp_configure_flags@ @extra_mpc_mpfr_configure_flags@
+	  --disable-shared @extra_mpc_gmp_configure_flags@ @extra_mpc_mpfr_configure_flags@ --disable-maintainer-mode
 @endif mpc-bootstrap
@@ -14341,3 +14341,3 @@ configure-stageautoprofile-mpc:
 	  $(STAGEautoprofile_CONFIGURE_FLAGS) \
-	  --disable-shared @extra_mpc_gmp_configure_flags@ @extra_mpc_mpfr_configure_flags@
+	  --disable-shared @extra_mpc_gmp_configure_flags@ @extra_mpc_mpfr_configure_flags@ --disable-maintainer-mode
 @endif mpc-bootstrap
@@ -14375,3 +14375,3 @@ configure-stageautofeedback-mpc:
 	  $(STAGEautofeedback_CONFIGURE_FLAGS) \
-	  --disable-shared @extra_mpc_gmp_configure_flags@ @extra_mpc_mpfr_configure_flags@
+	  --disable-shared @extra_mpc_gmp_configure_flags@ @extra_mpc_mpfr_configure_flags@ --disable-maintainer-mode
 @endif mpc-bootstrap

Re: [GCC][PATCH] Add __artificial__ attribute to Aarch64 NEON intrinsics

[James Greenhalgh]

On Tue, Sep 20, 2016 at 10:09:14AM +0100, James Greenhalgh wrote:
> On Tue, Sep 13, 2016 at 01:15:39PM +0100, Tamar Christina wrote:
>>
> Thanks, I applied this following your script above, and committed it as
> revision 240256, after tweaking one part of the script to replace a double
> space after __attribute__ introduced by the second sed command.

This patch caused a few new failures for AArch64:

  Failures:
gcc.target/aarch64/vect_int32x2x4_1.c
gcc.target/aarch64/vdup_lane_2.c
gcc.target/aarch64/vect_combine_zeroes_1.c

  Bisected to: 


  Author: jgreenhalgh 
  Date:   Tue Sep 20 09:06:13 2016 +

[GCC][PATCH] Add __artificial__ attribute to Aarch64 NEON intrinsics

Committed on behalf of Tamar Christina .

gcc/

* config/aarch64/arm_neon.h: Add gnu_inline and artificial
attributes to all inlined functions and make them extern.



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

Looking in to it, I see that the following intrinsics:

  vst2_s64
  vst2_u64
  vst2_f64
  vst2_s8
  vst3_s64
  vst3_u64
  vst3_f64
  vst3_s8
  vst4_s64
  vst4_u64
  vst4_f64
  vst4_s8

Are getting inlined, but their body is output in the assembly.

Looking in arm_neon.h, the reason seems clear:

  __extension__ extern __inline void
  vst2_s64 (int64_t * __a, int64x1x2_t val)
  {

These intrinsics have always been missing
__attribute__ ((__always_inline__)) and consequently were not updated by your
script to have __gnu_inline__, and now they are marked extern - they miss GNU
Inline semantics and keep their body after inlining.

A patch adding the correct attributes to these intrisnics is preapproved.

Thanks,
James

[PATCH][simple-object] Early LTO debug "debug copier"

[Richard Biener]

This implements (for ELF sofar) the Early LTO debug information copier.
To remind people what this is about with Early LTO Debug the compile-stage
emits the DWARF DIEs as they are at dwarf2out_early_finish into the
object files that also contain the LTO IL.  On ELF they end up in
sections prefixed by .gnu.debuglto_ but otherwise with the usual
DWARF debug section names and flags.  They can appear alongside regular
DWARF debug sections for the fat part of the object (if compiled with
-flto -ffat-lto-objects - the default on targets where linker plugin
support is not available).

Simple C hello world compiled with -g -flto -ffat-lto-objects:

Section Headers:
  [Nr] Name  TypeAddress  OffSize   ES 
Flg Lk Inf Al
  [ 0]   NULL 00 00 00  
0   0  0
  [ 1] .text PROGBITS 40 c6 00  
AX  0   0  1
  [ 2] .data PROGBITS 000106 00 00  
WA  0   0  1
  [ 3] .bss  NOBITS   000106 00 00  
WA  0   0  1
  [ 4] .gnu.debuglto_.debug_info PROGBITS 000106 
99 00   E  0   0  1
  [ 5] .rela.gnu.debuglto_.debug_info RELA 
0011b0 c0 18   I 27   4  8
  [ 6] .gnu.debuglto_.debug_abbrev PROGBITS 00019f 
88 00   E  0   0  1
  [ 7] .gnu.debuglto_.debug_str PROGBITS 000227 
bf 01 MSE  0   0  1
  [ 8] .gnu.lto_.inline.34f47533603cde40 PROGBITS 
0002e6 2b 00   E  0   0  1
  [ 9] .gnu.lto_main.34f47533603cde40 PROGBITS 
000311 0004db 00   E  0   0  1
  [10] .gnu.lto_.symbol_nodes.34f47533603cde40 PROGBITS
 0007ec 4c 00   E  0   0  1
  [11] .gnu.lto_.refs.34f47533603cde40 PROGBITS 
000838 0f 00   E  0   0  1
  [12] .gnu.lto_.decls.34f47533603cde40 PROGBITS 
000847 00039d 00   E  0   0  1
  [13] .gnu.lto_.symtab.34f47533603cde40 PROGBITS 
000be4 14 00   E  0   0  1
  [14] .gnu.lto_.optsPROGBITS 000bf8 b5 00   
E  0   0  1
  [15] .debug_info   PROGBITS 000cad e1 00  
0   0  1
  [16] .rela.debug_info  RELA 001270 000120 18   
I 27  15  8
  [17] .debug_abbrev PROGBITS 000d8e a1 00  
0   0  1
  [18] .debug_arangesPROGBITS 000e2f 30 00  
0   0  1
  [19] .rela.debug_aranges RELA 001390 30 
18   I 27  18  8
  [20] .debug_line   PROGBITS 000e5f 4f 00  
0   0  1
  [21] .rela.debug_line  RELA 0013c0 18 18   
I 27  20  8
  [22] .comment  PROGBITS 000eae 42 01  
MS  0   0  1
  [23] .note.GNU-stack   PROGBITS 000ef0 00 00  
0   0  1
  [24] .eh_frame PROGBITS 000ef0 40 00   
A  0   0  8
  [25] .rela.eh_frameRELA 0013d8 18 18   
I 27  24  8
  [26] .shstrtab STRTAB   0013f0 0001b9 00  
0   0  1
  [27] .symtab   SYMTAB   000f30 000258 18 
28  22  8
  [28] .strtab   STRTAB   001188 25 00  
0   0  1

Symbol table '.symtab' contains 25 entries:
   Num:Value  Size TypeBind   Vis  Ndx Name
...
22:  0 NOTYPE  WEAK   HIDDEN 4 gt.c.675919dd
23:    198 FUNCGLOBAL DEFAULT1 main
24: 0001 1 OBJECT  GLOBAL DEFAULT  COM __gnu_lto_v1

important parts to notice is that there are .gnu.debuglto_ sections
with relocations and with symbols we have to preserve.

The task of the simple-object worker is to copy the .gnu.debuglto_
sections to a new object, stripping the section name mangling
(and removing the Exclude flag).  The result of the operation to the
above sections is

Section Headers:
  [Nr] Name  TypeAddress  OffSize   ES 
Flg Lk Inf Al
  [ 0]   NULL 00 00 00  
0   0  0
  [ 1]   NULL 0007c0 00 00   
E  0   0  1
  [ 2]   NULL 0007c0 00 00   
E  0   0  1
  [ 3]   NULL 0007c0 00 00   
E  0   0  1
  [ 4] .debug_info   PROGBITS 0007c0 99 00  
0   0  1
  [ 5] .rela.gnu.debuglto_.debug_info RELA 
000860 c0 18   I 27   4  8
  [ 6] .debug_abbrev PROGBITS 000920 88 00  
0   0  1
  [ 7] .debug_strPROGBITS 0009a8 bf 01  
MS  

Re: [PATCH] manual: _Bool has trap representations

[Joseph Myers]

On Tue, 20 Sep 2016, Florian Weimer wrote:

> On 09/19/2016 11:26 PM, Joseph Myers wrote:
> > On powerpc-darwin _Bool is 4-byte not 1-byte, so saying values are
> > represented as bytes isn't accurate for all systems supported by GCC.
> 
> Interesting.
> 
> Is the treatment of 0/1/the rest still the same there?

Yes.

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

Re: PR35503 - warn for restrict pointer

[Joseph Myers]

On Tue, 20 Sep 2016, Prathamesh Kulkarni wrote:

> Could someone please take a look at the change to c-format.c, I am not sure
> if I have added that correctly.

Any changes to these GCC formats also require tests to be updated 
(gcc.dg/format/gcc_diag*).

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

Re: PR35503 - warn for restrict pointer

[Joseph Myers]

On Tue, 20 Sep 2016, Martin Sebor wrote:

> That said, since this specifier formats a vec, it seems that
> it might be useful to be able to format vectors of other elements,
> such as short and long.  With that in mind, would adding a new V
> length modifier instead be a more regular way to extend the pretty
> printer to these types?  The V length modifier would have to be
> accepted in conjunction with other length modifiers (h, l, etc
> and type specifiers (d, i, o, etc.).  E.g, "%hVu" would accept

That's much harder to support in format checking (which expects one length 
modifier, not a combination like that).

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

Re: Re: [PATCH, 5.x/6.x/7.x] Be more conservative in early inliner if FDO is enabled

[Yuan, Pengfei]

> >> > Btw, It occurs to me that then win in code-size might be purely due to 
> >> > the
> >> > smaller base value for the TU size we use to compute the maximum unit
> >> > growth with ... any idea how to improve it on this side?  Say, computing
> >> > the TU size before early optimization (uh, probably not ...)
> >> >
> >> > That said, the inliner always completely fills its budged, that is, 
> >> > increase
> >> > the unit by max-unit-growth?
> >>
> >> What I'm trying to say is that rather than limiting early inlining we 
> >> should
> >> maybe decrease inline-unit-growth when FDO is in effect?  Because we
> >> can better control where the inlining goes.  If there is over 8% reduction
> >> in size benchmarking (unpatched) compiler on Firefox with FDO and
> >> --param inline-unit-growth=12 might show if the results are the same.
> >>
> >> Richard.
> >>
> >> > Richard.
> >
> > AFAIK, param inline-unit-growth only affects pass_ipa_inline. However, the 
> > code
> > size reduction achieved with my patch is from pass_early_inline.
> 
> Any inlining you don't do at early inlining time will a) decrease the
> size inline-unit-growth
> is computed on b) just delays inlining to IPA inlining (with the now
> more constrained size budget).

Delaying inlining from early inlining to IPA inlining can save size because 
profile
feedback is effective at IPA inlining and inlining of cold functions can be 
avoided.

Otherwise, inlining done at early inlining can not be canceled later.

Yuan, Pengfei

> Richard.
> 
> > Yuan, Pengfei

Re: [accaf, fortran, 4/4] Allocatable components support in derived typed coarrays

[Andre Vehreschild]

Hi Andreas,

thanks for the report. I better use + there instead of the fixed number of
digits. Fixed as obvious as r240262.

Thanks again.

- Andre

On Tue, 20 Sep 2016 14:18:46 +0200
Andreas Schwab  wrote:

> On Sep 19 2016, Andre Vehreschild  wrote:
> 
> > Index: gcc/testsuite/gfortran.dg/coarray_allocate_7.f08
> > ===
> > --- gcc/testsuite/gfortran.dg/coarray_allocate_7.f08(nicht existent)
> > +++ gcc/testsuite/gfortran.dg/coarray_allocate_7.f08(Arbeitskopie)
> > @@ -0,0 +1,27 @@
> > +! { dg-do run }
> > +! { dg-options "-fcoarray=lib -lcaf_single -fdump-tree-original" }
> > +
> > +! Contributed by Damian Rouson
> > +! Checking whether (de-)registering of coarrays works.
> > +
> > +program main
> > +
> > +  implicit none
> > +
> > +  type mytype
> > +integer, allocatable :: indices(:)
> > +  end type
> > +
> > +  type(mytype), save :: object[*]
> > +  integer :: i,me
> > +
> > +  me=this_image() ! me is always 1 here
> > +  object%indices=[(i,i=1,me)]
> > +  if ( size(object%indices) /= 1 ) call abort()
> > +  ! therefore no array is present here and no array test needed.
> > +  if ( object%indices(1) /= 1 ) call abort()
> > +end program
> > +
> > +! { dg-final { scan-tree-dump-times "_gfortran_caf_register
> > \\(D.\[0-9\]{4}, 1, &\\(\\(struct mytype\\) \\*object\\).indices.token,
> > &\\(\\(struct mytype\\) \\*object\\).indices, 0B, 0B, 0\\);" 2
> > "original" } } +! { dg-final { scan-tree-dump-times
> > "_gfortran_caf_deregister \\(&\\(\\(struct mytype\\)
> > \\*object\\).indices.token, 0B, 0B, 0\\);" 1 "original" } }
> > +  
> 
> FAIL: gfortran.dg/coarray_allocate_7.f08   -O0   scan-tree-dump-times
> original "_gfortran_caf_register \\(D.[0-9]{4}, 1, &\\(\\(struct mytype\\)
> \\*object\\).indices.token, &\\(\\(struct mytype\\) \\*object\\).indices, 0B,
> 0B, 0\\);" 2 PASS: gfortran.dg/coarray_allocate_7.f08   -O0
> scan-tree-dump-times original "_gfortran_caf_deregister \\(&\\(\\(struct
> mytype\\) \\*object\\).indices.token, 0B, 0B, 0\\);" 1
> 
> $ grep _gfortran_caf_register coarray_allocate_7.f08.003t.original 
>   _gfortran_caf_register (28, 0, (void * *) _token.0, (void *) ,
> 0B, 0B, 0); _gfortran_caf_register (D.986, 1, &((struct mytype)
> *object).indices.token, &((struct mytype) *object).indices, 0B, 0B, 0);
> _gfortran_caf_register (D.986, 1, &((struct mytype) *object).indices.token,
> &((struct mytype) *object).indices, 0B, 0B, 0);
> 
> Andreas.
> 


-- 
Andre Vehreschild * Email: vehre ad gmx dot de 
Index: gcc/testsuite/ChangeLog
===
--- gcc/testsuite/ChangeLog	(Revision 240261)
+++ gcc/testsuite/ChangeLog	(Arbeitskopie)
@@ -1,3 +1,8 @@
+2016-09-20  Andre Vehreschild  
+
+	* gfortran.dg/coarray_allocate_7.f08: Using + instead of fixed number
+	of digits expected.
+
 2016-09-20  Richard Biener  
 
 	PR tree-optimization/77646
Index: gcc/testsuite/gfortran.dg/coarray_allocate_7.f08
===
--- gcc/testsuite/gfortran.dg/coarray_allocate_7.f08	(Revision 240261)
+++ gcc/testsuite/gfortran.dg/coarray_allocate_7.f08	(Arbeitskopie)
@@ -22,6 +22,6 @@
   if ( object%indices(1) /= 1 ) call abort()
 end program
 
-! { dg-final { scan-tree-dump-times "_gfortran_caf_register \\(D.\[0-9\]{4}, 1, &\\(\\(struct mytype\\) \\*object\\).indices.token, &\\(\\(struct mytype\\) \\*object\\).indices, 0B, 0B, 0\\);" 2 "original" } }
+! { dg-final { scan-tree-dump-times "_gfortran_caf_register \\(D.\[0-9\]+, 1, &\\(\\(struct mytype\\) \\*object\\).indices.token, &\\(\\(struct mytype\\) \\*object\\).indices, 0B, 0B, 0\\);" 2 "original" } }
 ! { dg-final { scan-tree-dump-times "_gfortran_caf_deregister \\(&\\(\\(struct mytype\\) \\*object\\).indices.token, 0B, 0B, 0\\);" 1 "original" } }
 

Re: [PATCH 3/3] S/390: Improved risbg usage.

[Dominik Vogt]

On Tue, Sep 20, 2016 at 01:37:18PM +0100, Dominik Vogt wrote:
> The following series of patches improves usage of the risbg and
> risbgn instructions on s390/s390x.  The patches have been
> regression tested on s390 and s390x and pass the Spec2006
> testsuite without any negative effects.

> Patch 3 adds new patterns and tests based on the first two patches
> to make better use of the risbg and risbgn instructions.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany
gcc/ChangeLog

* config/s390/s390.md ("*extzv_zEC12", "*extzv_z10")
("*extzv"):
Correct a typo in a comment.
Merged patterns.
("*insv_zEC12", "*insv_z10")
("*insv"): Ditto.
("*insv_zEC12_appendbitsleft")
("*insv_appendbitsleft")
("*insv_z10_appendbitsleft"): Ditto.
("*insv_zEC12_noshift", "*insv_z10_noshift")
("*insv_noshift"): Ditto.
Provide pattern with operands switched.
("*pre_z10_extv"):
Use new subst patterns.
("*extzvdi_lshiftrt", "*_ior_and_sr_ze")
("*extvsidi", "*_and_subregdi_rotr")
("*_and_subregdi_rotl", "*_di_and_rot")
("*insv_z10_noshift_cc", "*insv_z10_noshift_cconly")
("*__ior_and_lshiftrt")
("*_sidi_ior_and_lshiftrt")
("*trunc_sidi_and_subreg_lshrt"):
New patterns.
("*extzv__sll", "*extzv__srl")
("*extzv__srl")
("*extzv__sll"): Renamed patterns, use risbgn
on zEC12.
("SINT"): New mode_iterator with SI, HI, QI.
* config/s390/subst.md ("clobbercc_or_nocc_subst", "z10_or_zEC12_cond")
("clobbercc_or_nocc", "risbg_n"): New constructs for risbg pattern
duplication.
gcc/testsuite/ChangeLog

* gcc.target/s390/risbg-ll-1.c: Ported risbg tests from llvm.
* gcc.target/s390/risbg-ll-2.c: Ditto.
* gcc.target/s390/risbg-ll-3.c: Ditto.
>From cdba47ef23798cb17812f21516d40dcd24c18a17 Mon Sep 17 00:00:00 2001
From: Dominik Vogt 
Date: Fri, 5 Aug 2016 08:49:04 +0100
Subject: [PATCH 3/3] S/390: Improved risbg usage.

---
 gcc/config/s390/s390.md| 251 +++
 gcc/config/s390/subst.md   |  21 ++
 gcc/testsuite/gcc.target/s390/risbg-ll-1.c | 498 +
 gcc/testsuite/gcc.target/s390/risbg-ll-2.c | 123 +++
 gcc/testsuite/gcc.target/s390/risbg-ll-3.c |  44 +++
 5 files changed, 871 insertions(+), 66 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/s390/risbg-ll-1.c
 create mode 100644 gcc/testsuite/gcc.target/s390/risbg-ll-2.c
 create mode 100644 gcc/testsuite/gcc.target/s390/risbg-ll-3.c

diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md
index 2848b41..9de442f 100644
--- a/gcc/config/s390/s390.md
+++ b/gcc/config/s390/s390.md
@@ -606,6 +606,7 @@
 ;; same template.
 (define_mode_iterator INT [(DI "TARGET_ZARCH") SI HI QI])
 (define_mode_iterator DINT [(TI "TARGET_ZARCH") DI SI HI QI])
+(define_mode_iterator SINT [SI HI QI])
 
 ;; This iterator allows some 'ashift' and 'lshiftrt' pattern to be defined from
 ;; the same template.
@@ -3750,25 +3751,93 @@
 }
 })
 
-(define_insn "*extzv_zEC12"
+(define_insn "*extzv"
   [(set (match_operand:GPR 0 "register_operand" "=d")
   (zero_extract:GPR
 (match_operand:GPR 1 "register_operand" "d")
 (match_operand 2 "const_int_operand" "")   ; size
-(match_operand 3 "const_int_operand" "")))] ; start]
-  "TARGET_ZEC12"
-  "risbgn\t%0,%1,64-%2,128+63,%3+%2" ; dst, src, start, end, shift
-  [(set_attr "op_type" "RIE")])
+(match_operand 3 "const_int_operand" ""))) ; start
+  ]
+  ""
+  "\t%0,%1,64-%2,128+63,%3+%2" ; dst, src, start, end, 
shift
+  [(set_attr "op_type" "RIE")
+   (set_attr "z10prop" "z10_super_E1")])
 
-(define_insn "*extzv_z10"
-  [(set (match_operand:GPR 0 "register_operand" "=d")
-  (zero_extract:GPR
-   (match_operand:GPR 1 "register_operand" "d")
-   (match_operand 2 "const_int_operand" "")   ; size
-   (match_operand 3 "const_int_operand" ""))) ; start
-   (clobber (reg:CC CC_REGNUM))]
-  "TARGET_Z10"
-  "risbg\t%0,%1,64-%2,128+63,+%3+%2" ; dst, src, start, end, shift
+; 64 bit: (a & -16) | ((b >> 8) & 15)
+(define_insn "*extzvdi_lshiftrt"
+  [(set (zero_extract:DI (match_operand:DI 0 "register_operand" "+d")
+(match_operand 1 "const_int_operand" "")  ; size
+(match_operand 2 "const_int_operand" "")) ; start
+   (lshiftrt:DI (match_operand:DI 3 "register_operand" "d")
+(match_operand:DI 4 "nonzero_shift_count_operand" "")))]
+  "
+   && 64 - UINTVAL (operands[4]) >= UINTVAL (operands[1])"
+  "\t%0,%3,%2,%2+%1-1,128-%2-%1-%4"
+  [(set_attr "op_type" "RIE")
+   (set_attr "z10prop" "z10_super_E1")])
+
+; 32 bit: (a & -16) | ((b >> 8) & 15)
+(define_insn "*_ior_and_sr_ze"
+  [(set (match_operand:SI 0 "register_operand" "=d")
+   (ior:SI (and:SI
+(match_operand:SI 1 

Re: [PATCH 2/3] S/390: Enable wraparound in s390_contiguous_bitmask_p.

[Dominik Vogt]

On Tue, Sep 20, 2016 at 01:37:18PM +0100, Dominik Vogt wrote:
> The following series of patches improves usage of the risbg and
> risbgn instructions on s390/s390x.  The patches have been
> regression tested on s390 and s390x and pass the Spec2006
> testsuite without any negative effects.

> Patch 2 enables wraparound of bit ranges and rewrites portion of
> the bitrange calculation code to do so.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany
gcc/ChangeLog

* config/s390/predicates.md ("contiguous_bitmask_operand"): Adapt to new
interface of s390_contiguous_bitmask_p.
("contiguous_bitmask_nowrap_operand"): New predicate.
* ("*anddi3_cc", "*anddi3_cconly", "*anddi3"): Replace NxxDq with NxxDw.
* config/s390/constraints.md ("NxxDw", "NxxSq"): Adapt to new interface
of s390_contiguous_bitmask_p.
* ("NxxDw"): Rename NxxDq constraint to NxxDw.
("NxxSw"): New constraint.
* config/s390/s390.md ("*andsi3_zarch"): Enable bitmask wraparound.
* config/s390/s390-protos.h (s390_contiguous_bitmask_p): Updated
interface.
(s390_contiguous_bitmask_nowrap_p): Export.
* config/s390/s390.c (s390_contiguous_bitmask_nowrap_p): New name of
former s390_contiguous_bitmask_p.
(s390_contiguous_bitmask_p): Use s390_contiguous_bitmask_nowrap_p to
detect contiguous bit ranges with wraparound.  Change signature to
return START and END position instead of POS and LENGTH.
(s390_contiguous_bitmask_vector_p): Remove extra code for continous bit
ranges with wraparound.
(s390_extzv_shift_ok): Use s390_contiguous_bitmask_nowrap_p.
(s390_contiguous_bitmask_vector_p,s390_extzv_shift_ok,print_operand):
Adapt to new signature of s390_contiguous_bitmask_p.
>From 2de05c8978eb0e681bead0c7a877e9fa10293440 Mon Sep 17 00:00:00 2001
From: Dominik Vogt 
Date: Wed, 10 Aug 2016 14:45:12 +0100
Subject: [PATCH 2/3] S/390: Enable wraparound in s390_contiguous_bitmask_p.

---
 gcc/config/s390/constraints.md |  18 +++-
 gcc/config/s390/predicates.md  |  12 ++-
 gcc/config/s390/s390-protos.h  |   2 +-
 gcc/config/s390/s390.c | 200 +
 gcc/config/s390/s390.md|   8 +-
 5 files changed, 151 insertions(+), 89 deletions(-)

diff --git a/gcc/config/s390/constraints.md b/gcc/config/s390/constraints.md
index 190cdc9..ee505d0 100644
--- a/gcc/config/s390/constraints.md
+++ b/gcc/config/s390/constraints.md
@@ -55,7 +55,12 @@
 ;; D,S,H:   mode of the containing operand
 ;; 0,F: value of the other parts (F - all bits set)
 ;; --
-;; xx[DS]q  satisfies s390_contiguous_bitmask_p for DImode or SImode
+;; xxDq satisfies s390_contiguous_bitmask_p for DImode
+;;  (with possible wraparound of the one-bit range)
+;; xxSw satisfies s390_contiguous_bitmask_p for SImode
+;;  (with possible wraparound of the one-bit range)
+;; xxSq satisfies s390_contiguous_bitmask_nowrap_p for SImode
+;;  (without wraparound of the one-bit range)
 ;;
 ;; The constraint matches if the specified part of a constant
 ;; has a value different from its other parts.  If the letter x
@@ -346,15 +351,20 @@
   (and (match_code "const_int")
(match_test "s390_N_constraint_str (\"xQH0\", ival)")))
 
-(define_constraint "NxxDq"
+(define_constraint "NxxDw"
   "@internal"
   (and (match_code "const_int")
-   (match_test "s390_contiguous_bitmask_p (ival, 64, NULL, NULL)")))
+   (match_test "s390_contiguous_bitmask_p (ival, true, 64, NULL, NULL)")))
 
 (define_constraint "NxxSq"
   "@internal"
   (and (match_code "const_int")
-   (match_test "s390_contiguous_bitmask_p (ival, 32, NULL, NULL)")))
+   (match_test "s390_contiguous_bitmask_p (ival, false, 32, NULL, NULL)")))
+
+(define_constraint "NxxSw"
+  "@internal"
+  (and (match_code "const_int")
+   (match_test "s390_contiguous_bitmask_p (ival, true, 32, NULL, NULL)")))
 
 ;;
 ;; Double-letter constraints starting with O follow.
diff --git a/gcc/config/s390/predicates.md b/gcc/config/s390/predicates.md
index 75e4cb8..5b57344 100644
--- a/gcc/config/s390/predicates.md
+++ b/gcc/config/s390/predicates.md
@@ -176,10 +176,20 @@
   return false;
 })
 
+; Predicate that always allows wraparound of the one-bit range.
 (define_predicate "contiguous_bitmask_operand"
   (match_code "const_int")
 {
-  return s390_contiguous_bitmask_p (INTVAL (op), GET_MODE_BITSIZE (mode), 
NULL, NULL);
+  return s390_contiguous_bitmask_p (INTVAL (op), true,
+GET_MODE_BITSIZE (mode), NULL, NULL);
+})
+
+; Same without wraparound.
+(define_predicate "contiguous_bitmask_nowrap_operand"
+  (match_code "const_int")
+{
+  return s390_contiguous_bitmask_p
+(INTVAL (op), false, GET_MODE_BITSIZE (mode), NULL, NULL);
 })
 
 ;; Return true if OP 

Re: [PATCH 1/3] S/390: Mode attrs "bitoff[_plus]" simplify risbg instructions.

[Dominik Vogt]

On Tue, Sep 20, 2016 at 01:37:18PM +0100, Dominik Vogt wrote:
> The following series of patches improves usage of the risbg and
> risbgn instructions on s390/s390x.  The patches have been
> regression tested on s390 and s390x and pass the Spec2006
> testsuite without any negative effects.
 
> Patch 1 introduces a new mode attribute to simplify some
> instruction patterns.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany
gcc/ChangeLog

* config/s390/s390.md (bitoff, bitoff_plus): Neq mode attributes.
("*extzv_zEC12", "*insv_zEC12", "*insv_z10")
("*insv_zEC12_appendbitsleft")
("*insv_z10_appendbitsleft", "*rsbg__sll")
("*rsbg__srl"): Use new attributes.
gcc/testsuite/ChangeLog

* gcc.target/s390/md/rXsbg_mode_sXl.c: Adapt expected assembly output
to the simplified instructions.
>From 08ea0513fe57e0fdb7582ac7c4ff950495898e3a Mon Sep 17 00:00:00 2001
From: Dominik Vogt 
Date: Fri, 5 Aug 2016 08:52:44 +0100
Subject: [PATCH 1/3] S/390: Mode attrs "bitoff[_plus]" simplify risbg
 instructions.

---
 gcc/config/s390/s390.md   | 17 ++---
 gcc/testsuite/gcc.target/s390/md/rXsbg_mode_sXl.c | 16 
 2 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md
index 30ddc14..21ff33e 100644
--- a/gcc/config/s390/s390.md
+++ b/gcc/config/s390/s390.md
@@ -761,6 +761,9 @@
 
 ;; In place of GET_MODE_BITSIZE (mode)
 (define_mode_attr bitsize [(DI "64") (SI "32") (HI "16") (QI "8")])
+;; 64 - bitsize
+(define_mode_attr bitoff [(DI "0") (SI "32") (HI "48") (QI "56")])
+(define_mode_attr bitoff_plus [(DI "") (SI "32+") (HI "48+") (QI "56+")])
 
 ;; In place of GET_MODE_SIZE (mode)
 (define_mode_attr modesize [(DI "8") (SI "4")])
@@ -3754,7 +3757,7 @@
 (match_operand 2 "const_int_operand" "")   ; size
 (match_operand 3 "const_int_operand" "")))] ; start]
   "TARGET_ZEC12"
-  "risbgn\t%0,%1,64-%2,128+63,+%3+%2" ; dst, src, start, end, shift
+  "risbgn\t%0,%1,64-%2,128+63,%3+%2" ; dst, src, start, end, shift
   [(set_attr "op_type" "RIE")])
 
 (define_insn "*extzv_z10"
@@ -3846,7 +3849,7 @@
(match_operand:GPR 3 "nonimmediate_operand" "d"))]
   "TARGET_ZEC12
&& (INTVAL (operands[1]) + INTVAL (operands[2])) <= "
-  "risbgn\t%0,%3,64-+%2,64-+%2+%1-1,-%2-%1"
+  "risbgn\t%0,%3,%2,%2+%1-1,-%2-%1"
   [(set_attr "op_type" "RIE")])
 
 (define_insn "*insv_z10"
@@ -3857,7 +3860,7 @@
(clobber (reg:CC CC_REGNUM))]
   "TARGET_Z10
&& (INTVAL (operands[1]) + INTVAL (operands[2])) <= "
-  "risbg\t%0,%3,64-+%2,64-+%2+%1-1,-%2-%1"
+  "risbg\t%0,%3,%2,%2+%1-1,-%2-%1"
   [(set_attr "op_type" "RIE")
(set_attr "z10prop" "z10_super_E1")])
 
@@ -3894,7 +3897,7 @@
 (ashift:GPR (match_operand:GPR 3 "nonimmediate_operand" "d")
 (match_operand:GPR 4 "nonzero_shift_count_operand" 
""]
   "TARGET_ZEC12 && UINTVAL (operands[2]) == (1UL << UINTVAL (operands[4])) - 1"
-  "risbgn\t%0,%3,64-,64-%4-1,%4"
+  "risbgn\t%0,%3,,64-%4-1,%4"
   [(set_attr "op_type" "RIE")
(set_attr "z10prop" "z10_super_E1")])
 
@@ -3906,7 +3909,7 @@
 (match_operand:GPR 4 "nonzero_shift_count_operand" 
""
(clobber (reg:CC CC_REGNUM))]
   "TARGET_Z10 && !TARGET_ZEC12 && UINTVAL (operands[2]) == (1UL << UINTVAL 
(operands[4])) - 1"
-  "risbg\t%0,%3,64-,64-%4-1,%4"
+  "risbg\t%0,%3,,64-%4-1,%4"
   [(set_attr "op_type" "RIE")
(set_attr "z10prop" "z10_super_E1")])
 
@@ -4035,7 +4038,7 @@
  (match_operand:GPR 3 "nonimmediate_operand" "0")))
(clobber (reg:CC CC_REGNUM))]
   "TARGET_Z10"
-  "rsbg\t%0,%1,64-,63-%2,%2"
+  "rsbg\t%0,%1,,63-%2,%2"
   [(set_attr "op_type" "RIE")])
 
 ;; unsigned {int,long} a, b
@@ -4050,7 +4053,7 @@
  (match_operand:GPR 3 "nonimmediate_operand" "0")))
(clobber (reg:CC CC_REGNUM))]
   "TARGET_Z10"
-  "rsbg\t%0,%1,64-+%2,63,64-%2"
+  "rsbg\t%0,%1,%2,63,64-%2"
   [(set_attr "op_type" "RIE")])
 
 ;; These two are generated by combine for s.bf &= val.
diff --git a/gcc/testsuite/gcc.target/s390/md/rXsbg_mode_sXl.c 
b/gcc/testsuite/gcc.target/s390/md/rXsbg_mode_sXl.c
index 178a537..ad442da 100644
--- a/gcc/testsuite/gcc.target/s390/md/rXsbg_mode_sXl.c
+++ b/gcc/testsuite/gcc.target/s390/md/rXsbg_mode_sXl.c
@@ -39,28 +39,28 @@ rosbg_si_sll (unsigned int a, unsigned int b)
 {
   return a | (b << 1);
 }
-/* { dg-final { scan-assembler-times "rosbg\t%r.,%r.,64-32,63-1,1" 1 } } */
+/* { dg-final { scan-assembler-times "rosbg\t%r.,%r.,32,63-1,1" 1 } } */
 
 __attribute__ ((noinline)) unsigned int
 rosbg_si_srl (unsigned int a, unsigned int b)
 {
   return a | (b >> 2);
 }
-/* { dg-final { scan-assembler-times "rosbg\t%r.,%r.,64-32\\+2,63,64-2" 1 } } 
*/
+/* { dg-final { scan-assembler-times "rosbg\t%r.,%r.,32\\+2,63,64-2" 1 } } */
 
 __attribute__ ((noinline)) unsigned int
 rxsbg_si_sll (unsigned int a, unsigned int b)
 {
   return a ^ (b << 

[PATCH 0/3] S/390: Improved risbg usage.

[Dominik Vogt]

The following series of patches improves usage of the risbg and
risbgn instructions on s390/s390x.  The patches have been
regression tested on s390 and s390x and pass the Spec2006
testsuite without any negative effects.

Patch 1 introduces a new mode attribute to simplify some
instruction patterns.

Patch 2 enables wraparound of bit ranges and rewrites portion of
the bitrange calculation code to do so.

Patch 3 adds new patterns and tests based on the first two patches
to make better use of the risbg and risbgn instructions.

Please check the individual Changelogs for details.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

Re: [PATCH] Tree-level fix for PR 69526

[Robin Dapp]

Hi,

> I meant to do sth like
> 
> Index: tree-ssa-propagate.c
> ===
> --- tree-ssa-propagate.c(revision 240133)
> +++ tree-ssa-propagate.c(working copy)
> @@ -1105,10 +1105,10 @@ substitute_and_fold_dom_walker::before_d
>/* Replace real uses in the statement.  */
>did_replace |= replace_uses_in (stmt, get_value_fn);
> 
> -  /* If we made a replacement, fold the statement.  */
> -  if (did_replace)
> +  /* Fold the statement.  */
> +  if (fold_stmt (, follow_single_use_edges))
> {
> - fold_stmt (, follow_single_use_edges);
> + did_replace = true;
>   stmt = gsi_stmt (i);
> }
> 
> this would need compile-time cost evaluation (and avoid the tree-vrp.c
> folding part
> of your patch).

Using this causes the simplifications to be performed in ccp1 instead of
fwprop1. I noticed two tests failing that rely on propagation being
performed in fwprop. Should these be adapted or rather the patch be changed?

> Heh, but I think duplicating code is even worse.

Ok, I changed extract_range_... after all, it's not so bad as I had
thought.  Imho it should be simplified nevertheless, perhaps I'll do it
in the future if I find some time.

> + tree cst_inner = wide_int_to_tree (inner_type, cst);
> 
> What prevents CST being truncated here?  It looks like
> (long)intvar + 0xL will be mishandled that way.
> 

Right, it may be truncated here, but the following TYPE_PRECISION ()
guard prevents the value from being used in that case.  I pulled the
line into the condition for clarity.

> OTOH given that you use this to decide whether you can use a combined constant
> that doesn't look necessary (if the constant is correct, that is) --
> what you need
> to make sure is that the final operation, (T)(A) +- CST, does not overflow
> if ! TYPE_OVERFLOW_WRAPS and there wasn't any overflow in the
> original operation.  I think that always holds, thus the combine_ovf check 
> looks
> superfluous to me.
> 

I included this to account for cases like
 return (long)(a + 2) + LONG_MAX
which should not simplify as opposed to
 return (unsigned long)(a + 2) + LONG_MAX
where the constant is usable despite the overflow. Do you think it
should be handled differently?

Revised version attached.

Regards
 Robin

--

gcc/ChangeLog:

2016-09-20  Robin Dapp  

PR middle-end/69526
This enables combining of wrapped binary operations and fixes
the tree level part of the PR.

* match.pd: Introduce patterns to simplify binary operations
wrapped inside a cast.
* tree-vrp.h: Export extract_range_from_binary_expr ().
* tree-ssa-propagate.c
(substitute_and_fold_dom_walker::before_dom_children):
Try to fold regardless of prior use propagations.
* tree-vrp.c (extract_range_from_binary_expr_1):
Add overflow check arguments and wrapper function without them.
(extract_range_from_binary_expr):
 Add wrapper function.


gcc/testsuite/ChangeLog:

2016-09-20  Robin Dapp  

* gcc.dg/wrapped-binop-simplify-signed-1.c: New test.
* gcc.dg/wrapped-binop-simplify-signed-2.c: New test.
* gcc.dg/wrapped-binop-simplify-unsigned-1.c: New test.
* gcc.dg/wrapped-binop-simplify-unsigned-2.c: New test.
diff --git a/gcc/match.pd b/gcc/match.pd
index 123e754..f624e7e 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -1138,6 +1138,130 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(minus @0 (minus @0 @1))
@1)
 
+  /* ((T)(A +- CST)) +- CST -> (T)(A) +- CST)  */
+#if GIMPLE
+   (for outer_op (plus minus)
+ (for inner_op (plus minus)
+   (simplify
+	 (outer_op (convert (inner_op@3 @0 INTEGER_CST@1)) INTEGER_CST@2)
+	   (with
+	{
+	   /* If the inner operation is wrapped inside a conversion, we have
+	  to check it overflows/underflows and can only then perform the
+	  simplification, i.e. add the second constant to the first
+	  (wrapped) and convert afterwards.  */
+	bool inner_ovf = false;
+
+	bool combine_ovf = true;
+	tree cst = NULL_TREE;
+	bool combine_constants = false;
+	bool types_ok = false;
+
+	tree inner_type = TREE_TYPE (@3);
+	/* Check overflow in wrapped binop? */
+	bool check_inner_ovf = !TYPE_OVERFLOW_UNDEFINED (inner_type);
+	bool check_combine_ovf = !TYPE_OVERFLOW_WRAPS (type);
+
+	signop s1 = TYPE_SIGN (type);
+
+	if (TYPE_PRECISION (type) >= TYPE_PRECISION (inner_type))
+	  {
+		types_ok = true;
+
+		/* Check if the inner binop does not overflow i.e. we have VRP
+		   information and can prove prove it.  */
+		if (check_inner_ovf)
+		  {
+		value_range vr = VR_INITIALIZER;
+		if (@0 == NULL_TREE || @1 == NULL_TREE
+			|| inner_type == NULL_TREE
+			|| TREE_CODE (type) != INTEGER_TYPE)
+		  {
+			inner_ovf = true;
+		  }
+		else
+		  

Re: [PATCH] Handle VECTOR_CST in integer_nonzerop()

[Marc Glisse]

On Mon, 19 Sep 2016, Patrick Palka wrote:


On Wed, Sep 14, 2016 at 1:58 AM, Marc Glisse  wrote:

On Fri, 19 Aug 2016, Patrick Palka wrote:


On Fri, Aug 19, 2016 at 7:30 PM, Patrick Palka 
wrote:


integer_nonzerop() currently unconditionally returns false for a
VECTOR_CST argument.  This is confusing because one would expect that
integer_onep(x) => integer_nonzerop(x) for all x but that is currently
not the case.  For a VECTOR_CST of all ones i.e. {1,1,1,1},
integer_onep() returns true but integer_nonzerop() returns false.

This patch makes integer_nonzerop() handle VECTOR_CSTs in the obvious
way and also adds some self tests (the last of which fails without the
change).  Does this look OK to commit afetr bootstrap + regtesting on
x86_64-pc-linux-gnu?



Actually I guess there is some ambiguity as to whether
integer_nonzerop() should return true for a VECTOR_CST only if it has
at least one non-zero element or only if all of its elements are
non-zero...



In my opinion, the second one would make a lot more sense. I think most
(all?) current uses are protected by checks that mean it is never called on
vectors (where did you notice this issue?). But if we wanted to generalize,


Yeah I don't think integer_onep is called anywhere on a VECTOR_CST
currently. I ran into this issue because I tried using integer_onep on
a VECTOR_CST and found that it didn't work as expected.


I thought we were talking about integer_nonzerop? integer_onep is used on 
vectors just fine...



looking for instance at fold-const.c (A & C) == D where D & ~C != 0, we
would want that no element of D&~C is 0 to be able to simplify the whole
vector to 0. If we want an OR as in your patch, in most cases we could use
!integer_zerop, possibly with extra checks that it is indeed a constant.
Maybe we could solve this with more explicit names? integer_each_nonzerop vs
integer_not_all_zerop?


More precise naming would be nice.  Note that integer_all_onesp() and
integer_truep() already exist which is another reason I leaned towards
making integer_onep behave as an OR for VECTOR_CSTs since the AND
behavior is already covered by those aforementioned two predicates.
But since there is no consensus and no such user of integer_onep I
will just drop this patch.


--
Marc Glisse

Re: [accaf, fortran, 4/4] Allocatable components support in derived typed coarrays

[Andreas Schwab]

On Sep 19 2016, Andre Vehreschild  wrote:

> Index: gcc/testsuite/gfortran.dg/coarray_allocate_7.f08
> ===
> --- gcc/testsuite/gfortran.dg/coarray_allocate_7.f08  (nicht existent)
> +++ gcc/testsuite/gfortran.dg/coarray_allocate_7.f08  (Arbeitskopie)
> @@ -0,0 +1,27 @@
> +! { dg-do run }
> +! { dg-options "-fcoarray=lib -lcaf_single -fdump-tree-original" }
> +
> +! Contributed by Damian Rouson
> +! Checking whether (de-)registering of coarrays works.
> +
> +program main
> +
> +  implicit none
> +
> +  type mytype
> +integer, allocatable :: indices(:)
> +  end type
> +
> +  type(mytype), save :: object[*]
> +  integer :: i,me
> +
> +  me=this_image() ! me is always 1 here
> +  object%indices=[(i,i=1,me)]
> +  if ( size(object%indices) /= 1 ) call abort()
> +  ! therefore no array is present here and no array test needed.
> +  if ( object%indices(1) /= 1 ) call abort()
> +end program
> +
> +! { dg-final { scan-tree-dump-times "_gfortran_caf_register \\(D.\[0-9\]{4}, 
> 1, &\\(\\(struct mytype\\) \\*object\\).indices.token, &\\(\\(struct 
> mytype\\) \\*object\\).indices, 0B, 0B, 0\\);" 2 "original" } }
> +! { dg-final { scan-tree-dump-times "_gfortran_caf_deregister 
> \\(&\\(\\(struct mytype\\) \\*object\\).indices.token, 0B, 0B, 0\\);" 1 
> "original" } }
> +

FAIL: gfortran.dg/coarray_allocate_7.f08   -O0   scan-tree-dump-times original 
"_gfortran_caf_register \\(D.[0-9]{4}, 1, &\\(\\(struct mytype\\) 
\\*object\\).indices.token, &\\(\\(struct mytype\\) \\*object\\).indices, 0B, 
0B, 0\\);" 2
PASS: gfortran.dg/coarray_allocate_7.f08   -O0   scan-tree-dump-times original 
"_gfortran_caf_deregister \\(&\\(\\(struct mytype\\) 
\\*object\\).indices.token, 0B, 0B, 0\\);" 1

$ grep _gfortran_caf_register coarray_allocate_7.f08.003t.original 
  _gfortran_caf_register (28, 0, (void * *) _token.0, (void *) , 0B, 
0B, 0);
_gfortran_caf_register (D.986, 1, &((struct mytype) 
*object).indices.token, &((struct mytype) *object).indices, 0B, 0B, 0);
_gfortran_caf_register (D.986, 1, &((struct mytype) 
*object).indices.token, &((struct mytype) *object).indices, 0B, 0B, 0);

Andreas.

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

[PATCH] Fix PR77646

[Richard Biener]

This fixes the PR.

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

Richard.

2016-09-20  Richard Biener  

PR tree-optimization/77646
* tree-ssa-sccvn.c (visit_reference_op_call): Always value-number
a VDEF.

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

Index: gcc/tree-ssa-sccvn.c
===
*** gcc/tree-ssa-sccvn.c(revision 240255)
--- gcc/tree-ssa-sccvn.c(working copy)
*** visit_reference_op_call (tree lhs, gcall
*** 3470,3475 
--- 3577,3586 
  {
if (vnresult->result_vdef && vdef)
changed |= set_ssa_val_to (vdef, vnresult->result_vdef);
+   else if (vdef)
+   /* If the call was discovered to be pure or const reflect
+  that as far as possible.  */
+   changed |= set_ssa_val_to (vdef, vuse_ssa_val (gimple_vuse (stmt)));
  
if (!vnresult->result && lhs)
vnresult->result = lhs;
Index: gcc/testsuite/gcc.dg/torture/pr77646.c
===
*** gcc/testsuite/gcc.dg/torture/pr77646.c  (revision 0)
--- gcc/testsuite/gcc.dg/torture/pr77646.c  (working copy)
***
*** 0 
--- 1,21 
+ /* { dg-do compile } */
+ 
+ struct e {
+ int (*f)();
+ void (*g)();
+ } * c;
+ int a;
+ void *h();
+ typedef struct { struct e j; } k;
+ int l() { return a; }
+ const struct e b = {l};
+ void m()
+ {
+   k *d = h();
+   d->j = b;
+   c = (struct e *)d;
+   struct e *i = c;
+   if (i->f(c))
+ while (i->f(c))
+   i->g();
+ }

Re: [PATCH, 5.x/6.x/7.x] Be more conservative in early inliner if FDO is enabled

[Richard Biener]

On Tue, Sep 20, 2016 at 1:57 PM, Yuan, Pengfei  wrote:
>> > Btw, It occurs to me that then win in code-size might be purely due to the
>> > smaller base value for the TU size we use to compute the maximum unit
>> > growth with ... any idea how to improve it on this side?  Say, computing
>> > the TU size before early optimization (uh, probably not ...)
>> >
>> > That said, the inliner always completely fills its budged, that is, 
>> > increase
>> > the unit by max-unit-growth?
>>
>> What I'm trying to say is that rather than limiting early inlining we should
>> maybe decrease inline-unit-growth when FDO is in effect?  Because we
>> can better control where the inlining goes.  If there is over 8% reduction
>> in size benchmarking (unpatched) compiler on Firefox with FDO and
>> --param inline-unit-growth=12 might show if the results are the same.
>>
>> Richard.
>>
>> > Richard.
>
> AFAIK, param inline-unit-growth only affects pass_ipa_inline. However, the 
> code
> size reduction achieved with my patch is from pass_early_inline.

Any inlining you don't do at early inlining time will a) decrease the
size inline-unit-growth
is computed on b) just delays inlining to IPA inlining (with the now
more constrained size budget).

Richard.

> Yuan, Pengfei

Re: PR35503 - warn for restrict pointer

[Prathamesh Kulkarni]

On 20 September 2016 at 08:57, Martin Sebor  wrote:
>> and used
>> pp_format for formatting arg_positions by adding specifier %I (name
>> chosen arbitrarily).
>> Does that look OK ?
>
>
> diff --git a/gcc/pretty-print.c b/gcc/pretty-print.c
> index a39815e..e8bd1ef 100644
> --- a/gcc/pretty-print.c
> +++ b/gcc/pretty-print.c
> @@ -610,6 +610,23 @@ pp_format (pretty_printer *pp, text_info *text)
>   (pp, *text->args_ptr, precision, unsigned, "u");
>   break;
>
> +   case 'I':
>
> The comment just above pp_format that lists the format specifiers
> understood by the function should be updated.  There probably (I
> hope) is more formal documentation of the format specifiers
> somewhere else that should be updated as well.
>
> That said, since this specifier formats a vec, it seems that
> it might be useful to be able to format vectors of other elements,
> such as short and long.  With that in mind, would adding a new V
> length modifier instead be a more regular way to extend the pretty
> printer to these types?  The V length modifier would have to be
> accepted in conjunction with other length modifiers (h, l, etc
> and type specifiers (d, i, o, etc.).  E.g, "%hVu" would accept
> a vec*  as an argument and format it as a sequence
> of decimal numbers, and "%lVx" would do the same for vec*
> formatting them in hex.
Hi,
Thanks everyone for the suggestions.

This patch does the following:
a) adds warning_at_rich_loc_n(). Somehow I missed David's clear explanation of
warning_at_rich_loc_n earlier :(

b) clears TREE_VISITED flag in C/C++ FE after we are done with
warn_for_restrict().
I assumed that TREE_VISITED would be treated as a "scratch" flag and any pass
wishing to use it must set it first, so didn't bother clearing at first.

c) It appears '%I' is already used for some format specifier in
c-family/c-format.c ?
I changed name to %Z instead. Martin suggested a better idea above to
implement %Z as a length modifier so we can extend it to print vec of other
types, like %hZx would print vec formatted in hex.
I am not sure though if it would be a simple fix, and left it as a FIXME
in the patch adding %Z just to print vec, maybe we could revisit it later ?
Could someone please take a look at the change to c-format.c, I am not sure
if I have added that correctly.

Thanks,
Prathamesh
>
> Martin
>
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index fc25686..742f06d 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -47,6 +47,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "gimplify.h"
 #include "substring-locations.h"
 #include "spellcheck.h"
+#include "gcc-rich-location.h"
 
 cpp_reader *parse_in;		/* Declared in c-pragma.h.  */
 
@@ -13084,4 +13085,53 @@ diagnose_mismatched_attributes (tree olddecl, tree newdecl)
   return warned;
 }
 
+/* Warn if an argument at position param_pos is passed to a
+   restrict-qualified param, and it aliases with another argument.  */
+
+void
+warn_for_restrict (unsigned param_pos, vec *args)
+{
+  tree arg = (*args)[param_pos];
+  if (TREE_VISITED (arg) || operand_equal_p (arg, null_pointer_node, 0))
+return;
+
+  location_t loc = EXPR_LOC_OR_LOC (arg, input_location);
+  gcc_rich_location richloc (loc);
+
+  unsigned i;
+  tree current_arg;
+  auto_vec arg_positions;
+
+  FOR_EACH_VEC_ELT (*args, i, current_arg) 
+{
+  if (i == param_pos)
+	continue;
+
+  tree current_arg = (*args)[i];
+  if (operand_equal_p (arg, current_arg, 0))
+	{
+	  TREE_VISITED (current_arg) = 1; 
+	  arg_positions.safe_push (i + 1); 
+	}
+}
+
+  if (arg_positions.is_empty ())
+return;
+
+  int pos;
+  FOR_EACH_VEC_ELT (arg_positions, i, pos)
+{
+  tree arg = (*args)[pos - 1];
+  if (EXPR_HAS_LOCATION (arg))
+	richloc.add_range (EXPR_LOCATION (arg), false);
+}
+
+  warning_at_rich_loc_n (, OPT_Wrestrict, arg_positions.length (),
+			 "passing argument %i to restrict-qualified parameter"
+			 " aliases with argument %Z",
+			 "passing argument %i to restrict-qualified parameter"
+			 " aliases with arguments %Z",
+			 param_pos + 1, _positions); 
+}
+
 #include "gt-c-family-c-common.h"
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 5bbf951..f29ea1f 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -922,6 +922,7 @@ extern void c_parse_final_cleanups (void);
 
 extern void warn_for_omitted_condop (location_t, tree);
 extern void warn_for_memset (location_t, tree, tree, int);
+extern void warn_for_restrict (unsigned, vec *);
 
 /* These macros provide convenient access to the various _STMT nodes.  */
 
diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c
index bf39ee0..b27d34f 100644
--- a/gcc/c-family/c-format.c
+++ b/gcc/c-family/c-format.c
@@ -713,6 +713,7 @@ static const format_char_info gcc_tdiag_char_table[] =
   { "r",   1, STD_C89, { T89_C,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  

Re: [PATCH] Optimise the fpclassify builtin to perform integer operations when possible

[Tamar Christina]

On 16/09/16 20:49, Jeff Law wrote:

On 09/12/2016 10:19 AM, Tamar Christina wrote:

Hi All,
+
+  /* Re-interpret the float as an unsigned integer type
+ with equal precision.  */
+  int_arg_type = build_nonstandard_integer_type (TYPE_PRECISION 
(type), 0);

+  int_arg = fold_build1_loc (loc, INDIRECT_REF, int_arg_type,
+  fold_build1_loc (loc, NOP_EXPR,
+   build_pointer_type (int_arg_type),
+fold_build1_loc (loc, ADDR_EXPR,
+ build_pointer_type (type), arg)));
Doesn't this make ARG addressable?  Which in turn means ARG won't be 
exposed to the gimple/ssa optimizers.Or is it the case that when 
fpclassify is used its argument is already in memory (and thus 
addressable?)


I believe that it is the case that when fpclassify is use the argument 
is already addressable, but I am not 100% certain. I may be able to do 
this differently so I'll

come back to you on this one.

+ exp, const1));
+
+  /* Combine the values together.  */
+  specials = fold_build3_loc (loc, COND_EXPR, int_type, 
zero_check, fp_zero,

+   fold_build3_loc (loc, COND_EXPR, int_type, exp_lsb_set,
+fold_build3_loc (loc, COND_EXPR, int_type, 
mantissa_any_set,

+  HONOR_NANS (mode) ? fp_nan : fp_normal,
+  HONOR_INFINITIES (mode) ? fp_infinite : fp_normal),
+fp_subnormal));
So this implies you're running on generic, not gimple, right? 
Otherwise you can't generate these kinds of expressions.




Yes this is generic.


diff --git a/gcc/real.h b/gcc/real.h
index 
59af580e78f2637be84f71b98b45ec6611053222..36ded57cf4db7c30c935bdb24219a167480f39c8 
100644

--- a/gcc/real.h
+++ b/gcc/real.h
@@ -161,6 +161,15 @@ struct real_format
   bool has_signed_zero;
   bool qnan_msb_set;
   bool canonical_nan_lsbs_set;
+
+  /* This flag indicates whether the format can be used in the 
optimized

+ code paths for the __builtin_fpclassify function and friends.
+ The format has to have the same NaN and INF representation as 
normal
+ IEEE floats (e.g. exp must have all bits set), most significant 
bit must be
+ sign bit, followed by exp bits of at most 32 bits.  Lastly the 
floating
+ point number must be representable as an integer.  The base of 
the number

+ also must be base 2.  */
+  bool is_binary_ieee_compatible;
   const char *name;
 };
I think Joseph has already commented on the contents of the 
initializer and a few more cases were we can use the optimized paths.


However, I do have a general question.  There are some targets which 
have FPUs that are basically IEEE, but don't support certain IEEE 
features like NaNs, denorms, etc.


Presumably all that's needed is for those targets to define a hook to 
describe which checks will always be false and you can check the 
hook's return value.  Right?


Yes, that should be enough. Not supporting NAN and Infinities is already 
supported though, but it's tied to the real format rather than a 
particular target.


Can you please include some tests to verify you're getting the initial 
code generation you want?  Ideally there'd be execution tests too 
where you generate one of the special nodes, then call the __builtin 
and verify that you get the expected results back. The latter in 
particular are key since it'll allow us to catch problems much earlier 
across the wide variety of targets GCC supports.


I can add some code generation tests. There are I believe already some 
execution tests, which test both correct and incorrect output.


I think you already had plans to post an updated patch.  Please 
include the fixes noted above in that update.


Yes I will include your feedback in it. I'm currently waiting for some 
extra performance numbers.


Thanks,
Tamar

Re: [PATCH, 5.x/6.x/7.x] Be more conservative in early inliner if FDO is enabled

[Yuan, Pengfei]

> > Btw, It occurs to me that then win in code-size might be purely due to the
> > smaller base value for the TU size we use to compute the maximum unit
> > growth with ... any idea how to improve it on this side?  Say, computing
> > the TU size before early optimization (uh, probably not ...)
> >
> > That said, the inliner always completely fills its budged, that is, increase
> > the unit by max-unit-growth?
> 
> What I'm trying to say is that rather than limiting early inlining we should
> maybe decrease inline-unit-growth when FDO is in effect?  Because we
> can better control where the inlining goes.  If there is over 8% reduction
> in size benchmarking (unpatched) compiler on Firefox with FDO and
> --param inline-unit-growth=12 might show if the results are the same.
> 
> Richard.
> 
> > Richard.

AFAIK, param inline-unit-growth only affects pass_ipa_inline. However, the code
size reduction achieved with my patch is from pass_early_inline.

Yuan, Pengfei

Re: [PR72835] Incorrect arithmetic optimization involving bitfield arguments

[Richard Biener]

On Tue, Sep 20, 2016 at 1:32 AM, kugan
 wrote:
> Hi Richard,
> Thanks for the review.
>
>
> On 19/09/16 23:40, Richard Biener wrote:
>>
>> On Sun, Sep 18, 2016 at 10:21 PM, kugan
>>  wrote:
>>>
>>> Hi Richard,
>>>
>>>
>>> On 14/09/16 21:31, Richard Biener wrote:


 On Fri, Sep 2, 2016 at 10:09 AM, Kugan Vivekanandarajah
  wrote:
>
>
> Hi Richard,
>
> On 25 August 2016 at 22:24, Richard Biener 
> wrote:
>>
>>
>> On Thu, Aug 11, 2016 at 1:09 AM, kugan
>>  wrote:
>>>
>>>
>>> Hi,
>>>
>>>
>>> On 10/08/16 20:28, Richard Biener wrote:



 On Wed, Aug 10, 2016 at 10:57 AM, Jakub Jelinek 
 wrote:
>
>
>
> On Wed, Aug 10, 2016 at 08:51:32AM +1000, kugan wrote:
>>
>>
>>
>> I see it now. The problem is we are just looking at (-1) being in
>> the
>> ops
>> list for passing changed to rewrite_expr_tree in the case of
>> multiplication
>> by negate.  If we have combined (-1), as in the testcase, we will
>> not
>> have
>> the (-1) and will pass changed=false to rewrite_expr_tree.
>>
>> We should set changed based on what happens in
>> try_special_add_to_ops.
>> Attached patch does this. Bootstrap and regression testing are
>> ongoing.
>> Is
>> this OK for trunk if there is no regression.
>
>
>
>
> I think the bug is elsewhere.  In particular in
> undistribute_ops_list/zero_one_operation/decrement_power.
> All those look problematic in this regard, they change RHS of
> statements
> to something that holds a different value, while keeping the LHS.
> So, generally you should instead just add a new stmt next to the
> old
> one,
> and adjust data structures (replace the old SSA_NAME in some ->op
> with
> the new one).  decrement_power might be a problem here, dunno if
> all
> the
> builtins are const in all cases that DSE would kill the old one,
> Richard, any preferences for that?  reset flow sensitive info +
> reset
> debug
> stmt uses, or something different?  Though, replacing the LHS with
> a
> new
> anonymous SSA_NAME might be needed too, in case it is before
> SSA_NAME
> of
> a
> user var that doesn't yet have any debug stmts.




 I'd say replacing the LHS is the way to go, with calling the
 appropriate
 helper
 on the old stmt to generate a debug stmt for it / its uses (would
 need
 to look it
 up here).

>>>
>>> Here is an attempt to fix it. The problem arises when in
>>> undistribute_ops_list, we linearize_expr_tree such that NEGATE_EXPR
>>> is
>>> added
>>> (-1) MULT_EXPR (OP). Real problem starts when we handle this in
>>> zero_one_operation. Unlike what was done earlier, we now change the
>>> stmt
>>> (with propagate_op_to_signle use or by directly) such that the value
>>> computed by stmt is no longer what it used to be. Because of this,
>>> what
>>> is
>>> computed in undistribute_ops_list and rewrite_expr_tree are also
>>> changed.
>>>
>>> undistribute_ops_list already expects this but rewrite_expr_tree will
>>> not if
>>> we dont pass the changed as an argument.
>>>
>>> The way I am fixing this now is, in linearize_expr_tree, I set
>>> ops_changed
>>> to true if we change NEGATE_EXPR to (-1) MULT_EXPR (OP). Then when we
>>> call
>>> zero_one_operation with ops_changed = true, I replace all the LHS in
>>> zero_one_operation with the new SSA and replace all the uses. I also
>>> call
>>> the rewrite_expr_tree with changed = false in this case.
>>>
>>> Does this make sense? Bootstrapped and regression tested for
>>> x86_64-linux-gnu without any new regressions.
>>
>>
>>
>> I don't think this solves the issue.  zero_one_operation associates
>> the
>> chain starting at the first *def and it will change the intermediate
>> values
>> of _all_ of the stmts visited until the operation to be removed is
>> found.
>> Note that this is independent of whether try_special_add_to_ops did
>> anything.
>>
>> Even for the regular undistribution cases we get this wrong.
>>
>> So we need to back-track in zero_one_operation, replacing each LHS
>> and in the end the op in the opvector of the main chain.  That's
>> basically
>> the same as if 

Re: [PATCH, 5.x/6.x/7.x] Be more conservative in early inliner if FDO is enabled

[Richard Biener]

On Tue, Sep 20, 2016 at 1:31 PM, Richard Biener
 wrote:
> On Fri, Sep 16, 2016 at 2:00 PM, Jan Hubicka  wrote:
>>> > > I also like a new param better as it avoids a new magic constant and
>>> > > makes playing with
>>> > > it easier (your patch removes the ability to do statistics like you did 
>>> > > via the
>>> > > early-inlining-insns parameter by forcing it to two).
>>> >
>>> > Hmm, you are right that you do not know if this particular function will 
>>> > get
>>> > profile (forgot about that).  Still, please use two params - it is more
>>> > consistent with what we have now and we may make it profile specific in
>>> > future..
>>> >
>>> > Honza
>>> > >
>>> > > Thanks,
>>> > > Richard.
>>>
>>> A new patch for trunk is attached.
>>>
>>> Regards,
>>> Yuan, Pengfei
>>>
>>>
>>> 2016-09-16  Yuan Pengfei  
>>>
>>>   * doc/invoke.texi (--param early-inlining-insns-feedback): New.
>>>   * ipa-inline.c (want_early_inline_function_p): Use
>>>   PARAM_EARLY_INLINING_INSNS_FEEDBACK when FDO is enabled.
>>>   * params.def (PARAM_EARLY_INLINING_INSNS_FEEDBACK): Define.
>>>   (PARAM_EARLY_INLINING_INSNS): Change help string accordingly.
>
> Btw, It occurs to me that then win in code-size might be purely due to the
> smaller base value for the TU size we use to compute the maximum unit
> growth with ... any idea how to improve it on this side?  Say, computing
> the TU size before early optimization (uh, probably not ...)
>
> That said, the inliner always completely fills its budged, that is, increase
> the unit by max-unit-growth?

What I'm trying to say is that rather than limiting early inlining we should
maybe decrease inline-unit-growth when FDO is in effect?  Because we
can better control where the inlining goes.  If there is over 8% reduction
in size benchmarking (unpatched) compiler on Firefox with FDO and
--param inline-unit-growth=12 might show if the results are the same.

Richard.

> Richard.
>
>> OK,
>> thanks
>>
>> Honza