<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Hi Renato, <div><br></div><div>Thanks for working on this. The code looks correct. I agree with Arnold that this function is becoming even more complicated and it would be nice to refactor it somehow, but we can do it later. I don't mind if the tests are in one file or multiple files. I assume that you ran the test-suite and found no failures. Did you see any newly vectorized loops ? </div><div><br></div><div>Thanks,</div><div>Nadav</div><div><br><div><div>On Feb 20, 2013, at 10:51 AM, Renato Golin <<a href="mailto:renato.golin@linaro.org">renato.golin@linaro.org</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div dir="ltr">On 20 February 2013 18:31, Arnold Schwaighofer <span dir="ltr"><<a href="mailto:aschwaighofer@apple.com" target="_blank">aschwaighofer@apple.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">This looks like unnecessary code duplication:<br></blockquote><div>> (...) </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Can you put this into a function?<br></blockquote><div><br></div><div style="">So, this one deserves a bit more of explanation. I haven't done it because that part of the code uses a typedef from inside the function. If I move it to a separate function I'd have to have the typedef external, which is not a problem per se (since it'll be local to a file), but some people consider it code pollution.</div>
<div style=""><br></div><div style="">If you guys don't mind, I thought about adding a few typedefs regarding the types and iterators, so I can use them by name and make the code simpler.</div><div><br></div><div><br></div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Can you move the test cases into one file? The more single file .ll tests we have the more we pay for file open/close operations. And we want make check to stay fast :).<br>
</blockquote><div><br></div><div style="">I had one case, but specifically moved to one per case, as my previous test Nadav asked to split into smaller tests (maybe I got it wrong). I actually prefer one big test if the items are closely related. If every one agrees, I'll merge them back.</div>
<div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im"><span style="color:rgb(34,34,34)">Am I missing something?</span></div></blockquote>
<div><br></div><div style="">The main thing is the iteration that goes from one to two nested loops, and I was already adding too many nested loops to the code. With that being in a separate function, it might not be a problem any more. I'll see if I can merge them.</div>
<div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im"><span style="color:rgb(34,34,34)">Punctuation.</span></div></blockquote><div>
<br></div><div style="">Hum, that's a weird thing, but I see from the developer's policy that all comments have full stop. I guess I'll just have to get used to. ;)</div><div style=""><br></div><div style="">cheers,</div>
<div style="">--renato </div></div></div></div>
</blockquote></div><br></div></body></html>