[Patch, avr] Provide correct LD offset bound in avr_address_cost

[Senthil Kumar Selvaraj]

Hi,

  This patch fixes cost computation in avr_address_cost - instead of the
  hardcoded 61, it uses the already existing MAX_LD_OFFSET(mode) macro.

  This showed up when investigating a code size regression in the ivopts
  pass. That pass computes address_cost with and without an offset to
  decide on the right induction variable candidate(s). The legitimate
  address target hook returns false for offsets more than 63, so the
  pass calls the TARGET_ADDRESS_COST hook with 62 as the offset.

  The hook implementation returns 18, and the ivopts pass concludes that
  the cost of address with *any* offset is 18, which is not true - the higher
  cost is incurred only with offsets bigger than MAX_LD_OFFSET. This in
  turn results in a suboptimal choice of induction variables in the
  ivopts pass. The patch changes the hardcoded 61 to use the mode
  specific MAX_LD_OFFSET instead.

  Regression testing with just that fix showed one additional
  compilation timeout. That turned out to be the same as
  https://lists.nongnu.org/archive/html/avr-gcc-list/2014-03/msg00010.html
  - the middle end takes too much time to decide on the best strategy to
  multiply DImode values on a 64 bit host. This already causes timeouts
  for a few builtin-arith-overflow-* tests (see
  https://gcc.gnu.org/ml/gcc-testresults/2016-09/msg02018.html), so it
  isn't really related to this fix. Just providing a cost estimate for
  DImode mul fixes the timeout though, so the patch does that by scaling
  SImode costs by 2 for DImode muls.

  With both changes in, there are no regressions, and the
  builtin-arith-overflow-* tests now PASS and don't timeout.

  Ok to commit to trunk and backport to 5.x?

Regards
Senthil
   

gcc/ChangeLog:

2016-09-21  Senthil Kumar Selvaraj  

* config/avr/avr.c (avr_rtx_costs_1): Handle DImode MULT.
(avr_address_cost): Replace 61 with MAX_LD_OFFSET(mode).



diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c
index 148a61d..29f0022 100644
--- gcc/config/avr/avr.c
+++ gcc/config/avr/avr.c
@@ -10257,6 +10257,7 @@ avr_rtx_costs_1 (rtx x, machine_mode mode, int 
outer_code ATTRIBUTE_UNUSED,
   break;
 
case SImode:
+   case DImode:
  if (AVR_HAVE_MUL)
 {
   if (!speed)
@@ -10282,7 +10283,10 @@ avr_rtx_costs_1 (rtx x, machine_mode mode, int 
outer_code ATTRIBUTE_UNUSED,
 *total = COSTS_N_INSNS (AVR_HAVE_JMP_CALL ? 5 : 4);
 }
 
-  return true;
+  if (mode == DImode)
+*total *= 2;
+
+  return true;
 
default:
  return false;
@@ -10863,7 +10867,7 @@ avr_address_cost (rtx x, machine_mode mode 
ATTRIBUTE_UNUSED,
   && (REG_P (XEXP (x, 0))
   || GET_CODE (XEXP (x, 0)) == SUBREG))
 {
-  if (INTVAL (XEXP (x, 1)) >= 61)
+  if (INTVAL (XEXP (x, 1)) > MAX_LD_OFFSET(mode))
 cost = 18;
 }
   else if (CONSTANT_ADDRESS_P (x))

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

[Tim Shen]

On Wed, Sep 21, 2016 at 1:52 AM, Jonathan Wakely wrote:
> THanks, OK for trunk.

Committed.

Thanks!


-- 
Regards,
Tim Shen

Re: [PATCH] Fix various minor gimple-ssa-sprintf.c issues

[Martin Sebor]

On 09/21/2016 09:09 AM, Jakub Jelinek wrote:

Hi!

When looking at PR77676, I've noticed various small formatting etc.
issues, like not using is_gimple_* APIs where we have them, not using
gimple_call_builtin_p/gimple_call_fndecl (this one actually can show up,
if e.g. uses the builtin with incorrect arguments (fewer, different
types etc.)), one pasto, 2 spaces in comments instead of 1 in the middle
of sentences.  And, lastly 0 < var is very unusual ordering of the
comparison operands, while we have a couple of such cases in the sources,
usually it is when using 0 < var && var <= someotherconst, while
var > 0 is used hundred times more often.


Thanks for correcting the uses of the gimple APIs!  I appreciate
your fixing the various typos as well, but I see no value in
changing the order of operands in inequality expressions or in
moving code around for no apparent reason.  However, I won't
object if you feel strongly about committing those changes(*).

What I would be even more grateful for is a review of the error
prone parts like those that caused the bootstrap failure.  I.e.,
any lingering assumptions about integer sizes between the host
and the target, and the code that controls the return value
range optimization.  (Similar to what you just did for the
PR77676 patch -- that was very helpful, thanks again.)

I'll look into the outstanding issues you noted below next.

Thanks
Martin

PS [*] I think it's unproductive to dwell on these details when
there are much bigger problems to worry about.  IME, insisting
on perfect formatting or on a particular order of operands only
takes time and energy away from focusing on the parts that could
actually cause serious issues.



I see various other issues, which I'll happily defer to Martin, e.g.
  /* These are available as macros in the C and C++ front ends but,
 sadly, not here.  */
  static tree intmax_type_node;
  static tree uintmax_type_node;

  /* Initialize the intmax nodes above the first time through here.  */
  if (!intmax_type_node)
build_intmax_type_nodes (_type_node, _type_node);
This is a big no no, the trees aren't registered with GC this way, so
they could be garbage collected (you'd need to move those to file-scope to
be able to GC register them).
Or
/* Compute the maximum just once.  */
static const int a_max[] = {
  format_floating_max (double_type_node, 'a'),
  format_floating_max (long_double_type_node, 'a')
};
etc. (used several time), do we really need to mess up with the
(thread-safe) local static locking/guards etc.?  Can't you initialize
the cache say to -1 and compute first if you need it and it is still -1?
PR77676 shows various other issues in the pass.

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

2016-09-21  Jakub Jelinek  

* gimple-ssa-sprintf.c: Fix comment formatting.
(pass_sprintf_length::gate): Use x > 0 instead of 0 < x.
(format_integer): Use is_gimple_assign.
(format_floating, format_string, format_directive,
get_destination_size): Use x > 0 instead of 0 < x.
(pass_sprintf_length::handle_gimple_call): Likewise.  Use
gimple_call_builtin_p and gimple_call_fndecl.  Reorder
case BUILT_IN_SPRINTF_CHK.  Fix up BUILT_IN_SNPRINTF_CHK comment.
Replace "to to" with "to" in comment.
(pass_sprintf_length::execute): Use is_gimple_call.

--- gcc/gimple-ssa-sprintf.c.jj 2016-09-21 08:54:15.0 +0200
+++ gcc/gimple-ssa-sprintf.c2016-09-21 15:09:02.209261013 +0200
@@ -37,7 +37,7 @@ along with GCC; see the file COPYING3.

The pass handles all forms standard sprintf format directives,
including character, integer, floating point, pointer, and strings,
-   with  the standard C flags, widths, and precisions.  For integers
+   with the standard C flags, widths, and precisions.  For integers
and strings it computes the length of output itself.  For floating
point it uses MPFR to fornmat known constants with up and down
rounding and uses the resulting range of output lengths.  For
@@ -130,8 +130,8 @@ pass_sprintf_length::gate (function *)
  not optimizing and the pass is being invoked early, or when
  optimizing and the pass is being invoked during optimization
  (i.e., "late").  */
-  return ((0 < warn_format_length || flag_printf_return_value)
- && (0 < optimize) == fold_return_value);
+  return ((warn_format_length > 0 || flag_printf_return_value)
+ && (optimize > 0) == fold_return_value);
 }

 /* The result of a call to a formatted function.  */
@@ -464,7 +464,7 @@ struct conversion_spec

   /* Format conversion function that given a conversion specification
  and an argument returns the formatting result.  */
-  fmtresult  (*fmtfunc) (const conversion_spec &, tree);
+  fmtresult (*fmtfunc) (const conversion_spec &, tree);

   /* Return True when a the format flag CHR has been used.  */
   bool get_flag (char chr) const
@@ 

[PATCH] Introduce selftest::locate_file

[David Malcolm]

On Mon, 2016-09-19 at 11:31 -0600, Jeff Law wrote:
> On 09/16/2016 03:19 PM, David Malcolm wrote:
> > 
> > > When possible I don't think we want the tests to be target
> > > specific.
> > > Hmm, I'm probably about to argue for Bernd's work...  The 71779
> > > testcase
> > > is a great example -- it uses high/lo_sum.  Not all targets
> > > support
> > > that
> > > -- as long as we don't try to recognize those insns we're likely
> > > OK,
> > > but
> > > that seems fragile long term.  Having an idealized target means
> > > we
> > > can
> > > ignore much of these issues.
> > 
> > An alternative would be to pick a specific target for each test.
> It's an alternative, but not one I particularly like since those
> tests
> won't be consistently run.  With an abstracted target like Bernd
> suggests we ought to be able to make most tests work with the
> abstracted
> target and minimize the number of truely target specific tests.
> 
> 
> > > So I'm real curious, what happens if you run this RTL selftest
> > > under
> > > valgrind?  I have the sneaking suspicion that we'll start doing
> > > some
> > > uninitialized memory reads.
> > 
> > I'm seeing various leaks (an htab within linemap_init for all of
> > the
> > line_table fixtures), but no uninitialized reads.
> Wow.  I must say I'm surprised.  Good news though.
> 
> 
> > >   +
> > > > +  /* Dump taken from comment 2 of PR 71779, of
> > > > + "...the relevant memory access coming out of expand"
> > > > + with basic block IDs added, and prev/next insns set to
> > > > + 0 at ends.  */
> > > > +  const char *input_dump
> > > > += (";; MEM[(struct isl_obj *)] =
> > > > _obj_map_vtable;\n"
> > > > +   "(insn 1045 0 1046 2 (set (reg:SI 480)\n"
> > > > +   "(high:SI (symbol_ref:SI
> > > > (\"isl_obj_map_vtable\")
> > > > [flags 0xc0] )))
> > > > y.c:12702 -1\n"
> > > > +   " (nil))\n"
> > > > +   "(insn 1046 1045 1047 2 (set (reg/f:SI 479)\n"
> > > > +   "(lo_sum:SI (reg:SI 480)\n"
> > > > +   "(symbol_ref:SI (\"isl_obj_map_vtable\")
> > > > [flags
> > > > 0xc0] ))) y.c:12702
> > > > -1\n"
> > > > +   " (expr_list:REG_EQUAL (symbol_ref:SI
> > > > (\"isl_obj_map_vtable\") [flags 0xc0]  > > > isl_obj_map_vtable>)\n"
> > > > +   "(nil)))\n"
> > > > +   "(insn 1047 1046 1048 2 (set (reg:DI 481)\n"
> > > > +   "(subreg:DI (reg/f:SI 479) 0)) y.c:12702 -1\n"
> > > > +   " (nil))\n"
> > > > +   "(insn 1048 1047 1049 2 (set (zero_extract:DI (reg/v:DI
> > > > 191
> > > > [ obj1D.17368 ])\n"
> > > > +   "(const_int 32 [0x20])\n"
> > > > +   "(const_int 0 [0]))\n"
> > > > +   "(reg:DI 481)) y.c:12702 -1\n"
> > > > +   " (nil))\n"
> > > So looking at this just makes my head hurt.  Escaping, lots of
> > > quotes,
> > > unnecessary things in the dump, etc.  The question I would have
> > > is
> > > why
> > > not have a file with the dump and read the file?
> > 
> > (nods)
> > 
> > Seems like I need to add a mechanism for telling the selftests
> > which
> > directory to load the tests relative to.
> What about putting them inside the appropriate gcc.target
> directories?
> We could have one for the "generic" target assuming we build
> something
> like that, the others could live in their target specific directory.
> 
> 
> jeff

Having selftests that read RTL dumps load them from files rather than
embedding them as strings in the source requires a way to locate the
relevant files.

This patch adds a selftest::locate_file function for locating such
files, relative to "$(SRCDIR)/gcc/testsuite/selftests".  This is
done via a new argument to -fself-test, which supplies the current
value of "$(SRCDIR)/gcc" to cc1.

I chose "$(SRCDIR)/gcc/testsuite/selftests", so as to be below
gcc/testsuite, but not below any of the existing DejaGnu subdirectories,
to avoid selftest-specific files from being picked up by .exp globbing
patterns.  We could add target-specific directories below that dir if
necessary.

I've rewritten the rest of the patch kit to use this to load from .rtl
dump files within that directory, rather than embedding the dumps as
string literals in the C source.

The patch also exposes a selftests::path_to_src_gcc, which could be
used by a selftest to e.g. load a DejaGnu file, so that if need be
we could share .rtl input files between both -fself-test tests and
DejaGnu-based tests for the .rtl frontend.

