Closed Bug 866137 Opened 7 years ago Closed 7 years ago

IonMonkey: Teach range analysis about LoadTypedArrayElement

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: jandem, Assigned: sunfish)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

On the testcase below the "arr[3] + arr[4]" expression has an overflow check. We should add range information to LoadTypedArrayElement so that we can eliminate these.

function f(arr) {
    var res = 0;
    for (var i=0; i<2000; i++) {
	res = arr[3] + arr[4];
	arr[3] = 3;
    }
    return res;
}
var a = new Uint8Array(100);
f(a);
Attached patch a proposed fixSplinter Review
Assignee: jdemooij → sunfish
Attachment #770027 - Flags: review?(jdemooij)
Comment on attachment 770027 [details] [diff] [review]
a proposed fix

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

Nice, thanks for stealing this. r=me with nits addressed.

::: js/src/ion/RangeAnalysis.cpp
@@ +768,5 @@
>      Range input(getOperand(0));
>      setRange(new Range(input.lower(), input.upper()));
>  }
>  
> +static Range *GetTypedArrayRange(int type) {

Nit: format like this:

static Range *
GetTypedArrayRange(int type)
{
    ...

@@ +770,5 @@
>  }
>  
> +static Range *GetTypedArrayRange(int type) {
> +  switch (type) {
> +  case TypedArray::TYPE_UINT8_CLAMPED:

Nit: indent the switch with 4 spaces, and every case with 2 more spaces.
Attachment #770027 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/mozilla-central/rev/49c410370ad6
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.