Issue 2462993003: [Courgette] Refactor: Add and use Instruction*Receptor classes; call ParseFile() in 2 passes.

huangs

Description was changed from ========== [Courgette] Refactor: Add and use Instruction*Receptor classes; duplicat ParseFile() calls. ...

4 years, 1 month ago (2016-10-31 22:45:56 UTC) #1

Description was changed from

==========
[Courgette] Refactor: Add and use Instruction*Receptor classes; duplicat
ParseFile() calls.

We wish to reduce AssemblyProgram's memory usage on instruction storage.
This involves changing Disassembler*::Disassemble() to call ParseFile in
2 passes: (1) count bytes needed, (2) store bytes. This CL implements
the 2-pass system using stubs. Details:
- Disassembler*::ParseFile() (and parse functions it calls) is changed
  to const, to ensure no spurious state changes in the 2 passes
  - Exception: In ELF we sort abs32_locations_ and re32_locations_ in
    different traversal orders. So we make these mutable.
- Emit*() functions are moved from AssemblyProgram to new interface
  InstructionReceptor. Updated all Emit*() callers to accommodate
- Specialize InstructionReceptor to InstructionCountReceptor and
  InstructionStoreReceptor, with stub implementation. The latter wraps
  AssemblyProgram::Emit*() (now made private).
  - AssemblyProgram instantiates and mediates these classes.
- Disassembler*::Disassemble(): Performs the 2-pass.
- Rename Emit*Instruction() to Emit*().

BUG=660980
==========

to

==========
[Courgette] Refactor: Add and use Instruction*Receptor classes; call ParseFile()
in 2 passes.

We wish to reduce AssemblyProgram's memory usage on instruction storage.
This involves changing Disassembler*::Disassemble() to call ParseFile in
2 passes: (1) count bytes needed, (2) store bytes. This CL implements
the 2-pass system using stubs. Details:
- Disassembler*::ParseFile() (and parse functions it calls) is changed
  to const, to ensure no spurious state changes in the 2 passes
  - Exception: In ELF we sort abs32_locations_ and re32_locations_ in
    different traversal orders. So we make these mutable.
- Emit*() functions are moved from AssemblyProgram to new interface
  InstructionReceptor. Updated all Emit*() callers to accommodate
- Specialize InstructionReceptor to InstructionCountReceptor and
  InstructionStoreReceptor, with stub implementation. The latter wraps
  AssemblyProgram::Emit*() (now made private).
  - AssemblyProgram instantiates and mediates these classes.
- Disassembler*::Disassemble(): Performs the 2-pass.
- Rename Emit*Instruction() to Emit*().

BUG=660980
==========

huangs

Description was changed from ========== [Courgette] Refactor: Add and use Instruction*Receptor classes; call ParseFile() in ...

4 years, 1 month ago (2016-10-31 22:50:15 UTC) #2

Description was changed from

==========
[Courgette] Refactor: Add and use Instruction*Receptor classes; call ParseFile()
in 2 passes.

We wish to reduce AssemblyProgram's memory usage on instruction storage.
This involves changing Disassembler*::Disassemble() to call ParseFile in
2 passes: (1) count bytes needed, (2) store bytes. This CL implements
the 2-pass system using stubs. Details:
- Disassembler*::ParseFile() (and parse functions it calls) is changed
  to const, to ensure no spurious state changes in the 2 passes
  - Exception: In ELF we sort abs32_locations_ and re32_locations_ in
    different traversal orders. So we make these mutable.
- Emit*() functions are moved from AssemblyProgram to new interface
  InstructionReceptor. Updated all Emit*() callers to accommodate
- Specialize InstructionReceptor to InstructionCountReceptor and
  InstructionStoreReceptor, with stub implementation. The latter wraps
  AssemblyProgram::Emit*() (now made private).
  - AssemblyProgram instantiates and mediates these classes.
- Disassembler*::Disassemble(): Performs the 2-pass.
- Rename Emit*Instruction() to Emit*().

BUG=660980
==========

to

==========
[Courgette] Refactor: Add and use Instruction*Receptor classes; call ParseFile()
in 2 passes.

We wish to reduce AssemblyProgram's memory usage on instruction storage.
This involves changing Disassembler*::Disassemble() to call ParseFile in
2 passes: (1) count bytes needed, (2) store bytes. This CL implements
the 2-pass system using stubs. Details:
- Disassembler*::ParseFile() and downstream functions are changed to
  const, to ensure safeguard Disassembler state change for the 2 passes
  - Exception: In ELF we sort abs32_locations_ and re32_locations_ in
    different traversal orders. So we make these mutable.
