[PATCH] D102852: [RISCV] Fix a crash when lowering split float arguments

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck added a comment.

In D102852#2830433 <https://reviews.llvm.org/D102852#2830433>, @luismarques wrote:

> Some thoughts:
>
> 1. I agree with you that the psABI doesn't (for now, at least) specify this ABI condition so technically we can do whatever we want. Whether we should still follow the spirit of the ABI or be free to make something completely different I'm not sure about. As you noted, this IR construct might arguably also be outside of what a vector-aware ABI would specify (but clang isn't the only frontend).
>
> 2. Besides the psABI not covering vectors explicitly, I think what makes the situation more confusing is that arrays are passed by reference, so the specification of the ABI for non-scalar types ends up being less general than it could be. IMO vectors passed by value are reasonably analogous to the "Floating-point reals" and other structs, the way they are discussed in the spec.
>
>> Since the `Hardware Floating-point Calling Convention` doesn't refer to "aggregates" as much as it refers to "structs" I suppose it follows from your opinion about aggregates that, if the psABI is relevant, we should be looking entirely at the `Integer Calling Convention`? So there shouldn't be any use of floating-point registers in lowering these illegal vectors at all, no matter which ABI is used?
>
> A struct is an aggregate. I think the FP section just happens to use more specific language, as it details the rules for the actual language constructs (structs, reals, etc) that would be impacted by the FP calling convention. But if we decided to follow the spirit of the psABI my overall reading of it is that I would expect small (<= 2×XLEN) vectors passed by value to follow the FP convention, when available.
>
> I'm not opposed to this implementation as "something that works" and that "doesn't have to be compatible with anything" and that "solves our problems now". It's just not the ABI I would expect. It seemed to me like an unnecessary departure from convention, and that's why I didn't find it particularly appealing. But perhaps it doesn't matter? In any case, thanks for the patch and for your analysis. Let's hear from other people -- @kito-cheng @asb and others.

Thanks for all of your input and thoughts. It's great to have this discussion as this sort of stuff, as we can see, isn't something I feel comfortable asserting on by myself. I would like to hear from other people, but I should add that another reason I'm wary of treating vectors == aggregates as specified by the psABI because I don't think there's great value in passing vectors `<= 2*XLEN` (`<8 x i8>` on RV64, etc)  packed into one/two register(s). We'd just have to unpack it back into individual elements with shifts and truncates/extends on the caller side anyway since the vector operations would be scalarized by most compilers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102852