Re: [google] AddressSanitizer for gcc, first attempt. (issue 5272048)

First round of comments. I think we should add this to google/main. It's in sufficiently good shape for it. You can keep improving it in the branch. It is now too late for 4.7's stage 1, so I think a reasonable way to proceed is to keep it in google/main and then present it for trunk inclusion

Re: [google] AddressSanitizer for gcc, first attempt. (issue 5272048)

The invoke.texi change looks fine. The ChangeLog entry needs some work. http://codereview.appspot.com/5272048/diff/41001/ChangeLog.google-main File ChangeLog.google-main (right): http://codereview.appspot.com/5272048/diff/41001/ChangeLog.google-main#newcode6 ChangeLog.google-main:6: 1 2011-11

Re: [google] AddressSanitizer for gcc, first attempt. (issue 5272048)

OK for google/main with the nits below. http://codereview.appspot.com/5272048/diff/42003/ChangeLog.google-main File ChangeLog.google-main (right): http://codereview.appspot.com/5272048/diff/42003/ChangeLog.google-main#newcode1 ChangeLog.google-main:1: 2011-11-02 Kostya Serebryany 1 2011-

Re: [pph] Stream and merge line table. (issue4836050)

OK with these changes. As far as the trunk changes go. Just commit your changes to the branch. I will get the trunk changes whenever they get approved in some future merge. Diego. http://codereview.appspot.com/4836050/diff/1/gcc/cp/pph-streamer-in.c File gcc/cp/pph-streamer-in.c (right): h

Re: Remove LINEMAP_POSITION_FOR_COLUMN macro (issue 4874043)

On 2011/08/15 18:05:51, Gabriel Charette wrote: 2011-08-11 Gabriel Charette libcpp/ChangeLog * include/line-map.h (LINEMAP_POSITION_FOR_COLUMN): Remove. Update all users to use linemap_position_for_column instead. gcc/go/Change

Re: [pph] Fix child references being included multiple times (issue 4904050)

OK with a minor nit. http://codereview.appspot.com/4904050/diff/3001/gcc/cp/pph-streamer.h File gcc/cp/pph-streamer.h (right): http://codereview.appspot.com/4904050/diff/3001/gcc/cp/pph-streamer.h#newcode210 gcc/cp/pph-streamer.h:210: extern VEC(pph_stream_ptr, heap) *pph_read_images; 209 210

Re: [pph] Add support for line table streaming with includes (issue 4908051)

