[PATCH] D87130: [SelectionDAGBuilder] Remember to copy the FMF flags in visitBinary().

[Ulrich Weigand via Phabricator via llvm-commits]

uweigand added a comment.

In D87130#2256441 <https://reviews.llvm.org/D87130#2256441>, @jonpa wrote:

> In D87130#2256173 <https://reviews.llvm.org/D87130#2256173>, @uweigand wrote:
>
>> I think this makes sense to me, but I want to point out that this partially reverts the commit https://reviews.llvm.org/D46854.  I guess there may be other parts of this commit that should be reverted?
>
> The other thing that was removed by that commit was the propagation of the NoNaNs flag from the experimental_vector_reduce_fmax (/fmin) intrinsic to the replacing VECREDUCE_FMAX (/FMIN) node, but I am not sure why...

Well, the whole approach of D46854 <https://reviews.llvm.org/D46854> was to **remove** setting of the FMF in DAG.getNode() everywhere, and **instead** set the flags via setFlags() after the fact.  The point was to be that setting the flags in one place reduces code duplication.   However, we're now having the discussion that using setFlags() after the fact may be incorrect in the presence of memoized nodes, and instead we should be always setting flags directly in getNode() -- this basically argues for exactly the opposite direction of what that commit did.


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

https://reviews.llvm.org/D87130