[flang-commits] [flang] [flang][runtime] Fix another IsContiguous edge case (PR #69199)
via flang-commits
flang-commits at lists.llvm.org
Mon Oct 16 10:16:08 PDT 2023
https://github.com/jeanPerier updated https://github.com/llvm/llvm-project/pull/69199
>From 4c2595b8d8c092cb8ccd436ef512988608b340cd Mon Sep 17 00:00:00 2001
From: Jean Perier <jperier at nvidia.com>
Date: Mon, 16 Oct 2023 06:17:36 -0700
Subject: [PATCH 1/2] [flang][runtime] Fix another IsContiguous edge case
A recent PR addressed zero and one element edge cases but did not
cover another case where the descriptors of arrays with more than
two elements may have byte strides that are not perfect multiples,
like when creating a descriptor for A(:, 1:1:2).
In general, the byte stride in a dimension is only meaningful if that
dimension has more than one element. Update IsContiguous and
CFI_is_contiguous to reflect that.
---
flang/include/flang/Runtime/descriptor.h | 10 ++++--
flang/runtime/ISO_Fortran_binding.cpp | 9 +++---
.../Evaluate/ISO-Fortran-binding.cpp | 32 +++++++++++++++++++
3 files changed, 43 insertions(+), 8 deletions(-)
diff --git a/flang/include/flang/Runtime/descriptor.h b/flang/include/flang/Runtime/descriptor.h
index c69bb336dd29e7d..3eea3f30352947b 100644
--- a/flang/include/flang/Runtime/descriptor.h
+++ b/flang/include/flang/Runtime/descriptor.h
@@ -393,13 +393,17 @@ class Descriptor {
bool stridesAreContiguous{true};
for (int j{0}; j < leadingDimensions; ++j) {
const Dimension &dim{GetDimension(j)};
- stridesAreContiguous &= bytes == dim.ByteStride();
+ stridesAreContiguous &= bytes == dim.ByteStride() | dim.Extent() == 1;
bytes *= dim.Extent();
}
// One and zero element arrays are contiguous even if the descriptor
// byte strides are not perfect multiples.
- return stridesAreContiguous || bytes == 0 ||
- bytes == static_cast<SubscriptValue>(ElementBytes());
+ // Arrays with more than 2 elements may also be contiguous even if a
+ // byte stride in one dimension is not a perfect multiple, as long as
+ // this is the last dimension, or if the dimension has one extent and
+ // the following dimension have either one extents or contiguous byte
+ // strides.
+ return stridesAreContiguous || bytes == 0;
}
// Establishes a pointer to a section or element.
diff --git a/flang/runtime/ISO_Fortran_binding.cpp b/flang/runtime/ISO_Fortran_binding.cpp
index 103413cb7140aaa..c6320745ba17697 100644
--- a/flang/runtime/ISO_Fortran_binding.cpp
+++ b/flang/runtime/ISO_Fortran_binding.cpp
@@ -125,16 +125,15 @@ RT_API_ATTRS int CFI_establish(CFI_cdesc_t *descriptor, void *base_addr,
}
RT_API_ATTRS int CFI_is_contiguous(const CFI_cdesc_t *descriptor) {
+ // See Descriptor::IsContiguous for the rational.
bool stridesAreContiguous{true};
CFI_index_t bytes = descriptor->elem_len;
for (int j{0}; j < descriptor->rank; ++j) {
- stridesAreContiguous &= bytes == descriptor->dim[j].sm;
+ stridesAreContiguous &=
+ bytes == descriptor->dim[j].sm | descriptor->dim[j].extent == 1;
bytes *= descriptor->dim[j].extent;
}
- // One and zero element arrays are contiguous even if the descriptor
- // byte strides are not perfect multiples.
- if (stridesAreContiguous || bytes == 0 ||
- bytes == static_cast<CFI_index_t>(descriptor->elem_len)) {
+ if (stridesAreContiguous || bytes == 0) {
return 1;
}
return 0;
diff --git a/flang/unittests/Evaluate/ISO-Fortran-binding.cpp b/flang/unittests/Evaluate/ISO-Fortran-binding.cpp
index d1f0a31454056bf..3c98363f9004664 100644
--- a/flang/unittests/Evaluate/ISO-Fortran-binding.cpp
+++ b/flang/unittests/Evaluate/ISO-Fortran-binding.cpp
@@ -736,6 +736,38 @@ static void run_CFI_is_contiguous_tests() {
MATCH(true, retCode == CFI_SUCCESS);
MATCH(true, CFI_is_contiguous(section) == 0);
MATCH(false, sectionDesc->IsContiguous());
+
+ // Test section B = A(0:3:1,0:0:2) is contiguous.
+ lb[0] = 0;
+ lb[1] = 0;
+ ub[0] = 3;
+ ub[1] = 0;
+ strides[0] = 1;
+ strides[1] = 2;
+ retCode = CFI_section(section, dv, lb, ub, strides);
+ MATCH(true, retCode == CFI_SUCCESS);
+ MATCH(true, CFI_is_contiguous(section) == 1);
+ MATCH(true, sectionDesc->IsContiguous());
+
+ // INTEGER :: C(0:0, 0:3)
+ CFI_index_t c_extents[rank] = {1, 4};
+ CFI_CDESC_T(rank) c_dv_storage;
+ CFI_cdesc_t *cdv{&c_dv_storage};
+ retCode = CFI_establish(cdv, base_addr, CFI_attribute_other, CFI_type_int,
+ /*elem_len=*/0, rank, c_extents);
+ MATCH(retCode == CFI_SUCCESS, true);
+
+ // Test section B = C(0:0:2, 0:3:1) is contiguous.
+ lb[0] = 0;
+ lb[1] = 0;
+ ub[0] = 0;
+ ub[1] = 3;
+ strides[0] = 2;
+ strides[1] = 1;
+ retCode = CFI_section(section, cdv, lb, ub, strides);
+ MATCH(true, retCode == CFI_SUCCESS);
+ MATCH(true, CFI_is_contiguous(section) == 1);
+ MATCH(true, sectionDesc->IsContiguous());
}
int main() {
>From dfe160f1a80ad16a83d6df5ba4055ee704206393 Mon Sep 17 00:00:00 2001
From: Jean Perier <jperier at nvidia.com>
Date: Mon, 16 Oct 2023 10:14:38 -0700
Subject: [PATCH 2/2] address review comments
Rational -> Rationale.
Silence warning with extra parenthesis.
---
flang/include/flang/Runtime/descriptor.h | 2 +-
flang/runtime/ISO_Fortran_binding.cpp | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/flang/include/flang/Runtime/descriptor.h b/flang/include/flang/Runtime/descriptor.h
index 3eea3f30352947b..85240353e8ae9e0 100644
--- a/flang/include/flang/Runtime/descriptor.h
+++ b/flang/include/flang/Runtime/descriptor.h
@@ -393,7 +393,7 @@ class Descriptor {
bool stridesAreContiguous{true};
for (int j{0}; j < leadingDimensions; ++j) {
const Dimension &dim{GetDimension(j)};
- stridesAreContiguous &= bytes == dim.ByteStride() | dim.Extent() == 1;
+ stridesAreContiguous &= (bytes == dim.ByteStride()) | (dim.Extent() == 1);
bytes *= dim.Extent();
}
// One and zero element arrays are contiguous even if the descriptor
diff --git a/flang/runtime/ISO_Fortran_binding.cpp b/flang/runtime/ISO_Fortran_binding.cpp
index c6320745ba17697..c2e82758ae08aef 100644
--- a/flang/runtime/ISO_Fortran_binding.cpp
+++ b/flang/runtime/ISO_Fortran_binding.cpp
@@ -125,12 +125,12 @@ RT_API_ATTRS int CFI_establish(CFI_cdesc_t *descriptor, void *base_addr,
}
RT_API_ATTRS int CFI_is_contiguous(const CFI_cdesc_t *descriptor) {
- // See Descriptor::IsContiguous for the rational.
+ // See Descriptor::IsContiguous for the rationale.
bool stridesAreContiguous{true};
CFI_index_t bytes = descriptor->elem_len;
for (int j{0}; j < descriptor->rank; ++j) {
stridesAreContiguous &=
- bytes == descriptor->dim[j].sm | descriptor->dim[j].extent == 1;
+ (bytes == descriptor->dim[j].sm) | (descriptor->dim[j].extent == 1);
bytes *= descriptor->dim[j].extent;
}
if (stridesAreContiguous || bytes == 0) {
More information about the flang-commits
mailing list