- Emit*() functions are moved from AssemblyProgram to new interface
  InstructionReceptor. Updated all Emit*() callers to accommodate
- Specialize InstructionReceptor to InstructionCountReceptor and
  InstructionStoreReceptor, with stub implementation. The latter wraps
  AssemblyProgram::Emit*() (now made private).
  - AssemblyProgram instantiates and mediates these classes.
- Disassembler*::Disassemble(): Performs the 2-pass.
- Rename Emit*Instruction() to Emit*().

BUG=660980
==========

huangs

huangs@chromium.org changed reviewers: + chrisha@chromium.org, wfh@chromium.org

4 years, 1 month ago (2016-10-31 22:50:16 UTC) #3

huangs

Description was changed from ========== [Courgette] Refactor: Add and use Instruction*Receptor classes; call ParseFile() in ...

4 years, 1 month ago (2016-10-31 22:52:36 UTC) #4

Description was changed from

==========
[Courgette] Refactor: Add and use Instruction*Receptor classes; call ParseFile()
in 2 passes.

We wish to reduce AssemblyProgram's memory usage on instruction storage.
This involves changing Disassembler*::Disassemble() to call ParseFile in
2 passes: (1) count bytes needed, (2) store bytes. This CL implements
the 2-pass system using stubs. Details:
- Disassembler*::ParseFile() and downstream functions are changed to
  const, to ensure safeguard Disassembler state change for the 2 passes
  - Exception: In ELF we sort abs32_locations_ and re32_locations_ in
    different traversal orders. So we make these mutable.
- Emit*() functions are moved from AssemblyProgram to new interface
  InstructionReceptor. Updated all Emit*() callers to accommodate
- Specialize InstructionReceptor to InstructionCountReceptor and
  InstructionStoreReceptor, with stub implementation. The latter wraps
  AssemblyProgram::Emit*() (now made private).
  - AssemblyProgram instantiates and mediates these classes.
- Disassembler*::Disassemble(): Performs the 2-pass.
- Rename Emit*Instruction() to Emit*().

BUG=660980
==========

to

==========
[Courgette] Refactor: Add and use Instruction*Receptor classes; call ParseFile()
in 2 passes.

We wish to reduce AssemblyProgram's memory usage on instruction storage.
This involves changing Disassembler*::Disassemble() to call ParseFile in
2 passes: (1) count bytes needed, (2) store bytes. This CL implements
the 2-pass system using stubs. Details:
- Disassembler*::ParseFile() and downstream functions are changed to
  const, to ensure safeguard Disassembler state change for the 2 passes
  - Exception: In ELF we sort abs32_locations_ and re32_locations_ in
    different traversal orders. So we make these mutable.
- Emit*() functions are moved from AssemblyProgram to new interface
  InstructionReceptor. Updated all Emit*() callers to accommodate
- Specialize InstructionReceptor to InstructionCountReceptor and
  InstructionStoreReceptor, with stub implementation. The latter wraps
  AssemblyProgram::Emit*() (now made private).
  - AssemblyProgram instantiates and mediates these classes.
- Disassembler*::Disassemble(): Performs the 2-pass.
- Rename Emit*Instruction() to Emit*().

TEST=Correctness without behavioral change; Courgette-gen and Courgette-apply
are not slowed down, and do not use more memory.
BUG=660980
==========

huangs

Phase 1, CL 2, per design doc I sent out earlier today. PTAL. Thanks!

4 years, 1 month ago (2016-10-31 22:53:46 UTC) #5

huangs

The CQ bit was checked by huangs@chromium.org to run a CQ dry run

4 years, 1 month ago (2016-10-31 22:53:54 UTC) #6

commit-bot: I haz the power

Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2462993003/1

4 years, 1 month ago (2016-10-31 22:54:31 UTC) #7

commit-bot: I haz the power

The CQ bit was unchecked by commit-bot@chromium.org

4 years, 1 month ago (2016-10-31 23:57:07 UTC) #8

commit-bot: I haz the power

Dry run: This issue passed the CQ dry run.

4 years, 1 month ago (2016-10-31 23:57:08 UTC) #9

huangs

