Revert "add op array32 for indirect uniform access"
This reverts commit ac2d053ccfe80775b8144c069bf1f8660a5e8f9a.
Reason for revert: This has somehow impacted a bunch of the CPU SkSL tests: https://gold.skia.org/search?blame=ac2d053ccfe80775b8144c069bf1f8660a5e8f9a&corpus=gm
I didn't notice if the hash and/or instruction comparison functions were updated to accommodate the extra data. Might be relevant?
Original change's description:
> add op array32 for indirect uniform access
>
> Change-Id: I6249594a2348c7b24e4f057cce2f4e8a6a2c4409
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/431676
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Herb Derby <herb@google.com>
TBR=herb@google.com,brianosman@google.com
Change-Id: Id5fc865b265d4c61a8d4fc853a6a0ecf7c2fb066
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/432376
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
diff --git a/src/core/SkVM.cpp b/src/core/SkVM.cpp
index bb2e07f..bc49f16 100644
--- a/src/core/SkVM.cpp
+++ b/src/core/SkVM.cpp
@@ -233,8 +233,7 @@
z = inst.z,
w = inst.w;
int immA = inst.immA,
- immB = inst.immB,
- immC = inst.immC;
+ immB = inst.immB;
switch (op) {
case Op::assert_true: write(o, op, V{x}, V{y}); break;
@@ -257,7 +256,6 @@
case Op::gather32: write(o, V{id}, "=", op, Ptr{immA}, Hex{immB}, V{x}); break;
case Op::uniform32: write(o, V{id}, "=", op, Ptr{immA}, Hex{immB}); break;
- case Op::array32: write(o, V{id}, "=", op, Ptr{immA}, Hex{immB}, Hex{immC}); break;
case Op::splat: write(o, V{id}, "=", op, Splat{immA}); break;
@@ -348,8 +346,7 @@
z = inst.z,
w = inst.w;
int immA = inst.immA,
- immB = inst.immB,
- immC = inst.immC;
+ immB = inst.immB;
switch (op) {
case Op::assert_true: write(o, op, R{x}, R{y}); break;
@@ -372,7 +369,6 @@
case Op::gather32: write(o, R{d}, "=", op, Ptr{immA}, Hex{immB}, R{x}); break;
case Op::uniform32: write(o, R{d}, "=", op, Ptr{immA}, Hex{immB}); break;
- case Op::array32: write(o, R{d}, "=", op, Ptr{immA}, Hex{immB}, Hex{immC}); break;
case Op::splat: write(o, R{d}, "=", op, Splat{immA}); break;
@@ -471,8 +467,7 @@
std::vector<OptimizedInstruction> optimized(program.size());
for (Val id = 0; id < (Val)program.size(); id++) {
Instruction inst = program[id];
- optimized[id] = {inst.op, inst.x,inst.y,inst.z,inst.w,
- inst.immA,inst.immB,inst.immC,
+ optimized[id] = {inst.op, inst.x,inst.y,inst.z,inst.w, inst.immA,inst.immB,
/*death=*/id, /*can_hoist=*/true};
}
@@ -623,10 +618,6 @@
return {this, push(Op::uniform32, NA,NA,NA,NA, ptr.ix, offset)};
}
- I32 Builder::array32 (Ptr ptr, int offset, int index) {
- return {this, push(Op::array32, NA,NA,NA,NA, ptr.ix, offset, index)};
- }
-
I32 Builder::splat(int n) { return {this, push(Op::splat, NA,NA,NA,NA, n) }; }
// Be careful peepholing float math! Transformations you might expect to
@@ -2372,6 +2363,7 @@
// Put it all back together, preserving the high 8 bits and low 5.
inst = ((disp << 5) & (19_mask << 5))
| ((inst ) & ~(19_mask << 5));
+
memcpy(fCode + ref, &inst, 4);
}
}
@@ -2993,7 +2985,6 @@
lookup_register(inst.w),
inst.immA,
inst.immB,
- inst.immC,
};
fImpl->instructions.push_back(pinst);
};
@@ -3218,8 +3209,7 @@
z = inst.z,
w = inst.w;
const int immA = inst.immA,
- immB = inst.immB,
- immC = inst.immC;
+ immB = inst.immB;
// alloc_tmp() returns the first of N adjacent temporary registers,
// each freed manually with free_tmp() or noted as our result with mark_tmp_as_dst().
@@ -3623,10 +3613,6 @@
case Op::uniform32: a->vbroadcastss(dst(), A::Mem{arg[immA], immB});
break;
- case Op::array32: a->mov(GP0, A::Mem{arg[immA], immB});
- a->vbroadcastss(dst(), A::Mem{GP0, immC});
- break;
-
case Op::index: a->vmovd((A::Xmm)dst(), N);
a->vbroadcastss(dst(), dst());
a->vpsubd(dst(), dst(), &iota);
@@ -3899,11 +3885,6 @@
a->ld1r4s(dst(), GP0);
break;
- case Op::array32: a->add(GP0, arg[immA], immB);
- a->add(GP0, GP0, immC);
- a->ld1r4s(dst(), GP0);
- break;
-
case Op::gather8: {
// As usual, the gather base pointer is immB bytes off of uniform immA.
a->add (GP0, arg[immA], immB); // GP0 = &(gather base pointer)
diff --git a/src/core/SkVM.h b/src/core/SkVM.h
index ebe3546..e594a0c 100644
--- a/src/core/SkVM.h
+++ b/src/core/SkVM.h
@@ -439,7 +439,6 @@
M(index) \
M(gather8) M(gather16) M(gather32) \
M(uniform32) \
- M(array32) \
M(splat) \
M(add_f32) M(add_i32) \
M(sub_f32) M(sub_i32) \
@@ -555,9 +554,9 @@
SK_BEGIN_REQUIRE_DENSE
struct Instruction {
- Op op; // v* = op(x,y,z,w,immA,immB), where * == index of this Instruction.
- Val x,y,z,w; // Enough arguments for Op::store128.
- int immA,immB,immC; // Immediate bit pattern, shift count, pointer index, byte offset, etc.
+ Op op; // v* = op(x,y,z,w,immA,immB), where * == index of this Instruction.
+ Val x,y,z,w; // Enough arguments for Op::store128.
+ int immA,immB; // Immediate bit pattern, shift count, pointer index, byte offset, etc.
};
SK_END_REQUIRE_DENSE
@@ -569,7 +568,7 @@
struct OptimizedInstruction {
Op op;
Val x,y,z,w;
- int immA,immB,immC;
+ int immA,immB;
Val death;
bool can_hoist;
@@ -633,9 +632,6 @@
I32 uniform32(Ptr ptr, int offset);
F32 uniformF (Ptr ptr, int offset) { return pun_to_F32(uniform32(ptr,offset)); }
- // Load i32/f32 uniform with byte-count offset and index.
- I32 array32 (Ptr ptr, int offset, int index);
-
// Push and load this color as a uniform.
Color uniformColor(SkColor4f, Uniforms*);
@@ -940,9 +936,8 @@
}
private:
- Val push(
- Op op, Val x=NA, Val y=NA, Val z=NA, Val w=NA, int immA=0, int immB=0, int immC=0) {
- return this->push(Instruction{op, x,y,z,w, immA,immB,immC});
+ Val push(Op op, Val x=NA, Val y=NA, Val z=NA, Val w=NA, int immA=0, int immB=0) {
+ return this->push(Instruction{op, x,y,z,w, immA,immB});
}
template <typename T>
@@ -968,7 +963,7 @@
struct InterpreterInstruction {
Op op;
Reg d,x,y,z,w;
- int immA,immB,immC;
+ int immA,immB;
};
class Program {
diff --git a/src/opts/SkVM_opts.h b/src/opts/SkVM_opts.h
index cb92fdb..3f9132a 100644
--- a/src/opts/SkVM_opts.h
+++ b/src/opts/SkVM_opts.h
@@ -93,8 +93,7 @@
z = inst.z,
w = inst.w;
int immA = inst.immA,
- immB = inst.immB,
- immC = inst.immC;
+ immB = inst.immB;
// Ops that interact with memory need to know whether we're stride=1 or K,
// but all non-memory ops can run the same code no matter the stride.
@@ -217,12 +216,6 @@
r[d].i32 = *(const int*)( (const char*)args[immA] + immB );
break;
- CASE(Op::array32):
- const int* ptr;
- memcpy(&ptr, (const uint8_t*)args[immA] + immB, sizeof(ptr));
- r[d].i32 = ptr[immC/sizeof(int)];
- break;
-
CASE(Op::splat): r[d].i32 = immA; break;
CASE(Op::add_f32): r[d].f32 = r[x].f32 + r[y].f32; break;
diff --git a/tests/SkVMTest.cpp b/tests/SkVMTest.cpp
index 0432fc4..3d7ef42 100644
--- a/tests/SkVMTest.cpp
+++ b/tests/SkVMTest.cpp
@@ -769,46 +769,6 @@
});
}
-DEF_TEST(SKVM_array32, r) {
- skvm::Builder b;
- {
- skvm::Ptr buf0 = b.varying<int32_t>(),
- buf1 = b.varying<int32_t>(),
- uniforms = b.uniform();
-
- skvm::I32 x = b.array32(uniforms, 0, 0);
- b.store32(buf0, x);
- skvm::I32 y = b.array32(uniforms, 0, 4);
- b.store32(buf1, y);
- }
-
- test_jit_and_interpreter(b, [&](const skvm::Program& program) {
- const int K = 20;
- int i[2] = {3, 7};
- struct {
- int* g;
- } uniforms{i};
- int32_t buf0[K];
- int32_t buf1[K];
-
- program.eval(K, buf0, buf1, &uniforms);
- for (auto v : buf0) {
- REPORTER_ASSERT(r, v == 3);
- }
- for (auto v : buf1) {
- REPORTER_ASSERT(r, v == 7);
- }
- i[0] = 4;
- program.eval(K, buf0, buf1, &uniforms);
- for (auto v : buf0) {
- REPORTER_ASSERT(r, v == 4);
- }
- for (auto v : buf1) {
- REPORTER_ASSERT(r, v == 7);
- }
- });
-}
-
DEF_TEST(SkVM_sqrt, r) {
skvm::Builder b;
auto buf = b.varying<int>();
