Fix overzealous debug trace generation.

When the SkVM interpreter is operating in "stride=1" mode, we should
only look at the first lane of the trace/execution masks. The other
lanes are not being actively used, but the "index" opcode still fills
all the lanes, so it's entirely possible for the trace coordinate to
match in an unused lane.

Change-Id: I7f04f5c59431b3c50b4b072bc3ea4f52f8aa1a1b
Bug: skia:12752
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/488796
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This commit is contained in:
John Stiles 2021-12-28 09:31:57 -05:00 committed by SkCQ
parent 81134a7a62
commit 1a746070ac
3 changed files with 38 additions and 26 deletions

View File

@ -84,8 +84,16 @@ namespace SkVMInterpreterTypes {
r = (Slot*)addr;
}
const auto should_trace = [&](int immA, Reg x, Reg y) -> bool {
return immA >= 0 && immA < nTraceHooks && any(r[x].i32 & r[y].i32);
const auto should_trace = [&](int stride, int immA, Reg x, Reg y) -> bool {
if (immA < 0 || immA >= nTraceHooks) {
return false;
}
// When stride == K, all lanes are used.
if (stride == K) {
return any(r[x].i32 & r[y].i32);
}
// When stride == 1, only the first lane is used; the rest are not meaningful.
return r[x].i32[0] & r[y].i32[0];
};
// Step each argument pointer ahead by its stride a number of times.
@ -222,13 +230,13 @@ namespace SkVMInterpreterTypes {
break;
CASE(Op::trace_line):
if (should_trace(immA, x, y)) {
if (should_trace(stride, immA, x, y)) {
traceHooks[immA]->line(immB);
}
break;
CASE(Op::trace_var):
if (should_trace(immA, x, y)) {
if (should_trace(stride, immA, x, y)) {
for (int i = 0; i < K; ++i) {
if (r[x].i32[i] & r[y].i32[i]) {
traceHooks[immA]->var(immB, r[z].i32[i]);
@ -239,19 +247,19 @@ namespace SkVMInterpreterTypes {
break;
CASE(Op::trace_enter):
if (should_trace(immA, x, y)) {
if (should_trace(stride, immA, x, y)) {
traceHooks[immA]->enter(immB);
}
break;
CASE(Op::trace_exit):
if (should_trace(immA, x, y)) {
if (should_trace(stride, immA, x, y)) {
traceHooks[immA]->exit(immB);
}
break;
CASE(Op::trace_scope):
if (should_trace(immA, x, y)) {
if (should_trace(stride, immA, x, y)) {
traceHooks[immA]->scope(immB);
}
break;

View File

@ -68,7 +68,7 @@ namespace {
void line(int lineNum) override {
fTrace->fTraceInfo.push_back({SkSL::SkVMTraceInfo::Op::kLine,
/*data=*/{lineNum, 0}});
/*data=*/{lineNum, 0}});
}
void var(int slot, int32_t val) override {
fTrace->fTraceInfo.push_back({SkSL::SkVMTraceInfo::Op::kVar,

View File

@ -550,20 +550,22 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkRuntimeEffectSimple_GPU, r, ctxInfo) {
}
DEF_TEST(SkRuntimeEffectTraceShader, r) {
SkImageInfo info = SkImageInfo::Make(2, 2, kRGBA_8888_SkColorType, kPremul_SkAlphaType);
sk_sp<SkSurface> surface = SkSurface::MakeRaster(info);
REPORTER_ASSERT(r, surface);
TestEffect effect(r, surface);
std::string dump;
for (int imageSize : {2, 80}) {
SkImageInfo info = SkImageInfo::Make(imageSize, imageSize, kRGBA_8888_SkColorType,
kPremul_SkAlphaType);
sk_sp<SkSurface> surface = SkSurface::MakeRaster(info);
REPORTER_ASSERT(r, surface);
TestEffect effect(r, surface);
effect.build(R"(
half4 main(float2 p) {
float2 val = p - 0.5;
return half4(val, 0, 1);
}
)");
dump = effect.trace({0, 1});
REPORTER_ASSERT(r, dump == R"($0 = [main].result (float4 : slot 1/4, L2)
effect.build(R"(
half4 main(float2 p) {
float2 val = p - 0.5;
return val.0y01;
}
)");
int center = imageSize / 2;
std::string dump = effect.trace({center, 1});
auto expectation = SkSL::String::printf(R"($0 = [main].result (float4 : slot 1/4, L2)
$1 = [main].result (float4 : slot 2/4, L2)
$2 = [main].result (float4 : slot 3/4, L2)
$3 = [main].result (float4 : slot 4/4, L2)
@ -574,11 +576,11 @@ $7 = val (float2 : slot 2/2, L3)
F0 = half4 main(float2 p)
enter half4 main(float2 p)
p.x = 0.5
p.x = %d.5
p.y = 1.5
scope +1
line 3
val.x = 0
val.x = %d
val.y = 1
line 4
[main].result.x = 0
@ -587,9 +589,11 @@ enter half4 main(float2 p)
[main].result.w = 1
scope -1
exit half4 main(float2 p)
)",
"Trace output does not match expectation:\n%.*s\n",
(int)dump.size(), dump.data());
)", center, center);
REPORTER_ASSERT(r, dump == expectation,
"Trace output does not match expectation for %dx%d:\n%.*s\n",
imageSize, imageSize, (int)dump.size(), dump.data());
}
}
static void test_RuntimeEffect_Blenders(skiatest::Reporter* r, GrRecordingContext* rContext) {