-
Notifications
You must be signed in to change notification settings - Fork 9
Bunch of GPU detection work #44
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
Conversation
* Add myfnmatch.c/h for simple wildcard matching. Unix systems have
fnmatch() but Windows doesn't, so we provide a version from musl.
* Use fnmatch() to:
* Match GPU models, which is what you get on not-Linux. Should improve
detection on Windows and macOS.
* Match gfx-targets, which is what you get on Linux. Should cover more
AMD GPUs.
Also added matching for CDNA, which are chonkier versions of GCN5.
* Cover RDNA4 and RDNA3.5 as using RDNA3 features. Should help with primesearch#43
but definitely test before closing.
|
Your code was only compiling on Windows. I've fixed the macOS and Linux build errors. |
…ompilers and just causes them to print multiple warnings
08cabca to
9d9c0e6
Compare
… purposes and update musl home page
|
I don't see any issues with your changes. However, I'd like others to take a look at it as well if possible. In the meantime, I've improved the code formatting and INI file comments. Hope you don't mind. :-) |
|
These are all good changes! I am thinking about removing the mention of Rtl... whatever the glob thing on Windows is though, because with the use of brackets in the patterns it just won't work. I initially wanted to use simpler patterns (or at last some sort of two-stage matching where the complex character-sets are just replaced by |
|
I think the ideal solution would be to install external dependencies via a package manager. However, that's not really an option in this case because musl doesn't seem to be in the vcpkg repo. |
|
Wouldn't PathMatchSpecExA() work as a replacement for fnmatch() on Windows? |
|
The better replacement is the Rtl stuff in ntdll. The path one has, well, path-specific idiosyncrasies especially with 8.3 that we don't need. But even "better" isn't good enough as I've decided to be greedy and reach for the |
N-Storm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don’t see any obvious issues. I didn’t check the complex code in myfnmatch.c in detail, but it matches the upstream version from musl, taking into account the declared changes that remove unnecessary functionality for some flags.
What does concern me, though, is that the build with LTO on MinGW doesn’t work. It’s probably worth examining this further, but I don’t have enough spare time right now to look into it. So I’ll approve the PR for now, but this should be revisited later.
|
WAIT A SECOND fnmatch matches the whole string, so if anything is to be matched a bunch of * is needed woah great job me now I have a lot to explain |
Compiles on mingw-w64 (MSYS2 "Mingw64") with the following caveat: