[flang] Set fast math related function attributes for -Ofast/-ffast-math #79301

asb · 2024-01-24T15:08:00Z

The implemented logic matches the logic used for Clang in emitting these attributes. Although it's hoped that function attributes won't be needed in the future (vs using fast math flags in individual IR instructions), there are codegen differences currently with/without these attributes, as can be seen in issues like #79257 or by hacking Clang to avoid producing these attributes and observing codegen changes.

github-actions · 2024-01-24T15:10:42Z

✅ With the latest revision this PR passed the C/C++ code formatter.

kiranchandramohan · 2024-01-24T16:24:02Z

Is this patch addressing the issue in #79257 ? If not, can the other fast-math function attributes ("no-nans-fp-math"="true" and "no-signed-zeros-fp-math"="true") also be added?

asb · 2024-01-24T16:32:26Z

Is this patch addressing the issue in #79257 ? If not, can the other fast-math function attributes ("no-nans-fp-math"="true" and "no-signed-zeros-fp-math"="true") also be added?

I think we might have run into something similar to that issue independently - so thanks for pointing to that issue! I was actually checkpointing work to look more deeply into the issue as I'd heard the suspicion of differing optimisations due to these flags but didn't have a good example. I will indeed update this shortly with something more complete, and separate out the MLIR change.

llvmbot · 2024-01-29T11:46:14Z

@llvm/pr-subscribers-flang-fir-hlfir
@llvm/pr-subscribers-mlir-llvm

@llvm/pr-subscribers-mlir

Author: Alex Bradbury (asb)

Changes

Stacks on #79812.

The implemented logic matches the logic used for Clang in emitting these attributes. Although it's hoped that function attributes won't be needed in the future (vs using fast math flags in individual IR instructions), there are codegen differences currently with/without these attributes, as can be seen in issues like #79257 or by hacking Clang to avoid producing these attributes and observing codegen changes.

Full diff: https://github.com/llvm/llvm-project/pull/79301.diff

13 Files Affected:

(modified) flang/include/flang/Optimizer/Transforms/Passes.h (+3-1)
(modified) flang/include/flang/Optimizer/Transforms/Passes.td (+15)
(modified) flang/include/flang/Tools/CLOptions.inc (+12-3)
(modified) flang/include/flang/Tools/CrossToolHelpers.h (+19-1)
(modified) flang/lib/Frontend/FrontendActions.cpp (+2-1)
(modified) flang/lib/Optimizer/Transforms/FunctionAttr.cpp (+25-2)
(added) flang/test/Driver/func-attr-fast-math.f90 (+20)
(modified) mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td (+6-1)
(modified) mlir/lib/Target/LLVMIR/ModuleImport.cpp (+25)
(modified) mlir/lib/Target/LLVMIR/ModuleTranslation.cpp (+18)
(modified) mlir/test/Dialect/LLVMIR/func.mlir (+30)
(modified) mlir/test/Target/LLVMIR/Import/function-attributes.ll (+30)
(added) mlir/test/Target/LLVMIR/fp-math-function-attributes.mlir (+44)

diff --git a/flang/include/flang/Optimizer/Transforms/Passes.h b/flang/include/flang/Optimizer/Transforms/Passes.h
index 6970da8698ae84d..d33623c6904891e 100644
--- a/flang/include/flang/Optimizer/Transforms/Passes.h
+++ b/flang/include/flang/Optimizer/Transforms/Passes.h
@@ -91,7 +91,9 @@ struct FunctionAttrTypes {
 
 std::unique_ptr<mlir::Pass> createFunctionAttrPass();
 std::unique_ptr<mlir::Pass>
-createFunctionAttrPass(FunctionAttrTypes &functionAttr);
+createFunctionAttrPass(FunctionAttrTypes &functionAttr, bool noInfsFPMath,
+                       bool noNaNsFPMath, bool approxFuncFPMath,
+                       bool noSignedZerosFPMath, bool unsafeFPMath);
 
 // declarative passes
 #define GEN_PASS_REGISTRATION
diff --git a/flang/include/flang/Optimizer/Transforms/Passes.td b/flang/include/flang/Optimizer/Transforms/Passes.td
index e3c45d41f04cc71..397985383b226c7 100644
--- a/flang/include/flang/Optimizer/Transforms/Passes.td
+++ b/flang/include/flang/Optimizer/Transforms/Passes.td
@@ -366,6 +366,21 @@ def FunctionAttr : Pass<"function-attr", "mlir::func::FuncOp"> {
            "mlir::LLVM::framePointerKind::FramePointerKind", 
            /*default=*/"mlir::LLVM::framePointerKind::FramePointerKind{}",
            "frame pointer">,
+    Option<"noInfsFPMath", "no-infs-fp-math",
+           "bool", /*default=*/"false",
+           "Set the no-infs-fp-math attribute on functions in the module.">,
+    Option<"noNaNsFPMath", "no-nans-fp-math",
+           "bool", /*default=*/"false",
+           "Set the no-nans-fp-math attribute on functions in the module.">,
+    Option<"approxFuncFPMath", "approx-func-fp-math",
+           "bool", /*default=*/"false",
+           "Set the approx-func-fp-math attribute on functions in the module.">,
+    Option<"noSignedZerosFPMath", "no-signed-zeros-fp-math",
+           "bool", /*default=*/"false",
+           "Set the no-signed-zeros-fp-math attribute on functions in the module.">,
+    Option<"unsafeFPMath", "unsafe-fp-math",
+           "bool", /*default=*/"false",
+           "Set the unsafe-fp-math attribute on functions in the module.">,
   ];
   let constructor = "::fir::createFunctionAttrPass()";
 }
