codefrau / SqueakJS

A Squeak Smalltalk VM in Javascript
https://squeak.js.org
MIT License
365 stars 75 forks source link

adding inline caching, inlining and peepholes to JIT-Compiler #30

Closed abstraktor closed 9 years ago

abstraktor commented 9 years ago

As part of the VMs seminar, we implemented the following:

Due to rewriting the history a lot for beautification, the commits authors got blurred a bit. We are @abstraktor, @hotzenklotz and @nilos.

codefrau commented 9 years ago

Please put spaces outside the parentheses in control statements (if, for, etc): instead of if(...){ write if (...) {.

codefrau commented 9 years ago

Also, make the generated code look nice. Proper spacing and line breaks help in seeing problems. If the debug flag is enabled, generate useful comments (e.g. which bytecodes were collapsed by peephle optimization).

codefrau commented 9 years ago

Please debug the peephole optimizations with

Squeak.Compiler.comments = true

It will currently generate code like

// 4 <11> peephole-optimized push push numericop
if (typeof temp[7]  === 'number') {stack[++vm.sp] = temp[7] > 0 ? vm.trueObj : vm.falseObj;vm.pc = 7;continue;
}// 5 <> push

stack[++vm.sp] = temp[7];
// 5 <75> push
stack[++vm.sp] = 0;
// 6 <B3> numeric op >
var a = stack[vm.sp - 1], b = stack[vm.sp];
if (typeof a === 'number' && typeof b === 'number') {
   stack[--vm.sp] = a > b ? vm.trueObj : vm.falseObj;
} else { vm.pc = 7; vm.sendSpecial(3); if (context !== vm.activeContext || vm.breakOutOfInterpreter !== false) return bytecodes + 7}

But it does make no sense to check the type of temp[7] twice. I would have expected it to generate something more like

// 4 <11 75 B3> push push numericop > (peephole optimized)
if (typeof temp[7] === 'number') {
    stack[++vm.sp] = temp[7] > 0 ? vm.trueObj : vm.falseObj;
} else {
    stack[++vm.sp] = temp[7];
    stack[++vm.sp] = 0;
    vm.pc = 7; vm.sendSpecial(3); if (context !== vm.activeContext || vm.breakOutOfInterpreter !== false) return bytecodes + 7}
}

Also, in this example (which is from this.vm.findMethod('Set>>findElementOrNil:').compiled), why isn't it optimizing push-push-op-jump? There is a jump immediately following this.

abstraktor commented 9 years ago

ok that was a poor try, next time, we'll do better. Thanks Bert for looking at it. I'm sorry for actually wasting your time here. I could easily have seen some of those on my own ^^

abstraktor commented 9 years ago

@bertfreudenberg I think all your issues should be resolved now. Thanks for your patience. Will you recheck this?

codefrau commented 9 years ago

Will look at this asap, I'm just really busy at the moment.