TI: Wrong liveness information for variables defined inside loops inside try blocks

RESOLVED FIXED

Status

()

RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: shu, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

7 years ago
For the following code:

function g() {
  throw 0;
}

function f()
{
  try {
    for (var i = 0; i < 100; i++)
      var q = g();
  } catch(e) {
    print(q);
  }
}

|q| is marked as dead inside the loop at the RHS of the assignment. This is incorrect, as the RHS can almost always throw, and |q| is live at the catch block.
(Reporter)

Comment 1

7 years ago
Created attachment 547625 [details] [diff] [review]
fix v1

As per discussion with bhackett, fixes by extending the range all live variables that at exception entry points to the start of their respective try blocks.
Attachment #547625 - Flags: review?(bhackett1024)
(Reporter)

Comment 2

7 years ago
Created attachment 547627 [details] [diff] [review]
fix v1'

fix inconsistent unsigned type names
Attachment #547625 - Attachment is obsolete: true
Attachment #547625 - Flags: review?(bhackett1024)
Attachment #547627 - Flags: review?(bhackett1024)
Comment on attachment 547627 [details] [diff] [review]
fix v1'

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

::: js/src/jsanalyze.h
@@ +561,5 @@
>      /* Jump preceding the basic block which killed this variable. */
>      uint32 savedEnd;
>  
> +    /* If the variable needs to be kept alive until lifetime->start. */
> +    bool ensured : 1;

Can you make savedEnd be 31 bits, to avoid using an extra word on 32 bit platforms?
Attachment #547627 - Flags: review?(bhackett1024) → review+
(Reporter)

Comment 4

7 years ago
(In reply to comment #3)
> Comment on attachment 547627 [details] [diff] [review] [review]
> fix v1'
> 
> Review of attachment 547627 [details] [diff] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/jsanalyze.h
> @@ +561,5 @@
> >      /* Jump preceding the basic block which killed this variable. */
> >      uint32 savedEnd;
> >  
> > +    /* If the variable needs to be kept alive until lifetime->start. */
> > +    bool ensured : 1;
> 
> Can you make savedEnd be 31 bits, to avoid using an extra word on 32 bit
> platforms?

Fixed and pushed: http://hg.mozilla.org/projects/jaegermonkey/rev/7e3ed488cd20
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.