[PATCH] D73978: [WIP][FPEnv] Don't transform FSUB(-0.0,X)->FNEG(X) when flushing denormals
Sanjay Patel via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 4 12:33:07 PST 2020
spatel added inline comments.
================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:2997-3015
// -0.0 - X --> fneg
Type *Ty = I.getType();
if (isa<Constant>(I.getOperand(0)) &&
I.getOperand(0) == ConstantFP::getZeroValueForNegation(Ty)) {
SDValue Op2 = getValue(I.getOperand(1));
- setValue(&I, DAG.getNode(ISD::FNEG, getCurSDLoc(),
- Op2.getValueType(), Op2));
- return;
+ EVT VT = Op2.getValueType();
+
----------------
cameron.mcinally wrote:
> arsenm wrote:
> > arsenm wrote:
> > > Why does SelectionDAGBuilder bother doing this fold at all? It should just directly translate the fsub?
> > I think this should be just ripped out in a separate patch. The same problem seems to have been copied to IRTranslator, which should also be removed
> @sanjay, what do you think? Seems reasonable to me.
>
> I think it made sense to do this when FNEG(X) was the canonical form of FSUB(-0.0, X). Wouldn't want two forms floating around for even a small amount of time. But now that there are cases where the operations are distinct through llc, it seems ok to wait until DAGCombine.
Yes, now that we can use DenormMode to distinguish target behavior, it seems better to do it later in DAGCombiner if that would make sense for the target.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73978/new/
https://reviews.llvm.org/D73978
More information about the llvm-commits
mailing list