<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Nov 18, 2011, at 8:51 PM, Anshuman Dasgupta wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">
<meta http-equiv="content-type" content="text/html; charset=ISO-8859-1">
<div bgcolor="#FFFFFF" text="#000000"><tt>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.<br>
<br>
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.<br>
<br>
I would appreciate reviews and comments.<br></tt></div></blockquote><div><br></div><div>Hi Anshu,</div><div><br></div><div>Thanks for making this target-independent. It looks interesting.</div><div><br></div><div>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</div><div><br></div><div><div>+struct ltState</div><div>+{</div><div><br></div><div><div>+class DFA {</div><div>+ public:</div><div><br></div><div>Please make sure your braces and indentation matches the rest of LLVM. You also have a couple of 80-col violations.</div><div><br></div><div>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.</div><div><br></div><div><br></div><div><div>+#include <iostream></div><div><br></div><div>Please read <a href="http://llvm.org/docs/CodingStandards.html">http://llvm.org/docs/CodingStandards.html</a></div><div><br></div><div><br></div><div><div>+ // Set of transitions</div><div>+ std::set<Transition*> transitions;</div><div><br></div></div><div>Is this set used anywhere but here?:</div><div><br></div><div><div>+ // Add the new transition</div><div>+ transitions.insert(T);</div><div><br></div></div><div>If not, you can optimize that ;-)</div><div><br></div><div><br></div><div><div>+namespace llvm {</div><div>+ typedef std::pair<unsigned, unsigned> UnsignPair;</div><div><br></div></div><div>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.</div><div><br></div><div><br></div><div><div>+++ b/utils/TableGen/DFAPacketizerEmitter.h</div></div><div>+typedef unsigned Input;</div><div><br></div><div>Don't put this in the global namespace, or even in the llvm namespace.</div><div><br></div><div><br></div><div><div>+class State {</div></div><div><div>+struct Transition {</div></div><div><div>+class DFA {</div></div><div><br></div><div>Even for TableGen, these names are too generic to go in the llvm namespace.</div><div><br></div><div>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.</div><div><br></div><div>Thanks,</div><div>/jakob</div><div><br></div></div></div></div></div></body></html>