[flang] Pass to add frame pointer attribute #74598

Radu2k · 2023-12-06T14:46:06Z

Pass to add frame pointer attribute in Flang

llvmbot · 2023-12-06T14:46:40Z

@llvm/pr-subscribers-flang-driver

Author: Radu Salavat (Radu2k)

Changes

Pass to add frame pointer attribute in Flang

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

17 Files Affected:

(modified) flang/include/flang/Frontend/CodeGenOptions.def (+1)
(modified) flang/include/flang/Optimizer/Transforms/Passes.h (+8)
(modified) flang/include/flang/Optimizer/Transforms/Passes.td (+17)
(modified) flang/include/flang/Tools/CLOptions.inc (+4)
(modified) flang/include/flang/Tools/CrossToolHelpers.h (+2)
(modified) flang/lib/Frontend/CompilerInvocation.cpp (+13)
(modified) flang/lib/Optimizer/Transforms/CMakeLists.txt (+1)
(added) flang/lib/Optimizer/Transforms/FunctionAttr.cpp (+97)
(added) flang/test/Driver/func-attr.f90 (+12)
(modified) flang/test/Driver/mlir-debug-pass-pipeline.f90 (+2)
(modified) flang/test/Driver/mlir-pass-pipeline.f90 (+2)
(modified) flang/test/Driver/save-mlir-temps.f90 (+2-2)
(modified) flang/test/Fir/basic-program.fir (+2)
(modified) flang/test/Fir/box-offset-codegen.fir (+4-4)
(modified) flang/test/Fir/polymorphic.fir (+4-4)
(modified) flang/test/Fir/tbaa-codegen.fir (+1-1)
(modified) flang/test/Fir/tbaa-codegen2.fir (+1-1)

diff --git a/flang/include/flang/Frontend/CodeGenOptions.def b/flang/include/flang/Frontend/CodeGenOptions.def
index 72e7bdab12a14..774f225974d7e 100644
--- a/flang/include/flang/Frontend/CodeGenOptions.def
+++ b/flang/include/flang/Frontend/CodeGenOptions.def
@@ -38,6 +38,7 @@ CODEGENOPT(Underscoring, 1, 1)
 ENUM_CODEGENOPT(RelocationModel, llvm::Reloc::Model, 3, llvm::Reloc::PIC_) ///< Name of the relocation model to use.
 ENUM_CODEGENOPT(DebugInfo,  llvm::codegenoptions::DebugInfoKind, 4,  llvm::codegenoptions::NoDebugInfo) ///< Level of debug info to generate
 ENUM_CODEGENOPT(VecLib, llvm::driver::VectorLibrary, 3, llvm::driver::VectorLibrary::NoLibrary) ///< Vector functions library to use
+ENUM_CODEGENOPT(FramePointer, llvm::FramePointerKind, 2, llvm::FramePointerKind::None) /// frame-pointer: all,non-leaf,none
 
 #undef CODEGENOPT
 #undef ENUM_CODEGENOPT
diff --git a/flang/include/flang/Optimizer/Transforms/Passes.h b/flang/include/flang/Optimizer/Transforms/Passes.h
index 92bc7246eca70..c5f09fc2581bd 100644
--- a/flang/include/flang/Optimizer/Transforms/Passes.h
+++ b/flang/include/flang/Optimizer/Transforms/Passes.h
@@ -12,6 +12,7 @@
 #include "flang/Optimizer/Dialect/FIROps.h"
 #include "mlir/Pass/Pass.h"
 #include "mlir/Pass/PassRegistry.h"
