[PATCH] D153848: [RISCV] Apply promotion for f16 vector ops when only have zvfhmin.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:1078
+          // TODO: could split the f16 vector into two vectors and do promotion.
+          if (!useRVVForFixedLengthVectorVT(F32VecVT))
+            continue;
----------------
Use isTypeLegal?


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:4623
+    }
+    auto [LoOperand, HiOperand] = DAG.SplitVector(Op.getOperand(j), DL);
+    LoOperands[j] = LoOperand;
----------------
Does `std::tie(LoOperands[j], HiOperands[j]) =` work?


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:4801
   case ISD::IS_FPCLASS:
+
     return LowerIS_FPCLASS(Op, DAG);
----------------
Drop this change


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:5264
   case ISD::FADD:
+    if (Op.getValueType() == MVT::nxv32f16 &&
+        (Subtarget.hasVInstructionsF16Minimal() &&
----------------
Can we make a new function in place of `lowerToScalableOp` that does this check so we don't repeat it so many times.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:5418
   case ISD::VP_FADD:
+    if (Op.getValueType() == MVT::nxv32f16 &&
+        (Subtarget.hasVInstructionsF16Minimal() &&
----------------
Can we make a new function in place of `lowerVPOp` that encapsulates this check?


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:12869
+      return V;
+    // Don't combine to fp16 for zvfhmin.
+    if (N->getValueType(0).isScalableVector() &&
----------------
Can this be moved into performVFMADD_VLCombine?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153848