Skip to content

Commit

Permalink
Create dedicated op for inverted condition masks.
Browse files Browse the repository at this point in the history
This allows side-effecting ternaries and else-clauses to use one
fewer instruction, and it won't add a lot of code size.

Change-Id: I831ab059ba6d4c368dde682130ca0b49cce0cbb1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/684856
Reviewed-by: Jim Van Verth <[email protected]>
Commit-Queue: Jim Van Verth <[email protected]>
Auto-Submit: John Stiles <[email protected]>
  • Loading branch information
johnstiles-google authored and SkCQ committed Apr 28, 2023
1 parent 49412cb commit e88e5de
Show file tree
Hide file tree
Showing 11 changed files with 82 additions and 104 deletions.
9 changes: 5 additions & 4 deletions src/core/SkRasterPipelineOpList.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,11 @@

#define SK_RASTER_PIPELINE_OPS_SKSL(M) \
M(init_lane_masks) M(store_device_xy01) M(exchange_src) \
M(load_condition_mask) M(store_condition_mask) M(merge_condition_mask) \
M(load_loop_mask) M(store_loop_mask) M(mask_off_loop_mask) \
M(reenable_loop_mask) M(merge_loop_mask) M(case_op) M(continue_op) \
M(load_return_mask) M(store_return_mask) M(mask_off_return_mask) \
M(load_condition_mask) M(store_condition_mask) \
M(merge_condition_mask) M(merge_inv_condition_mask) \
M(load_loop_mask) M(store_loop_mask) M(mask_off_loop_mask) \
M(reenable_loop_mask) M(merge_loop_mask) M(case_op) M(continue_op) \
M(load_return_mask) M(store_return_mask) M(mask_off_return_mask) \
M(branch_if_all_lanes_active) M(branch_if_any_lanes_active) M(branch_if_no_lanes_active) \
M(branch_if_no_active_lanes_eq) M(jump) \
M(bitwise_and_n_ints) \
Expand Down
6 changes: 6 additions & 0 deletions src/opts/SkRasterPipeline_opts.h
Original file line number Diff line number Diff line change
Expand Up @@ -3356,6 +3356,12 @@ STAGE_TAIL(merge_condition_mask, I32* ptr) {
update_execution_mask();
}

STAGE_TAIL(merge_inv_condition_mask, I32* ptr) {
// Set the condition-mask to the intersection of the first mask and the inverse of the second.
r = sk_bit_cast<F>(ptr[0] & ~ptr[1]);
update_execution_mask();
}

