Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(122)

Issue 2827103002: [Courgette] Refactor: Add CourgetteFlow; improve courgette_tool.cc help text. (Closed)

Created:
2 years, 10 months ago by huangs
Modified:
2 years, 10 months ago
Reviewers:
chrisha
CC:
chromium-reviews, wfh+watch_chromium.org, huangs+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[Courgette] Refactor: Add CourgetteFlow; improve courgette_tool.cc help text. High level flows in Courgette has messy error-handling code that's hard to read. This CL adds CourgetteFlow to absorb commonly used data and error-handling code, and improve readability of high-level logic. Additional changes: - Fix Courgette-disadj (debugging flow), which likely was broken by http://crrev.com/2793153003; Adjust() is called but AssemblyProgram did not annotate Labels. - Add BasicBuffer interface, implemented by RegionBuffer and (existing) BufferedFileReader. - PatchGeneratorX86_32::Transform(): Implemented TODO to instantiate "new" AssemblyProgram to reduce Courgette-gen peak memory. - courgette_tools.cc: Make help text more uniform. BUG=660980 Review-Url: https://codereview.chromium.org/2827103002 Cr-Commit-Position: refs/heads/master@{#467157} Committed: https://chromium.googlesource.com/chromium/src/+/8b91a4c9f1d554685ee98468c96ee3c374b6633c

Patch Set 1 #

Patch Set 2 : Fix lint warnings. #

Patch Set 3 : Add missing check(); rename and arrange CourgetteFlow commands. #

Patch Set 4 : More renames, for consistency. #

Total comments: 18

Patch Set 5 : Code cleanup; get fuzz test to use CourgetteFlow. #

Patch Set 6 : Add virtual destructor to BasicBuffer and its implementations. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+604 lines, -346 lines) Patch
M courgette/BUILD.gn View 1 chunk +2 lines, -0 lines 0 comments Download
A courgette/courgette_flow.h View 1 2 3 4 5 1 chunk +144 lines, -0 lines 0 comments Download
A courgette/courgette_flow.cc View 1 2 3 4 1 chunk +189 lines, -0 lines 0 comments Download
M courgette/courgette_tool.cc View 1 2 3 4 5 9 chunks +122 lines, -177 lines 0 comments Download
M courgette/encode_decode_unittest.cc View 1 2 3 4 3 chunks +48 lines, -37 lines 0 comments Download
M courgette/encoded_program_fuzz_unittest.cc View 1 2 3 4 3 chunks +42 lines, -46 lines 0 comments Download
M courgette/patch_generator_x86_32.h View 1 2 3 3 chunks +23 lines, -52 lines 0 comments Download
M courgette/patcher_x86_32.h View 1 2 3 4 chunks +23 lines, -33 lines 0 comments Download
M courgette/streams.h View 1 2 3 4 5 2 chunks +11 lines, -1 line 0 comments Download

Messages

Total messages: 21 (14 generated)
huangs
PTAL. Thanks!
2 years, 10 months ago (2017-04-19 18:33:16 UTC) #4
chrisha
https://codereview.chromium.org/2827103002/diff/60001/courgette/courgette_flow.cc File courgette/courgette_flow.cc (right): https://codereview.chromium.org/2827103002/diff/60001/courgette/courgette_flow.cc#newcode80 courgette/courgette_flow.cc:80: if (ok()) { if (failed()) return; Then the rest ...
2 years, 10 months ago (2017-04-24 18:15:50 UTC) #9
huangs
Updated, also changing Fuzz test. PTAL. Thanks! https://codereview.chromium.org/2827103002/diff/60001/courgette/courgette_flow.cc File courgette/courgette_flow.cc (right): https://codereview.chromium.org/2827103002/diff/60001/courgette/courgette_flow.cc#newcode80 courgette/courgette_flow.cc:80: if (ok()) ...
2 years, 10 months ago (2017-04-24 20:08:14 UTC) #10
chrisha
lgtm!
2 years, 10 months ago (2017-04-25 20:58:37 UTC) #11
huangs
Thanks. Committing!
2 years, 10 months ago (2017-04-25 21:00:13 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2827103002/100001
2 years, 10 months ago (2017-04-25 22:56:55 UTC) #18
commit-bot: I haz the power
2 years, 10 months ago (2017-04-25 23:05:14 UTC) #21
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/8b91a4c9f1d554685ee98468c96e...

Powered by Google App Engine
This is Rietveld 408576698