<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Hi Anshu,<br>
<br>
Thanks for reviewing this. I added a top comment for AddInsnClass
and I fixed the violation of column numbers.<br>
<br>
On 15/06/2012 21:31, Anshuman Dasgupta wrote:
<blockquote cite="mid:4FDB8D9C.9030505@codeaurora.org" type="cite">
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
<div class="moz-cite-prefix"><tt>Hi Ivan,<br>
<br>
The patch looks good to me. I have a couple of minor comments:<br>
<br>
+void State::AddInsnClass(unsigned InsnClass,<br>
Add a top level comment describing the function<br>
<br>
+ std::map<State*, std::set<Transition*,
ltTransition>, ltState> stateTransitions;<br>
You should be able to use SmallSet here. Also, this line
exceeds 80 columns.<br>
</tt></div>
</blockquote>
<br>
I tried but SmallSet is not iterable. SmallSetPtr could be useful
here but it doesn't allow custom sorting.<br>
<br>
<blockquote cite="mid:4FDB8D9C.9030505@codeaurora.org" type="cite">
<div class="moz-cite-prefix"><tt> <br>
<br>
On a related note, is the CachedTable mechanism in
DFAPacketizer.h useful for your architecture? Currently the
DFA generator generates one table for a given architecture. I
had originally added the CachedTable mechanism since for a
given compilation and subtarget, the DFA only uses the subset
of the states and transitions. Using a "cache" made sense. At
one point, I'd like to change the code so that it can generate
one DFA for every subtarget and remove the CachedTable
mechanism. Given the size of the DFA for your architecture,
however, it may make sense to keep it around even if it
generates separate tables for each subtarget.<br>
</tt></div>
</blockquote>
<br>
I liked the cachedtable idea but I can't tell you if it's really
useful in our case, I didn't make any performance tests in that
regard.<br>
IMO, it wil be nice to keep it alive for performance comparisons.
Given the overall performance is rather determined by transition
searches on the current state, for small DFA tables may not be a win
and it may still be the case for greater ones. We have a huge number
of states but the number of distinct itineraries, or maximum
possible transitions, remains quite small (11, it wasn't 13, my
mistake).<br>
<br>
Ivan<br>
<br>
<blockquote cite="mid:4FDB8D9C.9030505@codeaurora.org" type="cite">
<div class="moz-cite-prefix"><tt> <br>
Thanks<br>
-Anshu<br>
<br>
--- <br>
Qualcomm Innovation Center, Inc is a member of the Code Aurora
Forum <br>
<br>
</tt> <tt><br>
<br>
<br>
<br>
On 6/14/2012 8:22 AM, Ivan Llopard wrote:<br>
</tt></div>
<blockquote cite="mid:4FD9E59C.7010600@gmail.com" type="cite"><tt>Hi,
<br>
<br>
I've refactored the DFA generator in TableGen because it takes
too much time to build the table of our BE and I'd like to
share it. </tt><tt><br>
We have 15 functional units and 13 different itineraries
which, in the worst case, can produce 13! states. Fortunately,
many of those states are reused :-) but it still takes up to
11min to build the entire table. This patch reduces the build
time to 5min, giving a speed-up factor greater than 2. <br>
<br>
It contains small changes: </tt><tt><br>
- Transitions are stored in a set for quicker searches <br>
- canAddInsnClass() API is split in two API's: <br>
- canAddInsnClass() which perform a quick verification about
the possibility of having new states for a given InsnClass <br>
- AddInsnClass() performs the actual computation of possible
states. <br>
<br>
I've regenerated the DFA table of Hexagon and all seems to be
ok. </tt><tt><br>
<br>
What do you think about these changes ? </tt><tt><br>
<br>
<br>
Ivan </tt><tt><br>
<br>
</tt>
<fieldset class="mimeAttachmentHeader"></fieldset>
<tt><br>
</tt>
<pre wrap=""><tt>_______________________________________________
llvm-commits mailing list
<a moz-do-not-send="true" class="moz-txt-link-abbreviated" href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>
<a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a>
</tt></pre>
</blockquote>
<tt><br>
<br>
</tt> </blockquote>
</body>
</html>