<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman",serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
span.EmailStyle17
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri",sans-serif;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">You’re right, GCC has these in a header as a #define. This patch didn’t interfere with that and was causing some other failures for us as a result. If its OK,
I’d prefer to revert ONLY the enable on Linux mode at the moment, the long-int fix WAS valuable as well.<o:p></o:p></span></p>
<p class="MsoNormal"><a name="_MailEndCompose"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></a></p>
<p class="MsoNormal"><a name="_____replyseparator"></a><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">From:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"> James Y Knight [mailto:jyknight@google.com]
<br>
<b>Sent:</b> Thursday, March 7, 2019 3:52 PM<br>
<b>To:</b> Keane, Erich <erich.keane@intel.com><br>
<b>Cc:</b> cfe-commits <cfe-commits@lists.llvm.org><br>
<b>Subject:</b> Re: r355322 - Enable _rotl, _lrotl, _rotr, _lrotr on all platforms.<o:p></o:p></span></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<div>
<div>
<div>
<div>
<div>
<p class="MsoNormal">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".<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">But, the commit message is wrong. GCC does _not_ define these as builtins, it defines them in ia32intrin.h header (publicly via x86intrin.h) .<o:p></o:p></p>
</div>
<div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">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">
2c8f9c2c23e0cafd7b85a7aec969c949349f747c</a> did, although I note that was reverted in
<a href="https://github.com/llvm/llvm-project/commit/b62c5bc64dee3642884c31b8208c07a6f74b81fd">
b62c5bc64dee3642884c31b8208c07a6f74b81fd</a>, reportedly due to breaking mingw.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">(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.)<o:p></o:p></p>
</div>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">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:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<p class="MsoNormal" style="margin-bottom:12.0pt">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" 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<o:p></o:p></p>
</blockquote>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</body>
</html>