Keyboard Shortcuts

File
u :up to issue
m :publish + mail comments
M :edit review message
j / k :jump to file after / before current file
J / K :jump to next file with a comment after / before current file
Side-by-side diff
i :toggle intra-line diffs
e :expand all comments
c :collapse all comments
s :toggle showing all comments
n / p :next / previous diff chunk or comment
N / P :next / previous comment
<Up> / <Down> :next / previous line
<Enter> :respond to / edit current comment
d :mark current comment as done
Issue
u :up to list of issues
m :publish + mail comments
j / k :jump to patch after / before current patch
o / <Enter> :open current patch in side-by-side view
i :open current patch in unified diff view
Issue List
j / k :jump to issue after / before current issue
o / <Enter> :open current issue
# : close issue
Comment/message editing
<Ctrl> + s or <Ctrl> + Enter :save comment
<Esc> :cancel edit
Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(383)
Issues Repositories Search
Open Issues | Closed Issues | All Issues | Sign in with your Google Account to create issues and add comments

Issue 97120: Add dump_stats() to _llvm

Can't Edit
Can't Publish+Mail
Start Review
Created:
16 years, 5 months ago by Collin Winter
Modified:
16 years, 5 months ago
Reviewers:
Jeffrey Yasskin
CC:
unladen-swallow_googlegroups.com
Visibility:
Public.
This produces reports like this: >>> import _llvm >>> _llvm.dump_stats() LLVM functions: 1833 LLVM basic blocks: 229867 LLVM instructions: 1321540 >>>

Patch Set 1 #

Patch Set 2 : Fix 80 col violation #

Patch Set 3 : Fix indentation #

Total comments: 4

Patch Set 4 : Respond to comments #

Total comments: 1
Created: 16 years, 5 months ago
Download [raw] [tar.bz2]
Unified diffs Side-by-side diffs Delta from patch set Stats (+57 lines, -2 lines) Patch
M Modules/_llvm.cc View 1 2 3 3 chunks +54 lines, -1 line 1 comment Download
M setup.py View 1 chunk +3 lines, -1 line 0 comments Download
Total messages: 4
|
Collin Winter
PTAL
16 years, 5 months ago (2009年07月24日 22:40:20 UTC) #1
PTAL
Sign in to reply to this message.
Jeffrey Yasskin
http://codereview.appspot.com/97120/diff/1002/8 File Modules/_llvm.cc (right): http://codereview.appspot.com/97120/diff/1002/8#newcode56 Line 56: void WalkModule(llvm::Module *mod) { I would do this ...
16 years, 5 months ago (2009年07月25日 03:55:13 UTC) #2
http://codereview.appspot.com/97120/diff/1002/8
File Modules/_llvm.cc (right):
http://codereview.appspot.com/97120/diff/1002/8#newcode56
Line 56: void WalkModule(llvm::Module *mod) {
I would do this with an overloaded Area() function. You'd call Area() on each
sub-object, and add up the sizes. Then you have a simple organization for the
concrete types, rather than having to do complex things in some of the functions
to reduce the number of functions.
http://codereview.appspot.com/97120/diff/1002/8#newcode67
Line 67: this->byte_count_ += sizeof(f.getArgumentList());
Do you want me to nitpick where you're missing bytes? The ArgumentList is just
the head node; each element also occupies space.
http://codereview.appspot.com/97120/diff/1002/8#newcode72
Line 72: this->byte_count_ += sizeof(*it);
The ValueSymbolTable's entries also hold it->getKeyLength() bytes for the
value's name.
http://codereview.appspot.com/97120/diff/1002/8#newcode84
Line 84: this->byte_count_ += sizeof(inst);
This only gets the size of the Instruction. Subclasses occupy extra space, and
there are usually Use instances hung off the beginning of the Instruction.
Sign in to reply to this message.
Collin Winter
I've dropped adding the bytes together; I'm wary of treating whatever number I come up ...
16 years, 5 months ago (2009年08月04日 00:10:38 UTC) #3
I've dropped adding the bytes together; I'm wary of treating whatever number I
come up with with too much weight just yet.
Sign in to reply to this message.
Jeffrey Yasskin
Otherwise, lgtm http://codereview.appspot.com/97120/diff/1005/2001 File Modules/_llvm.cc (right): http://codereview.appspot.com/97120/diff/1005/2001#newcode85 Line 85: fprintf(stderr, "LLVM functions: %ld\n", aggregator.FuncCount()); I ...
16 years, 5 months ago (2009年08月04日 22:59:12 UTC) #4
Otherwise, lgtm
http://codereview.appspot.com/97120/diff/1005/2001
File Modules/_llvm.cc (right):
http://codereview.appspot.com/97120/diff/1005/2001#newcode85
Line 85: fprintf(stderr, "LLVM functions: %ld\n", aggregator.FuncCount());
I think James was right that this should return a string or dict rather than
printing to stderr directly.
Sign in to reply to this message.
|
Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b

AltStyle によって変換されたページ (->オリジナル) /