Re: libstdc++ deque allocation

2016-06-23 Thread Jonathan Wakely
On 23 June 2016 at 01:10, Soul Studios wrote:
> Hi there-
> quick question,

It would have been better on the libstdc++ mailing list.

> does deque as defined in libstdc++ allocate upon initialisation or upon
> first insertion?

Unfortunately our std::deque allocates some memory in its default
constructor (and in move constructors and move assignment operators).

> Trying to dig through the code but can't figure it out.
> Reason being, it's insertion graphs seem to show a surprisingly linear
> progression from small amounts of N to large amounts.
> Thanks in advance,
> Matt


Re: Should we import gnulib under gcc/ or at the top-level like libiberty?

2016-06-23 Thread Pedro Alves
On 06/22/2016 07:17 PM, ayush goel wrote:
> 
> Hi, I am working on importing gnulib library inside the gcc tree.
> Should the library be imported in the top level directory along with
> other libraries (like libiberty, libatomic, liboffloadmic etc), or
> should it be imported inside gcc/ like it is done in the binutils-gdb
> tree. There they have a gnulib directory inside gdb/ in the top level
> directory.

I think that top level is better.

Let me touch a bit on why gdb doesn't put it at top level.
Follow the URL further below for more.

The way gdb's gnulib import is set up nowadays, we have a single
gnulib copy that is used by both gdb and gdbserver (two separate programs).

gdb imports gnulib in a way that makes it a separate library, configured
separately from the programs that use it (gdb and gdbserver), which is 
unlike the usual way gnulib is imported, but I think it's the
right thing to do.

gdb doesn't put that gnulib wrapper library at the top level, mainly
just because of history -- we didn't always have that wrapper
library -- and the fact that gdb/gdbserver/ itself is not at top
level either, even though it would be better moved to top level.

See this long email, explaining how the current gdb's gnulib import
is set up:

 https://sourceware.org/ml/gdb-patches/2012-04/msg00426.html

I suggest gcc reuses the whole of gdb's wrapper library and scripts:

 
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=tree;f=gdb/gnulib;h=cdf326774716ae427dc4fb47c9a410fcdf715563;hb=HEAD

... but put it in the top level instead.

A side effect of putting gnulib in a separately configured directory, is
that you end up with a config.h in the gnulib build directory that needs
to be included by the programs that make use of gnulib.  Meaning,
gdb #includes _two_ "config.h" files.  See gdb/common/common-defs.h.

Thanks,
Pedro Alves


How to improve the location of a gcc diagnostic

2016-06-23 Thread David Malcolm
A user filed a bug about a bad location in a warning.  It was marked as
an "easyhack" in bugzilla, and I had a go at fixing it.

I though it may be useful for new GCC developers if I document what I
did to fix it.

FWIW, the bug was PR c/71610
  i.e. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71610)
("Improve location for "warning: ISO C restricts enumerator values to
range of 'int' [-Wpedantic]"?").


Step 1: create a minimal reproducer for the diagnostic

This was easy for this bug: I copied it from the bug report.  Try to
avoid #include if possible; start with the simplest possible reproducer
(to minimize effort in the debugger), and work from there.

If you're working on a bug in our bugzilla it's good to click on the
"take" button next to "Assignee" to mark yourself as assignee, and to
set the status to ASSIGNED, so that we don't duplicate effort.


Step 2: create a file under the relevant subdirectory of gcc/testsuite.

This will be under one of gcc.dg, g++.dg or c-c++-common. If possible,
create it under c-c++-common so that we can test both the C and C++
frontends.

I created a file called "pr71610.c" under c-c++-common.  (If this was
for a specific warning, the name of the warning may have been better,
but my case was part of "-Wpedantic", which is too broad for a testcase
title).


Step 3: run the reproducer under DejaGnu

With a freshly built gcc, I ran the following from the "gcc" build
directory:

  make -jN -k && make check-gcc RUNTESTFLAGS="-v -v dg.exp=pr71610*"

changing the "N" in -jN to reflect the number of cores on my box.

This ensures that the compiler is rebuilt (useful when making changes),
and then runs the C compiler testsuite, filtering to just the new tests
that I'd added.

Given that the test case is for c-c++-common, it's also worth running
"make check-g++" to run the C++ compiler on the same tests.

I added DejaGnu directives to the test file to ensure that the test was
run with appropriate command-line flags to trigger the behavior in
question.  In this case, -Wpedantic is needed, and we want to ensure
that the correct source ranges are printed so -fdiagnostics-show-caret
is also needed (otherwise the testsuite defaults to -fno-diagnostics
-show-caret).

