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

[Luís Marques via Phabricator via llvm-commits]

luismarques added a comment.

In D66725#1717427 <https://reviews.llvm.org/D66725#1717427>, @efriedma wrote:

> For FCOPYSIGN, specifically, LegalizeDAG is going to query the target to ask, "is FCOPYSIGN legal for result type X", using the getOperationAction() API?  The target has a few different ways to respond: here, the relevant possibilities are "Legal", "Expand", or "Custom".  I guess the issue here is that on RISCV, this query is returning "Legal", when it actually isn't legal for all combinations of legal result/input types?
>  If this is a problem for a bunch of targets, maybe we need a different API for expressing whether an FCOPYSIGN is legal.  I'd prefer to follow existing convention for other operations which involve multiple types, though; for example, see TargetLoweringBase::getLoadExtAction.


@efriedma: one problem is that whether we want to do the combine or not doesn't map well to whether FCOPYSIGN is legal (for return type X). For instance, if you don't have an FPU and you expand the copysign (to integer bit manipulations) you actually still want to do the combine, since it's pointless to emit a libcall for promoting/truncating the floating-point sign value -- it's just as easy to extract the sign bit from the original value. In any case, I experimented with the proposed approach and I will submit the patch for review for comparison. But I think it ends up addressing this problem in an even kludgier way (I'll add some details in that patch's summary).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66725