Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(85)

Issue 4836050: [pph] Stream and merge line table. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 9 months ago by Gabriel Charette
Modified:
12 years, 8 months ago
Reviewers:
dodji, Diego Novillo, Lawrence Crowl
CC:
gcc-patches_gcc.gnu.org
Visibility:
Public.

Patch Set 1 #

Total comments: 7
Unified diffs Side-by-side diffs Delta from patch set Stats (+327 lines, -36 lines) Patch
M gcc/cp/decl.c View 1 chunk +2 lines, -11 lines 0 comments Download
M gcc/cp/pph-streamer.h View 5 chunks +19 lines, -2 lines 3 comments Download
M gcc/cp/pph-streamer.c View 1 chunk +2 lines, -0 lines 0 comments Download
M gcc/cp/pph-streamer-in.c View 4 chunks +133 lines, -0 lines 1 comment Download
M gcc/cp/pph-streamer-out.c View 3 chunks +112 lines, -0 lines 2 comments Download
M gcc/lto-streamer.h View 1 chunk +10 lines, -0 lines 0 comments Download
M gcc/lto-streamer-in.c View 1 chunk +12 lines, -4 lines 1 comment Download
M gcc/lto-streamer-out.c View 1 chunk +10 lines, -4 lines 0 comments Download
M libcpp/include/line-map.h View 3 chunks +25 lines, -2 lines 0 comments Download
M libcpp/line-map.c View 2 chunks +2 lines, -13 lines 0 comments Download

Messages

Total messages: 6
Gabriel Charette
IMPORTANT this patch goes on top of trunk patch issue4835047, which has yet to be ...
12 years, 9 months ago (2011-08-04 23:30:40 UTC) #1
Lawrence Crowl
On 8/4/11, Gabriel Charette <gchare@google.com> wrote: > IMPORTANT this patch goes on top of trunk ...
12 years, 9 months ago (2011-08-05 02:10:21 UTC) #2
Diego Novillo
OK with these changes. As far as the trunk changes go. Just commit your changes ...
12 years, 9 months ago (2011-08-05 16:01:15 UTC) #3
Gabriel Charette
[+ccoutant] >> Added lto streamer hooks to do this, but Cary (ccoutant) was saying he'd ...
12 years, 9 months ago (2011-08-05 17:28:38 UTC) #4
dodji_seketeli.org
Hello Gabriel, gchare@google.com (Gabriel Charette) a écrit: [...] > Do we want to apply the ...
12 years, 9 months ago (2011-08-05 17:32:36 UTC) #5
Gabriel Charette
12 years, 9 months ago (2011-08-05 17:40:27 UTC) #6
Fixed all, committing to pph.

Regarding builtins, it looked like some builtins like "int type" were streamed
out as part of streaming out a non-builtin decl, i.e. a non-builtins with
builtins in it's transitive closure.

http://codereview.appspot.com/4836050/diff/1/gcc/cp/pph-streamer-out.c
File gcc/cp/pph-streamer-out.c (right):

http://codereview.appspot.com/4836050/diff/1/gcc/cp/pph-streamer-out.c#newcod...
gcc/cp/pph-streamer-out.c:134: simply add them to the cache in the preload.  */
On 2011/08/05 16:01:15, Diego Novillo wrote:
>   132   /* FIXME pph: we are streaming builtin locations, which implies that
we
> are
>   133      streaming some builtins, we probably want to figure out what those
> are and
>   134      simply add them to the cache in the preload.  */
> 
> Hmm, we are streaming builtins?  The low-level streamer detects builtins and
> only emits the builtin code, not the whole tree.  What builtins are getting
> through?
> 
> This can be addressed on a follow-up patch.  It just sounds strange to me that
> we are streaming builtins and locations.

Right, I originally had an assert here that all locations written were in the
user-defined source_location range, but some were in the builtin range.

My plan is to look into this next (after fixing the last asm diff potentially)

http://codereview.appspot.com/4836050/diff/1/gcc/cp/pph-streamer.h
File gcc/cp/pph-streamer.h (right):

http://codereview.appspot.com/4836050/diff/1/gcc/cp/pph-streamer.h#newcode344
gcc/cp/pph-streamer.h:344: streamer hook back to pph_write_location.  */
On 2011/08/05 16:01:15, Diego Novillo wrote:
>  342    FIXME pph: If pph_trace didn't depend on STREAM, we could avoid having
> to
>  343    call this function, only for it to call lto_output_location, which
calls
> the
>  344    streamer hook back to pph_write_location.  */
> 
> Just call pph_write_location here.  No need to call lto_output_location
anymore.
>   We only rely on lto_output_location calling us when it's called from tree
> leaves inside the tree streamer routines.

Ah right, definitely :)!
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b