Skip to content

[flang][acc] Ensure fir.class is handled in type categorization #146174

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 30, 2025
+86 −6
Merged

[flang][acc] Ensure fir.class is handled in type categorization #146174

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fccfee4
[flang][acc] Ensure fir.class is handled in type categorization
Jun 27, 2025
a8b1627
Move runtest line to top of file
Jun 30, 2025
3d64e5d
Add better support for unlimited polymorphic and assumed type
Jun 30, 2025
556ec64
Fix formatting
Jun 30, 2025
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
22 changes: 17 additions & 5 deletions flang/lib/Optimizer/OpenACC/FIROpenACCTypeInterfaces.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,13 @@ template <>
mlir::acc::VariableTypeCategory
OpenACCMappableModel<fir::BaseBoxType>::getTypeCategory(mlir::Type type,
mlir::Value var) const {
// Class-type does not behave like a normal box because it does not hold an
// element type. Thus special handle it here.
if (mlir::isa<fir::ClassType>(type))
return mlir::acc::VariableTypeCategory::composite;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we make a distinction for unlimited polymorphic that can be smth else than a composite?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I am currently categorizing it as "uncategorized". Although the OpenACC spec does not discuss unlimited polymorphic or assumed type - the language semantics should apply. Namely, the type should be appropriately selected for the categorization to work.


mlir::Type eleTy = fir::dyn_cast_ptrOrBoxEleTy(type);
assert(eleTy && "expect to be able to unwrap the element type");

// If the type enclosed by the box is a mappable type, then have it
// provide the type category.
Expand All @@ -346,7 +351,7 @@ OpenACCMappableModel<fir::BaseBoxType>::getTypeCategory(mlir::Type type,
return mlir::acc::VariableTypeCategory::nonscalar;
}

static mlir::TypedValue<mlir::acc::PointerLikeType>
static mlir::Value
getBaseRef(mlir::TypedValue<mlir::acc::PointerLikeType> varPtr) {
// If there is no defining op - the unwrapped reference is the base one.
mlir::Operation *op = varPtr.getDefiningOp();
Expand All @@ -372,7 +377,7 @@ getBaseRef(mlir::TypedValue<mlir::acc::PointerLikeType> varPtr) {
})
.Default([&](mlir::Operation *) { return varPtr; });

return mlir::cast<mlir::TypedValue<mlir::acc::PointerLikeType>>(baseRef);
return baseRef;
}

static mlir::acc::VariableTypeCategory
Expand All @@ -384,10 +389,17 @@ categorizePointee(mlir::Type pointer,
// value would both be represented as !fir.ref<f32>. We do not want to treat
// such a reference as a scalar. Thus unwrap interior pointer calculations.
auto baseRef = getBaseRef(varPtr);
mlir::Type eleTy = baseRef.getType().getElementType();

if (auto mappableTy = mlir::dyn_cast<mlir::acc::MappableType>(eleTy))
return mappableTy.getTypeCategory(varPtr);
if (auto mappableTy =
mlir::dyn_cast<mlir::acc::MappableType>(baseRef.getType()))
return mappableTy.getTypeCategory(baseRef);

// It must be a pointer-like type since it is not a MappableType.
auto ptrLikeTy = mlir::cast<mlir::acc::PointerLikeType>(baseRef.getType());
mlir::Type eleTy = ptrLikeTy.getElementType();

if (auto mappableEleTy = mlir::dyn_cast<mlir::acc::MappableType>(eleTy))
return mappableEleTy.getTypeCategory(varPtr);

if (isScalarLike(eleTy))
return mlir::acc::VariableTypeCategory::scalar;
Expand Down
18 changes: 18 additions & 0 deletions flang/test/Fir/OpenACC/openacc-type-categories-class.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
module mm
Copy link
Contributor

Choose a reason for hiding this comment

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

This test should probably go in flang/test/Lower/OpenACC or starts from mlir directly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I just saw that you added another test like that in this folder as well. No strong opinion on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer the test this way - changing FIR directly, especially for derived types - is not quite the nicest experience due to the verbosity. I hope you can be OK with this!

type, public :: polyty
real :: field
end type
contains
subroutine init(this)
class(polyty), intent(inout) :: this
!$acc enter data copyin(this, this%field)
Comment on lines +9 to +10
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you check if it works as well with an unlimited polymorphic dummy?

class(*), intent(inout) :: this
!$acc enter data copyin(this)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It did not - and now I fixed it as I described below.

end subroutine
end module

! RUN: bbc -fopenacc -emit-hlfir %s -o - | fir-opt -pass-pipeline='builtin.module(test-fir-openacc-interfaces)' --mlir-disable-threading 2>&1 | FileCheck %s
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason to not add the run line at the top?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. It was on accident.

! CHECK: Visiting: {{.*}} acc.copyin {{.*}} {name = "this", structured = false}
! CHECK: Mappable: !fir.class<!fir.type<_QMmmTpolyty{field:f32}>>
! CHECK: Type category: composite
! CHECK: Visiting: {{.*}} acc.copyin {{.*}} {name = "this%field", structured = false}
! CHECK: Pointer-like: !fir.ref<f32>
! CHECK: Type category: composite
9 changes: 9 additions & 0 deletions flang/test/lib/OpenACC/TestOpenACCInterfaces.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,16 @@
//
//===----------------------------------------------------------------------===//

#include "mlir/Dialect/Arith/IR/Arith.h"
#include "mlir/Dialect/OpenACC/OpenACC.h"
#include "mlir/IR/Builders.h"
#include "mlir/IR/BuiltinOps.h"
#include "mlir/Pass/Pass.h"
#include "mlir/Support/LLVM.h"
#include "flang/Optimizer/Dialect/FIRDialect.h"
#include "flang/Optimizer/HLFIR/HLFIRDialect.h"
#include "flang/Optimizer/Support/DataLayout.h"
#include "mlir/Dialect/DLTI/DLTI.h"

using namespace mlir;

Expand All @@ -25,6 +29,11 @@ struct TestFIROpenACCInterfaces
StringRef getDescription() const final {
return "Test FIR implementation of the OpenACC interfaces.";
}
void getDependentDialects(::mlir::DialectRegistry &registry) const override {
registry.insert<fir::FIROpsDialect, hlfir::hlfirDialect,
mlir::arith::ArithDialect, mlir::acc::OpenACCDialect,
mlir::DLTIDialect>();
}
void runOnOperation() override {
mlir::ModuleOp mod = getOperation();
auto datalayout =
Expand Down
Loading