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

[jonpa]

Summary

This is a follow-up patch for the bugfix of https://reviews.llvm.org/D86871.

It fixes the XFAILING SystemZ/fp-mul-14.ll test, and also improves two others.

(D86871 is still pending, and this will be committed afterwards to fix its XFAILing test)

[uweigand]

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?

[jonpa]

In 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...

[uweigand]

In D87130#2256441, @jonpa wrote:

In 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 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.

[craig.topper]

In D87130#2256747, @uweigand wrote:

In D87130#2256441, @jonpa wrote:

In 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 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.

The after the fact handling also caused issue with constrained fp intrinsics which is why they were already excluded. It also had an issue with the vector reduction flag we used to have until I removed it in https://reviews.llvm.org/D76649.

[spatel]

In D87130#2256441, @jonpa wrote:

In 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...

In D87130#2256747, @uweigand wrote:

In D87130#2256441, @jonpa wrote:

In 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 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.

Right - we were trying to make a single point for transferring FMF instead of having every visitXXX() fend for itself. But if there's no way to do it safely/correctly, let's reverse course. At this point, I'm not sure if there is still some problem with always adding flags at getNode() time. Ie, we added the 'Defined' state bit to solve some other flags transfer problem, but is that still needed?

[jonpa]

Not needed anymore thanks to D87200.

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

void SelectionDAGBuilder::visitBinary(const User &I, unsigned Opcode) {
   SDNodeFlags Flags;
   if (auto *OFBinOp = dyn_cast<OverflowingBinaryOperator>(&I)) {
     Flags.setNoSignedWrap(OFBinOp->hasNoSignedWrap());
     Flags.setNoUnsignedWrap(OFBinOp->hasNoUnsignedWrap());
   }
   if (auto *ExactOp = dyn_cast<PossiblyExactOperator>(&I)) {
     Flags.setExact(ExactOp->isExact());
   }
+  if (auto *FPOp = dyn_cast<FPMathOperator>(&I))
+    Flags.copyFMF(*FPOp);
 
   SDValue Op1 = getValue(I.getOperand(0));
   SDValue Op2 = getValue(I.getOperand(1));
   SDValue BinNodeValue = DAG.getNode(Opcode, getCurSDLoc(), Op1.getValueType(),
                                      Op1, Op2, Flags);
   setValue(&I, BinNodeValue);
 }
 

llvm/test/CodeGen/AArch64/fp-const-fold.ll

 ; CHECK-NEXT:    mov x8, #-4503599627370496
 ; CHECK-NEXT:    fmov d1, x8
 ; CHECK-NEXT:    fmul d0, d0, d1
 ; CHECK-NEXT:    ret
   %r = fmul nnan double %x, 0xfff0000000000000
   ret double %r
 }
 
-; TODO: Should simplify to undef
-
 define double @fdiv_nnan_undef_op0(double %x) {
 ; CHECK-LABEL: fdiv_nnan_undef_op0:
 ; CHECK:       // %bb.0:
-; CHECK-NEXT:    mov x8, #9221120237041090560
-; CHECK-NEXT:    fmov d0, x8
 ; CHECK-NEXT:    ret
   %r = fdiv nnan double undef, %x
   ret double %r
 }
 
-; TODO: Should simplify to undef
-
 define double @fdiv_nnan_undef_op1(double %x) {
 ; CHECK-LABEL: fdiv_nnan_undef_op1:
 ; CHECK:       // %bb.0:
-; CHECK-NEXT:    mov x8, #9221120237041090560
-; CHECK-NEXT:    fmov d0, x8
 ; CHECK-NEXT:    ret
   %r = fdiv nnan double %x, undef
   ret double %r
 }
 
-; TODO: Should simplify to undef
-
 define double @fdiv_ninf_undef_op0(double %x) {
 ; CHECK-LABEL: fdiv_ninf_undef_op0:
 ; CHECK:       // %bb.0:
-; CHECK-NEXT:    mov x8, #9221120237041090560
-; CHECK-NEXT:    fmov d0, x8
 ; CHECK-NEXT:    ret
   %r = fdiv ninf double undef, %x
   ret double %r
 }
 
