[PATCH] D88321: [SVE][CodeGen] Lower scalable fp_extend & fp_round operations

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:15926
   for (const SDValue &V : Op->op_values()) {
-    assert((isa<CondCodeSDNode>(V) || V.getValueType().isScalableVector()) &&
+    assert((!V.getValueType().isVector() ||
+            V.getValueType().isScalableVector()) &&
----------------
sdesmalen wrote:
> is this change intentional?
This is to account for the extra non-vector argument, with the original assert being overly restrictive.


================
Comment at: llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td:1451
+
+  def : Pat<(nxv2f16 (AArch64fcvtr_mt (nxv2i1 PPR:$Pg), (nxv2f32 ZPR:$Zs), (i64 timm0_1), (nxv2f16 ZPR:$Zd))),
+            (FCVT_ZPmZ_StoH ZPR:$Zd, PPR:$Pg, ZPR:$Zs)>;
----------------
sdesmalen wrote:
> This matches both 0 and 1, but is otherwise ignored in this pattern.
> I wonder if this should match only `0` for normal rounding?
I don't believe the "extra" operand affects the functional requirements of the node, so it's not the case that you need to select "normal rounding" or something else. It looks to be purely an optimisation aid and I don't know why it's not a flag like isExact(), which can be attached to things like shifts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88321