[PATCH] D28675: [DAGCombine] require UnsafeFPMath for re-association of addition
Hal Finkel via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 17 07:47:36 PST 2017
hfinkel added a comment.
In https://reviews.llvm.org/D28675#648189, @spatel wrote:
> In https://reviews.llvm.org/D28675#647995, @nhaehnle wrote:
> > 1. Take individual UnsafeAlgebra flags into account.
> > There's a question of whether the flag should be required on both of the involved nodes, or if having it one of them is sufficient. I opted for requiring it on both of them, to prevent unsafe flags from leaking out of where they were originally added.
> That sounds right. We can't reassociate strict ops in general, so the lack of hasUnsafeAlgebra() on any of the operands that will change in these transforms should prevent the transform (unless the FMA is guaranteed to produce a better result like we showed in https://reviews.llvm.org/D26602?).
> And yes, it's a moot point for this patch currently because we haven't extended FMF beyond the binary FP ops.
Okay. Please add a FIXME wr.t. adding FMF on the FMA nodes, and proceed with fixing the bug.
More information about the llvm-commits