Hence I added:
  /* { dg-options "-pedantic -fdiagnostics-show-caret" } */
to the test case.

Given that I wanted to test ranges, I renamed some of the identifiers
in the test case to avoid using 1-character names.  That way it's easy
to verify that all of the identifier is properly underlined (since
finish != the start for a multi-character identifier).

At this point you should have a reproducer that runs within our DejaGnu
-based test suite, and demonstrates the problem.

Adding "-v -v" to RUNTESTFLAGS gives us the command-line invocations of
the test cases.  Copy and paste this and verify that the reproducer can
be run directly from the command-line.  This brings us to...


Step 4: run the reproducer under gdb

Having created a reproducer and identified a command-line invocation
for running it under the testsuite, add the following to the end of the
command-line:

  -wrapper gdb,--args

This will make the gcc driver run "cc1" under gdb.

A good breakpoint location is diagnostic_show_locus.  This is run
whenever a diagnostic is actually emitted (as opposed to, say, warning
and warning_at, which are called many times as the compiler runs, with
most of the calls doing nothing).

  (gdb) break diagnostic_show_locus
  (gdb) run

Hopefully the debugger now hits the breakpoint.  At this point, go up
the stack until you reach the frontend call that emits the diagnostic
(in my case, this was a call to pedwarn).  It's usually helpful to now
put a breakpoint on the actual emission point (to save a step when re
running).


Step 5: figure out where the location is coming from, and improve it

In the debugger you can investigate which location is being used for
the diagnostic, and use a better one.

Many older calls to the diagnostic subsystem implicitly use the
"input_location" global, which is typically the location (and range) of
the token currently being parsed.

In my case, the code was using:

  value_loc = c_parser_peek_token (parser)->location;

which gets the location (and range) of the first token within an
expression.

Usually it's better to display the range of an entire expression rather
than just a token.

Some expression tree nodes have locations, others do not.  I hope to
fix this for gcc 7.  In the meantime, the C frontend has a "struct
c_expr" which captures the range of an expression during parsing (even
for those that don't have location information), and the C++ frontend
has a "class cp_expr" which works similarly, capturing the location and
range (again, only during parsing).  Or you can use:

  EXPR_LOC_OR_LOC (expr, fallback_location)

If you want to emit multiple locations from a test, or emit fix-it
hints, try using the rich_location class (see libcpp/include/line
map.h).


Step 6: add more test cases

Ass

Re: Should we import gnulib under gcc/ or at the top-level like libiberty?

2016-06-23 Thread Szabolcs Nagy
On 23/06/16 12:18, Pedro Alves wrote:
> gdb doesn't put that gnulib wrapper library at the top level, mainly
> just because of history -- we didn't always have that wrapper
> library -- and the fact that gdb/gdbserver/ itself is not at top
> level either, even though it would be better moved to top level.
> 
> See this long email, explaining how the current gdb's gnulib import
> is set up:
> 
>  https://sourceware.org/ml/gdb-patches/2012-04/msg00426.html
> 
> I suggest gcc reuses the whole of gdb's wrapper library and scripts:
> 
>  
> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=tree;f=gdb/gnulib;h=cdf326774716ae427dc4fb47c9a410fcdf715563;hb=HEAD
> 
> ... but put it in the top level instead.

if both gcc and binutils used a toplevel gnulib directory
then shared tree build would have the same problem as
libiberty has now: gcc and binutils can depend on different
versions of libiberty and then the build can fail.

as far as i know the shared tree build is the only way to
build a toolchain without install (using in tree binutils)
and it would be nice to fix that use case.



Re: How to improve the location of a gcc diagnostic

2016-06-23 Thread Mikhail Maltsev
On 06/23/2016 17:04, David Malcolm wrote:
> A user filed a bug about a bad location in a warning.  It was marked as
> an "easyhack" in bugzilla, and I had a go at fixing it.
> 
> I though it may be useful for new GCC developers if I document what I
> did to fix it.
Shouldn't this also go to the wiki?

-- 
Mikhail Maltsev


Re: Should we import gnulib under gcc/ or at the top-level like libiberty?

2016-06-23 Thread Pedro Alves
On 06/23/2016 03:54 PM, Szabolcs Nagy wrote:

> if both gcc and binutils used a toplevel gnulib directory
> then shared tree build would have the same problem as
> libiberty has now: gcc and binutils can depend on different
> versions of libiberty and then the build can fail.
> as far as i know the shared tree build is the only way to
> build a toolchain without install (using in tree binutils)
> and it would be nice to fix that use case.

Sharing/not-sharing vs top-level-or-not are orthogonal issues.

