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

Michael Spencer via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 26 14:09:19 PST 2020


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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200226/661835e2/attachment.html>


More information about the cfe-commits mailing list