The fix for bug 134113 has Windows using part of the unix gcc xptcall code. That's nice but I have bug 195736 which proposes using the regparm calling convention for internal functions. This is (perhaps small) performance win on Linux but I have no idea if it even works on Windows. I see that there was some problem with using xptcstubs, maybe the regparm stuff was part of it. In any event I propose keeping Windows separate, at least until there's more testing. Patch upcoming.
Created attachment 116961 [details] [diff] [review] patch Patches Makefile.in and creates xptcinvoke_gcc_x86_win32.cpp.
dbradley specifically asked that we reuse the xptcinvoke_gcc_x86 code. You'll have to take this up with him. How is the gcc_x86 code going to be reusable for other OSes (as alluded to in bug I'm too tired to lookup) if Linux specific bits get added?
I need to take a look at this, get it straight in my head, and see if I can come up with some decent alternatives. I just hate duplicating all that code.
Is this still an issue? Bug 195736 has been fixed, so does this still need to be separated?
Yes, I still think it's far better to separate this. You're basically trying to mix two completely different OS's together just because they share a compiler. I know I once said that was a good idea but not I think I was, at least partially, wrong. There are just too many differences. Also, I'm a believer in one of K&R's rubrics on good programming practice, namely that ifdefs should be avoided. As they point out, if you have N ifdefs you have 2^N different states so you have exponential complexity. That's a big maintenance problem. To support Windows, two ifdefs were introduced. I don't see how a factor of 4 in complexity is a win.
Conversely by separating we'll be duplicating a lot of code that someone will forget to keep in sync, that's also a big maintenance problem. If we were #ifdef'ing large sections of this file based on OS, then I'd agree. These are relatively small tweaks and the bulk of the code is constant. At best it's a wash IMO. Adding another file needs some kind of payoff, because that will add another state for the build system to deal with.