Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

blasfeo windows support #119

Closed
wants to merge 3 commits into from

Conversation

TureganoJose
Copy link

I haven't tested completely. It solves some of the examples and I built a static library which I hooked up with hpipm and it seems to solve fine.

All I've done is switching to generic target, build the solution with CMake (tested with VS Express 2017 and compiler VC++ 2013) and then replaced all the void* with char*. I don't know if that would be an issue with gcc.

I hope it helps.

Regards,
Jose

@giaf
Copy link
Owner

giaf commented May 1, 2020

Thanks! It looks like gcc is not happy about replacing all void * with char *.

I guess the issue about visual studio and void * is the pointer arithmetic on void pointers.
I will try to make some changes only on that regard, and ping you when I'm done: I don't have access to a windows PC myself ATM, so it would be great if you could test if it still works on windows then.

@TureganoJose
Copy link
Author

Happy to test whatever you come up with. I'll try to have a look myself later today.

Cheers.

Thanks! It looks like gcc is not happy about replacing all void * with char *.

I guess the issue about visual studio and void * is the pointer arithmetic on void pointers.
I will try to make some changes only on that regard, and ping you when I'm done: I don't have access to a windows PC myself ATM, so it would be great if you could test if it still works on windows then.

… * from VS. Examples don't need to be linked to m.lib since MSVC deals with the maths
@giaf
Copy link
Owner

giaf commented May 2, 2020

Hi @TureganoJose at the end gcc was still not happy about the changes.

So I started from scratch and made all changes in your PR in a way that gcc likes.
The code is here https://github.com/giaf/blasfeo/tree/windows_support
Could you try to run this code on visual studion on windows and check if it works fine there?
Thanks!

@TureganoJose
Copy link
Author

Hi @TureganoJose at the end gcc was still not happy about the changes.

So I started from scratch and made all changes in your PR in a way that gcc likes.
The code is here https://github.com/giaf/blasfeo/tree/windows_support
Could you try to run this code on visual studion on windows and check if it works fine there?
Thanks!

Thanks for that @giaf. I've given it a go but it still complains about (void *). See image below.

image

@giaf
Copy link
Owner

giaf commented May 2, 2020

Hi @TureganoJose hopefully this will make the compiler happy now
9b6ea31

Can you give another try?
Thanks!

@TureganoJose
Copy link
Author

Hi @TureganoJose hopefully this will make the compiler happy now
9b6ea31

Can you give another try?
Thanks!

It does work now! See some instructions to build from CMake and to compile the examples.

image

image

C:/Program Files (x86)/Microsoft Visual Studio/2017/WDExpress/VC/Tools/MSVC/14.16.27023/bin/Hostx86/x64/cl.exe

image

CMake Error at CMakeLists.txt:199 (message):
MSVC compiler only supported for TARGET=GENERIC
You need to switch to GENERIC.

Maths are dealt by MSCVR in Windows, m.lib needs to be removed from all the examples. This step is not necessary to build blasfeo library.

image

And now it works:
image

@giaf
Copy link
Owner

giaf commented May 3, 2020

Hi @TureganoJose great that it works :)
And this should fix the issue with the math library in the examples
87c2c05

Now I will merge my branch on windows support into master, close this PR and create an issue about window support pointing to this PR.
Thanks for the help!

Since only the GENERIC target works with visual studio, the next step to improve support would be to check if the compiler can properly auto-vectorize the GENERIC code, and if not tweak it accordingly.
If you are interested in checking the performance, I can ping you once I put some work in this.

@giaf giaf mentioned this pull request May 3, 2020
@giaf giaf closed this May 3, 2020
@TureganoJose
Copy link
Author

Hi @TureganoJose great that it works :)
And this should fix the issue with the math library in the examples
87c2c05

Now I will merge my branch on windows support into master, close this PR and create an issue about window support pointing to this PR.
Thanks for the help!

Since only the GENERIC target works with visual studio, the next step to improve support would be to check if the compiler can properly auto-vectorize the GENERIC code, and if not tweak it accordingly.
If you are interested in checking the performance, I can ping you once I put some work in this.
Yes @giaf , happy to help. Let me know when the vectorization is ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants