r355322 - Enable _rotl, _lrotl, _rotr, _lrotr on all platforms.

[Reid Kleckner via cfe-commits]

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 at 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 at 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 at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190307/a93fc4c2/attachment.html>