Very nice. One potential application of this in the future would be to not only sequence the included files, but also the symbols and types. To support the cases where a child include depends on symbols exported by the parent before its inclusion (though I'm not sure we want to really support tha

Re: [pph] Independent pre-loaded cache for common nodes (issue 4956041)

OK with a couple of nits below. Diego. http://codereview.appspot.com/4956041/diff/1/gcc/cp/pph-streamer-in.c File gcc/cp/pph-streamer-in.c (right): http://codereview.appspot.com/4956041/diff/1/gcc/cp/pph-streamer-in.c#newcode155 gcc/cp/pph-streamer-in.c:155: || marker == PPH_RECORD_PREF; 154

Re: [pph] Fix pph_read_tree_header. (issue 5050045)

http://codereview.appspot.com/5050045/diff/1/gcc/cp/pph-streamer-in.c File gcc/cp/pph-streamer-in.c (right): http://codereview.appspot.com/5050045/diff/1/gcc/cp/pph-streamer-in.c#newcode2024 gcc/cp/pph-streamer-in.c:2024: /* Read the language-independent bitfields for expr. */ s/expr/EXPR/ htt

Re: [pph] Stream merging information (issue 5090041)

http://codereview.appspot.com/5090041/diff/1/gcc/cp/pph-streamer-in.c File gcc/cp/pph-streamer-in.c (right): http://codereview.appspot.com/5090041/diff/1/gcc/cp/pph-streamer-in.c#newcode2146 gcc/cp/pph-streamer-in.c:2146: pph_read_namespace_chain (pph_stream *stream, tree enclosing_namespace) 2

Re: [pph] Stream merging information (issue 5090041)

http://codereview.appspot.com/5090041/diff/1/gcc/cp/pph-streamer-out.c File gcc/cp/pph-streamer-out.c (right): http://codereview.appspot.com/5090041/diff/1/gcc/cp/pph-streamer-out.c#newcode1924 gcc/cp/pph-streamer-out.c:1924: tree enclosing_namespace ) 1922 void 1923 pph_write_namespace_tree (

Re: [pph] Make pph.h _the_ interface header. (issue 5247044)

http://codereview.appspot.com/5247044/diff/1/gcc/cp/pph-streamer.h File gcc/cp/pph-streamer.h (right): http://codereview.appspot.com/5247044/diff/1/gcc/cp/pph-streamer.h#newcode165 gcc/cp/pph-streamer.h:165: struct pph_stream { 165 struct pph_stream { You need the typedef. Stage 1 is still bui

Re: Run ThreadSanitizer test only on x86/linux (issue 5437087)

On 2011/11/30 14:08:34, dvyukov wrote: The patch is for google-main branch. Add directives to run ThreadSanitizer tests only on i386/x86_64-*-linux targets. Index: gcc/ChangeLog.google-main === --- gcc/ChangeLog.google-main (rev

Re: [pph] Various Tree Fields (issue4550064)

http://codereview.appspot.com/4550064/diff/1/gcc/cp/pph-streamer-in.c File gcc/cp/pph-streamer-in.c (right): http://codereview.appspot.com/4550064/diff/1/gcc/cp/pph-streamer-in.c#newcode251 gcc/cp/pph-streamer-in.c:251: { +static VEC(qualified_typedef_usage_t,gc) * +pph_stream_read_qual_use_vec

Re: [pph] More C++ Tree Nodes (issue4526083)

http://codereview.appspot.com/4526083/diff/1/gcc/cp/cp-objcp-common.c File gcc/cp/cp-objcp-common.c (right): http://codereview.appspot.com/4526083/diff/1/gcc/cp/cp-objcp-common.c#newcode103 gcc/cp/cp-objcp-common.c:103: case TEMPLATE_INFO: return sizeof (struct tree_template_info);

Re: [pph] Renaming output/write and input/read to out/in + standardizing pph_stream_* to pph_* (issue4532102)

Looks OK. One minor formatting comment that I will fix myself when I commit the patch. Diego. http://codereview.appspot.com/4532102/diff/1/pph-streamer-in.c File pph-streamer-in.c (right): http://codereview.appspot.com/4532102/diff/1/pph-streamer-in.c#newcode42 pph-streamer-in.c:42: pph_regi

Re: [pph] Renaming output/write and input/read to out/in + standardizing pph_stream_* to pph_* (issue4532102)

On 2011/06/01 13:06:15, Diego Novillo wrote: Looks OK. One minor formatting comment that I will fix myself when I commit the patch. Committed as rev 174530. Diego. http://codereview.appspot.com/4532102/

Re: [google] Enhance Annotalysis to support cloned functions/methods (issue4591066)

OK with some minor nits. Diego. http://codereview.appspot.com/4591066/diff/3001/gcc/tree-threadsafe-analyze.c File gcc/tree-threadsafe-analyze.c (right): http://codereview.appspot.com/4591066/diff/3001/gcc/tree-threadsafe-analyze.c#newcode1159 gcc/tree-threadsafe-analyze.c:1159: gcc_assert (

Re: [pph] Rename two pph_start_record functions adding in/out. (issue4629049)

On 2011/06/17 23:18:21, Gabriel Charette wrote: 2011-06-17 Gabriel Charette * gcc/cp/pph-streamer-in.c (pph_in_start_record): Rename from pph_start_record. Update all users. * gcc/cp/pph-streamer-out.c (pph_out_start_record): Rename

Re: [pph] Reorganize pph read/write file into their respective streamers (issue4657042)

On 2011/06/21 18:56:24, Gabriel Charette wrote: 2011-06-21 Gabriel Charette * gcc/cp/pph-streamer-in.c (pph_in_tree_vec): Make static. (pph_add_names_to_namespace): Moved from pph.c. (wrap_macro_def): Moved from pph.c. (report_valid

Re: [pph] Initialize cache_ix in all paths in pph_start_record (issue4642045)

On 2011/06/18 01:29:31, Gabriel Charette wrote: 2011-06-17 Gabriel Charette * gcc/cp/pph-streamer-in.c (pph_start_record): Initialize cache_ix in all paths. OK. Applied to branch. Diego. http://codereview.appspot.com/4642045/

Re: [pph] Stream scope_chain->bindings instead of global namespace (issue4661045)

http://codereview.appspot.com/4661045/diff/1/gcc/cp/pph-streamer-in.c File gcc/cp/pph-streamer-in.c (right): http://codereview.appspot.com/4661045/diff/1/gcc/cp/pph-streamer-in.c#newcode1003 gcc/cp/pph-streamer-in.c:1003: namespace. 1001 /* FIXME pph: this carried over from pph_add_names_

Re: [pph] Fix binding_level's names_size streaming (issue4634071)

On 2011/06/24 17:35:51, Gabriel Charette wrote: This was commited to trunk. Diego can you commit this patch to pph as well? Done. r175387. Diego. http://codereview.appspot.com/4634071/

Re: [pph] Moved token cache streaming to in/out respectively (issue4635073)

On 2011/06/27 18:51:22, Gabriel Charette wrote: 2011-06-27 Gabriel Charette * pph-streamer-in.c (pth_get_type_from_index): Moved from pph.c. (pth_load_number): Moved from pph.c. (pth_load_token_value): Moved from pph.c. (pth_load_to

Re: [pph] Rename token cache and identifiers streaming functions to reflect pph naming scheme (issue4636065)

On 2011/06/27 20:33:26, Gabriel Charette wrote: 2011-06-27 Gabriel Charette Remove the 'mailto:' prefix. * pph-streamer-in.c (pph_get_type_from_index): Rename from pth_get_type_from_index. Update all users. (pph_in_number): Rename from pth

Re: [pph] Fix var order when streaming in. (issue4635074)

On 2011/06/28 00:27:04, Gabriel Charette wrote: The names and namespaces chains are built by adding each new element to the front of the list. When streaming it in we traverse the list of names and re-add them to the current chains; thus reversing the order in which they were defined in the

Re: [pph] Fix var order when streaming in. (issue4635074)

http://codereview.appspot.com/4635074/diff/1/gcc/cp/pph-streamer-in.c File gcc/cp/pph-streamer-in.c (right): http://codereview.appspot.com/4635074/diff/1/gcc/cp/pph-streamer-in.c#newcode1144 gcc/cp/pph-streamer-in.c:1144: /* The chains are built backwards (ref: add_decl_to_level@name-lookup.c),

Re: [pph] Fixing string streaming functions and their comments (issue4654076)

On 2011/06/30 01:37:59, Gabriel Charette wrote: 2011-06-29 Gabriel Charette * pph-streamer.h (struct pph_stream): Fix comment of data_in field. (pph_out_string_with_length): lto_output_string_with_length now handles NULL strings, call it

Re: [pph] Expect checksums for tests marked with pph asm xdiff (issue4744043)

OK with the change below. Diego. http://codereview.appspot.com/4744043/diff/3001/gcc/testsuite/lib/dg-pph.exp File gcc/testsuite/lib/dg-pph.exp (right): http://codereview.appspot.com/4744043/diff/3001/gcc/testsuite/lib/dg-pph.exp#newcode131 gcc/testsuite/lib/dg-pph.exp:131: set adiff [catch {

Re: [pph] Add filter to prevent streaming out builtin identifiers (issue4802047)

On 2011/07/20 20:54:31, Gabriel Charette wrote: Should I instead add a new macro IS_BUILTIN or something like that to encompass that logic? Sure. But make it a static inline function, please. OK with that change. Diego. http://codereview.appspot.com/4802047/

Re: [pph] Test PPH include at global scope. (issue4399041)

On 2011/04/12 21:40:10, Lawrence Crowl wrote: Add a test to ensure that PPH files are #included at global scope. Initially, this test is XFAIL, as it's a low priority error. Index: gcc/testsuite/ChangeLog.pph 2011-04-12 Lawrence Crowl * g++.dg/pph/y2sm

Re: [pph] Clean positive tests. (issue4423044)

On 2011/04/15 18:22:19, Lawrence Crowl wrote: 2011-04-15 Lawrence Crowl * lib/dg-pph.exp (dg-pph-pos): Stop on first failure. Change names of assembly files to be clear on which is with and without PPH. OK. Diego. http://codereview.appspot.c

Re: [pph] Namespaces, step 1. Trace formatting. (issue4433054)

http://codereview.appspot.com/4433054/diff/1/gcc/cp/pph-streamer.c File gcc/cp/pph-streamer.c (right): http://codereview.appspot.com/4433054/diff/1/gcc/cp/pph-streamer.c#newcode144 gcc/cp/pph-streamer.c:144: return; + if ((type == PPH_TRACE_TREE || type == PPH_TRACE_CHAIN) + && !data && fl

Re: [google] Use R_ARM_GOT_PREL to simplify global address loading from GOT (issue4433079)

I only have some stylistic comments for this patch. The new pass looks OK to me, but I do not know this area well enough to do a good review. In your ChangeLog entries, please remove the directory prefix from the file names. http://codereview.appspot.com/4433079/diff/1/gcc/hooks.c File gcc/hoo

Re: [google] Check if the nonnull attribute is applied to 'this' (issue4446070)

On 2011/04/29 15:12:52, richard.guenther_gmail.com wrote: > > + spurious white-space change. Thanks. Fixed. Diego. http://codereview.appspot.com/4446070/

Re: [google] Use different peeling parameters with available profile (issue4438079)

On 2011/04/29 00:02:40, singhai wrote: 2011-04-28 Sharad Singhai gcc/ChangeLog.google-main * params.def: Add new parameters to control peeling. * tree-ssa-loop-ivcanon.c (try_unroll_loop_completely): Use different peeling parameter

Re: Disable tracer by default for profile use (issue4428074)

On 2011/04/28 18:53:24, Diego Novillo wrote: OK. Committed rev 173186. Diego. http://codereview.appspot.com/4428074/

Re: [google] Use different peeling parameters with available profile (issue4438079)

On 2011/04/29 19:21:00, Diego Novillo wrote: On 2011/04/29 00:02:40, singhai wrote: > 2011-04-28 Sharad Singhai > >gcc/ChangeLog.google-main >* params.def: Add new parameters to control peeling. >* tree-ssa-loop-ivcanon.c (try_unroll_loop_completely):

Re: [pph] Enable nested namespaces (issue4431071)

Looks OK. Some comments below. http://codereview.appspot.com/4431071/diff/1/gcc/c-family/c.opt File gcc/c-family/c.opt (right): http://codereview.appspot.com/4431071/diff/1/gcc/c-family/c.opt#newcode943 gcc/c-family/c.opt:943: +fpph-dump-tree +C++ Var(flag_pph_dump_tree) +-fpph-dump-tree

Re: [pph] Save keyed_classes, unemitted_tinfo_decls, TYPE_BINFO (issue4486042)

http://codereview.appspot.com/4486042/diff/1/gcc/cp/pph-streamer.h File gcc/cp/pph-streamer.h (right): http://codereview.appspot.com/4486042/diff/1/gcc/cp/pph-streamer.h#newcode152 gcc/cp/pph-streamer.h:152: for (i = 0; i < c; ++i) +#if 0 +static inline void +pph_output_tree_array (pph_stream *s

Re: [google] Allow relative profile paths. (issue4280074)

OK. A nit and a question below. Diego. http://codereview.appspot.com/4280074/diff/1/gcc/doc/invoke.texi File gcc/doc/invoke.texi (right): http://codereview.appspot.com/4280074/diff/1/gcc/doc/invoke.texi#newcode gcc/doc/invoke.texi:: and its related options. Both absolute and relative