+#include "llvm/Support/CodeGen.h"
 #include <memory>
 
 namespace mlir {
@@ -83,6 +84,13 @@ std::unique_ptr<mlir::Pass> createVScaleAttrPass();
 std::unique_ptr<mlir::Pass>
 createVScaleAttrPass(std::pair<unsigned, unsigned> vscaleAttr);
 
+struct FunctionAttrTypes{
+    llvm::FramePointerKind framePointerKind;
+};
+
+std::unique_ptr<mlir::Pass> createFunctionAttrPass();
+std::unique_ptr<mlir::Pass> createFunctionAttrPass(FunctionAttrTypes &functionAttr);
+
 // declarative passes
 #define GEN_PASS_REGISTRATION
 #include "flang/Optimizer/Transforms/Passes.h.inc"
diff --git a/flang/include/flang/Optimizer/Transforms/Passes.td b/flang/include/flang/Optimizer/Transforms/Passes.td
index c3768fd2d689c..eddf94e162e9c 100644
--- a/flang/include/flang/Optimizer/Transforms/Passes.td
+++ b/flang/include/flang/Optimizer/Transforms/Passes.td
@@ -349,4 +349,21 @@ def VScaleAttr : Pass<"vscale-attr", "mlir::func::FuncOp"> {
   let constructor = "::fir::createVScaleAttrPass()";
 }
 
+def FunctionAttr : Pass<"function-attr", "mlir::func::FuncOp"> {
+  let summary = "Enhance functions with different attributes";
+  let description = [{ This feature introduces a general attribute aimed at customizing function characteristics. 
+     Options include:
+     Add "frame-pointer" attribute to functions: Set an attribute for the frame pointer on functions, to enable 
+     the compiler, to avoid saving the frame pointer in a register in functions where it is unnecessary. 
+     This eliminates the need for instructions to save, establish, and restore frame pointers, while also freeing up an additional register in numerous functions. 
+     However, this approach can make debugging unfeasible on certain machines.
+  }];
+  let options = [
+    Option<"framePointerKind", "frame-pointer",
+           "mlir::LLVM::framePointerKind::FramePointerKind", /*default=*/"mlir::LLVM::framePointerKind::FramePointerKind{}",
+           "frame pointer">,
+  ];
+  let constructor = "::fir::createFunctionAttrPass()";
+}
+
 #endif // FLANG_OPTIMIZER_TRANSFORMS_PASSES
diff --git a/flang/include/flang/Tools/CLOptions.inc b/flang/include/flang/Tools/CLOptions.inc
index d3e4dc6cd4a24..3494e588b3460 100644
--- a/flang/include/flang/Tools/CLOptions.inc
+++ b/flang/include/flang/Tools/CLOptions.inc
@@ -311,6 +311,10 @@ inline void createDefaultFIRCodeGenPassPipeline(
   if (config.VScaleMin != 0)
     pm.addPass(fir::createVScaleAttrPass({config.VScaleMin, config.VScaleMax}));
 
+  fir::FunctionAttrTypes functionAttrs;
+  functionAttrs.framePointerKind = config.FramePointerKind;
+  pm.addPass(fir::createFunctionAttrPass(functionAttrs));
+
   fir::addFIRToLLVMPass(pm, config);
 }
 
diff --git a/flang/include/flang/Tools/CrossToolHelpers.h b/flang/include/flang/Tools/CrossToolHelpers.h
index ddec70fa9824c..e8722ebf77cd1 100644
--- a/flang/include/flang/Tools/CrossToolHelpers.h
+++ b/flang/include/flang/Tools/CrossToolHelpers.h
@@ -35,6 +35,7 @@ struct MLIRToLLVMPassPipelineConfig {
     LoopVersioning = opts.LoopVersioning;
     DebugInfo = opts.getDebugInfo();
     AliasAnalysis = opts.AliasAnalysis;
+    FramePointerKind = opts.getFramePointer();
   }
 
   llvm::OptimizationLevel OptLevel; ///< optimisation level
@@ -44,6 +45,7 @@ struct MLIRToLLVMPassPipelineConfig {
   bool AliasAnalysis = false; ///< Add TBAA tags to generated LLVMIR
   llvm::codegenoptions::DebugInfoKind DebugInfo =
       llvm::codegenoptions::NoDebugInfo; ///< Debug info generation.
+  llvm::FramePointerKind FramePointerKind;
   unsigned VScaleMin = 0; ///< SVE vector range minimum.
   unsigned VScaleMax = 0; ///< SVE vector range maximum.
 };
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index b3f32bb241d06..a89a93c2d3197 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -245,6 +245,19 @@ static void parseCodeGenArgs(Fortran::frontend::CodeGenOptions &opts,
 
   opts.AliasAnalysis = opts.OptimizationLevel > 0;
 
+  if (const llvm::opt::Arg *a =
+          args.getLastArg(clang::driver::options::OPT_mframe_pointer_EQ)) {
+    llvm::StringRef s = a->getValue();
+    assert(s == "none" || s == "non-leaf"|| s == "all");
+    if (s == "none")
+      opts.setFramePointer(llvm::FramePointerKind::None);
+    else
+      if (s == "non-leaf")
+        opts.setFramePointer(llvm::FramePointerKind::NonLeaf);
+      else
+        opts.setFramePointer(llvm::FramePointerKind::All);
+  }
+
   for (auto *a : args.filtered(clang::driver::options::OPT_fpass_plugin_EQ))
     opts.LLVMPassPlugins.push_back(a->getValue());
 
diff --git a/flang/lib/Optimizer/Transforms/CMakeLists.txt b/flang/lib/Optimizer/Transforms/CMakeLists.txt
index 03b67104a93b5..fc067ad358539 100644
--- a/flang/lib/Optimizer/Transforms/CMakeLists.txt
+++ b/flang/lib/Optimizer/Transforms/CMakeLists.txt
@@ -20,6 +20,7 @@ add_flang_library(FIRTransforms
   OMPFunctionFiltering.cpp
   OMPMarkDeclareTarget.cpp
   VScaleAttr.cpp
+  FunctionAttr.cpp
 
   DEPENDS
   FIRDialect
diff --git a/flang/lib/Optimizer/Transforms/FunctionAttr.cpp b/flang/lib/Optimizer/Transforms/FunctionAttr.cpp
new file mode 100644
index 0000000000000..dd7e3947f09e5
--- /dev/null
+++ b/flang/lib/Optimizer/Transforms/FunctionAttr.cpp
@@ -0,0 +1,97 @@
+//===- FunctionAttr.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
+//
+//===----------------------------------------------------------------------===//
+
+//===----------------------------------------------------------------------===//
+/// \file
+/// This pass adds a `vscale_range` attribute to function definitions.
+/// The attribute is used for scalable vector operations on Arm processors
+/// and should only be run on processors that support this feature. [It is
+/// likely harmless to run it on something else, but it is also not valuable].
+//===----------------------------------------------------------------------===//
+
+#include "flang/ISO_Fortran_binding_wrapper.h"
+#include "flang/Optimizer/Builder/BoxValue.h"
+#include "flang/Optimizer/Builder/FIRBuilder.h"
+#include "flang/Optimizer/Builder/Runtime/Inquiry.h"
+#include "flang/Optimizer/Dialect/FIRDialect.h"
+#include "flang/Optimizer/Dialect/FIROps.h"
+#include "flang/Optimizer/Dialect/FIRType.h"
+#include "flang/Optimizer/Dialect/Support/FIRContext.h"
+#include "flang/Optimizer/Dialect/Support/KindMapping.h"
+#include "flang/Optimizer/Transforms/Passes.h"
+#include "mlir/Dialect/LLVMIR/LLVMAttrs.h"
+#include "mlir/Dialect/LLVMIR/LLVMDialect.h"
+#include "mlir/IR/Matchers.h"
+#include "mlir/IR/TypeUtilities.h"
+#include "mlir/Pass/Pass.h"
+#include "mlir/Transforms/DialectConversion.h"
+#include "mlir/Transforms/GreedyPatternRewriteDriver.h"
+#include "mlir/Transforms/RegionUtils.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/raw_ostream.h"
+
+#include <algorithm>
+
+namespace fir {
+#define GEN_PASS_DECL_FUNCTIONATTR
+#define GEN_PASS_DEF_FUNCTIONATTR
+#include "flang/Optimizer/Transforms/Passes.h.inc"
+} // namespace fir
+
+#define DEBUG_TYPE "func-attr"
+
+namespace {
+
+class FunctionAttrPass : public fir::impl::FunctionAttrBase<FunctionAttrPass> {
+public:
+  FunctionAttrPass(const fir::FunctionAttrOptions &options) {
+    framePointerKind = options.framePointerKind;
+  }
+  FunctionAttrPass() {
+    
+  }
+  void runOnOperation() override;
+};
+
+} // namespace
+
+void FunctionAttrPass::runOnOperation() {
+  LLVM_DEBUG(llvm::dbgs() << "=== Begin " DEBUG_TYPE " ===\n");
+  mlir::func::FuncOp func = getOperation();
+
+  LLVM_DEBUG(llvm::dbgs() << "Func-name:" << func.getSymName() << "\n");
+
+  auto context = &getContext();
+  
+  func->setAttr("frame_pointer", mlir::LLVM::FramePointerKindAttr::get( context, framePointerKind ));
+
+  LLVM_DEBUG(llvm::dbgs() << "=== End " DEBUG_TYPE " ===\n");
+}
+
+std::unique_ptr<mlir::Pass> fir::createFunctionAttrPass(fir::FunctionAttrTypes &functionAttr) {
+  FunctionAttrOptions opts;
+
+  // Frame pointer 
+  switch (functionAttr.framePointerKind) {
+  case llvm::FramePointerKind::None:
+    opts.framePointerKind = mlir::LLVM::framePointerKind::symbolizeFramePointerKind(0).value();
+    break;
+  case llvm::FramePointerKind::NonLeaf:
+    opts.framePointerKind = mlir::LLVM::framePointerKind::symbolizeFramePointerKind(1).value();
+    break;
+  case llvm::FramePointerKind::All:
+    opts.framePointerKind = mlir::LLVM::framePointerKind::symbolizeFramePointerKind(2).value();
+    break;
+  }
+
+  return std::make_unique<FunctionAttrPass>(opts);
+}
+  
+std::unique_ptr<mlir::Pass> fir::createFunctionAttrPass() {
+  return std::make_unique<FunctionAttrPass>();
+}
diff --git a/flang/test/Driver/func-attr.f90 b/flang/test/Driver/func-attr.f90
new file mode 100644
index 0000000000000..10fe4657746e4
--- /dev/null
+++ b/flang/test/Driver/func-attr.f90
@@ -0,0 +1,12 @@
+! RUN: %flang_fc1 -triple aarch64-none-none -mframe-pointer=none -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-NONEFP
+! RUN: %flang_fc1 -triple aarch64-none-none -mframe-pointer=non-leaf -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-NONLEAFFP
+! RUN: %flang_fc1 -triple aarch64-none-none -mframe-pointer=all -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-ALLFP
+
+! CHECK-LABEL: @func_() #0
+subroutine func
+end subroutine func
+
+! CHECK-NONEFP: attributes #0 = { "frame-pointer"="none" }
+! CHECK-NONLEAFFP: attributes #0 = { "frame-pointer"="non-leaf" }
+! CHECK-ALLFP: attributes #0 = { "frame-pointer"="all" }
+
diff --git a/flang/test/Driver/mlir-debug-pass-pipeline.f90 b/flang/test/Driver/mlir-debug-pass-pipeline.f90
index a3ff416f4d779..701537b339de9 100644
--- a/flang/test/Driver/mlir-debug-pass-pipeline.f90
+++ b/flang/test/Driver/mlir-debug-pass-pipeline.f90
@@ -82,5 +82,7 @@
 ! ALL-NEXT: ExternalNameConversion
 ! DEBUG-NEXT: AddDebugFoundation
 ! NO-DEBUG-NOT: AddDebugFoundation
+! ALL-NEXT:  'func.func' Pipeline
+! ALL-NEXT:    FunctionAttr
 ! ALL-NEXT: FIRToLLVMLowering
 ! ALL-NOT: LLVMIRLoweringPass
diff --git a/flang/test/Driver/mlir-pass-pipeline.f90 b/flang/test/Driver/mlir-pass-pipeline.f90
index 3d8c42f123e2e..f0dcf68195c4b 100644
--- a/flang/test/Driver/mlir-pass-pipeline.f90
+++ b/flang/test/Driver/mlir-pass-pipeline.f90
@@ -73,5 +73,7 @@
 ! ALL-NEXT:   (S) 0 num-dce'd - Number of operations eliminated
 ! ALL-NEXT: TargetRewrite
 ! ALL-NEXT: ExternalNameConversion
+! ALL-NEXT:  'func.func' Pipeline
+! ALL-NEXT:    FunctionAttr
 ! ALL-NEXT: FIRToLLVMLowering
 ! ALL-NOT: LLVMIRLoweringPass
diff --git a/flang/test/Driver/save-mlir-temps.f90 b/flang/test/Driver/save-mlir-temps.f90
index 50bc83030caa9..1c8935fbd7aac 100644
--- a/flang/test/Driver/save-mlir-temps.f90
+++ b/flang/test/Driver/save-mlir-temps.f90
@@ -51,9 +51,9 @@
 ! Content to check from the MLIR outputs
 !--------------------------
 ! MLIR-FIR-NOT: llvm.func
-! MLIR-FIR: func.func @{{.*}}main() {
+! MLIR-FIR: func.func @{{.*}}main(){{.*}}{
 
 ! MLIR-FIR-NOT: func.func
-! MLIR-LLVMIR: llvm.func @{{.*}}main() {
+! MLIR-LLVMIR: llvm.func @{{.*}}main(){{.*}}{
 
 end program
diff --git a/flang/test/Fir/basic-program.fir b/flang/test/Fir/basic-program.fir
index d8a9e74c318ce..601e34ee04c27 100644
--- a/flang/test/Fir/basic-program.fir
+++ b/flang/test/Fir/basic-program.fir
@@ -80,5 +80,7 @@ func.func @_QQmain() {
 // PASSES-NEXT: CodeGenRewrite
 // PASSES-NEXT:   (S) 0 num-dce'd - Number of operations eliminated
 // PASSES-NEXT: TargetRewrite
+// PASSES-NEXT:  'func.func' Pipeline
+// PASSES-NEXT:    FunctionAttr
 // PASSES-NEXT: FIRToLLVMLowering
 // PASSES-NEXT: LLVMIRLoweringPass
diff --git a/flang/test/Fir/box-offset-codegen.fir b/flang/test/Fir/box-offset-codegen.fir
index 600555cd94cea..389ceebcc065c 100644
--- a/flang/test/Fir/box-offset-codegen.fir
+++ b/flang/test/Fir/box-offset-codegen.fir
@@ -7,7 +7,7 @@ func.func @scalar_addr(%scalar : !fir.ref<!fir.box<!fir.type<t>>>) -> !fir.llvm_
   return %addr : !fir.llvm_ptr<!fir.ref<!fir.type<t>>>
 }
 // CHECK-LABEL: define ptr @scalar_addr(
-// CHECK-SAME: ptr %[[BOX:.*]]) {
+// CHECK-SAME: ptr %[[BOX:.*]]){{.*}}{
 // CHECK:    %[[VAL_0:.*]] = getelementptr { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] }, ptr %[[BOX]], i32 0, i32 0
 // CHECK:    ret ptr %[[VAL_0]]
 
@@ -16,7 +16,7 @@ func.func @scalar_tdesc(%scalar : !fir.ref<!fir.box<!fir.type<t>>>) -> !fir.llvm
   return %tdesc : !fir.llvm_ptr<!fir.tdesc<!fir.type<t>>>
 }
 // CHECK-LABEL: define ptr @scalar_tdesc(
-// CHECK-SAME: ptr %[[BOX:.*]]) {
+// CHECK-SAME: ptr %[[BOX:.*]]){{.*}}{
 // CHECK:    %[[VAL_0:.*]] = getelementptr { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] }, ptr %[[BOX]], i32 0, i32 7
 // CHECK:    ret ptr %[[VAL_0]]
 
@@ -25,7 +25,7 @@ func.func @array_addr(%array : !fir.ref<!fir.class<!fir.ptr<!fir.array<?x!fir.ty
   return %addr : !fir.llvm_ptr<!fir.ptr<!fir.array<?x!fir.type<t>>>>
 }
 // CHECK-LABEL: define ptr @array_addr(
-// CHECK-SAME: ptr %[[BOX:.*]]) {
+// CHECK-SAME: ptr %[[BOX:.*]]){{.*}}{
 // CHECK:    %[[VAL_0:.*]] = getelementptr { ptr, i64, i32, i8, i8, i8, i8, [1 x [3 x i64]], ptr, [1 x i64] }, ptr %[[BOX]], i32 0, i32 0
 // CHECK:    ret ptr %[[VAL_0]]
 
@@ -34,6 +34,6 @@ func.func @array_tdesc(%array : !fir.ref<!fir.class<!fir.ptr<!fir.array<?x!fir.t
   return %tdesc : !fir.llvm_ptr<!fir.tdesc<!fir.type<t>>>
 }
 // CHECK-LABEL: define ptr @array_tdesc(
-// CHECK-SAME: ptr %[[BOX:.*]]) {
+// CHECK-SAME: ptr %[[BOX:.*]]){{.*}}{
 // CHECK:    %[[VAL_0:.*]] = getelementptr { ptr, i64, i32, i8, i8, i8, i8, [1 x [3 x i64]], ptr, [1 x i64] }, ptr %[[BOX]], i32 0, i32 8
 // CHECK:    ret ptr %[[VAL_0]]
diff --git a/flang/test/Fir/polymorphic.fir b/flang/test/Fir/polymorphic.fir
index ce8e43b0be656..ecdcdeb575310 100644
--- a/flang/test/Fir/polymorphic.fir
+++ b/flang/test/Fir/polymorphic.fir
@@ -10,7 +10,7 @@ func.func @_QMpolymorphic_testPtest_allocate_unlimited_polymorphic_non_derived()
   return
 }
 
-// CHECK-LABEL: define void @_QMpolymorphic_testPtest_allocate_unlimited_polymorphic_non_derived() {
+// CHECK-LABEL: define void @_QMpolymorphic_testPtest_allocate_unlimited_polymorphic_non_derived(){{.*}}{
 // CHECK:   %[[MEM:.*]] = alloca { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] }
 // CHECK:   %[[DESC:.*]] = alloca { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] }, i64 1
 // CHECK:   store { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } { ptr null, i64 0, i32 20180515, i8 0, i8 -1, i8 1, i8 1, ptr null, [1 x i64] undef }, ptr %[[MEM]]
@@ -87,7 +87,7 @@ func.func @_QMunlimitedPsub1(%arg0: !fir.class<!fir.array<?xnone>> {fir.bindc_na
 }
 
 // CHECK-LABEL: define void @_QMunlimitedPsub1(
-// CHECK-SAME: ptr %[[ARRAY:.*]]) {
+// CHECK-SAME: ptr %[[ARRAY:.*]]){{.*}}{
 // CHECK: %[[BOX:.*]] = alloca { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] }
 // CHECK: %{{.}} = getelementptr { ptr, i64, i32, i8, i8, i8, i8, [1 x [3 x i64]], ptr, [1 x i64] }, ptr %[[ARRAY]], i32 0, i32 7, i32 0, i32 2
 // CHECK: %[[TYPE_DESC_GEP:.*]] = getelementptr { ptr, i64, i32, i8, i8, i8, i8, [1 x [3 x i64]], ptr, [1 x i64] }, ptr %[[ARRAY]], i32 0, i32 8
@@ -151,7 +151,7 @@ func.func @_QQmain() {
   return
 }
 
-// CHECK-LABEL: define void @_QQmain() {
+// CHECK-LABEL: define void @_QQmain(){{.*}}{
 // CHECK: %[[CLASS_NONE:.*]] = alloca { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] }
 // CHECK: %[[DESC:.*]] = alloca { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] }, i64 1
 // CHECK: store { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } { ptr @_QMmod1Ea, i64 ptrtoint (ptr getelementptr (%_QMmod1TtK2, ptr null, i32 1) to i64), i32 20180515, i8 0, i8 42, i8 1, i8 1, ptr @_QMmod1E.dt.t.2, [1 x i64] undef }, ptr %[[CLASS_NONE]], align 8
@@ -175,7 +175,7 @@ func.func @_QMmod2Pinitp(%arg0: !fir.ref<!fir.class<!fir.ptr<none>>> {fir.bindc_
 func.func private @_FortranAPointerAssociate(!fir.ref<!fir.box<none>>, !fir.box<none>) -> none attributes {fir.runtime}
 
 // CHECK-LABEL: define void @_QMmod2Pinitp(
-// CHECK-SAME: ptr %[[ARG0:.*]]) {
+// CHECK-SAME: ptr %[[ARG0:.*]]){{.*}}{
 // CHECK: %[[ALLOCA_CLASS_NONE:.*]] = alloca { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] }
 // CHECK: %[[LOAD:.*]] = load { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] }, ptr %[[ARG0]]
 // CHECK: store { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } %[[LOAD]], ptr %[[ALLOCA_CLASS_NONE]]
diff --git a/flang/test/Fir/tbaa-codegen.fir b/flang/test/Fir/tbaa-codegen.fir
index fd0eb9c7304ee..87bb15c0fea6c 100644
--- a/flang/test/Fir/tbaa-codegen.fir
+++ b/flang/test/Fir/tbaa-codegen.fir
@@ -28,7 +28,7 @@ module attributes {fir.defaultkind = "a1c4d8i4l4r4", fir.kindmap = "", llvm.targ
 }
 
 // CHECK-LABEL: define void @_QPsimple(
-// CHECK-SAME:      ptr %[[ARG0:.*]]) {
+// CHECK-SAME:      ptr %[[ARG0:.*]]){{.*}}{
 // [...]
 // load  a(2):
 // CHECK:  %[[VAL20:.*]] = getelementptr i8, ptr %{{.*}}, i64 %{{.*}}
diff --git a/flang/test/Fir/tbaa-codegen2.fir b/flang/test/Fir/tbaa-codegen2.fir
index d73a7b96a5386..e649c06731c6b 100644
--- a/flang/test/Fir/tbaa-codegen2.fir
+++ b/flang/test/Fir/tbaa-codegen2.fir
@@ -60,7 +60,7 @@ module attributes {fir.defaultkind = "a1c4d8i4l4r4", fir.kindmap = "", llvm.targ
   }
 }
 // CHECK-LABEL: define void @_QPfunc(
-// CHECK-SAME:      ptr %[[ARG0:.*]]) {
+// CHECK-SAME:      ptr %[[ARG0:.*]]){{.*}}{
 // [...]
 // CHECK:  %[[VAL5:.*]] = getelementptr { ptr, i64, i32, i8, i8, i8, i8, [1 x [3 x i64]] }, ptr %[[ARG0]], i32 0, i32 7, i32 0, i32 0
 // box access:

llvmbot · 2023-12-06T14:46:41Z

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

Author: Radu Salavat (Radu2k)

Changes

Pass to add frame pointer attribute in Flang

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

17 Files Affected:

(modified) flang/include/flang/Frontend/CodeGenOptions.def (+1)
(modified) flang/include/flang/Optimizer/Transforms/Passes.h (+8)
(modified) flang/include/flang/Optimizer/Transforms/Passes.td (+17)
(modified) flang/include/flang/Tools/CLOptions.inc (+4)
(modified) flang/include/flang/Tools/CrossToolHelpers.h (+2)
(modified) flang/lib/Frontend/CompilerInvocation.cpp (+13)
(modified) flang/lib/Optimizer/Transforms/CMakeLists.txt (+1)
(added) flang/lib/Optimizer/Transforms/FunctionAttr.cpp (+97)
(added) flang/test/Driver/func-attr.f90 (+12)
(modified) flang/test/Driver/mlir-debug-pass-pipeline.f90 (+2)
(modified) flang/test/Driver/mlir-pass-pipeline.f90 (+2)
(modified) flang/test/Driver/save-mlir-temps.f90 (+2-2)
(modified) flang/test/Fir/basic-program.fir (+2)
(modified) flang/test/Fir/box-offset-codegen.fir (+4-4)
(modified) flang/test/Fir/polymorphic.fir (+4-4)
(modified) flang/test/Fir/tbaa-codegen.fir (+1-1)
(modified) flang/test/Fir/tbaa-codegen2.fir (+1-1)

diff --git a/flang/include/flang/Frontend/CodeGenOptions.def b/flang/include/flang/Frontend/CodeGenOptions.def
index 72e7bdab12a14..774f225974d7e 100644
--- a/flang/include/flang/Frontend/CodeGenOptions.def
+++ b/flang/include/flang/Frontend/CodeGenOptions.def
@@ -38,6 +38,7 @@ CODEGENOPT(Underscoring, 1, 1)
 ENUM_CODEGENOPT(RelocationModel, llvm::Reloc::Model, 3, llvm::Reloc::PIC_) ///< Name of the relocation model to use.
 ENUM_CODEGENOPT(DebugInfo,  llvm::codegenoptions::DebugInfoKind, 4,  llvm::codegenoptions::NoDebugInfo) ///< Level of debug info to generate
 ENUM_CODEGENOPT(VecLib, llvm::driver::VectorLibrary, 3, llvm::driver::VectorLibrary::NoLibrary) ///< Vector functions library to use
+ENUM_CODEGENOPT(FramePointer, llvm::FramePointerKind, 2, llvm::FramePointerKind::None) /// frame-pointer: all,non-leaf,none
 
 #undef CODEGENOPT
 #undef ENUM_CODEGENOPT
diff --git a/flang/include/flang/Optimizer/Transforms/Passes.h b/flang/include/flang/Optimizer/Transforms/Passes.h
index 92bc7246eca70..c5f09fc2581bd 100644
--- a/flang/include/flang/Optimizer/Transforms/Passes.h
+++ b/flang/include/flang/Optimizer/Transforms/Passes.h
@@ -12,6 +12,7 @@
 #include "flang/Optimizer/Dialect/FIROps.h"
 #include "mlir/Pass/Pass.h"
 #include "mlir/Pass/PassRegistry.h"
+#include "llvm/Support/CodeGen.h"
 #include <memory>
 
 namespace mlir {
@@ -83,6 +84,13 @@ std::unique_ptr<mlir::Pass> createVScaleAttrPass();
 std::unique_ptr<mlir::Pass>
 createVScaleAttrPass(std::pair<unsigned, unsigned> vscaleAttr);
 
+struct FunctionAttrTypes{
+    llvm::FramePointerKind framePointerKind;
+};
+
+std::unique_ptr<mlir::Pass> createFunctionAttrPass();
+std::unique_ptr<mlir::Pass> createFunctionAttrPass(FunctionAttrTypes &functionAttr);
+
 // declarative passes
 #define GEN_PASS_REGISTRATION
 #include "flang/Optimizer/Transforms/Passes.h.inc"
diff --git a/flang/include/flang/Optimizer/Transforms/Passes.td b/flang/include/flang/Optimizer/Transforms/Passes.td
index c3768fd2d689c..eddf94e162e9c 100644
--- a/flang/include/flang/Optimizer/Transforms/Passes.td
+++ b/flang/include/flang/Optimizer/Transforms/Passes.td
@@ -349,4 +349,21 @@ def VScaleAttr : Pass<"vscale-attr", "mlir::func::FuncOp"> {
   let constructor = "::fir::createVScaleAttrPass()";
 }
 
+def FunctionAttr : Pass<"function-attr", "mlir::func::FuncOp"> {
+  let summary = "Enhance functions with different attributes";
+  let description = [{ This feature introduces a general attribute aimed at customizing function characteristics. 
+     Options include:
+     Add "frame-pointer" attribute to functions: Set an attribute for the frame pointer on functions, to enable 
+     the compiler, to avoid saving the frame pointer in a register in functions where it is unnecessary. 
+     This eliminates the need for instructions to save, establish, and restore frame pointers, while also freeing up an additional register in numerous functions. 
+     However, this approach can make debugging unfeasible on certain machines.
+  }];
+  let options = [
+    Option<"framePointerKind", "frame-pointer",
+           "mlir::LLVM::framePointerKind::FramePointerKind", /*default=*/"mlir::LLVM::framePointerKind::FramePointerKind{}",
+           "frame pointer">,
+  ];
+  let constructor = "::fir::createFunctionAttrPass()";
+}
+
 #endif // FLANG_OPTIMIZER_TRANSFORMS_PASSES
diff --git a/flang/include/flang/Tools/CLOptions.inc b/flang/include/flang/Tools/CLOptions.inc
index d3e4dc6cd4a24..3494e588b3460 100644
--- a/flang/include/flang/Tools/CLOptions.inc
+++ b/flang/include/flang/Tools/CLOptions.inc
@@ -311,6 +311,10 @@ inline void createDefaultFIRCodeGenPassPipeline(
   if (config.VScaleMin != 0)
     pm.addPass(fir::createVScaleAttrPass({config.VScaleMin, config.VScaleMax}));
 
+  fir::FunctionAttrTypes functionAttrs;
+  functionAttrs.framePointerKind = config.FramePointerKind;
+  pm.addPass(fir::createFunctionAttrPass(functionAttrs));
+
   fir::addFIRToLLVMPass(pm, config);
 }
 
diff --git a/flang/include/flang/Tools/CrossToolHelpers.h b/flang/include/flang/Tools/CrossToolHelpers.h
index ddec70fa9824c..e8722ebf77cd1 100644
--- a/flang/include/flang/Tools/CrossToolHelpers.h
+++ b/flang/include/flang/Tools/CrossToolHelpers.h
@@ -35,6 +35,7 @@ struct MLIRToLLVMPassPipelineConfig {
     LoopVersioning = opts.LoopVersioning;
     DebugInfo = opts.getDebugInfo();
     AliasAnalysis = opts.AliasAnalysis;
+    FramePointerKind = opts.getFramePointer();
   }
 
   llvm::OptimizationLevel OptLevel; ///< optimisation level
@@ -44,6 +45,7 @@ struct MLIRToLLVMPassPipelineConfig {
   bool AliasAnalysis = false; ///< Add TBAA tags to generated LLVMIR
   llvm::codegenoptions::DebugInfoKind DebugInfo =
       llvm::codegenoptions::NoDebugInfo; ///< Debug info generation.
+  llvm::FramePointerKind FramePointerKind;
   unsigned VScaleMin = 0; ///< SVE vector range minimum.
   unsigned VScaleMax = 0; ///< SVE vector range maximum.
 };
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index b3f32bb241d06..a89a93c2d3197 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -245,6 +245,19 @@ static void parseCodeGenArgs(Fortran::frontend::CodeGenOptions &opts,
 
   opts.AliasAnalysis = opts.OptimizationLevel > 0;
 
+  if (const llvm::opt::Arg *a =
+          args.getLastArg(clang::driver::options::OPT_mframe_pointer_EQ)) {
+    llvm::StringRef s = a->getValue();
+    assert(s == "none" || s == "non-leaf"|| s == "all");
+    if (s == "none")
+      opts.setFramePointer(llvm::FramePointerKind::None);
+    else
+      if (s == "non-leaf")
+        opts.setFramePointer(llvm::FramePointerKind::NonLeaf);
+      else
+        opts.setFramePointer(llvm::FramePointerKind::All);
+  }
+
   for (auto *a : args.filtered(clang::driver::options::OPT_fpass_plugin_EQ))
     opts.LLVMPassPlugins.push_back(a->getValue());
 
diff --git a/flang/lib/Optimizer/Transforms/CMakeLists.txt b/flang/lib/Optimizer/Transforms/CMakeLists.txt
index 03b67104a93b5..fc067ad358539 100644
--- a/flang/lib/Optimizer/Transforms/CMakeLists.txt
+++ b/flang/lib/Optimizer/Transforms/CMakeLists.txt
@@ -20,6 +20,7 @@ add_flang_library(FIRTransforms
   OMPFunctionFiltering.cpp
   OMPMarkDeclareTarget.cpp
   VScaleAttr.cpp
+  FunctionAttr.cpp
 
   DEPENDS
   FIRDialect
diff --git a/flang/lib/Optimizer/Transforms/FunctionAttr.cpp b/flang/lib/Optimizer/Transforms/FunctionAttr.cpp
new file mode 100644
index 0000000000000..dd7e3947f09e5
--- /dev/null
+++ b/flang/lib/Optimizer/Transforms/FunctionAttr.cpp
@@ -0,0 +1,97 @@
+//===- FunctionAttr.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
+//
+//===----------------------------------------------------------------------===//
+
+//===----------------------------------------------------------------------===//
+/// \file
+/// This pass adds a `vscale_range` attribute to function definitions.
+/// The attribute is used for scalable vector operations on Arm processors
+/// and should only be run on processors that support this feature. [It is
+/// likely harmless to run it on something else, but it is also not valuable].
+//===----------------------------------------------------------------------===//
+
+#include "flang/ISO_Fortran_binding_wrapper.h"
+#include "flang/Optimizer/Builder/BoxValue.h"
+#include "flang/Optimizer/Builder/FIRBuilder.h"
+#include "flang/Optimizer/Builder/Runtime/Inquiry.h"
+#include "flang/Optimizer/Dialect/FIRDialect.h"
+#include "flang/Optimizer/Dialect/FIROps.h"
+#include "flang/Optimizer/Dialect/FIRType.h"
+#include "flang/Optimizer/Dialect/Support/FIRContext.h"
+#include "flang/Optimizer/Dialect/Support/KindMapping.h"
+#include "flang/Optimizer/Transforms/Passes.h"
+#include "mlir/Dialect/LLVMIR/LLVMAttrs.h"
+#include "mlir/Dialect/LLVMIR/LLVMDialect.h"
+#include "mlir/IR/Matchers.h"
+#include "mlir/IR/TypeUtilities.h"
+#include "mlir/Pass/Pass.h"
+#include "mlir/Transforms/DialectConversion.h"
+#include "mlir/Transforms/GreedyPatternRewriteDriver.h"
+#include "mlir/Transforms/RegionUtils.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/raw_ostream.h"
+
+#include <algorithm>
+
+namespace fir {
+#define GEN_PASS_DECL_FUNCTIONATTR
+#define GEN_PASS_DEF_FUNCTIONATTR
+#include "flang/Optimizer/Transforms/Passes.h.inc"
+} // namespace fir
+
+#define DEBUG_TYPE "func-attr"
+
+namespace {
+
+class FunctionAttrPass : public fir::impl::FunctionAttrBase<FunctionAttrPass> {
+public:
+  FunctionAttrPass(const fir::FunctionAttrOptions &options) {
+    framePointerKind = options.framePointerKind;
+  }
+  FunctionAttrPass() {
+    
+  }
+  void runOnOperation() override;
+};
+
+} // namespace
+
+void FunctionAttrPass::runOnOperation() {
+  LLVM_DEBUG(llvm::dbgs() << "=== Begin " DEBUG_TYPE " ===\n");
+  mlir::func::FuncOp func = getOperation();
+
+  LLVM_DEBUG(llvm::dbgs() << "Func-name:" << func.getSymName() << "\n");
+
+  auto context = &getContext();
+  
+  func->setAttr("frame_pointer", mlir::LLVM::FramePointerKindAttr::get( context, framePointerKind ));
+
+  LLVM_DEBUG(llvm::dbgs() << "=== End " DEBUG_TYPE " ===\n");
+}
+
+std::unique_ptr<mlir::Pass> fir::createFunctionAttrPass(fir::FunctionAttrTypes &functionAttr) {
+  FunctionAttrOptions opts;
+
+  // Frame pointer 
+  switch (functionAttr.framePointerKind) {
+  case llvm::FramePointerKind::None:
+    opts.framePointerKind = mlir::LLVM::framePointerKind::symbolizeFramePointerKind(0).value();
+    break;
+  case llvm::FramePointerKind::NonLeaf:
+    opts.framePointerKind = mlir::LLVM::framePointerKind::symbolizeFramePointerKind(1).value();
+    break;
+  case llvm::FramePointerKind::All:
+    opts.framePointerKind = mlir::LLVM::framePointerKind::symbolizeFramePointerKind(2).value();
+    break;
+  }
+
+  return std::make_unique<FunctionAttrPass>(opts);
+}
+  
+std::unique_ptr<mlir::Pass> fir::createFunctionAttrPass() {
+  return std::make_unique<FunctionAttrPass>();
+}
diff --git a/flang/test/Driver/func-attr.f90 b/flang/test/Driver/func-attr.f90
new file mode 100644
index 0000000000000..10fe4657746e4
--- /dev/null
+++ b/flang/test/Driver/func-attr.f90
@@ -0,0 +1,12 @@
+! RUN: %flang_fc1 -triple aarch64-none-none -mframe-pointer=none -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-NONEFP
+! RUN: %flang_fc1 -triple aarch64-none-none -mframe-pointer=non-leaf -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-NONLEAFFP
+! RUN: %flang_fc1 -triple aarch64-none-none -mframe-pointer=all -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-ALLFP
+
+! CHECK-LABEL: @func_() #0
+subroutine func
+end subroutine func
+
+! CHECK-NONEFP: attributes #0 = { "frame-pointer"="none" }
+! CHECK-NONLEAFFP: attributes #0 = { "frame-pointer"="non-leaf" }
+! CHECK-ALLFP: attributes #0 = { "frame-pointer"="all" }
+
diff --git a/flang/test/Driver/mlir-debug-pass-pipeline.f90 b/flang/test/Driver/mlir-debug-pass-pipeline.f90
index a3ff416f4d779..701537b339de9 100644
--- a/flang/test/Driver/mlir-debug-pass-pipeline.f90
+++ b/flang/test/Driver/mlir-debug-pass-pipeline.f90
@@ -82,5 +82,7 @@
 ! ALL-NEXT: ExternalNameConversion
 ! DEBUG-NEXT: AddDebugFoundation
 ! NO-DEBUG-NOT: AddDebugFoundation
+! ALL-NEXT:  'func.func' Pipeline
+! ALL-NEXT:    FunctionAttr
 ! ALL-NEXT: FIRToLLVMLowering
 ! ALL-NOT: LLVMIRLoweringPass
diff --git a/flang/test/Driver/mlir-pass-pipeline.f90 b/flang/test/Driver/mlir-pass-pipeline.f90
index 3d8c42f123e2e..f0dcf68195c4b 100644
--- a/flang/test/Driver/mlir-pass-pipeline.f90
+++ b/flang/test/Driver/mlir-pass-pipeline.f90
@@ -73,5 +73,7 @@
 ! ALL-NEXT:   (S) 0 num-dce'd - Number of operations eliminated
 ! ALL-NEXT: TargetRewrite
 ! ALL-NEXT: ExternalNameConversion
+! ALL-NEXT:  'func.func' Pipeline
+! ALL-NEXT:    FunctionAttr
 ! ALL-NEXT: FIRToLLVMLowering
 ! ALL-NOT: LLVMIRLoweringPass
diff --git a/flang/test/Driver/save-mlir-temps.f90 b/flang/test/Driver/save-mlir-temps.f90
index 50bc83030caa9..1c8935fbd7aac 100644
--- a/flang/test/Driver/save-mlir-temps.f90
+++ b/flang/test/Driver/save-mlir-temps.f90
@@ -51,9 +51,9 @@
 ! Content to check from the MLIR outputs
 !--------------------------
 ! MLIR-FIR-NOT: llvm.func
-! MLIR-FIR: func.func @{{.*}}main() {
+! MLIR-FIR: func.func @{{.*}}main(){{.*}}{
 
 ! MLIR-FIR-NOT: func.func
-! MLIR-LLVMIR: llvm.func @{{.*}}main() {
+! MLIR-LLVMIR: llvm.func @{{.*}}main(){{.*}}{
 
 end program
diff --git a/flang/test/Fir/basic-program.fir b/flang/test/Fir/basic-program.fir
index d8a9e74c318ce..601e34ee04c27 100644
--- a/flang/test/Fir/basic-program.fir
+++ b/flang/test/Fir/basic-program.fir
@@ -80,5 +80,7 @@ func.func @_QQmain() {
 // PASSES-NEXT: CodeGenRewrite
 // PASSES-NEXT:   (S) 0 num-dce'd - Number of operations eliminated
 // PASSES-NEXT: TargetRewrite
+// PASSES-NEXT:  'func.func' Pipeline
+// PASSES-NEXT:    FunctionAttr
 // PASSES-NEXT: FIRToLLVMLowering
 // PASSES-NEXT: LLVMIRLoweringPass
diff --git a/flang/test/Fir/box-offset-codegen.fir b/flang/test/Fir/box-offset-codegen.fir
index 600555cd94cea..389ceebcc065c 100644
--- a/flang/test/Fir/box-offset-codegen.fir
+++ b/flang/test/Fir/box-offset-codegen.fir
@@ -7,7 +7,7 @@ func.func @scalar_addr(%scalar : !fir.ref<!fir.box<!fir.type<t>>>) -> !fir.llvm_
   return %addr : !fir.llvm_ptr<!fir.ref<!fir.type<t>>>
 }
 // CHECK-LABEL: define ptr @scalar_addr(
-// CHECK-SAME: ptr %[[BOX:.*]]) {
+// CHECK-SAME: ptr %[[BOX:.*]]){{.*}}{
 // CHECK:    %[[VAL_0:.*]] = getelementptr { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] }, ptr %[[BOX]], i32 0, i32 0
 // CHECK:    ret ptr %[[VAL_0]]
 
@@ -16,7 +16,7 @@ func.func @scalar_tdesc(%scalar : !fir.ref<!fir.box<!fir.type<t>>>) -> !fir.llvm
   return %tdesc : !fir.llvm_ptr<!fir.tdesc<!fir.type<t>>>
 }
 // CHECK-LABEL: define ptr @scalar_tdesc(
-// CHECK-SAME: ptr %[[BOX:.*]]) {
+// CHECK-SAME: ptr %[[BOX:.*]]){{.*}}{
 // CHECK:    %[[VAL_0:.*]] = getelementptr { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] }, ptr %[[BOX]], i32 0, i32 7
 // CHECK:    ret ptr %[[VAL_0]]
 
@@ -25,7 +25,7 @@ func.func @array_addr(%array : !fir.ref<!fir.class<!fir.ptr<!fir.array<?x!fir.ty
   return %addr : !fir.llvm_ptr<!fir.ptr<!fir.array<?x!fir.type<t>>>>
 }
 // CHECK-LABEL: define ptr @array_addr(
-// CHECK-SAME: ptr %[[BOX:.*]]) {
+// CHECK-SAME: ptr %[[BOX:.*]]){{.*}}{
 // CHECK:    %[[VAL_0:.*]] = getelementptr { ptr, i64, i32, i8, i8, i8, i8, [1 x [3 x i64]], ptr, [1 x i64] }, ptr %[[BOX]], i32 0, i32 0
 // CHECK:    ret ptr %[[VAL_0]]
 
@@ -34,6 +34,6 @@ func.func @array_tdesc(%array : !fir.ref<!fir.class<!fir.ptr<!fir.array<?x!fir.t
   return %tdesc : !fir.llvm_ptr<!fir.tdesc<!fir.type<t>>>
 }
 // CHECK-LABEL: define ptr @array_tdesc(
-// CHECK-SAME: ptr %[[BOX:.*]]) {
+// CHECK-SAME: ptr %[[BOX:.*]]){{.*}}{
 // CHECK:    %[[VAL_0:.*]] = getelementptr { ptr, i64, i32, i8, i8, i8, i8, [1 x [3 x i64]], ptr, [1 x i64] }, ptr %[[BOX]], i32 0, i32 8
 // CHECK:    ret ptr %[[VAL_0]]
diff --git a/flang/test/Fir/polymorphic.fir b/flang/test/Fir/polymorphic.fir
index ce8e43b0be656..ecdcdeb575310 100644
--- a/flang/test/Fir/polymorphic.fir
+++ b/flang/test/Fir/polymorphic.fir
@@ -10,7 +10,7 @@ func.func @_QMpolymorphic_testPtest_allocate_unlimited_polymorphic_non_derived()
   return
 }
 
-// CHECK-LABEL: define void @_QMpolymorphic_testPtest_allocate_unlimited_polymorphic_non_derived() {
+// CHECK-LABEL: define void @_QMpolymorphic_testPtest_allocate_unlimited_polymorphic_non_derived(){{.*}}{
 // CHECK:   %[[MEM:.*]] = alloca { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] }
 // CHECK:   %[[DESC:.*]] = alloca { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] }, i64 1
 // CHECK:   store { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } { ptr null, i64 0, i32 20180515, i8 0, i8 -1, i8 1, i8 1, ptr null, [1 x i64] undef }, ptr %[[MEM]]
@@ -87,7 +87,7 @@ func.func @_QMunlimitedPsub1(%arg0: !fir.class<!fir.array<?xnone>> {fir.bindc_na
 }
 
 // CHECK-LABEL: define void @_QMunlimitedPsub1(
-// CHECK-SAME: ptr %[[ARRAY:.*]]) {
+// CHECK-SAME: ptr %[[ARRAY:.*]]){{.*}}{
 // CHECK: %[[BOX:.*]] = alloca { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] }
 // CHECK: %{{.}} = getelementptr { ptr, i64, i32, i8, i8, i8, i8, [1 x [3 x i64]], ptr, [1 x i64] }, ptr %[[ARRAY]], i32 0, i32 7, i32 0, i32 2
 // CHECK: %[[TYPE_DESC_GEP:.*]] = getelementptr { ptr, i64, i32, i8, i8, i8, i8, [1 x [3 x i64]], ptr, [1 x i64] }, ptr %[[ARRAY]], i32 0, i32 8
@@ -151,7 +151,7 @@ func.func @_QQmain() {
   return
 }
 
-// CHECK-LABEL: define void @_QQmain() {
+// CHECK-LABEL: define void @_QQmain(){{.*}}{
 // CHECK: %[[CLASS_NONE:.*]] = alloca { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] }
 // CHECK: %[[DESC:.*]] = alloca { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] }, i64 1
 // CHECK: store { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } { ptr @_QMmod1Ea, i64 ptrtoint (ptr getelementptr (%_QMmod1TtK2, ptr null, i32 1) to i64), i32 20180515, i8 0, i8 42, i8 1, i8 1, ptr @_QMmod1E.dt.t.2, [1 x i64] undef }, ptr %[[CLASS_NONE]], align 8
@@ -175,7 +175,7 @@ func.func @_QMmod2Pinitp(%arg0: !fir.ref<!fir.class<!fir.ptr<none>>> {fir.bindc_
 func.func private @_FortranAPointerAssociate(!fir.ref<!fir.box<none>>, !fir.box<none>) -> none attributes {fir.runtime}
 
 // CHECK-LABEL: define void @_QMmod2Pinitp(
-// CHECK-SAME: ptr %[[ARG0:.*]]) {
+// CHECK-SAME: ptr %[[ARG0:.*]]){{.*}}{
 // CHECK: %[[ALLOCA_CLASS_NONE:.*]] = alloca { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] }
 // CHECK: %[[LOAD:.*]] = load { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] }, ptr %[[ARG0]]
 // CHECK: store { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } %[[LOAD]], ptr %[[ALLOCA_CLASS_NONE]]
diff --git a/flang/test/Fir/tbaa-codegen.fir b/flang/test/Fir/tbaa-codegen.fir
index fd0eb9c7304ee..87bb15c0fea6c 100644
--- a/flang/test/Fir/tbaa-codegen.fir
+++ b/flang/test/Fir/tbaa-codegen.fir
@@ -28,7 +28,7 @@ module attributes {fir.defaultkind = "a1c4d8i4l4r4", fir.kindmap = "", llvm.targ
 }
 
 // CHECK-LABEL: define void @_QPsimple(
-// CHECK-SAME:      ptr %[[ARG0:.*]]) {
+// CHECK-SAME:      ptr %[[ARG0:.*]]){{.*}}{
 // [...]
 // load  a(2):
 // CHECK:  %[[VAL20:.*]] = getelementptr i8, ptr %{{.*}}, i64 %{{.*}}
diff --git a/flang/test/Fir/tbaa-codegen2.fir b/flang/test/Fir/tbaa-codegen2.fir
index d73a7b96a5386..e649c06731c6b 100644
--- a/flang/test/Fir/tbaa-codegen2.fir
+++ b/flang/test/Fir/tbaa-codegen2.fir
@@ -60,7 +60,7 @@ module attributes {fir.defaultkind = "a1c4d8i4l4r4", fir.kindmap = "", llvm.targ
   }
 }
 // CHECK-LABEL: define void @_QPfunc(
-// CHECK-SAME:      ptr %[[ARG0:.*]]) {
+// CHECK-SAME:      ptr %[[ARG0:.*]]){{.*}}{
 // [...]
 // CHECK:  %[[VAL5:.*]] = getelementptr { ptr, i64, i32, i8, i8, i8, i8, [1 x [3 x i64]] }, ptr %[[ARG0]], i32 0, i32 7, i32 0, i32 0
 // box access:

github-actions · 2023-12-06T14:48:19Z

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

kiranchandramohan · 2023-12-06T15:11:54Z

You have some clang-format issues.

jeanPerier

A few comments inline. My more generic questions are:

Can the equivalent be achieved by passing the option to LLVM codegen instead of decorating all functions?
The pass as it is today is not very flang specific, should it belong to the Func or MLIR LLVM dialect. Unless there is a point for MLIR optimization to know about these aspect, why does it have to be set up in the chain?

flang/include/flang/Tools/CrossToolHelpers.h

flang/include/flang/Tools/CLOptions.inc

flang/include/flang/Optimizer/Transforms/Passes.td

flang/lib/Optimizer/Transforms/FunctionAttr.cpp

Radu2k · 2023-12-07T08:53:14Z

@Dinistro @gysit Do you think that adding this pass as an LLVM Func pass would be acceptable?

flang/lib/Frontend/CompilerInvocation.cpp

flang/lib/Optimizer/Transforms/FunctionAttr.cpp

flang/include/flang/Optimizer/Transforms/Passes.td

kiranchandramohan · 2023-12-07T10:22:20Z

Thanks @jeanPerier for the review. The pass was just following the vscale_range pass that was added a month back. I guess it can be argued that vscale_range might be needed for any optimisation for SVE that happens in MLIR, although at the moment there is none.

A few comments inline. My more generic questions are:

Can the equivalent be achieved by passing the option to LLVM codegen instead of decorating all functions?

Would that be adding a conversion pattern for func.func in the Flang Codegen.cpp?

The pass as it is today is not very flang specific, should it belong to the Func or MLIR LLVM dialect. Unless there is a point for MLIR optimization to know about these aspect, why does it have to be set up in the chain?

This should work. I believe we have a precedent here for the comdat pass.

llvm-project/flang/lib/Optimizer/CodeGen/CodeGen.cpp

Line 3902 in 19f4cec

comdatPM.addPass(mlir::LLVM::createLLVMAddComdats());

If per-function control is required (by way of function directives) then we can revisit later i guess.

jeanPerier · 2023-12-07T13:51:40Z

Can the equivalent be achieved by passing the option to LLVM codegen instead of decorating all functions?
Would that be adding a conversion pattern for func.func in the Flang Codegen.cpp?

No, I meant later: i.e. in the actual LLVM IR processing that flang calls in FrontendActions.cpp.

llc has a --frame-pointer option, so there is likely a way to pass this option to the llvm codegen as a global option instead of setting it on all function at the MLIR level if the only goal is to propagate it to llvm backend.

flang/include/flang/Tools/CLOptions.inc

flang/include/flang/Optimizer/Transforms/Passes.td

flang/lib/Frontend/CompilerInvocation.cpp

flang/lib/Optimizer/Transforms/FunctionAttr.cpp

jeanPerier · 2023-12-07T17:55:04Z

llc has a --frame-pointer option, so there is likely a way to pass this option to the llvm codegen as a global option instead of setting it on all function at the MLIR level if the only goal is to propagate it to llvm backend.

I just discussed a bit with @vzakhari and one advantage of setting it per function is to later be able to merge llvm IR files compiled with different options. That would explain why clang does it. I would still add nothing if the option is None since this is the default, but seems OK to me to decorate the functions as you are doing based on this argument.

…ling

Radu2k · 2023-12-08T17:04:54Z

@jeanPerier Thank you for all the suggestions. Could you please check my last commits and just confirm that you agree with this implementation? Any suggestions are welcomed.

tblah

Thanks for the updates

flang/include/flang/Tools/CLOptions.inc

flang/lib/Frontend/CompilerInvocation.cpp

flang/lib/Optimizer/Transforms/FunctionAttr.cpp

flang/test/Driver/fixed-free-detection.f90

flang/test/Driver/func-attr.f90

tblah

Thanks for the update. LGTM

kiranchandramohan · 2023-12-18T10:21:21Z

llc has a --frame-pointer option, so there is likely a way to pass this option to the llvm codegen as a global option instead of setting it on all function at the MLIR level if the only goal is to propagate it to llvm backend.

I just discussed a bit with @vzakhari and one advantage of setting it per function is to later be able to merge llvm IR files compiled with different options. That would explain why clang does it. I would still add nothing if the option is None since this is the default, but seems OK to me to decorate the functions as you are doing based on this argument.

@jeanPerier just checking whether you are OK with this pass being here or would prefer it to be in LLVM dialect?

flang/include/flang/Frontend/CodeGenOptions.def

flang/include/flang/Optimizer/Transforms/Passes.td

flang/lib/Frontend/CompilerInvocation.cpp

flang/lib/Optimizer/Transforms/FunctionAttr.cpp

jeanPerier

Thanks for the update, please take care of Kiran's comments. LGTM otherwise.

flang/lib/Frontend/CompilerInvocation.cpp

banach-space · 2023-12-20T08:58:26Z

flang/test/Driver/save-mlir-temps.f90

@@ -51,9 +51,9 @@
 ! Content to check from the MLIR outputs
 !--------------------------
 ! MLIR-FIR-NOT: llvm.func
-! MLIR-FIR: func.func @{{.*}}main() {
+! MLIR-FIR: func.func @{{.*}}main(){{.*}}{


What do these extra braces capture?

This was added due to failing the test when it was a default parameter. I do not see how having {{.*}} instead of a whitespace should be an issue if no parameters are checked.

Could you explain why it was failing? I just don't see how this particular change is related to what this PR is implementing. It would help if you included code "before" and "after" this PR.

Since this is target dependent, frame pointer will be added or not. It is either adding {{.*}} or adding -fomit-frame-pointer.

flang/lib/Frontend/CompilerInvocation.cpp

kiranchandramohan

LG. Please wait for @banach-space

flang/include/flang/Optimizer/Transforms/Passes.td

banach-space · 2023-12-22T17:10:42Z

flang/test/Driver/frame-pointer-forwarding.f90

-! RUN: %flang -fno-omit-frame-pointer --target=x86-none-none -fsyntax-only -### %s -o %t 2>&1  | FileCheck %s
-! CHECK: "-mframe-pointer=all"
+! RUN: %flang --target=aarch64-none-none -fsyntax-only -### %s -o %t 2>&1  | FileCheck %s --check-prefix=CHECK-NOVALUE
+! CHECK-NOVALUE: "-mframe-pointer=non-leaf"


Suggested change

! CHECK-NOVALUE: "-mframe-pointer=non-leaf"

! CHECK-NOVALUE: "-fc1" "-mframe-pointer=non-leaf"

Same remark for other check lines.

If we add this then it will be added as "-fc1"{{.*}}". Why is it necessary?

See the comment at the top of the file:

! Test that flang-new forwards -fno-omit-frame-pointer and -fomit-frame-pointer Flang frontend

In order to identify that this is indeed checking the frontend driver invocation (as per the comment, that's the intention), you need to check for -fc1. That's something that was missing here to begin with, so strictly speaking it's tangential to your change. But since you are updating these CHECK lines anyway ...

banach-space · 2023-12-22T17:12:36Z

flang/test/Driver/mlir-debug-pass-pipeline.f90

This change doesn't make sense to me. This file tests default pass pipelines for various debug levels. Why is -fomit-frame-pointer needed here?

This is needed because depending on the target, if the frame pointer is not specified, then the best option will be automatically added, therefore a

'func.func' Pipeline FunctionAttr

will appear or not, depending of the target, at the end of the file.

Therefore it is mandatory to add omitting frame pointer, otherwise the test will crash.

Thanks for the explanation!

Therefore it is mandatory to add omitting frame pointer

No. You can also update the expected output. So there are 2 options. And what would be preferred - this test is checking pass pipelines and including -fomit-frame-pointer suggests that that flag will impact those pipelines. That's not the case though.

You can do this:

! ALL-LABEL: Fortran::lower::VerifierPass ! ALL-NEXT: 'func.func' Pipeline ! ALL: InlineElementals ! ALL-NEXT: LowerHLFIROrderedAssignments

That looks good to me, thought it needs to check line by line([flang/test/Driver/mlir-debug-pass-pipeline.f90](url) - lines 35-38) but will update, replacing ALL-NEXT to ALL at line 85.

banach-space · 2023-12-22T17:14:42Z

flang/test/Driver/save-mlir-temps.f90

@@ -51,9 +51,9 @@
 ! Content to check from the MLIR outputs
 !--------------------------
 ! MLIR-FIR-NOT: llvm.func
-! MLIR-FIR: func.func @{{.*}}main() {
+! MLIR-FIR: func.func @{{.*}}main(){{.*}}{


Could you explain why it was failing? I just don't see how this particular change is related to what this PR is implementing. It would help if you included code "before" and "after" this PR.

banach-space · 2023-12-22T17:17:32Z

flang/test/Driver/func-attr.f90

@@ -0,0 +1,23 @@
+! Test that -mframe-pointer can accept only specific values.


You seem to be testing a bit more in this file than this comment suggests (e.g. that -mframe-pointer will make the compiler insert the new attributes)

Please, could you:

update the comment to capture what's actually being tested (the -mframe-pointer flag)

rename the file - it's testing a particular flag rather than any "func"tion "attr"ibute

Failing bot: * https://lab.llvm.org/buildbot/#/builders/21/builds/88731 Failing test was introduced in: * #74598 Sending without a review as the fix is straightforward and I want to prioritize fixing the broken bot and unblocking everyone who's been affected.

Pass to add frame pointer attribute

fda39a4

llvmbot added flang:driver flang Flang issues not falling into any other category flang:fir-hlfir labels Dec 6, 2023

kiranchandramohan requested review from tblah, Leporacanthicus and jeanPerier December 6, 2023 15:11

kiranchandramohan requested review from gysit and Dinistro December 6, 2023 15:12

Fix formatting issues

85b8de6

jeanPerier reviewed Dec 6, 2023

View reviewed changes

nikic changed the title ~~Pass to add frame pointer attribute~~ [flang] Pass to add frame pointer attribute Dec 7, 2023

Radu2k added 2 commits December 7, 2023 10:05

Minor changes to address review comments

6e44350

Resolve format issues

34ea42a

kiranchandramohan reviewed Dec 7, 2023

View reviewed changes

tblah reviewed Dec 7, 2023

View reviewed changes

Radu2k added 6 commits December 8, 2023 13:51

Implement code review suggestions for framePointerKind attribute hand…

574e247

…ling

Fix format issues

fac406c

Fix more whitespace format issues

a675d75

Fix more whitespace format issues in CLOptions.inc

104c3d4

Change CompilerInvocation.cpp no-leaf typo

11e3a3a

Fix 'test the debug pass pipeline' issue for frame pointer

0a3fbca

tblah reviewed Dec 11, 2023

View reviewed changes

Adjust code based on code review

a7887f1

tblah approved these changes Dec 12, 2023

View reviewed changes

Merge branch 'llvm:main' into feature/pass-for-frame-pointer-attr

6eb06ac

kiranchandramohan reviewed Dec 19, 2023

View reviewed changes

jeanPerier reviewed Dec 19, 2023

View reviewed changes

Make further appropriate syntax and test adjustments

cef574b

banach-space reviewed Dec 20, 2023

View reviewed changes

Radu2k added 2 commits December 21, 2023 10:48

Replace if/else with StringSwitch and add appropriate test case

bcd3079

Fix format issues

461db3f

kiranchandramohan approved these changes Dec 21, 2023

View reviewed changes

flang/include/flang/Optimizer/Transforms/Passes.td Outdated Show resolved Hide resolved

flang/include/flang/Optimizer/Transforms/Passes.td Outdated Show resolved Hide resolved

Fix Passes.td 80 characters formatting limit

af00662

banach-space reviewed Dec 22, 2023

View reviewed changes

Fix review comments

9e3758c

Leporacanthicus merged commit 0487377 into llvm:main Dec 28, 2023
4 checks passed

kiranchandramohan mentioned this pull request Jan 16, 2024

[Flang][Lower] Attach target_cpu and target_features attributes to MLIR functions #78289

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[flang] Pass to add frame pointer attribute #74598

[flang] Pass to add frame pointer attribute #74598

Radu2k commented Dec 6, 2023 •

edited

llvmbot commented Dec 6, 2023

llvmbot commented Dec 6, 2023

github-actions bot commented Dec 6, 2023 •

edited

kiranchandramohan commented Dec 6, 2023

jeanPerier left a comment

Radu2k commented Dec 7, 2023

kiranchandramohan commented Dec 7, 2023 •

edited

jeanPerier commented Dec 7, 2023

jeanPerier commented Dec 7, 2023

Radu2k commented Dec 8, 2023

tblah left a comment

tblah left a comment

kiranchandramohan commented Dec 18, 2023

jeanPerier left a comment

banach-space Dec 20, 2023

Radu2k Dec 20, 2023

banach-space Dec 22, 2023

Radu2k Dec 22, 2023

kiranchandramohan left a comment

banach-space Dec 22, 2023

Radu2k Dec 22, 2023

banach-space Dec 22, 2023

banach-space Dec 22, 2023

Radu2k Dec 22, 2023 •

edited

banach-space Dec 22, 2023

Radu2k Dec 22, 2023 •

edited

banach-space Dec 22, 2023

banach-space Dec 22, 2023

	! CHECK-NOVALUE: "-mframe-pointer=non-leaf"
	! CHECK-NOVALUE: "-fc1" "-mframe-pointer=non-leaf"

		@@ -0,0 +1,23 @@
		! Test that -mframe-pointer can accept only specific values.

[flang] Pass to add frame pointer attribute #74598

[flang] Pass to add frame pointer attribute #74598

Conversation

Radu2k commented Dec 6, 2023 • edited

llvmbot commented Dec 6, 2023

llvmbot commented Dec 6, 2023

github-actions bot commented Dec 6, 2023 • edited

kiranchandramohan commented Dec 6, 2023

jeanPerier left a comment

Choose a reason for hiding this comment

Radu2k commented Dec 7, 2023

kiranchandramohan commented Dec 7, 2023 • edited

jeanPerier commented Dec 7, 2023

jeanPerier commented Dec 7, 2023

Radu2k commented Dec 8, 2023

tblah left a comment

Choose a reason for hiding this comment

tblah left a comment

Choose a reason for hiding this comment

kiranchandramohan commented Dec 18, 2023

jeanPerier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kiranchandramohan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Radu2k Dec 22, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Radu2k Dec 22, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Radu2k commented Dec 6, 2023 •

edited

github-actions bot commented Dec 6, 2023 •

edited

kiranchandramohan commented Dec 7, 2023 •

edited

Radu2k Dec 22, 2023 •

edited

Radu2k Dec 22, 2023 •

edited