[PATCH] D79709: [AArch64][BFloat] basic AArch64 bfloat support

Thu May 21 02:08:41 PDT 2020

stuij added a comment.

Hiya Francesco, thanks for the thorough comment :)

In D79709#2047996 <https://reviews.llvm.org/D79709#2047996>, @fpetrogalli wrote:

> In D79709#2046781 <https://reviews.llvm.org/D79709#2046781>, @stuij wrote:
>
> > Having a think, I think the calling convention test would be more meaningful if we have a bit more bfloat lowering support. Which we have in the bfloat lowering follow-up patch: https://reviews.llvm.org/D79712
>
>
> Yep - I'd make sure that the codegen patch is in before testing the CC. In fact, for the codegen patch you mention, I don't think you need CC at all:

I'm not talking about testing CC in general (tbh I'm not convinced we need extensive CC testing for every single type). I'm talking about servicing your request of a test for passing or returning bfloat:

> Is there a way we can test the calling convention with your changes? A test that shows that passing or returning bfloat (scalar or vector) via IR functions is mapping to the correct registers will be good to have.

As you mention, a sensible test would show we're passing bfloat in a sensible register, but this patch doesn't supply support for lowering of for example load. The follow-up patch does support this. However this patch implements passing bfloat as an argument, so this patch needs to be applied before we can apply any other patches.

My suggestion is to commit this patch as-is, and put a meaningful argument-passing test (which I've already implemented) in the above mentioned follow-up patch.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79709/new/

https://reviews.llvm.org/D79709