[clang] 7fe9435 - Work on cleaning up denormal mode handling

Matt Arsenault via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 19 08:32:01 PST 2019


Author: Matt Arsenault
Date: 2019-11-19T22:01:14+05:30
New Revision: 7fe9435dc88050ee78eb1d4adec87610dce468f7

URL: https://github.com/llvm/llvm-project/commit/7fe9435dc88050ee78eb1d4adec87610dce468f7
DIFF: https://github.com/llvm/llvm-project/commit/7fe9435dc88050ee78eb1d4adec87610dce468f7.diff

LOG: Work on cleaning up denormal mode handling

Cleanup handling of the denormal-fp-math attribute. Consolidate places
checking the allowed names in one place.

This is in preparation for introducing FP type specific variants of
the denormal-fp-mode attribute. AMDGPU will switch to using this in
place of the current hacky use of subtarget features for the denormal
mode.

Introduce a new header for dealing with FP modes. The constrained
intrinsic classes define related enums that should also be moved into
this header for uses in other contexts.

The verifier could use a check to make sure the denorm-fp-mode
attribute is sane, but there currently isn't one.

Currently, DAGCombiner incorrectly asssumes non-IEEE behavior by
default in the one current user. Clang must be taught to start
emitting this attribute by default to avoid regressions when this is
switched to assume ieee behavior if the attribute isn't present.

Added: 
    llvm/include/llvm/ADT/FloatingPointMode.h
    llvm/unittests/ADT/FloatingPointMode.cpp

Modified: 
    clang/include/clang/Basic/CodeGenOptions.h
    clang/lib/CodeGen/CGCall.cpp
    clang/lib/Frontend/CompilerInvocation.cpp
    llvm/include/llvm/CodeGen/MachineFunction.h
    llvm/include/llvm/CodeGen/SelectionDAG.h
    llvm/lib/CodeGen/MachineFunction.cpp
    llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
    llvm/unittests/ADT/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/CodeGenOptions.h b/clang/include/clang/Basic/CodeGenOptions.h
index f672c7df7d53..900620a39292 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -16,6 +16,7 @@
 #include "clang/Basic/DebugInfoOptions.h"
 #include "clang/Basic/Sanitizers.h"
 #include "clang/Basic/XRayInstr.h"
+#include "llvm/ADT/FloatingPointMode.h"
 #include "llvm/Support/CodeGen.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Target/TargetOptions.h"
