Missing support for clang builds under Windows MSVC #629
Comments
|
Peter, Does Hercules run faster when compiled with Clang? I don't think I applied your above changes to try out, and get this error pretty quickly: Bill |
|
Hi Bill, Thanks for your feedback ! The choice of You were right in that the Windows 11 VS 2022, which is already at CLANG version 16.0.5 or so (whereas my Windows 10 VS 2019 is at CLANG version 12.0.x) indeed needs one more correction to two files, changing --- SDL-hyperion/w32util.c 2024-02-19 11:03:38.003664596 +0100
+++ w32util.c 2024-02-20 17:32:33.061944693 +0100
@@ -2216,7 +2216,7 @@
// at a time. This may perhaps be slightly inefficient but it makes for a simpler
// implementation and besides, we don't really expect huge gobs of data coming in.
-static DWORD WINAPI ReadStdInThread( LPVOID lpParameter )
+static unsigned WINAPI ReadStdInThread( LPVOID lpParameter )
{
DWORD dwBytesRead = 0;
@@ -4696,12 +4696,12 @@
//////////////////////////////////////////////////////////////////////////////////////////
-DLL_EXPORT int w32_mlock( void* addr, size_t len )
+DLL_EXPORT int w32_mlock( const void* addr, size_t len )
{
return VirtualLock( addr, len ) ? 0 : -1;
}
-DLL_EXPORT int w32_munlock( void* addr, size_t len )
+DLL_EXPORT int w32_munlock( const void* addr, size_t len )
{
return VirtualUnlock( addr, len ) ? 0 : -1;
}
--- SDL-hyperion/fthreads.c 2024-02-19 11:03:37.563664125 +0100
+++ fthreads.c 2024-02-20 17:36:18.867419892 +0100
@@ -787,7 +787,7 @@
//----------------------------------------------------------------------------------
-static DWORD __stdcall FTWin32ThreadFunc
+static unsigned __stdcall FTWin32ThreadFunc
(
void* pMyArgs
)Please let me know how this would work for you. As to your question as to whether the CLANG built SDL-hyperion is faster, I got the feeling that it was a wee bit faster, but must admit I did not make measurements, so that feeling is not very scientific. Cheers, Peter |
|
Peter, This was done on Windows 10, VS2022 17.9.0, with this version of Clang: I applied your other edits, and it builds OK! Clang sure is a lot slower building than with MSVC! A lot of the time it wasted in Windows Defender, scanning god knows what. So I added an exclusion rule for the build directory. That helps, but Windows Antimalware still pops up a lot in Task Man. TortoiseGit status cache burns up a lot of time, too. You can always count on Windows to be very annoying. Running the resulting Hercules, with my canned MVS SYSGEN JCL, it's approximately the same elapsed time as with MSVC. Bill |
|
Thanks for the positive feedback Bill. Commit 5c33c1f closes this issue. Cheers, Peter |
|
When not using Clang, it doesn't build. Bill |
|
With Clang I'm seeing some tests fail: Also I'm seeing these tests fail: Bill |
I'm seeing the same thing with VS2008 too. I presume you're using VS2022, Bill? I'm about to try it myself in my VMware virtual machine. What I don't understand is why Yes, the Linux I don't think I am re-opening this issue until this problem can be resolved. |
|
Apologies for this problem I created, which should now be fixed, at least concerning the non-Clang build on MSVC. That I could make this fix was because in the more distant past, I had successfully tested with "const" removed from the first parameter on both the w32_mlock and w32_munlock in "w32util.h", bringing it in line with "w32util.c". Then later on I thought it more appropriate to do the opposite, that is to add the "const" in the "w32util.c", which I admittedly did not test on non-Clang MSVC builds. My bad. I've now removed "const" in all these, so that it matches again, which is what Clang (MSVC) was complaining about. As to the failing tests, those I did not experience, at least not on Ubuntu. I'll investigate. Cheers, Peter |
Fish-Git
added
Enhancement
Fish, yes, this was with VS2022 (17.9.0). Bill |
Unfortunately, I can't go beyond VS2022 (17.6.2), as VS2022 (17.9.0) refuses to install due to my still running Windows 7. In any case, it looks like Peter has fixed the problem. Thank you, Peter! ONE QUESTION THOUGH: In your comments in My question is, if a person sets Just wondering. |
OK, well, now I can't seem to build ANYTHING! That stupid problem with MSVC manifest tool (or linker) that complains the files are being used by another process has popped up. MSFT's had this problem on and off for like 15 years! I disabled Windows Defender as much as I could and it still doesn't want to work. They used to happen to me in the past, too, when the system was too fast. Try it over and over and finally it'll work. So sick and tired of Windows! |
|
Hi Bill and Fish, I've successfully completed my tests on Ubuntu (20.04 and 22.04), Windows 10 VS2019, and Windows 11 VS2022, including all tests as shown at the bottom. Fish had a question:
The Cheers, Peter P.S.: The tests: |
I have a similar problem with my antivirus too, but thankfully it doesn't cause the build to fail. It just delays it with a zillion popup dialogs asking me to respond whether to allow the action or not, causing the build to take forever as a result of the build waiting for me to respond to each one. Also thankfully, I can simply "pause" my antivirus during Hercules builds too, so that they finish in their normal amount of time without any of annoying popups occurring. I then "unpause" it afterwards of course. I've had customers in the distant pass have similar problems with ZoneAlarm that intercepted I/Os and ended up changing the I/O's actual parameters, thereby leading to unexpected failures. I find it amazing (but not all that surprising, sadly) that even Microsoft can't seem to get things right with their own damn operating system. Sad, that. And of course, you were involved with me with that customer issue where he couldn't get Hercules to run on his Windows 11 system.
Me too! But only with the versions they released after Windows 7 (e.g. 10 & 11). Windows 7 has been rock solid for me for many years and I have no intention of ever upgrading to 10 or 11. Annoyingly (but understandably) however, more and more vendors are dropping support for Windows 7 (most notably but understandably Microsoft products such as VS2022, but there are others now besides them), going so far as to refuse to even allow installing their product on Windows 7. For no real apparent reason. Not because it won't (or can't) run on Windows 7, but only because they purposely don't want it to be run on Windows 7. It's pissing me off. |
Ah! I think I understand now. It's the
Yep. That's what I'm seeing too. Works perfectly. I don't know what's going on with Bill. |
|
I'm closing this issue again. Bill? We can still work on your issue(s) here in this GH issue if you want, OR we can work on it offline. Either way is fine with me. |
Fish-Git
removed
the
QUESTION...
What version of clang-cl are you using? Mine says version 17.0.3: The test failures continue: I guess I could try it on Windows 11, but I doubt that would affect it. Bill |
|
Mine too says 17.0.3 on my Windows 11 23H2 VS2022 v. 17.9.1. I'll also (re-)check it on my Windows 10 22H2 VS2019 system later on. This time I tested on a laptop which had ooRexx 5.0.0 installed which caused a message ("Error 26") following the "runtest" command, but didn't hurt otherwise: Cheers, Peter |
The above is being caused by a known shortcoming in the Did 241 tests. All OK. Duration: 1 minute, 36 seconds End: ** Success! ** Wow. 1:36 ? Isn't that a little on the slow side? |
That takes about 30 seconds here. |
34 for me. |
|
Yes, 1:36, and it is reproducible, very disappointing. I suspect either a Windows processor P vs. E cores scheduling bug, or a Lenovo BIOS bug on my brand-new X1 Yoga gen 8. Or both. /* Expletives censored. */ I have a simple solution though for the date / time format problem in "runtest.cmd". First I create a Rexx one-liner called "datetime.rexx": Which I then subsequently use to replace the "for /f" loops with a single one : --- /mnt/c/Users/Peter/runtest.cmd 2024-02-23 13:55:50.335275600 +0100
+++ /mnt/c/Users/Peter/SDL-hyperion/tests/runtest.cmd 2024-02-24 12:02:24.046402000 +0100
@@ -203,11 +203,13 @@
:: wheras German for example uses "DD.MM.YY" and "HH:MM:SS,nn".
::-------------------------------------------------------------------
- for /f "delims=/. tokens=1-3" %%a in ("%date:~4%") do (
- for /f "delims=:., tokens=1-4" %%d in ("%time: =0%") do (
- set "@=TMP%%c%%a%%b%%d%%e%%f%%g%random%%file_ext%"
- )
- )
+ :: for /f "delims=/. tokens=1-3" %%a in ("%date:~4%") do (
+ :: for /f "delims=:., tokens=1-4" %%d in ("%time: =0%") do (
+ :: set "@=TMP%%c%%a%%b%%d%%e%%f%%g%random%%file_ext%"
+ :: )
+ :: )
+
+ for /f "tokens=1" %%d in ('rexx datetime.rexx') do set @=TMP%%d%random%%file_ext%
endlocal && set "%var_name%=%@%"
%return%This works, but of course no speedups: Perhaps "runtest.cmd" and "runtest" could be replaced by a single common "runtest.rexx", but that would be for another day -- if at all. Cheers, Peter |
Since you say it's brand new, could this be the cause? Notice the posting made on 2021-03-11 08:26:07: "Strangely, when I shut off all Lenovo service, It might just have a bunch of unwanted crap on it that you don't need? |
|
I can't imagine how much bloatware a pre-loaded Windows 10 or 11 probably comes with. Also, try running TaskMan while you're doing a build or tests, and see what's the top CPU consumer. Also, the Bill |
We're hotrodders. |
That's a TWO liner, not a one liner. Where did you create this "datetime.rexx"? In Hercules's BUT..., if I make a slight change: Then it works fine! |
Compared to Peter, yeah! |
|
Another thought: could it MAYBE be that clang builds are not optimized?? I don't know how Microsoft designed (hacked) their I'm just guessing/speculating here! I haven't actually tried this new clang-cl thingy for myself yet. What level of VS2022 is required? The latest? Because I can't run the latest. |
|
My times are the same, give or take, with MSVC and Clang.
|
|
FYI: I'm thinking we should probably make Peter's (@Peter-J-Jansen) suggested date/time fix for our And yes, I also think we should probably eventually replace our existing Let's get this date/time problem fixed first. Should I commit Peter's suggested change? Yea? Nay? |
Sure |
Done. |
|
Thanks Fish, following up to some older questions :
Yes, as where it is now. I'm not sure why I didn't need the
Sorry it took so long, but I finally was able to re-test it, and all is still fine. Cheers, Peter |
Currently support for CLANG builds under Windows MSVC is missing, but would be easy to add. If environment variable CL is set then the "cc" command can be replaced with "clang-cl" to perform clang builds under Windows MSVC. Two files would need to be changed to allow such CLANG builds.
Without adding the
SET "CL=-mcx16 /w"nothing changes. The reason for the additional-DHAVE_SOCKLEN_Toption is to avoid this error :A second file needs to be changed (actually corrected) as well :
This last correction is because MSVC Windows' CLANG v.12 will otherwise complain about this error :
This is what would be required for CLANG builds under MSVC Windows to be supported. I have been testing quite extensively this MSVC Windows CLANG builds and they work perfectly well for me. Shall I go ahead and commit these improvements please ?
Thanks,
Peter
The text was updated successfully, but these errors were encountered: