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

[Jakob Stoklund Olesen]

On Nov 18, 2011, at 8:51 PM, Anshuman Dasgupta wrote:

> I'm attaching a patch that adds support for a deterministic finite automaton (DFA) based packetizer for VLIW architectures. Specifically, it automatically generates a DFA from a VLIW target's Schedule.td file. In a VLIW machine, an instruction can typically be dispatched to one or many function units. The DFA determines whether there exists a legal mapping of instructions to functional unit assignments in a packet. This DFA can then be       queried by a backend packetization pass to determine which instructions can be grouped into a VLIW packet.
> 
> This patch contains the machine-independent code that adds a component to TableGen. The component autogenerates the DFA and implements the API that can be used to query the DFA during packetization. This can be used by any VLIW target to packetize its instructions. After the Hexagon backend is committed, I will post another Hexagon-specific patch that uses this mechanism to construct packets in the Hexagon backend. I ran the llvm test suite and this patch did not cause any regressions.
> 
> I would appreciate reviews and comments.

Hi Anshu,

Thanks for making this target-independent. It looks interesting.

I was missing documentation in your patch. What does the DFA do? How is it generated? How can I use it? I think this deserves a full section in docs/CodeGenerator.html

+struct ltState
+{

+class DFA {
+ public:

Please make sure your braces and indentation matches the rest of LLVM. You also have a couple of 80-col violations.

You are using std::set and std::map a lot. These containers are quite expensive, and should only be used if you actually need features that the llvm/ADT classes don't offer. They do get used more in TableGen where performance is less critical, but still.


+#include <iostream>

Please read http://llvm.org/docs/CodingStandards.html


+  // Set of transitions
+  std::set<Transition*> transitions;

Is this set used anywhere but here?:

+  // Add the new transition
+  transitions.insert(T);

If not, you can optimize that ;-)


+namespace llvm {
+  typedef std::pair<unsigned, unsigned> UnsignPair;

Please don't put trivial typedefs like that in the llvm namespace. Either move the type into your class, or just spell it out in the header.


+++ b/utils/TableGen/DFAPacketizerEmitter.h
+typedef unsigned Input;

Don't put this in the global namespace, or even in the llvm namespace.


+class State {
+struct Transition {
+class DFA {

Even for TableGen, these names are too generic to go in the llvm namespace.

I would like to review the functionality as well, but that will be a lot easier if you submit a new patch with documentation first.

Thanks,
/jakob

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