(Depends on the approved-when-needed
  "[PATCH 2/9] Add selftest::read_file"
https://gcc.gnu.org/ml/gcc-patches/2016-09/msg00476.html ).

Successfully bootstrapped on x86_64-pc-linux-gnu.

OK for trunk once the dependencies are in?

gcc/ChangeLog:
* Makefile.in (s-selftest) Add $(srcdir) as an argument of
-fself-test.
(selftest-gdb): Likewise.
(selftest-valgrind): Likewise.
* common.opt (fself-test): Rename to...
(fself-test=): ...this, documenting the meaning of the 

Re: [PATCH] fix ILP32 bootstrap failures due to -Wformat-length

[Martin Sebor]

On 09/21/2016 05:09 PM, Jakub Jelinek wrote:

On Wed, Sep 21, 2016 at 04:39:42PM -0600, Martin Sebor wrote:

diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index dddb026..652d3fb 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -210,8 +210,8 @@ struct format_result
 static HOST_WIDE_INT
 target_int_min ()
 {
-  static const unsigned HOST_WIDE_INT int_min
-= 1LLU << (sizeof int_min * CHAR_BIT
+  const unsigned HOST_WIDE_INT int_min
+= 1LLU << (HOST_BITS_PER_WIDE_INT


1LLU should be really HOST_WIDE_INT_1U


   - TYPE_PRECISION (integer_type_node) + 1);


Is the shift amount really what you mean?
HOST_BITS_PER_WIDE_INT - TYPE_PRECISION (integer_type_node) + 1
is usually 33 (or 17 or 9 in much rarer cases), so that is
0x2ULL.  Don't you want instead
  = HOST_WIDE_INT_1U << (TYPE_PRECISION (ingeger_type_node) - 1);
so that it will be
0x8000ULL?


Great catch, thank you!  That must have been a copy and paste mistake.




@@ -221,8 +221,8 @@ target_int_min ()
 static unsigned HOST_WIDE_INT
 target_int_max ()
 {
-  static const unsigned HOST_WIDE_INT int_max
-= HOST_WIDE_INT_M1U >> (sizeof int_max * CHAR_BIT
+  const unsigned HOST_WIDE_INT int_max
+= HOST_WIDE_INT_M1U >> (HOST_BITS_PER_WIDE_INT
- TYPE_PRECISION (integer_type_node) + 1);
   return int_max;
 }


This is expectedly -1ULL >> 33, i.e. 0x7fffULL, which looks ok.


Thanks for double checking that.  There is a test case for this
in the test suite for the optimization but there wasn't one for
the INT_MIN case.  I've added it in the attached patch.  I'll
beef up the return range testing some more before re-enabling
the optimization.

Martin
PR target/77676 - powerpc64 and powerpc64le stage2 bootstrap fail

gcc/testsuite/ChangeLog:
2016-09-21  Martin Sebor  

	PR target/77676
	* gcc.dg/tree-ssa/builtin-sprintf-2.c: Fix typo.
	* gcc.dg/tree-ssa/builtin-sprintf-3.c: New test.
	* gcc.dg/tree-ssa/builtin-sprintf-warn-5.c: New test.

gcc/ChangeLog:
2016-09-21  Martin Sebor  

	PR target/77676
	* gimple-ssa-sprintf.c (target_int_min, target_int_max): Use
	HOST_BITS_PER_WIDE_INT, make a static local variable auto.
	(target_int_min): Correct computation.
	(format_integer): Use long long as the argument for the ll length
	modifier.
	(format_floating): Use target_int_max().
	(get_string_length): Same.
	(format_string): Avoid setting the bounded flag for strings
	of unknown length.
	(try_substitute_return_value): Avoid setting range info when
	the result isn't bounded.
	* varasm.c (assemble_name): Increase buffer size.

diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index dddb026..d1d20ca 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -210,9 +210,9 @@ struct format_result
 static HOST_WIDE_INT
 target_int_min ()
 {
-  static const unsigned HOST_WIDE_INT int_min
-= 1LLU << (sizeof int_min * CHAR_BIT
-	   - TYPE_PRECISION (integer_type_node) + 1);
+  const unsigned HOST_WIDE_INT int_min
+= HOST_WIDE_INT_M1U << (TYPE_PRECISION (integer_type_node) - 1);
+
   return int_min;
 }
 
@@ -221,8 +221,8 @@ target_int_min ()
 static unsigned HOST_WIDE_INT
 target_int_max ()
 {
-  static const unsigned HOST_WIDE_INT int_max
-= HOST_WIDE_INT_M1U >> (sizeof int_max * CHAR_BIT
+  const unsigned HOST_WIDE_INT int_max
+= HOST_WIDE_INT_M1U >> (HOST_BITS_PER_WIDE_INT
 			- TYPE_PRECISION (integer_type_node) + 1);
   return int_max;
 }
@@ -851,7 +851,9 @@ format_integer (const conversion_spec , tree arg)
 
 case FMT_LEN_L:
 case FMT_LEN_ll:
-  dirtype = sign ? long_integer_type_node : long_unsigned_type_node;
+  dirtype = (sign
+		 ? long_long_integer_type_node
+		 : long_long_unsigned_type_node);
   break;
 
 case FMT_LEN_z:
@@ -1366,7 +1368,7 @@ format_floating (const conversion_spec , tree arg)
 	  *minmax[i] = mpfr_snprintf (NULL, 0, fmtstr, mpfrval);
 	}
 
-  res.bounded = res.range.min < HOST_WIDE_INT_MAX;
+  res.bounded = res.range.min < target_int_max ();
   return res;
 }
 
@@ -1420,7 +1422,7 @@ get_string_length (tree str)
   /* Set RES.BOUNDED to true if and only if all strings referenced
 	 by STR are known to be bounded (though not necessarily by their
 	 actual length but perhaps by their maximum possible length).  */
-  res.bounded = res.range.max < HOST_WIDE_INT_MAX;
+  res.bounded = res.range.max < target_int_max ();
 
   /* Set RES.CONSTANT to false even though that may be overly
 	 conservative in rare cases like: 'x ? a : b' where a and
@@ -1471,6 +1473,10 @@ format_string (const conversion_spec , tree arg)
: 2 == warn_format_length ? 0 <= prec ? prec : 1
: HOST_WIDE_INT_MAX);
 
+  /* The result is bounded unless overriddden for a non-constant string
+ of an unknown length.  */
+  bool bounded = true;
+
   if (spec.specifier == 'c')
 {
   if (spec.modifier == FMT_LEN_l)

Re: [PATCH] fix ILP32 bootstrap failures due to -Wformat-length

[Jakub Jelinek]

On Wed, Sep 21, 2016 at 04:39:42PM -0600, Martin Sebor wrote:
> diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
> index dddb026..652d3fb 100644
> --- a/gcc/gimple-ssa-sprintf.c
> +++ b/gcc/gimple-ssa-sprintf.c
> @@ -210,8 +210,8 @@ struct format_result
>  static HOST_WIDE_INT
>  target_int_min ()
>  {
> -  static const unsigned HOST_WIDE_INT int_min
> -= 1LLU << (sizeof int_min * CHAR_BIT
> +  const unsigned HOST_WIDE_INT int_min
> += 1LLU << (HOST_BITS_PER_WIDE_INT

1LLU should be really HOST_WIDE_INT_1U

>  - TYPE_PRECISION (integer_type_node) + 1);

Is the shift amount really what you mean?
HOST_BITS_PER_WIDE_INT - TYPE_PRECISION (integer_type_node) + 1
is usually 33 (or 17 or 9 in much rarer cases), so that is
0x2ULL.  Don't you want instead
  = HOST_WIDE_INT_1U << (TYPE_PRECISION (ingeger_type_node) - 1);
so that it will be
0x8000ULL?

> @@ -221,8 +221,8 @@ target_int_min ()
>  static unsigned HOST_WIDE_INT
>  target_int_max ()
>  {
> -  static const unsigned HOST_WIDE_INT int_max
> -= HOST_WIDE_INT_M1U >> (sizeof int_max * CHAR_BIT
> +  const unsigned HOST_WIDE_INT int_max
> += HOST_WIDE_INT_M1U >> (HOST_BITS_PER_WIDE_INT
>   - TYPE_PRECISION (integer_type_node) + 1);
>return int_max;
>  }

This is expectedly -1ULL >> 33, i.e. 0x7fffULL, which looks ok.

Jakub

[PATCH] fix ILP32 bootstrap failures due to -Wformat-length

[Martin Sebor]

As mentioned in PR77676. the gimple-ssa-sprintf.c pass committed
yesterday caused a number of bootstrap failures, including on ILP32
targets.  All of them were due to differences between integer sizes
between the host and the target.  To unblock the powerpc64 bootstrap
failure reported in the PR I temporarily disabled the
-fprintf-return-value optimization this morning.

The attached patch fixes the underlying problems.  I've retested
it on i386 (ILP32), x86_64, and powerpc64le, the latter with the
optimization enabled (even though the patch doesn't enable it).

I'd like to commit this patch first to unblock the ILP32 targets,
then re-enable the optimization in a subsequent commit, and then
look into fixing the outstanding minor issues in the test suite
on ILP32 (there's just one failure in my i368 test run) and some
of those pointed out by Jakub this morning in

  https://gcc.gnu.org/ml/gcc-patches/2016-09/msg01436.html

Thanks
Martin

PR target/77676 - powerpc64 and powerpc64le stage2 bootstrap fail

gcc/testsuite/ChangeLog:
2016-09-21  Martin Sebor  

	PR target/77676
	* gcc.dg/tree-ssa/builtin-sprintf-2.c: Fix typo.
	* gcc.dg/tree-ssa/builtin-sprintf-3.c: New test.
	* gcc.dg/tree-ssa/builtin-sprintf-warn-5.c: New test.

gcc/ChangeLog:
2016-09-21  Martin Sebor  

	PR target/77676
	* gimple-ssa-sprintf.c (target_int_min, target_int_max): Use
	HOST_BITS_PER_WIDE_INT, make a static local variable auto.
	(format_integer): Use long long as the argument for the ll length
	modifier.
	(format_floating): Use target_int_max().
	(get_string_length): Same.
	(format_string): Avoid setting the bounded flag for strings
	of unknown length.
	(try_substitute_return_value): Avoid setting range info when
	the result isn't bounded.
	* varasm.c (assemble_name): Increase buffer size.

diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index dddb026..652d3fb 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -210,8 +210,8 @@ struct format_result
 static HOST_WIDE_INT
 target_int_min ()
 {
-  static const unsigned HOST_WIDE_INT int_min
-= 1LLU << (sizeof int_min * CHAR_BIT
+  const unsigned HOST_WIDE_INT int_min
+= 1LLU << (HOST_BITS_PER_WIDE_INT
 	   - TYPE_PRECISION (integer_type_node) + 1);
   return int_min;
 }
@@ -221,8 +221,8 @@ target_int_min ()
 static unsigned HOST_WIDE_INT
 target_int_max ()
 {
-  static const unsigned HOST_WIDE_INT int_max
-= HOST_WIDE_INT_M1U >> (sizeof int_max * CHAR_BIT
+  const unsigned HOST_WIDE_INT int_max
+= HOST_WIDE_INT_M1U >> (HOST_BITS_PER_WIDE_INT
 			- TYPE_PRECISION (integer_type_node) + 1);
   return int_max;
 }
@@ -851,7 +851,9 @@ format_integer (const conversion_spec , tree arg)
 
 case FMT_LEN_L:
 case FMT_LEN_ll:
-  dirtype = sign ? long_integer_type_node : long_unsigned_type_node;
+  dirtype = (sign
+		 ? long_long_integer_type_node
+		 : long_long_unsigned_type_node);
   break;
 
 case FMT_LEN_z:
@@ -1366,7 +1368,7 @@ format_floating (const conversion_spec , tree arg)
 	  *minmax[i] = mpfr_snprintf (NULL, 0, fmtstr, mpfrval);
 	}
 
-  res.bounded = res.range.min < HOST_WIDE_INT_MAX;
+  res.bounded = res.range.min < target_int_max ();
   return res;
 }
 
@@ -1420,7 +1422,7 @@ get_string_length (tree str)
   /* Set RES.BOUNDED to true if and only if all strings referenced
 	 by STR are known to be bounded (though not necessarily by their
 	 actual length but perhaps by their maximum possible length).  */
-  res.bounded = res.range.max < HOST_WIDE_INT_MAX;
+  res.bounded = res.range.max < target_int_max ();
 
   /* Set RES.CONSTANT to false even though that may be overly
 	 conservative in rare cases like: 'x ? a : b' where a and
@@ -1471,6 +1473,10 @@ format_string (const conversion_spec , tree arg)
: 2 == warn_format_length ? 0 <= prec ? prec : 1
: HOST_WIDE_INT_MAX);
 
+  /* The result is bounded unless overriddden for a non-constant string
+ of an unknown length.  */
+  bool bounded = true;
+
   if (spec.specifier == 'c')
 {
   if (spec.modifier == FMT_LEN_l)
@@ -1550,16 +1556,17 @@ format_string (const conversion_spec , tree arg)
 	  if (0 <= prec)
 	{
 	  if ((unsigned)prec < slen.range.min
-		  || slen.range.min >= HOST_WIDE_INT_MAX)
+		  || slen.range.min >= target_int_max ())
 		slen.range.min = prec;
 	  if ((unsigned)prec < slen.range.max
-		  || slen.range.max >= HOST_WIDE_INT_MAX)
+		  || slen.range.max >= target_int_max ())
 		slen.range.max = prec;
 	}
-	  else if (slen.range.min >= HOST_WIDE_INT_MAX)
+	  else if (slen.range.min >= target_int_max ())
 	{
 	  slen.range.min = max_bytes_for_unknown_str;
 	  slen.range.max = max_bytes_for_unknown_str;
+	  bounded = false;
 	}
 
 	  res.range = slen.range;
@@ -1580,7 +1587,8 @@ format_string (const conversion_spec , tree arg)
 res.range.max = width;
 
   /* Adjust BOUNDED if width happens to make them 

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

[Doug Gilmore]

> From: Richard Biener [rguent...@suse.de]
> Sent: Wednesday, September 21, 2016 12:48 AM
> To: Doug Gilmore
> Cc: gcc-patches@gcc.gnu.org; rgue...@gcc.gnu.org
> Subject: RE: [PATCH] Fix PR tree-optimization/77654
> 
> On Tue, 20 Sep 2016, Doug Gilmore wrote:
> 
> > 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?
> 
> It checks for !ptr_info but it requires NAME to be an SSA name.
> 
> From the attachment in bugzilla (the attachment didn't make it
> here)
> 
> 
> +
> +  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));
> +   }
> +
> 
> I was talking about changing the if to
> 
> if (TREE_CODE (addr_base) == SSA_NAME
> && TREE_CODE (addr) == SSA_NAME)
Sorry I that missed point.  I glossed your comment "addr_base should
always be a pointer", causing me to go off into the weeds.

New patch attached.

Thanks,

Doug
> 
> because the addresses could be invariant as far as I can see.
> 
> > Also does it make sense to make a test case for this?
> 
> I'm not sure how to easily test this.
> 
> Richard.
> 
> ...
From 2d6cb0674ca66b4c5f6e335d73122e03413863e3 Mon Sep 17 00:00:00 2001
From: Doug Gilmore 
Date: Tue, 6 Sep 2016 10:18:42 -0700
Subject: [PATCH] Ensure points-to information is maintained for prefetch.

gcc/
PR tree-optimization/77654
* tree-ssa-alias.c (issue_prefetch_ref): Add call
to duplicate_ssa_name_ptr_info.
---
 gcc/tree-ssa-loop-prefetch.c |   12 
 1 file changed, 12 insertions(+)

diff --git a/gcc/tree-ssa-loop-prefetch.c b/gcc/tree-ssa-loop-prefetch.c
index 26cf0a0..d0bd2d3 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,17 @@ 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 (TREE_CODE (addr_base) == SSA_NAME
+  && TREE_CODE (addr) == SSA_NAME)
+	{
+	  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

Re: [PATCH] Handle VECTOR_CST in integer_nonzerop()

[Patrick Palka]

On Tue, Sep 20, 2016 at 8:20 AM, Marc Glisse  wrote:
> 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...

Oh, yeah... Sorry about the confusion.  Your argument to generalize
integer_nonzerop as an AND on vectors makes sense to me.

>
>
>>> 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: [PATCH] - improve sprintf buffer overflow detection (middle-end/49905)

[Gerald Pfeifer]

On Wed, 21 Sep 2016, Martin Sebor wrote:
>> Is it possible that is related to your warning patches?
> Yes, this is likely the same bug as mentioned in comment #6 on
> pr77676.  The bug in the comment ILP32-specific and only tangentially
> related to the PR itself.  I'm testing the patch that's attached to
> the PR that should fix both of these problems.  I don't have access
> to i?86-unknown-freebsd so if you could help validate it there I'd
> be grateful.

Yes, with this patch I was able to bootstrap i?86-unknown-freebsd11
again.

Thanks for looking into this so quickly, Martin!

Gerald

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

[Jakub Jelinek]

On Wed, Sep 21, 2016 at 01:55:33PM -0600, Martin Sebor wrote:
> On 09/21/2016 01:40 PM, Gerald Pfeifer wrote:
> >I noticed the following bootstrap failure on i?86-unknown-freebsd
> >that started in the last 24 hours:
> >
> >/scratch/tmp/gerald/gcc-HEAD/gcc/vec.c: In member function ‘void 
> >vec_usage::dump(mem_location*, mem_usage&) const’:
> >/scratch/tmp/gerald/gcc-HEAD/gcc/vec.c:79:3: error: ‘%s’ directive writing 
> >between 0 and 4294967295 bytes into a region of size 4096 
> >[-Werror=format-length=]
> >   dump (mem_location *loc, mem_usage ) const
> >   ^~~~
> >/scratch/tmp/gerald/gcc-HEAD/gcc/vec.c:83:36: note: format output between 6 
> >and4294967311 bytes into a destination of size 4096
> >   loc->m_line, loc->m_function);
> >^
> >cc1plus: all warnings being treated as errors
> >gmake[3]: *** [Makefile:2557: build/vec.o] Error 1
> >gmake[3]: Leaving directory '/scratch/tmp/gerald/OBJ-0921-1705/gcc'
> >gmake[2]: *** [Makefile:4612: all-stage2-gcc] Error 2
> >gmake[2]: Leaving directory '/scratch/tmp/gerald/OBJ-0921-1705'
> >gmake[1]: *** [Makefile:24365: stage2-bubble] Error 2
> >
> >Is it possible that is related to your warning patches?
> 
> Yes, this is likely the same bug as mentioned in comment #6 on
> pr77676.  The bug in the comment ILP32-specific and only tangentially
> related to the PR itself.  I'm testing the patch that's attached to
> the PR that should fix both of these problems.  I don't have access
> to i?86-unknown-freebsd so if you could help validate it there I'd
> be grateful.  (The patch just successfully bootstrapped on
> i386-pc-linux-gnu.)

Looking at target_int_max you are using in the new patch:
static unsigned HOST_WIDE_INT
target_int_max ()
{
  static const unsigned HOST_WIDE_INT int_max
= HOST_WIDE_INT_M1U >> (sizeof int_max * CHAR_BIT
- TYPE_PRECISION (integer_type_node) + 1);
  return int_max;
}

1) sizeof int_max * CHAR_BIT should IMNSHO be HOST_BITS_PER_WIDE_INT
2) why is the var static, subtraction and shift is very cheap, while C++
   local statics are expensive?  It needs a guard variable,
   __cxa_guard_acquire, __cxa_guard_release calls, etc.

Jakub

[PATCH] fix ICE on %lf directive in format_floating in gimple-ssa-sprintf.c (middle-end/77683)

[Martin Sebor]

The attached patch corrects the handling of the 'l' length modifier
for floating point directives (such as %lf) where it's specified to
have no effect.  It also replaces the gcc_unreachable() macro with
a return statement telling the pass to give up on unknown length
modifiers (rather than abort).

Martin
PR middle-end/77683 - ICE on %lf directive in format_floating in
  gimple-ssa-sprintf.c:1163

gcc/testsuite/ChangeLog:
2016-09-21  Martin Sebor  

	PR middle-end/77683
	* gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Add test cases.

gcc/ChangeLog:
2016-09-21  Martin Sebor  

	PR middle-end/77683
	* gimple-ssa-sprintf.c (format_integer): Fail gracefully when
	length modifier is not expected.
	(format_floating): Ignore l length modifier and fail gracefuly
	when it isn't one of the other expected ones.


diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index 2e4a89f..34595a5 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -867,7 +867,14 @@ format_integer (const conversion_spec , tree arg)
   break;
 
 default:
-  gcc_unreachable ();
+  {
+	fmtresult res = fmtresult ();
+	res.range.min = HOST_WIDE_INT_MAX;
+	res.range.max = HOST_WIDE_INT_MAX;
+	res.bounded = false;
+	res.constant = false;
+	return res;
+  }
 }
 
   /* The type of the argument to the directive, either deduced from
@@ -1145,6 +1152,7 @@ format_floating (const conversion_spec , int width, int prec)
 
   switch (spec.modifier)
 {
+case FMT_LEN_l:
 case FMT_LEN_none:
   type = double_type_node;
   break;
@@ -1160,7 +1168,14 @@ format_floating (const conversion_spec , int width, int prec)
   break;
 
 default:
-  gcc_unreachable ();
+  {
+	fmtresult res = fmtresult ();
+	res.range.min = HOST_WIDE_INT_MAX;
+	res.range.max = HOST_WIDE_INT_MAX;
+	res.bounded = false;
+	res.constant = false;
+	return res;
+  }
 }
 
   /* The minimum and maximum number of bytes produced by the directive.  */
@@ -1246,7 +1261,14 @@ format_floating (const conversion_spec , int width, int prec)
   }
 
 default:
-  gcc_unreachable ();
+  {
+	fmtresult res = fmtresult ();
+	res.range.min = HOST_WIDE_INT_MAX;
+	res.range.max = HOST_WIDE_INT_MAX;
+	res.bounded = false;
+	res.constant = false;
+	return res;
+  }
 }
 
   if (0 < width)
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
index 7261dbd..214dbdc 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
@@ -863,6 +863,8 @@ void test_sprintf_chk_z_const (void)
 void test_sprintf_chk_e_const (void)
 {
   T (-1, "%E",   0.0);
+  T (-1, "%lE",  0.0);
+
   T ( 0, "%E",   0.0);  /* { dg-warning "into a region" } */
   T ( 0, "%e",   0.0);  /* { dg-warning "into a region" } */
   T ( 1, "%E",   1.0);  /* { dg-warning "into a region" } */
@@ -1073,6 +1075,8 @@ void test_sprintf_chk_int_nonconst (int a)
 void test_sprintf_chk_e_nonconst (double d)
 {
   T (-1, "%E",  d);
+  T (-1, "%lE", d);
+
   T ( 0, "%E",  d);   /* { dg-warning "writing between 12 and 14 bytes into a region of size 0" } */
   T ( 0, "%e",  d);   /* { dg-warning "into a region" } */
   T ( 1, "%E",  d);   /* { dg-warning "into a region" } */
@@ -1104,6 +1108,8 @@ void test_sprintf_chk_e_nonconst (double d)
 void test_sprintf_chk_f_nonconst (double d)
 {
   T (-1, "%F",  d);
+  T (-1, "%lF", d);
+
   T ( 0, "%F",  d);   /* { dg-warning "into a region" } */
   T ( 0, "%f",  d);   /* { dg-warning "into a region" } */
   T ( 1, "%F",  d);   /* { dg-warning "into a region" } */

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

[Jason Merrill]

On Tue, Sep 20, 2016 at 1:01 PM, Martin Sebor  wrote:
> On 09/16/2016 12:19 PM, Jason Merrill wrote:
>> On 09/14/2016 01:03 PM, Martin Sebor wrote:
>>>
>>> +  /* 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]; };

The issue is I don't see anything that uses fldtype as a decl, and
it's strange to have a variable called "*type" that can either be a
type or a decl, which later code still assumes will be a type.

>>> +  /* The context of the flexible array member.  Either the struct
>>> + in which it's declared or, for anonymous structs and unions,
>>> + the struct/union of which the array is effectively a member.  */
>>> +  tree fmemctx = anon_context (fmem->array);
>>> +  bool anon_p = fmemctx != DECL_CONTEXT (fmem->array);
>>> +  if (!anon_p)
>>> +fmemctx = t;
>>
>> Why do you need to do something different here based on anon_p?  I don't
>> see why that would affect whether we want to look through intermediate
>> non-anonymous classes.
>
> In code like this:
>
>struct X { int n, a[]; };
>struct Y { int n, b[]; };
>
>struct D: X, Y { };
>
> The test above make the diagnostic point to the context of the invalid
> flexible array member, or Y::b, rather than that of X. Without it, we
> end up with:
>
>error: flexible array member ‘X::a’ not at end of ‘struct X’
>
> rather than with
>
>error: flexible array member ‘X::a’ not at end of ‘struct D’

Yes, but why does whether we want to talk about X or D change if a is
wrapped in struct { }?

>>> +  check_flexarrays (basetype, fmem, true);
>>
>> Please decorate boolean literal arguments like this with the name of the
>> parameter, e.g. /*base_p*/true.
>
> Sure.  I should mention that Jeff recently advised against it in
> another review, so it would be nice to adopt the same convention
> throughout and document it (I'm happy to add it to the Wiki once
> it has been agreed on):
>
>https://gcc.gnu.org/ml/gcc-patches/2016-08/msg00354.html

Interesting.  It's pretty common in the C++ front end.

> FWIW, I haven't found mentioning the formal argument in a comment
> a useful or consistent convention.  Other arguments' names aren't
> mentioned, and the bool argument's name cannot be mentioned when
> it has a default value.

True, but other arguments usually have more implied meaning and so I
think they are less of a barrier to reading.

> On the other hand, a convention I do find useful (though not one
> that seems to be used in GCC) is indicating in a comment in the
> definition of a function the value of the default argument in
> functions declared to take one.

I agree that would be a good convention.

Jason

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

[Bernd Edlinger]

On 09/21/16 21:57, Christophe Lyon wrote:
> Hi,
>
> The new testcase pr77550.C fails on arm:
> /testsuite/g++.dg/pr77550.C:39:43: error: 'operator new' takes type
> 'size_t' ('unsigned int') as first parameter [-fpermissive]
> compiler exited with status 1
>
> Christophe
>

Yes, I see, thanks.  This should fix it.

OK for trunk, gcc-6 after a while ?


Bernd.
2016-09-21  Bernd Edlinger  

	* g++.dg/pr77550.C: Use __SIZE_TYPE__.

Index: gcc/testsuite/g++.dg/pr77550.C
===
--- gcc/testsuite/g++.dg/pr77550.C	(revision 240330)
+++ gcc/testsuite/g++.dg/pr77550.C	(working copy)
@@ -36,7 +36,7 @@ struct B {
 template  using __ptr_rebind = B;
 template  _Tp max(_Tp p1, _Tp) { return p1; }
 }
-void *operator new(unsigned long, void *p2) { return p2; }
+void *operator new(__SIZE_TYPE__, void *p2) { return p2; }
 template  struct C {
   typedef _Tp *pointer;
   pointer allocate(int p1) {
@@ -47,7 +47,7 @@ template  struct C {
 namespace std {
 template  using __allocator_base = C<_Tp>;
 template  struct allocator : __allocator_base<_Tp> {
-  typedef unsigned long size_type;
+  typedef __SIZE_TYPE__ size_type;
   template  struct rebind { typedef allocator<_Tp1> other; };
 };
 struct D {

Re: [PATCH, v2] Introduce class rtx_reader

[Richard Sandiford]

David Malcolm  writes:
> On Fri, 2016-09-16 at 16:04 -0600, Jeff Law wrote:
>> On 09/08/2016 06:30 PM, David Malcolm wrote:
>> > Bundle up various global variables within gensupport.c into a
>> > class rtx_reader, with a view towards making it easier to run the
>> > code more than once in-process.
>> > 
>> > gcc/ChangeLog:
>> >* genconstants.c (main): Introduce noop_reader and convert call
>> >to read_md_files to a method call.
>> >* genenums.c (main): Likewise.
>> >* genmddeps.c (main): Likewise.
>> >* genpreds.c (write_tm_constrs_h): Replace use of "in_fname"
>> > with
>> >rtx_reader_ptr->get_top_level_filename ().
>> >(write_tm_preds_h): Likewise.
>> >(write_insn_preds_c): Likewise.
>> >* gensupport.c (class gen_reader): New subclass of rtx_reader.
>> >(rtx_handle_directive): Convert to...
>> >(gen_reader::handle_unknown_directive): ...this.
>> >(init_rtx_reader_args_cb): Convert return type from bool to
>> >rtx_reader *.  Create a gen_reader instance, using it for the
>> >call to read_md_files.  Return it if no errors occur.
>> >(init_rtx_reader_args): Convert return type from bool to
>> >rtx_reader *.
>> >* gensupport.h (init_rtx_reader_args_cb): Likewise.
>> >(init_rtx_reader_args_cb): Likewise.
>> >* read-md.c (struct file_name_list): Move to class rtx_reader.
>> >(read_md_file): Delete in favor of rtx_reader::m_read_md_file.
>> >(read_md_filename): Delete in favor of
>> >rtx_reader::m_read_md_filename.
>> >(read_md_lineno): Delete in favor of
>> > rtx_reader::m_read_md_lineno.
>> >(in_fname): Delete in favor of rtx_reader::m_toplevel_fname.
>> >(base_dir): Delete in favor of rtx_reader::m_base_dir.
>> >(first_dir_md_include): Delete in favor of
>> >rtx_reader::m_first_dir_md_include.
>> >(last_dir_md_include_ptr): Delete in favor of
>> >rtx_reader::m_last_dir_md_include_ptr.
>> >(max_include_len): Delete.
>> >(rtx_reader_ptr): New.
>> >(fatal_with_file_and_line): Use get_filename and get_lineno
>> >accessors of rtx_reader_ptr.
>> >(require_char_ws): Likewise.
>> >(rtx_reader::read_char): New method, based on ::read_char.
>> >(rtx_reader::unread_char): New method, based on ::unread_char.
>> >(read_escape): Use get_filename and get_lineno accessors of
>> >rtx_reader_ptr.
>> >(read_braced_string): Use get_lineno accessor of
>> > rtx_reader_ptr.
>> >(read_string): Use get_filename and get_lineno accessors of
>> >rtx_reader_ptr.
>> >(rtx_reader::rtx_reader): New ctor.
>> >(rtx_reader::~rtx_reader): New dtor.
>> >(handle_include): Convert from a function to...
>> >(rtx_reader::handle_include): ...this method, converting
>> >handle_directive from a callback to a virtual function.
>> >(handle_file): Likewise, converting to...
>> >(rtx_reader::handle_file): ...this method.
>> >(handle_toplevel_file): Likewise, converting to...
>> >(rtx_reader::handle_toplevel_file): ...this method.
>> >(rtx_reader::get_current_location): New method.
>> >(parse_include): Convert from a function to...
>> >(rtx_reader::add_include_path): ...this method, dropping
>> > redundant
>> >update to unused max_include_len.
>> >(read_md_files): Convert from a function to...
>> >(rtx_reader::read_md_files): ...this method, converting
>> >handle_directive from a callback to a virtual function.
>> >(noop_reader::handle_unknown_directive): New method.
>> >* read-md.h (directive_handler_t): Delete this typedef.
>> >(in_fname): Delete.
>> >(read_md_file): Delete.
>> >(read_md_lineno): Delete.
>> >(read_md_filename): Delete.
>> >(class rtx_reader): New class.
>> >(rtx_reader_ptr): New decl.
>> >(class noop_reader): New subclass of rtx_reader.
>> >(read_char): Reimplement in terms of rtx_reader::read_char.
>> >(unread_char): Reimplement in terms of rtx_reader::unread_char.
>> >(read_md_files): Delete.
>> >* read-rtl.c (read_rtx_code): Update for deletion of globals
>> >read_md_filename and read_md_lineno.
>> I don't see anything terribly objectionable here.
>> 
>> It looks like you use a singleton to avoid passing the object around
>> everywhere, but given the state of all this prior to this patch, I
>> can
>> live with the cleanup as a whole.
>
> Indeed; the point of the patch is to bundle up some global state to
> better allow the reader to be run more than once in-process, so that
> the RTL frontend can have selftests; having more than one reader *alive*
> at once is out-of-scope.  Hence the use of a singleton, to minimize the
> amount of churn.
>
>> I'll note Richard Sandiford. hasn't chimed in here.  You might ping
>> him
>> directly to see if he's got any feedback.  If he doesn't prior to say
>> Wed, this is OK for the trunk.
>> 
>> jeff
>
> Thanks.
>
> Here's a slightly tweaked version, fixing a couple of missing NULL
> initializations 

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

[Bernd Edlinger]

On 09/21/16 21:03, Jason Merrill wrote:
> On Wed, Sep 21, 2016 at 11:43 AM, Bernd Edlinger
>  wrote:
>> On 09/21/16 17:00, Jason Merrill wrote:
>>> On 09/20/2016 02:40 PM, Bernd Edlinger wrote:
 On 09/20/16 16:51, Jason Merrill wrote:
> On Tue, Sep 20, 2016 at 10:11 AM, Bernd Edlinger
>  wrote:
>> 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.
>
 +  if (warn_int_in_bool_context
 +  && !from_macro_expansion_at (EXPR_LOCATION (expr)))

 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?
>>>
>>> This should fix that, though I haven't run regression tests yet:
>>>
>>
>> Yes.  I think that goes in the right direction,
>> but it does not work yet.
>>
>> #define XXX (a ? 2 : 3)
>>
>> if (XXX) // prints a warning, but it should not.
>
> Indeed, that was too simplistic.  This patch adds a
> from_macro_definition_at test that ought to do what you want:
>

Yes, this works for me as well.

Here is an updated warning patch, using the new
from_macro_definition_at.


Bernd.
2016-09-21  Bernd Edlinger  

	* c-common.c (c_common_truthvalue_conversion): Inhibit
	Wint-in-bool-context warning with from_macro_definition_at.
	Mention the expression will always evaluate to true.

Index: gcc/c-family/c-common.c
===
--- gcc/c-family/c-common.c	(Revision 240313)
+++ gcc/c-family/c-common.c	(Arbeitskopie)
@@ -4652,7 +4652,8 @@ c_common_truthvalue_conversion (location_t locatio
 	   TREE_OPERAND (expr, 0));
 
 case COND_EXPR:
-  if (warn_int_in_bool_context)
+  if (warn_int_in_bool_context
+	  && !from_macro_definition_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 @@ c_common_truthvalue_conversion (location_t locatio
 	  && (!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 ())

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

[Jason Merrill]

On Wed, Sep 21, 2016 at 3:52 PM, Bernd Edlinger
 wrote:
> On 09/21/16 21:03, Jason Merrill wrote:
>> On Wed, Sep 21, 2016 at 11:43 AM, Bernd Edlinger
>>  wrote:
>>> On 09/21/16 17:00, Jason Merrill wrote:
 On 09/20/2016 02:40 PM, Bernd Edlinger wrote:
> On 09/20/16 16:51, Jason Merrill wrote:
>> On Tue, Sep 20, 2016 at 10:11 AM, Bernd Edlinger
>>  wrote:
>>> 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.
>>
> +  if (warn_int_in_bool_context
> +  && !from_macro_expansion_at (EXPR_LOCATION (expr)))
>
> 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?

 This should fix that, though I haven't run regression tests yet:

>>>
>>> Yes.  I think that goes in the right direction,
>>> but it does not work yet.
>>>
>>> #define XXX (a ? 2 : 3)
>>>
>>> if (XXX) // prints a warning, but it should not.
>>
>> Indeed, that was too simplistic.  This patch adds a
>> from_macro_definition_at test that ought to do what you want:
>>
>
> Yes, this works for me as well.
>
> Here is an updated warning patch, using the new
> from_macro_definition_at.

OK.

Jason

Re: C++ PATCH to forbid use of bool with the ++ operator

[Marek Polacek]

On Wed, Sep 21, 2016 at 03:45:55PM -0400, Jason Merrill wrote:
> On Wed, Sep 14, 2016 at 1:38 PM, Jason Merrill  wrote:
> >> * c-c++-common/gomp/atomic-12.c: Use -Wno-deprecated.
> >> * c-c++-common/gomp/atomic-13.c: Likewise.
> >> * c-c++-common/gomp/atomic-14.c: Likewise.
> >> * g++.dg/cpp1y/lambda-init11.C: Remove invalid code.
> >> * g++.dg/cpp1z/bool-increment1.C: New test.
> >> * c-c++-common/pr60439.c: Add dg-warning.
> >> * g++.dg/expr/bitfield4.C: Likewise.
> >> * g++.dg/expr/bitfield5.C: Likewise.
> >> * g++.dg/expr/bitfield6.C: Likewise.
> >> * g++.dg/expr/bool1.C: Likewise.
> >> * g++.dg/expr/bool3.C: Likewise.
> >> * g++.dg/expr/lval3.C: Likewise.
> >> * g++.dg/expr/lval4.C: Likewise.
> >> * g++.old-deja/g++.jason/bool5.C: Likewise.
> >> * g++.dg/expr/bitfield3.C: Adjust dg-error.
> >> * g++.dg/other/error18.C: Likewise.
> >> * g++.dg/gomp/atomic-14.C: Likewise.
> 
> Many of these tests now break with -std=c++1z; try 'make check-c++1z' to see.
 
Sorry.  I'll look into it.

> > FAIL: c-c++-common/gomp/atomic-12.c  -std=gnu++1z (test for excess errors)
> > FAIL: c-c++-common/gomp/atomic-13.c  -std=gnu++1z (test for excess errors)
> > FAIL: c-c++-common/gomp/atomic-14.c  -std=gnu++1z (test for excess errors)
> > FAIL: c-c++-common/pr60439.c  -std=c++1z (test for excess errors)
> > FAIL: c-c++-common/pr60439.c  -std=c++1z  (test for warnings, line 134)
> > FAIL: g++.dg/expr/bitfield4.C  -std=c++1z (test for excess errors)
> > FAIL: g++.dg/expr/bitfield4.C  -std=c++1z  (test for warnings, line 17)
> > FAIL: g++.dg/expr/bitfield4.C  -std=c++1z  (test for warnings, line 18)
> > FAIL: g++.dg/expr/bitfield5.C  -std=c++1z (test for excess errors)
> > FAIL: g++.dg/expr/bitfield5.C  -std=c++1z  (test for warnings, line 11)
> > FAIL: g++.dg/expr/bitfield5.C  -std=c++1z  (test for warnings, line 14)
> > FAIL: g++.dg/expr/bitfield6.C  -std=c++1z (test for excess errors)
> > FAIL: g++.dg/expr/bitfield6.C  -std=c++1z  (test for warnings, line 10)
> > FAIL: g++.dg/expr/bool1.C  -std=c++1z (test for excess errors)
> > FAIL: g++.dg/expr/bool1.C  -std=c++1z  (test for warnings, line 13)
> > FAIL: g++.dg/expr/bool1.C  -std=c++1z  (test for warnings, line 14)
> > FAIL: g++.dg/expr/bool3.C  -std=c++1z (test for excess errors)
> > FAIL: g++.dg/expr/bool3.C  -std=c++1z  (test for warnings, line 13)
> > FAIL: g++.dg/expr/bool3.C  -std=c++1z  (test for warnings, line 14)
> > FAIL: g++.dg/expr/lval3.C  -std=c++1z  (test for warnings, line 6)
> > FAIL: g++.dg/expr/lval4.C  -std=c++1z (test for excess errors)
> > FAIL: g++.dg/expr/lval4.C  -std=c++1z  (test for warnings, line 6)
> > FAIL: g++.old-deja/g++.jason/bool5.C  -std=c++1z (test for excess errors)
> > FAIL: g++.old-deja/g++.jason/bool5.C  -std=c++1z  (test for warnings, line 
> > 5)
> > FAIL: g++.old-deja/g++.jason/bool5.C  -std=c++1z  (test for warnings, line 
> > 8)

Marek

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

[Christophe Lyon]

On 21 September 2016 at 09:16, Richard Biener  wrote:
> On Tue, 20 Sep 2016, Bernd Edlinger wrote:
>
>> 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?
>
> Ok for trunk and gcc-6 branch after a while.
>
> Richard.


Hi,

The new testcase pr77550.C fails on arm:
/testsuite/g++.dg/pr77550.C:39:43: error: 'operator new' takes type
'size_t' ('unsigned int') as first parameter [-fpermissive]
compiler exited with status 1

Christophe

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

[Martin Sebor]

On 09/21/2016 01:40 PM, Gerald Pfeifer wrote:

I noticed the following bootstrap failure on i?86-unknown-freebsd
that started in the last 24 hours:

/scratch/tmp/gerald/gcc-HEAD/gcc/vec.c: In member function ‘void 
vec_usage::dump(mem_location*, mem_usage&) const’:
/scratch/tmp/gerald/gcc-HEAD/gcc/vec.c:79:3: error: ‘%s’ directive writing 
between 0 and 4294967295 bytes into a region of size 4096 
[-Werror=format-length=]
   dump (mem_location *loc, mem_usage ) const
   ^~~~
/scratch/tmp/gerald/gcc-HEAD/gcc/vec.c:83:36: note: format output between 6 
and4294967311 bytes into a destination of size 4096
   loc->m_line, loc->m_function);
^
cc1plus: all warnings being treated as errors
gmake[3]: *** [Makefile:2557: build/vec.o] Error 1
gmake[3]: Leaving directory '/scratch/tmp/gerald/OBJ-0921-1705/gcc'
gmake[2]: *** [Makefile:4612: all-stage2-gcc] Error 2
gmake[2]: Leaving directory '/scratch/tmp/gerald/OBJ-0921-1705'
gmake[1]: *** [Makefile:24365: stage2-bubble] Error 2

Is it possible that is related to your warning patches?


Yes, this is likely the same bug as mentioned in comment #6 on
pr77676.  The bug in the comment ILP32-specific and only tangentially
related to the PR itself.  I'm testing the patch that's attached to
the PR that should fix both of these problems.  I don't have access
to i?86-unknown-freebsd so if you could help validate it there I'd
be grateful.  (The patch just successfully bootstrapped on
i386-pc-linux-gnu.)

Thanks and sorry for the breakage.
Martin

Re: C/C++ PATCH to implement -Wpointer-compare warning (PR c++/64767)

[Jason Merrill]

On Mon, Sep 19, 2016 at 2:49 PM, Jason Merrill  wrote:
> I suppose that an INTEGER_CST of character type is necessarily a
> character constant, so adding a check for !char_type_p ought to do the
> trick.

Indeed it does.  I'm checking this in:
commit d2f237ef0f63b3ee3da79bcbfad08fedb325d554
Author: Jason Merrill 
Date:   Wed Sep 21 10:58:39 2016 -0400

Core 903
* call.c (null_ptr_cst_p): Check char_type_p.

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 393aab9..2804bd8 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -527,6 +527,7 @@ null_ptr_cst_p (tree t)
 {
   /* Core issue 903 says only literal 0 is a null pointer constant.  */
   if (TREE_CODE (type) == INTEGER_TYPE
+	  && !char_type_p (type)
 	  && TREE_CODE (t) == INTEGER_CST
 	  && integer_zerop (t)
 	  && !TREE_OVERFLOW (t))
diff --git a/gcc/testsuite/g++.dg/cpp0x/nullptr36.C b/gcc/testsuite/g++.dg/cpp0x/nullptr36.C
new file mode 100644
index 000..5f43881
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/nullptr36.C
@@ -0,0 +1,3 @@
+// { dg-do compile { target c++11 } }
+
+void *p = '\0';			// { dg-error "invalid conversion" }

Re: C++ PATCH to forbid use of bool with the ++ operator

[Jason Merrill]

On Wed, Sep 14, 2016 at 1:38 PM, Jason Merrill  wrote:
>> * c-c++-common/gomp/atomic-12.c: Use -Wno-deprecated.
>> * c-c++-common/gomp/atomic-13.c: Likewise.
>> * c-c++-common/gomp/atomic-14.c: Likewise.
>> * g++.dg/cpp1y/lambda-init11.C: Remove invalid code.
>> * g++.dg/cpp1z/bool-increment1.C: New test.
>> * c-c++-common/pr60439.c: Add dg-warning.
>> * g++.dg/expr/bitfield4.C: Likewise.
>> * g++.dg/expr/bitfield5.C: Likewise.
>> * g++.dg/expr/bitfield6.C: Likewise.
>> * g++.dg/expr/bool1.C: Likewise.
>> * g++.dg/expr/bool3.C: Likewise.
>> * g++.dg/expr/lval3.C: Likewise.
>> * g++.dg/expr/lval4.C: Likewise.
>> * g++.old-deja/g++.jason/bool5.C: Likewise.
>> * g++.dg/expr/bitfield3.C: Adjust dg-error.
>> * g++.dg/other/error18.C: Likewise.
>> * g++.dg/gomp/atomic-14.C: Likewise.

Many of these tests now break with -std=c++1z; try 'make check-c++1z' to see.

> FAIL: c-c++-common/gomp/atomic-12.c  -std=gnu++1z (test for excess errors)
> FAIL: c-c++-common/gomp/atomic-13.c  -std=gnu++1z (test for excess errors)
> FAIL: c-c++-common/gomp/atomic-14.c  -std=gnu++1z (test for excess errors)
> FAIL: c-c++-common/pr60439.c  -std=c++1z (test for excess errors)
> FAIL: c-c++-common/pr60439.c  -std=c++1z  (test for warnings, line 134)
> FAIL: g++.dg/expr/bitfield4.C  -std=c++1z (test for excess errors)
> FAIL: g++.dg/expr/bitfield4.C  -std=c++1z  (test for warnings, line 17)
> FAIL: g++.dg/expr/bitfield4.C  -std=c++1z  (test for warnings, line 18)
> FAIL: g++.dg/expr/bitfield5.C  -std=c++1z (test for excess errors)
> FAIL: g++.dg/expr/bitfield5.C  -std=c++1z  (test for warnings, line 11)
> FAIL: g++.dg/expr/bitfield5.C  -std=c++1z  (test for warnings, line 14)
> FAIL: g++.dg/expr/bitfield6.C  -std=c++1z (test for excess errors)
> FAIL: g++.dg/expr/bitfield6.C  -std=c++1z  (test for warnings, line 10)
> FAIL: g++.dg/expr/bool1.C  -std=c++1z (test for excess errors)
> FAIL: g++.dg/expr/bool1.C  -std=c++1z  (test for warnings, line 13)
> FAIL: g++.dg/expr/bool1.C  -std=c++1z  (test for warnings, line 14)
> FAIL: g++.dg/expr/bool3.C  -std=c++1z (test for excess errors)
> FAIL: g++.dg/expr/bool3.C  -std=c++1z  (test for warnings, line 13)
> FAIL: g++.dg/expr/bool3.C  -std=c++1z  (test for warnings, line 14)
> FAIL: g++.dg/expr/lval3.C  -std=c++1z  (test for warnings, line 6)
> FAIL: g++.dg/expr/lval4.C  -std=c++1z (test for excess errors)
> FAIL: g++.dg/expr/lval4.C  -std=c++1z  (test for warnings, line 6)
> FAIL: g++.old-deja/g++.jason/bool5.C  -std=c++1z (test for excess errors)
> FAIL: g++.old-deja/g++.jason/bool5.C  -std=c++1z  (test for warnings, line 5)
> FAIL: g++.old-deja/g++.jason/bool5.C  -std=c++1z  (test for warnings, line 8)

Re: [v3 PATCH] PR libstdc++/77288 and the newest proposed resolution for LWG 2756

[Ville Voutilainen]

On 21 September 2016 at 12:31, Jonathan Wakely  wrote:
> On 06/09/16 09:00 +0300, Ville Voutilainen wrote:
>>
>>PR libstdc++/77288
>>* include/std/optional (__is_optional_impl, __is_optional): Remove.
>>(__converts_from_optional, __assigns_from_optional): New.
>>(optional(_Up&&)): Use is_same instead of __is_optional.
>>(optional(const optional<_Up>&)): Constrain with
>>__converts_from_optional.
>>(optional(optional<_Up>&&)): Likewise.
>>(operator=(_Up&&)): Use is_same instead of __is_optional, check
>>is_same and is_scalar.
>>(operator=(const optional<_Up>&)): Constrain with
>>__converts_from_optional and __assigns_from_optional.
>>(operator=(optional<_Up>&&)): Likewise.
>>* testsuite/20_util/optional/77288.cc: New.
>>* testsuite/20_util/optional/cons/value.cc: Adjust.
>
>
> OK for trunk, thanks.

Thanks, applied - I will cook up a separate patch for making the same
fix for experimental::optional.
The plan is to then backport that one to the gcc-6 branch.

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

[Gerald Pfeifer]

I noticed the following bootstrap failure on i?86-unknown-freebsd
that started in the last 24 hours:

/scratch/tmp/gerald/gcc-HEAD/gcc/vec.c: In member function ‘void 
vec_usage::dump(mem_location*, mem_usage&) const’:
/scratch/tmp/gerald/gcc-HEAD/gcc/vec.c:79:3: error: ‘%s’ directive writing 
between 0 and 4294967295 bytes into a region of size 4096 
[-Werror=format-length=]
   dump (mem_location *loc, mem_usage ) const
   ^~~~
/scratch/tmp/gerald/gcc-HEAD/gcc/vec.c:83:36: note: format output between 6 
and4294967311 bytes into a destination of size 4096
   loc->m_line, loc->m_function);
^
cc1plus: all warnings being treated as errors
gmake[3]: *** [Makefile:2557: build/vec.o] Error 1
gmake[3]: Leaving directory '/scratch/tmp/gerald/OBJ-0921-1705/gcc'
gmake[2]: *** [Makefile:4612: all-stage2-gcc] Error 2
gmake[2]: Leaving directory '/scratch/tmp/gerald/OBJ-0921-1705'
gmake[1]: *** [Makefile:24365: stage2-bubble] Error 2

Is it possible that is related to your warning patches?

Gerald

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

[Bernd Edlinger]

On 09/21/16 21:03, Jason Merrill wrote:
> On Wed, Sep 21, 2016 at 11:43 AM, Bernd Edlinger
>  wrote:
>> Yes.  I think that goes in the right direction,
>> but it does not work yet.
>>
>> #define XXX (a ? 2 : 3)
>>
>> if (XXX) // prints a warning, but it should not.
>
> Indeed, that was too simplistic.  This patch adds a
> from_macro_definition_at test that ought to do what you want:
>

I tried something similar, which seems to work:

--- input.h (Revision 240313)
+++ input.h (Arbeitskopie)
@@ -73,10 +73,19 @@
 header, but expanded in a non-system file.  */
  #define in_system_header_at(LOC) \
(linemap_location_in_system_header_p (line_table, LOC))
-/* Return a positive value if LOCATION is the locus of a token that
-   comes from a macro expansion, O otherwise.  */
-#define from_macro_expansion_at(LOC) \
-  ((linemap_location_from_macro_expansion_p (line_table, LOC)))
+/* Return TRUE if LOCATION is the locus of a token that
+   comes from a macro expansion, FALSE otherwise.  */
+static inline bool
+from_macro_expansion_at (location_t loc)
+{
+  /* Resolve to the spelling location so we return false for arguments to a
+ macro.  */
+  return linemap_location_from_macro_expansion_p (line_table, loc)
+&& linemap_resolve_location (line_table, loc,
+ LRK_MACRO_DEFINITION_LOCATION, NULL)
+   == linemap_resolve_location (line_table, loc,
+LRK_SPELLING_LOCATION, NULL);
+}

  static inline location_t
  get_pure_location (location_t loc)


I will try your patch now, and see if it works for me.


Thanks
Bernd.

Re: Implement -Wimplicit-fallthrough (version 9)

[Jason Merrill]

On 09/21/2016 02:59 PM, Marek Polacek wrote:

+  if (statement == NULL_TREE
+  && attr != NULL_TREE
+  && maybe_attribute_fallthrough_p (attr))
+{
+  /* Turn [[fallthrough]]; into FALLTHROUGH ();.  */
+  statement = build_call_expr_internal_loc (loc, IFN_FALLTHROUGH,
+   void_type_node, 0);
+  attr = NULL_TREE;
+}
+
+  /* Allow "[[fallthrough]];", but warn otherwise.  */
+  if (attr != NULL_TREE)
+warning_at (loc, OPT_Wattributes,
+   "attributes at the beginning of statement are ignored");


This is very close, thanks.  Let's give a more helpful warning about

[[fallthrough]] 0;
__attribute__ ((fallthrough)) 0;

both here and in cp_parser_statement, something like "fallthrough 
attribute not followed by ';'"


Jason

Re: Implement -Wimplicit-fallthrough (version 9)

[Marek Polacek]

On Wed, Sep 21, 2016 at 10:06:07AM -0400, Jason Merrill wrote:
> On 09/21/2016 08:44 AM, Marek Polacek wrote:
> > @@ -10733,12 +10758,35 @@ cp_parser_expression_statement (cp_parser* 
> > parser, tree in_statement_expr)
> >   statement.  */
> >if (cp_lexer_next_token_is_not (parser->lexer, CPP_SEMICOLON))
> >  {
> > +  /* This might be attribute fallthrough.  */
> > +  if (cp_lexer_next_token_is_keyword (parser->lexer, RID_ATTRIBUTE))
> > {
> > + tree attr = cp_parser_gnu_attributes_opt (parser);
> > + if (maybe_attribute_fallthrough_p (attr))
> > +   statement = build_call_expr_internal_loc (token->location,
> > + IFN_FALLTHROUGH,
> > + void_type_node, 0);
> > + else if (cp_lexer_next_token_is (parser->lexer, CPP_SEMICOLON))
> > +   {
> > + cp_parser_error (parser, "only attribute % "
> > +  "can be applied to a null statement");
> > + return error_mark_node;
> > +   }
> > + else
> > +   {
> > + cp_parser_error (parser, "expected primary-expression");
> > + return error_mark_node;
> > +   }
> > +   }
> 
> Attributes that we don't handle should be warnings, not errors.  Like we do
> for C++11 attributes in cp_parser_statement, we should first parse
> attributes, then parse the expression-statement, then give any appropriate
> warnings.

Ok, I hope this is at least somewhat closer to what you imagine.  I had to
adjust g++.dg/warn/Wunused-label-1.C test because of this.  I also turned
errors for e.g. __attribute__((used)); into warnings in the C FE.

So hopefully one step closer.

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

2016-09-21  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_expression_statement): Handle attribute
fallthrough.
(cp_parser_statement): Likewise.
(cp_parser_label_for_labeled_statement): Set FALLTHROUGH_LABEL_P on
labels.
(cp_parser_std_attribute): Handle fallthrough attribute.
(cp_parser_check_std_attribute): Add %< %> quotes.
* 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.
* 

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

[Segher Boessenkool]

On Tue, Sep 20, 2016 at 07:38:43PM -0400, Michael Meissner wrote:
> 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.

Yes, please do, to make sure you didn't accidentally break something else.

> Is the patch ok to put into the trunk?


> +(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"
> +  "#"
> +  ""

"&& 1" here?

Okay with that change.  Thanks,


Segher

[PATCH] print-rtx.c: add 'h', v' and 'p' prefixes to regnos

[David Malcolm]

On Tue, 2016-09-20 at 15:12 -0400, David Malcolm wrote:
> 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)

(Replying to myself, in the hope of better demonstrating the idea)

The following patch implements this idea for RTL dumps, so that all REGNO
values in dumps get a one character prefix: 'h' for hard registers, 'v'
for virtual registers, and 'p' for non-virtual pseudos (making it easier
for both humans and parsers to grok the meaning of a REGNO).

For example, on compiling this:

  int test (int i)
  {
return i * i;
  }

for target==x86_64, the dump for expand looks like this:

(note 1 0 4 NOTE_INSN_DELETED)
(note 4 1 2 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
(insn 2 4 3 2 (set (mem/c:SI (plus:DI (reg/f:DI v82 virtual-stack-vars)
(const_int -4 [0xfffc])) [1 i+0 S4 A32])
(reg:SI h5 di [ i ])) ../../test.c:2 -1
 (nil))
(note 3 2 6 2 NOTE_INSN_FUNCTION_BEG)
(insn 6 3 7 2 (set (reg:SI p89)
(mem/c:SI (plus:DI (reg/f:DI v82 virtual-stack-vars)
(const_int -4 [0xfffc])) [1 i+0 S4 A32])) 
../../test.c:3 -1
 (nil))
(insn 7 6 10 2 (parallel [
(set (reg:SI p87 [ _2 ])
(mult:SI (reg:SI p89)
(mem/c:SI (plus:DI (reg/f:DI v82 virtual-stack-vars)
(const_int -4 [0xfffc])) [1 i+0 S4 
A32])))
(clobber (reg:CC h17 flags))
]) ../../test.c:3 -1
 (nil))
(insn 10 7 14 2 (set (reg:SI p88 [  ])
(reg:SI p87 [ _2 ])) ../../test.c:3 -1
 (nil))
(insn 14 10 15 2 (set (reg/i:SI h0 ax)
(reg:SI p88 [  ])) ../../test.c:4 -1
 (nil))
(insn 15 14 0 2 (use (reg/i:SI h0 ax)) ../../test.c:4 -1
 (nil))

Note the 'v' prefix here:
  (reg/f:DI v82 virtual-stack-vars)
the 'h' prefix here:
  (reg/i:SI h0 ax)
and the 'p' prefix here:
  (reg:SI p88 [  ])

Successfully bootstrapped on x86_64-pc-linux-gnu.
There are various regression test failures involving scan-rtl-dump
due to regexes not matching e.g.
 PASS -> FAIL : gcc.target/i386/pr20020-1.c scan-rtl-dump expand "\\(set 
\\(reg/i:TI 0 ax\\)"
 PASS -> FAIL : gcc.target/i386/pr20020-1.c scan-rtl-dump expand "\\(set 
\\(reg:TI [0-9]* \\[  \\]\\)"
If the approach is OK, I can do an updated patch that also fixes up the
relevant tests (adding the appropriate prefixes); this would touch
multiple targets.

gcc/ChangeLog:
* print-rtl.c (print_rtx): When dumping regnos, prefix the
number with 'h', 'v' or 'p', depending on whether it is a
hard register, a virtual register, or a non-virtual pseudo.
* doc/rtl.texi (Registers and Memory): Document the above change,
adding the prefix to examples that specifically refer to hard or
pseudo regs in the text.
---
 gcc/doc/rtl.texi | 26 

Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)

[Alessandro Fanfarillo]

Thanks Andre.

2016-09-19 9:55 GMT-06:00 Andre Vehreschild :
> Hi Alessandro,

> The if in resolve.c at 8837: resolve_failed_image (... is intentional? It is
> doing nothing. So do you plan to add more code, or will there never be
> anything. If the later I recommend to just put a comment there and remove the
> empty if.

I added the if statement during the development and I forgot to remove it.

>
> There still is no test when -fcoarray=single is used. This shouldn't be so
> hard, should it?

Done.

Built and regtested on x86_64-pc-linux-gnu.

>
> Regards,
> Andre
>
> On Mon, 19 Sep 2016 08:30:12 -0700
> Alessandro Fanfarillo  wrote:
>
>> * PING *
>>
>> On Sep 7, 2016 3:01 PM, "Alessandro Fanfarillo" 
>> wrote:
>>
>> > Dear all,
>> > the attached patch supports failed images also when -fcoarray=single is
>> > used.
>> >
>> > Built and regtested on x86_64-pc-linux-gnu.
>> >
>> > Cheers,
>> > Alessandro
>> >
>> > 2016-08-09 5:22 GMT-06:00 Paul Richard Thomas <
>> > paul.richard.tho...@gmail.com>:
>> > > Hi Sandro,
>> > >
>> > > As far as I can see, this is OK barring a couple of minor wrinkles and
>> > > a question:
>> > >
>> > > For coarray_failed_images_err.f90 and coarray_image_status_err.f90 you
>> > > have used the option -fdump-tree-original without making use of the
>> > > tree dump.
>> > >
>> > > Mikael asked you to provide an executable test with -fcoarray=single.
>> > > Is this not possible for some reason?
>> > >
>> > > Otherwise, this is OK for trunk.
>> > >
>> > > Thanks for the patch.
>> > >
>> > > Paul
>> > >
>> > > On 4 August 2016 at 05:07, Alessandro Fanfarillo
>> > >  wrote:
>> > >> * PING *
>> > >>
>> > >> 2016-07-21 13:05 GMT-06:00 Alessandro Fanfarillo <
>> > fanfarillo@gmail.com>:
>> > >>> Dear Mikael and all,
>> > >>>
>> > >>> in attachment the new patch, built and regtested on
>> > x86_64-pc-linux-gnu.
>> > >>>
>> > >>> Cheers,
>> > >>> Alessandro
>> > >>>
>> > >>> 2016-07-20 13:17 GMT-06:00 Mikael Morin :
>> >  Le 20/07/2016 à 11:39, Andre Vehreschild a écrit :
>> > >
>> > > Hi Mikael,
>> > >
>> > >
>> > >>> +  if(st == ST_FAIL_IMAGE)
>> > >>> +new_st.op = EXEC_FAIL_IMAGE;
>> > >>> +  else
>> > >>> +gcc_unreachable();
>> > >>
>> > >> You can use
>> > >> gcc_assert (st == ST_FAIL_IMAGE);
>> > >> foo...;
>> > >> instead of
>> > >> if (st == ST_FAIL_IMAGE)
>> > >> foo...;
>> > >> else
>> > >> gcc_unreachable ();
>> > >
>> > >
>> > > Be careful, this is not 100% identical in the general case. For older
>> > > gcc version (gcc < 4008) gcc_assert() is mapped to nothing, esp. not
>> > to
>> > > an abort(), so the behavior can change. But in this case everything
>> > is
>> > > fine, because the patch is most likely not backported.
>> > >
>> >  Didn't know about this. The difference seems to be very subtle.
>> >  I don't mind much anyway. The original version can stay if preferred,
>> > this
>> >  was just a suggestion.
>> > 
>> >  By the way, if the function is inlined in its single caller, the
>> > assert or
>> >  unreachable statement can be removed, which avoids choosing between
>> > them.
>> >  That's another suggestion.
>> > 
>> > 
>> > >>> +
>> > >>> +  return MATCH_YES;
>> > >>> +
>> > >>> + syntax:
>> > >>> +  gfc_syntax_error (st);
>> > >>> +
>> > >>> +  return MATCH_ERROR;
>> > >>> +}
>> > >>> +
>> > >>> +match
>> > >>> +gfc_match_fail_image (void)
>> > >>> +{
>> > >>> +  /* if (!gfc_notify_std (GFC_STD_F2008_TS, "FAIL IMAGE statement
>> > >>> at %C")) */
>> > >>> +  /*   return MATCH_ERROR; */
>> > >>> +
>> > >>
>> > >> Can this be uncommented?
>> > >>
>> > >>> +  return fail_image_statement (ST_FAIL_IMAGE);
>> > >>> +}
>> > >>>
>> > >>>  /* Match LOCK/UNLOCK statement. Syntax:
>> > >>>   LOCK ( lock-variable [ , lock-stat-list ] )
>> > >>> diff --git a/gcc/fortran/trans-intrinsic.c
>> > >>> b/gcc/fortran/trans-intrinsic.c index 1aaf4e2..b2f5596 100644
>> > >>> --- a/gcc/fortran/trans-intrinsic.c
>> > >>> +++ b/gcc/fortran/trans-intrinsic.c
>> > >>> @@ -1647,6 +1647,24 @@ trans_this_image (gfc_se * se, gfc_expr
>> > >>> *expr) m, lbound));
>> > >>>  }
>> > >>>
>> > >>> +static void
>> > >>> +gfc_conv_intrinsic_image_status (gfc_se *se, gfc_expr *expr)
>> > >>> +{
>> > >>> +  unsigned int num_args;
>> > >>> +  tree *args,tmp;
>> > >>> +
>> > >>> +  num_args = gfc_intrinsic_argument_list_length (expr);
>> > >>> +  args = XALLOCAVEC (tree, num_args);
>> > >>> +
>> > >>> +  gfc_conv_intrinsic_function_args (se, expr, args, num_args);
>> > >>> +
>> > >>> +  if (flag_coarray == 

Add _FloatN, _FloatNx tests for __builtin_fpclassify

[Joseph Myers]

This patch adds tests for _FloatN and _FloatNx arguments to the
__builtin_fpclassify type-generic built-in function, omitted from the
original tests for type-generic functions on these types.

Tested for x86_64-pc-linux-gnu; all the supported new tests pass.  
Committed.

2016-09-21  Joseph Myers  

* gcc.dg/torture/float128-tg-3.c, gcc.dg/torture/float128x-tg-3.c,
gcc.dg/torture/float16-tg-3.c, gcc.dg/torture/float32-tg-3.c,
gcc.dg/torture/float32x-tg-3.c, gcc.dg/torture/float64-tg-3.c,
gcc.dg/torture/float64x-tg-3.c, gcc.dg/torture/floatn-tg-3.h: New
tests.

Index: gcc/testsuite/gcc.dg/torture/float128-tg-3.c
===
--- gcc/testsuite/gcc.dg/torture/float128-tg-3.c(nonexistent)
+++ gcc/testsuite/gcc.dg/torture/float128-tg-3.c(working copy)
@@ -0,0 +1,10 @@
+/* Test _Float128 type-generic built-in functions: __builtin_fpclassify.  */
+/* { dg-do run } */
+/* { dg-options "" } */
+/* { dg-add-options float128 } */
+/* { dg-add-options ieee } */
+/* { dg-require-effective-target float128_runtime } */
+
+#define WIDTH 128
+#define EXT 0
+#include "floatn-tg-3.h"
Index: gcc/testsuite/gcc.dg/torture/float128x-tg-3.c
===
--- gcc/testsuite/gcc.dg/torture/float128x-tg-3.c   (nonexistent)
+++ gcc/testsuite/gcc.dg/torture/float128x-tg-3.c   (working copy)
@@ -0,0 +1,10 @@
+/* Test _Float128x type-generic built-in functions: __builtin_fpclassify.  */
+/* { dg-do run } */
+/* { dg-options "" } */
+/* { dg-add-options float128x } */
+/* { dg-add-options ieee } */
+/* { dg-require-effective-target float128x_runtime } */
+
+#define WIDTH 128
+#define EXT 1
+#include "floatn-tg-3.h"
Index: gcc/testsuite/gcc.dg/torture/float16-tg-3.c
===
--- gcc/testsuite/gcc.dg/torture/float16-tg-3.c (nonexistent)
+++ gcc/testsuite/gcc.dg/torture/float16-tg-3.c (working copy)
@@ -0,0 +1,10 @@
+/* Test _Float16 type-generic built-in functions: __builtin_fpclassify.  */
+/* { dg-do run } */
+/* { dg-options "" } */
+/* { dg-add-options float16 } */
+/* { dg-add-options ieee } */
+/* { dg-require-effective-target float16_runtime } */
+
+#define WIDTH 16
+#define EXT 0
+#include "floatn-tg-3.h"
Index: gcc/testsuite/gcc.dg/torture/float32-tg-3.c
===
--- gcc/testsuite/gcc.dg/torture/float32-tg-3.c (nonexistent)
+++ gcc/testsuite/gcc.dg/torture/float32-tg-3.c (working copy)
@@ -0,0 +1,10 @@
+/* Test _Float32 type-generic built-in functions: __builtin_fpclassify.  */
+/* { dg-do run } */
+/* { dg-options "" } */
+/* { dg-add-options float32 } */
+/* { dg-add-options ieee } */
+/* { dg-require-effective-target float32_runtime } */
+
+#define WIDTH 32
+#define EXT 0
+#include "floatn-tg-3.h"
Index: gcc/testsuite/gcc.dg/torture/float32x-tg-3.c
===
--- gcc/testsuite/gcc.dg/torture/float32x-tg-3.c(nonexistent)
+++ gcc/testsuite/gcc.dg/torture/float32x-tg-3.c(working copy)
@@ -0,0 +1,10 @@
+/* Test _Float32x type-generic built-in functions: __builtin_fpclassify.  */
+/* { dg-do run } */
+/* { dg-options "" } */
+/* { dg-add-options float32x } */
+/* { dg-add-options ieee } */
+/* { dg-require-effective-target float32x_runtime } */
+
+#define WIDTH 32
+#define EXT 1
+#include "floatn-tg-3.h"
Index: gcc/testsuite/gcc.dg/torture/float64-tg-3.c
===
--- gcc/testsuite/gcc.dg/torture/float64-tg-3.c (nonexistent)
+++ gcc/testsuite/gcc.dg/torture/float64-tg-3.c (working copy)
@@ -0,0 +1,10 @@
+/* Test _Float64 type-generic built-in functions: __builtin_fpclassify.  */
+/* { dg-do run } */
+/* { dg-options "" } */
+/* { dg-add-options float64 } */
+/* { dg-add-options ieee } */
+/* { dg-require-effective-target float64_runtime } */
+
+#define WIDTH 64
+#define EXT 0
+#include "floatn-tg-3.h"
Index: gcc/testsuite/gcc.dg/torture/float64x-tg-3.c
===
--- gcc/testsuite/gcc.dg/torture/float64x-tg-3.c(nonexistent)
+++ gcc/testsuite/gcc.dg/torture/float64x-tg-3.c(working copy)
@@ -0,0 +1,10 @@
+/* Test _Float64x type-generic built-in functions: __builtin_fpclassify.  */
+/* { dg-do run } */
+/* { dg-options "" } */
+/* { dg-add-options float64x } */
+/* { dg-add-options ieee } */
+/* { dg-require-effective-target float64x_runtime } */
+
+#define WIDTH 64
+#define EXT 1
+#include "floatn-tg-3.h"
Index: gcc/testsuite/gcc.dg/torture/floatn-tg-3.h
===
--- gcc/testsuite/gcc.dg/torture/floatn-tg-3.h  (nonexistent)
+++ gcc/testsuite/gcc.dg/torture/floatn-tg-3.h  (working copy)
@@ -0,0 +1,79 @@
+/* Tests for _FloatN / _FloatNx types: compile and execution 

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

[Pedro Alves]

On 09/21/2016 03:59 PM, Szabolcs Nagy wrote:
> On 21/09/16 15:37, Alexander Monakov wrote:
>> On Wed, 21 Sep 2016, Martin Sebor wrote:
>>> On 09/21/2016 01:11 AM, Markus Trippelsdorf wrote:
 The patch uses "nul" instead of "null" throughout.
>>>
>>> Yes, that's intentional.  NUL and null are alternate spellings for
>>> the same character. I went with nul to distinguish it from the null
>>> pointer and used all lowercase as per the GCC convention.
>>
>> Can you elaborate which guideline suggests spelling that in lowercase?
> 
> the c standard calls it "null character".

*nod*  

The character is called "NUL" just much as I'm called "palves".  :-)

I.e., three-letter uppercase "NUL" is a control character
name abbreviation, just like, ACK, LF, DEL, ESC, etc.

The original ASCII standard used "NULL" for abbreviation, and then
in later revisions, it was shortened to "NUL".  All control
characters have 2 or 3 letter abbreviations in the latest ASCII
standard, and are written in uppercase, which to me looks like
the obvious reason for "NUL" with single "L".

https://en.wikipedia.org/wiki/Control_character
https://en.wikipedia.org/wiki/ASCII

https://web.archive.org/web/20160605145632/http://worldpowersystems.com/archives/codes/X3.4-1963/page5.JPG
https://web.archive.org/web/20160613203624/https://tools.ietf.org/html/rfc20

>From the latter:

~~~
[...]
5.2 Control Characters

  NUL (Null): The all-zeros character which may serve to accomplish
   time fill and media fill.
  SOH (Start of Heading): A communication control character used at
   the beginning of a sequence of characters which constitute a
[...]
~~~

> 
>> It seems quite strange to me, especially given that the documentation
>> added with the patch uses "NUL character" (which I believe to be a more
>> common form), but then warnings use "nul" (without the "character" iiuc).
>>

To me too.  Lowercase "nul" has all the looks of a typo to me.
I'd only use lowercase like that if abbreviations of other characters
were lowercase too in similar contexts, like "... the esc character ...",
etc.

Thanks,
Pedro Alves

Re: [PATCH 4/4][Ada,DJGPP] Ada support for DJGPP

[Andris Pavenis]

On 09/04/2016 09:52 PM, Arnaud Charlet wrote:

This last patch (4/4) contains DJGPP related changes to adaint.c
(except one which belongs to patch 1/4).

This patch is quite intrusive. Are all these changes really needed?


New version of patch is in attachment.

  char
  __gnat_get_default_identifier_character_set (void)
  {
+#if defined (__DJGPP__)
+  return 'p';
+#else
return '1';
+#endif
  }

Why is this needed?

Removed in updated version of patch

-#elif defined (_WIN32)
+#elif defined (__DJGPP__) || defined (_WIN32)
/* args[0] must be quotes as it could contain a full pathname with spaces
*/
char *args_0 = args[0];
args[0] = (char *)xmalloc (strlen (args_0) + 3);
@@ -2606,6 +2630,12 @@ __gnat_portable_no_block_spawn (char *args[]
ATTRIBUTE_UNUSED)
/* Not supported.  */
return -1;
  
+#elif defined(__DJGPP__)

+  if (spawnvp (P_WAIT, args[0], args) != 0)
+return -1;
+  else
+return 0;
+
  #elif defined (_WIN32)
  
HANDLE h = NULL;

@@ -2649,6 +2679,7 @@ __gnat_portable_wait (int *process_status)
  
pid = win32_wait ();
  
+#elif defined (__DJGPP__)

  #else

You can't add an empty #elif without explaining it with a proper
comment.

Comment with explanation added in attached version of patch

Andris

>From 49015a63e708824dbd80fb90520c33b8e1607c43 Mon Sep 17 00:00:00 2001
From: Andris Pavenis 
Date: Thu, 15 Sep 2016 19:31:54 +0300
Subject: [PATCH 4/4] [DJGPP, Ada] Ada support

* ada/adaint.c: Include process.h, signal.h, dir.h and utime.h for DJGPP.
  ISALPHA: include  and define to isalpha for DJGPP when IN_RTS is defined.
  (DIR_SEPARATOR) define to '\\' for DJGPP.
  (__gnat_get_maximum_file_name_length): decide return value depending on
  availability of LFN for DJGPP
  (__gnat_get_file_names_case_sensitive): return 0 for DJGPP unless
  overriden in environment
  (__gnat_is_absolute_path): Support MS-DOS style absolute paths for DJGPP.
  (__gnat_portable_spawn): Use spewnvp for DJGPP.
  (__gnat_portable_no_block_spawn): Use spawnvp for DJGPP.
  (__gnat_portable_wait): Return 0 for DJGPP.



Signed-off-by: Andris Pavenis 
---
 gcc/ada/adaint.c | 43 ---
 1 file changed, 36 insertions(+), 7 deletions(-)

diff --git a/gcc/ada/adaint.c b/gcc/ada/adaint.c
index f317865..ed49ed7 100644
--- a/gcc/ada/adaint.c
+++ b/gcc/ada/adaint.c
@@ -112,7 +112,18 @@
 extern "C" {
 #endif
 
-#if defined (__MINGW32__) || defined (__CYGWIN__)
+#if defined (__DJGPP__)
+
+/* For isalpha-like tests in the compiler, we're expected to resort to
+   safe-ctype.h/ISALPHA.  This isn't available for the runtime library
+   build, so we fallback on ctype.h/isalpha there.  */
+
+#ifdef IN_RTS
+#include 
+#define ISALPHA isalpha
+#endif
+
+#elif defined (__MINGW32__) || defined (__CYGWIN__)
 
 #include "mingw32.h"
 
@@ -165,11 +176,16 @@ UINT CurrentCCSEncoding;
 #include 
 #endif
 
-#if defined (_WIN32)
-
+#if defined (__DJGPP__)
 #include 
 #include 
 #include 
+#include 
+#undef DIR_SEPARATOR
+#define DIR_SEPARATOR '\\'
+
+#elif defined (_WIN32)
+
 #include 
 #include 
 #include 
@@ -538,7 +554,11 @@ __gnat_try_lock (char *dir, char *file)
 int
 __gnat_get_maximum_file_name_length (void)
 {
+#if defined (__DJGPP__)
+  return (_use_lfn(".")) ? -1 : 8;
+#else
   return -1;
+#endif
 }
 
 /* Return nonzero if file names are case sensitive.  */
@@ -560,7 +580,7 @@ __gnat_get_file_names_case_sensitive (void)
 	{
 	  /* By default, we suppose filesystems aren't case sensitive on
 	 Windows and Darwin (but they are on arm-darwin).  */
-#if defined (WINNT) \
+#if defined (WINNT) || defined (__DJGPP__) \
   || (defined (__APPLE__) && !(defined (__arm__) || defined (__arm64__)))
 	  file_names_case_sensitive_cache = 0;
 #else
@@ -576,7 +596,7 @@ __gnat_get_file_names_case_sensitive (void)
 int
 __gnat_get_env_vars_case_sensitive (void)
 {
-#if defined (WINNT)
+#if defined (WINNT) || defined (__DJGPP__)
  return 0;
 #else
  return 1;
@@ -1646,7 +1666,7 @@ __gnat_is_absolute_path (char *name, int length)
 #else
   return (length != 0) &&
  (*name == '/' || *name == DIR_SEPARATOR
-#if defined (WINNT)
+#if defined (WINNT) || defined(__DJGPP__)
   || (length > 1 && ISALPHA (name[0]) && name[1] == ':')
 #endif
 	  );
@@ -2234,7 +2254,7 @@ __gnat_portable_spawn (char *args[] ATTRIBUTE_UNUSED)
 #if defined (__vxworks) || defined(__PikeOS__)
   return -1;
 
-#elif defined (_WIN32)
+#elif defined (__DJGPP__) || defined (_WIN32)
   /* args[0] must be quotes as it could contain a full pathname with spaces */
   char *args_0 = args[0];
   args[0] = (char *)xmalloc (strlen (args_0) + 3);
@@ -2606,6 +2626,12 @@ __gnat_portable_no_block_spawn (char *args[] ATTRIBUTE_UNUSED)
   /* Not supported.  */
   return -1;
 
+#elif defined(__DJGPP__)
+  if (spawnvp (P_WAIT, args[0], args) != 0)
+return -1;
+  else
+return 0;
+
 #elif defined (_WIN32)
 
   HANDLE h = NULL;
@@ -2649,6 +2675,9 @@ __gnat_portable_wait (int 

[RFC] Extend ipa-bitwise-cp with pointer alignment propagation

[Prathamesh Kulkarni]

Hi,
The attached patch tries to extend ipa bits propagation to handle
pointer alignment propagation.
The patch just disables ipa-cp-alignment pass, I suppose we want to
eventually remove it ?

Bootstrap+tested on x86_64-unknown-linux-gnu.
Cross-tested on arm*-*-*, aarch64*-*-*.
Does the patch look OK ?

Thanks,
Prathamesh
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 5ff7bed..d251223 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -1871,16 +1871,29 @@ propagate_bits_accross_jump_function (cgraph_edge *cs, int idx, ipa_jump_func *j
   unsigned precision = TYPE_PRECISION (parm_type);
   signop sgn = TYPE_SIGN (parm_type);
 
-  if (jfunc->type == IPA_JF_PASS_THROUGH)
+  if (jfunc->type == IPA_JF_PASS_THROUGH
+  || jfunc->type == IPA_JF_ANCESTOR)
 {
   struct ipa_node_params *caller_info = IPA_NODE_REF (cs->caller);
-  enum tree_code code = ipa_get_jf_pass_through_operation (jfunc);
   tree operand = NULL_TREE;
+  enum tree_code code;
+  unsigned src_idx;
 
-  if (code != NOP_EXPR)
-	operand = ipa_get_jf_pass_through_operand (jfunc);
+  if (jfunc->type == IPA_JF_PASS_THROUGH)
+	{
+	  code = ipa_get_jf_pass_through_operation (jfunc);
+	  src_idx = ipa_get_jf_pass_through_formal_id (jfunc);
+	  if (code != NOP_EXPR)
+	operand = ipa_get_jf_pass_through_operand (jfunc);
+	}
+  else
+	{
+	  code = POINTER_PLUS_EXPR; 
+	  src_idx = ipa_get_jf_ancestor_formal_id (jfunc);
+	  unsigned HOST_WIDE_INT offset = ipa_get_jf_ancestor_offset (jfunc) / BITS_PER_UNIT;
+	  operand = build_int_cstu (size_type_node, offset);
+	}
 
-  int src_idx = ipa_get_jf_pass_through_formal_id (jfunc);
   struct ipcp_param_lattices *src_lats
 	= ipa_get_parm_lattices (caller_info, src_idx);
 
@@ -2258,8 +2271,8 @@ propagate_constants_accross_call (struct cgraph_edge *cs)
 			 _plats->itself);
 	  ret |= propagate_context_accross_jump_function (cs, jump_func, i,
 			  _plats->ctxlat);
-	  ret |= propagate_alignment_accross_jump_function (cs, jump_func,
-			 _plats->alignment);
+//	  ret |= propagate_alignment_accross_jump_function (cs, jump_func,
+//			 _plats->alignment);
 	  ret |= propagate_bits_accross_jump_function (cs, i, jump_func,
 		   _plats->bits_lattice);
 	  ret |= propagate_aggs_accross_jump_function (cs, jump_func,
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 1629781..5cee27b 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -1701,6 +1701,16 @@ ipa_compute_jump_functions_for_edge (struct ipa_func_body_info *fbi,
 	  jfunc->bits.mask = 0;
 	}
 	}
+  else if (POINTER_TYPE_P (TREE_TYPE (arg)))
+	{
+	  unsigned HOST_WIDE_INT bitpos;
+	  unsigned align;
+
+	  jfunc->bits.known = true;
+	  get_pointer_alignment_1 (arg, , );
+	  jfunc->bits.mask = wi::mask(TYPE_PRECISION (TREE_TYPE (arg)), false).and_not (align / BITS_PER_UNIT - 1);
+	  jfunc->bits.value = bitpos / BITS_PER_UNIT;
+	}
   else
 	gcc_assert (!jfunc->bits.known);
 
@@ -5534,7 +5544,7 @@ ipcp_update_bits (struct cgraph_node *node)
   next_parm = DECL_CHAIN (parm);
 
   if (!bits[i].known
-	  || !INTEGRAL_TYPE_P (TREE_TYPE (parm))
+	  || !(INTEGRAL_TYPE_P (TREE_TYPE (parm)) || POINTER_TYPE_P (TREE_TYPE (parm)))
 	  || !is_gimple_reg (parm))
 	continue;   
 
@@ -5549,12 +5559,41 @@ ipcp_update_bits (struct cgraph_node *node)
 	  fprintf (dump_file, "\n");
 	}
 
-  unsigned prec = TYPE_PRECISION (TREE_TYPE (ddef));
-  signop sgn = TYPE_SIGN (TREE_TYPE (ddef));
+  if (INTEGRAL_TYPE_P (TREE_TYPE (ddef)))
+	{
+	  unsigned prec = TYPE_PRECISION (TREE_TYPE (ddef));
+	  signop sgn = TYPE_SIGN (TREE_TYPE (ddef));
+
+	  wide_int nonzero_bits = wide_int::from (bits[i].mask, prec, UNSIGNED)
+  | wide_int::from (bits[i].value, prec, sgn);
+	  set_nonzero_bits (ddef, nonzero_bits);
+	}
+  else
+	{
+	  unsigned tem = bits[i].mask.to_uhwi ();
+	  unsigned HOST_WIDE_INT bitpos = bits[i].value.to_uhwi (); 
+	  unsigned align = tem & -tem;
+	  unsigned misalign = bitpos & (align - 1);
 
-  wide_int nonzero_bits = wide_int::from (bits[i].mask, prec, UNSIGNED)
-			  | wide_int::from (bits[i].value, prec, sgn);
-  set_nonzero_bits (ddef, nonzero_bits);
+	  if (align > 1)
+	{
+	  if (dump_file)
+		fprintf (dump_file, "Adjusting align: %u, misalign: %u\n", align, misalign); 
+
+	  unsigned old_align, old_misalign;
+	  struct ptr_info_def *pi = get_ptr_info (ddef);
+	  bool old_known = get_ptr_info_alignment (pi, _align, _misalign);
+
+	  if (old_known
+		  && old_align > align)
+		{
+		  if (dump_file)
+		fprintf (dump_file, "But alignment was already %u.\n", old_align);
+		  continue;
+		}
+	  set_ptr_info_alignment (pi, align, misalign); 
+	}
+	}
 }
 }
 
diff --git a/gcc/testsuite/gcc.dg/ipa/propalign-1.c b/gcc/testsuite/gcc.dg/ipa/propalign-1.c
index f34552c..1491de8 100644
--- a/gcc/testsuite/gcc.dg/ipa/propalign-1.c
+++ b/gcc/testsuite/gcc.dg/ipa/propalign-1.c
@@ -27,5 +27,5 @@ bar (void)
 }
 
 
-/* { dg-final { 

[PATCH, v2] Introduce class rtx_reader

[David Malcolm]

On Fri, 2016-09-16 at 16:04 -0600, Jeff Law wrote:
> On 09/08/2016 06:30 PM, David Malcolm wrote:
> > Bundle up various global variables within gensupport.c into a
> > class rtx_reader, with a view towards making it easier to run the
> > code more than once in-process.
> > 
> > gcc/ChangeLog:
> > * genconstants.c (main): Introduce noop_reader and convert call
> > to read_md_files to a method call.
> > * genenums.c (main): Likewise.
> > * genmddeps.c (main): Likewise.
> > * genpreds.c (write_tm_constrs_h): Replace use of "in_fname"
> > with
> > rtx_reader_ptr->get_top_level_filename ().
> > (write_tm_preds_h): Likewise.
> > (write_insn_preds_c): Likewise.
> > * gensupport.c (class gen_reader): New subclass of rtx_reader.
> > (rtx_handle_directive): Convert to...
> > (gen_reader::handle_unknown_directive): ...this.
> > (init_rtx_reader_args_cb): Convert return type from bool to
> > rtx_reader *.  Create a gen_reader instance, using it for the
> > call to read_md_files.  Return it if no errors occur.
> > (init_rtx_reader_args): Convert return type from bool to
> > rtx_reader *.
> > * gensupport.h (init_rtx_reader_args_cb): Likewise.
> > (init_rtx_reader_args_cb): Likewise.
> > * read-md.c (struct file_name_list): Move to class rtx_reader.
> > (read_md_file): Delete in favor of rtx_reader::m_read_md_file.
> > (read_md_filename): Delete in favor of
> > rtx_reader::m_read_md_filename.
> > (read_md_lineno): Delete in favor of
> > rtx_reader::m_read_md_lineno.
> > (in_fname): Delete in favor of rtx_reader::m_toplevel_fname.
> > (base_dir): Delete in favor of rtx_reader::m_base_dir.
> > (first_dir_md_include): Delete in favor of
> > rtx_reader::m_first_dir_md_include.
> > (last_dir_md_include_ptr): Delete in favor of
> > rtx_reader::m_last_dir_md_include_ptr.
> > (max_include_len): Delete.
> > (rtx_reader_ptr): New.
> > (fatal_with_file_and_line): Use get_filename and get_lineno
> > accessors of rtx_reader_ptr.
> > (require_char_ws): Likewise.
> > (rtx_reader::read_char): New method, based on ::read_char.
> > (rtx_reader::unread_char): New method, based on ::unread_char.
> > (read_escape): Use get_filename and get_lineno accessors of
> > rtx_reader_ptr.
> > (read_braced_string): Use get_lineno accessor of
> > rtx_reader_ptr.
> > (read_string): Use get_filename and get_lineno accessors of
> > rtx_reader_ptr.
> > (rtx_reader::rtx_reader): New ctor.
> > (rtx_reader::~rtx_reader): New dtor.
> > (handle_include): Convert from a function to...
> > (rtx_reader::handle_include): ...this method, converting
> > handle_directive from a callback to a virtual function.
> > (handle_file): Likewise, converting to...
> > (rtx_reader::handle_file): ...this method.
> > (handle_toplevel_file): Likewise, converting to...
> > (rtx_reader::handle_toplevel_file): ...this method.
> > (rtx_reader::get_current_location): New method.
> > (parse_include): Convert from a function to...
> > (rtx_reader::add_include_path): ...this method, dropping
> > redundant
> > update to unused max_include_len.
> > (read_md_files): Convert from a function to...
> > (rtx_reader::read_md_files): ...this method, converting
> > handle_directive from a callback to a virtual function.
> > (noop_reader::handle_unknown_directive): New method.
> > * read-md.h (directive_handler_t): Delete this typedef.
> > (in_fname): Delete.
> > (read_md_file): Delete.
> > (read_md_lineno): Delete.
> > (read_md_filename): Delete.
> > (class rtx_reader): New class.
> > (rtx_reader_ptr): New decl.
> > (class noop_reader): New subclass of rtx_reader.
> > (read_char): Reimplement in terms of rtx_reader::read_char.
> > (unread_char): Reimplement in terms of rtx_reader::unread_char.
> > (read_md_files): Delete.
> > * read-rtl.c (read_rtx_code): Update for deletion of globals
> > read_md_filename and read_md_lineno.
> I don't see anything terribly objectionable here.
> 
> It looks like you use a singleton to avoid passing the object around
> everywhere, but given the state of all this prior to this patch, I
> can
> live with the cleanup as a whole.

Indeed; the point of the patch is to bundle up some global state to
better allow the reader to be run more than once in-process, so that
the RTL frontend can have selftests; having more than one reader *alive*
at once is out-of-scope.  Hence the use of a singleton, to minimize the
amount of churn.

> I'll note Richard Sandiford. hasn't chimed in here.  You might ping
> him
> directly to see if he's got any feedback.  If he doesn't prior to say
> Wed, this is OK for the trunk.
> 
> jeff

Thanks.

Here's a slightly tweaked version, fixing a couple of missing NULL
initializations in the rtx_reader ctor (of m_base_dir and m_read_md_file),
which avoids a crash for the 

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

[Florian Weimer]

* Carlos O'Donell:

>> Yes, and for the warning to go away, GCC's MALLOC_ABI_ALIGNMENT needs
>> to be increased as well, I think.
>
> This is not required.

Strictly speaking, that's true.

> The __attribute__((__aligned__(16))); is not required for pthread locks
> to work correctly, it is simply there to ensure structure padding and
> layout matches the pre-NPTL ABI to ensure we don't break ABI.
>
> In the original Linuxthreads code there was indeed a singular lock word
> that needed 16-byte alignment because the hppa 'load and clear word'
> atomic operation needed that alignment.
>
> In NPTL the alignment restrictions are lifted and we use a light-weight
> kernel helper (like ARM and m68k) which does compare-and-swap atomically
> using kernel-side locks (synchronized with futex operations in the
> kernel).
>
> So the alignment is only needed for structure layout.
>
> Malloc can return word aligned memory for a pthread_mutex_t and it would
> work just fine.

There is a slight risk that subsequent members will not sufficient
alignment if such an alignment was requested explicitly by the
programmer.

> The broader question is: How do we get rid of the warning?

We could add a “please align inside struct, but do not increase the
top-most struct alignment” feature to GCC if it does not have one yet.

But I don't think it's worth this effort to do this just for HPPA.

We should bump MALLOC_ABI_ALIGNMENT and max_align_t and be done with
it.  I don't see anything else worth doing which also gets rid of the
warning.  It's also fairly simple conceptually and carries less risk
of breaking in unexpected ways later.

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

[Jason Merrill]

On 09/20/2016 01:13 PM, Marek Polacek wrote:

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).


Hmm, I'm not sure that change is right for C.  But the C++ hunk is OK.

Jason

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

[Carlos O'Donell]

On 09/21/2016 10:56 AM, Florian Weimer wrote:
> * John David Anglin:
> 
>> On Tue, Sep 20, 2016 at 09:27:17PM +0200, Florian Weimer wrote:
>>> * 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.
>>
>> I'm not sure I understand the comment.  I created a short testcase.
>> I agree that the issue is hppa specific but the extended alignment
>> is coming from the __lock field in pthread_mutex_t.  I'm not sure
>> why this affects new std::ofstream().
> 
> Ahh, thanks for the explanation.  The C++ streams probably have some
> internal locks (hidden behind libstdc++ abstractions).

The alignment is a historical ABI accident that we inherited from
Linuxthreads.

>> The alignment of 16 arises in code that used the ldcw instruction.
>> Although this could be avoided in glibc there are numerous other
>> packages with objects requiring 16-byte alignment.  So, I'm tending
>> to think the typedef for max_align_t should be adjusted on hppa-linux
>> so that it has 16-byte alignment.  Is that the correct approach?
> 
> Yes, and for the warning to go away, GCC's MALLOC_ABI_ALIGNMENT needs
> to be increased as well, I think.

This is not required.

The __attribute__((__aligned__(16))); is not required for pthread locks
to work correctly, it is simply there to ensure structure padding and
layout matches the pre-NPTL ABI to ensure we don't break ABI.

In the original Linuxthreads code there was indeed a singular lock word
that needed 16-byte alignment because the hppa 'load and clear word'
atomic operation needed that alignment.

In NPTL the alignment restrictions are lifted and we use a light-weight
kernel helper (like ARM and m68k) which does compare-and-swap atomically
using kernel-side locks (synchronized with futex operations in the
kernel).

So the alignment is only needed for structure layout.

Malloc can return word aligned memory for a pthread_mutex_t and it would
work just fine.

The broader question is: How do we get rid of the warning?

> Nowadays, we can change the glibc malloc alignment fairly easily, but
> interposed mallocs won't be so nice.  But it is unlikely that any of
> this matters in practice because the issue is not new (only the
> warning is).

The issue is not new, but changing glibc's malloc alignment is not
required.

> I have Cc:ed a few people who might have ideas how to deal with this.
> 
> Torvald, would it be possible to align mutexes internally on hppa, to
> avoid the 16-byte alignment of the entire struct (that is, store a
> pointer to the actual mutex object, which points to a sub-region of
> the struct which is suitably aligned)?

The attribute aligned is used for laying out larger structures where the
pthread structures are embedded in them. Therefore removing attribute
aligned would result in an ABI break for all such embedded structures.
The attribute aligned must remain for such structures to continue to be
laid out the same way they were with Linuxthreads, and now NPTL.

As I mentioned before, the alignment is not required for the correct
operation of the locks.

> We probably could add libstdc++ compile-time tests which make sure
> that all relevant C++ types can be allocated with the global operator
> new.

So how do we get rid of the warning?

We need to keep __attribute__((__aligned(16))); because of structure
layout ABI compatibility, but we don't actually need the alignment at
runtime anymore.

-- 
Cheers,
Carlos.

Re: [PATCH][3/n] dwarf2out refactoring for early (LTO) debug

[Jason Merrill]

Looks good.

On Wed, Sep 21, 2016 at 4:37 AM, Richard Biener
 wrote:
> On Wed, Aug 26, 2015 at 1:34 PM, Richard Biener  wrote:
>>
>> The following fixes a GC issue I run into when doing
>> prune_unused_types_prune early.  The issue is that the DIE struct
>> has a chain_circular marked field (die_sib) which cannot tolerate
>> spurious extra entries from old removed entries into the circular
>> chain.  Otherwise we fail to properly mark parts of the chain.
>> Those stray entries are kept live referenced from TYPE_SYMTAB_DIE.
>>
>> So the following patch that makes sure to clear ->die_sib for
>> nodes we remove.  (these DIEs remaining in TYPE_SYMTAB_DIE also
>> means we may end up re-using them which is probably not what we
>> want ... in the original LTO experiment I had a ->removed flag
>> in the DIE struct and removed DIEs from the cache at cache lookup
>> time if I hit a removed DIE)
>>
>> Bootstrapped and tested on x86_64-unknown-linux-gnu, gdb tested there
>> as well.
>>
>> Ok for trunk?
>
> I am re-testing this now and will commit it as part of the piecewise early LTO
> merge.
>
> Richard.
>
>> Thanks,
>> Richard.
>>
>> 2015-08-26  Richard Biener  
>>
>> * dwarf2out.c (remove_child_with_prev): Clear child->die_sib.
>> (replace_child): Likewise.
>> (remove_child_TAG): Adjust.
>> (move_marked_base_types): Likewise.
>> (prune_unused_types_prune): Clear die_sib of removed children.
>>
>> Index: trunk/gcc/dwarf2out.c
>> ===
>> --- trunk.orig/gcc/dwarf2out.c  2015-08-26 09:30:54.679185817 +0200
>> +++ trunk/gcc/dwarf2out.c   2015-08-25 16:54:09.150506037 +0200
>> @@ -4827,6 +4827,7 @@ remove_child_with_prev (dw_die_ref child
>>  prev->die_sib = child->die_sib;
>>if (child->die_parent->die_child == child)
>>  child->die_parent->die_child = prev;
>> +  child->die_sib = NULL;
>>  }
>>
>>  /* Replace OLD_CHILD with NEW_CHILD.  PREV must have the property that
>> @@ -4853,6 +4854,7 @@ replace_child (dw_die_ref old_child, dw_
>>  }
>>if (old_child->die_parent->die_child == old_child)
>>  old_child->die_parent->die_child = new_child;
>> +  old_child->die_sib = NULL;
>>  }
>>
>>  /* Move all children from OLD_PARENT to NEW_PARENT.  */
>> @@ -4883,9 +4885,9 @@ remove_child_TAG (dw_die_ref die, enum d
>> remove_child_with_prev (c, prev);
>> c->die_parent = NULL;
>> /* Might have removed every child.  */
>> -   if (c == c->die_sib)
>> +   if (die->die_child == NULL)
>>   return;
>> -   c = c->die_sib;
>> +   c = prev->die_sib;
>>}
>>} while (c != die->die_child);
>>  }
>> @@ -24565,8 +24590,8 @@ prune_unused_types_prune (dw_die_ref die
>>
>>c = die->die_child;
>>do {
>> -dw_die_ref prev = c;
>> -for (c = c->die_sib; ! c->die_mark; c = c->die_sib)
>> +dw_die_ref prev = c, next;
>> +for (c = c->die_sib; ! c->die_mark; c = next)
>>if (c == die->die_child)
>> {
>>   /* No marked children between 'prev' and the end of the list.  */
>> @@ -24578,8 +24603,14 @@ prune_unused_types_prune (dw_die_ref die
>>   prev->die_sib = c->die_sib;
>>   die->die_child = prev;
>> }
>> + c->die_sib = NULL;
>>   return;
>> }
>> +  else
>> +   {
>> + next = c->die_sib;
>> + c->die_sib = NULL;
>> +   }
>>
>>  if (c != prev->die_sib)
>>prev->die_sib = c;
>> @@ -24824,8 +24855,8 @@ move_marked_base_types (void)
>>   remove_child_with_prev (c, prev);
>>   /* As base types got marked, there must be at least
>>  one node other than DW_TAG_base_type.  */
>> - gcc_assert (c != c->die_sib);
>> - c = c->die_sib;
>> + gcc_assert (die->die_child != NULL);
>> + c = prev->die_sib;
>> }
>>  }
>>while (c != die->die_child);

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

[Bernd Edlinger]

On 09/21/16 17:00, Jason Merrill wrote:
> On 09/20/2016 02:40 PM, Bernd Edlinger wrote:
>> On 09/20/16 16:51, Jason Merrill wrote:
>>> On Tue, Sep 20, 2016 at 10:11 AM, Bernd Edlinger
>>>  wrote:
 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.
>>>
>> +  if (warn_int_in_bool_context
>> +  && !from_macro_expansion_at (EXPR_LOCATION (expr)))
>>
>> 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?
>
> This should fix that, though I haven't run regression tests yet:
>

Yes.  I think that goes in the right direction,
but it does not work yet.

#define XXX (a ? 2 : 3)

if (XXX) // prints a warning, but it should not.


Bernd.

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

[Jeff Law]

On 09/21/2016 01:11 AM, Markus Trippelsdorf wrote:

On 2016.09.11 at 20:03 -0600, Martin Sebor wrote:

On 09/08/2016 04:10 PM, Joseph Myers wrote:

On Thu, 8 Sep 2016, Martin Sebor wrote:



PR middle-end/49905 - Better sanity checking on sprintf src & dest to
produce warning for dodgy code


The patch uses "nul" instead of "null" throughout.
It was referring to a character rather than a null pointer.  That's why 
I didn't call it out.


jeff

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

[Marek Polacek]

On Wed, Sep 21, 2016 at 11:37:32AM -0400, Jason Merrill wrote:
> On 09/20/2016 01:13 PM, Marek Polacek wrote:
> > 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).
> 
> Hmm, I'm not sure that change is right for C.  But the C++ hunk is OK.

Thanks.  Joseph, how about the C part?

Marek

Re: [PATCH] Fix ICE with inlining and #pragma omp simd (PR fortran/77665)

[Alexander Monakov]

On Wed, 21 Sep 2016, Jakub Jelinek wrote:
> The simduid pass uses the cfun->has_simduid_loops flag to determine if it
> needs to clean up any left-over GOMP_SIMD_* internal functions.
> During inlining, we set the flag if we inline some loop with simduid, or if
> we find GOMP_SIMD_ORDERED_* internal call, but the testcase shows that we
> need to do that for the other GOMP_SIMD_* ifns too, because what we inline
> might have turned the loop->simduid loops into non-loops (or they might not
> exist from the beginning because simd region had noreturn body).

I think this means that the comment at the declaration of has_simduid in
function.h needs to be updated.

I see that there are other places in the compiler that propagate the has_simduid
property. Now that the property is tied to just the presence of GOMP_SIMD_xyz
IFNs, won't those other places need changes too?

I wonder why this property is propagated with such fine granularity in the first
place. Wouldn't it be simpler and almost as efficient to propagate it on the
function basis, i.e. when inlining A into B set it on B if A had it, without
regard for whether uses in A have already been DCE'd?

Thanks.
Alexander

[PATCH, testsuite]: Remove debug statements from gcc.c-torture/unsorted/dump-noaddr.x

[Uros Bizjak]

2016-09-21  Uros Bizjak  

* gcc.c-torture/unsorted/dump-noaddr.x: Remove debug statements.

Tested on x86_64-linux-gnu {,-m32}, committed to mainline SVN and
release branches.

Uros.
diff --git a/gcc/testsuite/gcc.c-torture/unsorted/dump-noaddr.x 
b/gcc/testsuite/gcc.c-torture/unsorted/dump-noaddr.x
index 001dd6b..d14d494 100644
--- a/gcc/testsuite/gcc.c-torture/unsorted/dump-noaddr.x
+++ b/gcc/testsuite/gcc.c-torture/unsorted/dump-noaddr.x
@@ -19,7 +19,6 @@ proc dump_compare { src options } {
regsub dump1/ $dump1 dump2/ dump2
set dumptail "gcc.c-torture/unsorted/[file tail $dump1]"
regsub {\.\d+((t|r|i)\.[^.]+)$} $dumptail {.*\1} dumptail
-   #puts "$option $dump1"
set tmp [ diff "$dump1" "$dump2" ]
if { $tmp == 0 } {
untested "$dumptail, $option comparison"
@@ -28,14 +27,11 @@ proc dump_compare { src options } {
} else {
fail "$dumptail, $option comparison"
}
-   #exec diff $dump1 $dump2
}
 }
 file delete -force dump1
 file delete -force dump2
 }
 
-catch {dump_compare $src $options} result
-puts $result
-verbose result
+dump_compare $src $options
 return 1

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

[Martin Sebor]

On 09/21/2016 08:37 AM, Alexander Monakov wrote:

On Wed, 21 Sep 2016, Martin Sebor wrote:

On 09/21/2016 01:11 AM, Markus Trippelsdorf wrote:

The patch uses "nul" instead of "null" throughout.


Yes, that's intentional.  NUL and null are alternate spellings for
the same character. I went with nul to distinguish it from the null
pointer and used all lowercase as per the GCC convention.


Can you elaborate which guideline suggests spelling that in lowercase?
It seems quite strange to me, especially given that the documentation
added with the patch uses "NUL character" (which I believe to be a more
common form), but then warnings use "nul" (without the "character" iiuc).


I don't know if there is a specific guideline that applies here,
but it's my impression that conventionally GCC messages use
lowercase letters.  The only guideline I know about is in the GNU
Coding Standard that calls for messages not to begin with capital
letters (or end in a period;  though now that I've checked more
carefully there are a fair number of messages that ignore that
suggestion).

That said, if the lowercase nul is offensive it can certainly be
changed to some other spelling (as can any of the other messages
emitted by the pass).

Martin

PS Yes, the C and C++ standards refer to NUL as the null character.
NUL is an abbreviation, used extensively (for example) by POSIX.

[PATCH] Fix various minor gimple-ssa-sprintf.c issues

[Jakub Jelinek]

Hi!

When looking at PR77676, I've noticed various small formatting etc.
issues, like not using is_gimple_* APIs where we have them, not using
gimple_call_builtin_p/gimple_call_fndecl (this one actually can show up,
if e.g. uses the builtin with incorrect arguments (fewer, different
types etc.)), one pasto, 2 spaces in comments instead of 1 in the middle
of sentences.  And, lastly 0 < var is very unusual ordering of the
comparison operands, while we have a couple of such cases in the sources,
usually it is when using 0 < var && var <= someotherconst, while
var > 0 is used hundred times more often.

I see various other issues, which I'll happily defer to Martin, e.g.
  /* These are available as macros in the C and C++ front ends but,
 sadly, not here.  */
  static tree intmax_type_node;
  static tree uintmax_type_node;
  
  /* Initialize the intmax nodes above the first time through here.  */
  if (!intmax_type_node)
build_intmax_type_nodes (_type_node, _type_node);
This is a big no no, the trees aren't registered with GC this way, so
they could be garbage collected (you'd need to move those to file-scope to
be able to GC register them).
Or
/* Compute the maximum just once.  */
static const int a_max[] = {
  format_floating_max (double_type_node, 'a'),
  format_floating_max (long_double_type_node, 'a')
};
etc. (used several time), do we really need to mess up with the
(thread-safe) local static locking/guards etc.?  Can't you initialize
the cache say to -1 and compute first if you need it and it is still -1?
PR77676 shows various other issues in the pass.

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

2016-09-21  Jakub Jelinek  

* gimple-ssa-sprintf.c: Fix comment formatting.
(pass_sprintf_length::gate): Use x > 0 instead of 0 < x.
(format_integer): Use is_gimple_assign.
(format_floating, format_string, format_directive,
get_destination_size): Use x > 0 instead of 0 < x.
(pass_sprintf_length::handle_gimple_call): Likewise.  Use
gimple_call_builtin_p and gimple_call_fndecl.  Reorder
case BUILT_IN_SPRINTF_CHK.  Fix up BUILT_IN_SNPRINTF_CHK comment.
Replace "to to" with "to" in comment.
(pass_sprintf_length::execute): Use is_gimple_call.

--- gcc/gimple-ssa-sprintf.c.jj 2016-09-21 08:54:15.0 +0200
+++ gcc/gimple-ssa-sprintf.c2016-09-21 15:09:02.209261013 +0200
@@ -37,7 +37,7 @@ along with GCC; see the file COPYING3.
 
The pass handles all forms standard sprintf format directives,
including character, integer, floating point, pointer, and strings,
-   with  the standard C flags, widths, and precisions.  For integers
+   with the standard C flags, widths, and precisions.  For integers
and strings it computes the length of output itself.  For floating
point it uses MPFR to fornmat known constants with up and down
rounding and uses the resulting range of output lengths.  For
@@ -130,8 +130,8 @@ pass_sprintf_length::gate (function *)
  not optimizing and the pass is being invoked early, or when
  optimizing and the pass is being invoked during optimization
  (i.e., "late").  */
-  return ((0 < warn_format_length || flag_printf_return_value)
- && (0 < optimize) == fold_return_value);
+  return ((warn_format_length > 0 || flag_printf_return_value)
+ && (optimize > 0) == fold_return_value);
 }
 
 /* The result of a call to a formatted function.  */
@@ -464,7 +464,7 @@ struct conversion_spec
 
   /* Format conversion function that given a conversion specification
  and an argument returns the formatting result.  */
-  fmtresult  (*fmtfunc) (const conversion_spec &, tree);
+  fmtresult (*fmtfunc) (const conversion_spec &, tree);
 
   /* Return True when a the format flag CHR has been used.  */
   bool get_flag (char chr) const
@@ -1032,10 +1032,10 @@ format_integer (const conversion_spec 
{
  /* The argument here may be the result of promoting the actual
 argument to int.  Try to determine the type of the actual
-argument before promotion and  narrow down its range that
+argument before promotion and narrow down its range that
 way.  */
  gimple *def = SSA_NAME_DEF_STMT (arg);
- if (gimple_code (def) == GIMPLE_ASSIGN)
+ if (is_gimple_assign (def))
{
  tree_code code = gimple_assign_rhs_code (def);
  if (code == NOP_EXPR)
@@ -1188,7 +1188,7 @@ format_floating (const conversion_spec &
 case 'a':
   {
/* The minimum output is "0x.p+0".  */
-   res.range.min = 6 + (0 < prec ? prec : 0);
+   res.range.min = 6 + (prec > 0 ? prec : 0);
 
/* Compute the maximum just once.  */
static const int a_max[] = {
@@ -1249,7 +1249,7 @@ format_floating (const conversion_spec &
   gcc_unreachable ();
 }
 
-  if (0 < width)
+  if (width > 0)
 {
   

Re: [C++ PATCH] Aligned new option handling fixes (PR c++/77651)

[Jason Merrill]

OK.

On Wed, Sep 21, 2016 at 10:46 AM, Jakub Jelinek  wrote:
> Hi!
>
> The following patch fixes some ICEs which were because of missing
> RejectNegative for the *aligned-new= options - they have their "negative"
> values as the option argument, so the only options that should allow
> negative forms are -Wno-aligned-new and -fno-aligned-new, not
> -Wno-aligned-new=none or -fno-aligned-new=32.
>
> Another thing is that sometimes the -Waligned-new warning suggests to use
> -faligned-new even when it is already on, that looks just too weird (it can
> be e.g. because some class has custom operator new defined in it, but not with
> std::align_val_t argument).
>
> Plus, I believe aligned_new_threshhold is a spelling typo, too many h chars.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2016-09-21  Jakub Jelinek  
>
> PR c++/77651
> c-family/
> * c.opt (Waligned-new=): Add RejectNegative.
> (faligned-new=): Likewise.  Spelling fix - change
> aligned_new_threshhold to aligned_new_threshold.
> * c-cppbuiltin.c (c_cpp_builtins): Change aligned_new_threshhold
> to aligned_new_threshold.
> cp/
> * init.c (build_new_1): Don't suggest to use -faligned-new if
> aligned_new_threshold is non-zero.
> (type_has_new_extended_alignment): Change aligned_new_threshhold
> to aligned_new_threshold.
> * call.c (second_parm_is_size_t, aligned_allocation_fn_p,
> aligned_deallocation_fn_p, build_op_delete_call): Likewise.
> * decl.c (cxx_init_decl_processing): Likewise.
> testsuite/
> * g++.dg/cpp1z/aligned-new6.C: New test.
>
> --- gcc/c-family/c.opt.jj   2016-09-21 08:54:14.0 +0200
> +++ gcc/c-family/c.opt  2016-09-21 10:52:01.468493875 +0200
> @@ -288,7 +288,7 @@ C++ ObjC++ Alias(Waligned-new=,global,no
>  Warn about 'new' of type with extended alignment without -faligned-new.
>
>  Waligned-new=
> -C++ ObjC++ Var(warn_aligned_new) Enum(warn_aligned_new_level) Joined Warning 
> LangEnabledBy(C++ ObjC++,Wall,1,0)
> +C++ ObjC++ Var(warn_aligned_new) Enum(warn_aligned_new_level) Joined 
> RejectNegative Warning LangEnabledBy(C++ ObjC++,Wall,1,0)
>  -Waligned-new=all Warn even if 'new' uses a class member allocation function.
>
>  Wall
> @@ -1071,7 +1071,7 @@ C++ ObjC++ Alias(faligned-new=,1,0)
>  Support C++17 allocation of over-aligned types.
>
>  faligned-new=
> -C++ ObjC++ Joined Var(aligned_new_threshhold) UInteger Init(-1)
> +C++ ObjC++ Joined RejectNegative Var(aligned_new_threshold) UInteger Init(-1)
>  -faligned-new= Use C++17 over-aligned type allocation for alignments 
> greater than N.
>
>  fall-virtual
> --- gcc/c-family/c-cppbuiltin.c.jj  2016-09-13 10:43:54.0 +0200
> +++ gcc/c-family/c-cppbuiltin.c 2016-09-21 10:52:20.101258415 +0200
> @@ -944,11 +944,11 @@ c_cpp_builtins (cpp_reader *pfile)
> cpp_define (pfile, "__cpp_transactional_memory=210500");
>if (flag_sized_deallocation)
> cpp_define (pfile, "__cpp_sized_deallocation=201309");
> -  if (aligned_new_threshhold)
> +  if (aligned_new_threshold)
> {
>   cpp_define (pfile, "__cpp_aligned_new=201606");
>   cpp_define_formatted (pfile, "__STDCPP_DEFAULT_NEW_ALIGNMENT__=%d",
> -   aligned_new_threshhold);
> +   aligned_new_threshold);
> }
>  }
>/* Note that we define this for C as well, so that we know if
> --- gcc/cp/init.c.jj2016-09-14 23:49:03.0 +0200
> +++ gcc/cp/init.c   2016-09-21 11:06:57.953163363 +0200
> @@ -2574,8 +2574,8 @@ warn_placement_new_too_small (tree type,
>  bool
>  type_has_new_extended_alignment (tree t)
>  {
> -  return (aligned_new_threshhold
> - && TYPE_ALIGN_UNIT (t) > (unsigned)aligned_new_threshhold);
> +  return (aligned_new_threshold
> + && TYPE_ALIGN_UNIT (t) > (unsigned)aligned_new_threshold);
>  }
>
>  /* Generate code for a new-expression, including calling the "operator
> @@ -3026,8 +3026,9 @@ build_new_1 (vec **placemen
>"alignment %d", elt_type, TYPE_ALIGN_UNIT (elt_type));
>inform (input_location, "uses %qD, which does not have an alignment "
>   "parameter", alloc_fn);
> -  inform (input_location, "use %<-faligned-new%> to enable C++17 "
> - "over-aligned new support");
> +  if (!aligned_new_threshold)
> +   inform (input_location, "use %<-faligned-new%> to enable C++17 "
> +   "over-aligned new support");
>  }
>
>/* If we found a simple case of PLACEMENT_EXPR above, then copy it
> --- gcc/cp/call.c.jj2016-09-13 10:43:56.0 +0200
> +++ gcc/cp/call.c   2016-09-21 10:53:07.607658081 +0200
> @@ -5965,7 +5965,7 @@ second_parm_is_size_t (tree fn)
>t = TREE_CHAIN (t);
>if (t == void_list_node)
>  return true;
> -  if (aligned_new_threshhold && t
> +  if 

Re: Add FALLTHRU to gimple-ssa-sprintf.c

[Martin Sebor]

On 09/21/2016 05:07 AM, Marek Polacek wrote:

On Wed, Sep 21, 2016 at 12:57:23PM +0200, Jakub Jelinek wrote:

On Wed, Sep 21, 2016 at 12:52:33PM +0200, Marek Polacek wrote:

Pointed out by Tobias.  This looks like a missing fallthru marker.

Ok?

2016-09-21  Marek Polacek  

* gimple-ssa-sprintf.c (pass_sprintf_length::compute_format_length):
Add falls through comment.


This is obvious.


I wanted to give Martin a chance to comment, maybe it should've been
a break, but I'll commit this now.  Thanks.


Yes, falling through is intentional here and should have probably
been better documented.  Thanks for taking care of it!

Martin

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

[Szabolcs Nagy]

On 21/09/16 15:37, Alexander Monakov wrote:
> On Wed, 21 Sep 2016, Martin Sebor wrote:
>> On 09/21/2016 01:11 AM, Markus Trippelsdorf wrote:
>>> The patch uses "nul" instead of "null" throughout.
>>
>> Yes, that's intentional.  NUL and null are alternate spellings for
>> the same character. I went with nul to distinguish it from the null
>> pointer and used all lowercase as per the GCC convention.
> 
> Can you elaborate which guideline suggests spelling that in lowercase?

the c standard calls it "null character".

> It seems quite strange to me, especially given that the documentation
> added with the patch uses "NUL character" (which I believe to be a more
> common form), but then warnings use "nul" (without the "character" iiuc).
> 
> Thanks.
> Alexander
> 

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

[Florian Weimer]

* John David Anglin:

> On Tue, Sep 20, 2016 at 09:27:17PM +0200, Florian Weimer wrote:
>> * 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.
>
> I'm not sure I understand the comment.  I created a short testcase.
> I agree that the issue is hppa specific but the extended alignment
> is coming from the __lock field in pthread_mutex_t.  I'm not sure
> why this affects new std::ofstream().

Ahh, thanks for the explanation.  The C++ streams probably have some
internal locks (hidden behind libstdc++ abstractions).

> The alignment of 16 arises in code that used the ldcw instruction.
> Although this could be avoided in glibc there are numerous other
> packages with objects requiring 16-byte alignment.  So, I'm tending
> to think the typedef for max_align_t should be adjusted on hppa-linux
> so that it has 16-byte alignment.  Is that the correct approach?

Yes, and for the warning to go away, GCC's MALLOC_ABI_ALIGNMENT needs
to be increased as well, I think.

Nowadays, we can change the glibc malloc alignment fairly easily, but
interposed mallocs won't be so nice.  But it is unlikely that any of
this matters in practice because the issue is not new (only the
warning is).

I have Cc:ed a few people who might have ideas how to deal with this.

Torvald, would it be possible to align mutexes internally on hppa, to
avoid the 16-byte alignment of the entire struct (that is, store a
pointer to the actual mutex object, which points to a sub-region of
the struct which is suitably aligned)?

We probably could add libstdc++ compile-time tests which make sure
that all relevant C++ types can be allocated with the global operator
new.

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

[Bernd Edlinger]

On 09/21/16 16:38, Richard Earnshaw (lists) 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
>
> Surely, based on the logic of the previous ifdefs, this line should read
>   + || (HARD_FRAME_POINTER_IS_FRAME_POINTER && frame_pointer_needed
>

I think the block above does not match this logic, it was also modified
recently.  Why is it handling FP and HFP, but then only HFP ?

   /* Don't clobber traceback for noreturn functions.  */
   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);
 }


Bernd.

Re: [PATCH v2][AArch64] Fix symbol offset limit

[Wilco Dijkstra]

ping

    
Wilco wrote:    
> The original example is from GCC itself, the fixed_regs array is small but 
> due to
> optimization we can end up with _regs + 0x.

We could also check the bounds of each symbol if they exist, like the patch 
below.


In aarch64_classify_symbol symbols are allowed full-range offsets on 
relocations.
This means the offset can use all of the +/-4GB offset, leaving no offset 
available
for the symbol itself.  This results in relocation overflow and link-time errors
for simple expressions like _char + 0xff00.

To avoid this, limit the offset to +/-1GB so that the symbol needs to be within 
a
3GB offset from its references.  For the tiny code model use a 64KB offset, 
allowing
most of the 1MB range for code/data between the symbol and its references.
For symbols with a defined size, limit the offset to be within the size of the 
symbol.

ChangeLog:
2016-09-12  Wilco Dijkstra  

    gcc/
    * config/aarch64/aarch64.c (aarch64_classify_symbol):
    Apply reasonable limit to symbol offsets.

    testsuite/
    * gcc.target/aarch64/symbol-range.c (foo): Set new limit.
    * gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 
385bd560fb12cd5d404e6ddb2f01edf1fe72d729..275a828ac9e6e9b8187380c1b602ffb1b2bcfb21
 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9351,6 +9351,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
   if (aarch64_tls_symbol_p (x))
 return aarch64_classify_tls_symbol (x);
 
+  const_tree decl = SYMBOL_REF_DECL (x);
+
   switch (aarch64_cmodel)
 {
 case AARCH64_CMODEL_TINY:
@@ -9359,25 +9361,45 @@ aarch64_classify_symbol (rtx x, rtx offset)
  we have no way of knowing the address of symbol at compile time
  so we can't accurately say if the distance between the PC and
  symbol + offset is outside the addressible range of +/-1M in the
-    TINY code model.  So we rely on images not being greater than
-    1M and cap the offset at 1M and anything beyond 1M will have to
-    be loaded using an alternative mechanism.  Furthermore if the
-    symbol is a weak reference to something that isn't known to
-    resolve to a symbol in this module, then force to memory.  */
+    TINY code model.  So we limit the maximum offset to +/-64KB and
+    assume the offset to the symbol is not larger than +/-(1M - 64KB).
+    Furthermore force to memory if the symbol is a weak reference to
+    something that doesn't resolve to a symbol in this module.  */
   if ((SYMBOL_REF_WEAK (x)
    && !aarch64_symbol_binds_local_p (x))
- || INTVAL (offset) < -1048575 || INTVAL (offset) > 1048575)
+ || !IN_RANGE (INTVAL (offset), -0x1, 0x1))
 return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available.  */
+ if (decl && DECL_P (decl))
+   {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+   return SYMBOL_FORCE_TO_MEM;
+   }
+
   return SYMBOL_TINY_ABSOLUTE;
 
 case AARCH64_CMODEL_SMALL:
   /* Same reasoning as the tiny code model, but the offset cap here is
-    4G.  */
+    1G, allowing +/-3G for the offset to the symbol.  */
   if ((SYMBOL_REF_WEAK (x)
    && !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (INTVAL (offset), HOST_WIDE_INT_C (-4294967263),
-   HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (INTVAL (offset), -0x4000, 0x4000))
 return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available.  */
+ if (decl && DECL_P (decl))
+   {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+   return SYMBOL_FORCE_TO_MEM;
+   }
+
   return SYMBOL_SMALL_ABSOLUTE;
 
 case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c 
b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index 
d7e46b059e41f2672b3a1da5506fa8944e752e01..d399a3637ed834ddc4bb429594c4ec229b5c2ea8
 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
 /* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
 
-int fixed_regs[0x0020];
+char fixed_regs[0x0020];
 
 int
-foo()
+main()
 {
-  return fixed_regs[0x0008];
+  return fixed_regs[0x000ff000];
 }
 
 /* { 

[PATCH] Fix ICE with inlining and #pragma omp simd (PR fortran/77665)

[Jakub Jelinek]

Hi!

The simduid pass uses the cfun->has_simduid_loops flag to determine if it
needs to clean up any left-over GOMP_SIMD_* internal functions.
During inlining, we set the flag if we inline some loop with simduid, or if
we find GOMP_SIMD_ORDERED_* internal call, but the testcase shows that we
need to do that for the other GOMP_SIMD_* ifns too, because what we inline
might have turned the loop->simduid loops into non-loops (or they might not
exist from the beginning because simd region had noreturn body).

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

2016-09-21  Jakub Jelinek  

PR fortran/77665
* tree-inline.c (remap_gimple_stmt): Set has_simduid_loops
for all IFN_GOMP_SIMD_* internal fns, not just for
IFN_GOMP_SIMD_ORDERED_*.

* gfortran.dg/gomp/pr77665.f90: New test.

--- gcc/tree-inline.c.jj2016-08-06 12:11:57.0 +0200
+++ gcc/tree-inline.c   2016-09-21 13:40:23.153732520 +0200
@@ -1644,11 +1644,19 @@ remap_gimple_stmt (gimple *stmt, copy_bo
gimple_call_set_tail (call_stmt, false);
  if (gimple_call_from_thunk_p (call_stmt))
gimple_call_set_from_thunk (call_stmt, false);
- if (gimple_call_internal_p (call_stmt)
- && IN_RANGE (gimple_call_internal_fn (call_stmt),
-  IFN_GOMP_SIMD_ORDERED_START,
-  IFN_GOMP_SIMD_ORDERED_END))
-   DECL_STRUCT_FUNCTION (id->dst_fn)->has_simduid_loops = true;
+ if (gimple_call_internal_p (call_stmt))
+   switch (gimple_call_internal_fn (call_stmt))
+ {
+ case IFN_GOMP_SIMD_LANE:
+ case IFN_GOMP_SIMD_VF:
+ case IFN_GOMP_SIMD_LAST_LANE:
+ case IFN_GOMP_SIMD_ORDERED_START:
+ case IFN_GOMP_SIMD_ORDERED_END:
+   DECL_STRUCT_FUNCTION (id->dst_fn)->has_simduid_loops = true;
+   break;
+ default:
+   break;
+ }
}
 
   /* Remap the region numbers for __builtin_eh_{pointer,filter},
--- gcc/testsuite/gfortran.dg/gomp/pr77665.f90.jj   2016-09-21 
13:50:34.304948175 +0200
+++ gcc/testsuite/gfortran.dg/gomp/pr77665.f90  2016-09-21 13:51:01.163605047 
+0200
@@ -0,0 +1,18 @@
+! PR fortran/77665
+! { dg-do compile }
+! { dg-additional-options "-O2" }
+
+program pr77665
+  type t
+integer :: a = 0
+  end type
+  type(t) :: x
+  integer :: i
+  !$omp declare reduction (+:t: omp_out%a = omp_out%a + omp_in%a)
+  !$omp simd reduction(+:x)
+  do i = 1, 8
+if (abs(i) < 5) call abort
+x%a = x%a + 1
+  end do
+  print *, x%a
+end

Jakub

[C++ PATCH] Aligned new option handling fixes (PR c++/77651)

[Jakub Jelinek]

Hi!

The following patch fixes some ICEs which were because of missing
RejectNegative for the *aligned-new= options - they have their "negative"
values as the option argument, so the only options that should allow
negative forms are -Wno-aligned-new and -fno-aligned-new, not
-Wno-aligned-new=none or -fno-aligned-new=32.

Another thing is that sometimes the -Waligned-new warning suggests to use
-faligned-new even when it is already on, that looks just too weird (it can
be e.g. because some class has custom operator new defined in it, but not with
std::align_val_t argument).

Plus, I believe aligned_new_threshhold is a spelling typo, too many h chars.

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

2016-09-21  Jakub Jelinek  

PR c++/77651
c-family/
* c.opt (Waligned-new=): Add RejectNegative.
(faligned-new=): Likewise.  Spelling fix - change
aligned_new_threshhold to aligned_new_threshold.
* c-cppbuiltin.c (c_cpp_builtins): Change aligned_new_threshhold
to aligned_new_threshold.
cp/
* init.c (build_new_1): Don't suggest to use -faligned-new if
aligned_new_threshold is non-zero.
(type_has_new_extended_alignment): Change aligned_new_threshhold
to aligned_new_threshold.
* call.c (second_parm_is_size_t, aligned_allocation_fn_p,
aligned_deallocation_fn_p, build_op_delete_call): Likewise.
* decl.c (cxx_init_decl_processing): Likewise.
testsuite/
* g++.dg/cpp1z/aligned-new6.C: New test.

--- gcc/c-family/c.opt.jj   2016-09-21 08:54:14.0 +0200
+++ gcc/c-family/c.opt  2016-09-21 10:52:01.468493875 +0200
@@ -288,7 +288,7 @@ C++ ObjC++ Alias(Waligned-new=,global,no
 Warn about 'new' of type with extended alignment without -faligned-new.
 
 Waligned-new=
-C++ ObjC++ Var(warn_aligned_new) Enum(warn_aligned_new_level) Joined Warning 
LangEnabledBy(C++ ObjC++,Wall,1,0)
+C++ ObjC++ Var(warn_aligned_new) Enum(warn_aligned_new_level) Joined 
RejectNegative Warning LangEnabledBy(C++ ObjC++,Wall,1,0)
 -Waligned-new=all Warn even if 'new' uses a class member allocation function.
 
 Wall
@@ -1071,7 +1071,7 @@ C++ ObjC++ Alias(faligned-new=,1,0)
 Support C++17 allocation of over-aligned types.
 
 faligned-new=
-C++ ObjC++ Joined Var(aligned_new_threshhold) UInteger Init(-1)
+C++ ObjC++ Joined RejectNegative Var(aligned_new_threshold) UInteger Init(-1)
 -faligned-new= Use C++17 over-aligned type allocation for alignments 
greater than N.
 
 fall-virtual
--- gcc/c-family/c-cppbuiltin.c.jj  2016-09-13 10:43:54.0 +0200
+++ gcc/c-family/c-cppbuiltin.c 2016-09-21 10:52:20.101258415 +0200
@@ -944,11 +944,11 @@ c_cpp_builtins (cpp_reader *pfile)
cpp_define (pfile, "__cpp_transactional_memory=210500");
   if (flag_sized_deallocation)
cpp_define (pfile, "__cpp_sized_deallocation=201309");
-  if (aligned_new_threshhold)
+  if (aligned_new_threshold)
{
  cpp_define (pfile, "__cpp_aligned_new=201606");
  cpp_define_formatted (pfile, "__STDCPP_DEFAULT_NEW_ALIGNMENT__=%d",
-   aligned_new_threshhold);
+   aligned_new_threshold);
}
 }
   /* Note that we define this for C as well, so that we know if
--- gcc/cp/init.c.jj2016-09-14 23:49:03.0 +0200
+++ gcc/cp/init.c   2016-09-21 11:06:57.953163363 +0200
@@ -2574,8 +2574,8 @@ warn_placement_new_too_small (tree type,
 bool
 type_has_new_extended_alignment (tree t)
 {
-  return (aligned_new_threshhold
- && TYPE_ALIGN_UNIT (t) > (unsigned)aligned_new_threshhold);
+  return (aligned_new_threshold
+ && TYPE_ALIGN_UNIT (t) > (unsigned)aligned_new_threshold);
 }
 
 /* Generate code for a new-expression, including calling the "operator
@@ -3026,8 +3026,9 @@ build_new_1 (vec **placemen
   "alignment %d", elt_type, TYPE_ALIGN_UNIT (elt_type));
   inform (input_location, "uses %qD, which does not have an alignment "
  "parameter", alloc_fn);
-  inform (input_location, "use %<-faligned-new%> to enable C++17 "
- "over-aligned new support");
+  if (!aligned_new_threshold)
+   inform (input_location, "use %<-faligned-new%> to enable C++17 "
+   "over-aligned new support");
 }
 
   /* If we found a simple case of PLACEMENT_EXPR above, then copy it
--- gcc/cp/call.c.jj2016-09-13 10:43:56.0 +0200
+++ gcc/cp/call.c   2016-09-21 10:53:07.607658081 +0200
@@ -5965,7 +5965,7 @@ second_parm_is_size_t (tree fn)
   t = TREE_CHAIN (t);
   if (t == void_list_node)
 return true;
-  if (aligned_new_threshhold && t
+  if (aligned_new_threshold && t
   && same_type_p (TREE_VALUE (t), align_type_node)
   && TREE_CHAIN (t) == void_list_node)
 return true;
@@ -5978,7 +5978,7 @@ second_parm_is_size_t (tree fn)
 bool
 aligned_allocation_fn_p (tree t)
 {
-  if (!aligned_new_threshhold)
+  if 

Re: [PATCH][AArch64] Improve stack adjustment

[Wilco Dijkstra]

ping


From: Wilco Dijkstra
Sent: 23 August 2016 15:49
To: Richard Earnshaw; GCC Patches
Cc: nd
Subject: Re: [PATCH][AArch64] Improve stack adjustment
    
ping
    
Richard Earnshaw wrote:
> I see you've added a default argument for your new parameter.  I think
> doing that is fine, but I have two comments about how we might use that
> in this case.
> 
> Firstly, if this parameter is suitable for having a default value, then
> I think the preceding one should also be treated in the same way.
> 
> Secondly, I think (due to these parameters being BOOL with no useful
> context to make it clear which is which) that having wrapper functions
> (inlined, of course) that describe the intended behaviour more clearly
> would be useful.  So, defining
> 
> static inline void
> aarch64_add_frame_constant (mode, regnum, scratchreg, delta)
> {
>    aarch64_add_frame_constant (mode, regnum, scratchreg, delta, true);
> }
> 
> would make it clearer at the call point than having a lot of true and
> false parameters scattered round the code.
> 
> Alternatively we might remove all the default parameters and require
> wrappers like the above to make it more explicit which API is intended -
> this might make more sense if not all combinations make sense.

OK here is the new version using simple wrapper functions and no
default parameters:

ChangeLog:
2016-08-10  Wilco Dijkstra  

gcc/
    * config/aarch64/aarch64.c (aarch64_add_constant_internal):
    Add extra argument to allow emitting the move immediate.
    Use add/sub with positive immediate.
    (aarch64_add_constant): Add inline function.
    (aarch64_add_sp): Likewise.
    (aarch64_sub_sp): Likewise.
    (aarch64_expand_prologue): Call aarch64_sub_sp.
    (aarch64_expand_epilogue): Call aarch64_add_sp.
    Decide when to leave out move.
    (aarch64_output_mi_thunk): Call aarch64_add_constant.

testsuite/
    * gcc.target/aarch64/test_frame_17.c: New test.
--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 
b8536175a84b76f8c2939e61f1379ae279b20d43..5a25fce17785af9f9dc12e0f2a9609af09af0b35
 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -1941,15 +1941,21 @@ aarch64_expand_mov_immediate (rtx dest, rtx imm)
 }
 
 /* Add DELTA to REGNUM in mode MODE.  SCRATCHREG can be used to held
-   intermediate value if necessary.
+   intermediate value if necessary.  FRAME_RELATED_P should be true if
+   the RTX_FRAME_RELATED flag should be set and CFA adjustments added
+   to the generated instructions.  If SCRATCHREG is known to hold
+   abs (delta), EMIT_MOVE_IMM can be set to false to avoid emitting the
+   immediate again.
 
-   This function is sometimes used to adjust the stack pointer, so we must
-   ensure that it can never cause transient stack deallocation by writing an
-   invalid value into REGNUM.  */
+   Since this function may be used to adjust the stack pointer, we must
+   ensure that it cannot cause transient stack deallocation (for example
+   by first incrementing SP and then decrementing when adjusting by a
+   large immediate).  */
 
 static void
-aarch64_add_constant (machine_mode mode, int regnum, int scratchreg,
- HOST_WIDE_INT delta, bool frame_related_p)
+aarch64_add_constant_internal (machine_mode mode, int regnum, int scratchreg,
+  HOST_WIDE_INT delta, bool frame_related_p,
+  bool emit_move_imm)
 {
   HOST_WIDE_INT mdelta = abs_hwi (delta);
   rtx this_rtx = gen_rtx_REG (mode, regnum);
@@ -1967,11 +1973,11 @@ aarch64_add_constant (machine_mode mode, int regnum, 
int scratchreg,
   return;
 }
 
-  /* We need two add/sub instructions, each one performing part of the
- calculation.  Don't do this if the addend can be loaded into register with
- a single instruction, in that case we prefer a move to a scratch register
- following by an addition.  */
-  if (mdelta < 0x100 && !aarch64_move_imm (delta, mode))
+  /* We need two add/sub instructions, each one perform part of the
+ addition/subtraction, but don't this if the addend can be loaded into
+ register by single instruction, in that case we prefer a move to scratch
+ register following by addition.  */
+  if (mdelta < 0x100 && !aarch64_move_imm (mdelta, mode))
 {
   HOST_WIDE_INT low_off = mdelta & 0xfff;
 
@@ -1985,8 +1991,10 @@ aarch64_add_constant (machine_mode mode, int regnum, int 
scratchreg,
 
   /* Otherwise use generic function to handle all other situations.  */
   rtx scratch_rtx = gen_rtx_REG (mode, scratchreg);
-  aarch64_internal_mov_immediate (scratch_rtx, GEN_INT (delta), true, mode);
-  insn = emit_insn (gen_add2_insn (this_rtx, scratch_rtx));
+  if (emit_move_imm)
+    aarch64_internal_mov_immediate (scratch_rtx, GEN_INT (mdelta), true, mode);
+  insn = emit_insn (delta < 0 ? gen_sub2_insn (this_rtx, scratch_rtx)
+   

Re: [PATCH][AArch64] Align FP callee-saves

[Wilco Dijkstra]

ping


From: Wilco Dijkstra
Sent: 08 September 2016 14:35
To: GCC Patches
Cc: nd
Subject: [PATCH][AArch64] Align FP callee-saves
    
If the number of integer callee-saves is odd, the FP callee-saves use 8-byte 
aligned
LDP/STP.  Since 16-byte alignment may be faster on some CPUs, align the FP
callee-saves to 16 bytes and use the alignment gap for the last FP callee-save 
when
possible. Besides slightly different offsets for FP callee-saves, the generated 
code
doesn't change.

Bootstrap and regression pass, OK for commit?


ChangeLog:
2016-09-08  Wilco Dijkstra  

    * config/aarch64/aarch64.c (aarch64_layout_frame):
    Align FP callee-saves.
--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 
fed3b6e803821392194dc34a6c3df5f653d2e33e..075b3802c72a68f63b47574e19186e7ce3440b28
 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -2735,7 +2735,7 @@ static void
 aarch64_layout_frame (void)
 {
   HOST_WIDE_INT offset = 0;
-  int regno;
+  int regno, last_fp_reg = INVALID_REGNUM;
 
   if (reload_completed && cfun->machine->frame.laid_out)
 return;
@@ -2781,7 +2781,10 @@ aarch64_layout_frame (void)
   for (regno = V0_REGNUM; regno <= V31_REGNUM; regno++)
 if (df_regs_ever_live_p (regno)
 && !call_used_regs[regno])
-  cfun->machine->frame.reg_offset[regno] = SLOT_REQUIRED;
+  {
+   cfun->machine->frame.reg_offset[regno] = SLOT_REQUIRED;
+   last_fp_reg = regno;
+  }
 
   if (cfun->machine->frame.emit_frame_chain)
 {
@@ -2805,9 +2808,21 @@ aarch64_layout_frame (void)
 offset += UNITS_PER_WORD;
   }
 
+  HOST_WIDE_INT max_int_offset = offset;
+  offset = ROUND_UP (offset, STACK_BOUNDARY / BITS_PER_UNIT);
+  bool has_align_gap = offset != max_int_offset;
+
   for (regno = V0_REGNUM; regno <= V31_REGNUM; regno++)
 if (cfun->machine->frame.reg_offset[regno] == SLOT_REQUIRED)
   {
+   /* If there is an alignment gap between integer and fp callee-saves,
+  allocate the last fp register to it if possible.  */
+   if (regno == last_fp_reg && has_align_gap && (offset & 8) == 0)
+ {
+   cfun->machine->frame.reg_offset[regno] = max_int_offset;
+   break;
+ }
+
 cfun->machine->frame.reg_offset[regno] = offset;
 if (cfun->machine->frame.wb_candidate1 == INVALID_REGNUM)
   cfun->machine->frame.wb_candidate1 = regno;

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

[Richard Earnshaw (lists)]

On 13/09/16 12:35, Wilco Dijkstra wrote:
> Jakub wrote:
>> On Mon, Sep 12, 2016 at 04:19:32PM +, Tamar Christina wrote:
>>> 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.
>>
>> Is it generally preferable to use integer operations for this instead
>> of floating point operations?  I mean various targets have quite high costs
>> of moving data in between the general purpose and floating point register
>> file, often it has to go through memory etc.
> 
> It is generally preferable indeed - there was a *very* long discussion about 
> integer
> vs FP on the GLIBC mailing list when I updated math.h to use the GCC builtins 
> a
> while back (the GLIBC implementation used a non-inlined unoptimized integer
> implementation, so an inlined FP implementation seemed a good intermediate 
> solution).
> 
> Integer operations are generally lower latency and enable bit manipulation 
> tricks like the
> fast early exit. The FP version requires execution of 5 branches for a 
> "normal" FP value
> and loads several floating point immediates. There are also many targets with 
> emulated
> floating point types, so 5 calls to the comparison lib function would be 
> seriously slow.
> Note using so many FP comparisons is not just slow but they aren't correct 
> for signalling
> NaNs, so this patch also fixes bug 66462 for fpclassify.

And don't forget that getting the results of a floating-point comparison
back to the branch unit may be no faster than transferring the value in
the first place.

R.

> 
> I would suggest someone with access to a machine with slow FP moves (POWER?)
> to benchmark this using the fpclassify test 
> (glibc/benchtests/bench-math-inlines.c)
> so we know for sure.
> 
> Wilco
> 

Re: [PATCH][AArch64 - v3] Simplify eh_return implementation

[Wilco Dijkstra]

ping


From: Wilco Dijkstra
Sent: 02 September 2016 12:31
To: Ramana Radhakrishnan; GCC Patches
Cc: nd
Subject: Re: [PATCH][AArch64 - v3] Simplify eh_return implementation
    
Ramana Radhakrishnan wrote:
> Can you please file a PR for this and add some testcases ?  This sounds like 
> a serious enough problem that needs to be looked at probably going back since 
> the dawn of time.

I've created PR77455. Updated patch below:

This patch simplifies the handling of the EH return value.  We force the use of 
the
frame pointer so the return location is always at FP + 8.  This means we can 
emit
a simple volatile access in EH_RETURN_HANDLER_RTX without needing md
patterns, splitters and frame offset calculations.  The new implementation also
fixes various bugs in aarch64_final_eh_return_addr, which does not work with
-fomit-frame-pointer, alloca or outgoing arguments.

Bootstrap OK, GCC Regression OK, OK for trunk? Would it be useful to backport
this to GCC6.x?

ChangeLog:

2016-09-02  Wilco Dijkstra  

    PR77455
gcc/
    * config/aarch64/aarch64.md (eh_return): Remove pattern and splitter.
    * config/aarch64/aarch64.h (AARCH64_EH_STACKADJ_REGNUM): Remove.
    (EH_RETURN_HANDLER_RTX): New define.
    * config/aarch64/aarch64.c (aarch64_frame_pointer_required):
    Force frame pointer in EH return functions.
    (aarch64_expand_epilogue): Add barrier for eh_return.
    (aarch64_final_eh_return_addr): Remove.
    (aarch64_eh_return_handler_rtx): New function.
    * config/aarch64/aarch64-protos.h (aarch64_final_eh_return_addr):
    Remove.
    (aarch64_eh_return_handler_rtx): New prototype.

testsuite/
    * gcc.target/aarch64/eh_return.c: New test.
--
diff --git a/gcc/config/aarch64/aarch64-protos.h 
b/gcc/config/aarch64/aarch64-protos.h
index 
3cdd69b8af1089a839e5d45cda94bc70a15cd777..327c0a97f6f687604afef249b79ac22628418070
 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -358,7 +358,7 @@ int aarch64_hard_regno_mode_ok (unsigned, machine_mode);
 int aarch64_hard_regno_nregs (unsigned, machine_mode);
 int aarch64_uxt_size (int, HOST_WIDE_INT);
 int aarch64_vec_fpconst_pow_of_2 (rtx);
-rtx aarch64_final_eh_return_addr (void);
+rtx aarch64_eh_return_handler_rtx (void);
 rtx aarch64_mask_from_zextract_ops (rtx, rtx);
 const char *aarch64_output_move_struct (rtx *operands);
 rtx aarch64_return_addr (int, rtx);
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 
003fec87e41db618570663f28cc2387a87e8252a..fa81e4b853daf08842955288861ec7e7acca
 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -400,9 +400,9 @@ extern unsigned aarch64_architecture_version;
 #define ASM_DECLARE_FUNCTION_NAME(STR, NAME, DECL)  \
   aarch64_declare_function_name (STR, NAME, DECL)
 
-/* The register that holds the return address in exception handlers.  */
-#define AARCH64_EH_STACKADJ_REGNUM (R0_REGNUM + 4)
-#define EH_RETURN_STACKADJ_RTX gen_rtx_REG (Pmode, AARCH64_EH_STACKADJ_REGNUM)
+/* For EH returns X4 contains the stack adjustment.  */
+#define EH_RETURN_STACKADJ_RTX gen_rtx_REG (Pmode, R4_REGNUM)
+#define EH_RETURN_HANDLER_RTX  aarch64_eh_return_handler_rtx ()
 
 /* Don't use __builtin_setjmp until we've defined it.  */
 #undef DONT_USE_BUILTIN_SETJMP
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 
e742c19d76e6c62117aa62a990b9c2945aa06b74..f07d771ea343803e054e03f59c8c1efb698bf474
 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -2739,6 +2739,10 @@ aarch64_frame_pointer_required (void)
   && (!crtl->is_leaf || df_regs_ever_live_p (LR_REGNUM)))
 return true;
 
+  /* Force a frame pointer for EH returns so the return address is at FP+8.  */
+  if (crtl->calls_eh_return)
+    return true;
+
   return false;
 }
 
@@ -3298,7 +3302,8 @@ aarch64_expand_epilogue (bool for_sibcall)
  + cfun->machine->frame.saved_varargs_size) != 0;
 
   /* Emit a barrier to prevent loads from a deallocated stack.  */
-  if (final_adjust > crtl->outgoing_args_size || cfun->calls_alloca)
+  if (final_adjust > crtl->outgoing_args_size || cfun->calls_alloca
+  || crtl->calls_eh_return)
 {
   emit_insn (gen_stack_tie (stack_pointer_rtx, stack_pointer_rtx));
   need_barrier_p = false;
@@ -3366,52 +3371,15 @@ aarch64_expand_epilogue (bool for_sibcall)
 emit_jump_insn (ret_rtx);
 }
 
-/* Return the place to copy the exception unwinding return address to.
-   This will probably be a stack slot, but could (in theory be the
-   return register).  */
+/* Implement EH_RETURN_HANDLER_RTX.  The return address is stored at FP + 8.
+   The access needs to be volatile to prevent it from being removed.  */
 rtx
-aarch64_final_eh_return_addr (void)
+aarch64_eh_return_handler_rtx (void)
 {
-  HOST_WIDE_INT fp_offset;
-
-  aarch64_layout_frame ();
-
-  fp_offset = 

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

[John David Anglin]

On Tue, Sep 20, 2016 at 09:27:17PM +0200, Florian Weimer wrote:
> * 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.

I'm not sure I understand the comment.  I created a short testcase.
I agree that the issue is hppa specific but the extended alignment
is coming from the __lock field in pthread_mutex_t.  I'm not sure
why this affects new std::ofstream().

The alignment of 16 arises in code that used the ldcw instruction.
Although this could be avoided in glibc there are numerous other
packages with objects requiring 16-byte alignment.  So, I'm tending
to think the typedef for max_align_t should be adjusted on hppa-linux
so that it has 16-byte alignment.  Is that the correct approach?

[committed] Cherry-pick libasan C++17 aligned new support

[Jakub Jelinek]

Hi!

Upstream has accepted my PR77567 changes, I've bootstrapped/regtested its
backport on x86_64-linux and committed to trunk.

2016-09-21  Jakub Jelinek  

PR sanitizer/77567
* asan/asan_new_delete.cc: Cherry-pick upstream r282019.

--- libsanitizer/asan/asan_new_delete.cc.jj 2016-02-08 21:59:45.0 
+0100
+++ libsanitizer/asan/asan_new_delete.cc2016-09-12 21:58:53.266981094 
+0200
@@ -27,17 +27,30 @@
 
 using namespace __asan;  // NOLINT
 
+// FreeBSD prior v9.2 have wrong definition of 'size_t'.
+// http://svnweb.freebsd.org/base?view=revision=232261
+#if SANITIZER_FREEBSD && SANITIZER_WORDSIZE == 32
+#include 
+#if __FreeBSD_version <= 902001  // v9.2
+#define size_t unsigned
+#endif  // __FreeBSD_version
+#endif  // SANITIZER_FREEBSD && SANITIZER_WORDSIZE == 32
+
 // This code has issues on OSX.
 // See https://code.google.com/p/address-sanitizer/issues/detail?id=131.
 
-// Fake std::nothrow_t to avoid including .
+// Fake std::nothrow_t and std::align_val_t to avoid including .
 namespace std {
 struct nothrow_t {};
+enum class align_val_t: size_t {};
 }  // namespace std
 
 #define OPERATOR_NEW_BODY(type) \
   GET_STACK_TRACE_MALLOC;\
   return asan_memalign(0, size, , type);
+#define OPERATOR_NEW_BODY_ALIGN(type) \
+  GET_STACK_TRACE_MALLOC;\
+  return asan_memalign((uptr)align, size, , type);
 
 // On OS X it's not enough to just provide our own 'operator new' and
 // 'operator delete' implementations, because they're going to be in the
@@ -47,15 +60,6 @@ struct nothrow_t {};
 // To make sure that C++ allocation/deallocation operators are overridden on
 // OS X we need to intercept them using their mangled names.
 #if !SANITIZER_MAC
-// FreeBSD prior v9.2 have wrong definition of 'size_t'.
-// http://svnweb.freebsd.org/base?view=revision=232261
-#if SANITIZER_FREEBSD && SANITIZER_WORDSIZE == 32
-#include 
-#if __FreeBSD_version <= 902001  // v9.2
-#define size_t unsigned
-#endif  // __FreeBSD_version
-#endif  // SANITIZER_FREEBSD && SANITIZER_WORDSIZE == 32
-
 CXX_OPERATOR_ATTRIBUTE
 void *operator new(size_t size) { OPERATOR_NEW_BODY(FROM_NEW); }
 CXX_OPERATOR_ATTRIBUTE
@@ -66,6 +70,18 @@ void *operator new(size_t size, std::not
 CXX_OPERATOR_ATTRIBUTE
 void *operator new[](size_t size, std::nothrow_t const&)
 { OPERATOR_NEW_BODY(FROM_NEW_BR); }
+CXX_OPERATOR_ATTRIBUTE
+void *operator new(size_t size, std::align_val_t align)
+{ OPERATOR_NEW_BODY_ALIGN(FROM_NEW); }
+CXX_OPERATOR_ATTRIBUTE
+void *operator new[](size_t size, std::align_val_t align)
+{ OPERATOR_NEW_BODY_ALIGN(FROM_NEW_BR); }
+CXX_OPERATOR_ATTRIBUTE
+void *operator new(size_t size, std::align_val_t align, std::nothrow_t const&)
+{ OPERATOR_NEW_BODY_ALIGN(FROM_NEW); }
+CXX_OPERATOR_ATTRIBUTE
+void *operator new[](size_t size, std::align_val_t align, std::nothrow_t 
const&)
+{ OPERATOR_NEW_BODY_ALIGN(FROM_NEW_BR); }
 
 #else  // SANITIZER_MAC
 INTERCEPTOR(void *, _Znwm, size_t size) {
@@ -113,6 +129,32 @@ void operator delete[](void *ptr, size_t
   GET_STACK_TRACE_FREE;
   asan_sized_free(ptr, size, , FROM_NEW_BR);
 }
+CXX_OPERATOR_ATTRIBUTE
+void operator delete(void *ptr, std::align_val_t) NOEXCEPT {
+  OPERATOR_DELETE_BODY(FROM_NEW);
+}
+CXX_OPERATOR_ATTRIBUTE
+void operator delete[](void *ptr, std::align_val_t) NOEXCEPT {
+  OPERATOR_DELETE_BODY(FROM_NEW_BR);
+}
+CXX_OPERATOR_ATTRIBUTE
+void operator delete(void *ptr, std::align_val_t, std::nothrow_t const&) {
+  OPERATOR_DELETE_BODY(FROM_NEW);
+}
+CXX_OPERATOR_ATTRIBUTE
+void operator delete[](void *ptr, std::align_val_t, std::nothrow_t const&) {
+  OPERATOR_DELETE_BODY(FROM_NEW_BR);
+}
+CXX_OPERATOR_ATTRIBUTE
+void operator delete(void *ptr, size_t size, std::align_val_t) NOEXCEPT {
+  GET_STACK_TRACE_FREE;
+  asan_sized_free(ptr, size, , FROM_NEW);
+}
+CXX_OPERATOR_ATTRIBUTE
+void operator delete[](void *ptr, size_t size, std::align_val_t) NOEXCEPT {
+  GET_STACK_TRACE_FREE;
+  asan_sized_free(ptr, size, , FROM_NEW_BR);
+}
 
 #else  // SANITIZER_MAC
 INTERCEPTOR(void, _ZdlPv, void *ptr) {

Jakub

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

[Richard Earnshaw (lists)]

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

Surely, based on the logic of the previous ifdefs, this line should read
 + || (HARD_FRAME_POINTER_IS_FRAME_POINTER && frame_pointer_needed

R.

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

[Alexander Monakov]

On Wed, 21 Sep 2016, Martin Sebor wrote:
> On 09/21/2016 01:11 AM, Markus Trippelsdorf wrote:
> > The patch uses "nul" instead of "null" throughout.
> 
> Yes, that's intentional.  NUL and null are alternate spellings for
> the same character. I went with nul to distinguish it from the null
> pointer and used all lowercase as per the GCC convention.

Can you elaborate which guideline suggests spelling that in lowercase?
It seems quite strange to me, especially given that the documentation
added with the patch uses "NUL character" (which I believe to be a more
common form), but then warnings use "nul" (without the "character" iiuc).

Thanks.
Alexander

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

[Martin Sebor]

On 09/21/2016 01:11 AM, Markus Trippelsdorf wrote:

On 2016.09.11 at 20:03 -0600, Martin Sebor wrote:

On 09/08/2016 04:10 PM, Joseph Myers wrote:

On Thu, 8 Sep 2016, Martin Sebor wrote:



PR middle-end/49905 - Better sanity checking on sprintf src & dest to
produce warning for dodgy code


The patch uses "nul" instead of "null" throughout.


Yes, that's intentional.  NUL and null are alternate spellings for
the same character. I went with nul to distinguish it from the null
pointer and used all lowercase as per the GCC convention.

Martin

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

[Martin Sebor]

On 09/21/2016 04:36 AM, Christophe Lyon wrote:

Hi Martin,

On 21 September 2016 at 09:11, Markus Trippelsdorf
 wrote:

On 2016.09.11 at 20:03 -0600, Martin Sebor wrote:

On 09/08/2016 04:10 PM, Joseph Myers wrote:

On Thu, 8 Sep 2016, Martin Sebor wrote:



PR middle-end/49905 - Better sanity checking on sprintf src & dest to
  produce warning for dodgy code


The patch uses "nul" instead of "null" throughout.

--
Markus


I've noticed that some of the new tests fail on arm linux:
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 1220)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 1270)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 1381)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 815)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 816)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 817)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 820)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 821)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 824)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 825)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 828)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 829)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 832)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 833)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (test for excess errors)
  gcc.dg/tree-ssa/builtin-sprintf-warn-3.c  (test for warnings, line 46)
  gcc.dg/tree-ssa/builtin-sprintf-warn-3.c  (test for warnings, line 47)
  gcc.dg/tree-ssa/builtin-sprintf-warn-3.c (test for excess errors)
  gcc.dg/tree-ssa/builtin-sprintf.c (test for excess errors)

on aarch64 and arm bare-metal targets (using newlib), I've noticed
these failures:
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 99)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (test for excess errors)
  gcc.dg/tree-ssa/builtin-sprintf-warn-2.c sprintf transformed into
strcpy (test for warnings, line 83)
  gcc.dg/tree-ssa/builtin-sprintf-warn-2.c sprintf transformed into
strcpy (test for warnings, line 84)
  gcc.dg/tree-ssa/builtin-sprintf-warn-4.c (test for excess errors)
  gcc.dg/tree-ssa/builtin-sprintf.c execution test

You may download the results from
http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/240298/report-build-info.html


Thanks, I'll look into these today.

Martin

Re: Implement -Wimplicit-fallthrough (version 9)

[Jason Merrill]

On 09/21/2016 08:44 AM, Marek Polacek wrote:

@@ -10733,12 +10758,35 @@ cp_parser_expression_statement (cp_parser* parser, 
tree in_statement_expr)
  statement.  */
   if (cp_lexer_next_token_is_not (parser->lexer, CPP_SEMICOLON))
 {
+  /* This might be attribute fallthrough.  */
+  if (cp_lexer_next_token_is_keyword (parser->lexer, RID_ATTRIBUTE))
{
+ tree attr = cp_parser_gnu_attributes_opt (parser);
+ if (maybe_attribute_fallthrough_p (attr))
+   statement = build_call_expr_internal_loc (token->location,
+ IFN_FALLTHROUGH,
+ void_type_node, 0);
+ else if (cp_lexer_next_token_is (parser->lexer, CPP_SEMICOLON))
+   {
+ cp_parser_error (parser, "only attribute % "
+  "can be applied to a null statement");
+ return error_mark_node;
+   }
+ else
+   {
+ cp_parser_error (parser, "expected primary-expression");
+ return error_mark_node;
+   }
+   }


Attributes that we don't handle should be warnings, not errors.  Like we 
do for C++11 attributes in cp_parser_statement, we should first parse 
attributes, then parse the expression-statement, then give any 
appropriate warnings.


Jason

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

[Richard Biener]

On Tue, Sep 20, 2016 at 5:25 PM, Bin Cheng  wrote:
> 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?

Seeing

+  /* Infer index length from segment length.  */
+  unsigned HOST_WIDE_INT idx_len1 = (seg_len1 + abs_step - 1) / abs_step;
+  unsigned HOST_WIDE_INT idx_len2 = (seg_len2 + abs_step - 1) / abs_step;

Does this really work for DR_STEP that is bigger than 1 in index space?  The
segment length is generally vectorization-factor * DR_STEP but index space
is -- if you are talking about array-ref indexes -- the segment length divided
by the array element size.  Note that what the index space refers to for
a given DR_ACCESS_FN depends on whether this is from an ARRAY_REF
(element size) or the base pointer evolution (bytes).  I suppose looking at
the access functions type might distinguish the pointer vs. array index case.

What I'm looking for is a big comment on why the above "translation" is
correct.

Thanks,
Richard.

> 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.

Re: [PATCH] Implement C++17 node extraction and insertion (P0083R5)

[Jonathan Wakely]

This mail got rejected from gcc-patches as spam. The attachment can be
found at https://gcc.gnu.org/ml/libstdc++/2016-09/msg00115.html


On 21/09/16 14:48 +0100, Jonathan Wakely wrote:

This implements container node extraction/insertion, and merging. The
patch includes Debug Mode support and pretty printers for the node
handles.

Most of the changes are fairly straightforward, with two things worth
pointing out.

There's a FIXME in bits/hashtable.h due to an exception-safety issue.
If the hash function or equality predicate throws then the node is
destroyed and deallocated. It would be better to leave it unchanged in
the node_handle argument.

I didn't want to make all map and multimap specializations friends of
each other (and similarly for all sets and multisets, and again for
the unordered ones). That would make it too easy to accidentally
access the internals of a map from a map. So I defined the
_Rb_tree_merge_helper and _Hash_merge_helper class templates to
mediate access, so that any access to a "foreign" container type must
be done through that type, and only certain internals can be obtained.

I'll probably commit this tomorrow.

* doc/xml/manual/status_cxx2017.xml: Document status.
* doc/html/*: Regenerate.
* include/Makefile.am: Add bits/node_handle.h and reorder.
* include/Makefile.in: Regenerate.
* include/bits/hashtable.h (_Hashtable::node_type)
(_Hashtable::insert_return_type, _Hashtable::_M_reinsert_node)
(_Hashtable::_M_reinsert_node_multi, _Hashtable::extract)
(_Hashtable::_M_merge_unique, _Hashtable::_M_merge_multi): Define.
(_Hash_merge_helper): Define primary template.
* include/bits/node_handle.h: New header.
* include/bits/stl_map.h (map): Declare _Rb_tree_merge_helper as
friend.
(map::node_type, map::insert_return_type, map::extract, map::merge)
(map::insert(node_type&&), map::insert(const_iterator, node_type&&)):
Define new members.
(_Rb_tree_merge_helper): Specialize for map.
* include/bits/stl_multimap.h (multimap): Declare _Rb_tree_merge_helper
as friend.
(multimap::node_type, multimap::extract, multimap::merge)
(multimap::insert(node_type&&))
(multimap::insert(const_iterator, node_type&&)): Define.
(_Rb_tree_merge_helper): Specialize for multimap.
* include/bits/stl_multiset.h (multiset): Declare _Rb_tree_merge_helper
as friend.
(multiset::node_type, multiset::extract, multiset::merge)
(multiset::insert(node_type&&))
(multiset::insert(const_iterator, node_type&&)): Define.
* include/bits/stl_set.h (set): Declare _Rb_tree_merge_helper as
friend.
(set::node_type, set::insert_return_type, set::extract, set::merge)
(set::insert(node_type&&), set::insert(const_iterator, node_type&&)):
Define.
(_Rb_tree_merge_helper): Specialize for set.
* include/bits/stl_tree.h (_Rb_tree): Declare _Rb_tree<> as friend.
(_Rb_tree::node_type, _Rb_tree::insert_return_type)
(_Rb_tree::_M_reinsert_node_unique, _Rb_tree::_M_reinsert_node_equal)
(_Rb_tree::_M_reinsert_node_hint_unique)
(_Rb_tree::_M_reinsert_node_hint_equal, _Rb_tree::extract)
(_Rb_tree::_M_merge_unique, _Rb_tree::_M_merge_equal): Define.
(_Rb_tree_merge_helper): Specialize for multiset.
* include/bits/unordered_map.h (unordered_map): Declare
unordered_map<> and unordered_multimap<> as friends.
(unordered_map::node_type, unordered_map::insert_return_type)
(unordered_map::extract, unordered_map::merge)
(unordered_map::insert(node_type&&))
(unordered_map::insert(const_iterator, node_type&&))
(unordered_multimap): Declare _Hash_merge_helper as friend.
(unordered_multimap::node_type, unordered_multimap::extract)
(unordered_multimap::merge, unordered_multimap::insert(node_type&&))
(unordered_multimap::insert(const_iterator, node_type&&)): Define.
(_Hash_merge_helper): Specialize for unordered maps and multimaps.
* include/bits/unordered_set.h (unordered_set, unordered_multiset):
Declare _Hash_merge_helper as friend.
(unordered_set::node_type, unordered_set::insert_return_type)
(unordered_set::extract, unordered_set::merge)
(unordered_set::insert(node_type&&))
(unordered_set::insert(const_iterator, node_type&&)): Define.
(unordered_multiset::node_type, unordered_multiset::extract)
(unordered_multiset::merge, unordered_multiset::insert(node_type&&))
(unordered_multiset::insert(const_iterator, node_type&&)): Define.
(_Hash_merge_helper): Specialize for unordered sets and multisets.
* include/debug/map.h (map): Add using declarations or forwarding
functions for new members.
* include/debug/map.h (multimap): Likewise.
* 

Re: [patch] Fix ICE on ACATS test for Aarch64 at -O

[Eric Botcazou]

> Is this really safe.  If I look at where we call
> expand_shift/expand_shift_1 I get real worried that we could end up
> using that NULL value in a context where it's not expected.
>
> So while emit_store_flag_force knows what to do upon NULL return, I'm
> not sure the other users of expand_shift/expand_shift_1 do.

Yes, that's probably risky, and there are a lot of calls to them.

> Is there any way to address this problem in emit_store_flag_force?

Now that we're in C++, we could add an additional argument to expand_shift_1 
with a default value and call the function directly from emit_store_flag with 
the non-default value, thus instructing it to return 0 on failure instead of 
aborting.  emit_store_flag already returns 0 on failure so the result would be 
naturally propagated up to emit_store_flag_force.

-- 
Eric Botcazou

Re: [PATCH 1/17][ARM] Add ARMv8.2-A command line option and profile.

[Ramana Radhakrishnan]

On Mon, Jul 4, 2016 at 2:46 PM, Matthew Wahab
 wrote:
> On 17/05/16 15:22, Matthew Wahab wrote:
>> This patch adds the command options for the architecture ARMv8.2-A and
>> the half-precision extension. The architecture is selected by
>> -march=armv8.2-a and has all the properties of -march=armv8.1-a.
>>
>> This patch also enables the CRC extension (+crc) which is required
>> for both ARMv8.2-A and ARMv8.1-A architectures but is not currently
>> enabled by default for -march=armv8.1-a.
>>
>> The half-precision extension is selected using the extension +fp16. This
>> enables the VFP FP16 instructions if an ARMv8 VFP unit is also
>> specified, e.g. by -mfpu=fp-armv8. It also enables the FP16 NEON
>> instructions if an ARMv8 NEON unit is specified, e.g. by
>> -mfpu=neon-fp-armv8. Note that if the NEON FP16 instructions are enabled
>> then so are the VFP FP16 instructions.
>
> This a minor respin that moves the setting of arm_fp16_inst in
> arm_option_override to immediately before it is used to set the required
> arm_fp16_format.
>
> Tested the series for arm-none-linux-gnueabihf with native bootstrap and
> make check and for arm-none-eabi and armeb-none-eabi with make check on
> an ARMv8.2-A emulator.


OK.

Thanks,
Ramana
>
> 2016-07-04  Matthew Wahab  
>
>
> * config/arm/arm-arches.def ("armv8.1-a"): Add FL_CRC32.
> ("armv8.2-a"): New.
> ("armv8.2-a+fp16"): New.
> * config/arm/arm-protos.h (FL2_ARCH8_2): New.
> (FL2_FP16INST): New.
> (FL2_FOR_ARCH8_2A): New.
> * config/arm/arm-tables.opt: Regenerate.
> * config/arm/arm.c (arm_arch8_2): New.
> (arm_fp16_inst): New.
> (arm_option_override): Set arm_arch8_2 and arm_fp16_inst.  Check
> for incompatible fp16-format settings.
> * config/arm/arm.h (TARGET_VFP_FP16INST): New.
> (TARGET_NEON_FP16INST): New.
> (arm_arch8_2): Declare.
> (arm_fp16_inst): Declare.
> * config/arm/bpabi.h (BE8_LINK_SPEC): Add entries for
> march=armv8.2-a and march=armv8.2-a+fp16.
> * config/arm/t-aprofile (Arch Matches): Add entries for armv8.2-a
> and armv8.2-a+fp16.
> * doc/invoke.texi (ARM Options): Add "-march=armv8.1-a",
> "-march=armv8.2-a" and "-march=armv8.2-a+fp16".
>

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

[Trevor Saunders]

On Tue, Sep 20, 2016 at 11:48:04PM -0400, Eric Gallager wrote:
> 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:

Well, the goal is to reduce conditional compilation so no #if either,
and in other contexts undefined doesn't implicitly convert, so it
shouldn't be as important.  It might be good to do, but I suspect you'd
have to fix things up, and it just doesn't seem as important as other
things that can be done.

Trev

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

Re: [ARM] FP16 ARM Alternative format variants of AAPCS tests.

[Matthew Wahab]

On 03/08/16 12:43, Ramana Radhakrishnan wrote:

On Mon, Jun 27, 2016 at 11:09 AM, Matthew Wahab
 wrote:


Tests added for FP16 argument and return values being passed in
registers only check the case when the FP16 IEEE format is used. This
patch adds equivalent tests that also check the behaviour when the
ARM Alternative format is used.


[..]

testsuite/
2016-06-27  Matthew Wahab  

 * gcc.target/arm/fp16-aapcs-3.c: New.
 * gcc.target/arm/fp16-aapcs-4.c: New.
 * gcc.target/arm/aapcs/aapcs/vfp22.c: New.
 * gcc.target/arm/aapcs/aapcs/vfp23.c: New.
 * gcc.target/arm/aapcs/aapcs/vfp24.c: New.
 * gcc.target/arm/aapcs/aapcs/vfp25.c: New.


OK once the pre-reqs are in place.

Thanks,
Ramana




Committed as r240314 after checking the new tests with cross-compiled 
arm-none-linux-gnueabihf. Sorry for the delay.


Matthew

[PATCH] Refactor section/label init for early LTO debug

[Richard Biener]

The following patch ports a refactoring of section/label in it from
the early LTO debug work to trunk.  For early LTO debug we need to
be able to emit two sets of debug infos into two sets of different
sections - early LTO into .gnu.debuglto_ prefixed sections and
regular (early + late) debug for the FAT part of the object.

Thus this preparation splits out the section and label generation
from dwarf2out_init moving the text section related stuff to 
dwarf2out_assembly_start and the rest to a new function
init_sections_and_labels which is now called only before we start
outputting dwarf (in dwarf2out_finish).  It also removes some
dwarf_split_debug_info checks from the macro section name defines
(in the end I'll have up to four variants - regular, regular LTO,
DWO, DWO LTO).

And it removes an unused function.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.  I've
also bootstrapped with -O2 -g3 to exercise the .debug_macro path.

Ok?

Just noticed that DEBUG_STR_OFFSETS_SECTION needs similar massaging
for its dwarf_split_debug_info condition - will followup as obvious
if this one is approved.

Thanks,
Richard.

2016-09-21  Richard Biener  

* dwarf2out.c (stripattributes): Remove unused function.
(DEBUG_NORM_MACINFO_SECTION): Rename to DEBUG_MACINFO_SECTION.
Push dwarf_split_debug_info handling into init_sections_and_labels.
(DEBUG_NORM_MACRO_SECTION): Likewise to DEBUG_MACRO_SECTION.
(DEBUG_MACRO_SECTION_FLAGS): Remove.
(debug_macinfo_section_name): New global.
(output_macinfo): Use debug_macinfo_section_name.
(init_sections_and_labels): Split out section and label generation
from dwarf2out_init.  Set debug_macinfo_section_name.
(dwarf2out_init): Move text section label generation and emission
to ...
(dwarf2out_assembly_start): ... here.
(dwarf2out_finish): Call init_sections_and_labels before DWARF
output starts.

Index: gcc/dwarf2out.c
===
--- gcc/dwarf2out.c (revision 240303)
+++ gcc/dwarf2out.c (working copy)
@@ -159,6 +159,7 @@ static GTY(()) section *debug_skeleton_a
 static GTY(()) section *debug_aranges_section;
 static GTY(()) section *debug_addr_section;
 static GTY(()) section *debug_macinfo_section;
+static const char *debug_macinfo_section_name;
 static GTY(()) section *debug_line_section;
 static GTY(()) section *debug_skeleton_line_section;
 static GTY(()) section *debug_loc_section;
@@ -265,7 +266,6 @@ static GTY(()) dw_die_ref decltype_auto_
 
 /* Forward declarations for functions defined in this file.  */
 
-static char *stripattributes (const char *);
 static void output_call_frame_info (int);
 static void dwarf2out_note_section_used (void);
 
@@ -410,24 +410,6 @@ should_emit_struct_debug (tree type, enu
   return DUMP_GSTRUCT (type, usage, criterion, generic, false, false);
 }
 
-/* Return a pointer to a copy of the section string name S with all
-   attributes stripped off, and an asterisk prepended (for assemble_name).  */
-
-static inline char *
-stripattributes (const char *s)
-{
-  char *stripped = XNEWVEC (char, strlen (s) + 2);
-  char *p = stripped;
-
-  *p++ = '*';
-
-  while (*s && *s != ',')
-*p++ = *s++;
-
-  *p = '\0';
-  return stripped;
-}
-
 /* Switch [BACK] to eh_frame_section.  If we don't have an eh_frame_section,
switch to the data section instead, and write out a synthetic start label
for collect2 the first time around.  */
@@ -3514,27 +3496,17 @@ new_addr_loc_descr (rtx addr, enum dtpre
 #ifndef DEBUG_ADDR_SECTION
 #define DEBUG_ADDR_SECTION ".debug_addr"
 #endif
-#ifndef DEBUG_NORM_MACINFO_SECTION
-#define DEBUG_NORM_MACINFO_SECTION ".debug_macinfo"
+#ifndef DEBUG_MACINFO_SECTION
+#define DEBUG_MACINFO_SECTION ".debug_macinfo"
 #endif
 #ifndef DEBUG_DWO_MACINFO_SECTION
 #define DEBUG_DWO_MACINFO_SECTION  ".debug_macinfo.dwo"
 #endif
-#ifndef DEBUG_MACINFO_SECTION
-#define DEBUG_MACINFO_SECTION   \
-  (!dwarf_split_debug_info  \
-   ? (DEBUG_NORM_MACINFO_SECTION) : (DEBUG_DWO_MACINFO_SECTION))
-#endif
-#ifndef DEBUG_NORM_MACRO_SECTION
-#define DEBUG_NORM_MACRO_SECTION ".debug_macro"
-#endif
 #ifndef DEBUG_DWO_MACRO_SECTION
 #define DEBUG_DWO_MACRO_SECTION".debug_macro.dwo"
 #endif
 #ifndef DEBUG_MACRO_SECTION
-#define DEBUG_MACRO_SECTION \
-  (!dwarf_split_debug_info  \
-   ? (DEBUG_NORM_MACRO_SECTION) : (DEBUG_DWO_MACRO_SECTION))
+#define DEBUG_MACRO_SECTION".debug_macro"
 #endif
 #ifndef DEBUG_LINE_SECTION
 #define DEBUG_LINE_SECTION ".debug_line"
@@ -3580,10 +3552,6 @@ new_addr_loc_descr (rtx addr, enum dtpre
 #define TEXT_SECTION_NAME  ".text"
 #endif
 
-/* Section flags for .debug_macinfo/.debug_macro section.  */
-#define 

RE: [PATCH] Disable -mbranch-likely for -Os when targetting generic architecture

[Matthew Fortune]

Robert Suchanek  writes:
> Hi Catherine,
> 
> Apologies for the (very) late reply.
> It appears that I never replied to the last message.
> 
> > > gcc/
> > >   * config/mips/mips-cpus.def: Replace PTF_AVOID_BRANCHLIKELY with
> > >   PTF_AVOID_BRANCHLIKELY_ALWAYS for generic architecture and with
> > >   PTF_AVOID_BRANCHLIKELY_SPEED for others.
> > >   (mips2, mips3, mips4): Add PTF_AVOID_BRANCHLIKELY_SIZE to tune
> > > flags.
> > >   * config/mips/mips.c (mips_option_override): Enable the branch
> > > likely
> > >   depending on the tune flags and optimization level.
> > >   * config/mips/mips.h (PTF_AVOID_BRANCHLIKELY): Remove.
> > >   (PTF_AVOID_BRANCHLIKELY_SPEED): Define.
> > >   (PTF_AVOID_BRANCHLIKELY_SIZE): Likewise.
> > >   (PTF_AVOID_BRANCHLIKELY_ALWAYS): Likewise.
> > > ---
> > >  gcc/config/mips/mips-cpus.def | 56
> > > +---
> > > ---
> > >  gcc/config/mips/mips.c|  6 +++--
> > >  gcc/config/mips/mips.h| 20 
> > >  3 files changed, 47 insertions(+), 35 deletions(-)
> > >
> > > a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index
> > > 0e0ecf2..f8775c4
> > > 100644
> > > --- a/gcc/config/mips/mips.c
> > > +++ b/gcc/config/mips/mips.c
> > > @@ -17916,8 +17916,10 @@ mips_option_override (void)
> > >if ((target_flags_explicit & MASK_BRANCHLIKELY) == 0)
> > >  {
> > >if (ISA_HAS_BRANCHLIKELY
> > > -   && (optimize_size
> > > -   || (mips_tune_info->tune_flags & PTF_AVOID_BRANCHLIKELY)
> > > == 0))
> > > +   && ((optimize_size && (mips_tune_info->tune_flags
> > > +  & PTF_AVOID_BRANCHLIKELY_SIZE) == 0)
> > > +|| (!optimize_size && (mips_tune_info->tune_flags
> > > +   & PTF_AVOID_BRANCHLIKELY_SPEED) ==
> > > 0)))
> > >   target_flags |= MASK_BRANCHLIKELY;
> > >else
> > >   target_flags &= ~MASK_BRANCHLIKELY;
> >
> > Should this check be:
> > Index: mips.c
> > ===
> > --- mips.c  (revision 229138)
> > +++ mips.c  (working copy)
> > @@ -17758,8 +17758,15 @@
> >if ((target_flags_explicit & MASK_BRANCHLIKELY) == 0)
> >  {
> >if (ISA_HAS_BRANCHLIKELY
> > - && (optimize_size
> > - || (mips_tune_info->tune_flags & PTF_AVOID_BRANCHLIKELY)
> == 0))
> > + && ((optimize_size
> > +  && (mips_tune_info->tune_flags
> > +  & PTF_AVOID_BRANCHLIKELY_SIZE) == 0)
> > + || (!optimize_size
> > +&& optimize > 0
> > +&& ((mips_tune_info->tune_flags
> > + & PTF_AVOID_BRANCHLIKELY_SPEED) == 0))
> > +|| (mips_tune_info->tune_flags
> > + & PTF_AVOID_BRANCHLIKELY_ALWAYS) == 0))
> > target_flags |= MASK_BRANCHLIKELY;
> >else
> > target_flags &= ~MASK_BRANCHLIKELY;
> >
> > Instead?  I don't see a use of PTF_AVOID_BRANCH_ALWAYS in your patch,
> > but it seems like it should be checked.
> >
> 
> I did that on purpose at the time as the check looked redundant as it
> will be one or the other.  However, for easier reading and a potential
> redefinition of *_ALWAYS e.g. to a unique value then the extra check is
> a must.
> 
> I'm happy to include this.  Ok to commit with this change?

This looks like it got lost at some point. I think this is a reasonable
change for safety.

Go ahead and commit.

Thanks,
Matthew

Re: Implement -Wimplicit-fallthrough (version 9)

[Marek Polacek]

On Tue, Sep 20, 2016 at 12:26:11PM -0400, Jason Merrill wrote:
> 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.

All right.  I moved the attribute handling to cp_parser_expression_statement
in the following patch.  Anything else?  Thanks,

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

2016-09-21  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_expression_statement): 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): Add %< %> quotes.
* 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 test.
* c-c++-common/Wimplicit-fallthrough-20.c: New test.
* c-c++-common/Wimplicit-fallthrough-21.c: New test.
* c-c++-common/Wimplicit-fallthrough-2.c: New test.
* c-c++-common/Wimplicit-fallthrough-3.c: New test.
* c-c++-common/Wimplicit-fallthrough-4.c: New test.
* c-c++-common/Wimplicit-fallthrough-5.c: New test.
* c-c++-common/Wimplicit-fallthrough-6.c: New test.
* c-c++-common/Wimplicit-fallthrough-7.c: New test.
* c-c++-common/Wimplicit-fallthrough-8.c: New test.
* c-c++-common/Wimplicit-fallthrough-9.c: New test.
* 

Re: [PATCH] Fix JUMP_LABEL documentation

[Bernd Schmidt]

On 09/20/2016 08:51 PM, Segher Boessenkool wrote:


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


Ok, thanks.


Bernd

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.

FYI, with --param inline-unit-growth=12 --param early-inlining-insns=14,
the code size reduction is only 1.1%.

Yuan, Pengfei

> Richard.
> 
> > Richard.

Re: [RFC][IRA]Initialize ira_use_lra_p early by moving the initialization into ira_init_once ().

[Bernd Schmidt]

On 09/21/2016 01:46 PM, Renlin Li wrote:

* ira.c (ira): Move ira_use_lra_p initialization code to ...
(ira_init_once): Here.


LGTM.


Bernd

[RFC][IRA]Initialize ira_use_lra_p early by moving the initialization into ira_init_once ().

[Renlin Li]

Hi,

ira_use_lra_p is a global variable use in ira as well as 
backend_init_target ().
It's fine to be used in IRA as it's will be initialized at the beginning 
of ira pass.


However, early in backend_init_target (), this variable may not be 
initialized yet. There is a check in backend_init_target ():


'''
if (!ira_use_lra_p)
  init_reload ();
'''

In this case, init_reload () will always be called if ira_use_lra_p is 
not initialized.


ira_init_once () is a better place for the initialization.
It's called early in initialize_rtl (), just before
backend_init_target ().
And as the name suggests, it's called once to initialize function 
independent data structure.


aarch64-none-elf regression test Okay, x86-64-linux bootstrap Okay.

Regards,
Renlin

gcc/ChangeLog:

2016-09-21  Renlin Li  

* ira.c (ira): Move ira_use_lra_p initialization code to ...
(ira_init_once): Here.
diff --git a/gcc/ira.c b/gcc/ira.c
index f8a59e3..9e7ba52 100644
--- a/gcc/ira.c
+++ b/gcc/ira.c
@@ -1665,6 +1665,8 @@ ira_init_once (void)
 {
   ira_init_costs_once ();
   lra_init_once ();
+
+  ira_use_lra_p = targetm.lra_p ();
 }
 
 /* Free ira_max_register_move_cost, ira_may_move_in_cost and
@@ -5082,7 +5084,6 @@ ira (FILE *f)
 
   ira_conflicts_p = optimize > 0;
 
-  ira_use_lra_p = targetm.lra_p ();
   /* If there are too many pseudos and/or basic blocks (e.g. 10K
  pseudos and 10K blocks or 100K pseudos and 1K blocks), we will
  use simplified and faster algorithms in LRA.  */

Re: Make regcprop to eliminate noop moves better

[Eric Botcazou]

> while working on the GCN port I ended up with many redundant register copies
> of the form
>  mov reg, exec
>  do something
>  mov reg, exec
>  do something
>  ...
> these copies are generated by LRA because exec is small register class and
> needs a lot of reloading (it could be improved too, but I do not care
> because I want to handle exec specially later anyway).
> 
> I was however suprised this garbage survives postreload optimizations.  It
> is easy to fix in regcprop which already does some noop copy elimination,
> but only of the for mov reg, reg after substituting.

Right, this ought to be dealt with during postreload CSE, there is roughly the 
same code as yours:

/* See whether a single set SET is a noop.  */
static int
reload_cse_noop_set_p (rtx set)
{
  if (cselib_reg_set_mode (SET_DEST (set)) != GET_MODE (SET_DEST (set)))
return 0;

  return rtx_equal_for_cselib_p (SET_DEST (set), SET_SRC (set));
}

Any idea about why this doesn't work in your case?

-- 
Eric Botcazou

Re: Add FALLTHRU to gimple-ssa-sprintf.c

[Jakub Jelinek]

On Wed, Sep 21, 2016 at 01:07:20PM +0200, Marek Polacek wrote:
> On Wed, Sep 21, 2016 at 12:57:23PM +0200, Jakub Jelinek wrote:
> > On Wed, Sep 21, 2016 at 12:52:33PM +0200, Marek Polacek wrote:
> > > Pointed out by Tobias.  This looks like a missing fallthru marker.
> > > 
> > > Ok?
> > > 
> > > 2016-09-21  Marek Polacek  
> > > 
> > >   * gimple-ssa-sprintf.c (pass_sprintf_length::compute_format_length):
> > >   Add falls through comment.
> > 
> > This is obvious.
> 
> I wanted to give Martin a chance to comment, maybe it should've been
> a break, but I'll commit this now.  Thanks.

Well, generally it isn't obvious, but in this case IMHO it is, in the light
of the comment:
  /* Handle a sole '%' character the same as "%%" but since it's
 undefined prevent the result from being folded.  */
case '\0':
  --pf;
  res->bounded = false;
case '%':
  spec.fmtfunc = format_percent;
  break;
so, what it does for "...%" is that it decreases the char pointer, so it
acts almost like "...%%" - spec.specifier = *pf++; will be also '%' and
pf afterwards will point after it.

Jakub

Re: Add FALLTHRU to gimple-ssa-sprintf.c

[Marek Polacek]

On Wed, Sep 21, 2016 at 12:57:23PM +0200, Jakub Jelinek wrote:
> On Wed, Sep 21, 2016 at 12:52:33PM +0200, Marek Polacek wrote:
> > Pointed out by Tobias.  This looks like a missing fallthru marker.
> > 
> > Ok?
> > 
> > 2016-09-21  Marek Polacek  
> > 
> > * gimple-ssa-sprintf.c (pass_sprintf_length::compute_format_length):
> > Add falls through comment.
> 
> This is obvious.

I wanted to give Martin a chance to comment, maybe it should've been
a break, but I'll commit this now.  Thanks.

Marek

Re: Add FALLTHRU to gimple-ssa-sprintf.c

[Jakub Jelinek]

On Wed, Sep 21, 2016 at 12:52:33PM +0200, Marek Polacek wrote:
> Pointed out by Tobias.  This looks like a missing fallthru marker.
> 
> Ok?
> 
> 2016-09-21  Marek Polacek  
> 
>   * gimple-ssa-sprintf.c (pass_sprintf_length::compute_format_length):
>   Add falls through comment.

This is obvious.

> diff --git gcc/gimple-ssa-sprintf.c gcc/gimple-ssa-sprintf.c
> index 0afcf68..dddb026 100644
> --- gcc/gimple-ssa-sprintf.c
> +++ gcc/gimple-ssa-sprintf.c
> @@ -2260,6 +2260,7 @@ pass_sprintf_length::compute_format_length (const 
> call_info ,
>   case '\0':
> --pf;
> res->bounded = false;
> +   /* FALLTHRU */
>   case '%':
> spec.fmtfunc = format_percent;
> break;
> 
>   Marek

Jakub

Add FALLTHRU to gimple-ssa-sprintf.c

[Marek Polacek]

Pointed out by Tobias.  This looks like a missing fallthru marker.

Ok?

2016-09-21  Marek Polacek  

* gimple-ssa-sprintf.c (pass_sprintf_length::compute_format_length):
Add falls through comment.

diff --git gcc/gimple-ssa-sprintf.c gcc/gimple-ssa-sprintf.c
index 0afcf68..dddb026 100644
--- gcc/gimple-ssa-sprintf.c
+++ gcc/gimple-ssa-sprintf.c
@@ -2260,6 +2260,7 @@ pass_sprintf_length::compute_format_length (const 
call_info ,
case '\0':
  --pf;
  res->bounded = false;
+ /* FALLTHRU */
case '%':
  spec.fmtfunc = format_percent;
  break;

Marek

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

[Christophe Lyon]

Hi Martin,

On 21 September 2016 at 09:11, Markus Trippelsdorf
 wrote:
> On 2016.09.11 at 20:03 -0600, Martin Sebor wrote:
>> On 09/08/2016 04:10 PM, Joseph Myers wrote:
>> > On Thu, 8 Sep 2016, Martin Sebor wrote:
>
>> PR middle-end/49905 - Better sanity checking on sprintf src & dest to
>>   produce warning for dodgy code
>
> The patch uses "nul" instead of "null" throughout.
>
> --
> Markus

I've noticed that some of the new tests fail on arm linux:
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 1220)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 1270)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 1381)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 815)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 816)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 817)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 820)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 821)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 824)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 825)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 828)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 829)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 832)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 833)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (test for excess errors)
  gcc.dg/tree-ssa/builtin-sprintf-warn-3.c  (test for warnings, line 46)
  gcc.dg/tree-ssa/builtin-sprintf-warn-3.c  (test for warnings, line 47)
  gcc.dg/tree-ssa/builtin-sprintf-warn-3.c (test for excess errors)
  gcc.dg/tree-ssa/builtin-sprintf.c (test for excess errors)

