Re: adding -Wshadow-local and -Wshadow-compatible-local ?
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
On Tue, May 12, 2015 at 11:50 AM, Tom de Vries 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
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 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
> Date: Tue Apr 21 14:17:23 2015 +0300
>
> 2015-04-21 Yury Gribov
>
> * 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
On Wed, Apr 8, 2015 at 3:36 PM, Bernhard Reutner-Fischer wrote: > contrib/ChangeLog: > > 2015-04-08 Bernhard Reutner-Fischer > > * 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
On Sun, Feb 8, 2015 at 6:58 AM, Gerald Pfeifer 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.
Stepping down as global maintainer
It's been a long time since I did any significant work on GCC, and it is unlikely that I'll be doing much for the foreseeable future. While I still have some understanding of the modules I used to maintain, I don't think it is reasonable to have me making decisions on them. It's been too long and I am not likely to have a good understanding of these components anymore. As such, I propose to become a write-after-approval maintainer and relinquish all the other maintainer roles I had. I'll be committing this to trunk. Thanks. Diego. diff --git a/MAINTAINERS b/MAINTAINERS index 22a21ee..2cf1cc4 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -31,7 +31,6 @@ Michael Meissner Jason Merrill David S. Miller Joseph Myers -Diego Novillo Bernd Schmidt Ian Lance Taylor Jim Wilson @@ -226,7 +225,6 @@ build machinery (*.in) Alexandre Oliva build machinery (*.in) Ralf Wildenhues docs co-maintainer Gerald Pfeifer docs co-maintainer Joseph Myers -docstring relicensing Diego Novillo docstring relicensing Gerald Pfeifer docstring relicensing Joseph Myers predict.defJan Hubicka @@ -238,9 +236,7 @@ option handling Joseph Myers middle-end Jeff Law middle-end Roger Sayle middle-end Ian Lance Taylor -middle-end Diego Novillo middle-end Richard Biener -tree-ssa Diego Novillo tree-ssa Andrew MacLeod PREDaniel Berlin code sinking Daniel Berlin @@ -299,10 +295,8 @@ libsanitizer, asan.c Kostya Serebryany libsanitizer, asan.c Dmitry Vyukov loop optimizer Zdenek Dvorak loop optimizer Daniel Berlin -LTO Diego Novillo LTORichard Biener LTO plugin Cary Coutant -Plugin Diego Novillo Plugin Le-Chun Wu register allocationPeter Bergner register allocationKenneth Zadeck @@ -503,6 +497,7 @@ Adam Nemet Thomas Neumann Dan Nicolaescu James Norris +Diego Novillo Dorit Nuzman David O'Brien Braden Obrzut
Re: [PATCH] Added PLUGIN_FINISH_TYPE callback on enum type processing
On Mon, Feb 2, 2015 at 2:39 PM, Bruno Loff wrote: > 2014-10-19 Bruno Loff > > * 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
That's On Mon, Feb 2, 2015 at 2:39 PM, Bruno Loff wrote: > Sorry, first contribution ever :) Here is the entry: > > 2014-10-19 Bruno Loff > > * 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
On Mon, Feb 2, 2015 at 2:07 PM, Bruno Loff 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] Added PLUGIN_FINISH_TYPE callback on enum type processing
On Thu, Jan 29, 2015 at 4:32 PM, Bruno Loff 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 > 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 1/2] teach mklog to get name / email from git config when available
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.
On Fri, Nov 21, 2014 at 1:48 PM, Andrew MacLeod 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
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
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
On Thu, Sep 18, 2014 at 7:05 PM, Jing Yu wrote: > 2014-09-18 Jing Yu > * 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
On Tue, Sep 30, 2014 at 3:09 AM, Markus Trippelsdorf 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
On Fri, Sep 19, 2014 at 6:41 AM, Tom de Vries 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
On Thu, Sep 18, 2014 at 10:56 AM, Yury Gribov 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
On Mon, Jul 21, 2014 at 4:32 AM, Yury Gribov 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
On Tue, Jul 15, 2014 at 5:23 PM, Eric Christopher wrote:
> Just to document that it's passed directly on to the linker.
>
> OK? Wording changes?
>
> -eric
>
> 2014-07-15 Eric Christopher
>
> * 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
On Thu, Jun 26, 2014 at 9:42 AM, Teresa Johnson 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 2/2] allow running mklog as a filter
On Tue, Apr 29, 2014 at 6:16 AM, Trevor Saunders 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: [PATCH 1/2] teach mklog to get name / email from git config when available
On Mon, Apr 28, 2014 at 10:11 PM, wrote:
> 2014-04-28 Trevor Saunders
>
> * 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: Ping^3 GCC trunk 4.9: documentation patch on plugins
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: Ping^3 GCC trunk 4.9: documentation patch on plugins
On Tue, Mar 18, 2014 at 2:12 AM, Basile Starynkevitch
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
>
> * 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: Is testing libgomp outside of the build tree supported?
On Wed, Feb 5, 2014 at 5:10 PM, Matthias Klose 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: Is testing libgomp outside of the build tree supported?
On Mon, Feb 3, 2014 at 2:11 PM, Ian Lance Taylor 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] Add missing generated file to contrib/gcc_update list.
On Fri, Jan 31, 2014 at 10:43 PM, Brooks Moses 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: [PATCH] Fix handling of context diff patches in mklog
On Wed, Jan 22, 2014 at 9:36 AM, Yury Gribov wrote: > Ok to commit? OK. Thanks. Diego.
Re: [patch] fix doc header in contrib/mklog
The patch is OK. It qualifies as obvious, too. Thanks. On Thu, Jan 16, 2014 at 6:44 AM, Jonathan Wakely 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: [PING][PATCH]Improving mklog [was: Re: RFC Asan instrumentation control]
Apologies for the delay. The patch is OK. On Thu, Jan 16, 2014 at 12:59 AM, Tatiana Udalova 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: Improving mklog [was: Re: RFC Asan instrumentation control]
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]
On Thu, Dec 19, 2013 at 9:33 AM, Yury Gribov 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: Improving mklog [was: Re: RFC Asan instrumentation control]
On Thu, Dec 19, 2013 at 8:04 AM, Yury Gribov 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: [wwwdocs] Update obvious fix commit policy
On Wed, Dec 4, 2013 at 11:24 AM, Jeff Law wrote: > Here's feedback. Install it now :-) Works for me :) Committed. Diego.
Re: [wwwdocs] Update obvious fix commit policy
On Tue, Dec 3, 2013 at 6:55 PM, Gerald Pfeifer 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: >> -Fixes for obvious typos in ChangeLog files, docs, web pages, comments >> -and similar stuff. Just check in the fix and copy it to >> -gcc-patches. We don't want to get overly anal-retentive >> -about checkin policies. >> +Obvious fixes can be committed without prior approval. Just check >> +in the fix and copy it to gcc-patches. 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. > > 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
On Thu, Nov 28, 2013 at 10:32 AM, Richard Earnshaw 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: [wwwdocs] Update obvious fix commit policy
On Thu, Nov 28, 2013 at 9:25 AM, Richard Biener 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. The following changes can be made by everyone with SVN write access: -Fixes for obvious typos in ChangeLog files, docs, web pages, comments -and similar stuff. Just check in the fix and copy it to -gcc-patches. We don't want to get overly anal-retentive -about checkin policies. +Obvious fixes can be committed without prior approval. Just check +in the fix and copy it to gcc-patches. 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. Similarly, no outside approval is needed to revert a patch that you checked in.
Re: [wwwdocs] Update obvious fix commit policy
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. The following changes can be made by everyone with SVN write access: -Fixes for obvious typos in ChangeLog files, docs, web pages, comments -and similar stuff. Just check in the fix and copy it to -gcc-patches. We don't want to get overly anal-retentive -about checkin policies. +Obvious fixes to documentation, code and test cases can be +committed without prior approval. Just check in the fix and copy it +to gcc-patches. 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. Similarly, no outside approval is needed to revert a patch that you checked in.
Re: [wwwdocs] Update obvious fix commit policy
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. The following changes can be made by everyone with SVN write access: -Fixes for obvious typos in ChangeLog files, docs, web pages, comments -and similar stuff. Just check in the fix and copy it to -gcc-patches. We don't want to get overly anal-retentive -about checkin policies. +Obvious fixes to documentation, code and test cases can be +committed without prior approval. Just check in the fix and copy it +to gcc-patches. 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. Similarly, no outside approval is needed to revert a patch that you checked in.
[wwwdocs] Update obvious fix commit policy
[ 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. The following changes can be made by everyone with SVN write access: -Fixes for obvious typos in ChangeLog files, docs, web pages, comments -and similar stuff. Just check in the fix and copy it to -gcc-patches. We don't want to get overly anal-retentive -about checkin policies. +Obvious fixes to documentation, code and test cases can be +committed without prior approval. Just check in the fix and copy it +to gcc-patches. 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. Similarly, no outside approval is needed to revert a patch that you checked in.
Re: gcc's obvious patch policy
On Tue, Nov 26, 2013 at 12:40 PM, Iyer, Balaji V 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 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: gcc's obvious patch policy
On Tue, Nov 26, 2013 at 12:17 AM, Alan Modra 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: [GOOGLE] Refactor the profile propagation for AutoFDO
On Mon, Nov 25, 2013 at 1:30 PM, Dehao Chen wrote: > On Mon, Nov 25, 2013 at 10:26 AM, Diego Novillo wrote: >> On Mon, Nov 25, 2013 at 1:22 PM, Xinliang David Li >> 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: [GOOGLE] Refactor the profile propagation for AutoFDO
On Mon, Nov 25, 2013 at 1:22 PM, Xinliang David Li 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
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 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] Remove zero_edge propagation algorithm
On Fri, Nov 22, 2013 at 3:27 PM, Dehao Chen wrote:
> This patch removes the zero_edge heuristic during profile propagation.
> The zero_edge heuristic does not seem to be effective in improving
> performance.
>
> Tested:
> Bootstrapped and passed regression test and performance test.
>
> OK for google-4_8?
>
> Thanks,
> Dehao
>
> Index: gcc/auto-profile.c
> ===
> --- gcc/auto-profile.c (revision 205232)
> +++ gcc/auto-profile.c (working copy)
> @@ -1122,7 +1122,7 @@ afdo_propagate_multi_edge (bool is_succ)
>
>FOR_EACH_BB (bb)
> {
> - edge e, unknown_edge = NULL, zero_edge = NULL;
> + edge e, unknown_edge = NULL;
>edge_iterator ei;
>int num_unknown_edge = 0;
>gcov_type total_known_count = 0;
> @@ -1132,8 +1132,6 @@ afdo_propagate_multi_edge (bool is_succ)
>FOR_EACH_EDGE (e, ei, bb->succs)
> if ((e->flags & EDGE_ANNOTATED) == 0)
>num_unknown_edge ++, unknown_edge = e;
> -else if (e->count == 0)
> - zero_edge = e;
> else
>total_known_count += e->count;
> }
With this removal, you can now factor out the common branches of this
predicate. Not needed for this patch, though. You had mentioned other
cleanups you wanted to do here.
Looks OK.
Thanks. Diego.
Re: [patch 1/3] Flatten gimple.h
On Thu, Nov 21, 2013 at 3:04 PM, Andrew MacLeod 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: [patch] Privatize gimplify_ctx structure.
On Wed, Nov 20, 2013 at 1:40 PM, Jeff Law 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)
On Wed, Nov 20, 2013 at 10:10 AM, Trevor Saunders 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 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: Factor unrelated declarations out of tree.h (1/2)
On Wed, Nov 20, 2013 at 8:32 AM, H.J. Lu 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)
On Wed, Nov 20, 2013 at 6:44 AM, Eric Botcazou 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: [PATCH] C++-ify and simplify loop iterators
On Tue, Nov 19, 2013 at 10:09 AM, Michael Matz 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: patch PLUGIN_HEADER_FILE event for tracing of header inclusions.
On Tue, Nov 19, 2013 at 11:16 AM, Joseph S. Myers 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: Please don't commit changes to gcc/go/gofrontend
On Tue, Nov 19, 2013 at 9:48 AM, Ian Lance Taylor 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] fix graphite build
On Tue, Nov 19, 2013 at 8:36 AM, Andrew MacLeod wrote: > graphite-sese-to-poly.c needs expr.h to compile. Fixed thusly and checked > in as revision 205027. Thanks! Diego.
Re: Factor unrelated declarations out of tree.h (1/2)
On Tue, Nov 19, 2013 at 12:17 AM, Jeff Law wrote: > It looks OK to me. Thanks. Committed as rev 205023. Ian, the Go front end will need that patch committed now. Diego.
Re: Factor unrelated declarations out of tree.h (2/2)
On Thu, Nov 14, 2013 at 9:49 PM, Andrew MacLeod 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: Factor unrelated declarations out of tree.h (1/2)
On Fri, Nov 15, 2013 at 2:23 AM, Jeff Law 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)
On Thu, Nov 14, 2013 at 5: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. 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: Factor unrelated declarations out of tree.h (1/2)
On Thu, Nov 14, 2013 at 5:12 PM, Jeff Law 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.
Add a new header to PLUGIN_HEADERS
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 * 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: [patch 3/4] Separate gimple.[ch] and gimplify.[ch] - front end files
On Thu, Nov 14, 2013 at 11:13 AM, Andrew MacLeod 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.
Re: [patch 3/4] Separate gimple.[ch] and gimplify.[ch] - front end files
On Thu, Nov 14, 2013 at 10:34 AM, Andrew MacLeod 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
On Thu, Nov 14, 2013 at 10:26 AM, Michael Matz 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 ping: diagnostics finalization and plugins
On Mon, Nov 11, 2013 at 8:36 AM, Basile Starynkevitch wrote: > 2013-11-11 Basile Starynkevitch > > * toplev.c (toplev_main): Move PLUGIN_FINISH invocation before > diagnostic_finish. OK. Diego.
Re: [buildrobot] [PATCH] pdp11-aout broken
On Sat, Nov 9, 2013 at 9:43 AM, Jan-Benedict Glaw 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 > > gcc/ > * function.c (NAME__MAIN): Move to... > * cfgexpand.c (NAME__MAIN): ...here. This is OK. Thanks. Diego.
Re: [patch] remove superfluous sets to SSA_NAME_DEF_STMT
On Thu, Nov 7, 2013 at 3:14 PM, Aldy Hernandez 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: [patch] Create gimple-expr..[ch] ... was Re: RFC: gimple.[ch] break apart
On Thu, Nov 7, 2013 at 8:23 AM, Andrew MacLeod 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: Re-factor tree.h - Part 1
On Wed, Nov 6, 2013 at 11:54 AM, Jeff Law 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: Re-factor tree.h - Part 1
On Wed, Nov 6, 2013 at 2:04 AM, 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, 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: [PATCH] use stack vectors more
On Fri, Nov 1, 2013 at 3:51 PM, Diego Novillo wrote: > It must've been whitespace then. Your new patch applied just fine. > I'll be committing shortly. Committed at r204301. Diego.
Re: [PATCH] use stack vectors more
On Fri, Nov 1, 2013 at 3:33 PM, Trevor Saunders 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
On Thu, Oct 31, 2013 at 7:48 PM, wrote: > From: Trevor Saunders > > Hi, > > This patch is pretty dull, it just replaces a bunch of things of the form > vec x; > x.create (N); // N is a constant > blah blah > x.release (); > by > stack_vec 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 > > 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] Support multiline GTY markers in Doxygen filter; add unittests
On Thu, Oct 31, 2013 at 4:06 PM, David Malcolm 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 K&R 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 K&R 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 \p
Re: [PATCH] rewrite stack vectors
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_vec to a vec *, 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++; } - vec fewvars = vNULL; + stack_vec 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 vec adjust_vec; +static vec adjust_vec; A file global was declared as a stack vector? Sigh. Diego.
Re: [patch] Flatten tree-ssa-loops.h
On Wed, Oct 23, 2013 at 10:27 AM, Andrew MacLeod 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)
On Tue, Oct 22, 2013 at 4:22 AM, Jan-Benedict Glaw wrote: > On Mon, 2013-10-21 15:36:49 -0400, Diego Novillo 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 > > * config/tilepro/tilepro.c: Include "tree.h". Sure. It qualifies as obvious. Thanks. Diego.
Re: Re-factor inclusion of tree.h
On Mon, Oct 21, 2013 at 12:57 PM, Jeff Law 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-factor inclusion of tree.h
| 1 + 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 + + * 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 * 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 1006
Re: [PATCH][i386]Fix PR 57756
On Wed, Oct 16, 2013 at 6:06 PM, David Edelsohn 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.
On Wed, Oct 9, 2013 at 11:37 AM, Andrew MacLeod wrote: > bootstraps on x86_64-unknown-linux-gnu, regressions test are still running. > OK? Sure.
Re: GTY on simple struct (Was: Re: Ping^6: contribute Synopsys Designware ARC port)
On Tue, Oct 1, 2013 at 5:50 PM, Joern Rennecke wrote: > Quoting Diego Novillo : > >> 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.
Re: Ping^6: contribute Synopsys Designware ARC port
On Sat, Sep 28, 2013 at 9:54 AM, Joern Rennecke
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: Ping^6: contribute Synopsys Designware ARC port
On Tue, Oct 1, 2013 at 10:10 AM, Joern Rennecke 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
On Sat, Sep 28, 2013 at 9:54 AM, Joern Rennecke 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.
Remove enum ssa_mode
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 vec gimple_vec;
--
1.8.4
Re: [google, patch] Google-local version of fix for PR58312, libssp cross-compiling
On Fri, Sep 20, 2013 at 5:08 PM, Brooks Moses 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: Ping patch to enable *.cc files in gengtype
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 * gengtype.c (file_rules): Added rule for *.cc files. (get_output_file_with_visibility): Give fatal message when no rules found. OK. Diego.
Re: Ping^2: small patch to accept = inside GCC plugin arguments
On Mon, Sep 16, 2013 at 4:23 AM, Basile Starynkevitch 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: gimple build interface
On Fri, Sep 20, 2013 at 4:08 AM, Richard Biener wrote: > On Thu, Sep 19, 2013 at 6:56 PM, Andrew MacLeod 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: gimple build interface
On Thu, Sep 19, 2013 at 9:20 AM, Andrew MacLeod 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: RFC - Refactor tree.h
On Fri, Sep 13, 2013 at 11:25 AM, Diego Novillo wrote:
> On Fri, Sep 13, 2013 at 11:07 AM, Jakub Jelinek 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
+
+ * tree-core.h: Add missing comment lines from refactoring
+ of tree.h.
+
2013-09-16 Jan Hubicka
* 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
On Fri, Sep 13, 2013 at 11:07 AM, Jakub Jelinek 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.
Factor gimple structures out of gimple.h
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 * 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 \ $(srcdir)/stringpool.c
Re: RFC - Next refactoring steps
On Fri, Sep 6, 2013 at 4:14 AM, Mike Stump wrote: > On Sep 5, 2013, at 11:54 PM, Richard Biener > 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.
Re: [PATCH, vtv update] Add documentation for --enable-vtable-verify configure option...
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: [PATCH, libvtv] Fix most of the testsuite.
On Thu, Sep 5, 2013 at 4:52 PM, Caroline Tice wrote: > Ping? Could somebody please review this for me? Mike already approved this upthread.
Re: [PATCH 6/6] Add manual GTY hooks
On Wed, Sep 4, 2013 at 11:40 AM, David Malcolm wrote:
> On Sat, 2013-08-31 at 19:27 +0200, Richard Biener wrote:
>> David Malcolm wrote:
>> >On Fri, 2013-08-30 at 10:09 +0200, Richard Biener wrote:
>> >> On Thu, Aug 29, 2013 at 9:44 PM, Steven Bosscher
>> > wrote:
>> >> > On Thu, Aug 29, 2013 at 6:20 PM, David Malcolm
>> > 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 (vec *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
On Sat, Aug 31, 2013 at 5:57 AM, Richard Biener 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.
