Possible bug in Win64 ABI in Clang?

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

Possible bug in Win64 ABI in Clang?

David Blaikie via cfe-dev
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?


Christoffer
AEGIK / www.aegik.se


_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Possible bug in Win64 ABI in Clang?

David Blaikie via cfe-dev
On Sat, Nov 14, 2020 at 12:36 PM Christoffer Lernö via cfe-dev
<[hidden email]> 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
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Possible bug in Win64 ABI in Clang?

David Blaikie via cfe-dev
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 <[hidden email]>
Sent: Monday, November 16, 2020 8:24 AM
To: Christoffer Lernö <[hidden email]>; Keane, Erich <[hidden email]>
Cc: clang developer list <[hidden email]>
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 <[hidden email]> 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
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Possible bug in Win64 ABI in Clang?

David Blaikie via cfe-dev
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 <[hidden email]> 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 <[hidden email]>
> Sent: Monday, November 16, 2020 8:24 AM
> To: Christoffer Lernö <[hidden email]>; Keane, Erich <[hidden email]>
> Cc: clang developer list <[hidden email]>
> 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 <[hidden email]> 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

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Possible bug in Win64 ABI in Clang?

David Blaikie 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ö <[hidden email]>
Sent: Monday, November 16, 2020 9:33 AM
To: Keane, Erich <[hidden email]>
Cc: Hans Wennborg <[hidden email]>; clang developer list <[hidden email]>
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 <[hidden email]> 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 <[hidden email]>
> Sent: Monday, November 16, 2020 8:24 AM
> To: Christoffer Lernö <[hidden email]>; Keane, Erich <[hidden email]>
> Cc: clang developer list <[hidden email]>
> 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 <[hidden email]> 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

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Possible bug in Win64 ABI in Clang?

David Blaikie via cfe-dev
”Expand” is never valid for returns, as it simply splits an aggregate over multiple parameters. If it fits in 4 registers I assume it is a ”direct” as it is written.

Christoffer
AEGIK / www.aegik.se

On 16 Nov 2020, at 18:35, Keane, Erich <[hidden email]> wrote:

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ö <[hidden email]>
Sent: Monday, November 16, 2020 9:33 AM
To: Keane, Erich <[hidden email]>
Cc: Hans Wennborg <[hidden email]>; clang developer list <[hidden email]>
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 <[hidden email]> 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 <[hidden email]>
Sent: Monday, November 16, 2020 8:24 AM
To: Christoffer Lernö <[hidden email]>; Keane, Erich <[hidden email]>
Cc: clang developer list <[hidden email]>
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 <[hidden email]> 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



_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Possible bug in Win64 ABI in Clang?

David Blaikie via cfe-dev

I don’t particularly get the ‘expand is never valid for returns’, as I’m sure I’ve seen it before, but I’m also sure I don’t know the ABI code well enough to speak with authority.

 

There might be some other goofiness in that code as well.  I wouldn’t expect the return-value to decrease the available number of registers, since returns re-use registers. 


Either way, you’re likely right that that this code needs work.  Patches welcome 😊

 

From: Christoffer Lernö <[hidden email]>
Sent: Monday, November 16, 2020 9:37 AM
To: Keane, Erich <[hidden email]>
Cc: Hans Wennborg <[hidden email]>; clang developer list <[hidden email]>
Subject: Re: [cfe-dev] Possible bug in Win64 ABI in Clang?

 

”Expand” is never valid for returns, as it simply splits an aggregate over multiple parameters. If it fits in 4 registers I assume it is a ”direct” as it is written.

 

Christoffer

AEGIK / www.aegik.se



On 16 Nov 2020, at 18:35, Keane, Erich <[hidden email]> wrote:

 

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ö <[hidden email]>
Sent: Monday, November 16, 2020 9:33 AM
To: Keane, Erich <[hidden email]>
Cc: Hans Wennborg <[hidden email]>; clang developer list <[hidden email]>
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 <[hidden email]> 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 <[hidden email]>
Sent: Monday, November 16, 2020 8:24 AM
To: Christoffer Lernö <[hidden email]>; Keane, Erich <[hidden email]>
Cc: clang developer list <[hidden email]>
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 <[hidden email]> 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

 

 


_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Possible bug in Win64 ABI in Clang?

David Blaikie via cfe-dev
Expand assumes parameters, since it basically takes the struct and drops each part of the struct into a different parameter (e.g. void foo(struct { int , int }) -> void foo(int, int) ). Which cannot work because you can at most have a single value return value in the function prototype. So the ”expand” is not even implemented for return values in the code.

Christoffer
AEGIK / www.aegik.se

On 16 Nov 2020, at 18:39, Keane, Erich <[hidden email]> wrote:

I don’t particularly get the ‘expand is never valid for returns’, as I’m sure I’ve seen it before, but I’m also sure I don’t know the ABI code well enough to speak with authority.
 
There might be some other goofiness in that code as well.  I wouldn’t expect the return-value to decrease the available number of registers, since returns re-use registers. 

Either way, you’re likely right that that this code needs work.  Patches welcome 😊
 
From: Christoffer Lernö <[hidden email]> 
Sent: Monday, November 16, 2020 9:37 AM
To: Keane, Erich <[hidden email]>
Cc: Hans Wennborg <[hidden email]>; clang developer list <[hidden email]>
Subject: Re: [cfe-dev] Possible bug in Win64 ABI in Clang?
 
”Expand” is never valid for returns, as it simply splits an aggregate over multiple parameters. If it fits in 4 registers I assume it is a ”direct” as it is written.
 
Christoffer
AEGIK / www.aegik.se


On 16 Nov 2020, at 18:35, Keane, Erich <[hidden email]> wrote:
 
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ö <[hidden email]> 
Sent: Monday, November 16, 2020 9:33 AM
To: Keane, Erich <[hidden email]>
Cc: Hans Wennborg <[hidden email]>; clang developer list <[hidden email]>
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 <[hidden email]> 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 <[hidden email]> 
Sent: Monday, November 16, 2020 8:24 AM
To: Christoffer Lernö <[hidden email]>; Keane, Erich <[hidden email]>
Cc: clang developer list <[hidden email]>
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 <[hidden email]> 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


_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev