[PATCH] D114628: [AArch64][SVE] Duplicate FP_EXTEND/FP_TRUNC -> LOAD/STORE dag combines

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added a comment.

I suppose an alternate solution is a new TLI hook that is less reliant on the legalisation tables. However, if we're the only backend with this issue I guess copying a couple of DAG combines is not so bad.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:15952-15953
+  // store. We can do this even if this is already a truncstore.
+  // We purposefully don't care about legality of the nodes here as we know
+  // they can be split down into something legal.
+  if (Value.getOpcode() == ISD::FP_ROUND && Value.getNode()->hasOneUse() &&
----------------
Although true, here it's really type legalisation you don't want to be affected by and this cannot ask the usual question (because we only say we support custom lower for the legal types).  However, as is the combine could result in an unselectable DAG so I think you still need a `DCI.isBeforeLegalizeOps()`  check somewhere.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17313-17314
+  // fold (fpext (load x)) -> (fpext (fptrunc (extload x)))
+  // We purposefully don't care about legality of the nodes here as we know
+  // they can be split down into something legal.
+  if (ISD::isNormalLoad(N0.getNode()) && N0.hasOneUse() &&
----------------
Same `DCI.isBeforeLegalizeOps` comment as above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114628