[PATCH] D110237: [AArch64][SVE] Add DAG combines to improve SVE fixed type FP_EXTEND lowering

[Bradley Smith via Phabricator via llvm-commits]

bsmith added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:15242
 
+static SDValue performFpExtendCombine(SDNode *N, SelectionDAG &DAG,
+                                      const AArch64Subtarget *Subtarget) {
----------------
sdesmalen wrote:
> `DAGCombiner::visitFP_EXTEND` tries to do something similar, but unfortunately it is a bit too conservative. Look for:
> 
>   // fold (fpext (load x)) -> (fpext (fptrunc (extload x)))
> 
> It seems logical to reuse that code, rather than reimplementing the same thing here. Perhaps you can use something like `isVectorLoadExtDesirable` instead of `isLoadExtLegal` to determine whether to fold this.
There's actually a subtle difference here, that existing DAG combine is ultimately trying to fold the fpext into the load, creating an FP extending load, which is not something we can do. The DAG combine below is not trying to remove the fpext, but rather ensure the load is of the same size as the fpext so that is gets split in the same way.

I'm also not sure it makes sense for this to be in the generic DAG combine. The only reason this is sensible to do, is that our lowering of FP_EXTEND introduces an //integer// extend, which can then be folded into the integer truncate this DAG combine introduces. In absence of that fact, this DAG combine would likely just make things worse. It's also only really required because we can't sensibly lower extract_subvector.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110237