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

Issue 6811079: Fix a couple of bugs in BB optimizer. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 6 months ago by Siggi
Modified:
11 years, 6 months ago
Reviewers:
Roger McFarlane
CC:
sawbuck-changes_googlegroups.com, chrisha
Base URL:
http://sawbuck.googlecode.com/svn/trunk
Visibility:
Public.

Description

Fix a couple of bugs in BB optimizer. Translate offsets to RVAs for entry count lookups. Tolerate code->code reference for the special case of self-recursive functions. R=rogerm@chromium.org Committed: https://code.google.com/p/sawbuck/source/detail?r=1219

Patch Set 1 #

Patch Set 2 : Fix unittest. #

Total comments: 2

Patch Set 3 : Address Roger's comment. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+33 lines, -16 lines) Patch
M syzygy/reorder/basic_block_optimizer.cc View 1 2 3 chunks +20 lines, -6 lines 0 comments Download
M syzygy/reorder/basic_block_optimizer_unittest.cc View 1 2 chunks +11 lines, -10 lines 0 comments Download
M syzygy/reorder/reorder_app.cc View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 3
Siggi
11 years, 6 months ago (2012-11-05 14:40:43 UTC) #1
Roger McFarlane
lgtm modulo a wonky comment. https://codereview.appspot.com/6811079/diff/3001/syzygy/reorder/basic_block_optimizer.cc File syzygy/reorder/basic_block_optimizer.cc (right): https://codereview.appspot.com/6811079/diff/3001/syzygy/reorder/basic_block_optimizer.cc#newcode445 syzygy/reorder/basic_block_optimizer.cc:445: // is a re ...
11 years, 6 months ago (2012-11-05 15:41:03 UTC) #2
Siggi
11 years, 6 months ago (2012-11-05 15:44:43 UTC) #3
Thanks, committing.

https://codereview.appspot.com/6811079/diff/3001/syzygy/reorder/basic_block_o...
File syzygy/reorder/basic_block_optimizer.cc (right):

https://codereview.appspot.com/6811079/diff/3001/syzygy/reorder/basic_block_o...
syzygy/reorder/basic_block_optimizer.cc:445: // is a re (we should only see data
references here) we
On 2012/11/05 15:41:03, Roger McFarlane wrote:
> is a re?

Done.
Sign in to reply to this message.

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