On 28.01.2019 19:04, Alexandre Julliard wrote:
Konstantin Kharlamov hi-angel@yandex.ru writes:
On 28.01.2019 12:24, Alexandre Julliard wrote:
Konstantin Kharlamov Hi-Angel@yandex.ru writes:
The complex "#ifdef WINE_NO_TRACE_MSGS && __compiler__" ladder below reduces to "ignore WINE_NO_TRACE_MSGS when !__GNUC__ && !__SUNPRO_C", which is probably a bug.
No, it's on purpose, because some other compiler may not support varargs macros.
But then the workaround would result in incorrectly configured build. Is the complexity of the original code really worth the not even correct support of an obscure usecase?
WINE_NO_TRACE_MSGS is simply an optimization, there's not much harm if it doesn't work on some obscure compiler. It's better than breaking the build.
While on it, could you please advice on the naming in the 3-rd patch? Initially I've used "unlikely" because that's how it's used in Mesa. Later then I saw in wine lots of lower-case macros with __ prefix, so I changed it to "__unlikely". But then there are generic macros like "TRACE", and I think of renaming it to "UNLIKELY", which is 2 letters shorter, and is upper-case like yet another part of macros.
Considering that the benchmarks don't show an improvement, I don't think it's worth the trouble.
Wait, how so if it does? Besides, even without the benchmark it's obvious that the TRACE appear in so many places that there inevitably would be branch-misses.
In addition, this macro can be used for further optimization of performance critical parts of wine code.
And what you mean by "the trouble" — it's just a few lines of code that doesn't add any maintainance burden.
Also, adding macros that don't exist on Windows to standard headers is strongly discouraged.
Ah, I see, you mean that `windef.h` is a standard header from WinAPI. Okay, where then should I put it? windef.h just turned out to be one of 2 headers included by debug.h (the other being stdarg.h). I could put it directly into debug.h though.