<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:11.0pt;
font-family:"Calibri",sans-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;}
p.msonormal0, li.msonormal0, div.msonormal0
{mso-style-name:msonormal;
mso-margin-top-alt:auto;
margin-right:0in;
mso-margin-bottom-alt:auto;
margin-left:0in;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
span.EmailStyle18
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:windowtext;}
.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">Right, the intent was to get this to match Microsoft’s behavior better while still making sense on other platforms. IIRC this was in order to fix a codegen bug due to type mismatches, though I don’t completely remember (as it was nearly
a year ago).<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><b>From:</b> James Y Knight <jyknight@google.com> <br>
<b>Sent:</b> Wednesday, February 26, 2020 8:39 PM<br>
<b>To:</b> Michael Spencer <bigcheesegs@gmail.com><br>
<b>Cc:</b> Keane, Erich <erich.keane@intel.com>; cfe-commits <cfe-commits@lists.llvm.org><br>
<b>Subject:</b> Re: r355698 - Re-fix _lrotl/_lrotr to always take Long, no matter the platform.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">I'm not clear as to what you're saying is broken.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">On MS platforms, long is 32-bit, so either way this function takes a 32bit value, right? And, Microsoft's docs say this function takes a long.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">On Wed, Feb 26, 2020, 5:09 PM Michael Spencer 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">
<div>
<div>
<p class="MsoNormal">I believe this commit is wrong. These intrinsics were originally implemented to support Windows code which expects _lrot{l,r}'s first argument to be 32 bits, it's a breaking change to change these definitions. I'm fine with adding the Intel
version of these intrinsics, but you can't just break the Microsoft version of them.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><br clear="all">
<o:p></o:p></p>
<div>
<div>
<p class="MsoNormal">- Michael Spencer<o:p></o:p></p>
</div>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">On Fri, Mar 8, 2019 at 7:09 AM 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">Author: erichkeane<br>
Date: Fri Mar 8 07:10:07 2019<br>
New Revision: 355698<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=355698&view=rev" target="_blank">
http://llvm.org/viewvc/llvm-project?rev=355698&view=rev</a><br>
Log:<br>
Re-fix _lrotl/_lrotr to always take Long, no matter the platform.<br>
<br>
r355322 fixed this, however is being reverted due to concerns with<br>
enabling it in other modes.<br>
<br>
Change-Id: I6a939b7469b8fa196d5871a627eb2330dbd30f29<br>
<br>
Modified:<br>
cfe/trunk/include/clang/Basic/Builtins.def<br>
cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c<br>
<br>
Modified: cfe/trunk/include/clang/Basic/Builtins.def<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=355698&r1=355697&r2=355698&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=355698&r1=355697&r2=355698&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Basic/Builtins.def (original)<br>
+++ cfe/trunk/include/clang/Basic/Builtins.def Fri Mar 8 07:10:07 2019<br>
@@ -831,12 +831,12 @@ LANGBUILTIN(_ReturnAddress, "v*", "n", A<br>
LANGBUILTIN(_rotl8, "UcUcUc", "n", ALL_MS_LANGUAGES)<br>
LANGBUILTIN(_rotl16, "UsUsUc", "n", ALL_MS_LANGUAGES)<br>
LANGBUILTIN(_rotl, "UiUii", "n", ALL_MS_LANGUAGES)<br>
-LANGBUILTIN(_lrotl, "UNiUNii", "n", ALL_MS_LANGUAGES)<br>
+LANGBUILTIN(_lrotl, "ULiULii", "n", ALL_MS_LANGUAGES)<br>
LANGBUILTIN(_rotl64, "UWiUWii", "n", ALL_MS_LANGUAGES)<br>
LANGBUILTIN(_rotr8, "UcUcUc", "n", ALL_MS_LANGUAGES)<br>
LANGBUILTIN(_rotr16, "UsUsUc", "n", ALL_MS_LANGUAGES)<br>
LANGBUILTIN(_rotr, "UiUii", "n", ALL_MS_LANGUAGES)<br>
-LANGBUILTIN(_lrotr, "UNiUNii", "n", ALL_MS_LANGUAGES)<br>
+LANGBUILTIN(_lrotr, "ULiULii", "n", ALL_MS_LANGUAGES)<br>
LANGBUILTIN(_rotr64, "UWiUWii", "n", ALL_MS_LANGUAGES)<br>
LANGBUILTIN(__va_start, "vc**.", "nt", ALL_MS_LANGUAGES)<br>
LANGBUILTIN(__fastfail, "vUi", "nr", ALL_MS_LANGUAGES)<br>
<br>
Modified: cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c?rev=355698&r1=355697&r2=355698&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c?rev=355698&r1=355697&r2=355698&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c (original)<br>
+++ cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c Fri Mar 8 07:10:07 2019<br>
@@ -12,17 +12,10 @@<br>
// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG<br>
// RUN: %clang_cc1 -ffreestanding -fms-extensions -fms-compatibility -fms-compatibility-version=17.00 \<br>
// RUN: -triple x86_64--linux -emit-llvm %s -o - \<br>
-// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG<br>
+// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-64BIT-LONG<br>
// RUN: %clang_cc1 -ffreestanding -fms-extensions \<br>
// RUN: -triple x86_64--darwin -emit-llvm %s -o - \<br>
-// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG<br>
-<br>
-// LP64 targets use 'long' as 'int' for MS intrinsics (-fms-extensions)<br>
-#ifdef __LP64__<br>
-#define LONG int<br>
-#else<br>
-#define LONG long<br>
-#endif<br>
+// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-64BIT-LONG<br>
<br>
// rotate left<br>
<br>
@@ -47,12 +40,15 @@ unsigned int test_rotl(unsigned int valu<br>
// CHECK: [[R:%.*]] = call i32 @llvm.fshl.i32(i32 [[X:%.*]], i32 [[X]], i32 [[Y:%.*]])<br>
// CHECK: ret i32 [[R]]<br>
<br>
-unsigned LONG test_lrotl(unsigned LONG value, int shift) {<br>
+unsigned long test_lrotl(unsigned long value, int shift) {<br>
return _lrotl(value, shift);<br>
}<br>
// CHECK-32BIT-LONG: i32 @test_lrotl<br>
// CHECK-32BIT-LONG: [[R:%.*]] = call i32 @llvm.fshl.i32(i32 [[X:%.*]], i32 [[X]], i32 [[Y:%.*]])<br>
// CHECK-32BIT-LONG: ret i32 [[R]]<br>
+// CHECK-64BIT-LONG: i64 @test_lrotl<br>
+// CHECK-64BIT-LONG: [[R:%.*]] = call i64 @llvm.fshl.i64(i64 [[X:%.*]], i64 [[X]], i64 [[Y:%.*]])<br>
+// CHECK-64BIT-LONG: ret i64 [[R]]<br>
<br>
unsigned __int64 test_rotl64(unsigned __int64 value, int shift) {<br>
return _rotl64(value, shift);<br>
@@ -84,12 +80,15 @@ unsigned int test_rotr(unsigned int valu<br>
// CHECK: [[R:%.*]] = call i32 @llvm.fshr.i32(i32 [[X:%.*]], i32 [[X]], i32 [[Y:%.*]])<br>
// CHECK: ret i32 [[R]]<br>
<br>
-unsigned LONG test_lrotr(unsigned LONG value, int shift) {<br>
+unsigned long test_lrotr(unsigned long value, int shift) {<br>
return _lrotr(value, shift);<br>
}<br>
// CHECK-32BIT-LONG: i32 @test_lrotr<br>
// CHECK-32BIT-LONG: [[R:%.*]] = call i32 @llvm.fshr.i32(i32 [[X:%.*]], i32 [[X]], i32 [[Y:%.*]])<br>
// CHECK-32BIT-LONG: ret i32 [[R]]<br>
+// CHECK-64BIT-LONG: i64 @test_lrotr<br>
+// CHECK-64BIT-LONG: [[R:%.*]] = call i64 @llvm.fshr.i64(i64 [[X:%.*]], i64 [[X]], i64 [[Y:%.*]])<br>
+// CHECK-64BIT-LONG: ret i64 [[R]]<br>
<br>
unsigned __int64 test_rotr64(unsigned __int64 value, int shift) {<br>
return _rotr64(value, shift);<br>
<br>
<br>
_______________________________________________<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" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><o:p></o:p></p>
</blockquote>
</div>
</div>
<p class="MsoNormal">_______________________________________________<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" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><o:p></o:p></p>
</blockquote>
</div>
</div>
</div>
</div>
</body>
</html>