-; TODO: Should simplify to undef
-
 define double @fdiv_ninf_undef_op1(double %x) {
 ; CHECK-LABEL: fdiv_ninf_undef_op1:
 ; CHECK:       // %bb.0:
-; CHECK-NEXT:    mov x8, #9221120237041090560
-; CHECK-NEXT:    fmov d0, x8
 ; CHECK-NEXT:    ret
   %r = fdiv ninf double %x, undef
   ret double %r
 }
 
 declare double @llvm.fma.f64(double, double, double)

llvm/test/CodeGen/SystemZ/fp-mul-14.ll

 ; RUN: llc < %s -mtriple=s390x-linux-gnu | FileCheck %s
 ;
 ; Check that a multiply-and-add results.
 
-; FIXME: This test is xfailed temporarily
-; XFAIL: *
-
 define void @f1(float %arg, float* %Dst) {
 ; CHECK-LABEL: f1:
 ; CHECK: maeb
 bb:
   %i = fmul contract float %arg, 0xBE6777A5C0000000
   %i4 = fadd contract float %i, 1.000000e+00
   %i5 = fmul contract float %arg, 0xBE6777A5C0000000
   %i6 = fadd contract float %i5, 1.000000e+00
   %i7 = fmul contract float %i4, 2.000000e+00
   store float %i7, float* %Dst
   ret void
 }
 

llvm/test/CodeGen/X86/fp-undef.ll

