Skip to content

Commit 10b4aec

Browse files
committed
Revert "Avoid creating an immutable map in the Automaton class."
This reverts commit 051d330. It broke buildbots, for example, http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/21908.
1 parent 0734fb2 commit 10b4aec

File tree

2 files changed

+30
-43
lines changed

2 files changed

+30
-43
lines changed

llvm/include/llvm/Support/Automaton.h

Lines changed: 22 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -154,42 +154,29 @@ class NfaTranscriber {
154154
};
155155
} // namespace internal
156156

157-
// Type representing a transition in the DFA state.
158-
// The action type can be a custom type.
159-
template <typename ActionT> struct TransitionType {
160-
using StateT = unsigned;
161-
StateT FromDfaState; // The transitioned-from DFA state.
162-
ActionT Action; // The input symbol that causes this transition.
163-
StateT ToDfaState; // The transitioned-to DFA state.
164-
unsigned InfoIdx; // Start index into TransitionInfo.
165-
};
166-
167157
/// A deterministic finite-state automaton. The automaton is defined in
168158
/// TableGen; this object drives an automaton defined by tblgen-emitted tables.
169159
///
170160
/// An automaton accepts a sequence of input tokens ("actions"). This class is
171161
/// templated on the type of these actions.
172162
template <typename ActionT> class Automaton {
173-
using TransitionT = TransitionType<ActionT>;
174-
using TransitionKeyT = std::pair<decltype(TransitionT::FromDfaState),
175-
decltype(TransitionT::Action)>;
176-
using StateT = typename TransitionT::StateT;
177163
/// Map from {State, Action} to {NewState, TransitionInfoIdx}.
178164
/// TransitionInfoIdx is used by the DfaTranscriber to analyze the transition.
179-
ArrayRef<TransitionT> TransitionMap;
165+
/// FIXME: This uses a std::map because ActionT can be a pair type including
166+
/// an enum. In particular DenseMapInfo<ActionT> must be defined to use
167+
/// DenseMap here.
168+
/// This is a shared_ptr to allow very quick copy-construction of Automata; this
169+
/// state is immutable after construction so this is safe.
170+
using MapTy = std::map<std::pair<uint64_t, ActionT>, std::pair<uint64_t, unsigned>>;
171+
std::shared_ptr<MapTy> M;
180172
/// An optional transcription object. This uses much more state than simply
181173
/// traversing the DFA for acceptance, so is heap allocated.
182174
std::shared_ptr<internal::NfaTranscriber> Transcriber;
183175
/// The initial DFA state is 1.
184-
StateT State = 1;
176+
uint64_t State = 1;
185177
/// True if we should transcribe and false if not (even if Transcriber is defined).
186178
bool Transcribe;
187179

188-
static bool transitionLessThan(const TransitionT &A,
189-
const TransitionKeyT &B) {
190-
return std::make_pair(A.FromDfaState, A.Action) < B;
191-
}
192-
193180
public:
194181
/// Create an automaton.
195182
/// \param Transitions The Transitions table as created by TableGen. Note that
@@ -202,24 +189,21 @@ template <typename ActionT> class Automaton {
202189
/// NFA taken by the DFA. NOTE: This is substantially more work than simply
203190
/// driving the DFA, so unless you require the getPaths() method leave this
204191
/// empty.
205-
Automaton(ArrayRef<TransitionT> Transitions,
192+
template <typename InfoT>
193+
Automaton(ArrayRef<InfoT> Transitions,
206194
ArrayRef<NfaStatePair> TranscriptionTable = {}) {
207195
if (!TranscriptionTable.empty())
208196
Transcriber =
209197
std::make_shared<internal::NfaTranscriber>(TranscriptionTable);
210198
Transcribe = Transcriber != nullptr;
211-
TransitionMap = Transitions;
212-
assert(std::is_sorted(TransitionMap.begin(), TransitionMap.end(),
213-
[](const TransitionT &A, const TransitionT &B) {
214-
return std::make_pair(A.FromDfaState, A.Action) <
215-
std::make_pair(B.FromDfaState, B.Action);
216-
}) &&
217-
"The transitions array is expected to be sorted by FromDfaState and "
218-
"Action");
199+
M = std::make_shared<MapTy>();
200+
for (const auto &I : Transitions)
201+
// Greedily read and cache the transition table.
202+
M->emplace(std::make_pair(I.FromDfaState, I.Action),
203+
std::make_pair(I.ToDfaState, I.InfoIdx));
219204
}
220205
Automaton(const Automaton &Other)
221-
: TransitionMap(Other.TransitionMap), State(Other.State),
222-
Transcribe(Other.Transcribe) {
206+
: M(Other.M), State(Other.State), Transcribe(Other.Transcribe) {
223207
// Transcriber is not thread-safe, so create a new instance on copy.
224208
if (Other.Transcriber)
225209
Transcriber = std::make_shared<internal::NfaTranscriber>(
@@ -249,22 +233,19 @@ template <typename ActionT> class Automaton {
249233
/// If this function returns false, all methods are undefined until reset() is
250234
/// called.
251235
bool add(const ActionT &A) {
252-
auto I = lower_bound(TransitionMap.begin(), TransitionMap.end(),
253-
std::make_pair(State, A), transitionLessThan);
254-
if (I == TransitionMap.end() || State != I->FromDfaState || A != I->Action)
236+
auto I = M->find({State, A});
237+
if (I == M->end())
255238
return false;
256239
if (Transcriber && Transcribe)
257-
Transcriber->transition(I->InfoIdx);
258-
State = I->ToDfaState;
240+
Transcriber->transition(I->second.second);
241+
State = I->second.first;
259242
return true;
260243
}
261244

262245
/// Return true if the automaton can be transitioned based on input symbol A.
263246
bool canAdd(const ActionT &A) {
264-
auto I = lower_bound(TransitionMap.begin(), TransitionMap.end(),
265-
std::make_pair(State, A), transitionLessThan);
266-
return I != TransitionMap.end() && State == I->FromDfaState &&
267-
A == I->Action;
247+
auto I = M->find({State, A});
248+
return I != M->end();
268249
}
269250

270251
/// Obtain a set of possible paths through the input nondeterministic

llvm/utils/TableGen/DFAEmitter.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,9 +131,15 @@ void DfaEmitter::emit(StringRef Name, raw_ostream &OS) {
131131
OS << "}};\n\n";
132132

133133
OS << "// A transition in the generated " << Name << " DFA.\n";
134-
OS << "using " << Name << "Transition = TransitionType<";
134+
OS << "struct " << Name << "Transition {\n";
135+
OS << " unsigned FromDfaState; // The transitioned-from DFA state.\n";
136+
OS << " ";
135137
printActionType(OS);
136-
OS << ">;\n\n";
138+
OS << " Action; // The input symbol that causes this transition.\n";
139+
OS << " unsigned ToDfaState; // The transitioned-to DFA state.\n";
140+
OS << " unsigned InfoIdx; // Start index into " << Name
141+
<< "TransitionInfo.\n";
142+
OS << "};\n\n";
137143

138144
OS << "// A table of DFA transitions, ordered by {FromDfaState, Action}.\n";
139145
OS << "// The initial state is 1, not zero.\n";

0 commit comments

Comments
 (0)