add assert_true()

This is an assert that is active in debug mode.  For the moment it only
works in the interpreter, but I plan to follow up with JIT code too.

assert_true() is a data sink like a store() as far as lifetime goes,
though we take care to allow it to be hoisted if its inputs are.  An
assert_true's existence will keep all its inputs alive, and in release
builds where we skip the instruction, those inputs will all drop away
automatically.

Tested locally by forcing the interpreter.  It shouldn't be long before
I have at least x86 JIT asserts working too.

Change-Id: I7aba40d040436a57a6b930790f7b8962bafb1a8c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/253756
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This commit is contained in:
Mike Klein 2019-11-08 15:01:02 -06:00 committed by Skia Commit-Bot
parent 8283fa4066
commit 1360117174
4 changed files with 37 additions and 2 deletions

View File

@ -113,6 +113,8 @@ namespace skvm {
inst.used_in_loop ? "" :
"");
switch (op) {
case Op::assert_true: write(o, "assert_true" , V{x}); break;
case Op::store8: write(o, "store8" , Arg{imm}, V{x}); break;
case Op::store16: write(o, "store16", Arg{imm}, V{x}); break;
case Op::store32: write(o, "store32", Arg{imm}, V{x}); break;
@ -221,6 +223,8 @@ namespace skvm {
z = inst.z;
int imm = inst.imm;
switch (op) {
case Op::assert_true: write(o, "assert_true" , R{x}); break;
case Op::store8: write(o, "store8" , Arg{imm}, R{x}); break;
case Op::store16: write(o, "store16", Arg{imm}, R{x}); break;
case Op::store32: write(o, "store32", Arg{imm}, R{x}); break;
@ -374,7 +378,7 @@ namespace skvm {
Builder::Instruction& inst = fProgram[id];
// Varying loads (and gathers) and stores cannot be hoisted out of the loop.
if (inst.op <= Op::gather32) {
if (inst.op <= Op::gather32 && inst.op != Op::assert_true) {
inst.can_hoist = false;
}
@ -461,6 +465,12 @@ namespace skvm {
return {ix};
}
void Builder::assert_true(I32 val) {
#ifdef SK_DEBUG
(void)this->push(Op::assert_true, val.id,NA,NA);
#endif
}
void Builder::store8 (Arg ptr, I32 val) { (void)this->push(Op::store8 , val.id,NA,NA, ptr.ix); }
void Builder::store16(Arg ptr, I32 val) { (void)this->push(Op::store16, val.id,NA,NA, ptr.ix); }
void Builder::store32(Arg ptr, I32 val) { (void)this->push(Op::store32, val.id,NA,NA, ptr.ix); }
@ -1359,6 +1369,8 @@ namespace skvm {
// Ops that don't interact with memory should never care about the stride.
#define CASE(op) case 2*(int)op: /*fallthrough*/ case 2*(int)op+1
CASE(Op::assert_true): SkASSERT(all(r(x).i32)); break;
CASE(Op::index): static_assert(K == 16, "");
r(d).i32 = n - I32{0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15};
break;
@ -1831,6 +1843,8 @@ namespace skvm {
return false; // TODO: many new ops
#if defined(__x86_64__)
case Op::assert_true: /*TODO vptest + int3*/ break;
case Op::store8: if (scalar) { a->vpextrb (arg[imm], (A::Xmm)r[x], 0); }
else { a->vpackusdw(tmp(), r[x], r[x]);
a->vpermq (tmp(), tmp(), 0xd8);
@ -1948,6 +1962,8 @@ namespace skvm {
break;
#elif defined(__aarch64__)
case Op::assert_true: /*TODO somehow?*/ break;
case Op::store8: a->xtns2h(tmp(), r[x]);
a->xtnh2b(tmp(), tmp());
if (scalar) { a->strb (tmp(), arg[imm]); }

View File

@ -247,6 +247,7 @@ namespace skvm {
};
enum class Op : uint8_t {
assert_true,
store8, store16, store32,
// ↑ side effects / no side effects ↓
index,
@ -326,6 +327,8 @@ namespace skvm {
// TODO: sign extension (signed types) for <32-bit loads?
// TODO: unsigned integer operations where relevant (just comparisons?)?
void assert_true(I32 val);
// Store {8,16,32}-bit varying.
void store8 (Arg ptr, I32 val);
void store16(Arg ptr, I32 val);

View File

@ -225,6 +225,9 @@ namespace {
src.g = min(max(splat(0.0f), src.g), src.a);
src.b = min(max(splat(0.0f), src.b), src.a);
assert_true(gte(src.a, splat(0.0f)));
assert_true(lte(src.a, splat(1.0f)));
// Knowing that we're normalizing here and that blending and coverage
// won't affect that when the destination is normalized, we can avoid
// avoid a redundant clamp just before storing.
@ -314,7 +317,9 @@ namespace {
src.r = min(max(splat(0.0f), src.r), splat(1.0f));
src.g = min(max(splat(0.0f), src.g), splat(1.0f));
src.b = min(max(splat(0.0f), src.b), splat(1.0f));
// src.a should already be in [0,1].
assert_true(gte(src.a, splat(0.0f)));
assert_true(lte(src.a, splat(1.0f)));
}
if (force_opaque) { src.a = splat(1.0f); }

View File

@ -722,6 +722,17 @@ DEF_TEST(SkVM_MSAN, r) {
});
}
DEF_TEST(SkVM_assert, r) {
skvm::Builder b;
b.assert_true(b.lt(b.load32(b.varying<int>()),
b.splat(42)));
test_jit_and_interpreter(r, b.done(), [&](const skvm::Program& program) {
int buf[] = { 0,1,2,3,4 };
program.eval(SK_ARRAY_COUNT(buf), buf);
});
}
template <typename Fn>
static void test_asm(skiatest::Reporter* r, Fn&& fn, std::initializer_list<uint8_t> expected) {