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

cblas.h header is unusable in its current state #13

Closed
kiwifb opened this issue Mar 24, 2011 · 8 comments
Closed

cblas.h header is unusable in its current state #13

kiwifb opened this issue Mar 24, 2011 · 8 comments
Labels

Comments

@kiwifb
Copy link
Contributor

kiwifb commented Mar 24, 2011

I tried to compile a few packages against openblas and all of those which use the headers provided by the cblas implementation rather than their own internal headers have failed.
The problem is that blasint is not defined in cblas.h - it comes form common.h. However unless you design a package from scratch it is not a header you will usually look for when compiling against cblas.

So, some final cblas.h headers should be produced that include a definition for blasint.

@kiwifb
Copy link
Contributor Author

kiwifb commented Mar 24, 2011

Furthermore the header uses size_t but doesn't include the stddef.h header. There should be a line:
#include <stddef.h>

@xianyi
Copy link
Collaborator

xianyi commented Mar 24, 2011

Closed by 5e7f29b. Fixed blasint undefined bug in <cblas.h> file.

@xianyi xianyi closed this as completed Mar 24, 2011
@xianyi
Copy link
Collaborator

xianyi commented Mar 24, 2011

Hi kiwifb,

Thank you for reporting this bug.

I fixed this on x86 branch
(https://github.com/xianyi/OpenBLAS/tree/x86). Could you verify it?

Thanks again

Xianyi Zhang

I tried to compile a few packages against openblas and all of those which use the headers provided by the cblas implementation rather than their own internal headers have failed.
The problem is that blasint is not defined in cblas.h - it comes form common.h. However unless you design a package from scratch it is not a header you will usually look for when compiling against cblas.

So, some final cblas.h headers should be produced that include a definition for blasint.

@kiwifb
Copy link
Contributor Author

kiwifb commented Mar 24, 2011

The stddef.h part should be fine but I don't like the inclusion of common.h. Ultimately you want to be able to package this for people to use. And in common.h the definition depends on the parameter USE64BITINT which could lead to some interesting situations.

I guess you don't have an "install" target yet so you are still considering this a "do it yourself" operation. Ideally for a final product, we want the definition of "blasint" to be inside cblas.h and not depend of USE64BITINT. I'll see if I can code this for you.

@xianyi
Copy link
Collaborator

xianyi commented Mar 25, 2011

Hi kiwifb,

2011/3/25, kiwifb
reply@reply.github.com:

The stddef.h part should be fine but I don't like the inclusion of common.h.
Ultimately you want to be able to package this for people to use. And in
common.h the definition depends on the parameter USE64BITINT which could
lead to some interesting situations.

There is INTERFACE64 flag in Makefile.rule. If you set this flag, it
would use 64 bits integer. In this situation, I write "#define
USE64BITINT" to config.h which is included by common.h.

I guess you don't have an "install" target yet so you are still considering
this a "do it yourself" operation. Ideally for a final product, we want the
definition of "blasint" to be inside cblas.h and not depend of USE64BITINT.
I'll see if I can code this for you.

Yes, I don't have an "install" target. I'm not familiar with writing
the "install" target in makefile. I need try some experiments. :)

Thanks

Reply to this email directly or view it on GitHub:
#13 (comment)

@kiwifb
Copy link
Contributor Author

kiwifb commented Mar 25, 2011

Thanks for pointing that header out. So one would need common.h and config.h.

From the shipping point of view it would be nice if the relevant part of these 2 files were merged or added to cblas.h. I am not against shipping extra headers but then they may need less generic names.

Thanks for answering to my comments.

@xianyi
Copy link
Collaborator

xianyi commented Mar 25, 2011

Thanks for pointing that header out. So one would need common'h and config.h.

From the shipping point of view it would be nice if the relevant part of these 2 files were merged or added to cblas.h. I am not against shipping extra headers but then they may need less generic names.
Yes, I agree with you. I will try it.
Thank you for the suggestions.
Thanks for answering to my comments.

@kiwifb
Copy link
Contributor Author

kiwifb commented Mar 25, 2011

I notice perl is needed to generate a number of files. It should be fairly easy to generate a cblas.h file from a template with a small perl script - or even just a file called openblas-config.h. While it is probably overblown, I am thinking of the way configuration files and Makefiles are generated in ntl.
http://shoup.net/ntl/

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

No branches or pull requests

2 participants