From 00e1bbce3542ae07cfb8f6091bee3b623f8bb84d Mon Sep 17 00:00:00 2001 From: Matt Arsenault Date: Tue, 29 Oct 2019 16:16:05 -0700 Subject: [PATCH] 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. --- include/llvm/ADT/FloatingPointMode.h | 62 ++++++++++++++++++++++++ include/llvm/CodeGen/MachineFunction.h | 5 ++ include/llvm/CodeGen/SelectionDAG.h | 6 +++ lib/CodeGen/MachineFunction.cpp | 15 ++++++ lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 5 +- unittests/ADT/CMakeLists.txt | 1 + unittests/ADT/FloatingPointMode.cpp | 33 +++++++++++++ 7 files changed, 124 insertions(+), 3 deletions(-) create mode 100644 include/llvm/ADT/FloatingPointMode.h create mode 100644 unittests/ADT/FloatingPointMode.cpp diff --git a/include/llvm/ADT/FloatingPointMode.h b/include/llvm/ADT/FloatingPointMode.h new file mode 100644 index 00000000000..670b2368da9 --- /dev/null +++ b/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(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/include/llvm/CodeGen/MachineFunction.h b/include/llvm/CodeGen/MachineFunction.h index 57038d239fa..9eee7a56adb 100644 --- a/include/llvm/CodeGen/MachineFunction.h +++ b/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 @@ public: return const_cast(this)->getInfo(); } + /// 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/include/llvm/CodeGen/SelectionDAG.h b/include/llvm/CodeGen/SelectionDAG.h index a009418703e..602671bd640 100644 --- a/include/llvm/CodeGen/SelectionDAG.h +++ b/include/llvm/CodeGen/SelectionDAG.h @@ -1711,6 +1711,12 @@ public: 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/lib/CodeGen/MachineFunction.cpp b/lib/CodeGen/MachineFunction.cpp index b40f0b46bc1..b147a83cd16 100644 --- a/lib/CodeGen/MachineFunction.cpp +++ b/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/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index 2db30279fbd..9a9229e02fe 100644 --- a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/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/unittests/ADT/CMakeLists.txt b/unittests/ADT/CMakeLists.txt index b7acbdc86e6..b5f29b9b99e 100644 --- a/unittests/ADT/CMakeLists.txt +++ b/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/unittests/ADT/FloatingPointMode.cpp b/unittests/ADT/FloatingPointMode.cpp new file mode 100644 index 00000000000..c0d59823db6 --- /dev/null +++ b/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)); +} + +}