[PATCH] D66725: [DAGCombiner][TargetLowering] Target hook for FCOPYSIGN arg cast folding

[Sam Elliott via Phabricator via llvm-commits]

lenary marked an inline comment as done.
lenary added a comment.

I am happy with this. I think this hook is the correct way to go about choosing how to do this optimisation, and accurately conveys what's going on.

I would like to see a review by someone who works on cross-target parts of DAGCombiner, before this is landed.



================
Comment at: llvm/include/llvm/CodeGen/TargetLowering.h:2590
+  virtual bool canCopySign(EVT ValueTy, EVT SignTy) const {
+    return isOperationExpand(ISD::FCOPYSIGN, ValueTy) || ValueTy == SignTy;
+  }
----------------
luismarques wrote:
> luismarques wrote:
> > lenary wrote:
> > > Which targets use this default and not `SignTy != i128`, beyond RISC-V and WebAssembly?
> > AArch64 overrides the hook but accepts all types, including `f128`. In-tree targets that keep the default implementation in this patch are ARC, AVR, BPF, Lanai, MSP430, NVPTX, Sparc, RISCV and WebAssembly.
> ...and XCore.
Ok, nice, I just wanted to make sure this was a sensible default, rather than being overriden everywhere. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66725