From c378e52cb9d1197bd828008ffdeaf3cebdca1506 Mon Sep 17 00:00:00 2001 From: Matt Arsenault Date: Thu, 5 Dec 2019 15:07:27 +0530 Subject: [PATCH] Set some fast math attributes in setFunctionAttributes This will provide a more consistent view to codegen for these attributes. The current system is somewhat awkward, and the fields in TargetOptions are reset based on the command line flag if the attribute isn't set. By forcing these attributes with the flag, there can never be an inconsistency in the behavior if code directly inspects the attribute on the function without considering the command line flags. --- llvm/include/llvm/CodeGen/CommandFlags.inc | 16 ++++++++++++++++ llvm/lib/Target/TargetMachine.cpp | 12 ++++++------ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/llvm/include/llvm/CodeGen/CommandFlags.inc b/llvm/include/llvm/CodeGen/CommandFlags.inc index e9cd1f52ceac52..b65f7bb5b59737 100644 --- a/llvm/include/llvm/CodeGen/CommandFlags.inc +++ b/llvm/include/llvm/CodeGen/CommandFlags.inc @@ -377,6 +377,17 @@ LLVM_ATTRIBUTE_UNUSED static std::vector getFeatureList() { return Features.getFeatures(); } +static void renderBoolStringAttr(AttrBuilder &B, StringRef Name, bool Val) { + B.addAttribute(Name, Val ? "true" : "false"); +} + +#define HANDLE_BOOL_ATTR(CL, AttrName) \ + do { \ + if (CL.getNumOccurrences() > 0 && !F.hasFnAttribute(AttrName)) \ + renderBoolStringAttr(NewAttrs, AttrName, CL); \ + } while (0) + + /// Set function attributes of function \p F based on CPU, Features, and command /// line flags. LLVM_ATTRIBUTE_UNUSED static void @@ -415,6 +426,11 @@ setFunctionAttributes(StringRef CPU, StringRef Features, Function &F) { if (StackRealign) NewAttrs.addAttribute("stackrealign"); + HANDLE_BOOL_ATTR(EnableUnsafeFPMath, "unsafe-fp-math"); + HANDLE_BOOL_ATTR(EnableNoInfsFPMath, "no-infs-fp-math"); + HANDLE_BOOL_ATTR(EnableNoNaNsFPMath, "no-nans-fp-math"); + HANDLE_BOOL_ATTR(EnableNoSignedZerosFPMath, "no-signed-zeros-fp-math"); + if (TrapFuncName.getNumOccurrences() > 0) for (auto &B : F) for (auto &I : B) diff --git a/llvm/lib/Target/TargetMachine.cpp b/llvm/lib/Target/TargetMachine.cpp index 97a1eb2f190a96..88ed6a7210352b 100644 --- a/llvm/lib/Target/TargetMachine.cpp +++ b/llvm/lib/Target/TargetMachine.cpp @@ -46,17 +46,17 @@ bool TargetMachine::isPositionIndependent() const { } /// Reset the target options based on the function's attributes. +/// setFunctionAttributes should have made the raw attribute value consistent +/// with the command line flag if used. +// // FIXME: This function needs to go away for a number of reasons: // a) global state on the TargetMachine is terrible in general, // b) these target options should be passed only on the function // and not on the TargetMachine (via TargetOptions) at all. void TargetMachine::resetTargetOptions(const Function &F) const { -#define RESET_OPTION(X, Y) \ - do { \ - if (F.hasFnAttribute(Y)) \ - Options.X = (F.getFnAttribute(Y).getValueAsString() == "true"); \ - else \ - Options.X = DefaultOptions.X; \ +#define RESET_OPTION(X, Y) \ + do { \ + Options.X = (F.getFnAttribute(Y).getValueAsString() == "true"); \ } while (0) RESET_OPTION(UnsafeFPMath, "unsafe-fp-math");