Re: adding -Wshadow-local and -Wshadow-compatible-local ?

[Diego Novillo]

On Fri, Dec 11, 2015 at 6:41 PM, Jim Meyering  wrote:
>
> Hi Diego,
>
> I noticed this patch that adds support for improved -Wshadow-related options:
>
>   [google] Add two new -Wshadow warnings (issue4452058)
>https://gcc.gnu.org/ml/gcc-patches/2011-04/msg02317.html
>https://codereview.appspot.com/4452058/
>
> Here are the proposed descriptions:
>
> -Wshadow-local which warns if a local variable shadows another local
> variable or parameter,
>
> -Wshadow-compatible-local which warns if a local variable shadows another
> local variable or parameter whose type is compatible with that of the
> shadowing variable.
>
> Yet, I see no further discussion of them, other than Jason's review feedback.
> Was this change deemed unsuitable for upstream gcc?

TBH, I do not remember.  That patch is available in the google
branches, IIRC.   I have no plans to pursue it for trunk.  Feel free
to propose it again.


Diego.

Re: [PATCH][PING^2] Skip preprocessor directives in mklog

[Diego Novillo]

On Tue, May 12, 2015 at 11:50 AM, Tom de Vries tom_devr...@mentor.com wrote:

 I'm not a good choice to be the maintainer of a perl script.

I'm all kinds of sorry about the original choice of scripting
language. I'd just spend a couple of hours re-writing it in python.

Re: [PATCH][PING^2] Skip preprocessor directives in mklog

[Diego Novillo]

The patch looks fine to me.

I'm not really involved in GCC development anymore. I would suggest
that this script should be maintained by whoever's been hacking on it
the most. It's a simple script, so it shouldn't be hard to find a new
maintainer for it.


Diegop.

