r216491 - Win64 ABI shouldn't extend integer type arguments.

Rafael EspĂ­ndola rafael.espindola at gmail.com
Tue Aug 26 15:14:56 PDT 2014


Looks like this broke CodeGenCXX/microsoft-abi-member-pointers.cpp

On 26 August 2014 17:52, Julien Lerouge <jlerouge at apple.com> wrote:
> Author: jlerouge
> Date: Tue Aug 26 16:52:27 2014
> New Revision: 216491
>
> URL: http://llvm.org/viewvc/llvm-project?rev=216491&view=rev
> Log:
> Win64 ABI shouldn't extend integer type arguments.
>
> Summary:
> MSVC doesn't extend integer types smaller than 64bit, so to preserve
> binary compatibility, clang shouldn't either.
>
> For example, the following C code built with MSVC:
>
> unsigned test(unsigned v);
> unsigned foobar(unsigned short);
> int main() { return test(0xffffffff) + foobar(28); }
>
> Produces the following:
>
>   0000000000000004: B9 FF FF FF FF     mov         ecx,0FFFFFFFFh
>   0000000000000009: E8 00 00 00 00     call        test
>   000000000000000E: 89 44 24 20        mov         dword ptr [rsp+20h],eax
>   0000000000000012: 66 B9 1C 00        mov         cx,1Ch
>   0000000000000016: E8 00 00 00 00     call        foobar
>
> And as you can see, when setting up the call to foobar, only cx is overwritten.
>
> If foobar is compiled with clang, then the zero extension added by clang means
> the rest of the register, which contains garbage, could be used.
>
> For example if foobar is:
>
> unsigned foobar(unsigned short v) {
>     return v;
> }
>
> Compiled with clang -fomit-frame-pointer -O3 gives the following assembly:
>
> foobar:
>   0000000000000000: 89 C8              mov         eax,ecx
>   0000000000000002: C3                 ret
>
> And that function would return garbage because the 16 most significant bits of
> ecx still contain garbage from the first call.
>
> With this change, the code for that function is now:
>
> foobar:
>   0000000000000000: 0F B7 C1           movzx       eax,cx
>   0000000000000003: C3                 ret
>
> Reviewers: chapuni, rnk
>
> Reviewed By: rnk
>
> Subscribers: majnemer, cfe-commits
>
> Differential Revision: http://reviews.llvm.org/D4380
>
> Added:
>     cfe/trunk/test/CodeGen/x86_64-arguments-win32.c
> Modified:
>     cfe/trunk/lib/CodeGen/TargetInfo.cpp
>     cfe/trunk/test/CodeGen/2007-06-18-SextAttrAggregate.c
>
> Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=216491&r1=216490&r2=216491&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Tue Aug 26 16:52:27 2014
> @@ -2773,9 +2773,6 @@ ABIArgInfo WinX86_64ABIInfo::classify(Qu
>      return ABIArgInfo::getDirect(llvm::IntegerType::get(getVMContext(), Size));
>    }
>
> -  if (Ty->isPromotableIntegerType())
> -    return ABIArgInfo::getExtend();
> -
>    return ABIArgInfo::getDirect();
>  }
>
>
> Modified: cfe/trunk/test/CodeGen/2007-06-18-SextAttrAggregate.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/2007-06-18-SextAttrAggregate.c?rev=216491&r1=216490&r2=216491&view=diff
> ==============================================================================
> --- cfe/trunk/test/CodeGen/2007-06-18-SextAttrAggregate.c (original)
> +++ cfe/trunk/test/CodeGen/2007-06-18-SextAttrAggregate.c Tue Aug 26 16:52:27 2014
> @@ -1,11 +1,13 @@
>  // RUN: %clang_cc1 %s -o - -emit-llvm | FileCheck %s
> -// XFAIL: aarch64, arm64
> +// XFAIL: aarch64, arm64, x86_64-pc-win32
>
>  // PR1513
>
>  // AArch64 ABI actually requires the reverse of what this is testing: the callee
>  // does any extensions and remaining bits are unspecified.
>
> +// Win64 ABI does expect extensions for type smaller than 64bits.
> +
>  // Technically this test wasn't written to test that feature, but it's a
>  // valuable check nevertheless.
>
>
> Added: cfe/trunk/test/CodeGen/x86_64-arguments-win32.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/x86_64-arguments-win32.c?rev=216491&view=auto
> ==============================================================================
> --- cfe/trunk/test/CodeGen/x86_64-arguments-win32.c (added)
> +++ cfe/trunk/test/CodeGen/x86_64-arguments-win32.c Tue Aug 26 16:52:27 2014
> @@ -0,0 +1,16 @@
> +// RUN: %clang_cc1 -w -triple x86_64-pc-win32 -emit-llvm -o - %s | FileCheck %s
> +
> +// To be ABI compatible with code generated by MSVC, there shouldn't be any
> +// sign/zero extensions on types smaller than 64bit.
> +
> +// CHECK-LABEL: define void @f1(i8 %a)
> +void f1(char a) {}
> +
> +// CHECK-LABEL: define void @f2(i8 %a)
> +void f2(unsigned char a) {}
> +
> +// CHECK-LABEL: define void @f3(i16 %a)
> +void f3(short a) {}
> +
> +// CHECK-LABEL: define void @f4(i16 %a)
> +void f4(unsigned short a) {}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits



More information about the cfe-commits mailing list