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

[Keane, Erich 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ö <christoffer at aegik.com>
Sent: Monday, November 16, 2020 9:37 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?

”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<http://www.aegik.se>


On 16 Nov 2020, at 18:35, Keane, Erich <erich.keane at intel.com<mailto:erich.keane at intel.com>> 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ö <christoffer at aegik.com<mailto:christoffer at aegik.com>>
Sent: Monday, November 16, 2020 9:33 AM
To: Keane, Erich <erich.keane at intel.com<mailto:erich.keane at intel.com>>
Cc: Hans Wennborg <hans at chromium.org<mailto:hans at chromium.org>>; clang developer list <cfe-dev at lists.llvm.org<mailto: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<http://www.aegik.se>


On 16 Nov 2020, at 17:30, Keane, Erich <erich.keane at intel.com<mailto: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<mailto:hans at chromium.org>>
Sent: Monday, November 16, 2020 8:24 AM
To: Christoffer Lernö <christoffer at aegik.com<mailto:christoffer at aegik.com>>; Keane, Erich <erich.keane at intel.com<mailto:erich.keane at intel.com>>
Cc: clang developer list <cfe-dev at lists.llvm.org<mailto: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<mailto: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


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20201116/b47530c3/attachment.html>