chakra-core / chakra-core/ChakraCore

Not hoisting fixed field checks out of loop (at least in some cases)

Open
#222 1 comment 0 reactions 0 assignees View on GitHub
help wanted Performance Suggestion
Dominant language
JavaScript
Stars
9.3k
Forks
1.2k
PR merge metrics
No merged PRs in 30d

Description

For PR #215, I was looking at codegen on test\benchmarks\Kraken\audio-oscillator.js to try to understand perf regression, and I see this is essentially the entire benchmark:

``` js
for ( var i = 0; i < this.bufferSize; i++ ) {
offset = Math.round((frameOffset + i) * step);
this.signal[i] = this.waveTable[offset % this.waveTableLength] * this.amplitude;
}
```

surprising thing is we are emitting a fixed field check for Math.round inside the loop, but I'd think it should be provable that it is loop invariant.

I spoke with @LouisLaf and it seems because this isn't value producing, PRE doesn't catch it. It is meant to be caught by invariant hoisting, but currently that doesn't catch everything. So... if anyone wants to dive in and investigate, I think this could be low hanging fruit (relatively speaking) for JIT work. It's possible that it's more complicated... but that needs investigation.

This is check I'm seeing that I think should be hoisted:

```
Line 735: offset = Math.round((frameOffset + i) * step);
Col 5: ^
StatementBoundary #4 #0048

GLOBOPT INSTR: Nop #0048

TEST s80[LikelyCanBeTaggedValue_Object].var, 1 (0x1).i8 #
JNE $L59 #
s135.i32 = MOV [s80[LikelyCanBeTaggedValue_Object].var+4].i32 #
CMP s135.i32, [0xXXXXXXXX (&GuardValue)].u32 #
JNE $L59 #
JMP $L60 #
$L59: [helper] #
$L61: [helper] #
CALL SaveAllRegistersAndBailOut.u32 #004e Bailout: #004e (BailOutFailedFixedFieldTypeCheck)
JMP $L13 #
$L60: #
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.