On Tue, May 12, 2015 at 11:19 AM, Yury Gribov y.gri...@samsung.com wrote:
 On 04/30/2015 12:03 PM, Yury Gribov wrote:

 On 04/21/2015 02:26 PM, Yury Gribov wrote:

 Hi all,

 Contrib/mklog is currently faked by preprocessor directives inside
 functions to produce invalid ChangeLog.  The attached patch fixes this.

 Tested with my local mklog testsuite and http://paste.debian.net/167999/
 .  Ok to commit?


 Ping.


 commit 23a738d05393676e72db82cb527d5fb1b3060e2f
 Author: Yury Gribov y.gri...@samsung.com
 Date:   Tue Apr 21 14:17:23 2015 +0300

 2015-04-21  Yury Gribov  y.gri...@samsung.com

 * mklog: Ignore preprocessor directives.

 diff --git a/contrib/mklog b/contrib/mklog
 index f7974a7..455614b 100755
 --- a/contrib/mklog
 +++ b/contrib/mklog
 @@ -131,7 +131,6 @@ sub is_unified_hunk_start {
  }

  # Check if line is a top-level declaration.
 -# TODO: ignore preprocessor directives except maybe #define ?
  sub is_top_level {
 my ($function, $is_context_diff) = (@_);
 if (is_unified_hunk_start ($function)
 @@ -143,7 +142,7 @@ sub is_top_level {
 } else {
 $function =~ s/^.//;
 }
 -   return $function  $function !~ /^[\s{]/;
 +   return $function  $function !~ /^[\s{#]/;
  }

  # Read contents of .diff file

Re: [PATCH] mklog: Fallback to env author name and addr

[Diego Novillo]

On Wed, Apr 8, 2015 at 3:36 PM, Bernhard Reutner-Fischer
rep.dot@gmail.com wrote:
 contrib/ChangeLog:

 2015-04-08  Bernhard Reutner-Fischer  al...@gcc.gnu.org

 * mklog ($name, $addr): Fallback to env author settings.

This looks fine, but note that I no longer have approval rights for
patches. Code in contrib/ has looser requirements, and the patch is
clearly harmless. I would just add documentation on the GIT_AUTHOR_*
environment variables at the top of the script.


Diego.

Re: Stepping down as global maintainer

[Diego Novillo]

On Sun, Feb 8, 2015 at 6:58 AM, Gerald Pfeifer ger...@pfeifer.com wrote:
 On Friday 2015-02-06 16:42, Diego Novillo wrote:
 As such, I propose to become a write-after-approval maintainer
 and relinquish all the other maintainer roles I had.

 Thanks for your contributions over the years, Diego!

Thanks!

 I had a look at gcc/doc/contrib.texi and am not sure this properly
 reflects all of those.  If you'd like to update this, let me know.

Oh, right. I'll send a patch.

Diego.

Re: [PATCH] Added PLUGIN_FINISH_TYPE callback on enum type processing

[Diego Novillo]

That's

On Mon, Feb 2, 2015 at 2:39 PM, Bruno Loff bruno.l...@gmail.com wrote:
 Sorry, first contribution ever :) Here is the entry:

 2014-10-19  Bruno Loff bruno.l...@gmail.com

 * c-parser.c (c_parser_declspecs): Call invoke_plugin_callbacks after
 processing enum declaration.


This is fine. Thanks.


 The dates are off because I actually made the change a while ago (took
 me a while because I needed to test it with the plugin and make sure I
 didn't mess it up).

Just change the date to the date of when you commit the patch.


Diego.

Re: [PATCH] Added PLUGIN_FINISH_TYPE callback on enum type processing

[Diego Novillo]

On Mon, Feb 2, 2015 at 2:39 PM, Bruno Loff bruno.l...@gmail.com wrote:

 2014-10-19  Bruno Loff bruno.l...@gmail.com

 * c-parser.c (c_parser_declspecs): Call invoke_plugin_callbacks after
 processing enum declaration.

Thanks. Committed at r220358.


Diego.

Re: [PATCH] Added PLUGIN_FINISH_TYPE callback on enum type processing

[Diego Novillo]

On Thu, Jan 29, 2015 at 4:32 PM, Bruno Loff bruno.l...@gmail.com wrote:

 The issue was first reported by Joachim Wieland to the list
 g...@gcc.gnu.org, on Wed,
 Jan 19, 2011 (Subject: PLUGIN_FINISH_TYPE not executed for enums).


 A description of the problem/bug and how my patch addresses it.
 ---
 The problem was that when gcc plugins registered callbacks on the
 PLUGIN_FINISH_TYPE event, this event would not be triggered after an
 enum had finished processing.

 The function call that does this was not there; it seems to me that it
 has simply been forgotten.

 Bootstrapping and testing
 

 make bootstrap
 make -k check

  === gcc Summary ===

 # of expected passes106729
 # of expected failures  256
 # of unsupported tests  1409

 on x86_64 ubuntu linux 14.04

 Furthermore, I tested the plugin functionality (with a gcc-with-python
 script), and it now works properly. (However, changes to
 gcc-with-python also had to be made so that enum type info is properly
 converted to python types; see my github fork for these changes
 https://github.com/bloff/gcc-python-plugin)

 The Patch
 ---

 From: bloff bloff.si...@gmail.com
 Date: Sun, 19 Oct 2014 14:54:01 +0100
 Subject: [PATCH] Added PLUGIN_FINISH_TYPE callback on enum type processing

 First reported by Joachim Wieland to the list g...@gcc.gnu.org, on Wed,
 Jan 19, 2011 (Subject: PLUGIN_FINISH_TYPE not executed for enums).
 ---
  gcc/c/c-parser.c | 1 +
  1 file changed, 1 insertion(+)

 diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
 index 264c170..cb515aa 100644
 --- a/gcc/c/c-parser.c
 +++ b/gcc/c/c-parser.c
 @@ -2324,6 +2324,7 @@ c_parser_declspecs (c_parser *parser, struct
 c_declspecs *specs,
   attrs_ok = true;
   seen_type = true;
   t = c_parser_enum_specifier (parser);
 +  invoke_plugin_callbacks (PLUGIN_FINISH_TYPE, t.spec);
   declspecs_add_type (loc, specs, t);
   break;
 case RID_STRUCT:

This is OK with a ChangeLog entry.

Thanks. Diego.

Re: [PATCH] Added PLUGIN_FINISH_TYPE callback on enum type processing

[Diego Novillo]

On Mon, Feb 2, 2015 at 2:07 PM, Bruno Loff bruno.l...@gmail.com wrote:
 Something like:

 The PLUGIN_FINISH_TYPE callback for gcc plugins is now triggered for
 enum declarations.

 ?

ChangeLog entries in GCC are pretty pick as to how they want to be
formatted. See other entries for reference and
https://gcc.gnu.org/codingconventions.html#ChangeLogs for specific
documentation.


Diego.

Re: [PATCH 1/2] teach mklog to get name / email from git config when available

[Diego Novillo]

On 20/11/2014, 16:51 , Tom de Vries wrote:


OK for trunk?


This is fine. Thanks.


Diego.

Re: [RFC] First steps towards segregating types.

[Diego Novillo]

On Fri, Nov 21, 2014 at 1:48 PM, Andrew MacLeod amacl...@redhat.com wrote:

 1 - introduce a TYPE_REF tree node, which is effectively just a 'typed' tree
 node, and the TREE_TYPE() field of a TYPE_REF node would point to the type
 node.  Any routines which utilize a TYPE node in a tree list would have to
 be modified to make use of this new TYPE_REF node to refer to the type.

 2 - change the field (list-value in this case) to be a tagged union of {
 tree tree_value, tree_type_ptr type_value } and use a bit in the base to
 flag which kind of value it is. This would be compatible with GTY, and would
 require changing routines and algorithms to check the bit and use the right
 field.

Seems to me that option 2 would also help against code that blindly
looks at TREE_VALUE and assumes it to be a tree. Wouldn't that make
initial implementation a bit more challenging?

Option 1 does seem easier, but I kind of like the forcing of rvalues
that option 2 provides.

Also liking option 1. The final change to the final type should be
simpler that way.


Diego.

Re: [PATCH] Fix for mklog

[Diego Novillo]

On 11/11/14 09:46, Marat Zakirov wrote:



Attached patch make mklog to stop search for changes inside function
once '}' occur.

Ok, to commit?



OK. Thanks.


Diego.

Re: [PATCH] Fix for mklog

[Diego Novillo]

On 11/06/14 03:00, Marat Zakirov wrote:


Ok to commit?


OK. Thanks.


Diego.

Re: [PATCH]Add aarch64 to list of targets that support gold

[Diego Novillo]

On Thu, Sep 18, 2014 at 7:05 PM, Jing Yu jin...@google.com wrote:

 2014-09-18  Jing Yu  jin...@google.com
   * configure.ac: Add aarch64 to list of targets that support gold.
   * configure: Regenerate.

OK.

Thanks. Diego.

Re: [PATCH v2] Fix signed integer overflow in gcc/data-streamer.c

[Diego Novillo]

On Tue, Sep 30, 2014 at 3:09 AM, Markus Trippelsdorf
mar...@trippelsdorf.de wrote:
 On 2014.09.28 at 14:57 +0200, Markus Trippelsdorf wrote:
 On 2014.09.28 at 14:36 +0200, Steven Bosscher wrote:
 
  Can you use HOST_WIDE_INT_1U for this?

 Sure. Thanks for the suggestion.
 (Fix now resembles similar idiom in data-streamer-in.c)

 I checked in the fix as obvious.

Sorry for the delay. Yes, the fix is obvious. Thanks.

Re: [PATCH][PING] Keep patch file permissions in mklog

[Diego Novillo]

On Fri, Sep 19, 2014 at 6:41 AM, Tom de Vries tom_devr...@mentor.com wrote:

 So it's a question of predictability (always do the same or do nothing) vs.
 robustness (do as much as you can given the circumstances). I'm not sure
 which one is better in this case.

I think it's fine the way it is now. Thanks for the patch. Looks fine to me.


Diego.

Re: [PATCH][PING] Keep patch file permissions in mklog

[Diego Novillo]

On Thu, Sep 18, 2014 at 10:56 AM, Yury Gribov y.gri...@samsung.com wrote:

 On 08/04/2014 12:14 PM, Tom de Vries wrote:

 On 04-08-14 08:45, Yury Gribov wrote:

 Thanks! My 2 (actually 4) cents below.


 Hi Yuri,

 thanks for the review.

   +if ($#ARGV == 1  ($ARGV[0] eq -i || $ARGV[0] eq
 --inline)) {
   +$diff = $ARGV[1];

 Can we shift here and then just set $diff to $ARGV[0] unconditionally?


 Done.

   +if ($diff eq -) {
   +die Reading from - and using -i are not compatible;
   +}

 Hm, can't we dump ChangeLog to stdout in that case?
 The limitation looks rather strange.


 My original idea here was that --inline means 'in the patch file', which
 is not possible if the patch comes from stdin.

 I've now interpreted it such that --inline prints to stdout what it
 would print to the patch file otherwise, that is, both log and patch.
 Printing just the log to stdout can be already be achieved by not using
 --inline.

   +open (FILE1, '', $tmp) or die Could not open temp file;

 Could we use more descriptive name?


 I've used the slightly more descriptive 'OUTPUTFILE'.

   +system (cat $diff $tmp) == 0
   +or die Could not append patch to temp file;
   ...
   +unlink ($tmp) == 1 or die Could not remove temp file;

 The checks look like an overkill given that we don't check for result
 of mktemp...


 I've added a check for the result of mktemp, and removed the unlink
 result check. I've left in the Could not append patch to temp file
 check because the patch file might be read-only.

 OK for trunk?

 Thanks,
 - Tom


 Pinging the patch for Tom.


Apologies for the delay. Could someone post the latest patch. I see
it's gone through a cycle of reviews and changes.


Thanks. Diego.

Re: [PATCH] Fix mklog to support running from arbitrary folder

[Diego Novillo]

On Mon, Jul 21, 2014 at 4:32 AM, Yury Gribov y.gri...@samsung.com wrote:
 Hi all,

 Current mklog works only if run from GCC top-level folder. The patch allows
 running from arbitrary directory.

 I've used Linux directory separators which is probably ok because script
 already expects Linux environment (dirname, basename, etc.).

 Ok to commit?

OK.  Thanks.


Diego.

Re: [PATCH, docs] Document -z option

[Diego Novillo]

On Tue, Jul 15, 2014 at 5:23 PM, Eric Christopher echri...@gmail.com wrote:
 Just to document that it's passed directly on to the linker.

 OK? Wording changes?

 -eric

 2014-07-15  Eric Christopher  echri...@gmail.com

 * doc/invoke.texi (Link Options): Document -z option.

 Index: gcc/doc/invoke.texi
 ===
 --- gcc/doc/invoke.texi (revision 212574)
 +++ gcc/doc/invoke.texi (working copy)
 @@ -464,7 +464,7 @@
  -static-libasan -static-libtsan -static-liblsan -static-libubsan @gol
  -shared -shared-libgcc  -symbolic @gol
  -T @var{script}  -Wl,@var{option}  -Xlinker @var{option} @gol
 --u @var{symbol}}
 +-u @var{symbol} -z @var{keyword}}

  @item Directory Options
  @xref{Directory Options,,Options for Directory Search}.
 @@ -10690,6 +10690,12 @@
  Pretend the symbol @var{symbol} is undefined, to force linking of
  library modules to define it.  You can use @option{-u} multiple times with
  different symbols to force loading of additional library modules.
 +
 +@item -z @var{keyword}
 +@opindex z
 +@option{-z} is passed directly on to the linker along with the keyword
 +@var{keyword}. See the section in the documentation of your linker for
 +permitted values and their meanings.
  @end table

Looks fine.


Diego.

Re: [PATCH] Improve -fdump-tree-all efficiency

[Diego Novillo]

On Thu, Jun 26, 2014 at 9:42 AM, Teresa Johnson tejohn...@google.com wrote:

 * c-family/c-common.h (get_dump_info): Declare.
 * c-family/c-gimplify.c (c_genericize): Use saved dump files.
 * c-family/c-opts.c (c_common_parse_file): Begin and end dumps
 once around parsing invocation.
 (get_dump_info): New function.
 * cp/class.c (dump_class_hierarchy): Use saved dump files.
 (dump_vtable): Ditto.
 (dump_vtt): Ditto.

Looks fine.


Diego.

Re: [PATCH 1/2] teach mklog to get name / email from git config when available

[Diego Novillo]

On Mon, Apr 28, 2014 at 10:11 PM, tsaund...@mozilla.com wrote:

 2014-04-28  Trevor Saunders  tbsau...@mozilla.com

 * mklog: if in a git checkout try to get name and email from git.
 ---
  contrib/mklog | 14 ++
  1 file changed, 14 insertions(+)

 diff --git a/contrib/mklog b/contrib/mklog
 index fb489b0..5f5d98e 100755
 --- a/contrib/mklog
 +++ b/contrib/mklog
 @@ -38,6 +38,20 @@ $gcc_root = $0;
  $gcc_root =~ s/[^\\\/]+$/../;
  chdir $gcc_root;

 +# if this is a git tree then take name and email from the git configuration
 +if (-d .git) {

I would probably use git config directly here. It would work with both
git and svn checkouts (if you have a global .git configuration). But
testing for .git is fine with me as well.

I like Peter's idea of having a ~/.mklog file to override. This would
work for both svn and git checkouts.


Diego.

Re: [PATCH 2/2] allow running mklog as a filter

[Diego Novillo]

On Tue, Apr 29, 2014 at 6:16 AM, Trevor Saunders tsaund...@mozilla.com wrote:

 +# In any case if we got the diff on stdin then write the ChangeLog to 
 stdout.

 Hm, this is breaks semantics: you only dump CL instead of CL+diff just
 because diff comes from stdin. Perhaps we could append contents of
 @diff_lines here?

  I agree stdin gets different semantics than other files but I think
  that's sort of ok because it means you generated the patch somehow so
  presumably you can do that again if you need the patch.  Writing the
  diff to stdout seems possible, but atleast for my use cases its
  annoying, but I guess I'm open to adding a flag or something.

I slightly prefer the semantics that gets me just the ChangeLog. The
workflow I'm envisioning is:

$ cat mypatch | mklog  message.txt

Diego.


 +if ($diff == -) {

 This will work but 'eq' is preferred way to compare strings.

 oh perl

Indeed. I've always regretted writing this in perl.  A python version
would be so much more pleasant to maintain.

OK with Yuri's suggestion of assuming '-' when ARGV is empty.


Diego.

Re: Ping^3 GCC trunk 4.9: documentation patch on plugins

[Diego Novillo]

On Tue, Mar 18, 2014 at 2:12 AM, Basile Starynkevitch
bas...@starynkevitch.net wrote:
 On Sat, 2014-03-08 at 11:15 +0100, Basile Starynkevitch wrote:
 I am pinging again this documentation patch
 http://gcc.gnu.org/ml/gcc-patches/2014-02/msg00074.html
 (pinged at http://gcc.gnu.org/ml/gcc-patches/2014-02/msg01002.html on 
 febµ.17th 2014)
 and also pinged at
 http://gcc.gnu.org/ml/gcc-patches/2014-03/msg00387.html on march 8th
 2014

Apologies for the delay. Please feel free to include me for patches I
may be able to help with.

  gcc/ChangeLog entry

 2014-03-18  Basile Starynkevitch  bas...@starynkevitch.net

 * doc/plugins.texi (Plugin callbacks): Mention
 PLUGIN_INCLUDE_FILE.
 Italicize plugin event names in description.  Explain that
 PLUGIN_PRAGMAS has no sense for lto1. Explain
 PLUGIN_INCLUDE_FILE.
 Remind that no GCC functions should be called after
 PLUGIN_FINISH.
 Explain what pragmas with expansion are.

  the patch:
 Index: gcc/doc/plugins.texi
 ===
 --- gcc/doc/plugins.texi(revision 207422)
 +++ gcc/doc/plugins.texi(working copy)
 @@ -209,6 +209,10 @@
PLUGIN_EARLY_GIMPLE_PASSES_END,
/* Called when a pass is first instantiated.  */
PLUGIN_NEW_PASS,
 +/* Called when a file is #include-d or given thru #line directive.

s/given thru/given via the/

 +   Could happen many times.  The event data is the included file path,

s/Could/This could/

 +Pragmas registered with @code{c_register_pragma_with_expansion} or
 +@code{c_register_pragma_with_expansion_and_data} are allowing
 +preprocessor expansions, like e.g.

I can't parse the last bit: ... are allowing preprocessor expansions,
like e.g..  Did you mean something like support preprocessor
expansions. For example,


Diego.

Re: Ping^3 GCC trunk 4.9: documentation patch on plugins

[Diego Novillo]

OK with:

+Pragmas registered with @code{c_register_pragma_with_expansion} or
+@code{c_register_pragma_with_expansion_and_data} are supporting
+preprocessor expansions. For an example of using such a pragma:

s/are supporting/support/
s/For an example of using such a pragma/For example/


Diego.

Re: Is testing libgomp outside of the build tree supported?

[Diego Novillo]

On Wed, Feb 5, 2014 at 5:10 PM, Matthias Klose d...@ubuntu.com wrote:

 could somebody please shed some light on how this is done?  It's nice that
 everybody has this kind of testing, but the only bit in the gcc sources itself
 seems to be a bit bit-rot and incomplete (contrib/test_installed).

Our case is similar to what Jeff and Joseph already described. I wrote
a script that splits the testsuite directories in equal-sized chunks
and ships them off to different machines. Each machine generates its
site.exp file, and executes runtest with the list of files.

This has exposed a few problems with the testsuite. There are implied
dependencies that some directories have on others (e.g., using other
directories header files) and the multi-files tests are not explicit
about it. So, you may end up sending files needed in the same test to
different machines.


Diego.

Re: [patch] Add missing generated file to contrib/gcc_update list.

[Diego Novillo]

On Fri, Jan 31, 2014 at 10:43 PM, Brooks Moses bmo...@google.com wrote:
 The gcc_update file is missing an entry for
 gcc/config/aarch64/aarch64.md; this patch adds it.

 Ok for trunk?

OK.


Diego.

Re: Is testing libgomp outside of the build tree supported?

[Diego Novillo]

On Mon, Feb 3, 2014 at 2:11 PM, Ian Lance Taylor i...@google.com wrote:

 If the presence of the build
 tree makes writing some tests significantly simpler, I think that is
 OK.

I would like to discourage that.  Testing an already installed GCC for
which no build tree exists is a very useful feature.

Internally, we have a very strong dependency on this feature. If it
were to disappear, it would be almost impossible for us to test the
compiler at the massive scale that we do.


Diego.

Re: [PATCH] Fix handling of context diff patches in mklog

[Diego Novillo]

On Wed, Jan 22, 2014 at 9:36 AM, Yury Gribov y.gri...@samsung.com wrote:

 Ok to commit?

OK. Thanks.


Diego.

Re: [PING][PATCH]Improving mklog [was: Re: RFC Asan instrumentation control]

[Diego Novillo]

Apologies for the delay. The patch is OK.

On Thu, Jan 16, 2014 at 12:59 AM, Tatiana Udalova t.udal...@samsung.com wrote:
 Ping!

 Thank you,
 Tatiana Udalova


 --

 Hello,

 I have reproduced the problem with mklog mentioned by Jakub:

 In my experience mklog is pretty much useless, e.g. if you add a new
 function, it will list the previous function as being modified rather
 than the new one, etc.

 My focus was on functions from headers of diff-log chunks.

 I hacked a simple addition to mklog which skips unchanged functions in
 diff-log while adding function names to the final ChangeLog.

 New mklog results were verified by testsuite which compares reference
 ChangeLogs of patches from gcc trunk with logs generated by mklog.

 Patched mklog considerably reduced the number of unchanged functions in
 ChangeLog.

 Is it OK for trunk?

 Thank you,
 Tatiana Udalova

Re: [patch] fix doc header in contrib/mklog

[Diego Novillo]

The patch is OK. It qualifies as obvious, too. Thanks.

On Thu, Jan 16, 2014 at 6:44 AM, Jonathan Wakely jwakely@gmail.com wrote:
 The mklog script claims to write to stdout, but it actually modifies
 the input file in-place.

 OK to commit this change, which also updates the copyright dates?

Re: Improving mklog [was: Re: RFC Asan instrumentation control]

[Diego Novillo]

On 20/12/2013, 07:08 , Yury Gribov wrote:

Ultimately, mklog ought to write the ChangeLog itself.
We get rid of that headache, at least.


How about this then? Updated mklog now adds 'New file'/'New 
test'/'Remove' when necessary.


I did some tests with unified/context-diffed SVN and git and it worked 
as expected. I can do more testing if necessary.


This is OK. Thanks.


Diego.

Re: Improving mklog [was: Re: RFC Asan instrumentation control]

[Diego Novillo]

On Thu, Dec 19, 2013 at 8:04 AM, Yury Gribov y.gri...@samsung.com wrote:
 On 12/19/2013 04:17 PM, Yury Gribov wrote:
 In my experience mklog is pretty much useless, e.g. if you
 add a new function, it will list the previous function as being modified
 rather than the new one, etc.

 In my experience it prints both the old and the new one. If that's a
 problem we could probably fix it (I mean I can volunteer).

 Here's a draft patch for mklog which splits generated ChangeLog entry
 into several parts (so no more spurious gcc/ or gcc/testsuite/).
 I can continue working on this if people find it useful.

 Removed Kostya and Max, added Diego as original author of mklog.

Oh, crud, why did I have to write it in Perl? Sigh.

The patch is fine (some tweaks below). If someone volunteers to
re-write it in Python, I think it would make it easier to keep
extending. Ultimately, mklog ought to write the ChangeLog itself. We
get rid of that headache, at least.

diff --git a/contrib/mklog b/contrib/mklog
index a874c72..a9bf276 100755
--- a/contrib/mklog
+++ b/contrib/mklog
@@ -34,6 +34,10 @@ $name = @n[1]; chop($name);
 $addr = $username . \@my.domain.org;
 $date = `date +%Y-%m-%d`; chop ($date);

+$gcc_root = $0;
+$gcc_root =~ s/[^\\\/]+$/../;
+chdir $gcc_root;
+

 #-
 # Program starts here. You should not need to edit anything below this
@@ -53,13 +57,26 @@ $basename = `basename $diff`; chop ($basename);
 $cl = `mktemp /tmp/$basename.XX` || exit 1; chop ($cl);
 $hdrline = $date  $name  $addr;

-open (CLFILE, $cl) or die Could not open file $cl for writing;
-
-print CLFILE $hdrline\n\n;
+my %clog_entries;

I'd rather continue using 'cl' to abbreviate ChangeLog, instead of 'clog'.

Re: Improving mklog [was: Re: RFC Asan instrumentation control]

[Diego Novillo]

On Thu, Dec 19, 2013 at 9:33 AM, Yury Gribov y.gri...@samsung.com wrote:

 Frankly in my experience Perl with `use warnings' and `use strict' isn't
 that bad. We could just as well massage existing script.

I suppose.

 Got it. Attached new version of script and ChangeLog entry. Will submit
 tomorrow if noone objects.

The patch is OK. Feel free to submit now, if you want.


Diego.

Re: [wwwdocs] Update obvious fix commit policy

[Diego Novillo]

On Tue, Dec 3, 2013 at 6:55 PM, Gerald Pfeifer ger...@pfeifer.com wrote:
 On Thu, 28 Nov 2013, Richard Biener wrote:
 Why remove ChangeLog files, web pages and comments?

 I was going to complain about web pages being removed. :-)

 On Thu, 28 Nov 2013, Diego Novillo wrote:
 -pFixes for obvious typos in ChangeLog files, docs, web pages, comments
 -and similar stuff.  Just check in the fix and copy it to
 -codegcc-patches/code.  We don't want to get overly anal-retentive
 -about checkin policies./p
 +pObvious fixes can be committed without prior approval.  Just check
 +in the fix and copy it to codegcc-patches/code.  A good test to
 +determine whether a fix is obvious: qwill the person who objects to
 +my work the most be able to find a fault with my fix?/q  If the fix
 +is later found to be faulty, it can always be rolled back.  We don't
 +want to get overly restrictive about checkin policies./p

 I am in favor of this change.

 To some extent, this is more a clarification of what I have seen as
 our current policy than a change in policy, though to a laywer-minded
 person it surely looks like the latter.  Not sure what kind of approval
 this needs?  Mind it has.

I have not received any feedback against this change. I will wait
another 48 hours and commit.


Diego.

Re: [wwwdocs] Update obvious fix commit policy

[Diego Novillo]

On Wed, Dec 4, 2013 at 11:24 AM, Jeff Law l...@redhat.com wrote:

 Here's feedback.  Install it now :-)

Works for me :)  Committed.

Diego.

[wwwdocs] Update obvious fix commit policy

[Diego Novillo]

[ sent first version in html. apologies for the dup. ]

Based on the recent discussion on the obvious fix policy.

OK to commit?

Index: htdocs/svnwrite.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/svnwrite.html,v
retrieving revision 1.29
diff -u -d -u -p -r1.29 svnwrite.html
--- htdocs/svnwrite.html24 Sep 2013 18:26:29 -  1.29
+++ htdocs/svnwrite.html28 Nov 2013 12:04:40 -
@@ -147,10 +147,13 @@ list./p

 pThe following changes can be made by everyone with SVN write access:/p

-pFixes for obvious typos in ChangeLog files, docs, web pages, comments
-and similar stuff.  Just check in the fix and copy it to
-codegcc-patches/code.  We don't want to get overly anal-retentive
-about checkin policies./p
+pObvious fixes to documentation, code and test cases can be
+committed without prior approval.  Just check in the fix and copy it
+to codegcc-patches/code.  A good test to determine whether a fix
+is obvious: will the person who objects to my work the most be able
+to find a fault with my fix?.  If the fix is later found to be
+faulty, it can always be rolled back.  We don't want to get overly
+anal-retentive about checkin policies./p

 pSimilarly, no outside approval is needed to revert a patch that you
 checked in./p

Re: [wwwdocs] Update obvious fix commit policy

[Diego Novillo]

New version with a slightly cleaned up wording:

Index: htdocs/svnwrite.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/svnwrite.html,v
retrieving revision 1.29
diff -u -d -u -p -r1.29 svnwrite.html
--- htdocs/svnwrite.html24 Sep 2013 18:26:29 -  1.29
+++ htdocs/svnwrite.html28 Nov 2013 13:56:55 -
@@ -147,10 +147,13 @@ list./p

 pThe following changes can be made by everyone with SVN write access:/p

-pFixes for obvious typos in ChangeLog files, docs, web pages, comments
-and similar stuff.  Just check in the fix and copy it to
-codegcc-patches/code.  We don't want to get overly anal-retentive
-about checkin policies./p
+pObvious fixes to documentation, code and test cases can be
+committed without prior approval.  Just check in the fix and copy it
+to codegcc-patches/code.  A good test to determine whether a fix
+is obvious: will the person who objects to my work the most be able
+to find a fault with my fix?.  If the fix is later found to be
+faulty, it can always be rolled back.  We don't want to get overly
+restrictive about checkin policies./p

 pSimilarly, no outside approval is needed to revert a patch that you
 checked in./p

Re: [wwwdocs] Update obvious fix commit policy

[Diego Novillo]

Fixed quotation as per IRC feedback.

Index: htdocs/svnwrite.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/svnwrite.html,v
retrieving revision 1.29
diff -u -d -u -p -r1.29 svnwrite.html
--- htdocs/svnwrite.html24 Sep 2013 18:26:29 -  1.29
+++ htdocs/svnwrite.html28 Nov 2013 14:12:18 -
@@ -147,10 +147,13 @@ list./p

 pThe following changes can be made by everyone with SVN write access:/p

-pFixes for obvious typos in ChangeLog files, docs, web pages, comments
-and similar stuff.  Just check in the fix and copy it to
-codegcc-patches/code.  We don't want to get overly anal-retentive
-about checkin policies./p
+pObvious fixes to documentation, code and test cases can be
+committed without prior approval.  Just check in the fix and copy it
+to codegcc-patches/code.  A good test to determine whether a fix
+is obvious: qwill the person who objects to my work the most be able
+to find a fault with my fix?/q  If the fix is later found to be
+faulty, it can always be rolled back.  We don't want to get overly
+restrictive about checkin policies./p

 pSimilarly, no outside approval is needed to revert a patch that you
 checked in./p

Re: [wwwdocs] Update obvious fix commit policy

[Diego Novillo]

On Thu, Nov 28, 2013 at 9:25 AM, Richard Biener
richard.guent...@gmail.com wrote:

 Why remove ChangeLog files, web pages and comments?  Either
 enumerate everything or just enumerate nothing and simply say
 Obvious fixes can be committed without prior approval.

Thanks, that's much better.  I was trying to be more inclusive.


Index: htdocs/svnwrite.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/svnwrite.html,v
retrieving revision 1.29
diff -u -d -u -p -r1.29 svnwrite.html
--- htdocs/svnwrite.html24 Sep 2013 18:26:29 -  1.29
+++ htdocs/svnwrite.html28 Nov 2013 14:26:54 -
@@ -147,10 +147,12 @@ list./p

 pThe following changes can be made by everyone with SVN write access:/p

-pFixes for obvious typos in ChangeLog files, docs, web pages, comments
-and similar stuff.  Just check in the fix and copy it to
-codegcc-patches/code.  We don't want to get overly anal-retentive
-about checkin policies./p
+pObvious fixes can be committed without prior approval.  Just check
+in the fix and copy it to codegcc-patches/code.  A good test to
+determine whether a fix is obvious: qwill the person who objects to
+my work the most be able to find a fault with my fix?/q  If the fix
+is later found to be faulty, it can always be rolled back.  We don't
+want to get overly restrictive about checkin policies./p

 pSimilarly, no outside approval is needed to revert a patch that you
 checked in./p

Re: [wwwdocs] Update obvious fix commit policy

[Diego Novillo]

On Thu, Nov 28, 2013 at 10:32 AM, Richard Earnshaw rearn...@arm.com wrote:

 I think it might be worth saying that one class of 'obvious' fix that we
 don't want to go in without prior clearance are bulk white space
 clean-ups.  These can be a right-royal pain to deal with if you're in
 the middle of a big re-write of a hunk of code.

Hm, not sure I agree.  Those are the most obvious to me.  Particularly
after I get my clang format pony.  I've asked for GNU style support.
It will be a lot easier to keep files properly formatted to the GNU
guidelines.

Making exceptions to the obvious rule seems illogical to me.


Diego.

Re: gcc's obvious patch policy

[Diego Novillo]

On Tue, Nov 26, 2013 at 12:17 AM, Alan Modra amo...@gmail.com wrote:
 Was Re: [buildrobot] [PATCH] mips: Really remove ENTRY_BLOCK_PTR
 On Wed, Nov 20, 2013 at 10:08:45AM +0100, Steven Bosscher wrote:
 This patch is obvious and it fixes breakage. Please go ahead and commit it.

 Sorry to pick on you here Steven, but this doesn't meet gcc's
 definition of an obvious patch.  Don't believe me?  See
 http://gcc.gnu.org/svnwrite.html#policies

 Allowed as obvious in the gcc sources are typo fixes for comments or
 similar, or reverting a bad patch you made.  That's it.  The power to
 change anything else is reserved to the relevant maintainer.

Huh.  That's silly.  It allows nothing interesting!

 Can I recommend gdb's obvious patch policy?  It even tickles my sense
 of humour.  will the person who hates my work the most be able to
 find fault with the change - if so, then it's not obvious..

I like this one much better.  Anyone else opposed to changing the
obvious-commit policy to something along these lines?


Diego.

Re: gcc's obvious patch policy

[Diego Novillo]

On Tue, Nov 26, 2013 at 12:40 PM, Iyer, Balaji V
balaji.v.i...@intel.com wrote:


 -Original Message-
 From: Eric Botcazou [mailto:ebotca...@adacore.com]
 Sent: Tuesday, November 26, 2013 12:33 PM
 To: Iyer, Balaji V
 Cc: gcc-patches@gcc.gnu.org; Diego Novillo; Jeff Law; Steven Bosscher
 Subject: Re: gcc's obvious patch policy

  Can I make a suggestion that if someone is making an obvious change
  (with the exception of changing non-working code (comments, things
  inside #if 0, etc)), have a patch on the mailing list for 12-24 hrs.
  before putting it in? Maybe they could say something like, I will
  check this in by X time TIMEZONE tomorrow since this looks obvious
  to me. This way if the change hurts someone who is working on a
  feature in their local machine that is using the existing framework can
 chime in.

 I disagree, obvious patches cannot reasonably invalidate the work of others,
 or else they are simply not obvious.  At worst they can privatize a public
 function or remove an unused one, but then it's easy to undo that later.


 Those at worst examples you have mentioned is the ones that scare me. 
 Sometimes when a function becomes private, making it public back
 again is sometimes an uphill battle. I am not saying they shouldn't commit 
 it, but at least give a heads-up.

Nah. Simple patches like that are always easy to pinpoint and address
afterwards. Obvious patches will always be on the small side, after
all.


Diego.

Re: [GOOGLE] Refactor the profile propagation for AutoFDO

[Diego Novillo]

Thanks, Deaho.

One other thing that I've found on the LLVM implementation (that I'm
not sure happens in GCC): self-referential edges.  If a loop consists
of a single-basic block, the back edge will point to itself.  I
haven't been able to reproduce it with regular control flow constructs
in GCC.

When this happens, and if we've visited the block itself already
(i.e., the block has weights), I've simply set the weight of the
self-referential edge to the weight of the block itself.  Do you see
any problems with that heuristic?


Thanks.  Diego.

On Mon, Nov 25, 2013 at 12:56 PM, Dehao Chen de...@google.com wrote:
 afdo_propagate_multi_edge can do everything afdo_propagate_single_edge
 does. So we refactor the code to keep only one afdo_propagate_edge
 function.

 Bootstrapped and passed all unittests and performance tests.

 OK for googlge branch?

 Thanks,
 Dehao

 Index: gcc/auto-profile.c
 ===
 --- gcc/auto-profile.c (revision 205354)
 +++ gcc/auto-profile.c (working copy)
 @@ -1069,44 +1069,6 @@ afdo_find_equiv_class (void)
  }
  }

 -/* If a baisk block only has one in/out edge, then the bb count and he
 -   edge count should be the same.
 -   IS_SUCC is true if the out edge of the basic block is examined.
 -   Return TRUE if any basic block/edge count is changed.  */
 -
 -static bool
 -afdo_propagate_single_edge (bool is_succ)
 -{
 -  basic_block bb;
 -  bool changed = false;
 -
 -  FOR_EACH_BB (bb)
 -if (is_succ ? single_succ_p (bb) : single_pred_p (bb))
 -  {
 - edge e = is_succ ? single_succ_edge (bb) : single_pred_edge (bb);
 - if (((e-flags  EDGE_ANNOTATED) == 0)
 - ((bb-flags  BB_ANNOTATED) != 0))
 -  {
 -e-count = bb-count;
 -e-flags |= EDGE_ANNOTATED;
 -changed = true;
 -  }
 - else if (((e-flags  EDGE_ANNOTATED) != 0)
 - ((bb-flags  BB_ANNOTATED) == 0))
 -  {
 -bb-count = e-count;
 -bb-flags |= BB_ANNOTATED;
 -changed = true;
 -  }
 - else if (bb-count != e-count)
 -  {
 -e-count = bb-count = MAX (bb-count, e-count);
 -changed = true;
 -  }
 -  }
 -  return changed;
 -}
 -
  /* If a basic block's count is known, and only one of its in/out edges' count
 is unknown, its count can be calculated.
 Meanwhile, if all of the in/out edges' counts are known, then the basic
 @@ -1115,7 +1077,7 @@ afdo_find_equiv_class (void)
 Return TRUE if any basic block/edge count is changed.  */

  static bool
 -afdo_propagate_multi_edge (bool is_succ)
 +afdo_propagate_edge (bool is_succ)
  {
basic_block bb;
bool changed = false;
 @@ -1281,14 +1243,10 @@ afdo_propagate (void)
  {
changed = false;

 -  if (afdo_propagate_single_edge (true))
 +  if (afdo_propagate_edge (true))
   changed = true;
 -  if (afdo_propagate_single_edge (false))
 +  if (afdo_propagate_edge (false))
   changed = true;
 -  if (afdo_propagate_multi_edge (true))
 - changed = true;
 -  if (afdo_propagate_multi_edge (false))
 - changed = true;
afdo_propagate_circuit ();
  }
  }

Re: [GOOGLE] Refactor the profile propagation for AutoFDO

[Diego Novillo]

On Mon, Nov 25, 2013 at 1:22 PM, Xinliang David Li davi...@google.com wrote:
 In this case the backedge will be a critical edge, which will be split by GCC.

Right. So, if I split it, I will reach essentially the same
conclusion, I think. The new block will get the original block's
weight, which (in turn) will translate into the (now only outgoing)
edge.


Diego.

Re: [GOOGLE] Refactor the profile propagation for AutoFDO

[Diego Novillo]

On Mon, Nov 25, 2013 at 1:30 PM, Dehao Chen de...@google.com wrote:
 On Mon, Nov 25, 2013 at 10:26 AM, Diego Novillo dnovi...@google.com wrote:
 On Mon, Nov 25, 2013 at 1:22 PM, Xinliang David Li davi...@google.com 
 wrote:
 In this case the backedge will be a critical edge, which will be split by 
 GCC.

 Right. So, if I split it, I will reach essentially the same
 conclusion, I think. The new block will get the original block's
 weight, which (in turn) will translate into the (now only outgoing)
 edge.

 Why do you want to set the back edge count as the BB count? I think
 the right formula is: count(back_edge) = count(BB) -
 count(entry_edge), in which entry_edge is the edge that enters the
 loop.

Ah, yeah, you're right. The CFG I was looking at had an incoming
weight of 0 (the code snippet spends 99.9% of its runtime in that
loop.


Thanks.  Diego.

Re: [patch 1/3] Flatten gimple.h

[Diego Novillo]

On Thu, Nov 21, 2013 at 3:04 PM, Andrew MacLeod amacl...@redhat.com wrote:
 On 11/21/2013 02:26 PM, Jeff Law wrote:

 On 11/21/13 11:15, Andrew MacLeod wrote:


 Is there anything in particular one needs to do for plugins? I thought I
 saw a patch somewhere that changed something in the Makefile, but don't
 know if that is actually required since I never did that for any of the
 others.   Any plugin which used gimple.h probably needs a few more
 includes...

 We need to make sure the header files that are needed by plugins appear in
 Makefile.in::PLUGIN_HEADERS so that they get installed in a place where
 plugins can find them.


 stupid question perhaps, but aren't most  header files a potential plugin
 header?Why don't we just install them all...

  No one has complained yet, but in theory any .h I split up over the past
 couple of months has the potential to be required... maintaining that macro
 in Makefile.in seems kinda lame now that we don't maintain the macros for
 building.  I'm sure its rotted already.

That's true. It's essentially impossible to know what plugins will
need. The easy way is the brute-force approach of adding every header
that gets factored out of the original. I chose not to do this in the
tree.h refactoring because it exposes too much unnecessarily. I
exposed as much as was needed to get the plugins in the testsuite.

I think plugins authors are going to have to work closely with us as
we refactor headers. At some point, it will make sense to offer a
better set of headers for them to use.


Diego.

Re: Factor unrelated declarations out of tree.h (1/2)

[Diego Novillo]

On Wed, Nov 20, 2013 at 6:44 AM, Eric Botcazou ebotca...@adacore.com wrote:
 Thanks. Committed as rev 205023.

 Nice work, but why did you antedate the entries in the various ChangeLog

Oh, that's because of local commits and holding on to the patch for a
few days. That date is the date of the original local commit. Further
pulls from upstream pushed the entries down in the file. I just forgot
to move the entries forward when I committed the patch.

 directories (yes, we all know your opinion about ChangeLog files ;-)

What gave you that idea? ;)


Diego.

Re: Factor unrelated declarations out of tree.h (1/2)

[Diego Novillo]

On Wed, Nov 20, 2013 at 8:32 AM, H.J. Lu hjl.to...@gmail.com wrote:

 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59212

Thanks.  Fixed.


PR 59212
* g++.dg/plugin/selfassign.c: Include stringpool.h

diff --git a/gcc/testsuite/g++.dg/plugin/selfassign.c
b/gcc/testsuite/g++.dg/plugin/selfassign.c
index 2498153..cdab74a 100644
--- a/gcc/testsuite/g++.dg/plugin/selfassign.c
+++ b/gcc/testsuite/g++.dg/plugin/selfassign.c
@@ -8,6 +8,7 @@
 #include coretypes.h
 #include tm.h
 #include tree.h
+#include stringpool.h
 #include toplev.h
 #include basic-block.h
 #include gimple.h

Re: Factor unrelated declarations out of tree.h (1/2)

[Diego Novillo]

On Wed, Nov 20, 2013 at 10:10 AM, Trevor Saunders tsaund...@mozilla.com wrote:
 On Wed, Nov 20, 2013 at 08:43:57AM -0500, Diego Novillo wrote:
 On Wed, Nov 20, 2013 at 6:44 AM, Eric Botcazou ebotca...@adacore.com wrote:
  Thanks. Committed as rev 205023.
 
  Nice work, but why did you antedate the entries in the various ChangeLog

 Oh, that's because of local commits and holding on to the patch for a
 few days. That date is the date of the original local commit. Further
 pulls from upstream pushed the entries down in the file. I just forgot
 to move the entries forward when I committed the patch.

  directories (yes, we all know your opinion about ChangeLog files ;-)

 What gave you that idea? ;)

  This even seems like a good argument for that view, git log and I
  assume svn log could give you the same data without the possibility of
  this sort of issue ;)

This is the main reason why I think ChangeLogs are absolutely
worthless. But I don't want to hijack this thread to discuss ChangeLog
politics.


Diego.

Re: [patch] Privatize gimplify_ctx structure.

[Diego Novillo]

On Wed, Nov 20, 2013 at 1:40 PM, Jeff Law l...@redhat.com wrote:

 Do our coding standards allow using default arguments:

 extern void push_gimplify_context (bool in_ssa = false,
bool rhs_cond_ok = false);

Yes, as long as they are not expensive to construct (so, PODs mostly).
http://gcc.gnu.org/codingconventions.html#Default


Diego.

Re: Factor unrelated declarations out of tree.h (1/2)

[Diego Novillo]

On Tue, Nov 19, 2013 at 12:17 AM, Jeff Law l...@redhat.com wrote:

 It looks OK to me.

Thanks. Committed as rev 205023.

Ian,  the Go front end will need that patch committed now.


Diego.

Re: [patch] fix graphite build

[Diego Novillo]

On Tue, Nov 19, 2013 at 8:36 AM, Andrew MacLeod amacl...@redhat.com wrote:
 graphite-sese-to-poly.c needs expr.h to compile.  Fixed thusly and checked
 in as revision 205027.

Thanks!


Diego.

Re: Please don't commit changes to gcc/go/gofrontend

[Diego Novillo]

On Tue, Nov 19, 2013 at 9:48 AM, Ian Lance Taylor i...@google.com wrote:
 Hi, as noted in gcc/go/README.gcc, the files in gcc/go/gofrontend are
 actually mirrored from a different repository.  Please do not directly
 commit changes to those files.  Instead, send the changes to me.  I
 will commit them upstream.  Thanks.

 Ian

Ugh, sorry.  This is really counter-intuitive.  To properly test
changes, one needs the patch in the tree.  So when I do the final
commit, it is not easy to remember that I need to take it out (and
taking it out means undoing a local commit, which is yet another
operation).

Sorry, but I would expect these problems to continue.  Is it possible
for you to cope in some other way?  Like your merging script noticing
changes and incorporating them into your tree?

Alternately, would it be possible to install some kind of svn hook
that only allows certain users to commit?


Diego.

Re: patch PLUGIN_HEADER_FILE event for tracing of header inclusions.

[Diego Novillo]

On Tue, Nov 19, 2013 at 11:16 AM, Joseph S. Myers
jos...@codesourcery.com wrote:
 On Tue, 19 Nov 2013, Basile Starynkevitch wrote:

 Thanks for your attention. I am attaching a slightly improved patch
 against trunk svn rev. 305009 (the improvements are removing the spurious
 diff hunk, and better comments.)

 Still OK in the absence of plugin maintainer objections.

I'm OK with this patch.  Thanks, Basile.


Diego.

Re: [PATCH] C++-ify and simplify loop iterators

[Diego Novillo]

On Tue, Nov 19, 2013 at 10:09 AM, Michael Matz m...@suse.de wrote:

 OMG, finally a c++ification that _shrinks_ client code instead of
 bloating it.

Not quite. The patch that converted VEC macros removed a non-trivial
amount of client code.

$ git log -p -n1 f1f41a6cdc47d5123dd30ab110cc35c90f8189cb -- $(find
-name '*.c')| diffstat | tail -1
 272 files changed, 9309 insertions(+), 10157 deletions(-)


Diego.

Re: Factor unrelated declarations out of tree.h (1/2)

[Diego Novillo]

On Fri, Nov 15, 2013 at 2:23 AM, Jeff Law l...@redhat.com wrote:
 On 11/14/13 15:19, Diego Novillo wrote:

 A good chunk.  I'm doing these FIXMEs in the next sequence of patches,
 so we won't have them for long. Again, I was going for an orderly
 transition here.

 However, I'm much more concerned about this.  It really feels like a step
 backwards.

Sure. I'll play with this next week.

Diego.

Re: Factor unrelated declarations out of tree.h (2/2)

[Diego Novillo]

On Thu, Nov 14, 2013 at 9:49 PM, Andrew MacLeod amacl...@redhat.com wrote:
 On 11/14/2013 05:16 PM, Joseph S. Myers wrote:

 On Thu, 14 Nov 2013, Diego Novillo wrote:

 This patch contains the mechanical side-effects from
 http://gcc.gnu.org/ml/gcc-patches/2013-11/msg01663.html

 There are rather a lot of Include tm.h changes here - especially in
 front ends, where we've tried to eliminate tm.h calls, and put comments on
 some of those remaining saying exactly what target macros are used to make
 clear what's needed to eliminate them.  Putting in these includes, without
 clear comments explaining how to eliminate them, seems a step backwards.

 The problem is larger than that...  function.h includes tm.h as well... and
 something like 140ish files include function.h, not to mention another 5
 include files bring it in...  basic-block.h, cfgloop.h, cgraph.h, expr.h,
 and gimple-streamer.h

Yeah.  For now, I'm going to leave the builtins.h declarations in
tree.h. This is a similar problem that I had with expr.h. There are
parts of it that FEs want, but others they don't.

 I' ve been thinking that the only way to really tackle this is to flatten
 *everything* so that nothing but .c files have #includes, and then trim out
 all the includes that each .c requires, and then see where we sit.  .h files
 bringing in other .h files really muck things up.

Right. Perhaps starting with Joseph's suggestion of having
tree-{builtins,expr}.h is a good start.


Diego.

Re: [patch 3/4] Separate gimple.[ch] and gimplify.[ch] - front end files

[Diego Novillo]

On Thu, Nov 14, 2013 at 10:26 AM, Michael Matz m...@suse.de wrote:

 Put another way: what do you envision that gimple expressions would be.
 For example what would you propose we could do with them?

The only expressions I have in mind are memory references and
aggregates, which can get pretty convoluted.

Perhaps we could label them something different than expressions. They
would be in the same taxonomy of gimple values.  They are operands
to gimple statements, they can have multiple symbol references inside
and they may have a tree like structure.


Diego.

Re: [patch 3/4] Separate gimple.[ch] and gimplify.[ch] - front end files

[Diego Novillo]

On Thu, Nov 14, 2013 at 10:34 AM, Andrew MacLeod amacl...@redhat.com wrote:
 On 11/14/2013 10:26 AM, Michael Matz wrote:

 Hi,

 On Wed, 13 Nov 2013, Andrew MacLeod wrote:

 There needs to be a place which has gimple componentry that is not
 related to or require a statement.  gimple.h is becoming the home for
 just 0gimple statements.  There are 3 (for the moment) major classes of
 things that are in statements and are also used by other parts of the
 compiler .. Types, Decls, and Expressions.


 E.g. there won't ever be something like a gimple arithmetic addition
 expression (or I hope there never will be).  It's always a gimple
 statement that assigns the addition result somewhere.  Even the
 non-singleton objects don't exist in isolation, they're always part of
 some action (i.e. statement) to operate on those objects.

 That's why I think talking about a gimple expression as if they were
 somehow some stand-alone concept is fairly confusing, and introducing it
 now as if it would somewhen exist would lead to going down some inferior
 design paths.

 Well, for gimple expressions I was thinking more about the addressing
 expressions we currently leave as trees... MEM stuff... that where most of
 the remaining 'expressions' are I guess, so perhaps gimple-addressing is a
 better term...

 in any case, it refers mostly to the parts of trees which are tcc_expression
 and are not subsumed by gimple_statement contructs. So I use expression for
 lack of a better term since I don't know what exact uses there are yet.

I think we'll end up with a hierarchy that will have some generic
value at its base, with constants, symbols, aggregates, arrays,
memrefs, etc as children. But perhaps we can wait until we have a
better idea of how we want it to look like.


Diego.

Re: [patch 3/4] Separate gimple.[ch] and gimplify.[ch] - front end files

[Diego Novillo]

On Thu, Nov 14, 2013 at 11:13 AM, Andrew MacLeod amacl...@redhat.com wrote:

 very possibly, i just haven't gotten to those parts yet. I can change the
 name back to gimple-decl.[ch] or some such thing if you like that better.

As much as I hate to paint name sheds: gimple-val.[ch].


Diego.

Add a new header to PLUGIN_HEADERS

[Diego Novillo]

I did not add all headers factored out of tree.h because it is unclear
(and impossible to tell) what plugins need.  This adds the one header
used by the plugins in the testsuite.

This will be changing quite dramatically as we progress with the
header refactoring.


2013-11-14  Diego Novillo  dnovi...@google.com

* Makefile.in (PLUGIN_HEADERS): Add stringpool.h.

testsuite/ChangeLog

* gcc.dg/plugin/selfassign.c: Include stringpool.h.
* gcc.dg/plugin/start_unit_plugin.c: Likewise.

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 806b6ca..44b3eb4 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -3114,7 +3114,7 @@ PLUGIN_HEADERS = $(TREE_H) $(CONFIG_H) $(SYSTEM_H) 
coretypes.h $(TM_H) \
   cppdefault.h flags.h $(MD5_H) params.def params.h prefix.h tree-inline.h \
   $(GIMPLE_PRETTY_PRINT_H) realmpfr.h \
   $(IPA_PROP_H) $(TARGET_H) $(RTL_H) $(TM_P_H) $(CFGLOOP_H) $(EMIT_RTL_H) \
-  version.h
+  version.h stringpool.h
 
 # generate the 'build fragment' b-header-vars
 s-header-vars: Makefile
diff --git a/gcc/testsuite/gcc.dg/plugin/selfassign.c 
b/gcc/testsuite/gcc.dg/plugin/selfassign.c
index 2498153..cdab74a 100644
--- a/gcc/testsuite/gcc.dg/plugin/selfassign.c
+++ b/gcc/testsuite/gcc.dg/plugin/selfassign.c
@@ -8,6 +8,7 @@
 #include coretypes.h
 #include tm.h
 #include tree.h
+#include stringpool.h
 #include toplev.h
 #include basic-block.h
 #include gimple.h
diff --git a/gcc/testsuite/gcc.dg/plugin/start_unit_plugin.c 
b/gcc/testsuite/gcc.dg/plugin/start_unit_plugin.c
index 257aad8..39f4462 100644
--- a/gcc/testsuite/gcc.dg/plugin/start_unit_plugin.c
+++ b/gcc/testsuite/gcc.dg/plugin/start_unit_plugin.c
@@ -11,6 +11,7 @@
 #include coretypes.h
 #include tm.h
 #include tree.h
+#include stringpool.h
 #include toplev.h
 #include basic-block.h
 #include gimple.h
-- 
1.8.4.1

Re: Factor unrelated declarations out of tree.h (1/2)

[Diego Novillo]

On Thu, Nov 14, 2013 at 5:12 PM, Jeff Law l...@redhat.com wrote:
 On 11/14/13 13:28, Diego Novillo wrote:

 Functions in each corresponding .c file got moved to those
 headers and others that already existed. I wanted to make this
 patch as mechanical as possible, so I made no attempt to fix
 problems like having build_addr defined in tree-inline.c. I left
 that for later.

 This seems backwards to me and just ensures double-churn. Once to move it
 now, then again to its final resting spot.

 If this change is being made via some automated script, then, well, I guess
 it is what it is and we'll have to come back to them.  But if you're doing
 this by hand it seems to me that leaving it in its original location,
 possibly grouped with its friends, with a FIXME would be better.

Most of it was automated.  I want to stage it in, and I worked pretty
hard at not making additional changes. Particularly since it is not
clear where we will want some of these functions to end up in.  So, we
will still need several passes.  Making each pass self contained makes
sense to me.


 - Some header files always need another header file. I chose to
#include that header in the file. At this stage we want to do
the opposite, but this would've added even more bulk to the
change, so I left a FIXME marker for the next pass.

 This seems a bit like a mistake.  How much of this patch would be blocked if
 we didn't allow this right now.

A good chunk.  I'm doing these FIXMEs in the next sequence of patches,
so we won't have them for long. Again, I was going for an orderly
transition here.

Diego.

Re: Factor unrelated declarations out of tree.h (2/2)

[Diego Novillo]

On Thu, Nov 14, 2013 at 5:16 PM, Joseph S. Myers
jos...@codesourcery.com wrote:
 On Thu, 14 Nov 2013, Diego Novillo wrote:

 This patch contains the mechanical side-effects from
 http://gcc.gnu.org/ml/gcc-patches/2013-11/msg01663.html

 There are rather a lot of Include tm.h changes here - especially in
 front ends, where we've tried to eliminate tm.h calls, and put comments on
 some of those remaining saying exactly what target macros are used to make
 clear what's needed to eliminate them.  Putting in these includes, without
 clear comments explaining how to eliminate them, seems a step backwards.

These are due to builtins.h.  The structs defined in there need
FIRST_PSEUDO_REGISTER.  This means that we have parts of builtins.h
that are OK for FEs and others that aren't.  This is not good.

The best alternative for this change is to leave the declarations for
builtins.h inside tree.h and then decide what to do about builtins.h
itself. We clearly need it to declare everything related to builtins,
but from what you're stating about tm.h, we will need to have an FE
variant and an ME/BE variant?


Diego.

Re: patch ping: diagnostics finalization and plugins

[Diego Novillo]

On Mon, Nov 11, 2013 at 8:36 AM, Basile Starynkevitch
bas...@starynkevitch.net wrote:

 2013-11-11  Basile Starynkevitch  bas...@starynkevitch.net

 * toplev.c (toplev_main): Move PLUGIN_FINISH invocation before
   diagnostic_finish.

OK.


Diego.

Re: [buildrobot] [PATCH] pdp11-aout broken

[Diego Novillo]

On Sat, Nov 9, 2013 at 9:43 AM, Jan-Benedict Glaw jbg...@lug-owl.de wrote:

 /home/jbglaw/repos/gcc/gcc/cfgexpand.c: In function ‘void 
 expand_main_function()’:
 /home/jbglaw/repos/gcc/gcc/cfgexpand.c:5409:40: error: ‘NAME__MAIN’ was not 
 declared in this scope

Apologies for the breakage. I missed this message in my builds.

 2013-11-09  Jan-Benedict Glaw  jbg...@lug-owl.de

 gcc/
 * function.c (NAME__MAIN): Move to...
 * cfgexpand.c (NAME__MAIN): ...here.

This is OK.  Thanks.


Diego.

Re: [patch] Create gimple-expr..[ch] ... was Re: RFC: gimple.[ch] break apart

[Diego Novillo]

On Thu, Nov 7, 2013 at 8:23 AM, Andrew MacLeod amacl...@redhat.com wrote:
 On 11/07/2013 05:36 AM, Basile Starynkevitch wrote:

 On Tue, Nov 05, 2013 at 11:26:46AM -0500, Andrew MacLeod wrote:

 I decided to name the new file gimple-expr.[ch] instead of
 gimple-decl   This will eventually split into gimple-type.[ch],
 gimple-decl.[ch], and gimple-expr.[ch].


 Since we are adding *new* C++ files, can't we please name them *.cc
 for the implementation part, so at least create gimple-expr.h and
 gimple-expr.cc but not gimple-expr.c, please!

 Assuming we put this into stage 1 next year,  I would imagine we'd rename a
 number of things, including .cc, drop the tree-* from the tree-ssa-blah.[c]h
 files, etc etc.  There have been a few things people have suggested
 renaming... I think if we do renaming, they ought to be all at one time to
 minimize the pain.

 At the moment, the new gimple-* files I'm creating are still C, so they are
 .c files...

Agreed. When we start shuffling files around seems a better time.
Doing both operations at once will be easier than going through two
phases of naming changes.


Diego.

Re: [patch] remove superfluous sets to SSA_NAME_DEF_STMT

[Diego Novillo]

On Thu, Nov 7, 2013 at 3:14 PM, Aldy Hernandez al...@redhat.com wrote:
 SSA_NAME_DEF_STMT is set by default in gimple_build_assign(), by virtue of
 gimple_assign_set_lhs:

 static inline void
 gimple_assign_set_lhs (gimple gs, tree lhs)
 {
   GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
   gimple_set_op (gs, 0, lhs);

   if (lhs  TREE_CODE (lhs) == SSA_NAME)
 SSA_NAME_DEF_STMT (lhs) = gs;
 }


 This patch removes all the unnecessary sets to SSA_NAME_DEF_STMT in the
 compiler.

 Tested on x86-64 Linux.

 OK for trunk?

OK.


Diego.

Re: Re-factor tree.h - Part 1

[Diego Novillo]

On Wed, Nov 6, 2013 at 2:04 AM, Marc Glisse marc.gli...@inria.fr wrote:
 On Tue, 5 Nov 2013, Diego Novillo wrote:

 This is the first patch in a series of patches to cleanup tree.h to
 reduce the exposure it has all over the compiler.

 In this patch, I'm moving functions that are used once into the files
 that use them, and make them private to that file. These functions
 were declared extern in tree.h and called from exactly one place.


 I am not a big fan of doing it so automatically. For instance
 widest_int_cst_value should imho remain next to int_cst_value since they
 mostly share the same implementation. Doing this also doesn't promote code
 reuse: if I am looking for a function that does some basic operation on
 trees, I won't only need to look in the file that is semantically relevant,

Yeah, that was what I was trying to impose. Functions that
semantically make better sense in the place where they're called from.
I filtered functions that were used once but would not make much sense
to move (for instance, the init functions).

How about this. Below is the list of functions that I took out of
tree.h.  They are either not used or used only once, in which case I
moved them (and their private transitive closure) over to the file
that uses them.  There are more, but these are the ones that I
originally considered to be too specific to the user file to be
globally exposed.  Which ones would you folks keep global?

I have marked some that we may decide to keep extern and one, which I
would not mind to not move at all.  I also marked the functions I
removed and the ones that I just made private to their definition
file:

Moved to builtins.c:
more_const_call_expr_args_p
expand_stack_restore
expand_stack_save

Moved to cfgexpand.c:
expand_main_function
stack_protect_prologue
expand_asm_stmt
expand_computed_goto
expand_goto
expand_return

Moved to cgraphclones.c:
build_function_decl_skip_args

Moved to explow.c:
tree_expr_size (maybe keep extern?)

Moved to expr.c:
fields_length

Moved to fold-const.c:
size_low_cst

Moved to gimple-fold.c:
truth_type_for (maybe keep extern?)

Moved to symtab.c:
decl_assembler_name_hash
decl_assembler_name_equal

Moved to trans-mem.c:
is_tm_safe_or_pure

Moved to tree-eh.c:
in_array_bounds_p
range_in_array_bounds_p

Moved to tree-ssa-dom.c:
iterative_hash_exprs_commutative

Moved to tree-ssa-math-opts.c:
widest_int_cst_value (maybe keep extern?)

Moved to tree-vrp.c:
fold_unary_to_constant (?)

Moved to cp/call.c
ctor_to_vec

Moved to cp/decl.c:
supports_one_only
chain_member

Moved to java/class.c:
build_method_type

Removed (not used anywhere)
build_type_no_quals
omp_remove_redundant_declare_simd_attrs
fold_build3_initializer_loc
real_twop
print_vec_tree
list_equal_p
ssa_name_nonnegative_p
addr_expr_of_non_mem_decl_p
save_vtable_map_decl

Made static (only used in its defining file)
stabilize_reference_1
tree_expr_nonzero_p
tree_invalid_nonnegative_warnv_p
tree_expr_nonzero_warnv_p
fold_builtin_snprintf_chk
validate_arglist
simple_cst_list_equal
lookup_scoped_attribute_spec
get_attribute_namespace
fini_object_sizes


Thanks.  Diego.

Re: Re-factor tree.h - Part 1

[Diego Novillo]

On Wed, Nov 6, 2013 at 11:54 AM, Jeff Law l...@redhat.com wrote:
 On 11/06/13 00:04, Marc Glisse wrote:

 On Tue, 5 Nov 2013, Diego Novillo wrote:

 This is the first patch in a series of patches to cleanup tree.h to
 reduce the exposure it has all over the compiler.

 In this patch, I'm moving functions that are used once into the files
 that use them, and make them private to that file. These functions
 were declared extern in tree.h and called from exactly one place.


 I am not a big fan of doing it so automatically. For instance
 widest_int_cst_value should imho remain next to int_cst_value since they
 mostly share the same implementation. Doing this also doesn't promote
 code reuse: if I am looking for a function that does some basic
 operation on trees, I won't only need to look in the file that is
 semantically relevant, I'll also need to randomly grep through possible
 users to see if I should revert that part of your patch. On the other
 hand, most of those functions you move probably are better off in their
 new location, so you can ignore my post.

 What I think makes sense is to review what moved and where/why.  I suspect
 (but have not looked) that much of the movement of code makes sense.  Where
 it doesn't, then obviously we shouldn't make that change.

Ah, yes.  I just sent this list.  Perhaps that's easier to discuss
than the patch, which looks like line noise.


Thanks.  Diego.

Re: [PATCH] Support multiline GTY markers in Doxygen filter; add unittests

[Diego Novillo]

On Thu, Oct 31, 2013 at 4:06 PM, David Malcolm dmalc...@redhat.com wrote:
 It's possible to run GCC's sources through Doxygen by setting
 INPUT_FILTER   = contrib/filter_gcc_for_doxygen
 within contrib/gcc.doxy and invoking doxygen on the latter file.

 The script filters out various preprocessor constructs from GCC sources
 before Doxygen tries to parse them.

 However, as written, it works one line at-a-time, and thus can't cope
 with GTY markers that span multiple lines.  I added such a marker
 in r204170 (symtab_node_base), and I'd like to add more such markers
 (see http://gcc.gnu.org/ml/gcc-patches/2013-10/msg02703.html).

 So the attached patch rewrites the filtering script to cope with multiline
 GTY markers.

 I'm not familiar with Perl, so I rewrote the script in Python.

 I expanded the regexes for readability, and added a unit-testing suite.
 I also tweaked how comments get layed with @verbatim
 to avoid inconsistent indentation between the first line and subsequent
 lines within a comment.

 Tested with Python 2.7; you can see a sample of the output (from my
 gimple-as-C++-inheritance working copy) at:
 http://dmalcolm.fedorapeople.org/gcc/2013-10-31/doxygen/html/structgimple__statement__base.html

 In particular, with this patch Doxygen is able to cope with the symtab
 GTY marker, and thus parse the symtab class hierarchy, giving the output
 viewable here:
 http://dmalcolm.fedorapeople.org/gcc/2013-10-31/doxygen/html/classsymtab__node__base.html

 OK for trunk?

 contrib/
 * filter_gcc_for_doxygen: Use filter_params.py rather than
 filter_params.pl.
 * filter_params.pl: Delete in favor of...
 * filter_params.py: New, porting the perl script to python in
 order to cope with GTY markers that span multiple lines,
 tweaking the layout of comments, and adding a test suite.
 ---
  contrib/filter_gcc_for_doxygen |   2 +-
  contrib/filter_params.pl   |  14 ---
  contrib/filter_params.py   | 191 
 +
  3 files changed, 192 insertions(+), 15 deletions(-)
  delete mode 100755 contrib/filter_params.pl
  create mode 100644 contrib/filter_params.py

 diff --git a/contrib/filter_gcc_for_doxygen b/contrib/filter_gcc_for_doxygen
 index 3787eeb..ca1db31 100755
 --- a/contrib/filter_gcc_for_doxygen
 +++ b/contrib/filter_gcc_for_doxygen
 @@ -8,5 +8,5 @@
  # process is put on stdout.

  dir=`dirname $0`
 -perl $dir/filter_params.pl  $1 | perl $dir/filter_knr2ansi.pl
 +python $dir/filter_params.py $1 | perl $dir/filter_knr2ansi.pl
  exit 0
 diff --git a/contrib/filter_params.pl b/contrib/filter_params.pl
 deleted file mode 100755
 index 22dae6c..000
 --- a/contrib/filter_params.pl
 +++ /dev/null
 @@ -1,14 +0,0 @@
 -#!/usr/bin/perl
 -
 -# Filters out some of the #defines used throughout the GCC sources:
 -# - GTY(()) marks declarations for gengtype.c
 -# - PARAMS(()) is used for KR compatibility. See ansidecl.h.
 -
 -while () {
 -s/^\/\* /\/\*\* \@verbatim /;
 -s/\*\// \@endverbatim \*\//;
 -s/GTY[ \t]*\(\(.*\)\)//g;
 -s/[ \t]ATTRIBUTE_UNUSED//g;
 -s/PARAMS[ \t]*\(\((.*?)\)\)/\($1\)/sg;
 -print;
 -}
 diff --git a/contrib/filter_params.py b/contrib/filter_params.py
 new file mode 100644
 index 000..d5092f6
 --- /dev/null
 +++ b/contrib/filter_params.py
 @@ -0,0 +1,191 @@
 +#!/usr/bin/python
 +
 +Filters out some of the #defines used throughout the GCC sources:
 +- GTY(()) marks declarations for gengtype.c
 +- PARAMS(()) is used for KR compatibility. See ansidecl.h.
 +
 +When passed one or more filenames, acts on those files and prints the
 +results to stdout.
 +
 +When run without a filename, runs a unit-testing suite.
 +
 +import re
 +import sys
 +import unittest
 +
 +# Optional whitespace
 +opt_ws = '\s*'
 +
 +def filter_src(text):
 +
 +str - str.  We operate on the whole of the source file at once
 +(rather than individual lines) so that we can have multiline
 +regexes.
 +
 +
 +# First, convert tabs to spaces so that we can line things up
 +# sanely.
 +text = text.expandtabs(8)

Does it really matter?  I thought doxygen reformatted output anyway.

 +
 +# Convert C comments from GNU coding convention of:
 +#/* FIRST_LINE
 +#   NEXT_LINE
 +#   FINAL_LINE.  */
 +# to:
 +#/** @verbatim
 +#   FIRST_LINE
 +#   NEXT_LINE
 +#   FINAL_LINE.  @endverbatim */
 +# so that doxygen will parse them, and so the first line
 +# lines up correctly with subsequent lines.
 +# Single-line comments turn from:
 +#/* TEXT.  */
 +# into:
 +#/** @verbatim
 +#TEXT.  @endverbatim */

Oh, for @verbatim.  But, why @verbatim?  One trick we could do here is
recognize ALL CAPS parameters and turn them into \p PARAM. Later on,
we just emit \param.  Though I guess it would not be easy to pick up
the description. Anyway, we can think about this for later.

At the same 

Re: [PATCH] use stack vectors more

[Diego Novillo]

On Thu, Oct 31, 2013 at 7:48 PM,  tsaund...@mozilla.com wrote:
 From: Trevor Saunders tsaund...@mozilla.com

 Hi,

 This patch is pretty dull, it just replaces a bunch of things of the form
 vecT x;
 x.create (N); // N is a constant
 blah blah
 x.release ();
 by
 stack_vecT, N x;
 blah blah

 Of course its even nicer than that in some of the cases with many early 
 returns.

 bootstrapped and same test results as r204192 on x86_64-unknown-linux-gnu, ok?

 Trev

 2013-09-28  Trevor Saunders  tsaund...@mozilla.com

 cp/
 * semantics.c (build_anon_member_initialization): Convert fields to be
 a stack_vec.

 gcc/
 * function.c (reorder_blocks): Convert block_stack to a stack_vec.
 * gimplify.c (gimplify_compound_lval): Likewise.
 * graphite-clast-to-gimple.c (gloog): Likewise.
 * graphite-dependences.c (loop_is_parallel_p): Likewise.
 * graphite-scop-detection.c (scopdet_basic_block_info): Likewise.
 (limit_scop); Likewise.
 (build_scops): Likewise.
 (dot_scop): Likewise.
 * graphite-sese-to-poly.c (sese_dom_walker): Likewise.
 (build_scop_drs): Likewise.
 (insert_stmts): Likewise.
 (insert_out_of_ssa_copy): Likewise.
 (remove_phi): Likewise.
 (rewrite_commutative_reductions_out_of_ssa_close_phi): Likewise.
 * hw-doloop.c (discover_loop): Likewise.
 * tree-call-cdce.c (shrink_wrap_one_built_in_call): Likewise.
 * tree-dfa.c (dump_enumerated_decls): Likewise.
 * tree-if-conv.c (if_convertable_loop_p): Likewise.
 * tree-inline.c (tree_function_versioning): Likewise.
 * tree-loop-distribution.c (build_rdg): Likewise.
 (rdg_flag_vertex_and_dependent): Likewise.
 (distribute_loop): Likewise.
 * tree-parloops.c (loop_parallel_p): Likewise.
 (eliminate_local_variables): Likewise.
 (separate_decls_in_region): Likewise.
 * tree-predcom.c (tree_predictive_commoning_loop): Likewise.
 * tree-ssa-phiopt.c (cond_if_else_store_replacement): Likewise.
 * tree-ssa-uncprop.c (uncprop_dom_walker): Likewise.
 * tree-vect-loop.c (vect_analyze_scaler_cycles_1): Likewise.
 * tree-vect-patterns.c (vect_pattern_recog): Likewise.
 * tree-vect-stmts.c (vect_mark_stmts_to_be_vectorized): Likewise.
 (vectorizable_condition): Likewise.

The patch is OK, but it did not completely apply in my tree. Mind
sending an updated version (or point me at a git repo I can pull it
from).


Thanks.  Diego.

Re: [PATCH] use stack vectors more

[Diego Novillo]

On Fri, Nov 1, 2013 at 3:33 PM, Trevor Saunders tsaund...@mozilla.com wrote:
 The patch is OK, but it did not completely apply in my tree. Mind
 sending an updated version (or point me at a git repo I can pull it
 from).

 interesting, I just pulled and rebased it onto r204296 without any manual
 merging.  Patch against r204296 attached (obviously haven't tested it
 against that rev yet).


It must've been whitespace then.  Your new patch applied just fine.
I'll be committing shortly.


Diego.

Re: [PATCH] use stack vectors more

[Diego Novillo]

On Fri, Nov 1, 2013 at 3:51 PM, Diego Novillo dnovi...@google.com wrote:

 It must've been whitespace then.  Your new patch applied just fine.
 I'll be committing shortly.

Committed at r204301.


Diego.

Re: [PATCH] rewrite stack vectors

[Diego Novillo]

On 2013-10-10 14:07 , tsaund...@mozilla.com wrote:

This makes the implementation of stack vectors simpler and easier to use.  This 

   works by 
making the size of the on stack storage a template argument, so the
size is embedded in the type.  This allows you to implicitly convert a
stack_vecT, N to a vecT, va_heap *, and it will just work.  Because there's
no need to support stack vectors in unions we can make them be a more normal
c++ class with a constructor and destructor that are nontrivial.



Thanks.  This looks much simpler, indeed.  The patch is fine to commit.  
Just a couple of observations/questions:




@@ -638,7 +638,7 @@ propagate_threaded_block_debug_into (basic_block dest, 
basic_block src)
i++;
  }
  
-  vectree, va_stack fewvars = vNULL;

+  stack_vectree, alloc_count fewvars;
pointer_set_t *vars = NULL;


Hm, what will happen now if alloc_count == 0?  If I'm following the 
logic, this is tied to the presence of the 'vars' local, so it seems 
that we are fine.  Otherwise, the quick_push operation will run into 
trouble.
  
/* If we're already starting with 3/4 of alloc_count, go for a

@@ -646,8 +646,6 @@ propagate_threaded_block_debug_into (basic_block dest, 
basic_block src)
   VEC.  */
if (i * 4  alloc_count * 3)
  vars = pointer_set_create ();
-  else if (alloc_count)
-vec_stack_alloc (tree, fewvars, alloc_count);
  
/* Now go through the initial debug stmts in DEST again, this time

   actually inserting in VARS or FEWVARS.  Don't bother checking for
diff --git a/gcc/tree-vect-loop-manip.c b/gcc/tree-vect-loop-manip.c
index 574446a..4a14607 100644
--- a/gcc/tree-vect-loop-manip.c
+++ b/gcc/tree-vect-loop-manip.c
@@ -107,7 +107,7 @@ typedef struct
 with a PHI DEF that would soon become non-dominant, and when we got
 to the suitable one, it wouldn't have anything to substitute any
 more.  */
-static vecadjust_info, va_stack adjust_vec;
+static vecadjust_info, va_heap adjust_vec;


A file global was declared as a stack vector?  Sigh.



Diego.

Re: [patch] Flatten tree-ssa-loops.h

[Diego Novillo]

On Wed, Oct 23, 2013 at 10:27 AM, Andrew MacLeod amacl...@redhat.com wrote:
 Similar to tree-ssa.h,  tree-ssa-loops.h became an aggregator for 3 of the
 tree-ssa-loop* header files.  This remedies that situation.

 The average .c file required only 1 of the 3 includes from tree-ssa-loop.h.

 Bootstraps on x86_64-unknown-linux-gnu (with graphite enabled :-).
 Regressions are running, but I expect no issues due to this just being just
 an include shuffle.

Looks OK.

What do you think about Jan-Benedict's idea of installing a commit
hook that would check that no #includes of certain headers get into
other headers?  We could just leave this to code reviews, but perhaps
installing a hook makes it easier.


Diego.

Re: [PATCH][buildrobot] tilepro/tilegx: fallout after tree.h refactoring (was: Re-factor inclusion of tree.h)

[Diego Novillo]

On Tue, Oct 22, 2013 at 4:22 AM, Jan-Benedict Glaw jbg...@lug-owl.de wrote:
 On Mon, 2013-10-21 15:36:49 -0400, Diego Novillo dnovi...@google.com wrote:
 Can anyone think of some way that we can use to automatically block
 inclusions of tree.h from header files? Code review is the only way
 that comes to mind.

 Grep once, then install a commit hook.

Hm, that may work.  Thanks.

 I get some fallout for tilepro-linux, see
 http://toolchain.lug-owl.de/buildbot/show_build_details.php?id=21851 .

Sorry about that.  I missed it in my testing because tilepro-linux-gnu
fails early in libcpp with:

../../../gcc/libcpp/macro.c:2965:58: error: format not a string
literal and no format arguments [-Werror=format-security]
../../../gcc/libcpp/macro.c:2978:58: error: format not a string
literal and no format arguments [-Werror=format-security]


 This fixes it:

 2013-10-22  Jan-Benedict Glaw  jbg...@lug-owl.de

 * config/tilepro/tilepro.c: Include tree.h.

Sure.  It qualifies as obvious.  Thanks.


Diego.

Re-factor inclusion of tree.h

[Diego Novillo]

 +
 gcc/testsuite/gcc.dg/plugin/selfassign.c |  1 +
 gcc/testsuite/gcc.dg/plugin/start_unit_plugin.c  |  1 +
 gcc/tree-loop-distribution.c |  1 +
 gcc/tree-parloops.c  |  1 +
 gcc/tree-ssa-strlen.c|  1 +
 gcc/tree-streamer.c  |  1 +
 gcc/tree-streamer.h  |  1 -
 gcc/value-prof.c |  1 +
 49 files changed, 99 insertions(+), 9 deletions(-)

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index e839517..dfac4ec 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -88,6 +88,50 @@
(ix86_expand_movmem): Call ix86_expand_set_or_movmem.
(ix86_expand_setmem): Call ix86_expand_set_or_movmem.
 
+2013-10-21  Diego Novillo  dnovi...@google.com
+
+   * asan.c: Include tree.h
+   * bb-reorder.c: Likewise.
+   * cfgcleanup.c: Likewise.
+   * cfgloopmanip.c: Likewise.
+   * data-streamer-in.c: Likewise.
+   * data-streamer-out.c: Likewise.
+   * data-streamer.c: Likewise.
+   * dwarf2cfi.c: Likewise.
+   * graphite-blocking.c: Likewise.
+   * graphite-clast-to-gimple.c: Likewise.
+   * graphite-dependences.c: Likewise.
+   * graphite-interchange.c: Likewise.
+   * graphite-optimize-isl.c: Likewise.
+   * graphite-poly.c: Likewise.
+   * graphite-scop-detection.c: Likewise.
+   * graphite-sese-to-poly.c: Likewise.
+   * graphite.c: Likewise.
+   * ipa-devirt.c: Likewise.
+   * ipa-profile.c: Likewise.
+   * ipa.c: Likewise.
+   * ira.c: Likewise.
+   * loop-init.c: Likewise.
+   * loop-unroll.c: Likewise.
+   * lower-subreg.c: Likewise.
+   * lto/lto-object.c: Likewise.
+   * recog.c: Likewise.
+   * reginfo.c: Likewise.
+   * tree-loop-distribution.c: Likewise.
+   * tree-parloops.c: Likewise.
+   * tree-ssa-strlen.c: Likewise.
+   * tree-streamer.c: Likewise.
+   * value-prof.c: Likewise.
+   * expr.h: Include tree-core.h instead of tree.h.
+   * gimple.h: Likewise.
+   * ipa-prop.h: Likewise.
+   * ipa-utils.h: Likewise.
+   * lto-streamer.h: Likewise.
+   * streamer-hooks.h: Likewise.
+   * ipa-reference.h: Include cgraph.h instead of tree.h.
+   * cgraph.h: Include basic-block.h instead of tree.h.
+   * tree-streamer.h: Do not include tree.h.
+
 2013-10-20  Bill Schmidt  wschm...@linux.vnet.ibm.com
 
* config/rs6000/altivec.md (vec_unpacku_hi_v16qi): Adjust for
diff --git a/gcc/asan.c b/gcc/asan.c
index c037ebf..a5978df 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -22,6 +22,7 @@ along with GCC; see the file COPYING3.  If not see
 #include config.h
 #include system.h
 #include coretypes.h
+#include tree.h
 #include gimple.h
 #include tree-iterator.h
 #include tree-ssa.h
diff --git a/gcc/bb-reorder.c b/gcc/bb-reorder.c
index c5a42d3..8e2348f 100644
--- a/gcc/bb-reorder.c
+++ b/gcc/bb-reorder.c
@@ -82,6 +82,7 @@
 #include system.h
 #include coretypes.h
 #include tm.h
+#include tree.h
 #include rtl.h
 #include regs.h
 #include flags.h
diff --git a/gcc/cfgcleanup.c b/gcc/cfgcleanup.c
index 53a9975..5161190 100644
--- a/gcc/cfgcleanup.c
+++ b/gcc/cfgcleanup.c
@@ -34,6 +34,7 @@ along with GCC; see the file COPYING3.  If not see
 #include coretypes.h
 #include tm.h
 #include rtl.h
+#include tree.h
 #include hard-reg-set.h
 #include regs.h
 #include insn-config.h
diff --git a/gcc/cfgloopmanip.c b/gcc/cfgloopmanip.c
index b4840dc..cf9a7fc 100644
--- a/gcc/cfgloopmanip.c
+++ b/gcc/cfgloopmanip.c
@@ -24,6 +24,7 @@ along with GCC; see the file COPYING3.  If not see
 #include rtl.h
 #include basic-block.h
 #include cfgloop.h
+#include tree.h
 #include tree-ssa.h
 #include dumpfile.h
 
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index 69adf4d..7706419 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -24,7 +24,7 @@ along with GCC; see the file COPYING3.  If not see
 #include is-a.h
 #include plugin-api.h
 #include vec.h
-#include tree.h
+#include basic-block.h
 #include function.h
 #include ipa-ref.h
 
diff --git a/gcc/data-streamer-in.c b/gcc/data-streamer-in.c
index 93fe2ff..84db7cf 100644
--- a/gcc/data-streamer-in.c
+++ b/gcc/data-streamer-in.c
@@ -24,6 +24,7 @@ along with GCC; see the file COPYING3.  If not see
 #include system.h
 #include coretypes.h
 #include diagnostic.h
+#include tree.h
 #include data-streamer.h
 
 /* Read a string from the string table in DATA_IN using input block
diff --git a/gcc/data-streamer-out.c b/gcc/data-streamer-out.c
index 247ff63..2e55e3d 100644
--- a/gcc/data-streamer-out.c
+++ b/gcc/data-streamer-out.c
@@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.  If not see
 #include config.h
 #include system.h
 #include coretypes.h
+#include tree.h
 #include data-streamer.h
 
 /* Return index used to reference STRING of LEN characters in the string table
diff --git a/gcc/data-streamer.c b/gcc/data-streamer.c
index cbd1cb9..5915a41 100644
--- a/gcc/data-streamer.c

Re: Re-factor inclusion of tree.h

[Diego Novillo]

On Mon, Oct 21, 2013 at 12:57 PM, Jeff Law l...@redhat.com wrote:
 On 10/21/13 10:52, Diego Novillo wrote:

 I plan to commit this by tomorrow, unless there are objections.

 I can't think of a good reason to even bother waiting :-)

Heh, OK, thanks.

After analyzing all the build failures in config-list.mk, I found that
we also need explicit inclusion of tree.h in the gen* binaries. Some
of these generate all their includes in header files, but I figured we
can revise this later on. This was breaking cr16-elf and mips-netbsd.

Can anyone think of some way that we can use to automatically block
inclusions of tree.h from header files? Code review is the only way
that comes to mind.

Committed both patches to trunk.

Diego.

diff --git a/gcc/genattrtab.c b/gcc/genattrtab.c
index 973cade..f79380d 100644
--- a/gcc/genattrtab.c
+++ b/gcc/genattrtab.c
@@ -5100,6 +5100,7 @@ write_header (FILE *outf)
   fprintf (outf, #include \system.h\\n);
   fprintf (outf, #include \coretypes.h\\n);
   fprintf (outf, #include \tm.h\\n);
+  fprintf (outf, #include \tree.h\\n);
   fprintf (outf, #include \rtl.h\\n);
   fprintf (outf, #include \insn-attr.h\\n);
   fprintf (outf, #include \tm_p.h\\n);
diff --git a/gcc/genautomata.c b/gcc/genautomata.c
index a0bf076..f6c4b91c4 100644
--- a/gcc/genautomata.c
+++ b/gcc/genautomata.c
@@ -9665,6 +9665,7 @@ main (int argc, char **argv)
  #include \system.h\\n
  #include \coretypes.h\\n
  #include \tm.h\\n
+ #include \tree.h\\n
  #include \rtl.h\\n
  #include \tm_p.h\\n
  #include \insn-config.h\\n
diff --git a/gcc/genemit.c b/gcc/genemit.c
index d4bb301..724a114 100644
--- a/gcc/genemit.c
+++ b/gcc/genemit.c
@@ -790,6 +790,7 @@ from the machine description file `md'.  */\n\n);
   printf (#include \system.h\\n);
   printf (#include \coretypes.h\\n);
   printf (#include \tm.h\\n);
+  printf (#include \tree.h\\n);
   printf (#include \rtl.h\\n);
   printf (#include \tm_p.h\\n);
   printf (#include \function.h\\n);
diff --git a/gcc/genopinit.c b/gcc/genopinit.c
index 9c7cf2c..3efb71e 100644
--- a/gcc/genopinit.c
+++ b/gcc/genopinit.c
@@ -404,6 +404,7 @@ main (int argc, char **argv)
#include \system.h\\n
#include \coretypes.h\\n
#include \tm.h\\n
+   #include \tree.h\\n
#include \rtl.h\\n
#include \tm_p.h\\n
#include \flags.h\\n
diff --git a/gcc/genoutput.c b/gcc/genoutput.c
index c3a0936..2a7ee23 100644
--- a/gcc/genoutput.c
+++ b/gcc/genoutput.c
@@ -238,6 +238,7 @@ output_prologue (void)
   printf (#include \tm.h\\n);
   printf (#include \flags.h\\n);
   printf (#include \ggc.h\\n);
+  printf (#include \tree.h\\n);
   printf (#include \rtl.h\\n);
   printf (#include \expr.h\\n);
   printf (#include \insn-codes.h\\n);
diff --git a/gcc/genpeep.c b/gcc/genpeep.c
index a14d061..877fde3 100644
--- a/gcc/genpeep.c
+++ b/gcc/genpeep.c
@@ -359,6 +359,7 @@ from the machine description file `md'.  */\n\n);
   printf (#include \coretypes.h\\n);
   printf (#include \tm.h\\n);
   printf (#include \insn-config.h\\n);
+  printf (#include \tree.h\\n);
   printf (#include \rtl.h\\n);
   printf (#include \tm_p.h\\n);
   printf (#include \regs.h\\n);
diff --git a/gcc/target-globals.c b/gcc/target-globals.c
index 65ccb8a..9d223fc 100644
--- a/gcc/target-globals.c
+++ b/gcc/target-globals.c
@@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.  If not see
 #include tm.h
 #include insn-config.h
 #include machmode.h
+#include tree.h
 #include ggc.h
 #include toplev.h
 #include target-globals.h

Re: [PATCH][i386]Fix PR 57756

[Diego Novillo]

On Wed, Oct 16, 2013 at 6:06 PM, David Edelsohn dje@gmail.com wrote:

 How is Google going to change its patch commit policies to ensure that
 this does not happen again?

There is nothing to change.  Google follows
http://gcc.gnu.org/contribute.html, like everyone else. Sri just fixed
the oversight and if there is any other fallout from his patch, he
will address it.

I don't see anything out of the ordinary here.


Diego.

Re: [patch] The remainder of tree-flow.h refactored.

[Diego Novillo]

On Wed, Oct 9, 2013 at 11:37 AM, Andrew MacLeod amacl...@redhat.com wrote:

 bootstraps on x86_64-unknown-linux-gnu, regressions test are still running.
 OK?

Sure.

Re: Ping^6: contribute Synopsys Designware ARC port

[Diego Novillo]

On Sat, Sep 28, 2013 at 9:54 AM, Joern Rennecke
joern.renne...@embecosm.com wrote:
 The main part of the port (everything but the testsuite) is still waiting
 for review:
 http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00323.html
 http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00324.html
 http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00325.html
 http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00328.html
 http://gcc.gnu.org/ml/gcc-patches/2013-09/msg01870.html
 http://gcc.gnu.org/ml/gcc-patches/2013-09/msg02070.html

 I've retested a i686-pc-linux-gnu native bootstrap as well as the obvious
 arc-elf32 / arc-linux-uclibc builds in trunk r202981.

I have been reviewing these patches (I've gone through 2), and so far
I find nothing surprising in them.  I should be able to finish them
today or tomorrow.  Joern, I assume that you'll be one of the
maintainers for the port?  Anyone else?

SC folks, could you appoint Joern (and any other volunteer that Joern
suggests) as maintainers?

Thanks.  Diego.

Re: Ping^6: contribute Synopsys Designware ARC port

[Diego Novillo]

On Tue, Oct 1, 2013 at 10:10 AM, Joern Rennecke
joern.renne...@embecosm.com wrote:

 Yes.  Claudiu Zissulescu at Synopsys would in principle be available as
 co-maintainer, but I suppose it is customary to apply for write-after-
 approval status first.

I'm not sure.  A question for the SC.

 SC folks, could you appoint Joern (and any other volunteer that Joern
 suggests) as maintainers?


 I've already been appointed as maintainer back in January.

OK, great.  Here's me paying attention :)

Re: Ping^6: contribute Synopsys Designware ARC port

[Diego Novillo]

On Sat, Sep 28, 2013 at 9:54 AM, Joern Rennecke
joern.renne...@embecosm.com wrote:
 The main part of the port (everything but the testsuite) is still waiting
 for review:
 http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00323.html
 http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00324.html
 http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00325.html
 http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00328.html
 http://gcc.gnu.org/ml/gcc-patches/2013-09/msg01870.html
 http://gcc.gnu.org/ml/gcc-patches/2013-09/msg02070.html

I have finished reading through these patches.  They are OK to commit.

The changes indicated below are minor. Ideally, you'd address them
before committing the patch, but if it's easier to do it post-commit,
that's OK too.

- The Copyright years should be 2013 in every new file.  Or has this
port been released before?

- In config/arc/arc-protos.h:
+/* insn-attrtab.c doesn't include reload.h, which declares
regno_clobbered_p. */
+extern int regno_clobbered_p (unsigned int, rtx, enum machine_mode, int);

Why not include reload.h here?  Interface changes (however rare) make
this a hassle.

- In config/arc/simdext.md
+;; Va, [Ib,u8] instructions
+;; (define_insn vld32wh_insn
+;;   [(set (match_operand:V8HI 0 vector_register_operand   =v)
+;; (vec_concat:V8HI (unspec:V4HI [(match_operand:SI 1 immediate_operand P)
+;;  (vec_select:HI (match_operand:V8HI 2 vector_register_operand  v)
+;;  (parallel [(match_operand:SI 3 immediate_operand L)]))]
UNSPEC_ARC_SIMD_VLD32WH)
+;; (vec_select:V4HI (match_dup 0)

Necessary?  If so, please add a comment stating why it's commented out.

- In doc/extend.texi:
+Permissible values for this parameter are: @w{@code{ilink1}} and
+@w{@code{ilink2}}.
+

ARC developers already know what ilink1 and ilink2 mean?

+@cindex indirect calls on Epiphany
+These attribute specifies how a particular function is called on
+ARC, ARM and Epiphany

s/specifies/specify/


+because __alignof__ sees only the type of the dereference, wheras
+__builtin_arc_align uses alignment information from the pointer

s/wheras/whereas/

- I have not fully cross-referenced the list of documented builtins vs
the list of implemented builtins. Please double check them.

- Ditto the list of -m options. It looks like they're all documented,
but I haven't diff'd the doc vs the options file.

- In libgcc/config/arc/gmon/mcount.c

The file has a different copyright/license notice at the top.  Is this
from a third party source? Can it be changed to lgpl?

+#if 0
+ if (catomic_compare_and_exchange_bool_acq (p-state, GMON_PROF_BUSY,
+   GMON_PROF_ON))
+  return;

Get rid of this?

+#elif defined (__ARC700__)
+/* ??? This could temporrarily loose the ERROR / OFF condition in a race,

s/temporrarily/temporarily/
s/loose/lose/

- Many files in libgcc/config/arc/... have #if0 blocks. Are they
really necessary?

- In libgcc/config/arc/ieee-754/arc600-dsp/muldf3.S
+/* We have checked for infinitey / NaN input before, and transformed
+   denormalized inputs into normalized inputs.  Thus, the worst case

s/infinitey/infinity/

It  also happens in another muldf3.S file in a sibling directory.

- libgcc/config/arc/t-arc-newlib does not contain the exception clause.

- In config/arc/arc.md there are several define patterns commented
out.  Toss them out?

- In config/arc/arc.c:

No need to include stdio.h

No need to mark struct arc_frame_info with GTY. It contains no pointers.

In arc_expand_epilogue():
Get rid of fp_restored_p

  if (1)
{
  unsigned int pretend_size = cfun-machine-frame_info.pretend_size;

Just move everything out of the if()?

In output_shift(): Get rid of the #if 1s?

In arc_encode_section_info():

/* Symbols in the text segment can be accessed without indirecting via the
   constant pool; it may take an extra binary operation, but this is still
   faster than indirecting via memory.  Don't do this when not optimizing,
   since we won't be calculating al of the offsets necessary to do this
   simplification.  */

But that seems out of sync with the code. It never checks whether
optimizations are enabled.


Thanks.  Diego.

Re: GTY on simple struct (Was: Re: Ping^6: contribute Synopsys Designware ARC port)

[Diego Novillo]

On Tue, Oct 1, 2013 at 5:50 PM, Joern Rennecke
joern.renne...@embecosm.com wrote:
 Quoting Diego Novillo dnovi...@google.com:

 No need to mark struct arc_frame_info with GTY. It contains no pointers.


 That's not quite how it works.  machine_function needs GTY.  It uses
 arc_frame_info, hence arc_frame_info also needs GTY.

Gah, you're right.  I missed that connection.  Silly GC.


Diego.

Remove enum ssa_mode

[Diego Novillo]

The gimple builder no longer support normal form. The ssa_mode
enum is not needed now.

Committed to trunk.

* gimple.h (enum ssa_mode): Remove.
---
 gcc/gimple.h | 9 -
 1 file changed, 9 deletions(-)

diff --git a/gcc/gimple.h b/gcc/gimple.h
index 3047ab4..a031c8d 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -34,15 +34,6 @@ along with GCC; see the file COPYING3.  If not see
 
 typedef gimple gimple_seq_node;
 
-/* Types of supported temporaries.  GIMPLE temporaries may be symbols
-   in normal form (i.e., regular decls) or SSA names.  This enum is
-   used by create_gimple_tmp to tell it what kind of temporary the
-   caller wants.  */
-enum ssa_mode {
-M_SSA = 0,
-M_NORMAL
-};
-
 /* For each block, the PHI nodes that need to be rewritten are stored into
these vectors.  */
 typedef vecgimple gimple_vec;
-- 
1.8.4

Re: [google, patch] Google-local version of fix for PR58312, libssp cross-compiling

[Diego Novillo]

On Fri, Sep 20, 2013 at 5:08 PM, Brooks Moses bmo...@google.com wrote:

 Thus, this Google-local patch addresses our immediate need in a simple
 way.  Ok to commit to google/main and merge to google/gcc-4_8?

OK.  This should go in google/integration, actually.  google/main can
get it later when it merges from g/i (unless it's needed there now?)


Thanks.  Diego.

Re: gimple build interface

[Diego Novillo]

On Thu, Sep 19, 2013 at 9:20 AM, Andrew MacLeod amacl...@redhat.com wrote:

 I see the benefit in the streamlined asan.c code,  but I detest that
 ssa_mode flag.  And as long as it supports SSA, I don't think it should be
 in gimple.c.

Yeah, at the time that I introduced it, I had a hard time with the
normal/ssa form duality. I think it would be fine to only support SSA
form in this interface. The code that deals with gimple in normal form
is very short and low-level, anyway. I don't expect most developers to
need it. The only potential users are those generating normal code and
then putting it into ssa form, but that should be rare.

 I'd also suggest that the final optional parameter be changed to   tree *lhs
 = NULL_TREE,  which would allow the caller to specify the LHS if they want,
 otherwise make_ssa_name would be called.   If we want to leave it supporting
 both gimple and ssa, then anyone from gimple land could pass in a gimple LHS
 variable thus avoiding the call to make_ssa_name

Sure.


Diego.

Re: gimple build interface

[Diego Novillo]

On Fri, Sep 20, 2013 at 4:08 AM, Richard Biener
richard.guent...@gmail.com wrote:
 On Thu, Sep 19, 2013 at 6:56 PM, Andrew MacLeod amacl...@redhat.com wrote:
 On 09/19/2013 09:24 AM, Andrew MacLeod wrote:


 I think this is of most use to ssa passes that need to construct code
 snippets, so I propose we make this ssa specific and put it in tree-ssa.c
 (renaming it ssa_build_assign),  *OR* we could leave it general purpose and
 put it in its own set of files, gimple-ssa-build.[ch] or something that
 crosses the border between the two representations.

 I'd also suggest that the final optional parameter be changed to tree *lhs
 = NULL_TREE,  which would allow the caller to specify the LHS if they want,
 otherwise make_ssa_name would be called. If we want to leave it supporting
 both gimple and ssa, then anyone from gimple land could pass in a gimple LHS
 variable thus avoiding the call to make_ssa_name

 Thoughts?
 Andrew

 Anyway, here is a patch which does that and a bit more.  I didn't rename
 build_assign() to ssa_build_assign()..   even though those are the only kind
 actually created right now.   we can leave that for the day someone actually
 decides to flush this interface out, and maybe we'll want to pass in
 gimple_tmps and call them from front ends or other places... then it would
 have to be renamed again. So I just left it as is for the moment, but that
 could be changed.

 I also moved gimple_replace_lhs() to tree-ssa.c and renamed it
 ssa_replace_lhs(). It calls insert_debug_temp_for_var_def() from tree-ssa.c
 and that only works with the immediate use operands.. so that is an SSA
 specific routine, which makes this one SSA specific as well.

 Those 2 changes allow tree-ssa.h to no longer be included, it is replaced
 with tree-flow.h.   Some preliminary work to enable removing immediate use
 routines out of tree-flow.h include:

 struct count_ptr_d, count_ptr_derefs(), count_uses_and_derefs() also get
 moved to tree-ssa.c since those are also require the immediate use
 mechanism, and thus is also SSA dependent.

 This bootstraps on x86_64-unknown-linux-gnu and has no new regressions.
 OK?

 Can you move the builders to asan.c please?

No.  They don't belong there.  This is a high-level wrapper for the
low-level instruction builders.

 to have various issues so it shouldn't be used (I wonder who approved them
 in the end ... maybe it was even me).

Not really. I put it in and still need to flush it out a bit more.
I'll work on it.


Diego.

Re: Ping^2: small patch to accept = inside GCC plugin arguments

[Diego Novillo]

On Mon, Sep 16, 2013 at 4:23 AM, Basile Starynkevitch
bas...@starynkevitch.net wrote:
 Hello All,

 I'm pinging again my small patch to accept = inside plugin arguments
 http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00382.html

OK.


Diego.

Re: Ping patch to enable *.cc files in gengtype

[Diego Novillo]

On 2013-09-16 04:19 , Basile Starynkevitch wrote:

Hello all,

I'm pinging the patch (of september 2nd) on
http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00036.html


 gcc/ChangeLog entry

2013-09-16  Basile Starynkevitch  bas...@starynkevitch.net

* gengtype.c (file_rules): Added rule for *.cc files.
(get_output_file_with_visibility): Give fatal message when no
rules found.


OK.


Diego.

Re: RFC - Refactor tree.h

[Diego Novillo]

On Fri, Sep 13, 2013 at 11:25 AM, Diego Novillo dnovi...@google.com wrote:
 On Fri, Sep 13, 2013 at 11:07 AM, Jakub Jelinek ja...@redhat.com wrote:

 E.g. today I've noticed you've lost OMP_CLAUSE_LINEAR_NO_COPYIN
 comment that has been added to tree.h recently, but you haven't
 actually moved it into tree-core.h.

 Sorry about that.  I remember an update conflict, but I thought I had
 incorporated it all.  I will double check and fix.


Done.

 gcc/ChangeLog   |  5 +
 gcc/tree-core.h | 19 +--
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index b9a335a..906d01b 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,8 @@
+2013-09-16  Diego Novillo  dnovi...@google.com
+
+   * tree-core.h: Add missing comment lines from refactoring
+   of tree.h.
+
 2013-09-16  Jan Hubicka  j...@suse.cz

* gimple-fold.c (can_refer_decl_in_current_unit_p): Do not accept
diff --git a/gcc/tree-core.h b/gcc/tree-core.h
index b1bc56a..69777dc 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -780,6 +780,9 @@ struct GTY(()) tree_base {
OMP_CLAUSE_PRIVATE_DEBUG in
OMP_CLAUSE_PRIVATE

+   OMP_CLAUSE_LINEAR_NO_COPYIN in
+  OMP_CLAUSE_LINEAR
+
TRANSACTION_EXPR_RELAXED in
   TRANSACTION_EXPR

@@ -800,6 +803,9 @@ struct GTY(()) tree_base {
OMP_CLAUSE_PRIVATE_OUTER_REF in
   OMP_CLAUSE_PRIVATE

+   OMP_CLAUSE_LINEAR_NO_COPYOUT in
+  OMP_CLAUSE_LINEAR
+
TYPE_REF_IS_RVALUE in
   REFERENCE_TYPE

@@ -935,6 +941,9 @@ struct GTY(()) tree_base {

DECL_NONLOCAL_FRAME in
   VAR_DECL
+
+   TYPE_FINAL_P in
+  RECORD_TYPE, UNION_TYPE and QUAL_UNION_TYPE
 */

 struct GTY(()) tree_typed {
@@ -1197,8 +1206,7 @@ struct GTY(()) tree_decl_common {
   unsigned lang_flag_7 : 1;
   unsigned lang_flag_8 : 1;

-  /* In LABEL_DECL, this is DECL_ERROR_ISSUED.
- In VAR_DECL and PARM_DECL, this is DECL_REGISTER.  */
+  /* In VAR_DECL and PARM_DECL, this is DECL_REGISTER.  */
   unsigned decl_flag_0 : 1;
   /* In FIELD_DECL, this is DECL_BIT_FIELD
  In VAR_DECL and FUNCTION_DECL, this is DECL_EXTERNAL.
@@ -1403,6 +1411,9 @@ struct GTY(()) tree_statement_list
   struct tree_statement_list_node *tail;
 };

+
+/* Optimization options used by a function.  */
+
 struct GTY(()) tree_optimization_option {
   struct tree_common common;

@@ -1418,6 +1429,8 @@ struct GTY(()) tree_optimization_option {
   struct target_optabs *GTY ((skip)) base_optabs;
 };

+/* Target options used by a function.  */
+
 struct GTY(()) tree_target_option {
   struct tree_common common;

@@ -1563,6 +1576,8 @@ struct GTY(()) tree_map_base {
   tree from;
 };

+/* Map from a tree to another tree.  */
+
 struct GTY(()) tree_map {
   struct tree_map_base base;
   unsigned int hash;
--
1.8.4

Re: RFC - Refactor tree.h

[Diego Novillo]

On Fri, Sep 13, 2013 at 11:07 AM, Jakub Jelinek ja...@redhat.com wrote:

 E.g. today I've noticed you've lost OMP_CLAUSE_LINEAR_NO_COPYIN
 comment that has been added to tree.h recently, but you haven't
 actually moved it into tree-core.h.

Sorry about that.  I remember an update conflict, but I thought I had
incorporated it all.  I will double check and fix.


Diego.

Re: [PATCH, libvtv] Fix most of the testsuite.

[Diego Novillo]

On Thu, Sep 5, 2013 at 4:52 PM, Caroline Tice cmt...@google.com wrote:
 Ping?  Could somebody please review this for me?

Mike already approved this upthread.

Re: [PATCH, vtv update] Add documentation for --enable-vtable-verify configure option...

[Diego Novillo]

On 2013-08-28 17:15 , Caroline Tice wrote:


 # Least ordering for dependencies mean linking w/o libstdc++ for as
 # long as the development of libvtv does not absolutely require it.
Index: gcc/doc/install.texi
===
--- gcc/doc/install.texi(revision 202060)
+++ gcc/doc/install.texi(working copy)
@@ -1032,6 +1032,16 @@ and for cross builds configured with @op
 More documentation about multiarch can be found at
 @uref{http://wiki.debian.org/Multiarch}.

+@item --enable-vtable-verify
+Specify whether to enable or disable the vtable verification feature.
+Enabling this feature causes libstdc++ to be built with its virtual calls
+in verifiable mode.  This means that, when linked with libvtv, every
+virtual call in libstdc++ will verify the vtable pointer through 
which the
+call will be made before actually making the call.  If not linked 
with libvtv,
+the verifier will call stub functions (in libstdc++ itself) and do 
nothing.

+If vtable verification is disabled, then libstdc++ is not built with its
+virutal calls in verifiable mode at all.


s/virutal/virtual/

Could you clarify in the docs whether --enable-vtable-verify is the 
default behaviour or not?


Why would I need --disable-vtv, if I can use --disable-vtable-verify?


OK with those changes.


Diego.

Re: RFC - Next refactoring steps

[Diego Novillo]

On Fri, Sep 6, 2013 at 4:14 AM, Mike Stump mikest...@comcast.net wrote:
 On Sep 5, 2013, at 11:54 PM, Richard Biener richard.guent...@gmail.com 
 wrote:
 Most of the GCC headerfiles do not include all their required headers but
 rely on .c files doing that (in the appropriate order).  I somehow like
 that though I cannot explain why ;)

 Very old school.  I can explain why I don't like it, but I'll resist.  The 
 universal standard is for each header to include all it needs and for the 
 ordering of includes not to matter any.  Deviations from this are the 
 exception and should virtually never should be the case.

Agreed. Header files should be self-contained and include everything
they need. This will be increasingly more important in the presence of
C++ modules.


Diego.

Factor gimple structures out of gimple.h

[Diego Novillo]

This patch introduces gimple-core.h, which contains just the data
structures needed to define gimple. I left everything else in
gimple.h

The addition of alias.h to tree-ssa-alias.h is so that we can
include tree-ssa-alias.h in isolation. It now includes everything
it needs.

More discussion on rationale at the thread:
http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00300.html

Tested on x86_64.


2013-09-06  Diego Novillo  dnovi...@google.com

* Makefile.in (GIMPLE_CORE_H): New.
(GIMPLE_H): Depend on GIMPLE_CORE_H.
(TREE_SSA_ALIAS_H): New. Replace references to tree-ssa-alias.h with
TREE_SSA_ALIAS_H.
* gimple-core.h: New. Factor all gimple data structures out of ...
* gimple.h: ... here.
* tree-ssa-alias.h: Include alias.h.

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index a72b753..2be8846 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -882,15 +882,19 @@ TREE_H = tree.h $(TREE_CORE_H)  tree-check.h
 REGSET_H = regset.h $(BITMAP_H) hard-reg-set.h
 BASIC_BLOCK_H = basic-block.h $(PREDICT_H) $(VEC_H) $(FUNCTION_H) \
cfg-flags.def cfghooks.h
-GIMPLE_H = gimple.h gimple.def gsstruct.def pointer-set.h $(VEC_H) \
+GIMPLE_CORE_H = gimple-core.h $(CONFIG_H) $(SYSTEM_H) $(CORETYPES_H) \
+   $(INPUT_H) gimple.def gsstruct.def $(TREE_SSA_ALIAS_H) \
+   $(INTERNAL_FN_H) $(TREE_CORE_H)
+GIMPLE_H = gimple.h $(GIMPLE_CORE_H) pointer-set.h $(VEC_H) \
$(GGC_H) $(BASIC_BLOCK_H) $(TREE_H) tree-ssa-operands.h \
-   tree-ssa-alias.h $(INTERNAL_FN_H) $(HASH_TABLE_H)
+   $(HASH_TABLE_H)
 TRANS_MEM_H = trans-mem.h
 GCOV_IO_H = gcov-io.h gcov-iov.h auto-host.h
 COVERAGE_H = coverage.h $(GCOV_IO_H)
 DEMANGLE_H = $(srcdir)/../include/demangle.h
 RECOG_H = recog.h
 ALIAS_H = alias.h
+TREE_SSA_ALIAS_H = tree-ssa-alias.h $(ALIAS_H)
 EMIT_RTL_H = emit-rtl.h
 FLAGS_H = flags.h flag-types.h $(OPTIONS_H)
 OPTIONS_H = options.h flag-types.h $(OPTIONS_H_EXTRA)
@@ -946,7 +950,7 @@ TREE_PASS_H = tree-pass.h $(TIMEVAR_H) $(DUMPFILE_H)
 TREE_FLOW_H = tree-flow.h tree-flow-inline.h tree-ssa-operands.h \
$(BITMAP_H) sbitmap.h $(BASIC_BLOCK_H) $(GIMPLE_H) \
$(HASHTAB_H) $(CGRAPH_H) $(IPA_REFERENCE_H) \
-   tree-ssa-alias.h
+   $(TREE_SSA_ALIAS_H)
 TREE_HASHER_H = tree-hasher.h $(HASH_TABLE_H) $(TREE_FLOW_H)
 TREE_SSA_LIVE_H = tree-ssa-live.h $(PARTITION_H)
 SSAEXPAND_H = ssaexpand.h $(TREE_SSA_LIVE_H)
@@ -2234,7 +2238,7 @@ test-dump.o : test-dump.c $(CONFIG_H) $(SYSTEM_H) 
$(CORETYPES_H) \
$(BITMAP_H) sbitmap.h sreal.h $(TREE_H) $(CXX_PARSER_H) $(DWARF2OUT_H) \
$(GIMPLE_PRETTY_PRINT_H) $(BASIC_BLOCK_H) insn-config.h $(LRA_INT.H) \
$(SEL_SCHED_DUMP_H) $(IRA_INT_H) $(TREE_DATA_REF_H) $(TREE_FLOW_H) \
-   $(TREE_SSA_LIVE_H) tree-ssa-alias.h $(OMEGA_H) $(RTL_H)
+   $(TREE_SSA_LIVE_H) $(TREE_SSA_ALIAS_H) $(OMEGA_H) $(RTL_H)
 tree.o: tree.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(TREE_H) \
all-tree.def $(FLAGS_H) $(FUNCTION_H) $(PARAMS_H) \
toplev.h $(DIAGNOSTIC_CORE_H) $(GGC_H) $(HASHTAB_H) $(TARGET_H) output.h 
$(TM_P_H) \
@@ -2722,7 +2726,7 @@ targhooks.o : targhooks.c $(CONFIG_H) $(SYSTEM_H) 
coretypes.h $(TREE_H) \
$(EXPR_H) $(TM_H) $(RTL_H) $(TM_P_H) $(FUNCTION_H) output.h 
$(DIAGNOSTIC_CORE_H) \
$(MACHMODE_H) $(TARGET_DEF_H) $(TARGET_H) $(GGC_H) gt-targhooks.h \
$(OPTABS_H) $(RECOG_H) $(REGS_H) reload.h hard-reg-set.h intl.h $(OPTS_H) \
-   tree-ssa-alias.h $(TREE_FLOW_H)
+   $(TREE_SSA_ALIAS_H) $(TREE_FLOW_H)
 common/common-targhooks.o : common/common-targhooks.c $(CONFIG_H) $(SYSTEM_H) \
coretypes.h $(INPUT_H) $(TM_H) $(COMMON_TARGET_H) common/common-targhooks.h
 
@@ -2746,7 +2750,7 @@ toplev.o : toplev.c $(CONFIG_H) $(SYSTEM_H) coretypes.h 
$(TM_H) $(TREE_H) \
langhooks.h insn-flags.h $(CFGLOOP_H) hosthooks.h \
$(CGRAPH_H) $(COVERAGE_H) alloc-pool.h $(GGC_H) \
$(OPTS_H) params.def tree-mudflap.h $(TREE_PASS_H) $(GIMPLE_H) \
-   tree-ssa-alias.h $(PLUGIN_H) realmpfr.h tree-diagnostic.h \
+   $(TREE_SSA_ALIAS_H) $(PLUGIN_H) realmpfr.h tree-diagnostic.h \
$(TREE_PRETTY_PRINT_H) opts-diagnostic.h $(COMMON_TARGET_H) \
tsan.h diagnostic-color.h $(CONTEXT_H) $(PASS_MANAGER_H)
 
@@ -3303,7 +3307,7 @@ alias.o : alias.c $(CONFIG_H) $(SYSTEM_H) coretypes.h 
$(DUMPFILE_H) \
$(ALIAS_H) $(EMIT_RTL_H) $(GGC_H) $(FUNCTION_H) cselib.h $(TREE_H) 
$(TM_P_H) \
langhooks.h $(TARGET_H) gt-alias.h $(TIMEVAR_H) $(CGRAPH_H) \
$(SPLAY_TREE_H) $(DF_H) \
-   tree-ssa-alias.h pointer-set.h $(TREE_FLOW_H)
+   $(TREE_SSA_ALIAS_H) pointer-set.h $(TREE_FLOW_H)
 stack-ptr-mod.o : stack-ptr-mod.c $(CONFIG_H) $(SYSTEM_H) coretypes.h \
$(TM_H) $(TREE_H) $(RTL_H) $(REGS_H) $(EXPR_H) $(TREE_PASS_H) \
$(BASIC_BLOCK_H) $(FLAGS_H) output.h $(DF_H)
@@ -3819,7 +3823,7 @@ GTFILES = $(CPP_ID_DATA_H) $(srcdir)/input.h 
$(srcdir)/coretypes.h \
   $(srcdir)/reg-stack.c $(srcdir)/cfgrtl.c \
   $(srcdir)/sdbout.c $(srcdir)/stor-layout.c

Re: [PATCH 6/6] Add manual GTY hooks

[Diego Novillo]

On Wed, Sep 4, 2013 at 11:40 AM, David Malcolm dmalc...@redhat.com wrote:
 On Sat, 2013-08-31 at 19:27 +0200, Richard Biener wrote:
 David Malcolm dmalc...@redhat.com wrote:
 On Fri, 2013-08-30 at 10:09 +0200, Richard Biener wrote:
  On Thu, Aug 29, 2013 at 9:44 PM, Steven Bosscher
 stevenb@gmail.com wrote:
   On Thu, Aug 29, 2013 at 6:20 PM, David Malcolm
 dmalc...@redhat.com wrote:
   * gimple.c (gt_ggc_mx (gimple)): New, as required by
 GTY((user)).
   (gt_pch_nx (gimple)): Likewise.
  
   GIMPLE isn't supposed to end up in a PCH. Can you please make this
   function simply call gcc_unreachable()?
  
   FWIW 1: I really think all these hand-written markers aren't a good
   idea, we should really figure out a way to have automatic marker
   function generators, something less complex than gengtype, of
 course.
   But to have all these calls to the type-mangled marker functions
   (gt_pch_n_9tree_node, etc.) is going to be a problem in the long
 term.
  
   It seems to me that the first step in all these conversions to
   hand-written markers should be to make gengtype spit out the marker
   functions *without* the type name mangling, i.e. all marker
 functions
   should just be gt_ggc_mx(type) / gt_pch_nx(type).
 
  Yes, the original idea was that gengtype would do that.  For things
 we like
  to optimize the GTY((user)) thing would tell it that we do provide
 the markers.
  Like when you want to look through a non-GCed intermediate object.
 Or
  for things like GTY((chain)) stuff that doesn't really work if you
 have multiple
  chains (without clever GTY((skip))s...).
 
  The lack of the unmangled overloads is annoying :/  IIRC Diego
 halfway completed
  the transition to unmangled overloads / specializations?
 
 How would that work, and is that something that it would be productive
 for me to work on?
 
 Currently AIUI gtype-desc.h contains mangled macros and decls e.g.:
   extern void gt_ggc_mx_rtvec_def (void *);
   #define gt_ggc_m_9rtvec_def(X) do { \
 if (X != NULL) gt_ggc_mx_rtvec_def (X);\
 } while (0)
 
 and the corresponding functions in gtype-desc.c contain:
 
 void
 gt_ggc_mx_rtvec_def (void *x_p)
 {
   struct rtvec_def * const x = (struct rtvec_def *)x_p;
   if (ggc_test_and_set_mark (x))
 {
  /* visit fields of x, invoking the macros */
 }
 }
 
 Is the goal for the field-visiting code to all be able to simply do:
gt_ggc_mx (field)

 Yes. The advantage is that gengtype this way only needs to parse field names 
 and not types which is a lot easier.

 and have overloading resolve it all? (and handle e.g. chain_next etc
 etc)

 Yes. All bits that would require more complex parsing should instead be 
 telled explicit via a GTY annotation.

 Presumably if this were implemented, then hand-written GTY functions
 would be that much easier to maintain, and perhaps this gimple
 conversion idea would be more acceptable?  (or, in other words, should
 I
 work on this?)

 That would be very nice! IIRC Diego had issues with making all declarations 
 visible to make this work. Diego?

Yes.  I ran into the same problem that David just described.  When
trying to unmangle all the markers, you now require gengtype to have
actual visibility into all the headers, so that the compiler can see
the actual type definitions to resolve the overloads.

 An update: I've been trying this, but I'm running into what may be a
 blocker: types need to be visible in the declaration of the marker
 function.  Doing this in gtype-desc.h runs into a tangle of dependency
 issues.  For example:

   ./gtype-desc.h:2842:28: error: ‘ivarref_entry’ was not declared in this 
 scope
   extern void gt_ggc_mx (vecivarref_entry,va_gc *p);

This kind of issue.  Yes.

Perhaps this could be solved by not having a single header generated
for the markers.  Instead, markers are generated in separate header
files that are #included by the headers that have the actual type
definitions needed for the overloads to work.  Ditto for gtype-desc.c.

 BTW, what is the etymology of gt_ggc_mx and gt_pch_nx, and how do
 people pronounce them? (I call them the object-marking and
 object-noting hooks fwiw).  As far as I can see, the m and and the
 n come from mark and note; it's not clear to me where the trailing
 x came from.

That's how I think of them too.  I'm not sure what the 'x' denotes.


Diego.

Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy

[Diego Novillo]

On Sat, Aug 31, 2013 at 5:57 AM, Richard Biener
richard.guent...@gmail.com wrote:

 What do you do during stage1?  Have a collector that never collects?

Yes.  That was the pebble in the shoe.  The cc1plus built for the
purposes of gengtype does not need to look at a lot of code, so
turning off collection may not be a big problem.  We also considered
using a retargetable parser like Clang, or even plugins.

All these approaches meant keeping GC.  Neither of us is very fond of
GC, so we did not explore these alternatives very deeply.


Diego.

Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy

[Diego Novillo]

On Fri, Aug 30, 2013 at 11:21 AM, Michael Matz m...@suse.de wrote:

 Hi,

 On Fri, 30 Aug 2013, Gabriel Dos Reis wrote:

  On Fri, Aug 30, 2013 at 9:02 AM, Jakub Jelinek ja...@redhat.com wrote:
 
   I thought the principle that was acquired was that gengtype shouldn't
   be improved to support more than what it does now….
  
   If it means that we'll need to write and maintain tons of hand written 
   code
   that could otherwise be generated and maintained by a tool for us, that
   principle doesn't look very good.

 Exactly.

  Back in March 2013, I asked about gengtype support for inheritance.
 http://gcc.gnu.org/ml/gcc/2013-03/msg00273.html
  This
 http://gcc.gnu.org/ml/gcc/2013-03/msg00295.html
  was the definitive answer that appeared to be the consensus.

 Well, it was a wrong decision then.  For some smaller types writing manual
 marker might be a sensible thing, or for some extra complicated
 constructs.  But here we're talking about the most simple struct hierarchy
 imaginable.  Having to write manual markers for that one is absurd IMO.

I want to discourage extending gengtype more than necessary. Long
term, I would like to see memory pools replacing GC. However, that is
likely a long road so we should find an interim solution.

I vaguely remember thinking about what would be needed to have
gengtype deal with inheritance. It needed some pretty ugly
annotations. This made gengtype even more magic.  That's very bad for
maintenance.

One thing I liked is a suggestion that went something along the lines
of creating some base templates that could be used to facilitate
writing the manual markers.

Perhaps we could minimally extend gengtype to generate those. But I
think we can take advantage of C++ template features to facilitate
this.


Diego.

Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy

[Diego Novillo]

On Fri, Aug 30, 2013 at 11:37 AM, Jakub Jelinek ja...@redhat.com wrote:

 Teaching the gengtype parser about
 {struct,class} name : public {struct,class} someothername { ... }
 as opposed to current
 {struct,class} name { ... }
 shouldn't be that hard.  And, if the complaint is that we'd need to write
 whole C++ parser for it, then the response could be that we already have
 one C++ parser (and, even have plugin support and/or emit dwarf etc.).

It isn't.  It's annoying and a duplication of effort.

 So, gengtype could very well use a g++ plugin to emit the stuff it needs,
 or parse DWARF, etc.  And, we even could not require everybody actually
 emitting those themselves, we could check some text form of that
 (gengtype.state?) into the tree, so only people actually changing the
 compiler would need to run the plugin.

Yes.  Lawrence and I thought about moving gengtype inside g++.  That
seemed like a promising approach.

 Even if you have some stuff that helps you writing those, still it will be a
 big source of bugs (very hard to debug) and a maintainance nightmare.

Debugging gengtype is much harder.  It is magic code that is not visible.


Diego.