[llvm-commits] Deterministic finite automaton based packetizer for VLIW architectures

[Jakob Stoklund Olesen]

On Nov 28, 2011, at 1:38 PM, Anshuman Dasgupta wrote:

> Sure, I've attached documentation on the functionality of the DFA. The document presents an overview of how the DFA is constructed and what it does. I will work on a patch to CodeGenerator.html that will describe the DFA generator but this document should let you review the code's functionality.

Thanks, that helped.

How big do these tables get for Hexagon? How many resources? How many DFA states?


+//=- llvm/CodeGen/DFAPacketizer.h - DFA Packetizer for VLIW ---*- C++ -*-=====//

This file needs more concrete, less abstract documentation.  The DFA concept is quite simple, just explain it.

This class definitely needs to use DenseMap instead of std::map.  How big is your state space?  Would it make sense to use a sparse transition matrix instead?

I also think you should move some of the methods to a .cpp file. If you can avoid the #includes, that would be great.

You are building CachedTable on demand. Is that really necessary? How much of the table is built in a typical run? Would it be better/faster to just build the whole thing up front?

You have 4 table lookups per DFA transition by my count. It seems that 1 is enough. Can the API be improved to allow that?


+++ b/utils/TableGen/DFAPacketizerEmitter.cpp
+bool State::canAddInsnClass(Input InsnClass, std::set<Input>& PossibleStates) {

Would a BitVector be better than set<unsigned>? You can still iterate.

+// Format:
+// DFAStateInputTable[] = pairs of <Input, Transition> for all valid
+//                        transitions
+// DFAStateEntryTable[i] = Index of the first entry in DFAStateInputTable for
+//                         the ith state

Could you get rid of the DFAStateEntryTable by renumbering your states?

  s -> DFAStateEntryTable[s]

This would preclude the sparse transition matrix representation, of course.


+      if (!isValidTransition(*SI, j)) {
+        continue;
+      }
+      else {

Don't use else after continue;

+  OS << "const unsigned int " << TargetName << "DFAStateEntryTable[] = {\n";
+  for (unsigned i = 0; i < states.size(); ++i) {
+    // Multiply i by 2 since each entry in DFAStateInputTable is a set of
+    // two numbers
+    OS << StateEntry[i] * 2 << ", ";
+  }

How about using "const unsigned DFAStateInputTable[][2]" then?

+      std::set<unsigned> NewStateResources;
+      //
+      // If we haven't already created a transition for this input
+      // and the state can accommodate this InsnClass, create a transition
+      //
+      if (!D.getTransition(current, InsnClass) &&
+          current->canAddInsnClass(InsnClass, NewStateResources)) {
+        State* NewState = NULL;
+
+        //
+        // If we have seen this state before, then do not create a new state
+        //
+        //
+        std::string key = "";
+        std::stringstream keyInt;
+        // We should not generate more than 16^8 states!
+        const int MaxSize = 8;
+        for (std::set<Input>::iterator KI = NewStateResources.begin(),
+               KE = NewStateResources.end(); KI != KE; ++KI) {
+          keyInt << std::setfill('0') << std::setw(MaxSize)
+                 << std::hex << *KI << std::dec;
+        }
+        key = keyInt.str();

This string conversion is a bit strange. I think this is actually a case where std::map<std::set<unsigned>, ...> makes sense. Other TableGen backends do that.

Would it be reasonable to limit the number of resources to 64, and just use a uint64_t bit mask instead?

The Visited map can be a local variable like WorkList, right?

/jakob

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20111128/6779116b/attachment.html>