STAGE_TAIL(load_loop_mask, F* ctx) {
g = sk_unaligned_load<F>(ctx);
update_execution_mask();
Expand Down
10 changes: 8 additions & 2 deletions src/sksl/codegen/SkSLRasterPipelineBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1965,9 +1965,10 @@ void Program::makeStages(TArray<Stage>* pipeline,
pipeline->push_back({ProgramOp::load_condition_mask, src});
break;
}
case BuilderOp::merge_condition_mask: {
case BuilderOp::merge_condition_mask:
case BuilderOp::merge_inv_condition_mask: {
float* ptr = tempStackPtr - (2 * N);
pipeline->push_back({ProgramOp::merge_condition_mask, ptr});
pipeline->push_back({(ProgramOp)inst.fOp, ptr});
break;
}
case BuilderOp::push_loop_mask: {
Expand Down Expand Up @@ -2838,6 +2839,7 @@ void Program::dump(SkWStream* out) const {
break;
}
case POp::merge_condition_mask:
case POp::merge_inv_condition_mask:
case POp::add_float: case POp::add_int:
case POp::sub_float: case POp::sub_int:
case POp::mul_float: case POp::mul_int:
Expand Down Expand Up @@ -3042,6 +3044,10 @@ void Program::dump(SkWStream* out) const {
opText = "CondMask = " + opArg1 + " & " + opArg2;
break;

case POp::merge_inv_condition_mask:
opText = "CondMask = " + opArg1 + " & ~" + opArg2;
break;

case POp::load_loop_mask:
opText = "LoopMask = " + opArg1;
break;
Expand Down
5 changes: 5 additions & 0 deletions src/sksl/codegen/SkSLRasterPipelineBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,11 @@ class Builder {
fInstructions.push_back({BuilderOp::merge_condition_mask, {}});
}

void merge_inv_condition_mask() {
SkASSERT(this->executionMaskWritesAreEnabled());
fInstructions.push_back({BuilderOp::merge_inv_condition_mask, {}});
}

void push_loop_mask() {
SkASSERT(this->executionMaskWritesAreEnabled());
fInstructions.push_back({BuilderOp::push_loop_mask, {}});
Expand Down
13 changes: 4 additions & 9 deletions src/sksl/codegen/SkSLRasterPipelineCodeGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1882,11 +1882,8 @@ bool Generator::writeIfStatement(const IfStatement& i) {
}

if (i.ifFalse()) {
// Negate the test-condition, then reapply it to the condition-mask.
// Then, run the if-false branch.
fBuilder.push_constant_u(~0);
fBuilder.binary_op(BuilderOp::bitwise_xor_n_ints, /*slots=*/1);
fBuilder.merge_condition_mask();
// Apply the inverse condition-mask. Then run the if-false branch.
fBuilder.merge_inv_condition_mask();
if (!this->writeStatement(*i.ifFalse())) {
return unsupported();
}
Expand Down Expand Up @@ -3563,11 +3560,9 @@ bool Generator::pushTernaryExpression(const Expression& test,
return unsupported();
}

// Switch back to the test-expression stack temporarily, and negate the test condition.
// Switch back to the test-expression stack and apply the inverted test condition.
testStack.enter();
fBuilder.push_constant_u(~0);
fBuilder.binary_op(BuilderOp::bitwise_xor_n_ints, /*slots=*/1);
fBuilder.merge_condition_mask();
fBuilder.merge_inv_condition_mask();
testStack.exit();

// Push the false-expression onto the primary stack, immediately after the true-expression.
Expand Down
2 changes: 2 additions & 0 deletions tests/RasterPipelineBuilderTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ DEF_TEST(RasterPipelineBuilderPushPopMaskRegisters, r) {
builder.push_loop_mask(); // push into 1
builder.push_return_mask(); // push into 2
builder.merge_condition_mask(); // set the condition-mask to 1 & 2
builder.merge_inv_condition_mask(); // set the condition-mask to 1 & ~2
builder.pop_condition_mask(); // pop from 2
builder.merge_loop_mask(); // mask off the loop-mask against 1
builder.push_condition_mask(); // push into 2
Expand All @@ -121,6 +122,7 @@ R"(store_condition_mask $0 = CondMask
store_loop_mask $1 = LoopMask
store_return_mask $2 = RetMask
merge_condition_mask CondMask = $1 & $2
merge_inv_condition_mask CondMask = $1 & ~$2
load_condition_mask CondMask = $2
merge_loop_mask LoopMask &= $1
store_condition_mask $2 = CondMask
Expand Down
54 changes: 18 additions & 36 deletions tests/sksl/folding/ShortCircuitBoolFolding.skrp
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ merge_condition_mask CondMask = $0 & $1
copy_slot_unmasked $2 = _1_ok
add_imm_int $2 += 0x00000001
copy_slot_masked _1_ok = Mask($2)
bitwise_xor_imm_int $1 ^= 0xFFFFFFFF
merge_condition_mask CondMask = $0 & $1
merge_inv_condition_mask CondMask = $0 & ~$1
copy_slot_unmasked $2 = _2_bad
add_imm_int $2 += 0x00000001
copy_slot_masked _2_bad = Mask($2)
Expand All @@ -25,8 +24,7 @@ merge_condition_mask CondMask = $0 & $1
copy_slot_unmasked $2 = _2_bad
add_imm_int $2 += 0x00000001
copy_slot_masked _2_bad = Mask($2)
bitwise_xor_imm_int $1 ^= 0xFFFFFFFF
merge_condition_mask CondMask = $0 & $1
merge_inv_condition_mask CondMask = $0 & ~$1
copy_slot_unmasked $2 = _1_ok
add_imm_int $2 += 0x00000001
copy_slot_masked _1_ok = Mask($2)
Expand All @@ -37,8 +35,7 @@ merge_condition_mask CondMask = $0 & $1
copy_slot_unmasked $2 = _1_ok
add_imm_int $2 += 0x00000001
copy_slot_masked _1_ok = Mask($2)
bitwise_xor_imm_int $1 ^= 0xFFFFFFFF
merge_condition_mask CondMask = $0 & $1
merge_inv_condition_mask CondMask = $0 & ~$1
copy_slot_unmasked $2 = _2_bad
add_imm_int $2 += 0x00000001
copy_slot_masked _2_bad = Mask($2)
Expand All @@ -50,8 +47,7 @@ merge_condition_mask CondMask = $0 & $1
copy_slot_unmasked $2 = _1_ok
add_imm_int $2 += 0x00000001
copy_slot_masked _1_ok = Mask($2)
bitwise_xor_imm_int $1 ^= 0xFFFFFFFF
merge_condition_mask CondMask = $0 & $1
merge_inv_condition_mask CondMask = $0 & ~$1
copy_slot_unmasked $2 = _2_bad
add_imm_int $2 += 0x00000001
copy_slot_masked _2_bad = Mask($2)
Expand All @@ -62,8 +58,7 @@ merge_condition_mask CondMask = $0 & $1
copy_slot_unmasked $2 = _1_ok
add_imm_int $2 += 0x00000001
copy_slot_masked _1_ok = Mask($2)
bitwise_xor_imm_int $1 ^= 0xFFFFFFFF
merge_condition_mask CondMask = $0 & $1
merge_inv_condition_mask CondMask = $0 & ~$1
copy_slot_unmasked $2 = _2_bad
add_imm_int $2 += 0x00000001
copy_slot_masked _2_bad = Mask($2)
Expand All @@ -75,8 +70,7 @@ merge_condition_mask CondMask = $0 & $1
copy_slot_unmasked $2 = _2_bad
add_imm_int $2 += 0x00000001
copy_slot_masked _2_bad = Mask($2)
bitwise_xor_imm_int $1 ^= 0xFFFFFFFF
merge_condition_mask CondMask = $0 & $1
merge_inv_condition_mask CondMask = $0 & ~$1
copy_slot_unmasked $2 = _1_ok
add_imm_int $2 += 0x00000001
copy_slot_masked _1_ok = Mask($2)
Expand All @@ -88,8 +82,7 @@ merge_condition_mask CondMask = $0 & $1
copy_slot_unmasked $2 = _2_bad
add_imm_int $2 += 0x00000001
copy_slot_masked _2_bad = Mask($2)
bitwise_xor_imm_int $1 ^= 0xFFFFFFFF
merge_condition_mask CondMask = $0 & $1
merge_inv_condition_mask CondMask = $0 & ~$1
copy_slot_unmasked $2 = _1_ok
add_imm_int $2 += 0x00000001
copy_slot_masked _1_ok = Mask($2)
Expand All @@ -100,8 +93,7 @@ merge_condition_mask CondMask = $0 & $1
copy_slot_unmasked $2 = _1_ok
add_imm_int $2 += 0x00000001
copy_slot_masked _1_ok = Mask($2)
bitwise_xor_imm_int $1 ^= 0xFFFFFFFF
merge_condition_mask CondMask = $0 & $1
merge_inv_condition_mask CondMask = $0 & ~$1
copy_slot_unmasked $2 = _2_bad
add_imm_int $2 += 0x00000001
copy_slot_masked _2_bad = Mask($2)
Expand All @@ -112,8 +104,7 @@ merge_condition_mask CondMask = $0 & $1
copy_slot_unmasked $2 = _1_ok
add_imm_int $2 += 0x00000001
copy_slot_masked _1_ok = Mask($2)
bitwise_xor_imm_int $1 ^= 0xFFFFFFFF
merge_condition_mask CondMask = $0 & $1
merge_inv_condition_mask CondMask = $0 & ~$1
copy_slot_unmasked $2 = _2_bad
add_imm_int $2 += 0x00000001
copy_slot_masked _2_bad = Mask($2)
Expand All @@ -126,8 +117,7 @@ merge_condition_mask CondMask = $0 & $1
copy_slot_unmasked $2 = _2_bad
add_imm_int $2 += 0x00000001
copy_slot_masked _2_bad = Mask($2)
bitwise_xor_imm_int $1 ^= 0xFFFFFFFF
merge_condition_mask CondMask = $0 & $1
merge_inv_condition_mask CondMask = $0 & ~$1
copy_slot_unmasked $2 = _1_ok
add_imm_int $2 += 0x00000001
copy_slot_masked _1_ok = Mask($2)
Expand All @@ -138,8 +128,7 @@ merge_condition_mask CondMask = $0 & $1
copy_slot_unmasked $2 = _1_ok
add_imm_int $2 += 0x00000001
copy_slot_masked _1_ok = Mask($2)
bitwise_xor_imm_int $1 ^= 0xFFFFFFFF
merge_condition_mask CondMask = $0 & $1
merge_inv_condition_mask CondMask = $0 & ~$1
copy_slot_unmasked $2 = _2_bad
add_imm_int $2 += 0x00000001
copy_slot_masked _2_bad = Mask($2)
Expand All @@ -151,8 +140,7 @@ merge_condition_mask CondMask = $0 & $1
copy_slot_unmasked $2 = _1_ok
add_imm_int $2 += 0x00000001
copy_slot_masked _1_ok = Mask($2)
bitwise_xor_imm_int $1 ^= 0xFFFFFFFF
merge_condition_mask CondMask = $0 & $1
merge_inv_condition_mask CondMask = $0 & ~$1
copy_slot_unmasked $2 = _2_bad
add_imm_int $2 += 0x00000001
copy_slot_masked _2_bad = Mask($2)
Expand All @@ -163,8 +151,7 @@ merge_condition_mask CondMask = $0 & $1
copy_slot_unmasked $2 = _1_ok
add_imm_int $2 += 0x00000001
copy_slot_masked _1_ok = Mask($2)
bitwise_xor_imm_int $1 ^= 0xFFFFFFFF
merge_condition_mask CondMask = $0 & $1
merge_inv_condition_mask CondMask = $0 & ~$1
copy_slot_unmasked $2 = _2_bad
add_imm_int $2 += 0x00000001
copy_slot_masked _2_bad = Mask($2)
Expand All @@ -176,8 +163,7 @@ merge_condition_mask CondMask = $0 & $1
copy_slot_unmasked $2 = _2_bad
add_imm_int $2 += 0x00000001
copy_slot_masked _2_bad = Mask($2)
bitwise_xor_imm_int $1 ^= 0xFFFFFFFF
merge_condition_mask CondMask = $0 & $1
merge_inv_condition_mask CondMask = $0 & ~$1
copy_slot_unmasked $2 = _1_ok
add_imm_int $2 += 0x00000001
copy_slot_masked _1_ok = Mask($2)
Expand All @@ -189,8 +175,7 @@ merge_condition_mask CondMask = $0 & $1
copy_slot_unmasked $2 = _2_bad
add_imm_int $2 += 0x00000001
copy_slot_masked _2_bad = Mask($2)
bitwise_xor_imm_int $1 ^= 0xFFFFFFFF
merge_condition_mask CondMask = $0 & $1
merge_inv_condition_mask CondMask = $0 & ~$1
copy_slot_unmasked $2 = _1_ok
add_imm_int $2 += 0x00000001
copy_slot_masked _1_ok = Mask($2)
Expand All @@ -201,8 +186,7 @@ merge_condition_mask CondMask = $0 & $1
copy_slot_unmasked $2 = _1_ok
add_imm_int $2 += 0x00000001
copy_slot_masked _1_ok = Mask($2)
bitwise_xor_imm_int $1 ^= 0xFFFFFFFF
merge_condition_mask CondMask = $0 & $1
merge_inv_condition_mask CondMask = $0 & ~$1
copy_slot_unmasked $2 = _2_bad
add_imm_int $2 += 0x00000001
copy_slot_masked _2_bad = Mask($2)
Expand All @@ -220,8 +204,7 @@ merge_condition_mask CondMask = $0 & $1
copy_slot_unmasked $2 = _2_bad
add_imm_int $2 += 0x00000001
copy_slot_masked _2_bad = Mask($2)
bitwise_xor_imm_int $1 ^= 0xFFFFFFFF
merge_condition_mask CondMask = $0 & $1
merge_inv_condition_mask CondMask = $0 & ~$1
copy_slot_unmasked $2 = _1_ok
add_imm_int $2 += 0x00000001
copy_slot_masked _1_ok = Mask($2)
Expand All @@ -238,8 +221,7 @@ merge_condition_mask CondMask = $0 & $1
copy_slot_unmasked $2 = _1_ok
add_imm_int $2 += 0x00000001
copy_slot_masked _1_ok = Mask($2)
bitwise_xor_imm_int $1 ^= 0xFFFFFFFF
merge_condition_mask CondMask = $0 & $1
merge_inv_condition_mask CondMask = $0 & ~$1
copy_slot_unmasked $2 = _2_bad
add_imm_int $2 += 0x00000001
copy_slot_masked _2_bad = Mask($2)
Expand Down
17 changes: 7 additions & 10 deletions tests/sksl/shared/CompileTimeConstantVariables.skrp
Original file line number Diff line number Diff line change
Expand Up @@ -18,40 +18,37 @@ cmpeq_imm_int $1 = equal($1, 0)
merge_condition_mask CondMask = $0 & $1
splat_4_constants $2..5 = 0x4008F5C3 (2.14)
copy_4_slots_masked [main].result = Mask($2..5)
bitwise_xor_imm_int $1 ^= 0xFFFFFFFF
merge_condition_mask CondMask = $0 & $1
merge_inv_condition_mask CondMask = $0 & ~$1
store_condition_mask $2 = CondMask
copy_slot_unmasked $3 = integerInput
cmpeq_imm_int $3 = equal($3, 0x00000001)
merge_condition_mask CondMask = $2 & $3
copy_4_uniforms $4..7 = colorGreen
copy_4_slots_masked [main].result = Mask($4..7)
bitwise_xor_imm_int $3 ^= 0xFFFFFFFF
merge_condition_mask CondMask = $2 & $3
merge_inv_condition_mask CondMask = $2 & ~$3
store_condition_mask $4 = CondMask
copy_slot_unmasked $5 = integerInput
cmpeq_imm_int $5 = equal($5, 0x00000002)
merge_condition_mask CondMask = $4 & $5
copy_4_slots_unmasked $6..9 = kConstVec
copy_4_slots_masked [main].result = Mask($6..9)
bitwise_xor_imm_int $5 ^= 0xFFFFFFFF
merge_condition_mask CondMask = $4 & $5
merge_inv_condition_mask CondMask = $4 & ~$5
copy_constant $6 = 0x4048F5C3 (3.14)
copy_uniform $7 = colorGreen(0)
mul_imm_float $7 *= 0x4048F5C3 (3.14)
cmplt_float $6 = lessThan($6, $7)
branch_if_no_active_lanes_eq branch +4 (label 0 at #47) if no lanes of $6 == 0xFFFFFFFF
branch_if_no_active_lanes_eq branch +4 (label 0 at #44) if no lanes of $6 == 0xFFFFFFFF
splat_4_constants $7..10 = 0x4048F5C3 (3.14)
copy_4_slots_masked [main].result = Mask($7..10)
jump jump +15 (label 1 at #61)
jump jump +15 (label 1 at #58)
label label 0
copy_uniform $7 = colorGreen(0)
mul_imm_float $7 *= 0x4008F5C3 (2.14)
cmple_imm_float $7 = lessThanEqual($7, 0x4008F5C3 (2.14))
branch_if_no_active_lanes_eq branch +4 (label 2 at #55) if no lanes of $7 == 0xFFFFFFFF
branch_if_no_active_lanes_eq branch +4 (label 2 at #52) if no lanes of $7 == 0xFFFFFFFF
splat_4_constants $8..11 = 0
copy_4_slots_masked [main].result = Mask($8..11)
jump jump +6 (label 3 at #60)
jump jump +6 (label 3 at #57)
label label 0x00000002
copy_constant $8 = 0x3F800000 (1.0)
splat_2_constants $9..10 = 0
Expand Down
Loading

0 comments on commit e88e5de

Please sign in to comment.