IonMonkey: reverse Pool code does not use the correct pool

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mjrosenb, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
Created attachment 596219 [details] [diff] [review]
/home/mrosenberg/patches/help-r0.patch

There are two separate pools that are carried around, one for forward references, one for backwards references.  When we go to update the backwards reference, we do a check based on the forward pool, not the backwards one.  I also cleaned up some edge cases that I'd been conservative about.
Attachment #596219 - Flags: review?(dvander)
Comment on attachment 596219 [details] [diff] [review]
/home/mrosenberg/patches/help-r0.patch

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

::: js/src/ion/shared/IonAssemblerBufferWithConstantPools.h
@@ +57,2 @@
>      const int alignment;
> +  public:

nit: need newlines above public: and private:

@@ +125,4 @@
>      bool checkFullBackref(int poolOffset, int codeOffset) {
> +        if (!limitingUser.assigned()) {
> +            return false;
> +        }

nit: no braces needed here

@@ +143,5 @@
>          JS_ASSERT(!isBackref);
> +        // Not full if there aren't any uses.
> +        if (!limitingUser.assigned()) {
> +            return false;
> +        }

ditto

@@ +202,5 @@
>      bool isAligned(uint32 ptr) {
>          return ptr == align(ptr);
>      }
> +    bool getAlignment() {
> +        return alignment;

getAlignment returns a bool and alignment is an int.

@@ +414,5 @@
>          for (cur = &pools[numPoolKinds-1]; cur >= pools; cur--) {
>              // fetch the pool for the backwards half.
>              tmp = cur->other;
> +            if (p == cur) {
> +                tmp->updateLimiter(this->nextOffset());

nit: no braces needed here

@@ +446,4 @@
>          uint32 poolOffset = nextOffset;
>          Pool *tmp;
>          // If we need a guard instruction, reserve space for that.
> +        if (perforatedNode == NULL)

style nit: if (!perforatedNode)
Attachment #596219 - Flags: review?(dvander) → review+
(Reporter)

Updated

6 years ago
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.