[flang-commits] [flang] aaab704 - [flang] Fix handling of elem_len in CFI_establish

[Diana Picus via flang-commits]

Author: Diana Picus
Date: 2021-05-03T08:08:07Z
New Revision: aaab70407beb9526722b04ae3dd14def4eaab540

URL: https://github.com/llvm/llvm-project/commit/aaab70407beb9526722b04ae3dd14def4eaab540
DIFF: https://github.com/llvm/llvm-project/commit/aaab70407beb9526722b04ae3dd14def4eaab540.diff

LOG: [flang] Fix handling of elem_len in CFI_establish

The current code computes the minimum element length based on the `type`
used to create the descriptor and uses that as the element length
whenever it is greater than 0. This means that the `elem_len` parameter
is essentially ignored for any type where we can compute a minimum
element length (which includes `CFI_type_char[16|32]_t`), and we may
therefore end up with descriptors with a lower element length than
expected.

This patch fixes the issue by explicitly doing what the standard says,
i.e. it uses the given `elem_len` for character types, `CFI_type_struct`
and `CFI_type_other`, and ignores it (falls back to the minimum element
length) for everything else.

Differential Revision: https://reviews.llvm.org/D101659

Added: 
    

Modified: 
    flang/runtime/ISO_Fortran_binding.cpp
    flang/unittests/Evaluate/ISO-Fortran-binding.cpp

Removed: 
    


################################################################################
diff  --git a/flang/runtime/ISO_Fortran_binding.cpp b/flang/runtime/ISO_Fortran_binding.cpp
index 8180c22f01a7d..492d383b19bc2 100644
--- a/flang/runtime/ISO_Fortran_binding.cpp
+++ b/flang/runtime/ISO_Fortran_binding.cpp
@@ -234,11 +234,13 @@ int CFI_establish(CFI_cdesc_t *descriptor, void *base_addr,
   if (!descriptor) {
     return CFI_INVALID_DESCRIPTOR;
   }
-  std::size_t minElemLen{MinElemLen(type)};
-  if (minElemLen > 0) {
-    elem_len = minElemLen;
-  } else if (elem_len <= 0) {
-    return CFI_INVALID_ELEM_LEN;
+  if (type == CFI_type_struct || type == CFI_type_other ||
+      IsCharacterType(type)) {
+    if (elem_len <= 0)
+      return CFI_INVALID_ELEM_LEN;
+  } else {
+    elem_len = MinElemLen(type);
+    assert(elem_len > 0 && "Unknown element length for type");
   }
   descriptor->base_addr = base_addr;
   descriptor->elem_len = elem_len;

diff  --git a/flang/unittests/Evaluate/ISO-Fortran-binding.cpp b/flang/unittests/Evaluate/ISO-Fortran-binding.cpp
index e97024eb8de17..863189250a3d3 100644
--- a/flang/unittests/Evaluate/ISO-Fortran-binding.cpp
+++ b/flang/unittests/Evaluate/ISO-Fortran-binding.cpp
@@ -114,6 +114,7 @@ static void check_CFI_establish(CFI_cdesc_t *dv, void *base_addr,
       }
     }
     if (type == CFI_type_struct || type == CFI_type_char ||
+        type == CFI_type_char16_t || type == CFI_type_char32_t ||
         type == CFI_type_other) {
       MATCH(elem_len, res->ElementBytes());
     }
@@ -136,6 +137,7 @@ static void check_CFI_establish(CFI_cdesc_t *dv, void *base_addr,
   }
 
   if ((type == CFI_type_struct || type == CFI_type_char ||
+          type == CFI_type_char16_t || type == CFI_type_char32_t ||
           type == CFI_type_other) &&
       elem_len <= 0) {
     ++numErr;
@@ -166,7 +168,8 @@ static void run_CFI_establish_tests() {
   CFI_attribute_t attrCases[]{
       CFI_attribute_pointer, CFI_attribute_allocatable, CFI_attribute_other};
   CFI_type_t typeCases[]{CFI_type_int, CFI_type_struct, CFI_type_double,
-      CFI_type_char, CFI_type_other, CFI_TYPE_LAST + 1};
+      CFI_type_char, CFI_type_char16_t, CFI_type_char32_t, CFI_type_other,
+      CFI_TYPE_LAST + 1};
   CFI_index_t *extentCases[]{extents, nullptr};
   void *baseAddrCases[]{dummyAddr, nullptr};
   CFI_rank_t rankCases[]{0, 1, CFI_MAX_RANK, CFI_MAX_RANK + 1};