on aarch64 and arm bare-metal targets (using newlib), I've noticed
these failures:
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (test for warnings, line 99)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (test for excess errors)
  gcc.dg/tree-ssa/builtin-sprintf-warn-2.c sprintf transformed into
strcpy (test for warnings, line 83)
  gcc.dg/tree-ssa/builtin-sprintf-warn-2.c sprintf transformed into
strcpy (test for warnings, line 84)
  gcc.dg/tree-ssa/builtin-sprintf-warn-4.c (test for excess errors)
  gcc.dg/tree-ssa/builtin-sprintf.c execution test

You may download the results from
http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/240298/report-build-info.html

Thanks,

Christophe

Re: [v3 PATCH] Implement LWG 2729 for pair.

[Jonathan Wakely]

On 17/08/16 07:41 +0300, Ville Voutilainen wrote:

   Implement LWG 2729 for pair.
   * include/bits/stl_pair.h (_PCC): New.
   (_ConstructiblePair, _ImplicitlyConvertiblePair):
   Turn into static member functions of _PCC.
   (_MoveConstructiblePair, _ImplicitlyMoveConvertiblePair): Likewise.
   (_PCCP): New.
   (pair(const _T1&, const _T2&)): Adjust.
   (_PCCFP): New.
   (pair(const pair<_U1, _U2>&)): Adjust.
   (pair(_U1&&, const _T2&)): Likewise.
   (pair(const _T1&, _U2&&)): Likewise.
   (pair(_U1&&, _U2&&)): Likewise.
   (pair(pair<_U1, _U2>&&)): Likewise.
   (operator=(const pair&)): Make conditionally deleted.
   (operator=(pair&&)): Make conditionally suppressed.
   (operator=(const pair<_U1, _U2>&)): Constrain.
   (operator=(pair<_U1, _U2>&&): Likewise.
   * include/std/type_traits (__nonesuch): New.
   * testsuite/20_util/pair/traits.cc: Likewise.


I found the "Likewise" a bit confusing here. I realise it means "New"
but my first thought was it meant you were adding __nonesuch to that
test file. "New" for the test would be unambiguous :-)


