Closed Bug 1021229 Opened 10 years ago Closed 10 years ago

hoist enoughMemory_ into AssemblerShared

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: luke, Assigned: luke)

Details

Attachments

(1 file)

enoughMemory_ shows up, in all archs, at least 3 times: once in MacroAssembler, once in MacroAssemblerX, and once in AssemblerX.  I assume this is a bug.  It'd be nice to have this in the one base class, AssemblerShared both for sanity and so new AssemblerShared methods can report OOM.
Attachment #8435204 - Flags: review?(jdemooij)
Comment on attachment 8435204 [details] [diff] [review]
hoist-enough-memory

Review of attachment 8435204 [details] [diff] [review]:
-----------------------------------------------------------------

Much simpler and less error-prone.

::: js/src/jit/arm/MacroAssembler-arm.h
@@ +506,5 @@
>          framePushed_(0)
>      { }
>      bool oom() const {
>          return Assembler::oom() || !enoughMemory_;
>      }

Could this method be removed too (Assembler::oom() checks the same enoughMemory_ variable now)? Same for the other archs probably.
Attachment #8435204 - Flags: review?(jdemooij) → review+
> Could this method be removed too (Assembler::oom() checks the same
> enoughMemory_ variable now)? Same for the other archs probably.

Nice catch, done.

https://hg.mozilla.org/integration/mozilla-inbound/rev/1bc4f420340d
https://hg.mozilla.org/mozilla-central/rev/1bc4f420340d
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: