<div dir="ltr">Apart from Renato's points, I have two more issues,<br><div><br><div>1) Can you combine those two statements,</div></div><div><br></div><div><div>+  for (unsigned i = At; i != 0;) {</div><div>+    --i;</div>

</div><div><br></div><div>to be</div><div><br></div><div>for (unsigned i = At; i != 0; --i) {<br></div><div><br></div><div>otherwise, I thought there might be other special changes to i in the loop.</div>
<div><br></div><div>2) It seems this piece of code change is unnecessary, because we needn't to clean anything for a new chain, right? </div><div><br></div><div><div>@@ -1265,8 +1329,11 @@</div>
<div>         CurrSize = Size;</div><div>         CurrPred = Pred;</div><div>         CurrPredReg = PredReg;</div><div>+</div><div>+        unsigned Cleaned = CleanPrevLoadOpsInto(Reg, MemOps);</div><div>+</div><div>         MemOps.push_back(MemOpQueueEntry(Offset, Reg, isKill, Position, MBBI));</div>

<div>-        ++NumMemOps;</div><div>+        NumMemOps += 1 - Cleaned;</div><div>         Advance = true;</div><div>       } else {</div><div><br></div></div><div>Thanks,</div><div>-Jiangning</div></div><div class="gmail_extra">
<br><br><div class="gmail_quote">2013/3/27 Stepan Dyatkovskiy <span dir="ltr"><<a href="mailto:stpworld@narod.ru" target="_blank">stpworld@narod.ru</a>></span><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Oh.. sorry, I've applied outdated patch, since I totally refactored it in Monday. Reattached.<br>
Relative to getElementaries... So you propose just to keep the list of unit registers currently used? right?  I try to fix it today.<br>
-Stepan.<br>
Renato Golin wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
On 25 March 2013 16:20, Stepan Dyatkovskiy <<a href="mailto:stpworld@narod.ru" target="_blank">stpworld@narod.ru</a><br></div><div><div class="h5">
<mailto:<a href="mailto:stpworld@narod.ru" target="_blank">stpworld@narod.ru</a>>> wrote:<br>
<br>
    Hello Jiangning,<br>
    The main reason I did it, is that we need to analyze contents of<br>
    MemOps *each time* we modify it: we need to prevent more than one<br>
    "load" operations into the same place.<br>
<br>
<br>
Hi Stepan,<br>
<br>
I like the idea of inspecting it on every load, but the current patch is<br>
very inefficient.<br>
<br>
On:<br>
<br>
+    SmallVector<unsigned, 4> getElementaries(unsigned Reg) {<br>
<br>
You create the small vector on *every* call. This is really bad. And<br>
then you iterate and compare every register of the new operations with<br>
every register of every other operation so far. That's far from optimal.<br>
This is why STL sorts and creates new sets, etc.<br>
<br>
If I get correctly you don't need a list of memory ops and their<br>
respective registers, but the opposite, since you're looking for<br>
registers, not memory ops.<br>
<br>
So, instead of keeping a list of operations and querying their registers<br>
*every single time*, you can keep a list of registers "in use" with a<br>
pointer to a small vector with the memory operations that use that register.<br>
<br>
That will give you near constant time query over all registers in use<br>
and you'll only have to gather the registers (via getElementaries) once<br>
per operation.<br>
<br>
The rest looks ok, the encapsulation of MemOps into MemOpsTrckr seems to<br>
work (wrt push_back/insert).<br>
<br>
Would also be good to have the IR in the PR bug as a test, just to make<br>
sure the objective was met and to have a history on what this patch is<br>
fixing.<br>
<br>
Thanks,<br>
--renato<br>
</div></div></blockquote>
<br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br>Thanks,<div>-Jiangning</div>
</div>