[PATCH] D102607: [AArch64][SVE] Add fixed length codegen for FP_ROUND/FP_EXTEND

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added a comment.

I've added a recommendation that I believe will also simplify D102777 <https://reviews.llvm.org/D102777>.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17751-17774
+  EVT ContainerDstVT = getContainerForFixedLengthVector(DAG, VT);
+  EVT ContainerSrcVT =
+      getContainerForFixedLengthVector(DAG, Val.getValueType());
+  EVT ExtendVT = ContainerDstVT.changeVectorElementType(
+      ContainerSrcVT.getVectorElementType());
+
+  Val = convertToScalableVector(DAG, ContainerSrcVT, Val);
----------------
Although correct can I recommend a different approach.  Rather than "any_extending" the scalable value you should be able to actually `ISD::ANY_EXTEND` the fixed length operand directly.  This simplifies the lowering and means the `any_extend->uunplo` transform is kept in one place.  The flow can be something like:
```
src = bit cast(src_as_int, src)
src = any_extend(dst_as_int, src)
src = convertToScalable(src)
src = svesafecast(src)
src = fpextend(src)
```
The legaliser will then transform the any_extend separately.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17801
+                    Op.getOperand(1), DAG.getUNDEF(RoundVT));
+  Val = getSVESafeBitCast(ContainerSrcVT.changeVectorElementTypeToInteger(),
+                          Val, DAG);
----------------
Although this is more direct, if you wanted to loose the line wrapping you can use `changeTypeToInteger()`.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17804-17823
+  // Repeatedly truncate Val until the result is of the desired element type.
+  switch (Val.getValueType().getSimpleVT().SimpleTy) {
+  default:
+    llvm_unreachable("unimplemented container type");
+  case MVT::nxv2i64:
+    Val = DAG.getNode(ISD::BITCAST, DL, MVT::nxv4i32, Val);
+    Val = DAG.getNode(AArch64ISD::UZP1, DL, MVT::nxv4i32, Val, Val);
----------------
As above, by doing an actual `ISD::TRUNCATE` of the fixed length result you get to reuse the existing mechanism to lower fixed length truncates.  I'm thinking:
```
  Val = convertFromScalableVector(DAG, SrcVT.changeTypeToInteger(), Val);           
  Val = DAG.getNode(ISD::TRUNCATE, DL, VT.changeTypeToInteger(), Val);              
  return DAG.getNode(ISD::BITCAST, DL, VT, Val);   
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102607