<div dir="ltr"><div dir="ltr">I would recommend doing what was done for _xgetbv as in r351391:</div><div>- Provide __builtin_ia32_rot* everywhere (or something like that in the implementor's namespace)</div><div>- Conditionally #define _rot* to __builtin_ia32_rot* in ia32intrin.h, maybe ifdef _GNUC</div><div>- Also provide _rot* builtins when -fms-extensions is enabled</div><div><br></div><div>That way, we aren't non-conforming out of the box before Intel intrinsic headers start getting included.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Mar 7, 2019 at 3:52 PM James Y Knight via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div>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".</div><div><br></div><div>But, the commit message is wrong. GCC does _not_ define these as builtins, it defines them in ia32intrin.h header (publicly via x86intrin.h) .</div><div dir="ltr"><div><br></div><div>IMO, this commit should be reverted, and  functions defined in the header, instead. Perhaps similar to what <a href="https://github.com/llvm/llvm-project/commit/2c8f9c2c23e0cafd7b85a7aec969c949349f747c" target="_blank">2c8f9c2c23e0cafd7b85a7aec969c949349f747c</a> did, although I note that was reverted in <a href="https://github.com/llvm/llvm-project/commit/b62c5bc64dee3642884c31b8208c07a6f74b81fd" target="_blank">b62c5bc64dee3642884c31b8208c07a6f74b81fd</a>, reportedly due to breaking mingw.</div><div><br></div><div>(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.)</div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Mar 4, 2019 at 1:46 PM Erich Keane via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: erichkeane<br>
Date: Mon Mar  4 10:47:21 2019<br>
New Revision: 355322<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=355322&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=355322&view=rev</a><br>
Log:<br>
Enable _rotl, _lrotl, _rotr, _lrotr on all platforms.<br>
<br>
The above builtins are currently implemented for MSVC mode, however GCC<br>
also implements these.  This patch enables them for all platforms.<br>
<br>
Additionally, this corrects the type for these builtins to always be<br>
'long int' to match the specification in the Intel Intrinsics Guide.<br>
<br>
Change-Id: Ida34be98078709584ef5136c8761783435ec02b1<br>
<br>
Added:<br>
    cfe/trunk/test/CodeGen/rot-intrinsics.c   (with props)<br>
Modified:<br>
    cfe/trunk/include/clang/Basic/Builtins.def<br>
    cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c<br><br>
</blockquote></div>
</div></div></div></div></div>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>