Revert "byte align everything in SkSLInterpreter"
This reverts commit e5288369c89e85b4530e2c96efee7b70270d3f2f.
Reason for revert: bad merge with code adding calls to align()
Original change's description:
> byte align everything in SkSLInterpreter
>
> It's nicer to write code without having to think about alignment,
> and this appears to be faster too:
>
> $ ninja -C out nanobench && out/nanobench --config 8888 -m GM_runtime_cf_interp_1 --loops 0
> Before: 24/24 MB 1 18.4ms 18.5ms 18.5ms 18.6ms 0% █▆▅▅▅▁▅▅▅▅ 8888 GM_runtime_cf_interp_1
> After: 23/23 MB 1 16.6ms 16.6ms 16.6ms 16.7ms 0% ▁▁▃█▅▂▁▁█▅ 8888 GM_runtime_cf_interp_1
>
> While byte-aligning things I noticed the write16 and write32 calls could
> do all their bytes at once, in one call to resize() instead of 2-4 calls
> push_back.
>
> Looking at that disassembly, I noticed vector_instruction can be static.
>
> Change-Id: I22985b49d6745797da10bbd6b6f2002a7618f2ae
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/214338
> Reviewed-by: Brian Osman <brianosman@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> Commit-Queue: Mike Klein <mtklein@google.com>
TBR=mtklein@google.com,brianosman@google.com,ethannicholas@google.com
Change-Id: Id4d0c1dfcfe8b2f91cf90e636cae5bef760df0e6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/214353
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
diff --git a/src/sksl/SkSLByteCodeGenerator.cpp b/src/sksl/SkSLByteCodeGenerator.cpp
index 91a2d70..5c877a7 100644
--- a/src/sksl/SkSLByteCodeGenerator.cpp
+++ b/src/sksl/SkSLByteCodeGenerator.cpp
@@ -161,27 +161,40 @@
}
}
+void ByteCodeGenerator::align(int divisor, int remainder) {
+ switch (remainder - (int) fCode->size() % divisor) {
+ case 0: return;
+ case 3: this->write(ByteCodeInstruction::kNop3); // fall through
+ case 2: this->write(ByteCodeInstruction::kNop2); // fall through
+ case 1: this->write(ByteCodeInstruction::kNop1);
+ break;
+ default: SkASSERT(false);
+ }
+}
+
void ByteCodeGenerator::write8(uint8_t b) {
fCode->push_back(b);
}
void ByteCodeGenerator::write16(uint16_t i) {
- size_t n = fCode->size();
- fCode->resize(n+2);
- memcpy(fCode->data() + n, &i, 2);
+ SkASSERT(fCode->size() % 2 == 0);
+ this->write8(i >> 0);
+ this->write8(i >> 8);
}
void ByteCodeGenerator::write32(uint32_t i) {
- size_t n = fCode->size();
- fCode->resize(n+4);
- memcpy(fCode->data() + n, &i, 4);
+ SkASSERT(fCode->size() % 4 == 0);
+ this->write8((i >> 0) & 0xFF);
+ this->write8((i >> 8) & 0xFF);
+ this->write8((i >> 16) & 0xFF);
+ this->write8((i >> 24) & 0xFF);
}
void ByteCodeGenerator::write(ByteCodeInstruction i) {
this->write8((uint8_t) i);
}
-static ByteCodeInstruction vector_instruction(ByteCodeInstruction base, int count) {
+ByteCodeInstruction vector_instruction(ByteCodeInstruction base, int count) {
return ((ByteCodeInstruction) ((int) base + count - 1));
}
@@ -310,6 +323,7 @@
}
void ByteCodeGenerator::writeBoolLiteral(const BoolLiteral& b) {
+ this->align(4, 3);
this->write(ByteCodeInstruction::kPushImmediate);
this->write32(b.fValue ? 1 : 0);
}
@@ -364,6 +378,7 @@
}
void ByteCodeGenerator::writeFloatLiteral(const FloatLiteral& f) {
+ this->align(4, 3);
this->write(ByteCodeInstruction::kPushImmediate);
this->write32(Interpreter::Value((float) f.fValue).fUnsigned);
}
@@ -382,6 +397,7 @@
}
void ByteCodeGenerator::writeIntLiteral(const IntLiteral& i) {
+ this->align(4, 3);
this->write(ByteCodeInstruction::kPushImmediate);
this->write32(i.fValue);
}
@@ -398,6 +414,7 @@
SkASSERT(slot_count(p.fOperand->fType) == 1);
std::unique_ptr<LValue> lvalue = this->getLValue(*p.fOperand);
lvalue->load();
+ this->align(4, 3);
this->write(ByteCodeInstruction::kPushImmediate);
this->write32(type_category(p.fType) == TypeCategory::kFloat
? Interpreter::Value(1.0f).fUnsigned : 1);
@@ -501,9 +518,11 @@
void ByteCodeGenerator::writeTernaryExpression(const TernaryExpression& t) {
this->writeExpression(*t.fTest);
+ this->align(2, 1);
this->write(ByteCodeInstruction::kConditionalBranch);
DeferredLocation trueLocation(this);
this->writeExpression(*t.fIfFalse);
+ this->align(2, 1);
this->write(ByteCodeInstruction::kBranch);
DeferredLocation endLocation(this);
trueLocation.set();
@@ -701,11 +720,13 @@
}
void ByteCodeGenerator::writeBreakStatement(const BreakStatement& b) {
+ this->align(2, 1);
this->write(ByteCodeInstruction::kBranch);
fBreakTargets.top().emplace_back(this);
}
void ByteCodeGenerator::writeContinueStatement(const ContinueStatement& c) {
+ this->align(2, 1);
this->write(ByteCodeInstruction::kBranch);
fContinueTargets.top().emplace_back(this);
}
@@ -717,6 +738,7 @@
this->writeStatement(*d.fStatement);
this->setContinueTargets();
this->writeExpression(*d.fTest);
+ this->align(2, 1);
this->write(ByteCodeInstruction::kConditionalBranch);
this->write16(start);
this->setBreakTargets();
@@ -732,6 +754,7 @@
if (f.fTest) {
this->writeExpression(*f.fTest);
this->write(ByteCodeInstruction::kNot);
+ this->align(2, 1);
this->write(ByteCodeInstruction::kConditionalBranch);
DeferredLocation endLocation(this);
this->writeStatement(*f.fStatement);
@@ -740,6 +763,7 @@
this->writeExpression(*f.fNext);
this->write(vector_instruction(ByteCodeInstruction::kPop, slot_count(f.fNext->fType)));
}
+ this->align(2, 1);
this->write(ByteCodeInstruction::kBranch);
this->write16(start);
endLocation.set();
@@ -750,6 +774,7 @@
this->writeExpression(*f.fNext);
this->write(vector_instruction(ByteCodeInstruction::kPop, slot_count(f.fNext->fType)));
}
+ this->align(2, 1);
this->write(ByteCodeInstruction::kBranch);
this->write16(start);
}
@@ -760,9 +785,11 @@
if (i.fIfFalse) {
// if (test) { ..ifTrue.. } else { .. ifFalse .. }
this->writeExpression(*i.fTest);
+ this->align(2, 1);
this->write(ByteCodeInstruction::kConditionalBranch);
DeferredLocation trueLocation(this);
this->writeStatement(*i.fIfFalse);
+ this->align(2, 1);
this->write(ByteCodeInstruction::kBranch);
DeferredLocation endLocation(this);
trueLocation.set();
@@ -772,6 +799,7 @@
// if (test) { ..ifTrue.. }
this->writeExpression(*i.fTest);
this->write(ByteCodeInstruction::kNot);
+ this->align(2, 1);
this->write(ByteCodeInstruction::kConditionalBranch);
DeferredLocation endLocation(this);
this->writeStatement(*i.fIfTrue);
@@ -811,10 +839,12 @@
size_t start = fCode->size();
this->writeExpression(*w.fTest);
this->write(ByteCodeInstruction::kNot);
+ this->align(2, 1);
this->write(ByteCodeInstruction::kConditionalBranch);
DeferredLocation endLocation(this);
this->writeStatement(*w.fStatement);
this->setContinueTargets();
+ this->align(2, 1);
this->write(ByteCodeInstruction::kBranch);
this->write16(start);
endLocation.set();