As you note, combined tree conflicts are not a new issue introduced
by the potintialy shared top level gnulib.  A way to sort that out
that crosses my mind immediately, would be give gcc's and binutil's copies
different top level directory names, like say libiberty-gcc
and libiberty-binutils or some such.  You'd need to do the same
to shared files in the include/ directory, and probably others
that I'm not recalling off hand.  So if we wanted to aim for that, we
could call the new toplevel directory gnulib-gcc or some such.

IMO, combining gcc and binutils trees of different enough vintage
can't be guaranteed to work, so my suggestion is to treat that
as "it hurts when I do this; then don't do that".

Frankly, the idea of sharing a single gnulib import between
gcc and binutils-gdb scares me a bit, because on the gdb side
we're used to only care about making sure gdb works when we
need to think about  importing a new module.  Not that it
happens all that often though; maybe once a year.

But on the other hand, the idea of maintaining multiple gnulib
copies isn't that appealing either.  Considering that the long
term desired result ends up with a libiberty that is no longer a
portability library, but instead only an utilities library, then to
get to that stage, the other programs in the binutils-gdb repo which
rely on libiberty too, binutils proper, gas, ld, gold, etc., need
to be converted to use gnulib as well.  And then a single
gnulib sounds even more appealing.

In any case, we don't really _need_ to consider sharing right now.
gcc can start slow, and import and convert to use gnulib modules 
incrementally, instead of having it import all the modules
gdb is importing from the get go.

Thanks,
Pedro Alves



Re: How to improve the location of a gcc diagnostic

2016-06-23 Thread Jason Merrill
On Thu, Jun 23, 2016 at 5:04 PM, David Malcolm  wrote:
> Step 10: commit to svn
>
> Once the patch is approved, commit it to svn.  (FWIW I do all of my
> development work in git; I have an svn checkout that I use purely for
> the final checkin, having smoketested the patch first).

Why not checkin using git-svn?  I have no svn checkout.

Jason


gcc-6-20160623 is now available

2016-06-23 Thread gccadmin
Snapshot gcc-6-20160623 is now available on
  ftp://gcc.gnu.org/pub/gcc/snapshots/6-20160623/
and on various mirrors, see http://gcc.gnu.org/mirrors.html for details.

This snapshot has been generated from the GCC 6 SVN branch
with the following options: svn://gcc.gnu.org/svn/gcc/branches/gcc-6-branch 
revision 237749

You'll find:

 gcc-6-20160623.tar.bz2   Complete GCC

  MD5=9866e00c6776fb8a60dc3ff6d0bf0b9b
  SHA1=3ca9679a1bf899a0acc71d5f62a150716797eaf4

Diffs from 6-20160616 are available in the diffs/ subdirectory.

When a particular snapshot is ready for public consumption the LATEST-6
link is updated and a message is sent to the gcc list.  Please do not use
a snapshot before it has been announced that way.


Re: How to improve the location of a gcc diagnostic

2016-06-23 Thread Senthil Kumar Selvaraj

David Malcolm writes:

> A user filed a bug about a bad location in a warning.  It was marked as
> an "easyhack" in bugzilla, and I had a go at fixing it.
>
> I though it may be useful for new GCC developers if I document what I
> did to fix it.
>
> FWIW, the bug was PR c/71610
>   i.e. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71610)
> ("Improve location for "warning: ISO C restricts enumerator values to
> range of 'int' [-Wpedantic]"?").
>
>
> Step 1: create a minimal reproducer for the diagnostic
>
> This was easy for this bug: I copied it from the bug report.  Try to
> avoid #include if possible; start with the simplest possible reproducer
> (to minimize effort in the debugger), and work from there.
>
> If you're working on a bug in our bugzilla it's good to click on the
> "take" button next to "Assignee" to mark yourself as assignee, and to
> set the status to ASSIGNED, so that we don't duplicate effort.
>
>
> Step 2: create a file under the relevant subdirectory of gcc/testsuite.
>
> This will be under one of gcc.dg, g++.dg or c-c++-common. If possible,
> create it under c-c++-common so that we can test both the C and C++
> frontends.
>
> I created a file called "pr71610.c" under c-c++-common.  (If this was
> for a specific warning, the name of the warning may have been better,
> but my case was part of "-Wpedantic", which is too broad for a testcase
> title).
>
>
> Step 3: run the reproducer under DejaGnu
>
> With a freshly built gcc, I ran the following from the "gcc" build
> directory:
>
>   make -jN -k && make check-gcc RUNTESTFLAGS="-v -v dg.exp=pr71610*"
>

Worth mentioning that you need to configure/make with CXXFLAGS or BOOT_*
flags set to -O0 -g3 so that breakpoints actually work?

Regards
Senthil