diff --git a/libstdc++-v3/testsuite/20_util/pair/traits.cc 
b/libstdc++-v3/testsuite/20_util/pair/traits.cc
new file mode 100644
index 000..0989cf1
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/pair/traits.cc
@@ -0,0 +1,77 @@
+// { dg-do compile }
+// { dg-options "-std=gnu++11" }


This patch predates my testsuite changes, so this should now be
{ dg-do compile { target c++11 } } 


Otherwise OK for trunk, thanks.

[Patch, fortran] Clean up of error recovery in DTIO procedures

[Paul Richard Thomas]

Dear All,

Please find attached a patch to clean up the various issues with
errors in DTIO procedures. The tests were all provided by Gerhard
Steinmetz for which thanks are due.

I intend to commit this patch as 'obvious' tomorrow morning unless
there are any objections in the meantime.

Bootstrapped and regtested on x86_64/FC21 - OK for trunk?

Paul

2016-09-21  Paul Thomas  

* interface.c (check_dtio_interface1): Introduce errors for
alternate returns and incorrect numbers of arguments.
(gfc_find_specific_dtio_proc): Return cleanly if the derived
type either doesn't exist or has no namespace.

2016-09-21  Paul Thomas  

* gfortran.dg/dtio_13.f90: New test.
Index: gcc/fortran/interface.c
===
*** gcc/fortran/interface.c (revision 240301)
--- gcc/fortran/interface.c (working copy)
*** check_dtio_interface1 (gfc_symbol *deriv
*** 4629,4635 
  
for (intr = tb_io_st->n.sym->generic; intr; intr = intr->next)
{
! if (intr->sym && intr->sym->formal
  && ((intr->sym->formal->sym->ts.type == BT_CLASS
   && CLASS_DATA (intr->sym->formal->sym)->ts.u.derived
 == derived)
--- 4629,4635 
  
for (intr = tb_io_st->n.sym->generic; intr; intr = intr->next)
{
! if (intr->sym && intr->sym->formal && intr->sym->formal->sym
  && ((intr->sym->formal->sym->ts.type == BT_CLASS
   && CLASS_DATA (intr->sym->formal->sym)->ts.u.derived
 == derived)
*** check_dtio_interface1 (gfc_symbol *deriv
*** 4639,4644 
--- 4639,4650 
  dtio_sub = intr->sym;
  break;
}
+ else if (intr->sym && intr->sym->formal && !intr->sym->formal->sym)
+   {
+ gfc_error ("Alternate return at %L is not permitted in a DTIO "
+"procedure", >sym->declared_at);
+ return;
+   }
}
  
if (dtio_sub == NULL)
*** check_dtio_interface1 (gfc_symbol *deriv
*** 4647,4655 
  
gcc_assert (dtio_sub);
if (!dtio_sub->attr.subroutine)
! gfc_error ("DTIO procedure %s at %L must be a subroutine",
   dtio_sub->name, _sub->declared_at);
  
/* Now go through the formal arglist.  */
arg_num = 1;
for (formal = dtio_sub->formal; formal; formal = formal->next, arg_num++)
--- 4653,4680 
  
gcc_assert (dtio_sub);
if (!dtio_sub->attr.subroutine)
! gfc_error ("DTIO procedure '%s' at %L must be a subroutine",
   dtio_sub->name, _sub->declared_at);
  
+   arg_num = 0;
+   for (formal = dtio_sub->formal; formal; formal = formal->next)
+ arg_num++;
+ 
+   if (arg_num < (formatted ? 6 : 4))
+ {
+   gfc_error ("Too few dummy arguments in DTIO procedure '%s' at %L",
+dtio_sub->name, _sub->declared_at);
+   return;
+ }
+ 
+   if (arg_num > (formatted ? 6 : 4))
+ {
+   gfc_error ("Too many dummy arguments in DTIO procedure '%s' at %L",
+dtio_sub->name, _sub->declared_at);
+   return;
+ }
+ 
+ 
/* Now go through the formal arglist.  */
arg_num = 1;
for (formal = dtio_sub->formal; formal; formal = formal->next, arg_num++)
*** check_dtio_interface1 (gfc_symbol *deriv
*** 4657,4662 
--- 4682,4695 
if (!formatted && arg_num == 3)
arg_num = 5;
fsym = formal->sym;
+ 
+   if (fsym == NULL)
+   {
+ gfc_error ("Alternate return at %L is not permitted in a DTIO "
+"procedure", _sub->declared_at);
+ return;
+   }
+ 
switch (arg_num)
{
case(1):/* DTV  */
*** gfc_find_specific_dtio_proc (gfc_symbol
*** 4823,4828 
--- 4856,4864 
for (extended = derived; extended;
 extended = gfc_get_derived_super_type (extended))
  {
+   if (extended == NULL || extended->ns == NULL)
+   return NULL;
+ 
if (formatted == true)
{
  if (write == true)
Index: gcc/testsuite/gfortran.dg/dtio_13.f90
===
*** gcc/testsuite/gfortran.dg/dtio_13.f90   (revision 0)
--- gcc/testsuite/gfortran.dg/dtio_13.f90   (working copy)
***
*** 0 
--- 1,144 
+ ! { dg-do compile }
+ ! { dg-options -std=legacy }
+ !
+ ! Test elimination of various segfaults and ICEs on error recovery.
+ !
+ ! Contributed by Gerhard Steinmetz  
+ !
+ module m1
+type t
+end type
+interface write(formatted)
+   module procedure s
+end interface
+ contains
+subroutine s(dtv,unit,iotype,vlist,extra,iostat,iomsg) ! { dg-error "Too 
many dummy arguments" }
+   

[PATCH, libgfortran] Use rand_s on MinGW-w64, fix bounds overflow

[Janne Blomqvist]

Hi,

per private mail I got a bug report about the new RANDOM_NUMBER not
working correctly on windows in certain situations. I think the reason
for this might be an array bounds overflow in the fallback code (in
case /dev/urandom doesn't exist), which this patch fixes.

Also, on Windows the equivalent of /dev/urandom is something called
CryptGenRandom. However, using that seems complicated enough that
without a Windows system to test on I don't want to go there. Slightly
newer versions of windows, however, seem to supply a "rand_s"
function, which ostensibly does the same as CryptGenRandom in a
somewhat easier to use fashion. Unfortunately it seems that this
function is not available in MinGW, but it's present in MinGW-w64, so
one needs a bit of macro guards to check for it. Which this patch
does.

Committed r240309 as obvious/lacking active GFortran windows maintainers.

2016-09-21  Janne Blomqvist  

* intrinsics/random.c (getosrandom): Use rand_s() on
MinGW-w64. Fix bounds overflow in fallback code.


-- 
Janne Blomqvist
diff --git a/libgfortran/intrinsics/random.c b/libgfortran/intrinsics/random.c
index 739dbeb..00f1cb1 100644
--- a/libgfortran/intrinsics/random.c
+++ b/libgfortran/intrinsics/random.c
@@ -24,6 +24,9 @@ a copy of the GCC Runtime Library Exception along with this program;
 see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 .  */
 
+/* For rand_s.  */
+#define _CRT_RAND_S
+
 #include "libgfortran.h"
 #include 
 #include 
@@ -43,6 +46,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 #ifdef __MINGW32__
 #define HAVE_GETPID 1
 #include 
+#include <_mingw.h> /* For __MINGW64_VERSION_MAJOR  */
 #endif
 
 extern void random_r4 (GFC_REAL_4 *);
@@ -281,7 +285,7 @@ jump (xorshift1024star_state* rs)
 #define Q 127773 /* M / A (To avoid overflow on A * seed) */
 #define R 2836   /* M % A (To avoid overflow on A * seed) */
 
-static uint32_t
+__attribute__((unused)) static uint32_t
 lcg_parkmiller(uint32_t seed)
 {
 uint32_t hi = seed / Q;
@@ -297,14 +301,21 @@ lcg_parkmiller(uint32_t seed)
 #undef Q
 #undef R
 
+
 /* Get some random bytes from the operating system in order to seed
the PRNG.  */
 
 static int
 getosrandom (void *buf, size_t buflen)
 {
-  /* TODO: On Windows one could use CryptGenRandom
-
+  /* rand_s is available in MinGW-w64 but not plain MinGW.  */
+#ifdef __MINGW64_VERSION_MAJOR
+  unsigned int* b = buf;
+  for (unsigned i = 0; i < buflen / sizeof (unsigned int); i++)
+rand_s ([i]);
+  return buflen;
+#else
+  /*
  TODO: When glibc adds a wrapper for the getrandom() system call
  on Linux, one could use that.
 
@@ -333,12 +344,13 @@ getosrandom (void *buf, size_t buflen)
   seed ^= pid;
 #endif
   uint32_t* ub = buf;
-  for (size_t i = 0; i < buflen; i++)
+  for (size_t i = 0; i < buflen / sizeof (uint32_t); i++)
 {
   ub[i] = seed;
   seed = lcg_parkmiller (seed);
 }
   return buflen;
+#endif /* __MINGW64_VERSION_MAJOR  */
 }
 
 

Re: [v3 PATCH, RFC] Implement LWG 2729 for tuple

[Jonathan Wakely]

On 31/08/16 19:49 +0300, Ville Voutilainen wrote:

@@ -338,6 +345,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
  }
};

+  template
+struct __is_tuple_impl_trait_impl : false_type
+  { };
+
+  template
+struct __is_tuple_impl_trait_impl<_Tuple_impl<_Idx, _Tp...>> : true_type
+  { };
+
+  template
+struct __is_tuple_impl_trait : public __is_tuple_impl_trait_impl<_Tp>
+  { };


Please align the class bodies with the "struct" keyword here.

Otherwise OK for trunk, thanks.

Re: [Patch AArch64] Add floatdihf2 and floatunsdihf2 patterns

[James Greenhalgh]

On Tue, Sep 13, 2016 at 10:31:28AM +0100, James Greenhalgh wrote:
> On Tue, Sep 06, 2016 at 10:19:50AM +0100, James Greenhalgh wrote:
> > This patch adds patterns for conversion from 64-bit integer to 16-bit
> > floating-point values under AArch64 targets which don't have support for
> > the ARMv8.2-A 16-bit floating point extensions.
> > 
> > We implement these by first saturating to a SImode (we know that any
> > values >= 65504 will round to infinity after conversion to HFmode), then
> > converting to a DFmode (unsigned conversions could go to SFmode, but there
> > is no performance benefit to this). Then converting to HFmode.
> > 
> > Having added these patterns, the expansion path in "expand_float" will
> > now try to use them for conversions from SImode to HFmode as there is no
> > floatsihf2 pattern. expand_float first tries widening the integer size and
> > looking for a match, so it will try SImode -> DImode. But our DI mode
> > pattern is going to then saturate us back to SImode which is wasteful.
> > 
> > Better, would be for us to provide float(uns)sihf2 patterns directly.
> > So that's what this patch does.
> > 
> > The testcase add in this patch would fail on trunk for AArch64. There is
> > no libgcc routine to make the conversion, and we don't provide appropriate
> > patterns in the backend, so we get a link-time error.
> > 
> > Bootstrapped and tested on aarch64-none-linux-gnu
> > 
> > OK for trunk?
> 
> Ping.

Ping^2

Cheers,
James

> > 2016-09-06  James Greenhalgh  
> > 
> > * config/aarch64/aarch64.md (sihf2): Convert to expand.
> > (dihf2): Likewise.
> > (aarch64_fp16_hf2): New.
> > 
> > 2016-09-06  James Greenhalgh  
> > 
> > * gcc.target/aarch64/floatdihf2_1.c: New.
> > 
> 
> > diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> > index 6afaf90..1882a72 100644
> > --- a/gcc/config/aarch64/aarch64.md
> > +++ b/gcc/config/aarch64/aarch64.md
> > @@ -4630,7 +4630,14 @@
> >[(set_attr "type" "f_cvti2f")]
> >  )
> >  
> > -(define_insn "hf2"
> > +;; If we do not have ARMv8.2-A 16-bit floating point extensions, the
> > +;; midend will arrange for an SImode conversion to HFmode to first go
> > +;; through DFmode, then to HFmode.  But first it will try converting
> > +;; to DImode then down, which would match our DImode pattern below and
> > +;; give very poor code-generation.  So, we must provide our own emulation
> > +;; of the mid-end logic.
> > +
> > +(define_insn "aarch64_fp16_hf2"
> >[(set (match_operand:HF 0 "register_operand" "=w")
> > (FLOATUORS:HF (match_operand:GPI 1 "register_operand" "r")))]
> >"TARGET_FP_F16INST"
> > @@ -4638,6 +4645,53 @@
> >[(set_attr "type" "f_cvti2f")]
> >  )
> >  
> > +(define_expand "sihf2"
> > +  [(set (match_operand:HF 0 "register_operand")
> > +   (FLOATUORS:HF (match_operand:SI 1 "register_operand")))]
> > +  "TARGET_FLOAT"
> > +{
> > +  if (TARGET_FP_F16INST)
> > +emit_insn (gen_aarch64_fp16_sihf2 (operands[0], operands[1]));
> > +  else
> > +{
> > +  rtx convert_target = gen_reg_rtx (DFmode);
> > +  emit_insn (gen_sidf2 (convert_target, operands[1]));
> > +  emit_insn (gen_truncdfhf2 (operands[0], convert_target));
> > +}
> > +  DONE;
> > +}
> > +)
> > +
> > +;; For DImode there is no wide enough floating-point mode that we
> > +;; can convert through natively (TFmode would work, but requires a library
> > +;; call).  However, we know that any value >= 65504 will be rounded
> > +;; to infinity on conversion.  This is well within the range of SImode, so
> > +;; we can:
> > +;;   Saturate to SImode.
> > +;;   Convert from that to DFmode
> > +;;   Convert from that to HFmode (phew!).
> > +;; Note that the saturation to SImode requires the SIMD extensions.  If
> > +;; we ever need to provide this pattern where the SIMD extensions are not
> > +;; available, we would need a different approach.
> > +
> > +(define_expand "dihf2"
> > +  [(set (match_operand:HF 0 "register_operand")
> > +   (FLOATUORS:HF (match_operand:DI 1 "register_operand")))]
> > +  "TARGET_FLOAT && (TARGET_FP_F16INST || TARGET_SIMD)"
> > +{
> > +  if (TARGET_FP_F16INST)
> > +emit_insn (gen_aarch64_fp16_dihf2 (operands[0], operands[1]));
> > +  else
> > +{
> > +  rtx sat_target = gen_reg_rtx (SImode);
> > +  emit_insn (gen_aarch64_qmovndi (sat_target, operands[1]));
> > +  emit_insn (gen_sihf2 (operands[0], sat_target));
> > +}
> > +
> > +  DONE;
> > +}
> > +)
> > +
> >  ;; Convert between fixed-point and floating-point (scalar modes)
> >  
> >  (define_insn "3"
> > diff --git a/gcc/testsuite/gcc.target/aarch64/floatdihf2_1.c 
> > b/gcc/testsuite/gcc.target/aarch64/floatdihf2_1.c
> > new file mode 100644
> > index 000..9eaa4ba
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/aarch64/floatdihf2_1.c
> > @@ -0,0 +1,35 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2" } */
> > +
> > +/* Test that conversion from 32-bit 

Re: [v3 PATCH] PR libstdc++/77288 and the newest proposed resolution for LWG 2756

[Jonathan Wakely]

On 06/09/16 09:00 +0300, Ville Voutilainen wrote:

   PR libstdc++/77288
   * include/std/optional (__is_optional_impl, __is_optional): Remove.
   (__converts_from_optional, __assigns_from_optional): New.
   (optional(_Up&&)): Use is_same instead of __is_optional.
   (optional(const optional<_Up>&)): Constrain with
   __converts_from_optional.
   (optional(optional<_Up>&&)): Likewise.
   (operator=(_Up&&)): Use is_same instead of __is_optional, check
   is_same and is_scalar.
   (operator=(const optional<_Up>&)): Constrain with
   __converts_from_optional and __assigns_from_optional.
   (operator=(optional<_Up>&&)): Likewise.
   * testsuite/20_util/optional/77288.cc: New.
   * testsuite/20_util/optional/cons/value.cc: Adjust.


OK for trunk, thanks.

Re: [PATCH] (PR 65950) Improve predicate for exit(0);

[Richard Biener]

On Wed, Sep 21, 2016 at 10:45 AM, Andrew Pinski  wrote:
> On Wed, Sep 21, 2016 at 4:32 PM, Richard Biener
>  wrote:
>> On Wed, Sep 21, 2016 at 9:13 AM, Andrew Pinski  wrote:
>>> Hi,
>>>   While looking through bug reports, I noticed someone had reported
>>> that LTO caused the vectorizer not to kick in.  Originally it was not
>>> obvious why it would change the behavior since this was a simple
>>> program with nothing out of the ordinary.  Well it turned out paths
>>> leading to the exit(0); at the end of main was being marked as cold
>>> and in the LTO case the funciton (which had loop which should have
>>> been vectorized) was considered local and being marked as cold as it
>>> was only called now from the path leading to the exit(0);  Since
>>> exit(0); is considered a normal exit path, there is no reason to mark
>>> it as being as a cold path; let the other predicate handle it.
>>>
>>> So this patch changes the predicate for exit(0) not to mark the paths
>>> leading up to that function call as being cold.  Note this patch only
>>> disables that when the argument is a literal zero so if we a PHI node
>>> which contains the exit value, we still cause those paths to be
>>> considered cold; this will be for another patch.
>>
>> Hmm, it also doesn't work for main calling a function not returning but 
>> exiting
>> with exit (0) (it will be discovered as noreturn).
>>
>> I don't think that treating exit (0) as generally not terminating a cold code
>> sequence is good either.
>>
>> Predictions are always hard I guess ...
>>
>> But the thing to improve is maybe the different handling of main with
>> respect to the guessed profile -- this is something that should not happen
>> inconsistently between LTO / non-LTO as main is special in all cases.  Honza?
>
> Maybe I did not explain it very well but what is happening is a
> function which is only called in the cold path is marked as cold only
> in the LTO case as it is extern call.. Basically with LTO, the
> function becomes local so it can be marked as cold while without LTO,
> it cannot.

Ah, so you have

foo () { loop }
main()
{
  if ()
   {
  foo ();
  exit (0);
   }
...
  return 0;
}

and foo is marked cold because its only call is on the path to exit (0)?

noreturn prediction is quite aggressive but it works also quite well.  Given
you can only detect a very minor fraction of cases (consider exit (0) being
in foo itself) I'm not sure that your fix is good progress.  IMHO we might
want to switch from 'noreturn' to 'noreturn' + likely error which needs
another attribute / auto-discovery and IPA propagation to handle this case
better.

Anyway, I'll leave the patch to Honza.

Richard.

> Thanks,
> Andrew
>
>>
>> Richard.
>>
>>> OK?  Bootstrapped and tested on aarch64-linux-gnu with no regressions.
>>> Also tested it with SPEC CPU 2006 with no performance regressions.
>>>
>>> Thanks,
>>> Andrew Pinski
>>>
>>> ChangeLog:
>>> * predict.c (is_exit_with_zero_arg): New function.
>>> (tree_bb_level_predictions): Don't consider paths leading to exit(0)
>>> as nottaken.

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

[Jonathan Wakely]

On 20/09/16 11:03 -0700, Tim Shen wrote:

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.


THanks, OK for trunk.

Re: [PATCH GCC 6/9]Simplify control flow graph for vectorized loop

[Bin.Cheng]

On Wed, Sep 14, 2016 at 5:43 PM, Jeff Law  wrote:
> On 09/14/2016 07:21 AM, Richard Biener wrote:
>>
>> On Tue, Sep 6, 2016 at 8:52 PM, Bin Cheng  wrote:
>>>
>>> Hi,
>>> This is the main patch improving control flow graph for vectorized loop.
>>> It generally rewrites loop peeling stuff in vectorizer.  As described in
>>> patch, for a typical loop to be vectorized like:
>>>
>>>preheader:
>>>  LOOP:
>>>header_bb:
>>>  loop_body
>>>  if (exit_loop_cond) goto exit_bb
>>>  elsegoto header_bb
>>>exit_bb:
>>>
>>> This patch peels prolog and epilog from the loop, adds guards skipping
>>> PROLOG and EPILOG for various conditions.  As a result, the changed CFG
>>> would look like:
>>>
>>>guard_bb_1:
>>>  if (prefer_scalar_loop) goto merge_bb_1
>>>  elsegoto guard_bb_2
>>>
>>>guard_bb_2:
>>>  if (skip_prolog) goto merge_bb_2
>>>  else goto prolog_preheader
>>>
>>>prolog_preheader:
>>>  PROLOG:
>>>prolog_header_bb:
>>>  prolog_body
>>>  if (exit_prolog_cond) goto prolog_exit_bb
>>>  else  goto prolog_header_bb
>>>prolog_exit_bb:
>>>
>>>merge_bb_2:
>>>
>>>vector_preheader:
>>>  VECTOR LOOP:
>>>vector_header_bb:
>>>  vector_body
>>>  if (exit_vector_cond) goto vector_exit_bb
>>>  else  goto vector_header_bb
>>>vector_exit_bb:
>>>
>>>guard_bb_3:
>>>  if (skip_epilog) goto merge_bb_3
>>>  else goto epilog_preheader
>>>
>>>merge_bb_1:
>>>
>>>epilog_preheader:
>>>  EPILOG:
>>>epilog_header_bb:
>>>  epilog_body
>>>  if (exit_epilog_cond) goto merge_bb_3
>>>  else  goto epilog_header_bb
>>>
>>>merge_bb_3:
>>>
>>>
>>> Note this patch peels prolog and epilog only if it's necessary, as well
>>> as adds different guard_conditions/branches.  Also the first guard/branch
>>> could be further improved by merging it with loop versioning.
>>>
>>> Before this patch, up to 4 branch instructions need to be executed before
>>> the vectorized loop is reached in the worst case, while the number is
>>> reduced to 2 with this patch.  The patch also does better in compile time
>>> analysis to avoid unnecessary peeling/branching.
>>> From implementation's point of view, vectorizer needs to update induction
>>> variables and iteration bounds along with control flow changes.
>>> Unfortunately, it also becomes much harder to follow because slpeel_*
>>> functions updates SSA by itself, rather than using update_ssa interface.
>>> This patch tries to factor out SSA/IV/Niter_bound changes from CFG changes.
>>> This should make the implementation easier to read, and I think it maybe a
>>> step forward to replace slpeel_* functions with generic GIMPLE loop copy
>>> interfaces as Richard suggested.
>>
>>
>> I've skimmed over the patch and it looks reasonable to me.
>
> THanks.  I was maybe 15% of the way through the main patch.  Nothing that
> gave me cause for concern, but I wasn't ready to ACK it myself yet.
Hi Jeff,
Any update on this one?  Well, it might conflict with the epilogue
vectorization patch set?

Thanks,
bin

Re: [PATCH] (PR 65950) Improve predicate for exit(0);

[Andrew Pinski]

On Wed, Sep 21, 2016 at 4:32 PM, Richard Biener
 wrote:
> On Wed, Sep 21, 2016 at 9:13 AM, Andrew Pinski  wrote:
>> Hi,
>>   While looking through bug reports, I noticed someone had reported
>> that LTO caused the vectorizer not to kick in.  Originally it was not
>> obvious why it would change the behavior since this was a simple
>> program with nothing out of the ordinary.  Well it turned out paths
>> leading to the exit(0); at the end of main was being marked as cold
>> and in the LTO case the funciton (which had loop which should have
>> been vectorized) was considered local and being marked as cold as it
>> was only called now from the path leading to the exit(0);  Since
>> exit(0); is considered a normal exit path, there is no reason to mark
>> it as being as a cold path; let the other predicate handle it.
>>
>> So this patch changes the predicate for exit(0) not to mark the paths
>> leading up to that function call as being cold.  Note this patch only
>> disables that when the argument is a literal zero so if we a PHI node
>> which contains the exit value, we still cause those paths to be
>> considered cold; this will be for another patch.
>
> Hmm, it also doesn't work for main calling a function not returning but 
> exiting
> with exit (0) (it will be discovered as noreturn).
>
> I don't think that treating exit (0) as generally not terminating a cold code
> sequence is good either.
>
> Predictions are always hard I guess ...
>
> But the thing to improve is maybe the different handling of main with
> respect to the guessed profile -- this is something that should not happen
> inconsistently between LTO / non-LTO as main is special in all cases.  Honza?

Maybe I did not explain it very well but what is happening is a
function which is only called in the cold path is marked as cold only
in the LTO case as it is extern call.. Basically with LTO, the
function becomes local so it can be marked as cold while without LTO,
it cannot.

Thanks,
Andrew

>
> Richard.
>
>> OK?  Bootstrapped and tested on aarch64-linux-gnu with no regressions.
>> Also tested it with SPEC CPU 2006 with no performance regressions.
>>
>> Thanks,
>> Andrew Pinski
>>
>> ChangeLog:
>> * predict.c (is_exit_with_zero_arg): New function.
>> (tree_bb_level_predictions): Don't consider paths leading to exit(0)
>> as nottaken.

Re: [PATCH][3/n] dwarf2out refactoring for early (LTO) debug

[Richard Biener]

On Wed, Aug 26, 2015 at 1:34 PM, Richard Biener  wrote:
>
> The following fixes a GC issue I run into when doing
> prune_unused_types_prune early.  The issue is that the DIE struct
> has a chain_circular marked field (die_sib) which cannot tolerate
> spurious extra entries from old removed entries into the circular
> chain.  Otherwise we fail to properly mark parts of the chain.
> Those stray entries are kept live referenced from TYPE_SYMTAB_DIE.
>
> So the following patch that makes sure to clear ->die_sib for
> nodes we remove.  (these DIEs remaining in TYPE_SYMTAB_DIE also
> means we may end up re-using them which is probably not what we
> want ... in the original LTO experiment I had a ->removed flag
> in the DIE struct and removed DIEs from the cache at cache lookup
> time if I hit a removed DIE)
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, gdb tested there
> as well.
>
> Ok for trunk?

I am re-testing this now and will commit it as part of the piecewise early LTO
merge.

Richard.

> Thanks,
> Richard.
>
> 2015-08-26  Richard Biener  
>
> * dwarf2out.c (remove_child_with_prev): Clear child->die_sib.
> (replace_child): Likewise.
> (remove_child_TAG): Adjust.
> (move_marked_base_types): Likewise.
> (prune_unused_types_prune): Clear die_sib of removed children.
>
> Index: trunk/gcc/dwarf2out.c
> ===
> --- trunk.orig/gcc/dwarf2out.c  2015-08-26 09:30:54.679185817 +0200
> +++ trunk/gcc/dwarf2out.c   2015-08-25 16:54:09.150506037 +0200
> @@ -4827,6 +4827,7 @@ remove_child_with_prev (dw_die_ref child
>  prev->die_sib = child->die_sib;
>if (child->die_parent->die_child == child)
>  child->die_parent->die_child = prev;
> +  child->die_sib = NULL;
>  }
>
>  /* Replace OLD_CHILD with NEW_CHILD.  PREV must have the property that
> @@ -4853,6 +4854,7 @@ replace_child (dw_die_ref old_child, dw_
>  }
>if (old_child->die_parent->die_child == old_child)
>  old_child->die_parent->die_child = new_child;
> +  old_child->die_sib = NULL;
>  }
>
>  /* Move all children from OLD_PARENT to NEW_PARENT.  */
> @@ -4883,9 +4885,9 @@ remove_child_TAG (dw_die_ref die, enum d
> remove_child_with_prev (c, prev);
> c->die_parent = NULL;
> /* Might have removed every child.  */
> -   if (c == c->die_sib)
> +   if (die->die_child == NULL)
>   return;
> -   c = c->die_sib;
> +   c = prev->die_sib;
>}
>} while (c != die->die_child);
>  }
> @@ -24565,8 +24590,8 @@ prune_unused_types_prune (dw_die_ref die
>
>c = die->die_child;
>do {
> -dw_die_ref prev = c;
> -for (c = c->die_sib; ! c->die_mark; c = c->die_sib)
> +dw_die_ref prev = c, next;
> +for (c = c->die_sib; ! c->die_mark; c = next)
>if (c == die->die_child)
> {
>   /* No marked children between 'prev' and the end of the list.  */
> @@ -24578,8 +24603,14 @@ prune_unused_types_prune (dw_die_ref die
>   prev->die_sib = c->die_sib;
>   die->die_child = prev;
> }
> + c->die_sib = NULL;
>   return;
> }
> +  else
> +   {
> + next = c->die_sib;
> + c->die_sib = NULL;
> +   }
>
>  if (c != prev->die_sib)
>prev->die_sib = c;
> @@ -24824,8 +24855,8 @@ move_marked_base_types (void)
>   remove_child_with_prev (c, prev);
>   /* As base types got marked, there must be at least
>  one node other than DW_TAG_base_type.  */
> - gcc_assert (c != c->die_sib);
> - c = c->die_sib;
> + gcc_assert (die->die_child != NULL);
> + c = prev->die_sib;
> }
>  }
>while (c != die->die_child);

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

[Kyrill Tkachov]

On 20/09/16 18:56, Pedro Alves wrote:

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.


Thanks for the idea. I'll try to get that working.

Kyrill



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

Re: Fortran, committed: segfault with allocate and stat for derived types with default initialization (pr 68078)

[Louis Krupp]

Having heard no objections, I changed pr68068 to { target x86_64-*-linux* } in 
revision 240304.

My comment in pr68078.f90:

! This test calls set_vm_limit to set an artificially low address space
! limit.  set_vm_limit calls setrlimit, which has some portability
! considerations.  setrlimit gets errors on arm*linux and aarch64*linux,
! and when the main program calls malloc(), it in turn fails on Darwin.
! The code being tested is portable, calling ALLOCATED() or ASSOCIATED()
! to verify that allocation was successful, so the operating assumption
! is that as long as this test runs on at least one system, we can call
! it good.

Louis 
 
  On Mon, 19 Sep 2016 02:35:21 -0700 Christophe Lyon 
 wrote   
 > Hi,  
 >   
 >   
 > On 18 September 2016 at 21:19, Louis Krupp  wrote:  
 > > Two possibilities:  
 > >  
 > > 1. malloc() doesn't silently return NULL on Darwin when it runs out of 
 > > memory;  it always generates an error message.  
 > >  
 > > 2. setrlimit() doesn't work the same on Darwin as it does on Linux, and 
 > > the test program is hitting a system limit.  It so happens that when I 
 > > first looked at this bug, I ignored the "ulimit -v 100" instruction (I 
 > > have a bad habit of not paying attention), and I let the program do memory 
 > > allocation forever.  It eventually got a segfault deep within malloc().  I 
 > > was able to reproduce this in C as well as in Fortran.  Perhaps something 
 > > similar happens with Darwin, although at least on Darwin malloc() issues 
 > > an error, however mysterious, instead of crashing.  
 > >  
 > > Two ways to go from here:  
 > >  
 > > 1. If anyone can let me ssh into a Darwin system, I can see if I can get 
 > > this to work.  
 > >  
 > > 2. If that's not possible, add a dg-skip-if so that we don't try to run 
 > > this test on Darwin, since it requires some OS support, and it doesn't 
 > > just involve Fortran.  
 > >  
 > > Louis  
 >   
 > On my side, I've noticed that this new test fails on arm*linux and  
 > aarch64*linux, using QEMU:  
 > set_vm_limit: Operation not permitted  
 >   
 > Christophe  
 >   
 > >  
 > >  On Sun, 18 Sep 2016 03:45:38 -0700 Dominique 
 > > d'Humières wrote   
 > >  > > Fixed in revision 240219.  
 > >  >  
 > >  > The test fails on x86_64-apple-darwin15 (at least):  
 > >  >  
 > >  > FAIL: gfortran.dg/pr68078.f90   -O0  output pattern test  
 > >  > FAIL: gfortran.dg/pr68078.f90   -O1  output pattern test  
 > >  > FAIL: gfortran.dg/pr68078.f90   -O2  output pattern test  
 > >  > FAIL: gfortran.dg/pr68078.f90   -O3 -fomit-frame-pointer -funroll-loops 
 > > -fpeel-loops -ftracer -finline-functions  output pattern test  
 > >  > FAIL: gfortran.dg/pr68078.f90   -O3 -g  output pattern test  
 > >  > FAIL: gfortran.dg/pr68078.f90   -Os  output pattern test  
 > >  > FAIL: gfortran.dg/pr68078.f90   -g -flto  output pattern test  
 > >  >  
 > >  > The failures are  
 > >  >  
 > >  > Output was:  
 > >  > pr68078.exe(37410,0xa381f000) malloc: *** mach_vm_map(size=8388608) 
 > > failed (error code=3)  
 > >  > *** error: can't allocate region securely  
 > >  > *** set a breakpoint in malloc_error_break to debug  
 > >  >  foo_ptr allocation failed  
 > >  > pr68078.exe(37410,0xa381f000) malloc: *** mach_vm_map(size=8388608) 
 > > failed (error code=3)  
 > >  > *** error: can't allocate region securely  
 > >  > *** set a breakpoint in malloc_error_break to debug  
 > >  >  foo_obj allocation failed  
 > >  >  
 > >  > Should match:  
 > >  >  *foo_ptr allocation failed(  
 > >  > |^M  
 > >  > |^M) *foo_obj allocation failed(  
 > >  > |^M  
 > >  > |^M) *foo_array allocation failed(  
 > >  > |^M  
 > >  > |^M)  
 > >  >  
 > >  > with -O0  
 > >  >  
 > >  > and  
 > >  >  
 > >  > Output was:  
 > >  > pr68078.exe(39345,0xa381f000) malloc: *** mach_vm_map(size=8388608) 
 > > failed (error code=3)  
 > >  > *** error: can't allocate region securely  
 > >  > *** set a breakpoint in malloc_error_break to debug  
 > >  >  foo_ptr allocation failed  
 > >  >  
 > >  > Should match:  
 > >  >  *foo_ptr allocation failed(  
 > >  > |^M  
 > >  > |^M) *foo_obj allocation failed(  
 > >  > |^M  
 > >  > |^M) *foo_array allocation failed(  
 > >  > |^M  
 > >  > |^M)  
 > >  >  
 > >  > for the other options.  
 > >  >  
 > >  > Dominique  
 > >  >  
 > >  >  
 > >  >  
 > >  
 > >  
 > >  
 >  
 

Re: [PATCH] (PR 65950) Improve predicate for exit(0);

[Richard Biener]

On Wed, Sep 21, 2016 at 9:13 AM, Andrew Pinski  wrote:
> Hi,
>   While looking through bug reports, I noticed someone had reported
> that LTO caused the vectorizer not to kick in.  Originally it was not
> obvious why it would change the behavior since this was a simple
> program with nothing out of the ordinary.  Well it turned out paths
> leading to the exit(0); at the end of main was being marked as cold
> and in the LTO case the funciton (which had loop which should have
> been vectorized) was considered local and being marked as cold as it
> was only called now from the path leading to the exit(0);  Since
> exit(0); is considered a normal exit path, there is no reason to mark
> it as being as a cold path; let the other predicate handle it.
>
> So this patch changes the predicate for exit(0) not to mark the paths
> leading up to that function call as being cold.  Note this patch only
> disables that when the argument is a literal zero so if we a PHI node
> which contains the exit value, we still cause those paths to be
> considered cold; this will be for another patch.

Hmm, it also doesn't work for main calling a function not returning but exiting
with exit (0) (it will be discovered as noreturn).

I don't think that treating exit (0) as generally not terminating a cold code
sequence is good either.

Predictions are always hard I guess ...

But the thing to improve is maybe the different handling of main with
respect to the guessed profile -- this is something that should not happen
inconsistently between LTO / non-LTO as main is special in all cases.  Honza?

Richard.

> OK?  Bootstrapped and tested on aarch64-linux-gnu with no regressions.
> Also tested it with SPEC CPU 2006 with no performance regressions.
>
> Thanks,
> Andrew Pinski
>
> ChangeLog:
> * predict.c (is_exit_with_zero_arg): New function.
> (tree_bb_level_predictions): Don't consider paths leading to exit(0)
> as nottaken.

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

[Richard Biener]

On Tue, 20 Sep 2016, Doug Gilmore wrote:

> 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?

It checks for !ptr_info but it requires NAME to be an SSA name.

>From the attachment in bugzilla (the attachment didn't make it
here)


+
+  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));
+   }
+

I was talking about changing the if to

if (TREE_CODE (addr_base) == SSA_NAME
&& TREE_CODE (addr) == SSA_NAME)

because the addresses could be invariant as far as I can see.

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

I'm not sure how to easily test this.

Richard.

> 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.
> 
> 

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

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

[Richard Biener]

On Wed, 21 Sep 2016, Joseph Myers wrote:

> 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.)

Yeah, that's the reason we "lower" this early.

> * 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?

I think it makes sense to fold fpclassify (...) == N to more specific
classification builtins that do not have this issue if possible.  OTOH
RTL expansion could detect some of the non-builtin ways to do such checks
and see if an optab exists as well.

> (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.)

I think they do via jump-threading.

Richard.