Description was changed from ========== [Courgette] Refactor: Add and use Instruction*Receptor classes; call ParseFile() in ...

4 years, 1 month ago (2016-11-03 13:29:45 UTC) #10

Description was changed from

==========
[Courgette] Refactor: Add and use Instruction*Receptor classes; call ParseFile()
in 2 passes.

We wish to reduce AssemblyProgram's memory usage on instruction storage.
This involves changing Disassembler*::Disassemble() to call ParseFile in
2 passes: (1) count bytes needed, (2) store bytes. This CL implements
the 2-pass system using stubs. Details:
- Disassembler*::ParseFile() and downstream functions are changed to
  const, to ensure safeguard Disassembler state change for the 2 passes
  - Exception: In ELF we sort abs32_locations_ and re32_locations_ in
    different traversal orders. So we make these mutable.
- Emit*() functions are moved from AssemblyProgram to new interface
  InstructionReceptor. Updated all Emit*() callers to accommodate
- Specialize InstructionReceptor to InstructionCountReceptor and
  InstructionStoreReceptor, with stub implementation. The latter wraps
  AssemblyProgram::Emit*() (now made private).
  - AssemblyProgram instantiates and mediates these classes.
- Disassembler*::Disassemble(): Performs the 2-pass.
- Rename Emit*Instruction() to Emit*().

TEST=Correctness without behavioral change; Courgette-gen and Courgette-apply
are not slowed down, and do not use more memory.
BUG=660980
==========

to

==========
[Courgette] Refactor: Add and use Instruction*Receptor classes; call ParseFile()
in 2 passes.

We wish to reduce AssemblyProgram's memory usage on instruction storage.
This requires changing Disassembler*::Disassemble() to call ParseFile()
in 2 passes: (1) count bytes needed (so we can preallocate) (2) store
bytes. This CL implements the 2-pass system, focusing on callers. Details:
- Disassembler*::ParseFile() and downstream functions are changed to
  const, to assert absence of Disassembler state change for the passes.
  - Exception: In ELF we abs32_locations_ and re32_locations_ are sorted
    to obtain different traversal orders, but we don't add/remove
    anything. So we make these mutable.
- Emit*() functions are moved from AssemblyProgram to new interface
  InstructionReceptor. Updating all Emit*() calls to accommodate
- Specialize InstructionReceptor to InstructionCountReceptor and
  InstructionStoreReceptor, with stub implementation. The latter wraps
  AssemblyProgram::Emit*() (now made private).
  - AssemblyProgram instantiates and mediates these classes.
- Disassembler*::Disassemble(): Performs the 2-pass.
- Rename Emit*Instruction() to Emit*().

TEST=Correctness without behavioral change; Courgette-gen and Courgette-apply
are not slowed down, and do not use more memory.
BUG=660980
==========

grt (UTC plus 2)

it's hard for me to comment on much of this since it doesn't seem to ...

4 years, 1 month ago (2016-11-04 09:38:54 UTC) #13

huangs

Updated, PTAL. https://codereview.chromium.org/2462993003/diff/20001/courgette/assembly_program.cc File courgette/assembly_program.cc (right): https://codereview.chromium.org/2462993003/diff/20001/courgette/assembly_program.cc#newcode122 courgette/assembly_program.cc:122: // TODO(huangs): Populate these with size_ += ...

4 years, 1 month ago (2016-11-04 18:20:40 UTC) #14

huangs

Preview of the next CL is here: https://codereview.chromium.org/2476863004 Right now it also contains this CL; ...

4 years, 1 month ago (2016-11-04 22:25:26 UTC) #15

grt (UTC plus 2)

https://codereview.chromium.org/2462993003/diff/20001/courgette/assembly_program.cc File courgette/assembly_program.cc (right): https://codereview.chromium.org/2462993003/diff/20001/courgette/assembly_program.cc#newcode173 courgette/assembly_program.cc:173: CHECK(program_ != nullptr); On 2016/11/04 18:20:39, huangs wrote: > ...

4 years, 1 month ago (2016-11-07 09:36:18 UTC) #16

https://codereview.chromium.org/2462993003/diff/20001/courgette/assembly_prog...
File courgette/assembly_program.cc (right):

