[PATCH] D88033: [SVE][CodeGen] Legalisation of integer -> floating point conversions

Wed Sep 23 12:03:17 PDT 2020

efriedma added a comment.

In D88033#2289829 <https://reviews.llvm.org/D88033#2289829>, @paulwalker-arm wrote:

> In D88033#2288287 <https://reviews.llvm.org/D88033#2288287>, @efriedma wrote:
>
>> LGTM
>>
>> Alternatively, we could make CONCAT_VECTOR "legal", and lower it using an isel pattern.  But I'm not sure that's actually an improvement.
>
> @efriedma At this level do we need to care about CONCAT_VECTORS that do not have two operands?  If so then we'll need custom lowering anyway to ensure those cases get expanded?

I think we could end up with a CONCAT_VECTORS with more than two operands.  At least, I can't think of anything that would prevent it (as long as the operands have a legal type).  You could still pattern-match that to a tree of uzp1, but I'd be more worried about blocking useful optimizations at that point; probably custom-lowering that makes sense.

================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:9087-9089
+  if (!VT.getVectorElementType().isFloatingPoint() ||
+      VT.getVectorElementCount() != (OpVT.getVectorElementCount()*2))
+    return SDValue();
----------------
sdesmalen wrote:
> paulwalker-arm wrote:
> > I imagine that CONCAT_VECTORS operand counts have been normalise to two by this point but given its restriction that all operands must be the same type I think you can protect against this and your `VT == OpVT*2` requirement using `if (getNumOperands() != 2)`.
> > 
> > Can the isFloatingPoint check be made part of the assert?
> > Can the isFloatingPoint check be made part of the assert?
> Could this also work for predicates (e.g. nxv8i1 : nxv8i1)?
We actually have patterns for i1 already.  But would probably work, sure.

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

https://reviews.llvm.org/D88033