<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
Hi Anshu,<br>
<br>
Just in case you have forgotten this thread ;-). Is this patch ok to
commit or does it not apply to trunk properly ?<br>
I can fix it if that's the problem.<br>
<br>
Ivan<br>
<br>
On 20/06/2012 19:33, Anshuman Dasgupta wrote:
<blockquote cite="mid:4FE20979.1000105@codeaurora.org" type="cite">
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
<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> </blockquote>
</body>
</html>