[PATCH] D73068: Reapply Avoid creating an immutable map in the Automaton class.

[Marcello Maggioni via Phabricator via llvm-commits]

kariddi created this revision.
kariddi added reviewers: jmolloy, ThomasRaoux.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

In the DFAPacketizer we copy the Transitions array
into a map in order to later access the transitions
based on a "Current State/Action" pair as a key.
This map lives in the Automaton object used by the DFAPacketizer.
It is never changed during the life of the object after
having been created during the creation of the Automaton
itself.

This map creation can make the creation of a DFAPacketizer
quite expensive if the target contains a considerable
amount of transition states.
This patch avoids creating the new map by using the TableGen
generated array as the map based on the assumption that this
array is already sorted in the DFAPacketizer by State/Action
pairs (the key of the map) and we can perform cheap log(N)
searches using std::lower_bound() directly on this array.

The previous patch in 051d330314cb1f175025ca37da8e5e1d851e1790 <https://reviews.llvm.org/rG051d330314cb1f175025ca37da8e5e1d851e1790>
used this approach for every possible user of the Automaton.
In this patch this approach is used only in the DFAPacketizer.
The problem was that, because the Action object can be customized,
there could be cases where the relative order of the actions
is different between the time TableGen generates the transitions
array and the time the Automaton is instantiated if a customized
action type is used.

An example is the "BinPackerAutomatonAccepts" that asserted on
an assert() added on the previous commit.
In this test the "BinRequirementKindEnum" is generated through
the "Searchable Tables" infrastructure.
When the transitions array is generated every element of the enum
is assigned a temporary value and the transitions array is ordered
based on that value. In the transition array the enum is left with
its textual name and not the underlying value (like BRK_0_to_4,
BRK_0_to_6, ... etc in this case).
The C++ code that is included by users of the enum is generated
subsequently by the "Searchable Tables" infrastructure and is at this
point that each element of the enum have their actual numerical value
assigned. With this new assignment the relative order between enum elements
could be different from the one temporarily determined during the transitions
array generation, causing the array to be effectively unordered now and
to fix that is necessary to actually create the map and we can't
perform the optimization.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73068

Files:
  llvm/include/llvm/CodeGen/DFAPacketizer.h
  llvm/include/llvm/Support/Automaton.h
  llvm/utils/TableGen/DFAEmitter.cpp
  llvm/utils/TableGen/DFAPacketizerEmitter.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D73068.239210.patch
Type: text/x-patch
Size: 10764 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200121/a3586803/attachment.bin>