774075 - IonMonkey: eliminate redundant lower bound checks

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect)

Description

[Tom S. (please needinfo tschuster)]

For code like this we currently emit three lower bound checks.

var n = 0;
for (var i = 0; i < (a.length - 4); i++) {
  n = a[i] + a[i + 1] + a[i + 2];
}

We should just more or less duplicate ion::EliminateRedundantBoundsChecks for MBoundsCheckLower.

Comment 1

[Tom S. (please needinfo tschuster)]

Attachment: WIP

Pretty straight forward, just duplicated to the logic for MBoundsCheck in EliminateRedundantBoundsChecks the pass and adopted it for MBoundsCheckLower.

Comment 2

[Tom S. (please needinfo tschuster)]

I think Brian changed a lot of this stuff recently. Is this useful?

Comment 3

[Brian Hackett [Laid off!]]

Hmm, I think this could be used to eliminate checks, but MBoundsCheckLower will only be introduced when we were able to hoist a check from the loop and thus the eliminated checks will always be in loop preheaders rather than the loop body.

Comment 4

[Tom S. (please needinfo tschuster)]

This could be an easy fix.

Comment 5

[Mayank Bansal]

Still relevant?

Comment 6

[Iain Ireland [:iain]]

ion::EliminateRedundantBoundsChecks no longer exists. It looks to me like, at least in this case, range analysis already eliminates two of the three bounds checks. Combined with Brian's observation (which still seems to be true) that we only get these lower bounds checks when hoisting code outside of hot inner loops, and the general riskiness of messing around with bounds checks (from a security bug perspective), I think we can close this.