diff --git a/flang/include/flang/Tools/CLOptions.inc b/flang/include/flang/Tools/CLOptions.inc
index 96d3869cd093912..7a327ad226bf7e6 100644
--- a/flang/include/flang/Tools/CLOptions.inc
+++ b/flang/include/flang/Tools/CLOptions.inc
@@ -315,15 +315,24 @@ inline void createDefaultFIRCodeGenPassPipeline(
   // Add function attributes
   fir::FunctionAttrTypes functionAttrs;
 
-  if (config.FramePointerKind != llvm::FramePointerKind::None) {
+  if (config.FramePointerKind != llvm::FramePointerKind::None || 
+      config.NoInfsFPMath || config.NoNaNsFPMath || config.ApproxFuncFPMath ||
+      config.NoSignedZerosFPMath || config.UnsafeFPMath) {
     if (config.FramePointerKind == llvm::FramePointerKind::NonLeaf)
       functionAttrs.framePointerKind =
           mlir::LLVM::framePointerKind::FramePointerKind::NonLeaf;
-    else
+    else if (config.FramePointerKind == llvm::FramePointerKind::All)
       functionAttrs.framePointerKind =
           mlir::LLVM::framePointerKind::FramePointerKind::All;
+    else
+      functionAttrs.framePointerKind =
+          mlir::LLVM::framePointerKind::FramePointerKind::None;
 
-    pm.addPass(fir::createFunctionAttrPass(functionAttrs));
+    pm.addPass(
+      fir::createFunctionAttrPass(functionAttrs, config.NoInfsFPMath,
+                                  config.NoNaNsFPMath, config.ApproxFuncFPMath,
+                                  config.NoSignedZerosFPMath,
+                                  config.UnsafeFPMath));
   }
 
   fir::addFIRToLLVMPass(pm, config);
diff --git a/flang/include/flang/Tools/CrossToolHelpers.h b/flang/include/flang/Tools/CrossToolHelpers.h
index b61224ff4f1b3cd..0f39bdb86102f2c 100644
--- a/flang/include/flang/Tools/CrossToolHelpers.h
+++ b/flang/include/flang/Tools/CrossToolHelpers.h
@@ -13,6 +13,7 @@
 #ifndef FORTRAN_TOOLS_CROSS_TOOL_HELPERS_H
 #define FORTRAN_TOOLS_CROSS_TOOL_HELPERS_H
 
+#include "flang/Common/MathOptionsBase.h"
 #include "flang/Frontend/CodeGenOptions.h"
 #include "flang/Frontend/LangOptions.h"
 #include <cstdint>
@@ -28,7 +29,8 @@ struct MLIRToLLVMPassPipelineConfig {
     OptLevel = level;
   }
   explicit MLIRToLLVMPassPipelineConfig(llvm::OptimizationLevel level,
-      const Fortran::frontend::CodeGenOptions &opts) {
+      const Fortran::frontend::CodeGenOptions &opts,
+      const Fortran::common::MathOptionsBase &mathOpts) {
     OptLevel = level;
     StackArrays = opts.StackArrays;
     Underscoring = opts.Underscoring;
@@ -36,6 +38,15 @@ struct MLIRToLLVMPassPipelineConfig {
     DebugInfo = opts.getDebugInfo();
     AliasAnalysis = opts.AliasAnalysis;
     FramePointerKind = opts.getFramePointer();
+    // The logic for setting these attributes is intended to match the logic
+    // used in Clang.
+    NoInfsFPMath = mathOpts.getNoHonorInfs();
+    NoNaNsFPMath = mathOpts.getNoHonorNaNs();
+    ApproxFuncFPMath = mathOpts.getApproxFunc();
+    NoSignedZerosFPMath = mathOpts.getNoSignedZeros();
+    UnsafeFPMath = mathOpts.getAssociativeMath() &&
+        mathOpts.getReciprocalMath() && NoSignedZerosFPMath &&
+        ApproxFuncFPMath && mathOpts.getFPContractEnabled();
   }
 
   llvm::OptimizationLevel OptLevel; ///< optimisation level
@@ -49,6 +60,13 @@ struct MLIRToLLVMPassPipelineConfig {
       llvm::FramePointerKind::None; ///< Add frame pointer to functions.
   unsigned VScaleMin = 0; ///< SVE vector range minimum.
   unsigned VScaleMax = 0; ///< SVE vector range maximum.
+  bool NoInfsFPMath = false; ///< Set no-infs-fp-math attribute for functions.
+  bool NoNaNsFPMath = false; ///< Set no-nans-fp-math attribute for functions.
+  bool ApproxFuncFPMath =
+      false; ///< Set approx-func-fp-math attribute for functions.
+  bool NoSignedZerosFPMath =
+      false; ///< Set no-signed-zeros-fp-math attribute for functions.
+  bool UnsafeFPMath = false; ///< Set unsafe-fp-math attribute for functions.
 };
 
 struct OffloadModuleOpts {
diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index 65c4df7388f97b2..17d5670b4134d86 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -787,6 +787,7 @@ void CodeGenAction::generateLLVMIR() {
 
   CompilerInstance &ci = this->getInstance();
   auto opts = ci.getInvocation().getCodeGenOpts();
+  auto mathOpts = ci.getInvocation().getLoweringOpts().getMathOptions();
   llvm::OptimizationLevel level = mapToLevel(opts);
 
   fir::support::loadDialects(*mlirCtx);
@@ -799,7 +800,7 @@ void CodeGenAction::generateLLVMIR() {
   pm.addPass(std::make_unique<Fortran::lower::VerifierPass>());
   pm.enableVerifier(/*verifyPasses=*/true);
 
-  MLIRToLLVMPassPipelineConfig config(level, opts);
+  MLIRToLLVMPassPipelineConfig config(level, opts, mathOpts);
 
   if (auto vsr = getVScaleRange(ci)) {
     config.VScaleMin = vsr->first;
diff --git a/flang/lib/Optimizer/Transforms/FunctionAttr.cpp b/flang/lib/Optimizer/Transforms/FunctionAttr.cpp
index 55b908ba5d86139..55ffc4ed43933a8 100644
--- a/flang/lib/Optimizer/Transforms/FunctionAttr.cpp
+++ b/flang/lib/Optimizer/Transforms/FunctionAttr.cpp
@@ -27,6 +27,11 @@ class FunctionAttrPass : public fir::impl::FunctionAttrBase<FunctionAttrPass> {
 public:
   FunctionAttrPass(const fir::FunctionAttrOptions &options) {
     framePointerKind = options.framePointerKind;
+    noInfsFPMath = options.noInfsFPMath;
+    noNaNsFPMath = options.noNaNsFPMath;
+    approxFuncFPMath = options.approxFuncFPMath;
+    noSignedZerosFPMath = options.noSignedZerosFPMath;
+    unsafeFPMath = options.unsafeFPMath;
   }
   FunctionAttrPass() {}
   void runOnOperation() override;
@@ -45,14 +50,32 @@ void FunctionAttrPass::runOnOperation() {
     func->setAttr("frame_pointer", mlir::LLVM::FramePointerKindAttr::get(
                                        context, framePointerKind));
 
+  if (noInfsFPMath)
+    func->setAttr("no_infs_fp_math", mlir::BoolAttr::get(context, true));
+  if (noNaNsFPMath)
+    func->setAttr("no_nans_fp_math", mlir::BoolAttr::get(context, true));
+  if (approxFuncFPMath)
+    func->setAttr("approx_func_fp_math", mlir::BoolAttr::get(context, true));
+  if (noSignedZerosFPMath)
+    func->setAttr("no_signed_zeros_fp_math",
+                  mlir::BoolAttr::get(context, true));
+  if (unsafeFPMath)
+    func->setAttr("unsafe_fp_math", mlir::BoolAttr::get(context, true));
+
   LLVM_DEBUG(llvm::dbgs() << "=== End " DEBUG_TYPE " ===\n");
 }
 
-std::unique_ptr<mlir::Pass>
-fir::createFunctionAttrPass(fir::FunctionAttrTypes &functionAttr) {
+std::unique_ptr<mlir::Pass> fir::createFunctionAttrPass(
+    fir::FunctionAttrTypes &functionAttr, bool noInfsFPMath, bool noNaNsFPMath,
+    bool approxFuncFPMath, bool noSignedZerosFPMath, bool unsafeFPMath) {
   FunctionAttrOptions opts;
   // Frame pointer
   opts.framePointerKind = functionAttr.framePointerKind;
+  opts.noInfsFPMath = noInfsFPMath;
+  opts.noNaNsFPMath = noNaNsFPMath;
+  opts.approxFuncFPMath = approxFuncFPMath;
+  opts.noSignedZerosFPMath = noSignedZerosFPMath;
+  opts.unsafeFPMath = unsafeFPMath;
 
   return std::make_unique<FunctionAttrPass>(opts);
 }
diff --git a/flang/test/Driver/func-attr-fast-math.f90 b/flang/test/Driver/func-attr-fast-math.f90
new file mode 100644
index 000000000000000..2f078dd1b218365
--- /dev/null
+++ b/flang/test/Driver/func-attr-fast-math.f90
@@ -0,0 +1,20 @@
+! Test that -mframe-pointer can accept only specific values and when given an invalid value, check it raises an error.
+
+! RUN: %flang -O1 -emit-llvm -S -o - %s 2>&1| FileCheck %s --check-prefix=CHECK-NOFASTMATH
+! RUN: %flang -Ofast -emit-llvm -S -o - %s 2>&1| FileCheck %s --check-prefix=CHECK-OFAST
+! RUN: %flang -O1 -ffast-math -emit-llvm -S -o - %s 2>&1| FileCheck %s --check-prefix=CHECK-FFAST-MATH
+
+subroutine func
+end subroutine func
+
+! CHECK-NOFASTMATH-LABEL: define void @func_() local_unnamed_addr
+! CHECK-NOFASTMATH-SAME: #[[ATTRS:[0-9]+]]
+! CHECK-NOFASTMATH: attributes #[[ATTRS]] = { mustprogress nofree norecurse nosync nounwind willreturn memory(none) }
+!
+! CHECK-OFAST-LABEL: define void @func_() local_unnamed_addr
+! CHECK-OFAST-SAME: #[[ATTRS:[0-9]+]]
+! CHECK-OFAST: attributes #[[ATTRS]] = { mustprogress nofree norecurse nosync nounwind willreturn memory(none) "approx-func-fp-math"="true" "no-infs-fp-math"="true" "no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" "unsafe-fp-math"="true" }
+
+! CHECK-FFAST-MATH-LABEL: define void @func_() local_unnamed_addr
+! CHECK-FFAST-MATH-SAME: #[[ATTRS:[0-9]+]]
+! CHECK-FFAST-MATH: attributes #[[ATTRS]] = { mustprogress nofree norecurse nosync nounwind willreturn memory(none) "approx-func-fp-math"="true" "no-infs-fp-math"="true" "no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" "unsafe-fp-math"="true" }
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index 01d476f530b1c57..ad67fba5a81cf8b 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
@@ -1428,7 +1428,12 @@ def LLVM_LLVMFuncOp : LLVM_Op<"func", [
     OptionalAttr<LLVM_VScaleRangeAttr>:$vscale_range,
     OptionalAttr<FramePointerKindAttr>:$frame_pointer,
     OptionalAttr<StrAttr>:$target_cpu,
-    OptionalAttr<LLVM_TargetFeaturesAttr>:$target_features
+    OptionalAttr<LLVM_TargetFeaturesAttr>:$target_features,
+    OptionalAttr<BoolAttr>:$unsafe_fp_math,
+    OptionalAttr<BoolAttr>:$no_infs_fp_math,
+    OptionalAttr<BoolAttr>:$no_nans_fp_math,
+    OptionalAttr<BoolAttr>:$approx_func_fp_math,
+    OptionalAttr<BoolAttr>:$no_signed_zeros_fp_math
   );
 
   let regions = (region AnyRegion:$body);
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index 928d8077175ccf5..5ca4a9fd68d650d 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -1646,6 +1646,11 @@ static constexpr std::array ExplicitAttributes{
     StringLiteral("vscale_range"),
     StringLiteral("frame-pointer"),
     StringLiteral("target-features"),
+    StringLiteral("unsafe-fp-math"),
+    StringLiteral("no-infs-fp-math"),
+    StringLiteral("no-nans-fp-math"),
+    StringLiteral("approx-func-fp-math"),
+    StringLiteral("no-signed-zeros-fp-math"),
 };
 
 static void processPassthroughAttrs(llvm::Function *func, LLVMFuncOp funcOp) {
@@ -1752,6 +1757,26 @@ void ModuleImport::processFunctionAttributes(llvm::Function *func,
       attr.isStringAttribute())
     funcOp.setTargetFeaturesAttr(
         LLVM::TargetFeaturesAttr::get(context, attr.getValueAsString()));
+
+  if (llvm::Attribute attr = func->getFnAttribute("unsafe-fp-math");
+      attr.isStringAttribute())
+    funcOp.setUnsafeFpMath(attr.getValueAsBool());
+
+  if (llvm::Attribute attr = func->getFnAttribute("no-infs-fp-math");
+      attr.isStringAttribute())
+    funcOp.setNoInfsFpMath(attr.getValueAsBool());
+
+  if (llvm::Attribute attr = func->getFnAttribute("no-nans-fp-math");
+      attr.isStringAttribute())
+    funcOp.setNoNansFpMath(attr.getValueAsBool());
+
+  if (llvm::Attribute attr = func->getFnAttribute("approx-func-fp-math");
+      attr.isStringAttribute())
+    funcOp.setApproxFuncFpMath(attr.getValueAsBool());
+
+  if (llvm::Attribute attr = func->getFnAttribute("no-signed-zeros-fp-math");
+      attr.isStringAttribute())
+    funcOp.setNoSignedZerosFpMath(attr.getValueAsBool());
 }
 
 DictionaryAttr
diff --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
index 69a1cbe5969e859..6364cacbd19245f 100644
--- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -37,6 +37,7 @@
 
 #include "llvm/ADT/PostOrderIterator.h"
 #include "llvm/ADT/SetVector.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/TypeSwitch.h"
 #include "llvm/Frontend/OpenMP/OMPIRBuilder.h"
 #include "llvm/IR/BasicBlock.h"
@@ -1214,6 +1215,23 @@ LogicalResult ModuleTranslation::convertOneFunction(LLVMFuncOp func) {
         getLLVMContext(), attr->getMinRange().getInt(),
         attr->getMaxRange().getInt()));
 
+  if (auto unsafeFpMath = func.getUnsafeFpMath())
+    llvmFunc->addFnAttr("unsafe-fp-math", llvm::toStringRef(*unsafeFpMath));
+
+  if (auto noInfsFpMath = func.getNoInfsFpMath())
+    llvmFunc->addFnAttr("no-infs-fp-math", llvm::toStringRef(*noInfsFpMath));
+
+  if (auto noNansFpMath = func.getNoNansFpMath())
+    llvmFunc->addFnAttr("no-nans-fp-math", llvm::toStringRef(*noNansFpMath));
+
+  if (auto approxFuncFpMath = func.getApproxFuncFpMath())
+    llvmFunc->addFnAttr("approx-func-fp-math",
+                        llvm::toStringRef(*approxFuncFpMath));
+
+  if (auto noSignedZerosFpMath = func.getNoSignedZerosFpMath())
+    llvmFunc->addFnAttr("no-signed-zeros-fp-math",
+                        llvm::toStringRef(*noSignedZerosFpMath));
+
   // Add function attribute frame-pointer, if found.
   if (FramePointerKindAttr attr = func.getFramePointerAttr())
     llvmFunc->addFnAttr("frame-pointer",
diff --git a/mlir/test/Dialect/LLVMIR/func.mlir b/mlir/test/Dialect/LLVMIR/func.mlir
index 9dc1bc57034e02f..006f2f64a27277c 100644
--- a/mlir/test/Dialect/LLVMIR/func.mlir
+++ b/mlir/test/Dialect/LLVMIR/func.mlir
@@ -257,6 +257,36 @@ module {
   llvm.func @frame_pointer_roundtrip() attributes {frame_pointer = #llvm.framePointerKind<"non-leaf">} {
     llvm.return
   }
+
+  llvm.func @unsafe_fp_math_roundtrip() attributes {unsafe_fp_math = true} {
+    // CHECK: @unsafe_fp_math_roundtrip
+    // CHECK-SAME: attributes {unsafe_fp_math = true}
+    llvm.return
+  }
+
+  llvm.func @no_infs_fp_math_roundtrip() attributes {no_infs_fp_math = true} {
+    // CHECK: @no_infs_fp_math_roundtrip
+    // CHECK-SAME: attributes {no_infs_fp_math = true}
+    llvm.return
+  }
+
+  llvm.func @no_nans_fp_math_roundtrip() attributes {no_nans_fp_math = true} {
+    // CHECK: @no_nans_fp_math_roundtrip
+    // CHECK-SAME: attributes {no_nans_fp_math = true}
+    llvm.return
+  }
+
+  llvm.func @approx_func_fp_math_roundtrip() attributes {approx_func_fp_math = true} {
+    // CHECK: @approx_func_fp_math_roundtrip
+    // CHECK-SAME: attributes {approx_func_fp_math = true}
+    llvm.return
+  }
+
+  llvm.func @no_signed_zeros_fp_math_roundtrip() attributes {no_signed_zeros_fp_math = true} {
+    // CHECK: @no_signed_zeros_fp_math_roundtrip
+    // CHECK-SAME: attributes {no_signed_zeros_fp_math = true}
+    llvm.return
+  }
 }
 
 // -----
diff --git a/mlir/test/Target/LLVMIR/Import/function-attributes.ll b/mlir/test/Target/LLVMIR/Import/function-attributes.ll
index af2ef9db96ae333..70387789aa97e4a 100644
--- a/mlir/test/Target/LLVMIR/Import/function-attributes.ll
+++ b/mlir/test/Target/LLVMIR/Import/function-attributes.ll
@@ -272,3 +272,33 @@ define void @align_func() align 2 {
 ; CHECK-LABEL: @align_decl
 ; CHECK-SAME: attributes {alignment = 64 : i64}
 declare void @align_decl() align 64
+
+; // -----
+
+; CHECK-LABEL: @func_attr_unsafe_fp_math
+; CHECK-SAME: attributes {unsafe_fp_math = true}
+declare void @func_attr_unsafe_fp_math() "unsafe-fp-math"="true"
+
+; // -----
+
+; CHECK-LABEL: @func_attr_no_infs_fp_math
+; CHECK-SAME: attributes {no_infs_fp_math = true}
+declare void @func_attr_no_infs_fp_math() "no-infs-fp-math"="true"
+
+; // -----
+
+; CHECK-LABEL: @func_attr_no_nans_fp_math
+; CHECK-SAME: attributes {no_nans_fp_math = true}
+declare void @func_attr_no_nans_fp_math() "no-nans-fp-math"="true"
+
+; // -----
+
+; CHECK-LABEL: @func_attr_approx_func_fp_math
+; CHECK-SAME: attributes {approx_func_fp_math = true}
+declare void @func_attr_approx_func_fp_math() "approx-func-fp-math"="true"
+
+; // -----
+
+; CHECK-LABEL: @func_attr_no_signed_zeros_fp_math
+; CHECK-SAME: attributes {no_signed_zeros_fp_math = true}
+declare void @func_attr_no_signed_zeros_fp_math() "no-signed-zeros-fp-math"="true"
diff --git a/mlir/test/Target/LLVMIR/fp-math-function-attributes.mlir b/mlir/test/Target/LLVMIR/fp-math-function-attributes.mlir
new file mode 100644
index 000000000000000..ccae06cc3dd76d6
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/fp-math-function-attributes.mlir
@@ -0,0 +1,44 @@
+// RUN: mlir-translate -mlir-to-llvmir -split-input-file %s | FileCheck %s
+
+// CHECK-LABEL: define void @unsafe_fp_math_func()
+// CHECK-SAME: #[[ATTRS:[0-9]+]]
+llvm.func @unsafe_fp_math_func() attributes {unsafe_fp_math = true}  {
+  llvm.return
+}
+// CHECK: attributes #[[ATTRS]] = { "unsafe-fp-math"="true" }
+
+// -----
+
+// CHECK-LABEL: define void @no_infs_fp_math_func()
+// CHECK-SAME: #[[ATTRS:[0-9]+]]
+llvm.func @no_infs_fp_math_func() attributes {no_infs_fp_math = true}  {
+  llvm.return
+}
+// CHECK: attributes #[[ATTRS]] = { "no-infs-fp-math"="true" }
+
+// -----
+
+// CHECK-LABEL: define void @no_nans_fp_math_func()
+// CHECK-SAME: #[[ATTRS:[0-9]+]]
+llvm.func @no_nans_fp_math_func() attributes {no_nans_fp_math = true}  {
+  llvm.return
+}
+// CHECK: attributes #[[ATTRS]] = { "no-nans-fp-math"="true" }
+
+// -----
+
+// CHECK-LABEL: define void @approx_func_fp_math_func()
+// CHECK-SAME: #[[ATTRS:[0-9]+]]
+llvm.func @approx_func_fp_math_func() attributes {approx_func_fp_math = true}  {
+  llvm.return
+}
+// CHECK: attributes #[[ATTRS]] = { "approx-func-fp-math"="true" }
+
+// -----
+
+// CHECK-LABEL: define void @no_signed_zeros_fp_math_func()
+// CHECK-SAME: #[[ATTRS:[0-9]+]]
+llvm.func @no_signed_zeros_fp_math_func() attributes {no_signed_zeros_fp_math = true}  {
+  llvm.return
+}
+// CHECK: attributes #[[ATTRS]] = { "no-signed-zeros-fp-math"="true" }

llvmbot · 2024-01-29T11:46:15Z

@llvm/pr-subscribers-flang-driver

Author: Alex Bradbury (asb)

Changes

Stacks on #79812.

The implemented logic matches the logic used for Clang in emitting these attributes. Although it's hoped that function attributes won't be needed in the future (vs using fast math flags in individual IR instructions), there are codegen differences currently with/without these attributes, as can be seen in issues like #79257 or by hacking Clang to avoid producing these attributes and observing codegen changes.

Full diff: https://github.com/llvm/llvm-project/pull/79301.diff

13 Files Affected:

(modified) flang/include/flang/Optimizer/Transforms/Passes.h (+3-1)
(modified) flang/include/flang/Optimizer/Transforms/Passes.td (+15)
(modified) flang/include/flang/Tools/CLOptions.inc (+12-3)
(modified) flang/include/flang/Tools/CrossToolHelpers.h (+19-1)
(modified) flang/lib/Frontend/FrontendActions.cpp (+2-1)
(modified) flang/lib/Optimizer/Transforms/FunctionAttr.cpp (+25-2)
(added) flang/test/Driver/func-attr-fast-math.f90 (+20)
(modified) mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td (+6-1)
(modified) mlir/lib/Target/LLVMIR/ModuleImport.cpp (+25)
(modified) mlir/lib/Target/LLVMIR/ModuleTranslation.cpp (+18)
(modified) mlir/test/Dialect/LLVMIR/func.mlir (+30)
(modified) mlir/test/Target/LLVMIR/Import/function-attributes.ll (+30)
(added) mlir/test/Target/LLVMIR/fp-math-function-attributes.mlir (+44)

diff --git a/flang/include/flang/Optimizer/Transforms/Passes.h b/flang/include/flang/Optimizer/Transforms/Passes.h
index 6970da8698ae84..d33623c6904891 100644
--- a/flang/include/flang/Optimizer/Transforms/Passes.h
+++ b/flang/include/flang/Optimizer/Transforms/Passes.h
@@ -91,7 +91,9 @@ struct FunctionAttrTypes {
 
 std::unique_ptr<mlir::Pass> createFunctionAttrPass();
 std::unique_ptr<mlir::Pass>
-createFunctionAttrPass(FunctionAttrTypes &functionAttr);
+createFunctionAttrPass(FunctionAttrTypes &functionAttr, bool noInfsFPMath,
+                       bool noNaNsFPMath, bool approxFuncFPMath,
+                       bool noSignedZerosFPMath, bool unsafeFPMath);
 
 // declarative passes
 #define GEN_PASS_REGISTRATION
diff --git a/flang/include/flang/Optimizer/Transforms/Passes.td b/flang/include/flang/Optimizer/Transforms/Passes.td
index e3c45d41f04cc7..397985383b226c 100644
--- a/flang/include/flang/Optimizer/Transforms/Passes.td
+++ b/flang/include/flang/Optimizer/Transforms/Passes.td
@@ -366,6 +366,21 @@ def FunctionAttr : Pass<"function-attr", "mlir::func::FuncOp"> {
            "mlir::LLVM::framePointerKind::FramePointerKind", 
            /*default=*/"mlir::LLVM::framePointerKind::FramePointerKind{}",
            "frame pointer">,
+    Option<"noInfsFPMath", "no-infs-fp-math",
+           "bool", /*default=*/"false",
+           "Set the no-infs-fp-math attribute on functions in the module.">,
+    Option<"noNaNsFPMath", "no-nans-fp-math",
+           "bool", /*default=*/"false",
+           "Set the no-nans-fp-math attribute on functions in the module.">,
+    Option<"approxFuncFPMath", "approx-func-fp-math",
+           "bool", /*default=*/"false",
+           "Set the approx-func-fp-math attribute on functions in the module.">,
+    Option<"noSignedZerosFPMath", "no-signed-zeros-fp-math",
+           "bool", /*default=*/"false",
+           "Set the no-signed-zeros-fp-math attribute on functions in the module.">,
+    Option<"unsafeFPMath", "unsafe-fp-math",
+           "bool", /*default=*/"false",
+           "Set the unsafe-fp-math attribute on functions in the module.">,
   ];
   let constructor = "::fir::createFunctionAttrPass()";
 }
diff --git a/flang/include/flang/Tools/CLOptions.inc b/flang/include/flang/Tools/CLOptions.inc
index 96d3869cd09391..7a327ad226bf7e 100644
--- a/flang/include/flang/Tools/CLOptions.inc
+++ b/flang/include/flang/Tools/CLOptions.inc
@@ -315,15 +315,24 @@ inline void createDefaultFIRCodeGenPassPipeline(
   // Add function attributes
   fir::FunctionAttrTypes functionAttrs;
 
-  if (config.FramePointerKind != llvm::FramePointerKind::None) {
+  if (config.FramePointerKind != llvm::FramePointerKind::None || 
+      config.NoInfsFPMath || config.NoNaNsFPMath || config.ApproxFuncFPMath ||
+      config.NoSignedZerosFPMath || config.UnsafeFPMath) {
     if (config.FramePointerKind == llvm::FramePointerKind::NonLeaf)
       functionAttrs.framePointerKind =
           mlir::LLVM::framePointerKind::FramePointerKind::NonLeaf;
-    else
+    else if (config.FramePointerKind == llvm::FramePointerKind::All)
       functionAttrs.framePointerKind =
           mlir::LLVM::framePointerKind::FramePointerKind::All;
+    else
+      functionAttrs.framePointerKind =
+          mlir::LLVM::framePointerKind::FramePointerKind::None;
 
-    pm.addPass(fir::createFunctionAttrPass(functionAttrs));
+    pm.addPass(
+      fir::createFunctionAttrPass(functionAttrs, config.NoInfsFPMath,
+                                  config.NoNaNsFPMath, config.ApproxFuncFPMath,
+                                  config.NoSignedZerosFPMath,
+                                  config.UnsafeFPMath));
   }
 
   fir::addFIRToLLVMPass(pm, config);
diff --git a/flang/include/flang/Tools/CrossToolHelpers.h b/flang/include/flang/Tools/CrossToolHelpers.h
index b61224ff4f1b3c..0f39bdb86102f2 100644
--- a/flang/include/flang/Tools/CrossToolHelpers.h
+++ b/flang/include/flang/Tools/CrossToolHelpers.h
@@ -13,6 +13,7 @@
 #ifndef FORTRAN_TOOLS_CROSS_TOOL_HELPERS_H
 #define FORTRAN_TOOLS_CROSS_TOOL_HELPERS_H
 
+#include "flang/Common/MathOptionsBase.h"
 #include "flang/Frontend/CodeGenOptions.h"
 #include "flang/Frontend/LangOptions.h"
 #include <cstdint>
@@ -28,7 +29,8 @@ struct MLIRToLLVMPassPipelineConfig {
     OptLevel = level;
   }
   explicit MLIRToLLVMPassPipelineConfig(llvm::OptimizationLevel level,
-      const Fortran::frontend::CodeGenOptions &opts) {
+      const Fortran::frontend::CodeGenOptions &opts,
+      const Fortran::common::MathOptionsBase &mathOpts) {
     OptLevel = level;
     StackArrays = opts.StackArrays;
     Underscoring = opts.Underscoring;
@@ -36,6 +38,15 @@ struct MLIRToLLVMPassPipelineConfig {
     DebugInfo = opts.getDebugInfo();
     AliasAnalysis = opts.AliasAnalysis;
     FramePointerKind = opts.getFramePointer();
+    // The logic for setting these attributes is intended to match the logic
+    // used in Clang.
+    NoInfsFPMath = mathOpts.getNoHonorInfs();
+    NoNaNsFPMath = mathOpts.getNoHonorNaNs();
+    ApproxFuncFPMath = mathOpts.getApproxFunc();
+    NoSignedZerosFPMath = mathOpts.getNoSignedZeros();
+    UnsafeFPMath = mathOpts.getAssociativeMath() &&
+        mathOpts.getReciprocalMath() && NoSignedZerosFPMath &&
+        ApproxFuncFPMath && mathOpts.getFPContractEnabled();
   }
 
   llvm::OptimizationLevel OptLevel; ///< optimisation level
@@ -49,6 +60,13 @@ struct MLIRToLLVMPassPipelineConfig {
       llvm::FramePointerKind::None; ///< Add frame pointer to functions.
   unsigned VScaleMin = 0; ///< SVE vector range minimum.
   unsigned VScaleMax = 0; ///< SVE vector range maximum.
+  bool NoInfsFPMath = false; ///< Set no-infs-fp-math attribute for functions.
+  bool NoNaNsFPMath = false; ///< Set no-nans-fp-math attribute for functions.
+  bool ApproxFuncFPMath =
+      false; ///< Set approx-func-fp-math attribute for functions.
+  bool NoSignedZerosFPMath =
+      false; ///< Set no-signed-zeros-fp-math attribute for functions.
+  bool UnsafeFPMath = false; ///< Set unsafe-fp-math attribute for functions.
 };
 
 struct OffloadModuleOpts {
diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index 65c4df7388f97b..17d5670b4134d8 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -787,6 +787,7 @@ void CodeGenAction::generateLLVMIR() {
 
   CompilerInstance &ci = this->getInstance();
   auto opts = ci.getInvocation().getCodeGenOpts();
+  auto mathOpts = ci.getInvocation().getLoweringOpts().getMathOptions();
   llvm::OptimizationLevel level = mapToLevel(opts);
 
   fir::support::loadDialects(*mlirCtx);
@@ -799,7 +800,7 @@ void CodeGenAction::generateLLVMIR() {
   pm.addPass(std::make_unique<Fortran::lower::VerifierPass>());
   pm.enableVerifier(/*verifyPasses=*/true);
 
-  MLIRToLLVMPassPipelineConfig config(level, opts);
+  MLIRToLLVMPassPipelineConfig config(level, opts, mathOpts);
 
   if (auto vsr = getVScaleRange(ci)) {
     config.VScaleMin = vsr->first;
diff --git a/flang/lib/Optimizer/Transforms/FunctionAttr.cpp b/flang/lib/Optimizer/Transforms/FunctionAttr.cpp
index 55b908ba5d8613..55ffc4ed43933a 100644
--- a/flang/lib/Optimizer/Transforms/FunctionAttr.cpp
+++ b/flang/lib/Optimizer/Transforms/FunctionAttr.cpp
@@ -27,6 +27,11 @@ class FunctionAttrPass : public fir::impl::FunctionAttrBase<FunctionAttrPass> {
 public:
   FunctionAttrPass(const fir::FunctionAttrOptions &options) {
     framePointerKind = options.framePointerKind;
+    noInfsFPMath = options.noInfsFPMath;
+    noNaNsFPMath = options.noNaNsFPMath;
+    approxFuncFPMath = options.approxFuncFPMath;
+    noSignedZerosFPMath = options.noSignedZerosFPMath;
+    unsafeFPMath = options.unsafeFPMath;
   }
   FunctionAttrPass() {}
   void runOnOperation() override;
@@ -45,14 +50,32 @@ void FunctionAttrPass::runOnOperation() {
     func->setAttr("frame_pointer", mlir::LLVM::FramePointerKindAttr::get(
                                        context, framePointerKind));
 
+  if (noInfsFPMath)
+    func->setAttr("no_infs_fp_math", mlir::BoolAttr::get(context, true));
+  if (noNaNsFPMath)
+    func->setAttr("no_nans_fp_math", mlir::BoolAttr::get(context, true));
+  if (approxFuncFPMath)
+    func->setAttr("approx_func_fp_math", mlir::BoolAttr::get(context, true));
+  if (noSignedZerosFPMath)
+    func->setAttr("no_signed_zeros_fp_math",
+                  mlir::BoolAttr::get(context, true));
+  if (unsafeFPMath)
+    func->setAttr("unsafe_fp_math", mlir::BoolAttr::get(context, true));
+
   LLVM_DEBUG(llvm::dbgs() << "=== End " DEBUG_TYPE " ===\n");
 }
 
-std::unique_ptr<mlir::Pass>
-fir::createFunctionAttrPass(fir::FunctionAttrTypes &functionAttr) {
+std::unique_ptr<mlir::Pass> fir::createFunctionAttrPass(
+    fir::FunctionAttrTypes &functionAttr, bool noInfsFPMath, bool noNaNsFPMath,
+    bool approxFuncFPMath, bool noSignedZerosFPMath, bool unsafeFPMath) {
   FunctionAttrOptions opts;
   // Frame pointer
   opts.framePointerKind = functionAttr.framePointerKind;
+  opts.noInfsFPMath = noInfsFPMath;
+  opts.noNaNsFPMath = noNaNsFPMath;
+  opts.approxFuncFPMath = approxFuncFPMath;
+  opts.noSignedZerosFPMath = noSignedZerosFPMath;
+  opts.unsafeFPMath = unsafeFPMath;
 
   return std::make_unique<FunctionAttrPass>(opts);
 }
diff --git a/flang/test/Driver/func-attr-fast-math.f90 b/flang/test/Driver/func-attr-fast-math.f90
new file mode 100644
index 00000000000000..2f078dd1b21836
--- /dev/null
+++ b/flang/test/Driver/func-attr-fast-math.f90
@@ -0,0 +1,20 @@
+! Test that -mframe-pointer can accept only specific values and when given an invalid value, check it raises an error.
+
+! RUN: %flang -O1 -emit-llvm -S -o - %s 2>&1| FileCheck %s --check-prefix=CHECK-NOFASTMATH
+! RUN: %flang -Ofast -emit-llvm -S -o - %s 2>&1| FileCheck %s --check-prefix=CHECK-OFAST
+! RUN: %flang -O1 -ffast-math -emit-llvm -S -o - %s 2>&1| FileCheck %s --check-prefix=CHECK-FFAST-MATH
+
+subroutine func
+end subroutine func
+
+! CHECK-NOFASTMATH-LABEL: define void @func_() local_unnamed_addr
+! CHECK-NOFASTMATH-SAME: #[[ATTRS:[0-9]+]]
+! CHECK-NOFASTMATH: attributes #[[ATTRS]] = { mustprogress nofree norecurse nosync nounwind willreturn memory(none) }
+!
+! CHECK-OFAST-LABEL: define void @func_() local_unnamed_addr
+! CHECK-OFAST-SAME: #[[ATTRS:[0-9]+]]
+! CHECK-OFAST: attributes #[[ATTRS]] = { mustprogress nofree norecurse nosync nounwind willreturn memory(none) "approx-func-fp-math"="true" "no-infs-fp-math"="true" "no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" "unsafe-fp-math"="true" }
+
+! CHECK-FFAST-MATH-LABEL: define void @func_() local_unnamed_addr
+! CHECK-FFAST-MATH-SAME: #[[ATTRS:[0-9]+]]
+! CHECK-FFAST-MATH: attributes #[[ATTRS]] = { mustprogress nofree norecurse nosync nounwind willreturn memory(none) "approx-func-fp-math"="true" "no-infs-fp-math"="true" "no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" "unsafe-fp-math"="true" }
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index 01d476f530b1c5..ad67fba5a81cf8 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
@@ -1428,7 +1428,12 @@ def LLVM_LLVMFuncOp : LLVM_Op<"func", [
     OptionalAttr<LLVM_VScaleRangeAttr>:$vscale_range,
     OptionalAttr<FramePointerKindAttr>:$frame_pointer,
     OptionalAttr<StrAttr>:$target_cpu,
-    OptionalAttr<LLVM_TargetFeaturesAttr>:$target_features
+    OptionalAttr<LLVM_TargetFeaturesAttr>:$target_features,
+    OptionalAttr<BoolAttr>:$unsafe_fp_math,
+    OptionalAttr<BoolAttr>:$no_infs_fp_math,
+    OptionalAttr<BoolAttr>:$no_nans_fp_math,
+    OptionalAttr<BoolAttr>:$approx_func_fp_math,
+    OptionalAttr<BoolAttr>:$no_signed_zeros_fp_math
   );
 
   let regions = (region AnyRegion:$body);
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index 928d8077175ccf..5ca4a9fd68d650 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -1646,6 +1646,11 @@ static constexpr std::array ExplicitAttributes{
     StringLiteral("vscale_range"),
     StringLiteral("frame-pointer"),
     StringLiteral("target-features"),
+    StringLiteral("unsafe-fp-math"),
+    StringLiteral("no-infs-fp-math"),
+    StringLiteral("no-nans-fp-math"),
+    StringLiteral("approx-func-fp-math"),
+    StringLiteral("no-signed-zeros-fp-math"),
 };
 
 static void processPassthroughAttrs(llvm::Function *func, LLVMFuncOp funcOp) {
@@ -1752,6 +1757,26 @@ void ModuleImport::processFunctionAttributes(llvm::Function *func,
       attr.isStringAttribute())
     funcOp.setTargetFeaturesAttr(
         LLVM::TargetFeaturesAttr::get(context, attr.getValueAsString()));
+
+  if (llvm::Attribute attr = func->getFnAttribute("unsafe-fp-math");
+      attr.isStringAttribute())
+    funcOp.setUnsafeFpMath(attr.getValueAsBool());
+
+  if (llvm::Attribute attr = func->getFnAttribute("no-infs-fp-math");
+      attr.isStringAttribute())
+    funcOp.setNoInfsFpMath(attr.getValueAsBool());
+
+  if (llvm::Attribute attr = func->getFnAttribute("no-nans-fp-math");
+      attr.isStringAttribute())
+    funcOp.setNoNansFpMath(attr.getValueAsBool());
+
+  if (llvm::Attribute attr = func->getFnAttribute("approx-func-fp-math");
+      attr.isStringAttribute())
+    funcOp.setApproxFuncFpMath(attr.getValueAsBool());
+
+  if (llvm::Attribute attr = func->getFnAttribute("no-signed-zeros-fp-math");
+      attr.isStringAttribute())
+    funcOp.setNoSignedZerosFpMath(attr.getValueAsBool());
 }
 
 DictionaryAttr
diff --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
index 69a1cbe5969e85..6364cacbd19245 100644
--- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -37,6 +37,7 @@
 
 #include "llvm/ADT/PostOrderIterator.h"
 #include "llvm/ADT/SetVector.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/TypeSwitch.h"
 #include "llvm/Frontend/OpenMP/OMPIRBuilder.h"
 #include "llvm/IR/BasicBlock.h"
@@ -1214,6 +1215,23 @@ LogicalResult ModuleTranslation::convertOneFunction(LLVMFuncOp func) {
         getLLVMContext(), attr->getMinRange().getInt(),
         attr->getMaxRange().getInt()));
 
+  if (auto unsafeFpMath = func.getUnsafeFpMath())
+    llvmFunc->addFnAttr("unsafe-fp-math", llvm::toStringRef(*unsafeFpMath));
+
+  if (auto noInfsFpMath = func.getNoInfsFpMath())
+    llvmFunc->addFnAttr("no-infs-fp-math", llvm::toStringRef(*noInfsFpMath));
+
+  if (auto noNansFpMath = func.getNoNansFpMath())
+    llvmFunc->addFnAttr("no-nans-fp-math", llvm::toStringRef(*noNansFpMath));
+
+  if (auto approxFuncFpMath = func.getApproxFuncFpMath())
+    llvmFunc->addFnAttr("approx-func-fp-math",
+                        llvm::toStringRef(*approxFuncFpMath));
+
+  if (auto noSignedZerosFpMath = func.getNoSignedZerosFpMath())
+    llvmFunc->addFnAttr("no-signed-zeros-fp-math",
+                        llvm::toStringRef(*noSignedZerosFpMath));
+
   // Add function attribute frame-pointer, if found.
   if (FramePointerKindAttr attr = func.getFramePointerAttr())
     llvmFunc->addFnAttr("frame-pointer",
diff --git a/mlir/test/Dialect/LLVMIR/func.mlir b/mlir/test/Dialect/LLVMIR/func.mlir
index 9dc1bc57034e02..006f2f64a27277 100644
--- a/mlir/test/Dialect/LLVMIR/func.mlir
+++ b/mlir/test/Dialect/LLVMIR/func.mlir
@@ -257,6 +257,36 @@ module {
   llvm.func @frame_pointer_roundtrip() attributes {frame_pointer = #llvm.framePointerKind<"non-leaf">} {
     llvm.return
   }
+
+  llvm.func @unsafe_fp_math_roundtrip() attributes {unsafe_fp_math = true} {
+    // CHECK: @unsafe_fp_math_roundtrip
+    // CHECK-SAME: attributes {unsafe_fp_math = true}
+    llvm.return
+  }
+
+  llvm.func @no_infs_fp_math_roundtrip() attributes {no_infs_fp_math = true} {
+    // CHECK: @no_infs_fp_math_roundtrip
+    // CHECK-SAME: attributes {no_infs_fp_math = true}
+    llvm.return
+  }
+
+  llvm.func @no_nans_fp_math_roundtrip() attributes {no_nans_fp_math = true} {
+    // CHECK: @no_nans_fp_math_roundtrip
+    // CHECK-SAME: attributes {no_nans_fp_math = true}
+    llvm.return
+  }
+
+  llvm.func @approx_func_fp_math_roundtrip() attributes {approx_func_fp_math = true} {
+    // CHECK: @approx_func_fp_math_roundtrip
+    // CHECK-SAME: attributes {approx_func_fp_math = true}
+    llvm.return
+  }
+
+  llvm.func @no_signed_zeros_fp_math_roundtrip() attributes {no_signed_zeros_fp_math = true} {
+    // CHECK: @no_signed_zeros_fp_math_roundtrip
+    // CHECK-SAME: attributes {no_signed_zeros_fp_math = true}
+    llvm.return
+  }
 }
 
 // -----
diff --git a/mlir/test/Target/LLVMIR/Import/function-attributes.ll b/mlir/test/Target/LLVMIR/Import/function-attributes.ll
index af2ef9db96ae33..70387789aa97e4 100644
--- a/mlir/test/Target/LLVMIR/Import/function-attributes.ll
+++ b/mlir/test/Target/LLVMIR/Import/function-attributes.ll
@@ -272,3 +272,33 @@ define void @align_func() align 2 {
 ; CHECK-LABEL: @align_decl
 ; CHECK-SAME: attributes {alignment = 64 : i64}
 declare void @align_decl() align 64
+
+; // -----
+
+; CHECK-LABEL: @func_attr_unsafe_fp_math
+; CHECK-SAME: attributes {unsafe_fp_math = true}
+declare void @func_attr_unsafe_fp_math() "unsafe-fp-math"="true"
+
+; // -----
+
+; CHECK-LABEL: @func_attr_no_infs_fp_math
+; CHECK-SAME: attributes {no_infs_fp_math = true}
+declare void @func_attr_no_infs_fp_math() "no-infs-fp-math"="true"
+
+; // -----
+
+; CHECK-LABEL: @func_attr_no_nans_fp_math
+; CHECK-SAME: attributes {no_nans_fp_math = true}
+declare void @func_attr_no_nans_fp_math() "no-nans-fp-math"="true"
+
+; // -----
+
+; CHECK-LABEL: @func_attr_approx_func_fp_math
+; CHECK-SAME: attributes {approx_func_fp_math = true}
+declare void @func_attr_approx_func_fp_math() "approx-func-fp-math"="true"
+
+; // -----
+
+; CHECK-LABEL: @func_attr_no_signed_zeros_fp_math
+; CHECK-SAME: attributes {no_signed_zeros_fp_math = true}
+declare void @func_attr_no_signed_zeros_fp_math() "no-signed-zeros-fp-math"="true"
diff --git a/mlir/test/Target/LLVMIR/fp-math-function-attributes.mlir b/mlir/test/Target/LLVMIR/fp-math-function-attributes.mlir
new file mode 100644
index 00000000000000..ccae06cc3dd76d
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/fp-math-function-attributes.mlir
@@ -0,0 +1,44 @@
+// RUN: mlir-translate -mlir-to-llvmir -split-input-file %s | FileCheck %s
+
+// CHECK-LABEL: define void @unsafe_fp_math_func()
+// CHECK-SAME: #[[ATTRS:[0-9]+]]
+llvm.func @unsafe_fp_math_func() attributes {unsafe_fp_math = true}  {
+  llvm.return
+}
+// CHECK: attributes #[[ATTRS]] = { "unsafe-fp-math"="true" }
+
+// -----
+
+// CHECK-LABEL: define void @no_infs_fp_math_func()
+// CHECK-SAME: #[[ATTRS:[0-9]+]]
+llvm.func @no_infs_fp_math_func() attributes {no_infs_fp_math = true}  {
+  llvm.return
+}
+// CHECK: attributes #[[ATTRS]] = { "no-infs-fp-math"="true" }
+
+// -----
+
+// CHECK-LABEL: define void @no_nans_fp_math_func()
+// CHECK-SAME: #[[ATTRS:[0-9]+]]
+llvm.func @no_nans_fp_math_func() attributes {no_nans_fp_math = true}  {
+  llvm.return
+}
+// CHECK: attributes #[[ATTRS]] = { "no-nans-fp-math"="true" }
+
+// -----
+
+// CHECK-LABEL: define void @approx_func_fp_math_func()
+// CHECK-SAME: #[[ATTRS:[0-9]+]]
+llvm.func @approx_func_fp_math_func() attributes {approx_func_fp_math = true}  {
+  llvm.return
+}
+// CHECK: attributes #[[ATTRS]] = { "approx-func-fp-math"="true" }
+
+// -----
+
+// CHECK-LABEL: define void @no_signed_zeros_fp_math_func()
+// CHECK-SAME: #[[ATTRS:[0-9]+]]
+llvm.func @no_signed_zeros_fp_math_func() attributes {no_signed_zeros_fp_math = true}  {
+  llvm.return
+}
+// CHECK: attributes #[[ATTRS]] = { "no-signed-zeros-fp-math"="true" }

asb · 2024-01-29T11:53:25Z

Now ready for review - and sets all the fast-math related attributes using the same logic as Clang. The MLIR side is split out at #79812. I'm not as familiar with flang reviewers as other parts of LLVM, so please add yourself / add others as appropriate.

tblah

Thanks for working on this! LGTM

vzakhari · 2024-01-30T02:00:36Z

flang/lib/Optimizer/Transforms/FunctionAttr.cpp

@@ -45,14 +50,32 @@ void FunctionAttrPass::runOnOperation() {
    func->setAttr("frame_pointer", mlir::LLVM::FramePointerKindAttr::get(
                                       context, framePointerKind));

+  if (noInfsFPMath)
+    func->setAttr("no_infs_fp_math", mlir::BoolAttr::get(context, true));


I am not a fan of using literal constants while they have to be kept in sync between multiple components. Is it reasonable to make them named attributes in LLVM dialect and provide the name getters for them in LLVMDialect.td?

Adding a function to get the attribute name from LLVMDialect.td is also not completely save since the attribute name on LLVMFuncOp is generated based on the ODS operation definition. That means things could still get out of sync.

Could it potentially make sense to run this pass after lowering to LLVMFuncOp? It would have setters for the attributes. Alternatively, LLVMFuncOp has getters to obtain the attribute names.

I'm happy to go with whatever is preferred here, but I agree with @gysit that it's not clear adding such getters would be a big win. We're emitting the attribute once in our code base, and if it was typoed we'd see it fail to show up (due to being unrecognised) in the test case I added.

My main concern is that people may change the naming of the MLIR attributes in LLVM dialect, and do not run any Flang testing (I think it is a usual practice to limit the testing to the components that your changes affect). Flang LIT testing will break after the commit, while it is preferable not to have the breakage in the first place. So I would prefer any solution that breaks the names dependency between Flang and LLVM dialect. Using the getters from LLVMFuncOp sounds acceptable. Moving this pass after the FIRToLLVMLowering pass seems to be an option as well.

@vzakhari Though it's probably common to not build-test flang as well in that kind of scenario. I'm not sure I see any appropriate static getters in LLVMFuncOp that would get this information, and I don't believe I can get an instance of an LLVMFuncOp at this stage (though could be wrong - I'm new to MLIR and Flang). Could you please point to what you were imagining I could use?

After #79812 we should get static methods in LLVMFuncOp class that return the required attribute names, e.g. getUnsafeFpMathAttrName, getNoNansFpMathAttrName, etc. You may look it up in tools/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.h.inc in your llvm build location.

Thanks. I've now modified to use those static helpers. They require an mlir::OperationName parameter, which I've had to produce manually with mlir::OperationName("llvm.func", context); (at this stage, func.getOperationName returns "func.func").

The implemented logic matches the logic used for Clang in emitting these attributes. Although it's hoped that function attributes won't be needed in the future (vs using fast math flags in individual IR instructions), there are codegen differences currently with/without these attributes, as can be seen in issues like llvm#79257 or by hacking Clang to avoid producing these attributes and observing codegen changes.

…ributes being produced (e.g. target-cpu, target-features)

asb · 2024-01-30T15:52:44Z

Rebased now that the dependent MLIR patch has landed and adjusted test case now that target-cpu and target-features are also produced in emitted IR.

flang/test/Driver/func-attr-fast-math.f90

As requested during review.

asb · 2024-02-05T10:42:24Z

I believe all review comments are now addressed.

banach-space · 2024-02-05T10:49:49Z

I believe all review comments are now addressed.

The driver changes LGTM, thanks!

asb · 2024-02-05T11:27:53Z

Thanks - I'll wait on @vzakhari to confirm they're happy with the changes to attribute name handling before committing.

vzakhari

Thank you! Looks good (with one minor suggestions inlined).

flang/lib/Optimizer/Transforms/FunctionAttr.cpp

…ath (llvm#79301) The implemented logic matches the logic used for Clang in emitting these attributes. Although it's hoped that function attributes won't be needed in the future (vs using fast math flags in individual IR instructions), there are codegen differences currently with/without these attributes, as can be seen in issues like llvm#79257 or by hacking Clang to avoid producing these attributes and observing codegen changes.

asb force-pushed the 2024q1-flang-add-unsafe-fp-math-for-ofast branch from 5548f7d to 6ad7944 Compare January 24, 2024 15:50

asb force-pushed the 2024q1-flang-add-unsafe-fp-math-for-ofast branch from 082b789 to e245fcd Compare January 29, 2024 11:43

asb changed the title ~~[flang][Draft/RFC] Set unsafe_fp_math attribute for -ffast-math~~ [flang] Set fast math related function attributes for -Ofast/-ffast-math Jan 29, 2024

asb marked this pull request as ready for review January 29, 2024 11:45

llvmbot added mlir:llvm flang:driver mlir flang Flang issues not falling into any other category flang:fir-hlfir labels Jan 29, 2024

asb requested review from davemgreen and kiranchandramohan January 29, 2024 11:50

asb mentioned this pull request Jan 29, 2024

[Flang] TSVC s314: needs fast-math flags in function attributes for vectorization #79257

Closed

davemgreen requested review from tblah and jeanPerier January 29, 2024 11:55

tblah approved these changes Jan 29, 2024

View reviewed changes

tblah requested a review from vzakhari January 29, 2024 13:48

vzakhari reviewed Jan 30, 2024

View reviewed changes

asb added 2 commits January 30, 2024 14:04

Modify func-attr-fast-math.f90 test case to be resilient to other att…

a8c899b

…ributes being produced (e.g. target-cpu, target-features)

asb force-pushed the 2024q1-flang-add-unsafe-fp-math-for-ofast branch from a39abb4 to a8c899b Compare January 30, 2024 15:48

banach-space reviewed Jan 30, 2024

View reviewed changes

flang/test/Driver/func-attr-fast-math.f90 Outdated Show resolved Hide resolved

asb added 2 commits January 30, 2024 17:15

func-attr-fast-math.f90 doesn't need to be x86-specific

02f0070

Use static LLVMFuncOp helpers to get attribute names

416c7d9

As requested during review.

banach-space approved these changes Feb 5, 2024

View reviewed changes

vzakhari approved these changes Feb 5, 2024

View reviewed changes

flang/lib/Optimizer/Transforms/FunctionAttr.cpp Outdated Show resolved Hide resolved

Typo fix, and apply suggestion from review

6188298

asb merged commit 22544e2 into llvm:main Feb 5, 2024

[flang] Set fast math related function attributes for -Ofast/-ffast-math #79301

[flang] Set fast math related function attributes for -Ofast/-ffast-math #79301

Uh oh!

Conversation

asb commented Jan 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kiranchandramohan commented Jan 24, 2024

Uh oh!

asb commented Jan 24, 2024

Uh oh!

llvmbot commented Jan 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jan 29, 2024

Uh oh!

asb commented Jan 29, 2024

Uh oh!

tblah left a comment

Choose a reason for hiding this comment

Uh oh!

vzakhari Jan 30, 2024

Choose a reason for hiding this comment

Uh oh!

gysit Jan 30, 2024

Choose a reason for hiding this comment

Uh oh!

asb Jan 30, 2024

Choose a reason for hiding this comment

Uh oh!

vzakhari Jan 30, 2024

Choose a reason for hiding this comment

Uh oh!

asb Jan 31, 2024

Choose a reason for hiding this comment

Uh oh!

vzakhari Jan 31, 2024

Choose a reason for hiding this comment

Uh oh!

asb Feb 5, 2024

Choose a reason for hiding this comment

Uh oh!

asb commented Jan 30, 2024

Uh oh!

Uh oh!

asb commented Feb 5, 2024

Uh oh!

banach-space commented Feb 5, 2024

Uh oh!

asb commented Feb 5, 2024

Uh oh!

vzakhari left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

asb commented Jan 24, 2024 •

edited

Loading

github-actions bot commented Jan 24, 2024 •

edited

Loading

llvmbot commented Jan 29, 2024 •

edited

Loading