DAG: Recognize no-signed-zeros-fp-math attribute

[arsenm]

Summary

clang already emits this with -cl-no-signed-zeros, but codegen
doesn't do anything with it. Treat it like the other fast math
attributes, and change one place to use it.

[kzhuravl]

include/llvm/Target/TargetOptions.h

163

rezo->zero.

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

632

line exceeds 80 chars.

lib/Target/TargetMachine.cpp

81

This line duplicates the previous one.

[arsenm]

Address feedback

[echristo]

Justin has been looking at these things lately.

[jlebar]

include/llvm/Target/TargetOptions.h

161

"when the -foo flag is specified" or "when -foo is specified"

lib/Target/TargetMachine.cpp

91

Personally I'd prefer to do this in a separate patch, and to make a reasonable attempt at updating our documentation to say that this implies the others. It was only after working on these flags for a few weeks that I even realized that UnsafeFPMath implies the others in IR (and therefore that it should imply the others elsewhere).

I even went as far as to add a separate unsafe-fp-math flag to XLA separate from the fast-math flag. Oops. :)

test/CodeGen/AMDGPU/fsub.ll

115

Can we add a (brief ok) comment explaining why this one gets an xor but the others don't?

[jlebar]

lib/Target/TargetMachine.cpp

91

Oh, I see why you're doing it here -- you don't want to regress existing codegen that sets UnsafeFPMath but not this flag (which is new).

I guess you have to do it this way. We should still do this with the intent to update the docs and set the other flags, though.

[arsenm]

lib/Target/TargetMachine.cpp

91

I would actually prefer to someday split unsafe math into an unsafe algebra flag which does not imply the others for places where you can reassociate without changing nan/inf behavior.

Oddly, I checked what clang does for just -funsafe-math-optimizations. It sets unsurprisingly:
"unsafe-fp-math"="true"
"no-signed-zeros-fp-math"="true"
"no-trapping-math"="true"

But I was surprised it still emits:
"less-precise-fpmad"="false"
"no-infs-fp-math"="false"
"no-nans-fp-math"="false"

but these are enabled by -ffast-math. So from a practical point of view in the current world it doesn't really need to imply it, but it also confuses me about what unsafe-fp-math is really supposed to mean if before it was a stand in for no-signed-zeros.

[jlebar]

lib/Target/TargetMachine.cpp

91

I would actually prefer to someday split unsafe math into an unsafe algebra flag which does not imply the others for places where you can reassociate without changing nan/inf behavior.

I mean, me too. At least, I *think* I want to be able to reassociate / lose precision without assuming that I never produce a NaN or Inf. I *think* that's rather useful, but maybe someone can learn me why it's not.

And clearly the flags are a mess, yikes.

But at an IR level, the only way to set "unsafe-fp-math" on an instruction is with the "fast" attr, and that implies all the the things. I'm also not a fan of pretending that we have more configurability than we actually do...

[arsenm]

Don't have unsafe-fp-math imply no-signed-zeros. Surprisingly only one other test failed besides the ones I was editing in the first place for this

[jlebar]

lgtm with the other outstanding comments addressed.

[arsenm]

r293024

include/llvm/CodeGen/CommandFlags.h

EnableNoInfsFPMath("enable-no-infs-fp-math",
                 cl::init(false));
 
 cl::opt<bool>
 EnableNoNaNsFPMath("enable-no-nans-fp-math",
                    cl::desc("Enable FP math optimizations that assume no NaNs"),
                    cl::init(false));
 
 cl::opt<bool>
