r355698 - Re-fix _lrotl/_lrotr to always take Long, no matter the platform.

James Y Knight via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 26 20:38:56 PST 2020


I'm not clear as to what you're saying is broken.

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.


On Wed, Feb 26, 2020, 5:09 PM Michael Spencer via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> 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.
>
> - Michael Spencer
>
> On Fri, Mar 8, 2019 at 7:09 AM Erich Keane via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> Author: erichkeane
>> Date: Fri Mar  8 07:10:07 2019
>> New Revision: 355698
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=355698&view=rev
>> Log:
>> Re-fix _lrotl/_lrotr to always take Long, no matter the platform.
>>
>> r355322 fixed this, however is being reverted due to concerns with
>> enabling it in other modes.
>>
>> Change-Id: I6a939b7469b8fa196d5871a627eb2330dbd30f29
>>
>> Modified:
>>     cfe/trunk/include/clang/Basic/Builtins.def
>>     cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c
>>
>> Modified: cfe/trunk/include/clang/Basic/Builtins.def
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=355698&r1=355697&r2=355698&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/include/clang/Basic/Builtins.def (original)
>> +++ cfe/trunk/include/clang/Basic/Builtins.def Fri Mar  8 07:10:07 2019
>> @@ -831,12 +831,12 @@ LANGBUILTIN(_ReturnAddress, "v*", "n", A
>>  LANGBUILTIN(_rotl8,  "UcUcUc",    "n", ALL_MS_LANGUAGES)
>>  LANGBUILTIN(_rotl16, "UsUsUc",    "n", ALL_MS_LANGUAGES)
>>  LANGBUILTIN(_rotl,   "UiUii",     "n", ALL_MS_LANGUAGES)
>> -LANGBUILTIN(_lrotl,  "UNiUNii",   "n", ALL_MS_LANGUAGES)
>> +LANGBUILTIN(_lrotl,  "ULiULii",   "n", ALL_MS_LANGUAGES)
>>  LANGBUILTIN(_rotl64, "UWiUWii",   "n", ALL_MS_LANGUAGES)
>>  LANGBUILTIN(_rotr8,  "UcUcUc",    "n", ALL_MS_LANGUAGES)
>>  LANGBUILTIN(_rotr16, "UsUsUc",    "n", ALL_MS_LANGUAGES)
>>  LANGBUILTIN(_rotr,   "UiUii",     "n", ALL_MS_LANGUAGES)
>> -LANGBUILTIN(_lrotr,  "UNiUNii",   "n", ALL_MS_LANGUAGES)
>> +LANGBUILTIN(_lrotr,  "ULiULii",   "n", ALL_MS_LANGUAGES)
>>  LANGBUILTIN(_rotr64, "UWiUWii",   "n", ALL_MS_LANGUAGES)
>>  LANGBUILTIN(__va_start,       "vc**.", "nt", ALL_MS_LANGUAGES)
>>  LANGBUILTIN(__fastfail, "vUi",    "nr", ALL_MS_LANGUAGES)
>>
>> Modified: cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c?rev=355698&r1=355697&r2=355698&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c (original)
>> +++ cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c Fri Mar  8 07:10:07
>> 2019
>> @@ -12,17 +12,10 @@
>>  // RUN:         | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
>>  // RUN: %clang_cc1 -ffreestanding -fms-extensions -fms-compatibility
>> -fms-compatibility-version=17.00 \
>>  // RUN:         -triple x86_64--linux -emit-llvm %s -o - \
>> -// RUN:         | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
>> +// RUN:         | FileCheck %s --check-prefixes CHECK,CHECK-64BIT-LONG
>>  // RUN: %clang_cc1 -ffreestanding -fms-extensions \
>>  // RUN:         -triple x86_64--darwin -emit-llvm %s -o - \
>> -// RUN:         | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
>> -
>> -// LP64 targets use 'long' as 'int' for MS intrinsics (-fms-extensions)
>> -#ifdef __LP64__
>> -#define LONG int
>> -#else
>> -#define LONG long
>> -#endif
>> +// RUN:         | FileCheck %s --check-prefixes CHECK,CHECK-64BIT-LONG
>>
>>  // rotate left
>>
>> @@ -47,12 +40,15 @@ unsigned int test_rotl(unsigned int valu
>>  // CHECK:   [[R:%.*]] = call i32 @llvm.fshl.i32(i32 [[X:%.*]], i32
>> [[X]], i32 [[Y:%.*]])
>>  // CHECK:   ret i32 [[R]]
>>
>> -unsigned LONG test_lrotl(unsigned LONG value, int shift) {
>> +unsigned long test_lrotl(unsigned long value, int shift) {
>>    return _lrotl(value, shift);
>>  }
>>  // CHECK-32BIT-LONG: i32 @test_lrotl
>>  // CHECK-32BIT-LONG:   [[R:%.*]] = call i32 @llvm.fshl.i32(i32
>> [[X:%.*]], i32 [[X]], i32 [[Y:%.*]])
>>  // CHECK-32BIT-LONG:   ret i32 [[R]]
>> +// CHECK-64BIT-LONG: i64 @test_lrotl
>> +// CHECK-64BIT-LONG:   [[R:%.*]] = call i64 @llvm.fshl.i64(i64
>> [[X:%.*]], i64 [[X]], i64 [[Y:%.*]])
>> +// CHECK-64BIT-LONG:   ret i64 [[R]]
>>
>>  unsigned __int64 test_rotl64(unsigned __int64 value, int shift) {
>>    return _rotl64(value, shift);
>> @@ -84,12 +80,15 @@ unsigned int test_rotr(unsigned int valu
>>  // CHECK:   [[R:%.*]] = call i32 @llvm.fshr.i32(i32 [[X:%.*]], i32
>> [[X]], i32 [[Y:%.*]])
>>  // CHECK:   ret i32 [[R]]
>>
>> -unsigned LONG test_lrotr(unsigned LONG value, int shift) {
>> +unsigned long test_lrotr(unsigned long value, int shift) {
>>    return _lrotr(value, shift);
>>  }
>>  // CHECK-32BIT-LONG: i32 @test_lrotr
>>  // CHECK-32BIT-LONG:   [[R:%.*]] = call i32 @llvm.fshr.i32(i32
>> [[X:%.*]], i32 [[X]], i32 [[Y:%.*]])
>>  // CHECK-32BIT-LONG:   ret i32 [[R]]
>> +// CHECK-64BIT-LONG: i64 @test_lrotr
>> +// CHECK-64BIT-LONG:   [[R:%.*]] = call i64 @llvm.fshr.i64(i64
>> [[X:%.*]], i64 [[X]], i64 [[Y:%.*]])
>> +// CHECK-64BIT-LONG:   ret i64 [[R]]
>>
>>  unsigned __int64 test_rotr64(unsigned __int64 value, int shift) {
>>    return _rotr64(value, shift);
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
> _______________________________________________
> 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/20200226/ad410b25/attachment.html>


More information about the cfe-commits mailing list