Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions flang/lib/Optimizer/Transforms/FunctionAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
/// \file
/// This is a generic pass for adding attributes to functions.
//===----------------------------------------------------------------------===//
#include "flang/Optimizer/Dialect/FIROpsSupport.h"
#include "flang/Optimizer/Support/InternalNames.h"
#include "flang/Optimizer/Transforms/Passes.h"
#include "mlir/Dialect/LLVMIR/LLVMAttrs.h"
#include "mlir/Dialect/LLVMIR/LLVMDialect.h"
Expand Down Expand Up @@ -45,6 +47,25 @@ void FunctionAttrPass::runOnOperation() {

LLVM_DEBUG(llvm::dbgs() << "Func-name:" << func.getSymName() << "\n");

llvm::StringRef name = func.getSymName();
auto deconstructed = fir::NameUniquer::deconstruct(name);
bool isFromModule = !deconstructed.second.modules.empty();

if ((isFromModule || !func.isDeclaration()) &&
!fir::hasBindcAttr(func.getOperation())) {
llvm::StringRef nocapture = mlir::LLVM::LLVMDialect::getNoCaptureAttrName();
mlir::UnitAttr unitAttr = mlir::UnitAttr::get(func.getContext());

for (auto [index, argType] : llvm::enumerate(func.getArgumentTypes())) {
if (mlir::isa<fir::ReferenceType>(argType) &&
!fir::isPointerType(argType) &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You do not need the fir::isPointerType check here: it prevents putting nocapture on fir.ref<fir.box<fir.ptr<>> (addresses of descriptor created for Fortran POINTERs). It would be incorrect to add nocapture on the base address of the POINTER (which is the first struct member of the descriptor), but the address of the descriptor itself cannot be taken in Fortran.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the review.
I thought that since the dummy argument defined as integer, pointer :: x is translated to fir.ref<fir.box<fir.ptr<i32>>> (https://godbolt.org/z/qbYn59d6M), it could be determined using fir::isPointerType.
As another way to determine whether a dummy argument has the pointer attribute, should I add the fir.pointer attribute, similar to target and asynchronous, and check it with !func.getArgAttr(index, fir::getPointerAttrName())?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I am pointing out is not the ability to detect or not the the argument is for a Fortran pointer, it is that Fotran Pointers are always passed as the address of a descriptor, and while the address inside the descriptor may be captured because it is the target of a Fortran POINTER, the address of the descriptor cannot be taken (there is no way to do that in Fortran). The llvm nocapture applies to the address that is the function argument, which in this case is the one of the descriptor, not the base address, so it is still correct to add nocapture to a fir.ref<fir.box<fir.ptr<>>.

Let me use C to illustrate here: when passing a Fortran pointer, the ABI looks like:

typedef struct {
  void* base_address;
  // ... more fields
} Descriptor;

void foo(Descriptor* fortran_pointer);

Fortran tells you that fortran_pointer->base_address may be taken inside foo, but fortran_pointer address cannot. As far as I understand llvm.nocapture, it applies to the SSA argument address, not the addresses inside the memory pointed to by the SSA argument address. Descriptor* is the C equivalent of fir.ref<fir.box<>>.

Copy link
Contributor Author

@s-watanabe314 s-watanabe314 Nov 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the detailed explanation. I understand that applying nocapture is correct because it's a descriptor. I've removed fir::isPointerType and corrected the test.

!func.getArgAttr(index, fir::getTargetAttrName()) &&
!func.getArgAttr(index, fir::getAsynchronousAttrName()) &&
!func.getArgAttr(index, fir::getVolatileAttrName()))
func.setArgAttr(index, nocapture, unitAttr);
}
}

mlir::MLIRContext *context = &getContext();
if (framePointerKind != mlir::LLVM::framePointerKind::FramePointerKind::None)
func->setAttr("frame_pointer", mlir::LLVM::FramePointerKindAttr::get(
Expand Down
45 changes: 45 additions & 0 deletions flang/test/Transforms/function-attrs.fir
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// RUN: fir-opt --function-attr %s | FileCheck %s

// If a function has a body and is not bind(c), and if the dummy argument doesn't have the target,
// asynchronous, volatile, or pointer attribute, then add llvm.nocapture to the dummy argument.

func.func @_QParg_nocapture(%arg0: !fir.ref<i32> {fir.bindc_name = "tar", fir.target}, %arg1: !fir.ref<i32> {fir.asynchronous, fir.bindc_name = "asynch"}, %arg2: !fir.ref<i32> {fir.bindc_name = "vol", fir.volatile}, %arg3: !fir.ref<!fir.box<!fir.ptr<i32>>> {fir.bindc_name = "ptr"}, %arg4: !fir.ref<i32> {fir.bindc_name = "nocap"}) {
%0 = fir.dummy_scope : !fir.dscope
%1 = fir.declare %arg0 dummy_scope %0 {fortran_attrs = #fir.var_attrs<target>, uniq_name = "_QFarg_nocaptureEtar"} : (!fir.ref<i32>, !fir.dscope) -> !fir.ref<i32>
%2 = fir.declare %arg1 dummy_scope %0 {fortran_attrs = #fir.var_attrs<asynchronous>, uniq_name = "_QFarg_nocaptureEasynch"} : (!fir.ref<i32>, !fir.dscope) -> !fir.ref<i32>
%3 = fir.declare %arg2 dummy_scope %0 {uniq_name = "_QFarg_nocaptureEvol"} : (!fir.ref<i32>, !fir.dscope) -> !fir.ref<i32>
%4 = fir.declare %arg3 dummy_scope %0 {fortran_attrs = #fir.var_attrs<pointer>, uniq_name = "_QFarg_nocaptureEptr"} : (!fir.ref<!fir.box<!fir.ptr<i32>>>, !fir.dscope) -> !fir.ref<!fir.box<!fir.ptr<i32>>>
%5 = fir.declare %arg4 dummy_scope %0 {uniq_name = "_QFarg_nocaptureEnocap"} : (!fir.ref<i32>, !fir.dscope) -> !fir.ref<i32>
return
}
// CHECK-LABEL: func.func @_QParg_nocapture(
// CHECK-SAME: %[[ARG0:.*]]: !fir.ref<i32> {fir.bindc_name = "tar", fir.target},
// CHECK-SAME: %[[ARG1:.*]]: !fir.ref<i32> {fir.asynchronous, fir.bindc_name = "asynch"},
// CHECK-SAME: %[[ARG2:.*]]: !fir.ref<i32> {fir.bindc_name = "vol", fir.volatile},
// CHECK-SAME: %[[ARG3:.*]]: !fir.ref<!fir.box<!fir.ptr<i32>>> {fir.bindc_name = "ptr"},
// CHECK-SAME: %[[ARG4:.*]]: !fir.ref<i32> {fir.bindc_name = "nocap", llvm.nocapture}) {
// CHECK: return
// CHECK-NEXT: }

func.func @arg_nocapture_bindc(%arg0: !fir.ref<i32> {fir.bindc_name = "tar", fir.target}, %arg1: !fir.ref<i32> {fir.bindc_name = "nocap"}) attributes {fir.bindc_name = "arg_nocapture_bindc", fir.proc_attrs = #fir.proc_attrs<bind_c>} {
%0 = fir.dummy_scope : !fir.dscope
%1 = fir.declare %arg0 dummy_scope %0 {fortran_attrs = #fir.var_attrs<target>, uniq_name = "_QFarg_nocapture_bindcEtar"} : (!fir.ref<i32>, !fir.dscope) -> !fir.ref<i32>
%2 = fir.declare %arg1 dummy_scope %0 {uniq_name = "_QFarg_nocapture_bindcEnocap"} : (!fir.ref<i32>, !fir.dscope) -> !fir.ref<i32>
return
}
// CHECK-LABEL: func.func @arg_nocapture_bindc(
// CHECK-NOT: llvm.nocapture


// If a function declaration is from a module and is not bind(c), and if the dummy argument doesn't have
// the target, asynchronous, volatile, or pointer attribute, then add llvm.nocapture to the dummy argument.

func.func private @_QMarg_modPcheck_args(!fir.ref<i32> {fir.target}, !fir.ref<i32> {fir.asynchronous}, !fir.ref<i32> {fir.volatile}, !fir.ref<!fir.box<!fir.ptr<i32>>>, !fir.ref<i32>, !fir.boxchar<1>, !fir.ref<complex<f32>>)
// CHECK-LABEL: func.func private @_QMarg_modPcheck_args(
// CHECK-SAME: !fir.ref<i32> {fir.target},
// CHECK-SAME: !fir.ref<i32> {fir.asynchronous},
// CHECK-SAME: !fir.ref<i32> {fir.volatile},
// CHECK-SAME: !fir.ref<!fir.box<!fir.ptr<i32>>>,
// CHECK-SAME: !fir.ref<i32> {llvm.nocapture},
// CHECK-SAME: !fir.boxchar<1>,
// CHECK-SAME: !fir.ref<complex<f32>> {llvm.nocapture})
Loading