@@ -163,7 +164,7 @@ class CodeGenOptions : public CodeGenOptionsBase {
   std::string FloatABI;
 
   /// The floating-point denormal mode to use.
-  std::string FPDenormalMode;
+  llvm::DenormalMode FPDenormalMode = llvm::DenormalMode::Invalid;
 
   /// The float precision limit to use, if non-empty.
   std::string LimitFloatPrecision;

diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index e832e4c28334..dc4c5ca2159f 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -1741,8 +1741,9 @@ void CodeGenModule::ConstructDefaultFnAttrList(StringRef Name, bool HasOptnone,
 
     if (CodeGenOpts.NullPointerIsValid)
       FuncAttrs.addAttribute("null-pointer-is-valid", "true");
-    if (!CodeGenOpts.FPDenormalMode.empty())
-      FuncAttrs.addAttribute("denormal-fp-math", CodeGenOpts.FPDenormalMode);
+    if (CodeGenOpts.FPDenormalMode != llvm::DenormalMode::Invalid)
+      FuncAttrs.addAttribute("denormal-fp-math",
+                             llvm::denormalModeName(CodeGenOpts.FPDenormalMode));
 
     FuncAttrs.addAttribute("no-trapping-math",
                            llvm::toStringRef(CodeGenOpts.NoTrappingMath));

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 79975722a475..17ef037f3e6d 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -1266,13 +1266,8 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
 
   if (Arg *A = Args.getLastArg(OPT_fdenormal_fp_math_EQ)) {
     StringRef Val = A->getValue();
-    if (Val == "ieee")
-      Opts.FPDenormalMode = "ieee";
-    else if (Val == "preserve-sign")
-      Opts.FPDenormalMode = "preserve-sign";
-    else if (Val == "positive-zero")
-      Opts.FPDenormalMode = "positive-zero";
-    else
+    Opts.FPDenormalMode = llvm::parseDenormalFPAttribute(Val);
+    if (Opts.FPDenormalMode == llvm::DenormalMode::Invalid)
       Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Val;
   }
 

diff  --git a/llvm/include/llvm/ADT/FloatingPointMode.h b/llvm/include/llvm/ADT/FloatingPointMode.h
new file mode 100644
index 000000000000..670b2368da9f
--- /dev/null
+++ b/llvm/include/llvm/ADT/FloatingPointMode.h
@@ -0,0 +1,62 @@
+//===- llvm/Support/FloatingPointMode.h -------------------------*- 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
+//
+//===----------------------------------------------------------------------===//
+//
+// Utilities for dealing with flags related to floating point mode controls.
+//
+//===----------------------------------------------------------------------===/
+
+#ifndef LLVM_FLOATINGPOINTMODE_H
+#define LLVM_FLOATINGPOINTMODE_H
+
+#include "llvm/ADT/StringSwitch.h"
+
+namespace llvm {
+
+/// Represent handled modes for denormal (aka subnormal) modes in the floating
+/// point environment.
+enum class DenormalMode {
+  Invalid = -1,
+
+  /// IEEE-754 denormal numbers preserved.
+  IEEE,
+
+  /// The sign of a flushed-to-zero number is preserved in the sign of 0
+  PreserveSign,
+
+  /// Denormals are flushed to positive zero.
+  PositiveZero
+};
+
+/// Parse the expected names from the denormal-fp-math attribute.
+inline DenormalMode parseDenormalFPAttribute(StringRef Str) {
+  // Assume ieee on unspecified attribute.
+  return StringSwitch<DenormalMode>(Str)
+    .Cases("", "ieee", DenormalMode::IEEE)
+    .Case("preserve-sign", DenormalMode::PreserveSign)
+    .Case("positive-zero", DenormalMode::PositiveZero)
+    .Default(DenormalMode::Invalid);
+}
+
+/// Return the name used for the denormal handling mode used by the the
+/// expected names from the denormal-fp-math attribute.
+inline StringRef denormalModeName(DenormalMode Mode) {
+  switch (Mode) {
+  case DenormalMode::IEEE:
+    return "ieee";
+  case DenormalMode::PreserveSign:
+    return "preserve-sign";
+  case DenormalMode::PositiveZero:
+    return "positive-zero";
+  default:
+    return "";
+  }
+}
+
+}
+
+#endif // LLVM_FLOATINGPOINTMODE_H

diff  --git a/llvm/include/llvm/CodeGen/MachineFunction.h b/llvm/include/llvm/CodeGen/MachineFunction.h
index 57038d239fa2..9eee7a56adb9 100644
--- a/llvm/include/llvm/CodeGen/MachineFunction.h
+++ b/llvm/include/llvm/CodeGen/MachineFunction.h
@@ -20,6 +20,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/FloatingPointMode.h"
 #include "llvm/ADT/GraphTraits.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
@@ -574,6 +575,10 @@ class MachineFunction {
      return const_cast<MachineFunction*>(this)->getInfo<Ty>();
   }
 
+  /// Returns the denormal handling type for the default rounding mode of the
+  /// function.
+  DenormalMode getDenormalMode(const fltSemantics &FPType) const;
+
   /// getBlockNumbered - MachineBasicBlocks are automatically numbered when they
   /// are inserted into the machine function.  The block number for a machine
   /// basic block can be found by using the MBB::getNumber method, this method

diff  --git a/llvm/include/llvm/CodeGen/SelectionDAG.h b/llvm/include/llvm/CodeGen/SelectionDAG.h
index a009418703ee..602671bd6408 100644
--- a/llvm/include/llvm/CodeGen/SelectionDAG.h
+++ b/llvm/include/llvm/CodeGen/SelectionDAG.h
@@ -1711,6 +1711,12 @@ class SelectionDAG {
     return It->second.HeapAllocSite;
   }
 
+  /// Return the current function's default denormal handling kind for the given
+  /// floating point type.
+  DenormalMode getDenormalMode(EVT VT) const {
+    return MF->getDenormalMode(EVTToAPFloatSemantics(VT));
+  }
+
 private:
   void InsertNode(SDNode *N);
   bool RemoveNodeFromCSEMaps(SDNode *N);

diff  --git a/llvm/lib/CodeGen/MachineFunction.cpp b/llvm/lib/CodeGen/MachineFunction.cpp
index b40f0b46bc12..b147a83cd167 100644
--- a/llvm/lib/CodeGen/MachineFunction.cpp
+++ b/llvm/lib/CodeGen/MachineFunction.cpp
@@ -270,6 +270,21 @@ getOrCreateJumpTableInfo(unsigned EntryKind) {
   return JumpTableInfo;
 }
 
+DenormalMode MachineFunction::getDenormalMode(const fltSemantics &FPType) const {
+  // TODO: Should probably avoid the connection to the IR and store directly
+  // in the MachineFunction.
+  Attribute Attr = F.getFnAttribute("denormal-fp-math");
+
+  // FIXME: This should assume IEEE behavior on an unspecified
+  // attribute. However, the one current user incorrectly assumes a non-IEEE
+  // target by default.
+  StringRef Val = Attr.getValueAsString();
+  if (Val.empty())
+    return DenormalMode::Invalid;
+
+  return parseDenormalFPAttribute(Val);
+}
+
 /// Should we be emitting segmented stack stuff for the function
 bool MachineFunction::shouldSplitStack() const {
   return getFunction().hasFnAttribute("split-stack");

diff  --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 2db30279fbdd..9a9229e02fe6 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -20464,9 +20464,8 @@ SDValue DAGCombiner::buildSqrtEstimateImpl(SDValue Op, SDNodeFlags Flags,
         SDLoc DL(Op);
         EVT CCVT = getSetCCResultType(VT);
         ISD::NodeType SelOpcode = VT.isVector() ? ISD::VSELECT : ISD::SELECT;
-        const Function &F = DAG.getMachineFunction().getFunction();
-        Attribute Denorms = F.getFnAttribute("denormal-fp-math");
-        if (Denorms.getValueAsString().equals("ieee")) {
+        DenormalMode DenormMode = DAG.getDenormalMode(VT);
+        if (DenormMode == DenormalMode::IEEE) {
           // fabs(X) < SmallestNormal ? 0.0 : Est
           const fltSemantics &FltSem = DAG.EVTToAPFloatSemantics(VT);
           APFloat SmallestNorm = APFloat::getSmallestNormalized(FltSem);

diff  --git a/llvm/unittests/ADT/CMakeLists.txt b/llvm/unittests/ADT/CMakeLists.txt
index b7acbdc86e65..b5f29b9b99ed 100644
--- a/llvm/unittests/ADT/CMakeLists.txt
+++ b/llvm/unittests/ADT/CMakeLists.txt
@@ -21,6 +21,7 @@ add_llvm_unittest(ADTTests
   EnumeratedArrayTest.cpp
   EquivalenceClassesTest.cpp
   FallibleIteratorTest.cpp
+  FloatingPointMode.cpp
   FoldingSet.cpp
   FunctionExtrasTest.cpp
   FunctionRefTest.cpp

diff  --git a/llvm/unittests/ADT/FloatingPointMode.cpp b/llvm/unittests/ADT/FloatingPointMode.cpp
new file mode 100644
index 000000000000..c0d59823db6c
--- /dev/null
+++ b/llvm/unittests/ADT/FloatingPointMode.cpp
@@ -0,0 +1,33 @@
+//===- llvm/unittest/ADT/FloatingPointMode.cpp ----------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/ADT/FloatingPointMode.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+namespace {
+
+TEST(FloatingPointModeTest, ParseDenormalFPAttribute) {
+  EXPECT_EQ(DenormalMode::IEEE, parseDenormalFPAttribute("ieee"));
+  EXPECT_EQ(DenormalMode::IEEE, parseDenormalFPAttribute(""));
+  EXPECT_EQ(DenormalMode::PreserveSign,
+            parseDenormalFPAttribute("preserve-sign"));
+  EXPECT_EQ(DenormalMode::PositiveZero,
+            parseDenormalFPAttribute("positive-zero"));
+  EXPECT_EQ(DenormalMode::Invalid, parseDenormalFPAttribute("foo"));
+}
+
+TEST(FloatingPointModeTest, DenormalAttributeName) {
+  EXPECT_EQ("ieee", denormalModeName(DenormalMode::IEEE));
+  EXPECT_EQ("preserve-sign", denormalModeName(DenormalMode::PreserveSign));
+  EXPECT_EQ("positive-zero", denormalModeName(DenormalMode::PositiveZero));
+  EXPECT_EQ("", denormalModeName(DenormalMode::Invalid));
+}
+
+}


        


More information about the cfe-commits mailing list