Fix denormal-fp-math flag and attribute interaction

[arsenm]

Summary

Make these behave the same way unsafe-fp-math and co. The command line
flag should add the attribute to functions that do not already have
it, and leave existing attributes. The attribute is the actual
implementation, but the flag is useful in some testing situations.

AMDGPU has a variety of tests with denormals enabled/disabled that
would require a painful level of test duplication without a flag. This
doesn't expose setting the separate input/output modes, or add a flag
for the f32 version yet.

Tests will be included in future patch.

[arsenm]

ping

[arsenm]

ping

[arsenm]

Rebase and fix patch split

[echristo]

This is wanting a command line flag rather than setting it on the function, or rather in addition to being able to set it on the function? Also "this" in the commit message isn't quite clear. Having this be a separate set of both global function attributes and different than the other fp-math attributes means it really should be called out fairly explicitly why they need to be different, what the semantics are, etc.

[arsenm]

In D71353#1913550, @echristo wrote:

This is wanting a command line flag rather than setting it on the function, or rather in addition to being able to set it on the function? Also "this" in the commit message isn't quite clear. Having this be a separate set of both global function attributes and different than the other fp-math attributes means it really should be called out fairly explicitly why they need to be different, what the semantics are, etc.

This makes the denormal handling consistent with the fp-math attributes. We previously had inconsistent behaviors, but now the flags should never override an existing attribute

[arsenm]

ping

[echristo]

So I'm still concerned that this is on TargetOptions at all. Ideally for new attributes like this we'd only put them on the IR - at the way you've got it we're making resetTargetOptions even harder to remove. I don't think this is the right path forward (as removing resetTargetOptions is the right path) and this is only making that harder.

[arsenm]

Drop parts threading through TargetOptiions like the other flags

[arsenm]

ping

[arsenm]

Rebase

[arsenm]

ping

[echristo]

Sure. I'd prefer we remove it from the struct completely, but that can happen later.

[arsenm]

0ab5b5b8581d9f2951575f7245824e6e4fc57dec

llvm/include/llvm/CodeGen/CommandFlags.h

 //===-- CommandFlags.h - Command Line Flags Interface -----------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file contains codegen-specific flags that are shared between different
 // command line tools. The tools "llc" and "opt" both use this file to prevent
 // flag duplication.
 //
 //===----------------------------------------------------------------------===//
 
+#include "llvm/ADT/FloatingPointMode.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/Module.h"
 #include "llvm/MC/MCTargetOptionsCommandFlags.h"
 #include "llvm/MC/SubtargetFeature.h"
 #include "llvm/Support/CodeGen.h"
 #include "llvm/Support/CommandLine.h"

 bool getEnableNoInfsFPMath();
 
 bool getEnableNoNaNsFPMath();
 
 bool getEnableNoSignedZerosFPMath();
 
 bool getEnableNoTrappingFPMath();
 
-llvm::FPDenormal::DenormalMode getDenormalFPMath();
+DenormalMode::DenormalModeKind getDenormalFPMath();
 
 bool getEnableHonorSignDependentRoundingFPMath();
 
 llvm::FloatABI::ABIType getFloatABIForCalls();
 
 llvm::FPOpFusion::FPOpFusionMode getFuseFPOps();
 
 bool getDontPlaceZerosInBSS();

llvm/include/llvm/Target/TargetOptions.h

 //===-- llvm/Target/TargetOptions.h - Target Options ------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines command line option flags that are shared across various
 // targets.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_TARGET_TARGETOPTIONS_H
 #define LLVM_TARGET_TARGETOPTIONS_H
 
