Hi, sorry for the delayed answer per mail.
2013/8/9 dw <limegreenso...@yahoo.com>: > So, no response here, other than a few (brief) comments on irc. > > It's hard to know how to advocate for a patch when I'm not sure what the > objections are. But it seems likely that that biggest areas of concern > aren't the changes to the inline asm, but the files I'm deleting from > \intrinsc\. Yes, I asked that on irc. IMHO it has some advantages to have some of those functions used by static-library instead of calling into a DLL. This might make a performance difference. We spare here one indirect jump ... and there is still the opportunity that things getting better optimized with LTO. > With that in mind, let me try to clarify why I'm proposing deleting these > files. > > RtlSecureZeroMemory.c - This function is NOT defined by MSDN as an > intrinsic. This means that it is NOT provided automatically by the compiler > as a builtin, and in fact is not available from any MS library or DLL. The > only place the function exists is as an inline in winnt.h. The same as what > mingw-w64 does. Sure, but we want to make sure that this function is present. So we want to provide it in libmingwex.a anyway, even if it is no intrinsic. The need to add it to libmingwex is that this function isn't present on all supported Windoof OSes, so we need to handle that. > UnsignedMultiplyExtract128.c & MultiplyExtract128.c: Likewise, these are > not intrinsics. Again, they are not available in libraries but only exist > in winnt.h, and mingw-64's winnt.h already has this. Same cause, see above. IMHO no need to remove them ... we might move those functions into a different folder in crt for making clear they are no intrinsics per definition. > _InterlockedAdd.c & _InterlockedAdd64.c: Perhaps the most interesting of the > 3, these *are* defined by MSDN as intrinsics. However, they are only > defined as intrinsics for Itanium processors. Does the inline assembler > code we have in the intrinsc directory even work on Itanium? No, IA64 isn't supported in our assembler. We lack IA64 hardware for testing ... > I have no > idea. But I do see that it is not protected with an #ifdef to limit it to > that platform. Further, MS defines a non-intrinsic version (but still > FORCEINLINE) of these functions in winnt.h, which mingw-w64 also provides. Yes, we should provide it IMHO. It might not fit into category of intrinsic on all platforms, nevertheless it fits more to this category then into another IMHO. > In general: > > - These deletes bring mingw-w64 in line with MS. That isn't necessarily the thing we want. I admit that in point of incompatiblity, we should think pretty hard about it, if we want to do that, but by default we have the goal of having a working environment for gcc (and other FOSS) compilers. > - Mingw-64 provides no means to access these functions from the library. This can be enforced by gcc's options to disable inlines and then those functions are there by the lib as fallback. > All the declarations provided are for the inlines in winnt. > - People who program to mingw-w64 and (somehow) use these functions from the > library will have problems if they try to move back to Windows where they > won't exist. Well, a fair point, but in terms of other differences we have not a really hard point IMHO. > I accept that there may be reasons not to do these deletes. And whether I > agree with the reasons or not, I do agree that someone needs to have a > vision for what mingw-w64 includes and what it doesn't, and that person > isn't me. So if these files must remain, just let me know, and I'll > re-submit the patch with the deletes deleted. > > dw > On 8/6/2013 4:23 PM, dw wrote: > > I think this is about it for intrinsics work for v3. This patch is (mostly) > for the files in intrinsc\*.c that weren't changed by any previous work. > It's possible that not everything in this patch will get approved, but I > figure it's easier to ask forgiveness than permission. > > __movsb, __movsd, __movsq, __movsw: Moved to intrin-impl. > __rdtsc: Change to use builtin, moved to intrin-impl, resolved conflict with > ia32intrin. > _umul128 & _mul128: Moved to intrin-impl. > __shiftright128 & __shiftleft128: Re-written as asm, moved to intrin-impl.h. > > _lrotr, _lrotl: Fix bug caused by ia32intrin.h when longs are 4 bytes long. > > RtlSecureZeroMemory - According to msdn, this is not an intrinsic and should > only be defined in winnt.h. File deleted from intrincs. > > UnsignedMultiplyExtract128 & MultiplyExtract128: According to msdn, these > are not intrinsics. Also, MultiplyExtract128 doesn't work right. Files > deleted from intrincs and code fixed in winnt.h. > > _InterlockedAdd & _InterlockedAdd64: According to msdn, these intrinsics are > only available for itanium. I'm not sure the inline asm we have will run > properly there, and there are no #if's around it to limit it to that > platform. Note that winnt.h has inlines for x86/x64 for these. Files > deleted from intrincs. > > dw > > > > ------------------------------------------------------------------------------ > Get 100% visibility into Java/.NET code with AppDynamics Lite! > It's a free troubleshooting tool designed for production. > Get down to code-level detail for bottlenecks, with <2% overhead. > Download for free and get started troubleshooting in minutes. > http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk > _______________________________________________ > Mingw-w64-public mailing list > Mingw-w64-public@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/mingw-w64-public > ------------------------------------------------------------------------------ Get 100% visibility into Java/.NET code with AppDynamics Lite! It's a free troubleshooting tool designed for production. Get down to code-level detail for bottlenecks, with <2% overhead. Download for free and get started troubleshooting in minutes. http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk _______________________________________________ Mingw-w64-public mailing list Mingw-w64-public@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
