[PATCH] Global Structure Vectorization

Nadav Rotem nrotem at apple.com
Wed Feb 20 11:18:24 PST 2013


Hi Renato, 

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 ? 

Thanks,
Nadav

On Feb 20, 2013, at 10:51 AM, Renato Golin <renato.golin at linaro.org> wrote:

> On 20 February 2013 18:31, Arnold Schwaighofer <aschwaighofer at apple.com> wrote:
> This looks like unnecessary code duplication:
> > (...) 
> Can you put this into a function?
> 
> 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.
> 
> 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.
> 
> 
> 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 :).
> 
> 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.
> 
> 
> Am I missing something?
> 
> 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.
> 
> 
> Punctuation.
> 
> 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. ;)
> 
> cheers,
> --renato 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130220/c30735e2/attachment.html>


More information about the llvm-commits mailing list