1021229 - hoist enoughMemory_ into AssemblerShared

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Luke Wagner [:luke]]

Attachment: hoist-enough-memory

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.

Comment 1

[Jan de Mooij [:jandem]]

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.

Comment 2

[Luke Wagner [:luke]]

> 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

Comment 3

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1bc4f420340d