-
Notifications
You must be signed in to change notification settings - Fork 150
4/4 - Implement native compilation - plug backend + more tests. #114
Conversation
Codecov Report
@@ Coverage Diff @@
## master #114 +/- ##
==========================================
+ Coverage 64.58% 65.51% +0.93%
==========================================
Files 40 41 +1
Lines 3871 3883 +12
==========================================
+ Hits 2500 2544 +44
+ Misses 1109 1072 -37
- Partials 262 267 +5
Continue to review full report at Codecov.
|
I've merged master in, this should be good for review :) |
exec/native_compile_test.go
Outdated
// setup mocks. | ||
nc.Scanner.(*mockSequenceScanner).emit = []compile.CompilationCandidate{ | ||
// Sequence with single integer op - should not compiled. | ||
compile.CompilationCandidate{Start: 0, End: 7, EndInstruction: 3, Metrics: compile.Metrics{IntegerOps: 1}}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking at the history of the changes, it doesn't seem to me EndInstruction
's value was changed here, nor below.
shouldn't they be (resp.) 4
and 10
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
Note: Start/EndInstruction is not part of the code under test, this test just tests tryNativeCompile()
, and mocks everything else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Please review/merge the other PRs in this series first - this builds on those commits.
This is the final PR that enables a minimal (but working) amd64 native backend.
Future PRs will be small additions to support more and more opcodes.