[cfe-dev] Possible bug in Win64 ABI in Clang?

[Keane, Erich via cfe-dev]

Does:
>HVA results have each data element returned by value in registers XMM0:XMM3 or YMM0:YMM3, depending on element size. Other result types are returned by reference to memory allocated by the caller.
 
Mean we should be doing 'expand' if the type fits in 4 registers?  

-----Original Message-----
From: Christoffer Lernö <christoffer at aegik.com> 
Sent: Monday, November 16, 2020 9:33 AM
To: Keane, Erich <erich.keane at intel.com>
Cc: Hans Wennborg <hans at chromium.org>; clang developer list <cfe-dev at lists.llvm.org>
Subject: Re: [cfe-dev] Possible bug in Win64 ABI in Clang?

As far as I can tell it should simply do an indirect here:

> Results of __vectorcall functions are returned by value in registers when possible. Results of integer type, including structs or unions of 4 bytes or less, are returned by value in EAX. Integer type structs or unions of 8 bytes or less are returned by value in EDX:EAX. Vector type results are returned by value in XMM0 or YMM0, depending on size. HVA results have each data element returned by value in registers XMM0:XMM3 or YMM0:YMM3, depending on element size. Other result types are returned by reference to memory allocated by the caller.

Christoffer
AEGIK / www.aegik.se

> On 16 Nov 2020, at 17:30, Keane, Erich <erich.keane at intel.com> wrote:
> 
> That was long enough ago that I don't really remember.  At the time, I wrote tests to validate the behaviors I think (which would mean it didn't crash?), but I could buy that I did something wrong back then.  Do we have an idea what the return-type ABIArgInfo should be?  I'm sorry I cannot be more helpful here.  
> 
> -----Original Message-----
> From: Hans Wennborg <hans at chromium.org> 
> Sent: Monday, November 16, 2020 8:24 AM
> To: Christoffer Lernö <christoffer at aegik.com>; Keane, Erich <erich.keane at intel.com>
> Cc: clang developer list <cfe-dev at lists.llvm.org>
> Subject: Re: [cfe-dev] Possible bug in Win64 ABI in Clang?
> 
> On Sat, Nov 14, 2020 at 12:36 PM Christoffer Lernö via cfe-dev <cfe-dev at lists.llvm.org> wrote:
>> 
>> Perusing the Clang source, I found something rather confusing:
>> 
>> if ((IsVectorCall || IsRegCall) &&
>>      isHomogeneousAggregate(Ty, Base, NumElts)) {
>>    if (IsRegCall) {
>>      if (FreeSSERegs >= NumElts) {
>>        FreeSSERegs -= NumElts;
>>        if (IsReturnType || Ty->isBuiltinType() || Ty->isVectorType())
>>          return ABIArgInfo::getDirect();
>>        return ABIArgInfo::getExpand();
>>      }
>>      return ABIArgInfo::getIndirect(Align, /*ByVal=*/false);
>>    } else if (IsVectorCall) {
>>      if (FreeSSERegs >= NumElts &&
>>          (IsReturnType || Ty->isBuiltinType() || Ty->isVectorType())) {
>>        FreeSSERegs -= NumElts;
>>        return ABIArgInfo::getDirect();
>>      } else if (IsReturnType) {
>>        return ABIArgInfo::getExpand();
>>      } else if (!Ty->isBuiltinType() && !Ty->isVectorType()) {
>>        // HVAs are delayed and reclassified in the 2nd step.
>>        return ABIArgInfo::getIndirect(Align, /*ByVal=*/false);
>>      }
>>    }
>>  }
>> 
>> 
>> If we look at ”isReturnType” for IsVectorCall = true has ”ABIArgInfo::getExpand()” however, ”expand” is not a valid type of ABIArgInfo and will throw an error.
>> 
>> So this seems to be incorrect and should crash on vectorcall with HVA. Can someone confirm?
> 
> For reference, that code is from WinX86_64ABIInfo::classify() here:
> https://github.com/llvm/llvm-project/blob/bc7df035ae68648fe39304d9e77cd7618812cca8/clang/lib/CodeGen/TargetInfo.cpp#L4200
> 
> I'm not familiar with this code, but it looks like Erich wrote it in
> https://reviews.llvm.org/D27529 Maybe he can comment?
> 
> Thanks,
> Hans