+EnableNoSignedZerosFPMath("enable-no-signed-zeros-fp-math",
+                          cl::desc("Enable FP math optimizations that assume "
+                                   "the sign of 0 is insignificant"),
+                          cl::init(false));
+
+cl::opt<bool>
 EnableNoTrappingFPMath("enable-no-trapping-fp-math",
                        cl::desc("Enable setting the FP exceptions build "
                                 "attribute not to use exceptions"),
                        cl::init(false));
 
 cl::opt<llvm::FPDenormal::DenormalMode>
 DenormalMode("denormal-fp-math",
           cl::desc("Select which denormal numbers the code is permitted to require"),

 // a TargetOptions object with CodeGen flags and returns it.
 static inline TargetOptions InitTargetOptionsFromCodeGenFlags() {
   TargetOptions Options;
   Options.LessPreciseFPMADOption = EnableFPMAD;
   Options.AllowFPOpFusion = FuseFPOps;
   Options.UnsafeFPMath = EnableUnsafeFPMath;
   Options.NoInfsFPMath = EnableNoInfsFPMath;
   Options.NoNaNsFPMath = EnableNoNaNsFPMath;
+  Options.NoSignedZerosFPMath = EnableNoSignedZerosFPMath;
   Options.NoTrappingFPMath = EnableNoTrappingFPMath;
   Options.FPDenormalMode = DenormalMode;
   Options.HonorSignDependentRoundingFPMathOption =
       EnableHonorSignDependentRoundingFPMath;
   if (FloatABIForCalls != FloatABI::Default)
     Options.FloatABIType = FloatABIForCalls;
   Options.NoZerosInBSS = DontPlaceZerosInBSS;
   Options.GuaranteedTailCallOpt = EnableGuaranteedTailCallOpt;

include/llvm/Target/TargetOptions.h

public:
     unsigned NoInfsFPMath : 1;
 
     /// NoNaNsFPMath - This flag is enabled when the
     /// -enable-no-nans-fp-math flag is specified on the command line. When
     /// this flag is off (the default), the code generator is not allowed to
     /// assume the FP arithmetic arguments and results are never NaNs.
     unsigned NoNaNsFPMath : 1;
 
-    /// NoTrappingFPMath - This flag is enabled when the 
-    /// -enable-no-trapping-fp-math is specified on the command line. This 
+    /// NoTrappingFPMath - This flag is enabled when the
+    /// -enable-no-trapping-fp-math is specified on the command line. This
     /// specifies that there are no trap handlers to handle exceptions.
     unsigned NoTrappingFPMath : 1;
 
+    /// NoSignedZerosFPMath - This flag is enabled when the
+    /// -enable-no-signed-zeros-fp-math is specified on the command line. This
+    /// specifies that optimizations are allowed to treat the sign of a rezo
+    /// argument or result as insignificant.
+    unsigned NoSignedZerosFPMath : 1;
+
     /// HonorSignDependentRoundingFPMath - This returns true when the
     /// -enable-sign-dependent-rounding-fp-math is specified.  If this returns
     /// false (the default), the code generator is allowed to assume that the
     /// rounding behavior is the default (round-to-zero for all floating point
     /// to integer conversions, and round-to-nearest for all other arithmetic
     /// truncations).  If this is enabled (set to true), the code generator must
     /// assume that the rounding mode may dynamically change.
     unsigned HonorSignDependentRoundingFPMathOption : 1;

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

case ISD::FADD:
     if (char V = isNegatibleForFree(Op.getOperand(0), LegalOperations, TLI,
                                     Options, Depth + 1))
       return V;
     // fold (fneg (fadd A, B)) -> (fsub (fneg B), A)
     return isNegatibleForFree(Op.getOperand(1), LegalOperations, TLI, Options,
                               Depth + 1);
   case ISD::FSUB:
     // We can't turn -(A-B) into B-A when we honor signed zeros.
-    if (!Options->UnsafeFPMath && !Op.getNode()->getFlags()->hasNoSignedZeros())
+    if (!Options->NoSignedZerosFPMath && !Op.getNode()->getFlags()->hasNoSignedZeros())
       return 0;
 
     // fold (fneg (fsub A, B)) -> (fsub B, A)
     return 1;
 
   case ISD::FMUL:
   case ISD::FDIV:
     if (Options->HonorSignDependentRoundingFPMath()) return 0;

lib/Target/TargetMachine.cpp

do {                                                                         \
     else                                                                       \
       Options.X = DefaultOptions.X;                                            \
   } while (0)
 
   RESET_OPTION(LessPreciseFPMADOption, "less-precise-fpmad");
   RESET_OPTION(UnsafeFPMath, "unsafe-fp-math");
   RESET_OPTION(NoInfsFPMath, "no-infs-fp-math");
   RESET_OPTION(NoNaNsFPMath, "no-nans-fp-math");
+  RESET_OPTION(NoNaNsFPMath, "no-nans-fp-math");
   RESET_OPTION(NoTrappingFPMath, "no-trapping-math");
+  RESET_OPTION(NoSignedZerosFPMath, "no-signed-zeros-fp-math");
+
+  if (Options.UnsafeFPMath) {
+    // Should this imply the others?
+    Options.NoSignedZerosFPMath = true;
+  }
 
   StringRef Denormal =
     F.getFnAttribute("denormal-fp-math").getValueAsString();
   if (Denormal == "ieee")
     Options.FPDenormalMode = FPDenormal::IEEE;
   else if (Denormal == "preserve-sign")
     Options.FPDenormalMode = FPDenormal::PreserveSign;
   else if (Denormal == "positive-zero")
     Options.FPDenormalMode = FPDenormal::PositiveZero;
   else
     Options.FPDenormalMode = DefaultOptions.FPDenormalMode;

test/CodeGen/AMDGPU/enable-no-signed-zeros-fp-math.ll

+; RUN: llc -march=amdgcn -enable-no-signed-zeros-fp-math=0 < %s | FileCheck -check-prefix=GCN -check-prefix=GCN-SAFE %s
+; RUN: llc -march=amdgcn -enable-no-signed-zeros-fp-math=1 < %s | FileCheck -check-prefix=GCN -check-prefix=GCN-UNSAFE %s
+; RUN: llc -march=amdgcn -enable-unsafe-fp-math < %s | FileCheck -check-prefix=GCN -check-prefix=GCN-UNSAFE %s
+
+; Test that the -enable-no-signed-zeros-fp-math flag works
+
+; GCN-LABEL: {{^}}fneg_fsub_f32:
+; GCN: v_subrev_f32_e32 [[SUB:v[0-9]+]], {{v[0-9]+}}, {{v[0-9]+}}
+; GCN-SAFE: v_xor_b32_e32 v{{[0-9]+}}, 0x80000000, [[SUB]]
+
+; GCN-UNSAFE-NOT: xor
+define void @fneg_fsub_f32(float addrspace(1)* %out, float addrspace(1)* %in) #0 {
+  %b_ptr = getelementptr float, float addrspace(1)* %in, i32 1
+  %a = load float, float addrspace(1)* %in, align 4
+  %b = load float, float addrspace(1)* %b_ptr, align 4
+  %result = fsub float %a, %b
+  %neg.result = fsub float -0.0, %result
+  store float %neg.result, float addrspace(1)* %out, align 4
+  ret void
+}
+
+attributes #0 = { nounwind }

test/CodeGen/AMDGPU/fsub.ll

 ; SI: v_subrev_f32_e32 {{v[0-9]+}}, {{s[0-9]+}}, {{v[0-9]+}}
 ; SI: v_subrev_f32_e32 {{v[0-9]+}}, {{s[0-9]+}}, {{v[0-9]+}}
 ; SI: s_endpgm
 define void @s_fsub_v4f32(<4 x float> addrspace(1)* %out, <4 x float> %a, <4 x float> %b) {
   %result = fsub <4 x float> %a, %b
   store <4 x float> %result, <4 x float> addrspace(1)* %out, align 16
   ret void
 }
+
+
+; FUNC-LABEL: {{^}}v_fneg_fsub_f32:
+; SI: v_subrev_f32_e32 [[SUB:v[0-9]+]], {{v[0-9]+}}, {{v[0-9]+}}
+; SI: v_xor_b32_e32 v{{[0-9]+}}, 0x80000000, [[SUB]]
+define void @v_fneg_fsub_f32(float addrspace(1)* %out, float addrspace(1)* %in) {
+  %b_ptr = getelementptr float, float addrspace(1)* %in, i32 1
+  %a = load float, float addrspace(1)* %in, align 4
+  %b = load float, float addrspace(1)* %b_ptr, align 4
+  %result = fsub float %a, %b
+  %neg.result = fsub float -0.0, %result
+  store float %neg.result, float addrspace(1)* %out, align 4
+  ret void
+}
+
+; FUNC-LABEL: {{^}}v_fneg_fsub_nsz_f32:
+; SI: v_subrev_f32_e32 [[SUB:v[0-9]+]], {{v[0-9]+}}, {{v[0-9]+}}
+; SI-NOT: xor
+define void @v_fneg_fsub_nsz_f32(float addrspace(1)* %out, float addrspace(1)* %in) {
+  %b_ptr = getelementptr float, float addrspace(1)* %in, i32 1
+  %a = load float, float addrspace(1)* %in, align 4
+  %b = load float, float addrspace(1)* %b_ptr, align 4
+  %result = fsub nsz float %a, %b
+  %neg.result = fsub float -0.0, %result
+  store float %neg.result, float addrspace(1)* %out, align 4
+  ret void
+}
+
+; FUNC-LABEL: {{^}}v_fneg_fsub_nsz_attribute_f32:
+; SI: v_subrev_f32_e32 [[SUB:v[0-9]+]], {{v[0-9]+}}, {{v[0-9]+}}
+; SI-NOT: xor
+define void @v_fneg_fsub_nsz_attribute_f32(float addrspace(1)* %out, float addrspace(1)* %in) #0 {
+  %b_ptr = getelementptr float, float addrspace(1)* %in, i32 1
+  %a = load float, float addrspace(1)* %in, align 4
+  %b = load float, float addrspace(1)* %b_ptr, align 4
+  %result = fsub float %a, %b
+  %neg.result = fsub float -0.0, %result
+  store float %neg.result, float addrspace(1)* %out, align 4
+  ret void
+}
+
+; FUNC-LABEL: {{^}}v_fneg_fsub_nsz_false_attribute_f32:
+; SI: v_subrev_f32_e32 [[SUB:v[0-9]+]], {{v[0-9]+}}, {{v[0-9]+}}
+; SI: v_xor_b32_e32 v{{[0-9]+}}, 0x80000000, [[SUB]]
+define void @v_fneg_fsub_nsz_false_attribute_f32(float addrspace(1)* %out, float addrspace(1)* %in) #1 {
+  %b_ptr = getelementptr float, float addrspace(1)* %in, i32 1
+  %a = load float, float addrspace(1)* %in, align 4
+  %b = load float, float addrspace(1)* %b_ptr, align 4
+  %result = fsub float %a, %b
+  %neg.result = fsub float -0.0, %result
+  store float %neg.result, float addrspace(1)* %out, align 4
+  ret void
+}
+
+attributes #0 = { nounwind "no-signed-zeros-fp-math"="true" }
+attributes #1 = { nounwind "no-signed-zeros-fp-math"="false" }