I would recommend doing what was done for _xgetbv as in r351391: - Provide __builtin_ia32_rot* everywhere (or something like that in the implementor's namespace) - Conditionally #define _rot* to __builtin_ia32_rot* in ia32intrin.h, maybe ifdef _GNUC - Also provide _rot* builtins when -fms-extensions is enabled
That way, we aren't non-conforming out of the box before Intel intrinsic headers start getting included. On Thu, Mar 7, 2019 at 3:52 PM James Y Knight via cfe-commits < cfe-commits@lists.llvm.org> wrote: > This patch breaks some code which is (conditionally) defining functions of > these names on certain platforms. Now, it's true that it shouldn't be doing > that, and if the claim in the commit message about GCC was true, I'd just > say "don't do that". > > But, the commit message is wrong. GCC does _not_ define these as builtins, > it defines them in ia32intrin.h header (publicly via x86intrin.h) . > > IMO, this commit should be reverted, and functions defined in the header, > instead. Perhaps similar to what 2c8f9c2c23e0cafd7b85a7aec969c949349f747c > <https://github.com/llvm/llvm-project/commit/2c8f9c2c23e0cafd7b85a7aec969c949349f747c> > did, although I note that was reverted in > b62c5bc64dee3642884c31b8208c07a6f74b81fd > <https://github.com/llvm/llvm-project/commit/b62c5bc64dee3642884c31b8208c07a6f74b81fd>, > reportedly due to breaking mingw. > > (I'd further note that even MSVC doesn't enable these builtins by default! > It implements a `#pragma intrinsic(NAME)` mechanism which you need to use > in order to enable them (and #include <windows.h> will do so). But clang > doesn't really implement that pragma, and instead enables all the > MSVC-builtins unconditionally.) > > On Mon, Mar 4, 2019 at 1:46 PM Erich Keane via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: erichkeane >> Date: Mon Mar 4 10:47:21 2019 >> New Revision: 355322 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=355322&view=rev >> Log: >> Enable _rotl, _lrotl, _rotr, _lrotr on all platforms. >> >> The above builtins are currently implemented for MSVC mode, however GCC >> also implements these. This patch enables them for all platforms. >> >> Additionally, this corrects the type for these builtins to always be >> 'long int' to match the specification in the Intel Intrinsics Guide. >> >> Change-Id: Ida34be98078709584ef5136c8761783435ec02b1 >> >> Added: >> cfe/trunk/test/CodeGen/rot-intrinsics.c (with props) >> Modified: >> cfe/trunk/include/clang/Basic/Builtins.def >> cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c >> >> _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
