<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix"><tt><br>
> </tt><tt>Thanks for reviewing this. I added a top comment
for AddInsnClass and I fixed the violation of column numbers.<br>
<br>
Great. Looks good to me.<br>
<br>
</tt><tt>> IMO, it wil be nice to keep it alive for performance
comparisons. Given the overall performance<br>
> is rather determined by transition searches on the current
state, for small DFA tables may not be a win<br>
> </tt><tt>and it may still be the case for greater ones.<br>
<br>
</tt><tt>I agree; let's keep it around for now.<br>
<br>
-Anshu<br>
<br>
</tt><tt>--- <br>
Qualcomm Innovation Center, Inc is a member of the Code Aurora
Forum<br>
<br>
</tt><tt><br>
<br>
On 6/18/2012 3:47 AM, Ivan Llopard wrote:<br>
</tt></div>
<blockquote cite="mid:4FDEEB1A.5050008@gmail.com" type="cite">
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
<tt> Hi Anshu,<br>
<br>
Thanks for reviewing this. I added a top comment for
AddInsnClass and I fixed the violation of column numbers. </tt><tt><br>
<br>
On 15/06/2012 21:31, Anshuman Dasgupta wrote: </tt>
<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>
<tt><br>
I tried but SmallSet is not iterable. SmallSetPtr could be
useful here but it doesn't allow custom sorting.<br>
<br>
</tt>
<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>
<tt><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 </tt><tt><br>
<br>
</tt>
<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>
</blockquote>
<tt><br>
<br>
</tt>
</body>
</html>