; ANY-NEXT:    retq
   ret float %r
 }
 
 ; Repeat all tests with fast-math-flags. Alternate 'nnan' and 'fast' for more coverage.
 
 define float @fadd_undef_op0_nnan(float %x) {
 ; ANY-LABEL: fadd_undef_op0_nnan:
 ; ANY:       # %bb.0:
-; ANY-NEXT:    movss {{.*#+}} xmm0 = mem[0],zero,zero,zero
 ; ANY-NEXT:    retq
   %r = fadd nnan float undef, %x
   ret float %r
 }
 
 define float @fadd_undef_op1_fast(float %x) {
 ; ANY-LABEL: fadd_undef_op1_fast:
 ; ANY:       # %bb.0:
-; ANY-NEXT:    movss {{.*#+}} xmm0 = mem[0],zero,zero,zero
 ; ANY-NEXT:    retq
   %r = fadd fast float %x, undef
   ret float %r
 }
 
 define float @fsub_undef_op0_fast(float %x) {
 ; ANY-LABEL: fsub_undef_op0_fast:
 ; ANY:       # %bb.0:
-; ANY-NEXT:    movss {{.*#+}} xmm0 = mem[0],zero,zero,zero
 ; ANY-NEXT:    retq
   %r = fsub fast float undef, %x
   ret float %r
 }
 
 define float @fsub_undef_op1_nnan(float %x) {
 ; ANY-LABEL: fsub_undef_op1_nnan:
 ; ANY:       # %bb.0:
-; ANY-NEXT:    movss {{.*#+}} xmm0 = mem[0],zero,zero,zero
 ; ANY-NEXT:    retq
   %r = fsub nnan float %x, undef
   ret float %r
 }
 
 define float @fmul_undef_op0_nnan(float %x) {
 ; ANY-LABEL: fmul_undef_op0_nnan:
 ; ANY:       # %bb.0:
-; ANY-NEXT:    movss {{.*#+}} xmm0 = mem[0],zero,zero,zero
 ; ANY-NEXT:    retq
   %r = fmul nnan float undef, %x
   ret float %r
 }
 
 define float @fmul_undef_op1_fast(float %x) {
 ; ANY-LABEL: fmul_undef_op1_fast:
 ; ANY:       # %bb.0:
-; ANY-NEXT:    movss {{.*#+}} xmm0 = mem[0],zero,zero,zero
 ; ANY-NEXT:    retq
   %r = fmul fast float %x, undef
   ret float %r
 }
 
 define float @fdiv_undef_op0_fast(float %x) {
 ; ANY-LABEL: fdiv_undef_op0_fast:
 ; ANY:       # %bb.0:
-; ANY-NEXT:    movss {{.*#+}} xmm0 = mem[0],zero,zero,zero
 ; ANY-NEXT:    retq
   %r = fdiv fast float undef, %x
   ret float %r
 }
 
 define float @fdiv_undef_op1_nnan(float %x) {
 ; ANY-LABEL: fdiv_undef_op1_nnan:
 ; ANY:       # %bb.0:
-; ANY-NEXT:    movss {{.*#+}} xmm0 = mem[0],zero,zero,zero
 ; ANY-NEXT:    retq
   %r = fdiv nnan float %x, undef
   ret float %r
 }
 
 define float @frem_undef_op0_nnan(float %x) {
 ; ANY-LABEL: frem_undef_op0_nnan:
 ; ANY:       # %bb.0:
-; ANY-NEXT:    movss {{.*#+}} xmm0 = mem[0],zero,zero,zero
 ; ANY-NEXT:    retq
   %r = frem nnan float undef, %x
   ret float %r
 }
 
 define float @frem_undef_op1_fast(float %x) {
 ; ANY-LABEL: frem_undef_op1_fast:
 ; ANY:       # %bb.0:
-; ANY-NEXT:    movss {{.*#+}} xmm0 = mem[0],zero,zero,zero
 ; ANY-NEXT:    retq
   %r = frem fast float %x, undef
   ret float %r
 }
 
 ; Constant folding - undef undef.
 
 define double @fadd_undef_undef(double %x) {
; ANY-NEXT:    retq
   ret double %r
 }
 
 ; Constant folding.
 
 define float @fadd_undef_op0_nnan_constant(float %x) {
 ; ANY-LABEL: fadd_undef_op0_nnan_constant:
 ; ANY:       # %bb.0:
-; ANY-NEXT:    movss {{.*#+}} xmm0 = mem[0],zero,zero,zero
 ; ANY-NEXT:    retq
   %r = fadd nnan float undef, 1.0
   ret float %r
 }
 
 define float @fadd_undef_op1_constant(float %x) {
 ; ANY-LABEL: fadd_undef_op1_constant:
 ; ANY:       # %bb.0:
 ; ANY-NEXT:    movss {{.*#+}} xmm0 = mem[0],zero,zero,zero
 ; ANY-NEXT:    retq
   %r = fadd float 2.0, undef
   ret float %r
 }
 
 define float @fsub_undef_op0_fast_constant(float %x) {
 ; ANY-LABEL: fsub_undef_op0_fast_constant:
 ; ANY:       # %bb.0:
-; ANY-NEXT:    movss {{.*#+}} xmm0 = mem[0],zero,zero,zero
 ; ANY-NEXT:    retq
   %r = fsub fast float undef, 3.0
   ret float %r
 }
 
 define float @fsub_undef_op1_constant(float %x) {
 ; ANY-LABEL: fsub_undef_op1_constant:
 ; ANY:       # %bb.0:
 ; ANY-NEXT:    movss {{.*#+}} xmm0 = mem[0],zero,zero,zero
 ; ANY-NEXT:    retq
   %r = fsub float 4.0, undef
   ret float %r
 }
 
 define float @fmul_undef_op0_nnan_constant(float %x) {
 ; ANY-LABEL: fmul_undef_op0_nnan_constant:
 ; ANY:       # %bb.0:
-; ANY-NEXT:    movss {{.*#+}} xmm0 = mem[0],zero,zero,zero
 ; ANY-NEXT:    retq
   %r = fmul nnan float undef, 5.0
   ret float %r
 }
 
 define float @fmul_undef_op1_constant(float %x) {
 ; ANY-LABEL: fmul_undef_op1_constant:
 ; ANY:       # %bb.0:
 ; ANY-NEXT:    movss {{.*#+}} xmm0 = mem[0],zero,zero,zero
 ; ANY-NEXT:    retq
   %r = fmul float 6.0, undef
   ret float %r
 }
 
 define float @fdiv_undef_op0_fast_constant(float %x) {
 ; ANY-LABEL: fdiv_undef_op0_fast_constant:
 ; ANY:       # %bb.0:
-; ANY-NEXT:    movss {{.*#+}} xmm0 = mem[0],zero,zero,zero
 ; ANY-NEXT:    retq
   %r = fdiv fast float undef, 7.0
   ret float %r
 }
 
 define float @fdiv_undef_op1_constant(float %x) {
 ; ANY-LABEL: fdiv_undef_op1_constant:
 ; ANY:       # %bb.0:
 ; ANY-NEXT:    movss {{.*#+}} xmm0 = mem[0],zero,zero,zero
 ; ANY-NEXT:    retq
   %r = fdiv float 8.0, undef
   ret float %r
 }
 
 define float @frem_undef_op0_nnan_constant(float %x) {
 ; ANY-LABEL: frem_undef_op0_nnan_constant:
 ; ANY:       # %bb.0:
-; ANY-NEXT:    movss {{.*#+}} xmm0 = mem[0],zero,zero,zero
 ; ANY-NEXT:    retq
   %r = frem nnan float undef, 9.0
   ret float %r
 }
 
 define float @frem_undef_op1_constant(float %x) {
 ; ANY-LABEL: frem_undef_op1_constant:
 ; ANY:       # %bb.0:

 ; ANY-NEXT:    retq
   %r = fadd double undef, 0x7FF8000000000000
   ret double %r
 }
 
 define double @fadd_undef_op1_fast_constant_nan(double %x) {
 ; ANY-LABEL: fadd_undef_op1_fast_constant_nan:
 ; ANY:       # %bb.0:
-; ANY-NEXT:    movsd {{.*#+}} xmm0 = mem[0],zero
 ; ANY-NEXT:    retq
   %r = fadd fast double 0xFFF0000000000001, undef
   ret double %r
 }
 
 define double @fsub_undef_op0_constant_nan(double %x) {
 ; ANY-LABEL: fsub_undef_op0_constant_nan:
 ; ANY:       # %bb.0:
 ; ANY-NEXT:    movsd {{.*#+}} xmm0 = mem[0],zero
 ; ANY-NEXT:    retq
   %r = fsub double undef, 0xFFF8000000000010
   ret double %r
 }
 
 define double @fsub_undef_op1_nnan_constant_nan(double %x) {
 ; ANY-LABEL: fsub_undef_op1_nnan_constant_nan:
 ; ANY:       # %bb.0:
-; ANY-NEXT:    movsd {{.*#+}} xmm0 = mem[0],zero
 ; ANY-NEXT:    retq
   %r = fsub nnan double 0x7FF0000000000011, undef
   ret double %r
 }
 
 define double @fmul_undef_op0_constant_nan(double %x) {
 ; ANY-LABEL: fmul_undef_op0_constant_nan:
 ; ANY:       # %bb.0:
 ; ANY-NEXT:    movsd {{.*#+}} xmm0 = mem[0],zero
 ; ANY-NEXT:    retq
   %r = fmul double undef, 0x7FF8000000000100
   ret double %r
 }
 
 define double @fmul_undef_op1_fast_constant_nan(double %x) {
 ; ANY-LABEL: fmul_undef_op1_fast_constant_nan:
 ; ANY:       # %bb.0:
-; ANY-NEXT:    movsd {{.*#+}} xmm0 = mem[0],zero
 ; ANY-NEXT:    retq
   %r = fmul fast double 0xFFF0000000000101, undef
   ret double %r
 }
 
 define double @fdiv_undef_op0_constant_nan(double %x) {
 ; ANY-LABEL: fdiv_undef_op0_constant_nan:
 ; ANY:       # %bb.0:
 ; ANY-NEXT:    movsd {{.*#+}} xmm0 = mem[0],zero
 ; ANY-NEXT:    retq
   %r = fdiv double undef, 0xFFF8000000000110
   ret double %r
 }
 
 define double @fdiv_undef_op1_nnan_constant_nan(double %x) {
 ; ANY-LABEL: fdiv_undef_op1_nnan_constant_nan:
 ; ANY:       # %bb.0:
-; ANY-NEXT:    movsd {{.*#+}} xmm0 = mem[0],zero
 ; ANY-NEXT:    retq
   %r = fdiv nnan double 0x7FF0000000000111, undef
   ret double %r
 }
 
 define double @frem_undef_op0_constant_nan(double %x) {
 ; ANY-LABEL: frem_undef_op0_constant_nan:
 ; ANY:       # %bb.0:
 ; ANY-NEXT:    movsd {{.*#+}} xmm0 = mem[0],zero
 ; ANY-NEXT:    retq
   %r = frem double undef, 0x7FF8000000001000
   ret double %r
 }
 
 define double @frem_undef_op1_fast_constant_nan(double %x) {
 ; ANY-LABEL: frem_undef_op1_fast_constant_nan:
 ; ANY:       # %bb.0:
-; ANY-NEXT:    movsd {{.*#+}} xmm0 = mem[0],zero
 ; ANY-NEXT:    retq
   %r = frem fast double 0xFFF0000000001001, undef
   ret double %r
 }
 
 ; Constant folding - special constants: Inf.
 
 define double @fadd_undef_op0_constant_inf(double %x) {
 ; ANY-LABEL: fadd_undef_op0_constant_inf:
 ; ANY:       # %bb.0:
 ; ANY-NEXT:    movsd {{.*#+}} xmm0 = mem[0],zero
 ; ANY-NEXT:    retq
   %r = fadd double undef, 0x7FF0000000000000
   ret double %r
 }
 
 define double @fadd_undef_op1_fast_constant_inf(double %x) {
 ; ANY-LABEL: fadd_undef_op1_fast_constant_inf:
 ; ANY:       # %bb.0:
-; ANY-NEXT:    movsd {{.*#+}} xmm0 = mem[0],zero
 ; ANY-NEXT:    retq
   %r = fadd fast double 0xFFF0000000000000, undef
   ret double %r
 }
 
 define double @fsub_undef_op0_constant_inf(double %x) {
 ; ANY-LABEL: fsub_undef_op0_constant_inf:
 ; ANY:       # %bb.0:
 ; ANY-NEXT:    movsd {{.*#+}} xmm0 = mem[0],zero
 ; ANY-NEXT:    retq
   %r = fsub double undef, 0xFFF0000000000000
   ret double %r
 }
 
 define double @fsub_undef_op1_ninf_constant_inf(double %x) {
 ; ANY-LABEL: fsub_undef_op1_ninf_constant_inf:
 ; ANY:       # %bb.0:
-; ANY-NEXT:    movsd {{.*#+}} xmm0 = mem[0],zero
 ; ANY-NEXT:    retq
   %r = fsub ninf double 0x7FF0000000000000, undef
   ret double %r
 }
 
 define double @fmul_undef_op0_constant_inf(double %x) {
 ; ANY-LABEL: fmul_undef_op0_constant_inf:
 ; ANY:       # %bb.0:
 ; ANY-NEXT:    movsd {{.*#+}} xmm0 = mem[0],zero
 ; ANY-NEXT:    retq
   %r = fmul double undef, 0x7FF0000000000000
   ret double %r
 }
 
 define double @fmul_undef_op1_fast_constant_inf(double %x) {
 ; ANY-LABEL: fmul_undef_op1_fast_constant_inf:
 ; ANY:       # %bb.0:
-; ANY-NEXT:    movsd {{.*#+}} xmm0 = mem[0],zero
 ; ANY-NEXT:    retq
   %r = fmul fast double 0xFFF0000000000000, undef
   ret double %r
 }
 
 define double @fdiv_undef_op0_constant_inf(double %x) {
 ; ANY-LABEL: fdiv_undef_op0_constant_inf:
 ; ANY:       # %bb.0:
 ; ANY-NEXT:    movsd {{.*#+}} xmm0 = mem[0],zero
 ; ANY-NEXT:    retq
   %r = fdiv double undef, 0xFFF0000000000000
   ret double %r
 }
 
 define double @fdiv_undef_op1_ninf_constant_inf(double %x) {
 ; ANY-LABEL: fdiv_undef_op1_ninf_constant_inf:
 ; ANY:       # %bb.0:
-; ANY-NEXT:    movsd {{.*#+}} xmm0 = mem[0],zero
 ; ANY-NEXT:    retq
   %r = fdiv ninf double 0x7FF0000000000000, undef
   ret double %r
 }
 
 define double @frem_undef_op0_constant_inf(double %x) {
 ; ANY-LABEL: frem_undef_op0_constant_inf:
 ; ANY:       # %bb.0:
 ; ANY-NEXT:    movsd {{.*#+}} xmm0 = mem[0],zero
 ; ANY-NEXT:    retq
   %r = frem double undef, 0x7FF0000000000000
   ret double %r
 }
 
 define double @frem_undef_op1_fast_constant_inf(double %x) {
 ; ANY-LABEL: frem_undef_op1_fast_constant_inf:
 ; ANY:       # %bb.0:
-; ANY-NEXT:    movsd {{.*#+}} xmm0 = mem[0],zero
 ; ANY-NEXT:    retq
   %r = frem fast double 0xFFF0000000000000, undef
   ret double %r
 }
 
 define <2 x double> @fadd_undef_op1_constant_vec(<2 x double> %x) {
 ; ANY-LABEL: fadd_undef_op1_constant_vec:
 ; ANY:       # %bb.0: