Skip to content

Commit 687bbf8

Browse files
LebedevRILebedevRI
committed
[llvm-exegesis] CombinationGenerator: don't store function_ref
function_ref is non-owning, so if we get it as a parameter in constructor, our reference goes out-of-scope as soon as constructor returns. Instead, let's just take it as a parameter to the actual `generate()` call
1 parent e26c24b commit 687bbf8

File tree

3 files changed

+54
-61
lines changed

3 files changed

+54
-61
lines changed

‎llvm/tools/llvm-exegesis/lib/SnippetGenerator.h

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -156,9 +156,9 @@ class CombinationGenerator {
156156
};
157157

158158
const ArrayRef<choices_storage_type> VariablesChoices;
159-
const function_ref<bool(ArrayRef<choice_type>)> &Callback;
160159

161-
void performGeneration() const {
160+
void performGeneration(
161+
const function_ref<bool(ArrayRef<choice_type>)> Callback) const {
162162
SmallVector<WrappingIterator<choice_type>, variable_smallsize>
163163
VariablesState;
164164

@@ -200,9 +200,8 @@ class CombinationGenerator {
200200
};
201201

202202
public:
203-
CombinationGenerator(ArrayRef<choices_storage_type> VariablesChoices_,
204-
const function_ref<bool(ArrayRef<choice_type>)> &Cb_)
205-
: VariablesChoices(VariablesChoices_), Callback(Cb_) {
203+
CombinationGenerator(ArrayRef<choices_storage_type> VariablesChoices_)
204+
: VariablesChoices(VariablesChoices_) {
206205
#ifndef NDEBUG
207206
assert(!VariablesChoices.empty() && "There should be some variables.");
208207
llvm::for_each(VariablesChoices, [](ArrayRef<choice_type> VariableChoices) {
@@ -225,7 +224,9 @@ class CombinationGenerator {
225224

226225
// Actually perform exhaustive combination generation.
227226
// Each result will be passed into the callback.
228-
void generate() { performGeneration(); }
227+
void generate(const function_ref<bool(ArrayRef<choice_type>)> Callback) {
228+
performGeneration(Callback);
229+
}
229230
};
230231

231232
} // namespace exegesis

‎llvm/tools/llvm-exegesis/lib/X86/Target.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -784,18 +784,18 @@ std::vector<InstructionTemplate> ExegesisX86Target::generateInstructionVariants(
784784
std::vector<InstructionTemplate> Variants;
785785
size_t NumVariants;
786786
CombinationGenerator<MCOperand, decltype(VariableChoices)::value_type, 4> G(
787-
VariableChoices, [&](ArrayRef<MCOperand> State) -> bool {
788-
Variants.emplace_back(&Instr);
789-
Variants.back().setVariableValues(State);
790-
// Did we run out of space for variants?
791-
return Variants.size() >= NumVariants;
792-
});
787+
VariableChoices);
793788

794789
// How many operand combinations can we produce, within the limit?
795790
NumVariants = std::min(G.numCombinations(), (size_t)MaxConfigsPerOpcode);
796791
// And actually produce all the wanted operand combinations.
797792
Variants.reserve(NumVariants);
798-
G.generate();
793+
G.generate([&](ArrayRef<MCOperand> State) -> bool {
794+
Variants.emplace_back(&Instr);
795+
Variants.back().setVariableValues(State);
796+
// Did we run out of space for variants?
797+
return Variants.size() >= NumVariants;
798+
});
799799

800800
assert(Variants.size() == NumVariants &&
801801
Variants.size() <= MaxConfigsPerOpcode &&

‎llvm/unittests/tools/llvm-exegesis/SnippetGeneratorTest.cpp

Lines changed: 40 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,12 @@ TEST(CombinationGenerator, Square) {
2020
const std::vector<std::vector<int>> Choices{{0, 1}, {2, 3}};
2121

2222
std::vector<std::vector<int>> Variants;
23-
CombinationGenerator<int, std::vector<int>, 4> G(
24-
Choices, [&](ArrayRef<int> State) -> bool {
25-
Variants.emplace_back(State);
26-
return false; // keep going
27-
});
23+
CombinationGenerator<int, std::vector<int>, 4> G(Choices);
2824
const size_t NumVariants = G.numCombinations();
29-
G.generate();
25+
G.generate([&](ArrayRef<int> State) -> bool {
26+
Variants.emplace_back(State);
27+
return false; // keep going
28+
});
3029

3130
const std::vector<std::vector<int>> ExpectedVariants{
3231
{0, 2},
@@ -42,13 +41,12 @@ TEST(CombinationGenerator, MiddleColumn) {
4241
const std::vector<std::vector<int>> Choices{{0}, {1, 2}, {3}};
4342

4443
std::vector<std::vector<int>> Variants;
45-
CombinationGenerator<int, std::vector<int>, 4> G(
46-
Choices, [&](ArrayRef<int> State) -> bool {
47-
Variants.emplace_back(State);
48-
return false; // keep going
49-
});
44+
CombinationGenerator<int, std::vector<int>, 4> G(Choices);
5045
const size_t NumVariants = G.numCombinations();
51-
G.generate();
46+
G.generate([&](ArrayRef<int> State) -> bool {
47+
Variants.emplace_back(State);
48+
return false; // keep going
49+
});
5250

5351
const std::vector<std::vector<int>> ExpectedVariants{
5452
{0, 1, 3},
@@ -62,13 +60,12 @@ TEST(CombinationGenerator, SideColumns) {
6260
const std::vector<std::vector<int>> Choices{{0, 1}, {2}, {3, 4}};
6361

6462
std::vector<std::vector<int>> Variants;
65-
CombinationGenerator<int, std::vector<int>, 4> G(
66-
Choices, [&](ArrayRef<int> State) -> bool {
67-
Variants.emplace_back(State);
68-
return false; // keep going
69-
});
63+
CombinationGenerator<int, std::vector<int>, 4> G(Choices);
7064
const size_t NumVariants = G.numCombinations();
71-
G.generate();
65+
G.generate([&](ArrayRef<int> State) -> bool {
66+
Variants.emplace_back(State);
67+
return false; // keep going
68+
});
7269

7370
const std::vector<std::vector<int>> ExpectedVariants{
7471
{0, 2, 3},
@@ -84,13 +81,12 @@ TEST(CombinationGenerator, LeftColumn) {
8481
const std::vector<std::vector<int>> Choices{{0, 1}, {2}};
8582

8683
std::vector<std::vector<int>> Variants;
87-
CombinationGenerator<int, std::vector<int>, 4> G(
88-
Choices, [&](ArrayRef<int> State) -> bool {
89-
Variants.emplace_back(State);
90-
return false; // keep going
91-
});
84+
CombinationGenerator<int, std::vector<int>, 4> G(Choices);
9285
const size_t NumVariants = G.numCombinations();
93-
G.generate();
86+
G.generate([&](ArrayRef<int> State) -> bool {
87+
Variants.emplace_back(State);
88+
return false; // keep going
89+
});
9490

9591
const std::vector<std::vector<int>> ExpectedVariants{
9692
{0, 2},
@@ -104,13 +100,12 @@ TEST(CombinationGenerator, RightColumn) {
104100
const std::vector<std::vector<int>> Choices{{0}, {1, 2}};
105101

106102
std::vector<std::vector<int>> Variants;
107-
CombinationGenerator<int, std::vector<int>, 4> G(
108-
Choices, [&](ArrayRef<int> State) -> bool {
109-
Variants.emplace_back(State);
110-
return false; // keep going
111-
});
103+
CombinationGenerator<int, std::vector<int>, 4> G(Choices);
112104
const size_t NumVariants = G.numCombinations();
113-
G.generate();
105+
G.generate([&](ArrayRef<int> State) -> bool {
106+
Variants.emplace_back(State);
107+
return false; // keep going
108+
});
114109

115110
const std::vector<std::vector<int>> ExpectedVariants{
116111
{0, 1},
@@ -124,13 +119,12 @@ TEST(CombinationGenerator, Column) {
124119
const std::vector<std::vector<int>> Choices{{0, 1}};
125120

126121
std::vector<std::vector<int>> Variants;
127-
CombinationGenerator<int, std::vector<int>, 4> G(
128-
Choices, [&](ArrayRef<int> State) -> bool {
129-
Variants.emplace_back(State);
130-
return false; // keep going
131-
});
122+
CombinationGenerator<int, std::vector<int>, 4> G(Choices);
132123
const size_t NumVariants = G.numCombinations();
133-
G.generate();
124+
G.generate([&](ArrayRef<int> State) -> bool {
125+
Variants.emplace_back(State);
126+
return false; // keep going
127+
});
134128

135129
const std::vector<std::vector<int>> ExpectedVariants{
136130
{0},
@@ -144,13 +138,12 @@ TEST(CombinationGenerator, Row) {
144138
const std::vector<std::vector<int>> Choices{{0}, {1}};
145139

146140
std::vector<std::vector<int>> Variants;
147-
CombinationGenerator<int, std::vector<int>, 4> G(
148-
Choices, [&](ArrayRef<int> State) -> bool {
149-
Variants.emplace_back(State);
150-
return false; // keep going
151-
});
141+
CombinationGenerator<int, std::vector<int>, 4> G(Choices);
152142
const size_t NumVariants = G.numCombinations();
153-
G.generate();
143+
G.generate([&](ArrayRef<int> State) -> bool {
144+
Variants.emplace_back(State);
145+
return false; // keep going
146+
});
154147

155148
const std::vector<std::vector<int>> ExpectedVariants{
156149
{0, 1},
@@ -163,13 +156,12 @@ TEST(CombinationGenerator, Singleton) {
163156
const std::vector<std::vector<int>> Choices{{0}};
164157

165158
std::vector<std::vector<int>> Variants;
166-
CombinationGenerator<int, std::vector<int>, 4> G(
167-
Choices, [&](ArrayRef<int> State) -> bool {
168-
Variants.emplace_back(State);
169-
return false; // keep going
170-
});
159+
CombinationGenerator<int, std::vector<int>, 4> G(Choices);
171160
const size_t NumVariants = G.numCombinations();
172-
G.generate();
161+
G.generate([&](ArrayRef<int> State) -> bool {
162+
Variants.emplace_back(State);
163+
return false; // keep going
164+
});
173165

174166
const std::vector<std::vector<int>> ExpectedVariants{
175167
{0},

0 commit comments

Comments
 (0)