Add mechanism for recycling temp slots.
Previously, every time we used a value slot for something ephemeral
(e.g. a continue-mask in a loop), the slot was lost forever. Now,
it is added to a recycle-list and can be reclaimed.
I think I will need a temporary value slot to implement non-constant
array indexing, and it would be pretty wasteful if every array index
burned a slot forever.
Change-Id: I1a31e7cea8b2c437c9d00d47c709e980694df401
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/635179
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
diff --git a/src/sksl/codegen/SkSLRasterPipelineCodeGenerator.cpp b/src/sksl/codegen/SkSLRasterPipelineCodeGenerator.cpp
index cc689b4..63e256f 100644
--- a/src/sksl/codegen/SkSLRasterPipelineCodeGenerator.cpp
+++ b/src/sksl/codegen/SkSLRasterPipelineCodeGenerator.cpp
@@ -86,15 +86,20 @@
Position pos,
bool isFunctionReturnValue);
- /** Implements low-level slot creation; slots will not be known to the debugger. */
- SlotRange createSlots(int slots);
-
/** Creates slots associated with an SkSL variable or return value. */
SlotRange createSlots(std::string name,
const Type& type,
Position pos,
bool isFunctionReturnValue);
+ /**
+ * Creates a single temporary slot for scratch storage. Temporary slots can be recycled, which
+ * frees them up for reuse. Temporary slots are not assigned a name and have an arbitrary type.
+ */
+ SlotRange createTemporarySlot(const Type& type);
+ void recycleTemporarySlot(SlotRange temporarySlot);
+
+
/** Looks up the slots associated with an SkSL variable; creates the slot if necessary. */
SlotRange getVariableSlots(const Variable& v);
@@ -109,8 +114,12 @@
int slotCount() const { return fSlotCount; }
private:
+ std::string makeTempName() { return SkSL::String::printf("[temporary %d]", fTemporaryCount++); }
+
SkTHashMap<const IRNode*, SlotRange> fSlotMap;
+ SkTArray<Slot> fRecycledSlots;
int fSlotCount = 0;
+ int fTemporaryCount = 0;
std::vector<SlotDebugInfo>* fSlotDebugInfo;
};
@@ -263,10 +272,6 @@
}
BuilderOp getTypedOp(const SkSL::Type& type, const TypedOps& ops) const;
- std::string makeMaskName(const char* name) {
- return SkSL::String::printf("[%s %d]", name, fTempNameIndex++);
- }
-
bool needsReturnMask() {
Analysis::ReturnComplexity* complexity = fReturnComplexityMap.find(fCurrentFunction);
if (!complexity) {
@@ -302,7 +307,6 @@
SlotRange fCurrentFunctionResult;
SlotRange fCurrentContinueMask;
int fCurrentTempStack = 0;
- int fTempNameIndex = 0;
SkTHashMap<const FunctionDefinition*, Analysis::ReturnComplexity> fReturnComplexityMap;
@@ -589,10 +593,38 @@
SkASSERT((size_t)groupIndex == type.slotCount());
}
-SlotRange SlotManager::createSlots(int slots) {
- SlotRange range = {fSlotCount, slots};
- fSlotCount += slots;
- return range;
+SlotRange SlotManager::createTemporarySlot(const Type& type) {
+ SkASSERT(type.slotCount() == 1);
+
+ // If we have an available slot to reclaim, take it now.
+ if (!fRecycledSlots.empty()) {
+ SlotRange result = {fRecycledSlots.back(), 1};
+ fRecycledSlots.pop_back();
+ return result;
+ }
+
+ // Synthesize a new temporary slot.
+ if (fSlotDebugInfo) {
+ // Our debug slot-info table should have the same length as the actual slot table.
+ SkASSERT(fSlotDebugInfo->size() == (size_t)fSlotCount);
+
+ // Add this temporary slot to the debug slot-info table. It's just scratch space which can
+ // be reused over the course of execution, so it doesn't get a name or type (uint will do).
+ this->addSlotDebugInfo(this->makeTempName(), type, Position{},
+ /*isFunctionReturnValue=*/false);
+
+ // Confirm that we added the expected number of slots.
+ SkASSERT(fSlotDebugInfo->size() == (size_t)(fSlotCount + 1));
+ }
+
+ SlotRange result = {fSlotCount, 1};
+ ++fSlotCount;
+ return result;
+}
+
+void SlotManager::recycleTemporarySlot(SlotRange temporarySlot) {
+ SkASSERT(temporarySlot.count == 1);
+ fRecycledSlots.push_back(temporarySlot.index);
}
SlotRange SlotManager::createSlots(std::string name,
@@ -615,7 +647,9 @@
SkASSERT(fSlotDebugInfo->size() == (size_t)(fSlotCount + nslots));
}
- return this->createSlots(nslots);
+ SlotRange result = {fSlotCount, (int)nslots};
+ fSlotCount += nslots;
+ return result;
}
SlotRange SlotManager::getVariableSlots(const Variable& v) {
@@ -805,12 +839,10 @@
fBuilder.enableExecutionMaskWrites();
fBuilder.push_loop_mask();
- // Create a dedicated slot for continue-mask storage.
+ // Acquire a temporary slot for continue-mask storage.
SlotRange previousContinueMask = fCurrentContinueMask;
- fCurrentContinueMask = fProgramSlots.createSlots(this->makeMaskName("do-loop continue mask"),
- *fProgram.fContext->fTypes.fBool,
- Position{},
- /*isFunctionReturnValue=*/false);
+ fCurrentContinueMask = fProgramSlots.createTemporarySlot(*fProgram.fContext->fTypes.fUInt);
+
// Write the do-loop body.
int labelID = fBuilder.nextLabelID();
fBuilder.label(labelID);
@@ -837,6 +869,7 @@
// Restore the loop and continue masks.
fBuilder.pop_loop_mask();
fBuilder.disableExecutionMaskWrites();
+ fProgramSlots.recycleTemporarySlot(fCurrentContinueMask);
fCurrentContinueMask = previousContinueMask;
return true;
@@ -911,14 +944,11 @@
return unsupported();
}
- // Create a dedicated slot for continue-mask storage.
- SlotRange previousContinueMask = fCurrentContinueMask;
+ // Acquire a temporary slot for continue-mask storage.
+ SlotRange previousContinueMask;
if (loopInfo.fHasContinue) {
- fCurrentContinueMask =
- fProgramSlots.createSlots(this->makeMaskName("for-loop continue mask"),
- *fProgram.fContext->fTypes.fBool,
- Position{},
- /*isFunctionReturnValue=*/false);
+ previousContinueMask = fCurrentContinueMask;
+ fCurrentContinueMask = fProgramSlots.createTemporarySlot(*fProgram.fContext->fTypes.fUInt);
}
// Save off the original loop mask.
@@ -970,7 +1000,10 @@
// Restore the loop and continue masks.
fBuilder.pop_loop_mask();
fBuilder.disableExecutionMaskWrites();
- fCurrentContinueMask = previousContinueMask;
+ if (loopInfo.fHasContinue) {
+ fProgramSlots.recycleTemporarySlot(fCurrentContinueMask);
+ fCurrentContinueMask = previousContinueMask;
+ }
return true;
}
diff --git a/tests/sksl/runtime/LoopFloat.skrp b/tests/sksl/runtime/LoopFloat.skrp
index 5f58c7f..b60f704 100644
--- a/tests/sksl/runtime/LoopFloat.skrp
+++ b/tests/sksl/runtime/LoopFloat.skrp
@@ -62,20 +62,20 @@
62. zero_2_slots_unmasked sum(0..1) = 0
63. store_loop_mask $16 = LoopMask
64. jump jump +19 (#83)
- 65. zero_slot_unmasked [for-loop continue mask 0] = 0
+ 65. zero_slot_unmasked [temporary 0] = 0
66. store_condition_mask $17 = CondMask
67. copy_slot_unmasked $18 = i
68. copy_slot_unmasked $19 = five
69. cmplt_float $18 = lessThan($18, $19)
70. merge_condition_mask CondMask = $17 & $18
71. copy_constant $19 = 0xFFFFFFFF
- 72. copy_slot_masked [for-loop continue mask 0] = Mask($19)
+ 72. copy_slot_masked [temporary 0] = Mask($19)
73. mask_off_loop_mask LoopMask &= ~(CondMask & LoopMask & RetMask)
74. load_condition_mask CondMask = $17
75. copy_2_slots_unmasked $17..18 = sum(0..1)
76. add_float $17 += $18
77. copy_slot_masked sum = Mask($17)
- 78. reenable_loop_mask LoopMask |= [for-loop continue mask 0]
+ 78. reenable_loop_mask LoopMask |= [temporary 0]
79. copy_slot_unmasked $17 = i
80. copy_constant $18 = 0x3F800000 (1.0)
81. add_float $17 += $18
diff --git a/tests/sksl/runtime/LoopInt.skrp b/tests/sksl/runtime/LoopInt.skrp
index 8c1d03d..d05c4ea 100644
--- a/tests/sksl/runtime/LoopInt.skrp
+++ b/tests/sksl/runtime/LoopInt.skrp
@@ -62,20 +62,20 @@
62. zero_2_slots_unmasked sum(0..1) = 0
63. store_loop_mask $72 = LoopMask
64. jump jump +19 (#83)
- 65. zero_slot_unmasked [for-loop continue mask 0] = 0
+ 65. zero_slot_unmasked [temporary 0] = 0
66. store_condition_mask $73 = CondMask
67. copy_slot_unmasked $74 = i
68. copy_slot_unmasked $75 = five
69. cmplt_int $74 = lessThan($74, $75)
70. merge_condition_mask CondMask = $73 & $74
71. copy_constant $75 = 0xFFFFFFFF
- 72. copy_slot_masked [for-loop continue mask 0] = Mask($75)
+ 72. copy_slot_masked [temporary 0] = Mask($75)
73. mask_off_loop_mask LoopMask &= ~(CondMask & LoopMask & RetMask)
74. load_condition_mask CondMask = $73
75. copy_2_slots_unmasked $73..74 = sum(0..1)
76. add_int $73 += $74
77. copy_slot_masked sum = Mask($73)
- 78. reenable_loop_mask LoopMask |= [for-loop continue mask 0]
+ 78. reenable_loop_mask LoopMask |= [temporary 0]
79. copy_slot_unmasked $73 = i
80. copy_constant $74 = 0x00000001 (1.401298e-45)
81. add_int $73 += $74
diff --git a/tests/sksl/shared/DeadReturnES3.skrp b/tests/sksl/shared/DeadReturnES3.skrp
index 08bb2de..6f96766 100644
--- a/tests/sksl/shared/DeadReturnES3.skrp
+++ b/tests/sksl/shared/DeadReturnES3.skrp
@@ -11,11 +11,11 @@
11. branch_if_no_active_lanes branch_if_no_active_lanes +15 (#26)
12. store_return_mask $36 = RetMask
13. store_loop_mask $37 = LoopMask
- 14. zero_slot_unmasked [do-loop continue mask 0] = 0
+ 14. zero_slot_unmasked [temporary 0] = 0
15. copy_constant $38 = 0xFFFFFFFF
16. copy_slot_masked [test_return].result = Mask($38)
17. mask_off_return_mask RetMask &= ~(CondMask & LoopMask & RetMask)
- 18. reenable_loop_mask LoopMask |= [do-loop continue mask 0]
+ 18. reenable_loop_mask LoopMask |= [temporary 0]
19. zero_slot_unmasked $38 = 0
20. merge_loop_mask LoopMask &= $38
21. stack_rewind
@@ -27,9 +27,9 @@
27. merge_condition_mask CondMask = $35 & $36
28. branch_if_no_active_lanes branch_if_no_active_lanes +13 (#41)
29. store_loop_mask $10 = LoopMask
- 30. zero_slot_unmasked [do-loop continue mask 1] = 0
+ 30. zero_slot_unmasked [temporary 0] = 0
31. mask_off_loop_mask LoopMask &= ~(CondMask & LoopMask & RetMask)
- 32. reenable_loop_mask LoopMask |= [do-loop continue mask 1]
+ 32. reenable_loop_mask LoopMask |= [temporary 0]
33. zero_slot_unmasked $11 = 0
34. merge_loop_mask LoopMask &= $11
35. stack_rewind
@@ -43,11 +43,11 @@
43. merge_condition_mask CondMask = $8 & $9
44. branch_if_no_active_lanes branch_if_no_active_lanes +15 (#59)
45. store_loop_mask $33 = LoopMask
- 46. zero_slot_unmasked [do-loop continue mask 2] = 0
+ 46. zero_slot_unmasked [temporary 0] = 0
47. copy_constant $34 = 0xFFFFFFFF
- 48. copy_slot_masked [do-loop continue mask 2] = Mask($34)
+ 48. copy_slot_masked [temporary 0] = Mask($34)
49. mask_off_loop_mask LoopMask &= ~(CondMask & LoopMask & RetMask)
- 50. reenable_loop_mask LoopMask |= [do-loop continue mask 2]
+ 50. reenable_loop_mask LoopMask |= [temporary 0]
51. zero_slot_unmasked $34 = 0
52. merge_loop_mask LoopMask &= $34
53. stack_rewind
@@ -62,7 +62,7 @@
62. branch_if_no_active_lanes branch_if_no_active_lanes +27 (#89)
63. store_return_mask $41 = RetMask
64. store_loop_mask $42 = LoopMask
- 65. zero_slot_unmasked [do-loop continue mask 3] = 0
+ 65. zero_slot_unmasked [temporary 0] = 0
66. zero_slot_unmasked $43 = 0
67. copy_constant $44 = colorGreen(1)
68. cmplt_float $43 = lessThan($43, $44)
@@ -73,9 +73,9 @@
73. jump jump +2 (#75)
74. mask_off_loop_mask LoopMask &= ~(CondMask & LoopMask & RetMask)
75. copy_constant $43 = 0xFFFFFFFF
- 76. copy_slot_masked [do-loop continue mask 3] = Mask($43)
+ 76. copy_slot_masked [temporary 0] = Mask($43)
77. mask_off_loop_mask LoopMask &= ~(CondMask & LoopMask & RetMask)
- 78. reenable_loop_mask LoopMask |= [do-loop continue mask 3]
+ 78. reenable_loop_mask LoopMask |= [temporary 0]
79. zero_slot_unmasked $43 = 0
80. merge_loop_mask LoopMask &= $43
81. stack_rewind
@@ -91,7 +91,7 @@
91. merge_condition_mask CondMask = $39 & $40
92. branch_if_no_active_lanes branch_if_no_active_lanes +21 (#113)
93. store_loop_mask $19 = LoopMask
- 94. zero_slot_unmasked [do-loop continue mask 4] = 0
+ 94. zero_slot_unmasked [temporary 0] = 0
95. zero_slot_unmasked $20 = 0
96. copy_constant $21 = colorGreen(1)
97. cmplt_float $20 = lessThan($20, $21)
@@ -99,9 +99,9 @@
99. mask_off_loop_mask LoopMask &= ~(CondMask & LoopMask & RetMask)
100. jump jump +4 (#104)
101. copy_constant $21 = 0xFFFFFFFF
- 102. copy_slot_masked [do-loop continue mask 4] = Mask($21)
+ 102. copy_slot_masked [temporary 0] = Mask($21)
103. mask_off_loop_mask LoopMask &= ~(CondMask & LoopMask & RetMask)
- 104. reenable_loop_mask LoopMask |= [do-loop continue mask 4]
+ 104. reenable_loop_mask LoopMask |= [temporary 0]
105. zero_slot_unmasked $20 = 0
106. merge_loop_mask LoopMask &= $20
107. stack_rewind
@@ -116,7 +116,7 @@
116. branch_if_no_active_lanes branch_if_no_active_lanes +24 (#140)
117. store_return_mask $27 = RetMask
118. store_loop_mask $28 = LoopMask
- 119. zero_slot_unmasked [do-loop continue mask 5] = 0
+ 119. zero_slot_unmasked [temporary 0] = 0
120. copy_constant $29 = colorGreen(1)
121. zero_slot_unmasked $30 = 0
122. cmpeq_float $29 = equal($29, $30)
@@ -128,7 +128,7 @@
128. copy_constant $30 = 0xFFFFFFFF
129. copy_slot_masked [test_else].result = Mask($30)
130. mask_off_return_mask RetMask &= ~(CondMask & LoopMask & RetMask)
- 131. reenable_loop_mask LoopMask |= [do-loop continue mask 5]
+ 131. reenable_loop_mask LoopMask |= [temporary 0]
132. zero_slot_unmasked $29 = 0
133. merge_loop_mask LoopMask &= $29
134. stack_rewind
diff --git a/tests/sksl/shared/DoWhileControlFlow.skrp b/tests/sksl/shared/DoWhileControlFlow.skrp
index 89191a0..4a3a77f 100644
--- a/tests/sksl/shared/DoWhileControlFlow.skrp
+++ b/tests/sksl/shared/DoWhileControlFlow.skrp
@@ -5,7 +5,7 @@
5. copy_constant x(2) = 0x3F800000 (1.0)
6. copy_constant x(3) = 0x3F800000 (1.0)
7. store_loop_mask $0 = LoopMask
- 8. zero_slot_unmasked [do-loop continue mask 0] = 0
+ 8. zero_slot_unmasked [temporary 0] = 0
9. copy_slot_unmasked $1 = x(0)
10. copy_constant $2 = 0x3E800000 (0.25)
11. sub_float $1 -= $2
@@ -17,7 +17,7 @@
17. merge_condition_mask CondMask = $1 & $2
18. mask_off_loop_mask LoopMask &= ~(CondMask & LoopMask & RetMask)
19. load_condition_mask CondMask = $1
- 20. reenable_loop_mask LoopMask |= [do-loop continue mask 0]
+ 20. reenable_loop_mask LoopMask |= [temporary 0]
21. copy_slot_unmasked $1 = x(3)
22. copy_constant $2 = 0x3F800000 (1.0)
23. cmpeq_float $1 = equal($1, $2)
@@ -26,7 +26,7 @@
26. branch_if_any_active_lanes branch_if_any_active_lanes -18 (#8)
27. load_loop_mask LoopMask = $0
28. store_loop_mask $0 = LoopMask
- 29. zero_slot_unmasked [do-loop continue mask 1] = 0
+ 29. zero_slot_unmasked [temporary 0] = 0
30. copy_slot_unmasked $1 = x(2)
31. copy_constant $2 = 0x3E800000 (0.25)
32. sub_float $1 -= $2
@@ -37,12 +37,12 @@
37. cmpeq_float $2 = equal($2, $3)
38. merge_condition_mask CondMask = $1 & $2
39. copy_constant $3 = 0xFFFFFFFF
- 40. copy_slot_masked [do-loop continue mask 1] = Mask($3)
+ 40. copy_slot_masked [temporary 0] = Mask($3)
41. mask_off_loop_mask LoopMask &= ~(CondMask & LoopMask & RetMask)
42. load_condition_mask CondMask = $1
43. zero_slot_unmasked $1 = 0
44. copy_slot_masked x(1) = Mask($1)
- 45. reenable_loop_mask LoopMask |= [do-loop continue mask 1]
+ 45. reenable_loop_mask LoopMask |= [temporary 0]
46. zero_slot_unmasked $1 = 0
47. copy_slot_unmasked $2 = x(2)
48. cmplt_float $1 = lessThan($1, $2)
diff --git a/tests/sksl/shared/EmptyBlocksES3.skrp b/tests/sksl/shared/EmptyBlocksES3.skrp
index 6bcd329..5555ae3 100644
--- a/tests/sksl/shared/EmptyBlocksES3.skrp
+++ b/tests/sksl/shared/EmptyBlocksES3.skrp
@@ -24,8 +24,8 @@
24. branch_if_any_active_lanes branch_if_any_active_lanes -5 (#19)
25. load_loop_mask LoopMask = $0
26. store_loop_mask $0 = LoopMask
- 27. zero_slot_unmasked [do-loop continue mask 0] = 0
- 28. reenable_loop_mask LoopMask |= [do-loop continue mask 0]
+ 27. zero_slot_unmasked [temporary 0] = 0
+ 28. reenable_loop_mask LoopMask |= [temporary 0]
29. copy_constant $1 = colorWhite(0)
30. copy_constant $2 = 0x40000000 (2.0)
31. cmpeq_float $1 = equal($1, $2)
diff --git a/tests/sksl/shared/ForLoopControlFlow.skrp b/tests/sksl/shared/ForLoopControlFlow.skrp
index 50426c5..dc59bbe 100644
--- a/tests/sksl/shared/ForLoopControlFlow.skrp
+++ b/tests/sksl/shared/ForLoopControlFlow.skrp
@@ -32,7 +32,7 @@
32. copy_constant b = 0x40A00000 (5.0)
33. store_loop_mask $0 = LoopMask
34. jump jump +20 (#54)
- 35. zero_slot_unmasked [for-loop continue mask 0] = 0
+ 35. zero_slot_unmasked [temporary 0] = 0
36. copy_slot_unmasked $1 = b
37. copy_slot_masked x(2) = Mask($1)
38. store_condition_mask $1 = CondMask
@@ -41,12 +41,12 @@
41. cmpeq_float $2 = equal($2, $3)
42. merge_condition_mask CondMask = $1 & $2
43. copy_constant $3 = 0xFFFFFFFF
- 44. copy_slot_masked [for-loop continue mask 0] = Mask($3)
+ 44. copy_slot_masked [temporary 0] = Mask($3)
45. mask_off_loop_mask LoopMask &= ~(CondMask & LoopMask & RetMask)
46. load_condition_mask CondMask = $1
47. zero_slot_unmasked $1 = 0
48. copy_slot_masked x(1) = Mask($1)
- 49. reenable_loop_mask LoopMask |= [for-loop continue mask 0]
+ 49. reenable_loop_mask LoopMask |= [temporary 0]
50. copy_slot_unmasked $1 = b
51. copy_constant $2 = 0x3F800000 (1.0)
52. sub_float $1 -= $2
diff --git a/tests/sksl/shared/UnusedVariables.skrp b/tests/sksl/shared/UnusedVariables.skrp
index 09626d4..38bec84 100644
--- a/tests/sksl/shared/UnusedVariables.skrp
+++ b/tests/sksl/shared/UnusedVariables.skrp
@@ -35,11 +35,11 @@
35. zero_slot_unmasked x = 0
36. store_loop_mask $0 = LoopMask
37. jump jump +10 (#47)
- 38. zero_slot_unmasked [for-loop continue mask 0] = 0
+ 38. zero_slot_unmasked [temporary 0] = 0
39. copy_constant $1 = 0xFFFFFFFF
- 40. copy_slot_masked [for-loop continue mask 0] = Mask($1)
+ 40. copy_slot_masked [temporary 0] = Mask($1)
41. mask_off_loop_mask LoopMask &= ~(CondMask & LoopMask & RetMask)
- 42. reenable_loop_mask LoopMask |= [for-loop continue mask 0]
+ 42. reenable_loop_mask LoopMask |= [temporary 0]
43. copy_slot_unmasked $1 = x
44. copy_constant $2 = 0x00000001 (1.401298e-45)
45. add_int $1 += $2
diff --git a/tests/sksl/shared/WhileLoopControlFlow.skrp b/tests/sksl/shared/WhileLoopControlFlow.skrp
index 65716c3..792061f 100644
--- a/tests/sksl/shared/WhileLoopControlFlow.skrp
+++ b/tests/sksl/shared/WhileLoopControlFlow.skrp
@@ -26,7 +26,7 @@
26. load_loop_mask LoopMask = $0
27. store_loop_mask $0 = LoopMask
28. jump jump +18 (#46)
- 29. zero_slot_unmasked [for-loop continue mask 0] = 0
+ 29. zero_slot_unmasked [temporary 0] = 0
30. copy_slot_unmasked $1 = x(2)
31. copy_constant $2 = 0x3E800000 (0.25)
32. sub_float $1 -= $2
@@ -37,12 +37,12 @@
37. cmpeq_float $2 = equal($2, $3)
38. merge_condition_mask CondMask = $1 & $2
39. copy_constant $3 = 0xFFFFFFFF
- 40. copy_slot_masked [for-loop continue mask 0] = Mask($3)
+ 40. copy_slot_masked [temporary 0] = Mask($3)
41. mask_off_loop_mask LoopMask &= ~(CondMask & LoopMask & RetMask)
42. load_condition_mask CondMask = $1
43. zero_slot_unmasked $1 = 0
44. copy_slot_masked x(1) = Mask($1)
- 45. reenable_loop_mask LoopMask |= [for-loop continue mask 0]
+ 45. reenable_loop_mask LoopMask |= [temporary 0]
46. zero_slot_unmasked $1 = 0
47. copy_slot_unmasked $2 = x(2)
48. cmplt_float $1 = lessThan($1, $2)
