[flang][Lower] Convert OMP Map and related functions to evaluate::Expr #81626
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kparzysz
requested review from
clementval,
skatrak,
tblah,
kiranchandramohan and
agozillon
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-openmp @llvm/pr-subscribers-openacc Author: Krzysztof Parzyszek (kparzysz) ChangesThe related functions are The difference in the test case comes from unfolded conversions of index expressions, which are explicitly of type integer(kind=8). Delete now unused Add Patch is 38.06 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/81626.diff 4 Files Affected:
diff --git a/flang/include/flang/Evaluate/tools.h b/flang/include/flang/Evaluate/tools.h
index e9999974944e88..d5713cfe420a2e 100644
--- a/flang/include/flang/Evaluate/tools.h
+++ b/flang/include/flang/Evaluate/tools.h
@@ -148,6 +148,14 @@ inline Expr<SomeType> AsGenericExpr(Expr<SomeType> &&x) { return std::move(x); }
std::optional<Expr<SomeType>> AsGenericExpr(DataRef &&);
std::optional<Expr<SomeType>> AsGenericExpr(const Symbol &);
+// Propagate std::optional from input to output.
+template <typename A>
+std::optional<Expr<SomeType>> AsGenericExpr(std::optional<A> &&x) {
+ if (!x)
+ return std::nullopt;
+ return AsGenericExpr(std::move(*x));
+}
+
template <typename A>
common::IfNoLvalue<Expr<SomeKind<ResultType<A>::category>>, A> AsCategoryExpr(
A &&x) {
diff --git a/flang/lib/Lower/DirectivesCommon.h b/flang/lib/Lower/DirectivesCommon.h
index 8d560db34e05bf..2fa90572bc63eb 100644
--- a/flang/lib/Lower/DirectivesCommon.h
+++ b/flang/lib/Lower/DirectivesCommon.h
@@ -808,6 +808,75 @@ genBaseBoundsOps(fir::FirOpBuilder &builder, mlir::Location loc,
return bounds;
}
+namespace detail {
+template <typename T> //
+static T &&AsRvalueRef(T &&t) {
+ return std::move(t);
+}
+template <typename T> //
+static T AsRvalueRef(T &t) {
+ return t;
+}
+template <typename T> //
+static T AsRvalueRef(const T &t) {
+ return t;
+}
+
+// Helper class for stripping enclosing parentheses and a conversion that
+// preserves type category. This is used for triplet elements, which are
+// always of type integer(kind=8). The lower/upper bounds are converted to
+// an "index" type, which is 64-bit, so the explicit conversion to kind=8
+// (if present) is not needed. When it's present, though, it causes generated
+// names to contain "int(..., kind=8)".
+struct PeelConvert {
+ template <Fortran::common::TypeCategory Category, int Kind>
+ static Fortran::semantics::MaybeExpr visit_with_category(
+ const Fortran::evaluate::Expr<Fortran::evaluate::Type<Category, Kind>>
+ &expr) {
+ return std::visit(
+ [](auto &&s) { return visit_with_category<Category, Kind>(s); },
+ expr.u);
+ }
+ template <Fortran::common::TypeCategory Category, int Kind>
+ static Fortran::semantics::MaybeExpr visit_with_category(
+ const Fortran::evaluate::Convert<Fortran::evaluate::Type<Category, Kind>,
+ Category> &expr) {
+ return AsGenericExpr(AsRvalueRef(expr.left()));
+ }
+ template <Fortran::common::TypeCategory Category, int Kind, typename T>
+ static Fortran::semantics::MaybeExpr visit_with_category(const T &) {
+ return std::nullopt; //
+ }
+ template <Fortran::common::TypeCategory Category, typename T>
+ static Fortran::semantics::MaybeExpr visit_with_category(const T &) {
+ return std::nullopt; //
+ }
+
+ template <Fortran::common::TypeCategory Category>
+ static Fortran::semantics::MaybeExpr
+ visit(const Fortran::evaluate::Expr<Fortran::evaluate::SomeKind<Category>>
+ &expr) {
+ return std::visit([](auto &&s) { return visit_with_category<Category>(s); },
+ expr.u);
+ }
+ static Fortran::semantics::MaybeExpr
+ visit(const Fortran::evaluate::Expr<Fortran::evaluate::SomeType> &expr) {
+ return std::visit([](auto &&s) { return visit(s); }, expr.u);
+ }
+ template <typename T> //
+ static Fortran::semantics::MaybeExpr visit(const T &) {
+ return std::nullopt;
+ }
+};
+
+static Fortran::semantics::SomeExpr
+peelOuterConvert(Fortran::semantics::SomeExpr &expr) {
+ if (auto peeled = PeelConvert::visit(expr))
+ return *peeled;
+ return expr;
+}
+} // namespace detail
+
/// Generate bounds operations for an array section when subscripts are
/// provided.
template <typename BoundsOp, typename BoundsType>
@@ -815,7 +884,7 @@ llvm::SmallVector<mlir::Value>
genBoundsOps(fir::FirOpBuilder &builder, mlir::Location loc,
Fortran::lower::AbstractConverter &converter,
Fortran::lower::StatementContext &stmtCtx,
- const std::list<Fortran::parser::SectionSubscript> &subscripts,
+ const std::vector<Fortran::evaluate::Subscript> &subscripts,
std::stringstream &asFortran, fir::ExtendedValue &dataExv,
bool dataExvIsAssumedSize, AddrAndBoundsInfo &info,
bool treatIndexAsSection = false) {
@@ -828,8 +897,7 @@ genBoundsOps(fir::FirOpBuilder &builder, mlir::Location loc,
mlir::Value one = builder.createIntegerConstant(loc, idxTy, 1);
const int dataExvRank = static_cast<int>(dataExv.rank());
for (const auto &subscript : subscripts) {
- const auto *triplet{
- std::get_if<Fortran::parser::SubscriptTriplet>(&subscript.u)};
+ const auto *triplet{std::get_if<Fortran::evaluate::Triplet>(&subscript.u)};
if (triplet || treatIndexAsSection) {
if (dimension != 0)
asFortran << ',';
@@ -868,13 +936,18 @@ genBoundsOps(fir::FirOpBuilder &builder, mlir::Location loc,
strideInBytes = true;
}
- const Fortran::lower::SomeExpr *lower{nullptr};
+ Fortran::semantics::MaybeExpr lower;
if (triplet) {
- if (const auto &tripletLb{std::get<0>(triplet->t)})
- lower = Fortran::semantics::GetExpr(*tripletLb);
+ if ((lower = Fortran::evaluate::AsGenericExpr(triplet->lower())))
+ lower = detail::peelOuterConvert(*lower);
} else {
- const auto &index{std::get<Fortran::parser::IntExpr>(subscript.u)};
- lower = Fortran::semantics::GetExpr(index);
+ // Case of IndirectSubscriptIntegerExpr
+ using IndirectSubscriptIntegerExpr =
+ Fortran::evaluate::IndirectSubscriptIntegerExpr;
+ using SubscriptInteger = Fortran::evaluate::SubscriptInteger;
+ Fortran::evaluate::Expr<SubscriptInteger> oneInt =
+ std::get<IndirectSubscriptIntegerExpr>(subscript.u).value();
+ lower = Fortran::evaluate::AsGenericExpr(std::move(oneInt));
if (lower->Rank() > 0) {
mlir::emitError(
loc, "vector subscript cannot be used for an array section");
@@ -912,10 +985,12 @@ genBoundsOps(fir::FirOpBuilder &builder, mlir::Location loc,
extent = one;
} else {
asFortran << ':';
- const auto &upper{std::get<1>(triplet->t)};
+ Fortran::semantics::MaybeExpr upper =
+ Fortran::evaluate::AsGenericExpr(triplet->upper());
if (upper) {
- uval = Fortran::semantics::GetIntValue(upper);
+ upper = detail::peelOuterConvert(*upper);
+ uval = Fortran::evaluate::ToInt64(*upper);
if (uval) {
if (defaultLb) {
ubound = builder.createIntegerConstant(loc, idxTy, *uval - 1);
@@ -925,22 +1000,21 @@ genBoundsOps(fir::FirOpBuilder &builder, mlir::Location loc,
}
asFortran << *uval;
} else {
- const Fortran::lower::SomeExpr *uexpr =
- Fortran::semantics::GetExpr(*upper);
mlir::Value ub =
- fir::getBase(converter.genExprValue(loc, *uexpr, stmtCtx));
+ fir::getBase(converter.genExprValue(loc, *upper, stmtCtx));
ub = builder.createConvert(loc, baseLb.getType(), ub);
ubound = builder.create<mlir::arith::SubIOp>(loc, ub, baseLb);
- asFortran << uexpr->AsFortran();
+ asFortran << upper->AsFortran();
}
}
if (lower && upper) {
if (lval && uval && *uval < *lval) {
mlir::emitError(loc, "zero sized array section");
break;
- } else if (std::get<2>(triplet->t)) {
- const auto &strideExpr{std::get<2>(triplet->t)};
- if (strideExpr) {
+ } else {
+ // Stride is mandatory in evaluate::Triplet. Make sure it's 1.
+ auto val = Fortran::evaluate::ToInt64(triplet->GetStride());
+ if (!val || *val != 1) {
mlir::emitError(loc, "stride cannot be specified on "
"an array section");
break;
@@ -993,150 +1067,157 @@ genBoundsOps(fir::FirOpBuilder &builder, mlir::Location loc,
return bounds;
}
-template <typename ObjectType, typename BoundsOp, typename BoundsType>
+namespace detail {
+template <typename Ref, typename Expr> //
+std::optional<Ref> getRef(Expr &&expr) {
+ if constexpr (std::is_same_v<llvm::remove_cvref_t<Expr>,
+ Fortran::evaluate::DataRef>) {
+ if (auto *ref = std::get_if<Ref>(&expr.u))
+ return *ref;
+ return std::nullopt;
+ } else {
+ auto maybeRef = Fortran::evaluate::ExtractDataRef(expr);
+ if (!maybeRef || !std::holds_alternative<Ref>(maybeRef->u))
+ return std::nullopt;
+ return std::get<Ref>(maybeRef->u);
+ }
+}
+} // namespace detail
+
+template <typename BoundsOp, typename BoundsType>
AddrAndBoundsInfo gatherDataOperandAddrAndBounds(
Fortran::lower::AbstractConverter &converter, fir::FirOpBuilder &builder,
- Fortran::semantics::SemanticsContext &semanticsContext,
- Fortran::lower::StatementContext &stmtCtx, const ObjectType &object,
+ semantics::SemanticsContext &semaCtx,
+ Fortran::lower::StatementContext &stmtCtx,
+ Fortran::semantics::SymbolRef symbol,
+ const Fortran::semantics::MaybeExpr &maybeDesignator,
mlir::Location operandLocation, std::stringstream &asFortran,
llvm::SmallVector<mlir::Value> &bounds, bool treatIndexAsSection = false) {
+ using namespace Fortran;
+
AddrAndBoundsInfo info;
- std::visit(
- Fortran::common::visitors{
- [&](const Fortran::parser::Designator &designator) {
- if (auto expr{Fortran::semantics::AnalyzeExpr(semanticsContext,
- designator)}) {
- if (((*expr).Rank() > 0 || treatIndexAsSection) &&
- Fortran::parser::Unwrap<Fortran::parser::ArrayElement>(
- designator)) {
- const auto *arrayElement =
- Fortran::parser::Unwrap<Fortran::parser::ArrayElement>(
- designator);
- const auto *dataRef =
- std::get_if<Fortran::parser::DataRef>(&designator.u);
- fir::ExtendedValue dataExv;
- bool dataExvIsAssumedSize = false;
- if (Fortran::parser::Unwrap<
- Fortran::parser::StructureComponent>(
- arrayElement->base)) {
- auto exprBase = Fortran::semantics::AnalyzeExpr(
- semanticsContext, arrayElement->base);
- dataExv = converter.genExprAddr(operandLocation, *exprBase,
- stmtCtx);
- info.addr = fir::getBase(dataExv);
- info.rawInput = info.addr;
- asFortran << (*exprBase).AsFortran();
- } else {
- const Fortran::parser::Name &name =
- Fortran::parser::GetLastName(*dataRef);
- dataExvIsAssumedSize = Fortran::semantics::IsAssumedSizeArray(
- name.symbol->GetUltimate());
- info = getDataOperandBaseAddr(converter, builder,
- *name.symbol, operandLocation);
- dataExv = converter.getSymbolExtendedValue(*name.symbol);
- asFortran << name.ToString();
- }
-
- if (!arrayElement->subscripts.empty()) {
- asFortran << '(';
- bounds = genBoundsOps<BoundsOp, BoundsType>(
- builder, operandLocation, converter, stmtCtx,
- arrayElement->subscripts, asFortran, dataExv,
- dataExvIsAssumedSize, info, treatIndexAsSection);
- }
- asFortran << ')';
- } else if (auto structComp = Fortran::parser::Unwrap<
- Fortran::parser::StructureComponent>(designator)) {
- fir::ExtendedValue compExv =
- converter.genExprAddr(operandLocation, *expr, stmtCtx);
- info.addr = fir::getBase(compExv);
- info.rawInput = info.addr;
- if (fir::unwrapRefType(info.addr.getType())
- .isa<fir::SequenceType>())
- bounds = genBaseBoundsOps<BoundsOp, BoundsType>(
- builder, operandLocation, converter, compExv,
- /*isAssumedSize=*/false);
- asFortran << (*expr).AsFortran();
-
- bool isOptional = Fortran::semantics::IsOptional(
- *Fortran::parser::GetLastName(*structComp).symbol);
- if (isOptional)
- info.isPresent = builder.create<fir::IsPresentOp>(
- operandLocation, builder.getI1Type(), info.rawInput);
-
- if (auto loadOp = mlir::dyn_cast_or_null<fir::LoadOp>(
- info.addr.getDefiningOp())) {
- if (fir::isAllocatableType(loadOp.getType()) ||
- fir::isPointerType(loadOp.getType()))
- info.addr = builder.create<fir::BoxAddrOp>(operandLocation,
- info.addr);
- info.rawInput = info.addr;
- }
-
- // If the component is an allocatable or pointer the result of
- // genExprAddr will be the result of a fir.box_addr operation or
- // a fir.box_addr has been inserted just before.
- // Retrieve the box so we handle it like other descriptor.
- if (auto boxAddrOp = mlir::dyn_cast_or_null<fir::BoxAddrOp>(
- info.addr.getDefiningOp())) {
- info.addr = boxAddrOp.getVal();
- info.rawInput = info.addr;
- bounds = genBoundsOpsFromBox<BoundsOp, BoundsType>(
- builder, operandLocation, converter, compExv, info);
- }
- } else {
- if (Fortran::parser::Unwrap<Fortran::parser::ArrayElement>(
- designator)) {
- // Single array element.
- const auto *arrayElement =
- Fortran::parser::Unwrap<Fortran::parser::ArrayElement>(
- designator);
- (void)arrayElement;
- fir::ExtendedValue compExv =
- converter.genExprAddr(operandLocation, *expr, stmtCtx);
- info.addr = fir::getBase(compExv);
- info.rawInput = info.addr;
- asFortran << (*expr).AsFortran();
- } else if (const auto *dataRef{
- std::get_if<Fortran::parser::DataRef>(
- &designator.u)}) {
- // Scalar or full array.
- const Fortran::parser::Name &name =
- Fortran::parser::GetLastName(*dataRef);
- fir::ExtendedValue dataExv =
- converter.getSymbolExtendedValue(*name.symbol);
- info = getDataOperandBaseAddr(converter, builder,
- *name.symbol, operandLocation);
- if (fir::unwrapRefType(info.addr.getType())
- .isa<fir::BaseBoxType>()) {
- bounds = genBoundsOpsFromBox<BoundsOp, BoundsType>(
- builder, operandLocation, converter, dataExv, info);
- }
- bool dataExvIsAssumedSize =
- Fortran::semantics::IsAssumedSizeArray(
- name.symbol->GetUltimate());
- if (fir::unwrapRefType(info.addr.getType())
- .isa<fir::SequenceType>())
- bounds = genBaseBoundsOps<BoundsOp, BoundsType>(
- builder, operandLocation, converter, dataExv,
- dataExvIsAssumedSize);
- asFortran << name.ToString();
- } else { // Unsupported
- llvm::report_fatal_error(
- "Unsupported type of OpenACC operand");
- }
- }
- }
- },
- [&](const Fortran::parser::Name &name) {
- info = getDataOperandBaseAddr(converter, builder, *name.symbol,
- operandLocation);
- asFortran << name.ToString();
- }},
- object.u);
+
+ if (!maybeDesignator) {
+ info = getDataOperandBaseAddr(converter, builder, symbol, operandLocation);
+ asFortran << symbol->name().ToString();
+ return info;
+ }
+
+ semantics::SomeExpr designator = *maybeDesignator;
+
+ if ((designator.Rank() > 0 || treatIndexAsSection) &&
+ IsArrayElement(designator)) {
+ auto arrayRef = detail::getRef<evaluate::ArrayRef>(designator);
+ // This shouldn't fail after IsArrayElement(designator).
+ assert(arrayRef && "Expecting ArrayRef");
+
+ fir::ExtendedValue dataExv;
+ bool dataExvIsAssumedSize = false;
+
+ auto toMaybeExpr = [&](auto &&base) {
+ using BaseType = llvm::remove_cvref_t<decltype(base)>;
+ evaluate::ExpressionAnalyzer ea{semaCtx};
+
+ if constexpr (std::is_same_v<evaluate::NamedEntity, BaseType>) {
+ if (auto *ref = base.UnwrapSymbolRef())
+ return ea.Designate(evaluate::DataRef{*ref});
+ if (auto *ref = base.UnwrapComponent())
+ return ea.Designate(evaluate::DataRef{*ref});
+ llvm_unreachable("Unexpected NamedEntity");
+ } else {
+ static_assert(std::is_same_v<semantics::SymbolRef, BaseType>);
+ return ea.Designate(evaluate::DataRef{base});
+ }
+ };
+
+ auto arrayBase = toMaybeExpr(arrayRef->base());
+ assert(arrayBase);
+
+ if (detail::getRef<evaluate::Component>(*arrayBase)) {
+ dataExv = converter.genExprAddr(operandLocation, *arrayBase, stmtCtx);
+ info.addr = fir::getBase(dataExv);
+ info.rawInput = info.addr;
+ asFortran << arrayBase->AsFortran();
+ } else {
+ const semantics::Symbol &sym = arrayRef->GetLastSymbol();
+ dataExvIsAssumedSize =
+ Fortran::semantics::IsAssumedSizeArray(sym.GetUltimate());
+ info = getDataOperandBaseAddr(converter, builder, sym, operandLocation);
+ dataExv = converter.getSymbolExtendedValue(sym);
+ asFortran << sym.name().ToString();
+ }
+
+ if (!arrayRef->subscript().empty()) {
+ asFortran << '(';
+ bounds = genBoundsOps<BoundsOp, BoundsType>(
+ builder, operandLocation, converter, stmtCtx, arrayRef->subscript(),
+ asFortran, dataExv, dataExvIsAssumedSize, info, treatIndexAsSection);
+ }
+ asFortran << ')';
+ } else if (auto compRef = detail::getRef<evaluate::Component>(designator)) {
+ fir::ExtendedValue compExv =
+ converter.genExprAddr(operandLocation, designator, stmtCtx);
+ info.addr = fir::getBase(compExv);
+ info.rawInput = info.addr;
+ if (fir::unwrapRefType(info.addr.getType()).isa<fir::SequenceType>())
+ bounds = genBaseBoundsOps<BoundsOp, BoundsType>(builder, operandLocation,
+ converter, compExv,
+ /*isAssumedSize=*/false);
+ asFortran << designator.AsFortran();
+
+ if (semantics::IsOptional(compRef->GetLastSymbol())) {
+ info.isPresent = builder.create<fir::IsPresentOp>(
+ operandLocation, builder.getI1Type(), info.rawInput);
+ }
+
+ if (auto loadOp =
+ mlir::dyn_cast_or_null<fir::LoadOp>(info.addr.getDefiningOp())) {
+ if (fir::isAllocatableType(loadOp.getType()) ||
+ fir::isPointerType(loadOp.getType()))
+ info.addr = builder.create<fir::BoxAddrOp>(operandLocation, info.addr);
+ info.rawInput = info.addr;
+ }
+
+ // If the component is an allocatable or pointer the result of
+ // genExprAddr will be the result of a fir.box_addr operation or
+ // a fir.box_addr has been inserted just before.
+ // Retrieve the box so we handle it like other descriptor.
+ if (auto boxAddrOp =
+ mlir::dyn_cast_or_null<fir::BoxAddrOp>(info.add...
[truncated]
|
kparzysz
force-pushed
the
users/kparzysz/spr/b03-rep1
branch
from
be33fa2
to
841f10e
Compare
kparzysz
force-pushed
the
users/kparzysz/spr/b04-rep2
branch
from
f4ed9e5
to
8743715
Compare
Introduce a set of generic classes (templates) that represent OpenMP clauses in a language-agnostic manner. OpenMP clauses can contain expressions and data objects and the exact representation of each depends on the source language of the compiled program. To deal with this, the templates depend on two type parameters: - IdType: type that represent object's identity (in a way that satisfied OpenMP requirements), and - ExprType: type that can represent numeric values, as well as data references (e.g. x.y[1].z[2]). In addition to that, implement code instantiating these templates from flang's AST. This patch only introduces the new classes, they are not yet used anywhere.
Temporarily rename old clause list to `clauses2`, old clause iterator to `ClauseIterator2`. Change `findUniqueClause` to iterate over `omp::Clause` objects, modify all handlers to operate on 'omp::clause::xyz` equivalents.
…essor Rename `findRepeatableClause` to `findRepeatableClause2`, and make the new `findRepeatableClause` operate on new `omp::Clause` objects. Leave `Map` unchanged, because it will require more changes for it to work.
kparzysz
force-pushed
the
users/kparzysz/spr/b03-rep1
branch
from
e32bfaa
to
655dce5
Compare
The related functions are `gatherDataOperandAddrAndBounds` and `genBoundsOps`. The former is used in OpenACC as well, and it was updated to pass evaluate::Expr instead of parser objects. The difference in the test case comes from unfolded conversions of index expressions, which are explicitly of type integer(kind=8). Delete now unused `findRepeatableClause2` and `findClause2`. Add `AsGenericExpr` that takes std::optional. It already returns optional Expr. Making it accept an optional Expr as input would reduce the number of necessary checks when handling frequent optional values in evaluator.
kparzysz
force-pushed
the
users/kparzysz/spr/b04-rep2
branch
from
923b546
to
bf64cd3
Compare
|
Wrong PR. |
skatrak
reviewed
Mar 12, 2024
|
Hey @clementval, @jeanPerier, could you take a look at the OpenACC changes? |
I don't see any test case updates. What is this referring to? |
clementval
reviewed
Mar 18, 2024
| // Helper class for stripping enclosing parentheses and a conversion that | ||
| // preserves type category. This is used for triplet elements, which are | ||
| // always of type integer(kind=8). The lower/upper bounds are converted to | ||
| // an "index" type, which is 64-bit, so the explicit conversion to kind=8 |
There was a problem hiding this comment.
index type in MLIR is not guaranteed to be 64-bit.
There was a problem hiding this comment.
Ok.
Then I can remove this function to keep the semantics right, but the variable names in some test files will need to be updated to have the explicit "int(..., kind=8)" in there. Would that be ok?
There was a problem hiding this comment.
Can you post an example of var name before/after? I guess this is coming from the AsFortran function.
There was a problem hiding this comment.
One specific difference below. There are several instances of this with different variables. The name is in acc.create, second line from the end in each snippet.
1,9c1,11
< %28 = fir.convert %27 : (i32) -> index
< %29 = arith.subi %28, %6 : index
< %30 = fir.load %3#0 : !fir.ref<i32>
< %31 = fir.convert %30 : (i32) -> index
< %32 = arith.subi %31, %6 : index
< %33 = acc.bounds lowerbound(%29 : index) upperbound(%32 : index) extent(%13 : index) stride(%26#2 : index) startIdx(%6 : index) {strideInBytes = true}
< %34 = fir.box_addr %15#0 : (!fir.box<!fir.array<?xf32>>) -> !fir.ref<!fir.array<?xf32>>
< %35 = acc.create varPtr(%34 : !fir.ref<!fir.array<?xf32>>) bounds(%33) -> !fir.ref<!fir.array<?xf32>> {name = "b(n:m)", structured = false}
< acc.enter_data dataOperands(%35 : !fir.ref<!fir.array<?xf32>>)
---
> %28 = fir.convert %27 : (i32) -> i64
> %29 = fir.convert %28 : (i64) -> index
> %30 = arith.subi %29, %6 : index
> %31 = fir.load %3#0 : !fir.ref<i32>
> %32 = fir.convert %31 : (i32) -> i64
> %33 = fir.convert %32 : (i64) -> index
> %34 = arith.subi %33, %6 : index
> %35 = acc.bounds lowerbound(%30 : index) upperbound(%34 : index) extent(%13 : index) stride(%26#2 : index) startIdx(%6 : index) {strideInBytes = true}
> %36 = fir.box_addr %15#0 : (!fir.box<!fir.array<?xf32>>) -> !fir.ref<!fir.array<?xf32>>
> %37 = acc.create varPtr(%36 : !fir.ref<!fir.array<?xf32>>) bounds(%35) -> !fir.ref<!fir.array<?xf32>> {name = "b(int(n,kind=8):int(m,kind=8))", structured = false}
> acc.enter_data dataOperands(%37 : !fir.ref<!fir.array<?xf32>>)
There was a problem hiding this comment.
Ok I see. This is indeed not ideal since the name is meant to be used for error reporting and so on and should at much as possible represent what was in the original code. Would it be possible to peal the expr only when we use it with the asFortran stringstream and keep the real expression to keep the semantic right?
There was a problem hiding this comment.
Thanks. The changes on the OpenACC and DirectiveCommon side look ok to me. I'll let someone from the OpenMP side to approve the rest of the changes.
I can give you a better answer once I try it out again (I forgot which tests were affected), but some variable names in the output MLIR had the string "kind(8)" or "kind=8" added to it. This was one visible difference from before my patch, which I wanted to explicitly highlight. I thought it might be controversial, so eventually I added a function |
|
Windows failure is due to |
kparzysz
commented
Mar 20, 2024
| AddrAndBoundsInfo info; | ||
| std::visit( |
There was a problem hiding this comment.
The biggest change in this code was eliminating the call to std::visit, and instead handling the two cases for a former OmpObject object individually:
- if the object was just a Name, it won't have a designator (which is now provided as an argument)
- otherwise the designator will be non-nullopt.
In addition to that the nested call to AnalyzeExpr was no longer needed, which together shifted the code starting at line 1009 (in the old version) to the left. The new version is basically the interior of the "designator" visitor reformatted, with potential adjustments to handle new arguments instead of the old ones.
|
The build failure is due to the Windows bot and the seemingly traditional |
chencha3
pushed a commit
to chencha3/llvm-project
that referenced
this pull request
Mar 23, 2024
llvm#81626) The related functions are `gatherDataOperandAddrAndBounds` and `genBoundsOps`. The former is used in OpenACC as well, and it was updated to pass evaluate::Expr instead of parser objects. The difference in the test case comes from unfolded conversions of index expressions, which are explicitly of type integer(kind=8). Delete now unused `findRepeatableClause2` and `findClause2`. Add `AsGenericExpr` that takes std::optional. It already returns optional Expr. Making it accept an optional Expr as input would reduce the number of necessary checks when handling frequent optional values in evaluator. [Clause representation 4/6]
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The related functions are
gatherDataOperandAddrAndBoundsandgenBoundsOps. The former is used in OpenACC as well, and it was updated to pass evaluate::Expr instead of parser objects.The difference in the test case comes from unfolded conversions of index expressions, which are explicitly of type integer(kind=8).
Delete now unused
findRepeatableClause2andfindClause2.Add
AsGenericExprthat takes std::optional. It already returns optional Expr. Making it accept an optional Expr as input would reduce the number of necessary checks when handling frequent optional values in evaluator.[Clause representation 4/6]