+#include "llvm/ADT/FloatingPointMode.h"
 #include "llvm/MC/MCTargetOptions.h"
 
 #include <memory>
 
 namespace llvm {
+  struct fltSemantics;
   class MachineFunction;
   class MemoryBuffer;
   class Module;
 
   namespace FloatABI {
     enum ABIType {
       Default, // Target-specific (either soft or hard depending on triple, etc).
       Soft,    // Soft float.
namespace llvm {
 
   namespace ThreadModel {
     enum Model {
       POSIX,  // POSIX Threads
       Single  // Single Threaded Environment
     };
   }
 
-  namespace FPDenormal {
-    enum DenormalMode {
-      IEEE,           // IEEE 754 denormal numbers
-      PreserveSign,   // the sign of a flushed-to-zero number is preserved in
-                      // the sign of 0
-      PositiveZero    // denormals are flushed to positive zero
-    };
-  }
-
   enum class BasicBlockSection {
     All,    // Use Basic Block Sections for all basic blocks.  A section
             // for every basic block can significantly bloat object file sizes.
     List,   // Get list of functions & BBs from a file. Selectively enables
             // basic block sections for a subset of basic blocks which can be
             // used to control object size bloats from creating sections.
     Labels, // Do not use Basic Block Sections but label basic blocks.  This
             // is useful when associating profile counts from virtual addresses
TargetOptions()
           DisableIntegratedAS(false), RelaxELFRelocations(false),
           FunctionSections(false), DataSections(false),
           UniqueSectionNames(true), UniqueBBSectionNames(false),
           TrapUnreachable(false), NoTrapAfterNoreturn(false), TLSSize(0),
           EmulatedTLS(false), ExplicitEmulatedTLS(false), EnableIPRA(false),
           EmitStackSizeSection(false), EnableMachineOutliner(false),
           SupportsDefaultOutlining(false), EmitAddrsig(false),
           EmitCallSiteInfo(false), EnableDebugEntryValues(false),
-          ForceDwarfFrameSection(false) {}
+          ForceDwarfFrameSection(false),
+          FPDenormalMode(DenormalMode::IEEE, DenormalMode::IEEE) {}
 
     /// PrintMachineCode - This flag is enabled when the -print-machineinstrs
     /// option is specified on the command line, and should enable debugging
     /// output from the code generator.
     unsigned PrintMachineCode : 1;
 
     /// DisableFramePointerElim - This returns true if frame pointer elimination
     /// optimization should be disabled for the given machine function.
public:
     ThreadModel::Model ThreadModel = ThreadModel::POSIX;
 
     /// EABIVersion - This flag specifies the EABI version
     EABI EABIVersion = EABI::Default;
 
     /// Which debugger to tune for.
     DebuggerKind DebuggerTuning = DebuggerKind::Default;
 
-    /// FPDenormalMode - This flags specificies which denormal numbers the code
-    /// is permitted to require.
-    FPDenormal::DenormalMode FPDenormalMode = FPDenormal::IEEE;
+  private:
+    /// Flushing mode to assume in default FP environment.
+    DenormalMode FPDenormalMode;
+
+    /// Flushing mode to assume in default FP environment, for float/vector of
+    /// float.
+    DenormalMode FP32DenormalMode;
+
+  public:
+    void setFPDenormalMode(DenormalMode Mode) {
+      FPDenormalMode = Mode;
+    }
+
+    void setFP32DenormalMode(DenormalMode Mode) {
+      FP32DenormalMode = Mode;
+    }
+
+    DenormalMode getRawFPDenormalMode() const {
+      return FPDenormalMode;
+    }
+
+    DenormalMode getRawFP32DenormalMode() const {
+      return FP32DenormalMode;
+    }
+
+    DenormalMode getDenormalMode(const fltSemantics &FPType) const;
 
     /// What exception model to use
     ExceptionHandling ExceptionModel = ExceptionHandling::None;
 
     /// Machine level options.
     MCTargetOptions MCOptions;
   };
 
 } // End llvm namespace
 
 #endif

llvm/lib/CodeGen/CommandFlags.cpp

 CGOPT(ExceptionHandling, ExceptionModel)
 CGOPT_EXP(CodeGenFileType, FileType)
 CGOPT(FramePointer::FP, FramePointerUsage)
 CGOPT(bool, EnableUnsafeFPMath)
 CGOPT(bool, EnableNoInfsFPMath)
 CGOPT(bool, EnableNoNaNsFPMath)
 CGOPT(bool, EnableNoSignedZerosFPMath)
 CGOPT(bool, EnableNoTrappingFPMath)
-CGOPT(FPDenormal::DenormalMode, DenormalFPMath)
+CGOPT(DenormalMode::DenormalModeKind, DenormalFPMath)
 CGOPT(bool, EnableHonorSignDependentRoundingFPMath)
 CGOPT(FloatABI::ABIType, FloatABIForCalls)
 CGOPT(FPOpFusion::FPOpFusionMode, FuseFPOps)
 CGOPT(bool, DontPlaceZerosInBSS)
 CGOPT(bool, EnableGuaranteedTailCallOpt)
 CGOPT(bool, DisableTailCalls)
 CGOPT(bool, StackSymbolOrdering)
 CGOPT(unsigned, OverrideStackAlignment)
#define CGBINDOPT(NAME)                                                        \
 
   static 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));
   CGBINDOPT(EnableNoTrappingFPMath);
 
-  static cl::opt<FPDenormal::DenormalMode> DenormalFPMath(
+  static cl::opt<DenormalMode::DenormalModeKind> DenormalFPMath(
       "denormal-fp-math",
       cl::desc(
           "Select which denormal numbers the code is permitted to require"),
-      cl::init(FPDenormal::IEEE),
+      cl::init(DenormalMode::IEEE),
       cl::values(
-          clEnumValN(FPDenormal::IEEE, "ieee", "IEEE 754 denormal numbers"),
-          clEnumValN(FPDenormal::PreserveSign, "preserve-sign",
+          clEnumValN(DenormalMode::IEEE, "ieee", "IEEE 754 denormal numbers"),
+          clEnumValN(DenormalMode::PreserveSign, "preserve-sign",
                      "the sign of a  flushed-to-zero number is preserved "
                      "in the sign of 0"),
-          clEnumValN(FPDenormal::PositiveZero, "positive-zero",
+          clEnumValN(DenormalMode::PositiveZero, "positive-zero",
                      "denormals are flushed to positive zero")));
   CGBINDOPT(DenormalFPMath);
 
   static cl::opt<bool> EnableHonorSignDependentRoundingFPMath(
       "enable-sign-dependent-rounding-fp-math", cl::Hidden,
       cl::desc("Force codegen to assume rounding mode can change dynamically"),
       cl::init(false));
   CGBINDOPT(EnableHonorSignDependentRoundingFPMath);

 TargetOptions codegen::InitTargetOptionsFromCodeGenFlags() {
   TargetOptions Options;
   Options.AllowFPOpFusion = getFuseFPOps();
   Options.UnsafeFPMath = getEnableUnsafeFPMath();
   Options.NoInfsFPMath = getEnableNoInfsFPMath();
   Options.NoNaNsFPMath = getEnableNoNaNsFPMath();
   Options.NoSignedZerosFPMath = getEnableNoSignedZerosFPMath();
   Options.NoTrappingFPMath = getEnableNoTrappingFPMath();
-  Options.FPDenormalMode = getDenormalFPMath();
+
+  DenormalMode::DenormalModeKind DenormKind = getDenormalFPMath();
+
+// FIXME: Should have separate input and output flags
+  Options.setFPDenormalMode(DenormalMode(DenormKind, DenormKind));
+
   Options.HonorSignDependentRoundingFPMathOption =
       getEnableHonorSignDependentRoundingFPMath();
   if (getFloatABIForCalls() != FloatABI::Default)
     Options.FloatABIType = getFloatABIForCalls();
   Options.NoZerosInBSS = getDontPlaceZerosInBSS();
   Options.GuaranteedTailCallOpt = getEnableGuaranteedTailCallOpt();
   Options.StackAlignmentOverride = getOverrideStackAlignment();
   Options.StackSymbolOrdering = getStackSymbolOrdering();
void codegen::setFunctionAttributes(StringRef CPU, StringRef Features,
   if (getStackRealign())
     NewAttrs.addAttribute("stackrealign");
 
   HANDLE_BOOL_ATTR(EnableUnsafeFPMathView, "unsafe-fp-math");
   HANDLE_BOOL_ATTR(EnableNoInfsFPMathView, "no-infs-fp-math");
   HANDLE_BOOL_ATTR(EnableNoNaNsFPMathView, "no-nans-fp-math");
   HANDLE_BOOL_ATTR(EnableNoSignedZerosFPMathView, "no-signed-zeros-fp-math");
 
+  if (DenormalFPMathView->getNumOccurrences() > 0 &&
+      !F.hasFnAttribute("denormal-fp-math")) {
+    DenormalMode::DenormalModeKind DenormKind = getDenormalFPMath();
+
+    // FIXME: Command line flag should expose separate input/output modes.
+    NewAttrs.addAttribute("denormal-fp-math",
+                          DenormalMode(DenormKind, DenormKind).str());
+  }
+
   if (TrapFuncNameView->getNumOccurrences() > 0)
     for (auto &B : F)
       for (auto &I : B)
         if (auto *Call = dyn_cast<CallInst>(&I))
           if (const auto *F = Call->getCalledFunction())
             if (F->getIntrinsicID() == Intrinsic::debugtrap ||
                 F->getIntrinsicID() == Intrinsic::trap)
               Call->addAttribute(

llvm/lib/Target/ARM/ARMAsmPrinter.cpp

void ARMAsmPrinter::emitAttributes() {
   } else {
     ATS.emitAttribute(ARMBuildAttrs::ABI_PCS_GOT_use,
                       ARMBuildAttrs::AddressDirect);
   }
 
   // Set FP Denormals.
   if (checkDenormalAttributeConsistency(*MMI->getModule(),
                                         "denormal-fp-math",
-                                        DenormalMode::getPreserveSign()) ||
-      TM.Options.FPDenormalMode == FPDenormal::PreserveSign)
+                                        DenormalMode::getPreserveSign()))
     ATS.emitAttribute(ARMBuildAttrs::ABI_FP_denormal,
                       ARMBuildAttrs::PreserveFPSign);
   else if (checkDenormalAttributeConsistency(*MMI->getModule(),
                                              "denormal-fp-math",
-                                             DenormalMode::getPositiveZero()) ||
-           TM.Options.FPDenormalMode == FPDenormal::PositiveZero)
+                                             DenormalMode::getPositiveZero()))
     ATS.emitAttribute(ARMBuildAttrs::ABI_FP_denormal,
                       ARMBuildAttrs::PositiveZero);
   else if (!TM.Options.UnsafeFPMath)
     ATS.emitAttribute(ARMBuildAttrs::ABI_FP_denormal,
                       ARMBuildAttrs::IEEEDenormals);
   else {
     if (!STI.hasVFP2Base()) {
       // When the target doesn't have an FPU (by design or