steinwurf / cpuid

C++ library for detecting CPU capabilities
https://cpuid.steinwurf.com
Other
105 stars 21 forks source link

Source file splitting #5

Closed nestorjhernandezm closed 10 years ago

nestorjhernandezm commented 10 years ago

Hi @mortenvp and @jpihl !

Here it's code spliting with the implementation for NEON. Please check it. I saw on the buildslaves that I worked on some ARM CPU's. I would like to take a look to the log files because must of them are test failures which means that each buildslave has to get its own argument parameters (as mentioned earlier).

Take a look and give your opinion. We are approaching to those X's in speed increment! :+1:

mortenvp commented 10 years ago

Somehow I "feel" that there is too much code in the implementation :) Maybe it can be done with less work. Also I don't like when the #ifdef are mixed into the source code since it makes the code harder to read.. You need to play the role of the preprocessor.

But I will need to think a bit about it - I might do some commits if ok with you?

mortenvp commented 10 years ago

I've been thinking about - I'm still not a big fan of the interspersed preprocessor and code.. It is very hard to verify mentally if the code does the right thing all the time.. but the header is ok - so that is something we can change later if needed.

nestorjhernandezm commented 10 years ago

@mortenvp ,

Made some other comments. I saw the code and I agree with you. Maybe we can split it more, actually, as we were originally aiming to. And, of course, you can do some commits :+1:

nestorjhernandezm commented 10 years ago

I made a review on your comments and agree with each of them. I have to review some stuff for Crossfire, but afterwards will make a brief "report" of this project on the mailing list. This will help us to visualize the roadmap here and to stablish what else is necessary to have it working properly.

I would like to take a look at the .log files from the buildslaves with someone to see what are we missing, correct everything and take a look at the intrinsics :+1: :+1: