[flang] Retrieve shape from selector when generating assoc sym type #137117
Conversation
|
@llvm/pr-subscribers-flang-fir-hlfir Author: Kareem Ergawy (ergawy) ChangesThis PR extends associate(a => aa(4:))
do concurrent (i = 4:11) local(a)
a(i) = 0
end do
end associatebefore the changes in the PR, flang would assert that we are casting between incompatible types. The issue happened since for the associating symbol ( Full diff: https://github.com/llvm/llvm-project/pull/137117.diff 2 Files Affected:
diff --git a/flang/lib/Lower/ConvertType.cpp b/flang/lib/Lower/ConvertType.cpp
index d45f9e7c0bf1b..875bdba6cc6ba 100644
--- a/flang/lib/Lower/ConvertType.cpp
+++ b/flang/lib/Lower/ConvertType.cpp
@@ -279,6 +279,23 @@ struct TypeBuilderImpl {
bool isPolymorphic = (Fortran::semantics::IsPolymorphic(symbol) ||
Fortran::semantics::IsUnlimitedPolymorphic(symbol)) &&
!Fortran::semantics::IsAssumedType(symbol);
+ if (const auto *assocDetails =
+ ultimate.detailsIf<Fortran::semantics::AssocEntityDetails>()) {
+ const auto &selector = assocDetails->expr();
+
+ if (selector && selector->Rank() > 0) {
+ auto shapeExpr = Fortran::evaluate::GetShape(
+ converter.getFoldingContext(), selector);
+
+ fir::SequenceType::Shape shape;
+ // If there is no shapExpr, this is an assumed-rank, and the empty shape
+ // will build the desired fir.array<*:T> type.
+ if (shapeExpr)
+ translateShape(shape, std::move(*shapeExpr));
+ ty = fir::SequenceType::get(shape, ty);
+ }
+ }
+
if (ultimate.IsObjectArray()) {
auto shapeExpr =
Fortran::evaluate::GetShape(converter.getFoldingContext(), ultimate);
diff --git a/flang/test/Lower/do_concurrent_local_assoc_entity.f90 b/flang/test/Lower/do_concurrent_local_assoc_entity.f90
new file mode 100644
index 0000000000000..ca16ecaa5c137
--- /dev/null
+++ b/flang/test/Lower/do_concurrent_local_assoc_entity.f90
@@ -0,0 +1,22 @@
+! RUN: %flang_fc1 -emit-hlfir -o - %s | FileCheck %s
+
+subroutine local_assoc
+ implicit none
+ integer i
+ real, dimension(2:11) :: aa
+
+ associate(a => aa(4:))
+ do concurrent (i = 4:11) local(a)
+ a(i) = 0
+ end do
+ end associate
+end subroutine local_assoc
+
+! CHECK: %[[C8:.*]] = arith.constant 8 : index
+
+! CHECK: fir.do_loop {{.*}} unordered {
+! CHECK: %[[LOCAL_ALLOC:.*]] = fir.alloca !fir.array<8xf32> {bindc_name = "a", pinned, uniq_name = "{{.*}}local_assocEa"}
+! CHECK: %[[LOCAL_SHAPE:.*]] = fir.shape %[[C8]] :
+! CHECK: %[[LOCAL_DECL:.*]]:2 = hlfir.declare %[[LOCAL_ALLOC]](%[[LOCAL_SHAPE]])
+! CHECK: hlfir.designate %[[LOCAL_DECL]]#0 (%{{.*}})
+! CHECK: }
|
| @@ -279,6 +279,23 @@ struct TypeBuilderImpl { | ||
| bool isPolymorphic = (Fortran::semantics::IsPolymorphic(symbol) || | ||
| Fortran::semantics::IsUnlimitedPolymorphic(symbol)) && | ||
| !Fortran::semantics::IsAssumedType(symbol); | ||
| if (const auto *assocDetails = |
There was a problem hiding this comment.
Not very familiar with this part of the codebase, so hopefully this is a sane fix.
There was a problem hiding this comment.
Did you look into why ultimate.IsObjectArray is returning false for that case?
That seems a bit odd to me.
There was a problem hiding this comment.
Thanks for the review and sorry for the late response, was busy doing other stuff.
Did you look into why
ultimate.IsObjectArrayis returning false for that case?
This is because it calls Symbol::GetShape and Symbol::GetShape returns the shape only in the case of ObjectEntityDetails. For AssocEntityDetails (and other variants), it returns null. See https://github.com/llvm/llvm-project/blob/main/flang/lib/Semantics/symbol.cpp#L401.
Do you think we should extend GetShape to work with AssocEntityDetails as well instead of what I am doing here?
d5d5db3 to
c559af9
Compare
| @@ -279,6 +279,23 @@ struct TypeBuilderImpl { | ||
| bool isPolymorphic = (Fortran::semantics::IsPolymorphic(symbol) || | ||
| Fortran::semantics::IsUnlimitedPolymorphic(symbol)) && | ||
| !Fortran::semantics::IsAssumedType(symbol); | ||
| if (const auto *assocDetails = |
There was a problem hiding this comment.
Did you look into why ultimate.IsObjectArray is returning false for that case?
That seems a bit odd to me.
|
Ping! please take another look when you have time. |
c559af9 to
10e4545
Compare
This PR extends `genSymbolType` so that the type of an associating symbol
carries the shape of the selector expression, if any. This is a fix for
a bug that triggered when an associating symbol is used in a locality
specifier. For example, given the following input:
```fortran
associate(a => aa(4:))
do concurrent (i = 4:11) local(a)
a(i) = 0
end do
end associate
```
before the changes in the PR, flang would assert that we are casting
between incompatible types. The issue happened since for the associating
symbol (`a`), flang generated its type as `f32` rather than
`!fir.array<8xf32>` as it should be in this case.
10e4545 to
6211696
Compare
This PR extends
genSymbolTypeso that the type of an associating symbol carries the shape of the selector expression, if any. This is a fix for a bug that triggered when an associating symbol is used in a locality specifier. For example, given the following input:before the changes in the PR, flang would assert that we are casting between incompatible types. The issue happened since for the associating symbol (
a), flang generated its type asf32rather than!fir.array<8xf32>as it should be in this case.