https://codereview.chromium.org/2462993003/diff/20001/courgette/assembly_prog...
courgette/assembly_program.cc:173: CHECK(program_ != nullptr);
On 2016/11/04 18:20:39, huangs wrote:
> Should be consistent with later uses (DCHECK). Comparing choices:
> 
> CHECK(pointer);  // Must not be null.
> CHECK(!pointer);  // Must be null.
> 
> vs.
> 
> CHECK_NE(pointer, nullptr);  // Must not be null.
> CHECK_EQ(pointer, nullptr);  // Must be null.
> 
> Changing to the first one for now because I think it's easier to read. But let
> me know if you prefer the other and I'll change throughout.

I like the brief approach as well.

https://codereview.chromium.org/2462993003/diff/20001/courgette/assembly_prog...
courgette/assembly_program.cc:345: CheckBool
AssemblyProgram::CreateInstructionCountReceptor(
On 2016/11/04 18:20:39, huangs wrote:
> For symmetry with CreateInstructionStoreReceptor(), which may fail.

Ah. May fail in the future in a different CL. I'm not sure that splitting this
into two CLs makes sense since this CL raises questions that are answered in the
other. I'll try to review them independently anyway.

https://codereview.chromium.org/2462993003/diff/40001/courgette/assembly_prog...
File courgette/assembly_program.h (right):

https://codereview.chromium.org/2462993003/diff/40001/courgette/assembly_prog...
courgette/assembly_program.h:218: CheckBool
CreateInstructionCountReceptor(InstructionCountReceptor** ret);
consumers of these methods don't need to know the concrete types, do they? how
about making the two concrete classes implementation details of AP and having
these return the base interface ptr?

https://codereview.chromium.org/2462993003/diff/40001/courgette/disassembler_...
File courgette/disassembler_elf_32.cc (right):

https://codereview.chromium.org/2462993003/diff/40001/courgette/disassembler_...
courgette/disassembler_elf_32.cc:182: // Pass 1: Count the space needed to store
instructions.
does it make sense to push these two passes into AssemblyProgram so that
disassemblers don't need to jump through these hoops? What if it looked like
this:

  if (!target->Parse(this))
    return false;

or, if AssemblyProgram isn't supposed to know much about Diassemblers:

  if (!target->Parse(base::Bind(&DisassemblerElf32::Parse,
base::Unretained(this))))
    return false;

CheckBool AssemblyProgram::Parse(const
base::Callback<CheckBool>(AssemblyProgram*, InstructionReceptor*)& parser) {
  InstructionCountReceptor count_receptor;
  if (!parser.Run(base::Unretained(this), base::Unretained(&count_receptor)))
    return false;
  // make the store receptor
  return parser.Run(base::Unretained(this), base::Unretained(&store_receptor));
}

huangs

Description was changed from ========== [Courgette] Refactor: Add and use Instruction*Receptor classes; call ParseFile() in ...

4 years, 1 month ago (2016-11-07 19:46:34 UTC) #17

Description was changed from

==========
[Courgette] Refactor: Add and use Instruction*Receptor classes; call ParseFile()
in 2 passes.

We wish to reduce AssemblyProgram's memory usage on instruction storage.
This requires changing Disassembler*::Disassemble() to call ParseFile()
in 2 passes: (1) count bytes needed (so we can preallocate) (2) store
bytes. This CL implements the 2-pass system, focusing on callers. Details:
- Disassembler*::ParseFile() and downstream functions are changed to
  const, to assert absence of Disassembler state change for the passes.
  - Exception: In ELF we abs32_locations_ and re32_locations_ are sorted
    to obtain different traversal orders, but we don't add/remove
    anything. So we make these mutable.
- Emit*() functions are moved from AssemblyProgram to new interface
  InstructionReceptor. Updating all Emit*() calls to accommodate
- Specialize InstructionReceptor to InstructionCountReceptor and
  InstructionStoreReceptor, with stub implementation. The latter wraps
  AssemblyProgram::Emit*() (now made private).
  - AssemblyProgram instantiates and mediates these classes.
- Disassembler*::Disassemble(): Performs the 2-pass.
- Rename Emit*Instruction() to Emit*().

TEST=Correctness without behavioral change; Courgette-gen and Courgette-apply
are not slowed down, and do not use more memory.
BUG=660980
==========

to

==========
[Courgette] Refactor: Add and use Instruction*Receptor classes; call ParseFile()
in 2 passes.

We wish to reduce AssemblyProgram's memory usage on instruction storage.
This requires changing Disassembler*::Disassemble() to call ParseFile()
in 2 passes: (1) count bytes needed (so we can preallocate) (2) store
bytes. This CL implements the 2-pass system, focusing on callers. Details:
- Disassembler*::ParseFile() and downstream functions are changed to
  const, to assert absence of Disassembler state change for the passes.
  - Exception: For ELF, abs32_locations_ and re32_locations_ are sorted
    to obtain different traversal orders, but we don't add/remove
    anything. So we make these mutable.
- Emit*() functions are moved from AssemblyProgram to new interface
  InstructionReceptor. Updating all Emit*() calls to accommodate
- Specialize InstructionReceptor to InstructionCountReceptor and
  InstructionStoreReceptor, with stub implementation. The latter wraps
  AssemblyProgram::Emit*().
  - AssemblyProgram instantiates and mediates these classes.
- Added AssemblyProgram::GenerateInstructions() as main entrance point
  from Disassembler*. It takes InstructionGenerator, which is just
  Disassembler*::ParseFile() wrapped in a Closure.
- Rename Emit*Instruction() to Emit*().

TEST=Correctness without behavioral change; Courgette-gen and Courgette-apply
are not slowed down, and do not use more memory.
BUG=660980
==========

huangs

Updated, PTAL. https://codereview.chromium.org/2462993003/diff/20001/courgette/assembly_program.cc File courgette/assembly_program.cc (right): https://codereview.chromium.org/2462993003/diff/20001/courgette/assembly_program.cc#newcode173 courgette/assembly_program.cc:173: CHECK(program_ != nullptr); On 2016/11/07 09:36:17, grt ...

4 years, 1 month ago (2016-11-07 19:46:52 UTC) #18

grt (UTC plus 2)

lgtm. please wait for wfh's review before committing. as i said, i'm no expert in ...

4 years, 1 month ago (2016-11-08 11:27:34 UTC) #19

huangs

Description was changed from ========== [Courgette] Refactor: Add and use Instruction*Receptor classes; call ParseFile() in ...

4 years, 1 month ago (2016-11-08 16:36:25 UTC) #20

Description was changed from

==========
[Courgette] Refactor: Add and use Instruction*Receptor classes; call ParseFile()
in 2 passes.

We wish to reduce AssemblyProgram's memory usage on instruction storage.
This requires changing Disassembler*::Disassemble() to call ParseFile()
in 2 passes: (1) count bytes needed (so we can preallocate) (2) store
bytes. This CL implements the 2-pass system, focusing on callers. Details:
- Disassembler*::ParseFile() and downstream functions are changed to
  const, to assert absence of Disassembler state change for the passes.
  - Exception: For ELF, abs32_locations_ and re32_locations_ are sorted
    to obtain different traversal orders, but we don't add/remove
    anything. So we make these mutable.
- Emit*() functions are moved from AssemblyProgram to new interface
  InstructionReceptor. Updating all Emit*() calls to accommodate
- Specialize InstructionReceptor to InstructionCountReceptor and
  InstructionStoreReceptor, with stub implementation. The latter wraps
  AssemblyProgram::Emit*().
  - AssemblyProgram instantiates and mediates these classes.
- Added AssemblyProgram::GenerateInstructions() as main entrance point
  from Disassembler*. It takes InstructionGenerator, which is just
  Disassembler*::ParseFile() wrapped in a Closure.
- Rename Emit*Instruction() to Emit*().

TEST=Correctness without behavioral change; Courgette-gen and Courgette-apply
are not slowed down, and do not use more memory.
BUG=660980
==========

to

==========
[Courgette] Refactor: Add and use Instruction*Receptor classes; call ParseFile()
in 2 passes.

We wish to reduce AssemblyProgram's memory usage on instruction storage.
This requires changing Disassembler*::Disassemble() to call ParseFile()
in 2 passes: (1) count bytes needed (so we can preallocate) (2) store
bytes. This CL implements the 2-pass system, focusing on callers. Details:
- Add AssemblyProgram::GenerateInstructions() as main entrance point
  from Disassembler*. It takes InstructionGenerator, which is just
  Disassembler*::ParseFile() wrapped in a Closure.
- Change Disassembler*::ParseFile() and downstream functions to const,
  to assert absence of Disassembler state change for the passes.
  - Exception: For ELF, we need to stort abs32_locations_ and
    re32_locations_ to obtain different traversal orders. This doesn't
    add/remove elements. So we make these containers mutable.
- Emit*() functions are moved from AssemblyProgram to new interface
  InstructionReceptor. Updating all Emit*() calls to accommodate.
- Add InstructionReceptor interface with 2 implementations:
  - InstructionCountReceptor for pass 1 (no-op in this CL).
  - InstructionStoreReceptor for pass 2 (calls AssemblyProgram::Emit*()
    in this CL).
- Rename Emit*Instruction() to Emit*().

TEST=Correctness without behavioral change; Courgette-gen and Courgette-apply
are not slowed down, and do not use more memory.
BUG=660980
==========

huangs

Thanks! Ping wfh@ for review, PTAL. https://codereview.chromium.org/2462993003/diff/60001/courgette/assembly_program.h File courgette/assembly_program.h (right): https://codereview.chromium.org/2462993003/diff/60001/courgette/assembly_program.h#newcode26 courgette/assembly_program.h:26: class AssemblyProgram; On ...

4 years, 1 month ago (2016-11-08 16:45:16 UTC) #21

huangs

The patchset sent to the CQ was uploaded after l-g-t-m from grt@chromium.org Link to the ...

4 years, 1 month ago (2016-11-09 21:28:35 UTC) #27

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/2462993003/80001

4 years, 1 month ago (2016-11-09 21:29:19 UTC) #28

commit-bot: I haz the power

Description was changed from ========== [Courgette] Refactor: Add and use Instruction*Receptor classes; call ParseFile() in ...

4 years, 1 month ago (2016-11-09 22:28:52 UTC) #29

Message was sent while issue was closed.

Description was changed from

==========
[Courgette] Refactor: Add and use Instruction*Receptor classes; call ParseFile()
in 2 passes.

We wish to reduce AssemblyProgram's memory usage on instruction storage.
This requires changing Disassembler*::Disassemble() to call ParseFile()
in 2 passes: (1) count bytes needed (so we can preallocate) (2) store
bytes. This CL implements the 2-pass system, focusing on callers. Details:
- Add AssemblyProgram::GenerateInstructions() as main entrance point
  from Disassembler*. It takes InstructionGenerator, which is just
  Disassembler*::ParseFile() wrapped in a Closure.
- Change Disassembler*::ParseFile() and downstream functions to const,
  to assert absence of Disassembler state change for the passes.
  - Exception: For ELF, we need to stort abs32_locations_ and
    re32_locations_ to obtain different traversal orders. This doesn't
    add/remove elements. So we make these containers mutable.
- Emit*() functions are moved from AssemblyProgram to new interface
  InstructionReceptor. Updating all Emit*() calls to accommodate.
- Add InstructionReceptor interface with 2 implementations:
  - InstructionCountReceptor for pass 1 (no-op in this CL).
  - InstructionStoreReceptor for pass 2 (calls AssemblyProgram::Emit*()
    in this CL).
- Rename Emit*Instruction() to Emit*().

TEST=Correctness without behavioral change; Courgette-gen and Courgette-apply
are not slowed down, and do not use more memory.
BUG=660980
==========

to

==========
[Courgette] Refactor: Add and use Instruction*Receptor classes; call ParseFile()
in 2 passes.

We wish to reduce AssemblyProgram's memory usage on instruction storage.
This requires changing Disassembler*::Disassemble() to call ParseFile()
in 2 passes: (1) count bytes needed (so we can preallocate) (2) store
bytes. This CL implements the 2-pass system, focusing on callers. Details:
- Add AssemblyProgram::GenerateInstructions() as main entrance point
  from Disassembler*. It takes InstructionGenerator, which is just
  Disassembler*::ParseFile() wrapped in a Closure.
- Change Disassembler*::ParseFile() and downstream functions to const,
  to assert absence of Disassembler state change for the passes.
  - Exception: For ELF, we need to stort abs32_locations_ and
    re32_locations_ to obtain different traversal orders. This doesn't
    add/remove elements. So we make these containers mutable.
- Emit*() functions are moved from AssemblyProgram to new interface
  InstructionReceptor. Updating all Emit*() calls to accommodate.
- Add InstructionReceptor interface with 2 implementations:
  - InstructionCountReceptor for pass 1 (no-op in this CL).
  - InstructionStoreReceptor for pass 2 (calls AssemblyProgram::Emit*()
    in this CL).
- Rename Emit*Instruction() to Emit*().

TEST=Correctness without behavioral change; Courgette-gen and Courgette-apply
are not slowed down, and do not use more memory.
BUG=660980
==========

commit-bot: I haz the power

Description was changed from ========== [Courgette] Refactor: Add and use Instruction*Receptor classes; call ParseFile() in ...

4 years, 1 month ago (2016-11-09 22:42:18 UTC) #31

Message was sent while issue was closed.

Description was changed from

==========
[Courgette] Refactor: Add and use Instruction*Receptor classes; call ParseFile()
in 2 passes.

We wish to reduce AssemblyProgram's memory usage on instruction storage.
This requires changing Disassembler*::Disassemble() to call ParseFile()
in 2 passes: (1) count bytes needed (so we can preallocate) (2) store
bytes. This CL implements the 2-pass system, focusing on callers. Details:
- Add AssemblyProgram::GenerateInstructions() as main entrance point
  from Disassembler*. It takes InstructionGenerator, which is just
  Disassembler*::ParseFile() wrapped in a Closure.
- Change Disassembler*::ParseFile() and downstream functions to const,
  to assert absence of Disassembler state change for the passes.
  - Exception: For ELF, we need to stort abs32_locations_ and
    re32_locations_ to obtain different traversal orders. This doesn't
    add/remove elements. So we make these containers mutable.
- Emit*() functions are moved from AssemblyProgram to new interface
  InstructionReceptor. Updating all Emit*() calls to accommodate.
- Add InstructionReceptor interface with 2 implementations:
  - InstructionCountReceptor for pass 1 (no-op in this CL).
  - InstructionStoreReceptor for pass 2 (calls AssemblyProgram::Emit*()
    in this CL).
- Rename Emit*Instruction() to Emit*().

TEST=Correctness without behavioral change; Courgette-gen and Courgette-apply
are not slowed down, and do not use more memory.
BUG=660980
==========

to

==========
[Courgette] Refactor: Add and use Instruction*Receptor classes; call ParseFile()
in 2 passes.

We wish to reduce AssemblyProgram's memory usage on instruction storage.
This requires changing Disassembler*::Disassemble() to call ParseFile()
in 2 passes: (1) count bytes needed (so we can preallocate) (2) store
bytes. This CL implements the 2-pass system, focusing on callers. Details:
- Add AssemblyProgram::GenerateInstructions() as main entrance point
  from Disassembler*. It takes InstructionGenerator, which is just
  Disassembler*::ParseFile() wrapped in a Closure.
- Change Disassembler*::ParseFile() and downstream functions to const,
  to assert absence of Disassembler state change for the passes.
  - Exception: For ELF, we need to stort abs32_locations_ and
    re32_locations_ to obtain different traversal orders. This doesn't
    add/remove elements. So we make these containers mutable.
- Emit*() functions are moved from AssemblyProgram to new interface
  InstructionReceptor. Updating all Emit*() calls to accommodate.
- Add InstructionReceptor interface with 2 implementations:
  - InstructionCountReceptor for pass 1 (no-op in this CL).
  - InstructionStoreReceptor for pass 2 (calls AssemblyProgram::Emit*()
    in this CL).
- Rename Emit*Instruction() to Emit*().

TEST=Correctness without behavioral change; Courgette-gen and Courgette-apply
are not slowed down, and do not use more memory.
BUG=660980

Committed: https://crrev.com/7b221a5ec1175ffba9955aede222b9bdd4056183
Cr-Commit-Position: refs/heads/master@{#431053}
==========

commit-bot: I haz the power

4 years, 1 month ago (2016-11-09 22:42:19 UTC) #32

Message was sent while issue was closed.

Patchset 5 (id:??) landed as
https://crrev.com/7b221a5ec1175ffba9955aede222b9bdd4056183
Cr-Commit-Position: refs/heads/master@{#431053}

Issue 2462993003: [Courgette] Refactor: Add and use Instruction*Receptor classes; call ParseFile() in 2 passes. (Closed)

Description

Patch Set 1 #

Patch Set 2 : Make AssemblyProgram::CreateInstruction*Receptor() return CheckBool. #

Patch Set 3 : Fix style and comments; lint; remove size param from InstructionStoreReceptor ctor. #

Patch Set 4 : Hide 2-pass from caller. #

Patch Set 5 : Fix comments. #

Messages