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

[Julien Lerouge]

Ah, thanks, I’ll revert and take a look.

On Aug 26, 2014, at 3:14 PM, Rafael Espíndola <rafael.espindola at gmail.com> wrote:

> 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