[flang] Better IS_CONTIGUOUS folding for substrings #115970

klausler · 2024-11-13T00:44:49Z

At present, the compiler doesn't analyze substring references for contiguity. But there are cases where substrings can be known to be contiguous (scalar base, empty substring, or complete substring) or can be known to be discontiguous, and references to the intrinsic function IS_CONTIGUOUS in those cases may appear in constant expressions.

Fixes #115675.

llvmbot · 2024-11-13T00:45:23Z

@llvm/pr-subscribers-flang-fir-hlfir

Author: Peter Klausler (klausler)

Changes

At present, the compiler doesn't analyze substring references for contiguity. But there are cases where substrings can be known to be contiguous (scalar base, empty substring, or complete substring) or can be known to be discontiguous, and references to the intrinsic function IS_CONTIGUOUS in those cases may appear in constant expressions.

Fixes #115675.

Full diff: https://github.com/llvm/llvm-project/pull/115970.diff

6 Files Affected:

(modified) flang/lib/Evaluate/check-expression.cpp (+33-1)
(modified) flang/test/Evaluate/folding09.f90 (+7)
(modified) flang/test/Lower/HLFIR/maxloc.f90 (+2-2)
(modified) flang/test/Lower/HLFIR/maxval.f90 (+2-2)
(modified) flang/test/Lower/HLFIR/minloc.f90 (+2-2)
(modified) flang/test/Lower/HLFIR/minval.f90 (+2-2)

diff --git a/flang/lib/Evaluate/check-expression.cpp b/flang/lib/Evaluate/check-expression.cpp
index 38794a2d8aacc7..efc08763aa9ad6 100644
--- a/flang/lib/Evaluate/check-expression.cpp
+++ b/flang/lib/Evaluate/check-expression.cpp
@@ -908,7 +908,39 @@ class IsContiguousHelper
   Result operator()(const ComplexPart &x) const {
     return x.complex().Rank() == 0;
   }
-  Result operator()(const Substring &) const { return std::nullopt; }
+  Result operator()(const Substring &x) const {
+    auto base{x.GetBaseObject()};
+    if (x.Rank() == 0) {
+      return true; // scalar substring always contiguous
+    }
+    Result result{(*this)(base)};
+    if (!result || *result) {
+      if (auto lower{ToInt64(Fold(context_, x.lower()))}) {
+        auto upperExpr{x.upper()};
+        auto lenExpr{base.LEN()};
+        if (!upperExpr) {
+          upperExpr = lenExpr;
+        }
+        if (!upperExpr) {
+          return std::nullopt; // could be empty substrings
+        } else if (auto upper{ToInt64(Fold(context_, std::move(*upperExpr)))}) {
+          if (*upper < *lower) {
+            return true; // empty substrings: vacuously contiguous
+          } else if (*lower > 1) {
+            return false; // lower > 1
+          } else if (lenExpr) {
+            if (auto lenVal{ToInt64(Fold(context_, std::move(*lenExpr)))};
+                lenVal && *lenVal != *upper) {
+              return false; // upper < length
+            }
+          }
+        }
+      } else {
+        return std::nullopt; // lower bound not known
+      }
+    }
+    return result;
+  }
 
   Result operator()(const ProcedureRef &x) const {
     if (auto chars{characteristics::Procedure::Characterize(
diff --git a/flang/test/Evaluate/folding09.f90 b/flang/test/Evaluate/folding09.f90
index 863b5e873a1e5b..b9c00710a086ca 100644
--- a/flang/test/Evaluate/folding09.f90
+++ b/flang/test/Evaluate/folding09.f90
@@ -21,6 +21,7 @@ subroutine test(arr1, arr2, arr3, mat, alloc)
     real, intent(in), contiguous :: arr3(:)
     real, allocatable :: alloc(:)
     real :: scalar
+    character(5) charr(5)
     integer(kind=merge(1,-1,       is_contiguous(0)))               t01
     integer(kind=merge(1,-1,       is_contiguous(scalar)))          t02
     integer(kind=merge(1,-1,       is_contiguous(scalar + scalar))) t03
@@ -35,6 +36,12 @@ subroutine test(arr1, arr2, arr3, mat, alloc)
     integer(kind=merge(1,-1, .not. is_contiguous(arr3(1:10:2))))    t12
     integer(kind=merge(1,-1,       is_contiguous(f())))             t13
     integer(kind=merge(1,-1,       is_contiguous(alloc)))           t14
+    integer(kind=merge(1,-1,       is_contiguous(charr(:)(:))))     t15
+    integer(kind=merge(1,-1,       is_contiguous(charr(1)(2:3))))   t16
+    integer(kind=merge(1,-1,       is_contiguous(charr(:)(1:))))    t17
+    integer(kind=merge(1,-1,       is_contiguous(charr(:)(3:2))))   t18
+    integer(kind=merge(1,-1,       is_contiguous(charr(:)(1:5))))   t19
+    integer(kind=merge(1,-1, .not. is_contiguous(charr(:)(1:4))))   t20
     associate (x => arr2)
       block
         integer(kind=merge(1,-1,is_contiguous(x))) n
diff --git a/flang/test/Lower/HLFIR/maxloc.f90 b/flang/test/Lower/HLFIR/maxloc.f90
index 166a1b9db1724e..539affad2d7df1 100644
--- a/flang/test/Lower/HLFIR/maxloc.f90
+++ b/flang/test/Lower/HLFIR/maxloc.f90
@@ -341,8 +341,8 @@ end subroutine test_unknown_char_len_result
 ! CHECK-DAG:       %[[C1_7:.*]] = arith.constant 1 : index
 ! CHECK-DAG:       %[[C3_8:.*]] = arith.constant 3 : index
 ! CHECK-DAG:       %[[C3_9:.*]] = arith.constant 3 : index
-! CHECK-DAG:       %[[ARRAY_BOX:.*]] = hlfir.designate %[[ARRAY]]#0 (%[[C1]]:%[[C3_0]]:%[[C1_3]], %[[C1]]:%[[C3_1]]:%[[C1_5]]) substr %[[C1_7]], %[[C3_8]]  shape %[[SHAPE]] typeparams %[[C3_9]]
-! CHECK:           %[[EXPR:.*]] = hlfir.maxloc %[[ARRAY_BOX]] {fastmath = #arith.fastmath<contract>} : (!fir.box<!fir.array<3x3x!fir.char<1,3>>>) -> !hlfir.expr<2xi32>
+! CHECK-DAG:       %[[ARRAY_REF:.*]] = hlfir.designate %[[ARRAY]]#0 (%[[C1]]:%[[C3_0]]:%[[C1_3]], %[[C1]]:%[[C3_1]]:%[[C1_5]]) substr %[[C1_7]], %[[C3_8]]  shape %[[SHAPE]] typeparams %[[C3_9]] : (!fir.ref<!fir.array<3x3x!fir.char<1,3>>>, index, index, index, index, index, index, index, index, !fir.shape<2>, index) -> !fir.ref<!fir.array<3x3x!fir.char<1,3>>>
+! CHECK:           %[[EXPR:.*]] = hlfir.maxloc %[[ARRAY_REF]] {fastmath = #arith.fastmath<contract>} : (!fir.ref<!fir.array<3x3x!fir.char<1,3>>>) -> !hlfir.expr<2xi32>
 ! CHECK-NEXT:      hlfir.assign %[[EXPR]] to %[[RES]]#0 : !hlfir.expr<2xi32>, !fir.ref<!fir.array<2xi32>>
 ! CHECK-NEXT:      hlfir.destroy %[[EXPR]]
 ! CHECK-NEXT:      return
diff --git a/flang/test/Lower/HLFIR/maxval.f90 b/flang/test/Lower/HLFIR/maxval.f90
index 5adad286a77d28..32e1a80417a27b 100644
--- a/flang/test/Lower/HLFIR/maxval.f90
+++ b/flang/test/Lower/HLFIR/maxval.f90
@@ -254,8 +254,8 @@ end subroutine test_unknown_char_len_result
 ! CHECK-DAG:       %[[C1_7:.*]] = arith.constant 1 : index
 ! CHECK-DAG:       %[[C3_8:.*]] = arith.constant 3 : index
 ! CHECK-DAG:       %[[C3_9:.*]] = arith.constant 3 : index
-! CHECK-DAG:       %[[ARRAY_BOX:.*]] = hlfir.designate %[[ARRAY]]#0 (%[[C1]]:%[[C3_0]]:%[[C1_3]], %[[C1]]:%[[C3_1]]:%[[C1_5]]) substr %[[C1_7]], %[[C3_8]]  shape %[[SHAPE]] typeparams %[[C3_9]]
-! CHECK:           %[[EXPR:.*]] = hlfir.maxval %[[ARRAY_BOX]] {fastmath = #arith.fastmath<contract>} : (!fir.box<!fir.array<3x3x!fir.char<1,3>>>) -> !hlfir.expr<!fir.char<1,3>>
+! CHECK-DAG:       %[[ARRAY_REF:.*]] = hlfir.designate %[[ARRAY]]#0 (%[[C1]]:%[[C3_0]]:%[[C1_3]], %[[C1]]:%[[C3_1]]:%[[C1_5]]) substr %[[C1_7]], %[[C3_8]]  shape %[[SHAPE]] typeparams %[[C3_9]] : (!fir.ref<!fir.array<3x3x!fir.char<1,3>>>, index, index, index, index, index, index, index, index, !fir.shape<2>, index) -> !fir.ref<!fir.array<3x3x!fir.char<1,3>>>
+! CHECK:           %[[EXPR:.*]] = hlfir.maxval %[[ARRAY_REF]] {fastmath = #arith.fastmath<contract>} : (!fir.ref<!fir.array<3x3x!fir.char<1,3>>>) -> !hlfir.expr<!fir.char<1,3>>
 ! CHECK-NEXT:      hlfir.assign %[[EXPR]] to %[[RES]]#0 : !hlfir.expr<!fir.char<1,3>>, !fir.ref<!fir.char<1,3>>
 ! CHECK-NEXT:      hlfir.destroy %[[EXPR]]
 ! CHECK-NEXT:      return
diff --git a/flang/test/Lower/HLFIR/minloc.f90 b/flang/test/Lower/HLFIR/minloc.f90
index f835cf54b2a73e..ce149ffcfb54fd 100644
--- a/flang/test/Lower/HLFIR/minloc.f90
+++ b/flang/test/Lower/HLFIR/minloc.f90
@@ -341,8 +341,8 @@ end subroutine test_unknown_char_len_result
 ! CHECK-DAG:       %[[C1_7:.*]] = arith.constant 1 : index
 ! CHECK-DAG:       %[[C3_8:.*]] = arith.constant 3 : index
 ! CHECK-DAG:       %[[C3_9:.*]] = arith.constant 3 : index
-! CHECK-DAG:       %[[ARRAY_BOX:.*]] = hlfir.designate %[[ARRAY]]#0 (%[[C1]]:%[[C3_0]]:%[[C1_3]], %[[C1]]:%[[C3_1]]:%[[C1_5]]) substr %[[C1_7]], %[[C3_8]]  shape %[[SHAPE]] typeparams %[[C3_9]]
-! CHECK:           %[[EXPR:.*]] = hlfir.minloc %[[ARRAY_BOX]] {fastmath = #arith.fastmath<contract>} : (!fir.box<!fir.array<3x3x!fir.char<1,3>>>) -> !hlfir.expr<2xi32>
+! CHECK-DAG:       %[[ARRAY_REF:.*]] = hlfir.designate %[[ARRAY]]#0 (%[[C1]]:%[[C3_0]]:%[[C1_3]], %[[C1]]:%[[C3_1]]:%[[C1_5]]) substr %[[C1_7]], %[[C3_8]]  shape %[[SHAPE]] typeparams %[[C3_9]] : (!fir.ref<!fir.array<3x3x!fir.char<1,3>>>, index, index, index, index, index, index, index, index, !fir.shape<2>, index) -> !fir.ref<!fir.array<3x3x!fir.char<1,3>>>
+! CHECK:           %[[EXPR:.*]] = hlfir.minloc %[[ARRAY_REF]] {fastmath = #arith.fastmath<contract>} : (!fir.ref<!fir.array<3x3x!fir.char<1,3>>>) -> !hlfir.expr<2xi32>
 ! CHECK-NEXT:      hlfir.assign %[[EXPR]] to %[[RES]]#0 : !hlfir.expr<2xi32>, !fir.ref<!fir.array<2xi32>>
 ! CHECK-NEXT:      hlfir.destroy %[[EXPR]]
 ! CHECK-NEXT:      return
diff --git a/flang/test/Lower/HLFIR/minval.f90 b/flang/test/Lower/HLFIR/minval.f90
index 01b0ce77e2d30e..2ac9aba850b6f4 100644
--- a/flang/test/Lower/HLFIR/minval.f90
+++ b/flang/test/Lower/HLFIR/minval.f90
@@ -254,8 +254,8 @@ end subroutine test_unknown_char_len_result
 ! CHECK-DAG:       %[[C1_7:.*]] = arith.constant 1 : index
 ! CHECK-DAG:       %[[C3_8:.*]] = arith.constant 3 : index
 ! CHECK-DAG:       %[[C3_9:.*]] = arith.constant 3 : index
-! CHECK-DAG:       %[[ARRAY_BOX:.*]] = hlfir.designate %[[ARRAY]]#0 (%[[C1]]:%[[C3_0]]:%[[C1_3]], %[[C1]]:%[[C3_1]]:%[[C1_5]]) substr %[[C1_7]], %[[C3_8]]  shape %[[SHAPE]] typeparams %[[C3_9]]
-! CHECK:           %[[EXPR:.*]] = hlfir.minval %[[ARRAY_BOX]] {fastmath = #arith.fastmath<contract>} : (!fir.box<!fir.array<3x3x!fir.char<1,3>>>) -> !hlfir.expr<!fir.char<1,3>>
+! CHECK-DAG:       %[[ARRAY_REF:.*]] = hlfir.designate %[[ARRAY]]#0 (%[[C1]]:%[[C3_0]]:%[[C1_3]], %[[C1]]:%[[C3_1]]:%[[C1_5]]) substr %[[C1_7]], %[[C3_8]]  shape %[[SHAPE]] typeparams %[[C3_9]] : (!fir.ref<!fir.array<3x3x!fir.char<1,3>>>, index, index, index, index, index, index, index, index, !fir.shape<2>, index) -> !fir.ref<!fir.array<3x3x!fir.char<1,3>>>
+! CHECK:           %[[EXPR:.*]] = hlfir.minval %[[ARRAY_REF]] {fastmath = #arith.fastmath<contract>} : (!fir.ref<!fir.array<3x3x!fir.char<1,3>>>) -> !hlfir.expr<!fir.char<1,3>>
 ! CHECK-NEXT:      hlfir.assign %[[EXPR]] to %[[RES]]#0 : !hlfir.expr<!fir.char<1,3>>, !fir.ref<!fir.char<1,3>>
 ! CHECK-NEXT:      hlfir.destroy %[[EXPR]]
 ! CHECK-NEXT:      return

llvmbot · 2024-11-13T00:45:24Z

@llvm/pr-subscribers-flang-semantics

Author: Peter Klausler (klausler)

Changes

At present, the compiler doesn't analyze substring references for contiguity. But there are cases where substrings can be known to be contiguous (scalar base, empty substring, or complete substring) or can be known to be discontiguous, and references to the intrinsic function IS_CONTIGUOUS in those cases may appear in constant expressions.

Fixes #115675.

Full diff: https://github.com/llvm/llvm-project/pull/115970.diff

6 Files Affected:

(modified) flang/lib/Evaluate/check-expression.cpp (+33-1)
(modified) flang/test/Evaluate/folding09.f90 (+7)
(modified) flang/test/Lower/HLFIR/maxloc.f90 (+2-2)
(modified) flang/test/Lower/HLFIR/maxval.f90 (+2-2)
(modified) flang/test/Lower/HLFIR/minloc.f90 (+2-2)
(modified) flang/test/Lower/HLFIR/minval.f90 (+2-2)

diff --git a/flang/lib/Evaluate/check-expression.cpp b/flang/lib/Evaluate/check-expression.cpp
index 38794a2d8aacc7..efc08763aa9ad6 100644
--- a/flang/lib/Evaluate/check-expression.cpp
+++ b/flang/lib/Evaluate/check-expression.cpp
@@ -908,7 +908,39 @@ class IsContiguousHelper
   Result operator()(const ComplexPart &x) const {
     return x.complex().Rank() == 0;
   }
-  Result operator()(const Substring &) const { return std::nullopt; }
+  Result operator()(const Substring &x) const {
+    auto base{x.GetBaseObject()};
+    if (x.Rank() == 0) {
+      return true; // scalar substring always contiguous
+    }
+    Result result{(*this)(base)};
+    if (!result || *result) {
+      if (auto lower{ToInt64(Fold(context_, x.lower()))}) {
+        auto upperExpr{x.upper()};
+        auto lenExpr{base.LEN()};
+        if (!upperExpr) {
+          upperExpr = lenExpr;
+        }
+        if (!upperExpr) {
+          return std::nullopt; // could be empty substrings
+        } else if (auto upper{ToInt64(Fold(context_, std::move(*upperExpr)))}) {
+          if (*upper < *lower) {
+            return true; // empty substrings: vacuously contiguous
+          } else if (*lower > 1) {
+            return false; // lower > 1
+          } else if (lenExpr) {
+            if (auto lenVal{ToInt64(Fold(context_, std::move(*lenExpr)))};
+                lenVal && *lenVal != *upper) {
+              return false; // upper < length
+            }
+          }
+        }
+      } else {
+        return std::nullopt; // lower bound not known
+      }
+    }
+    return result;
+  }
 
   Result operator()(const ProcedureRef &x) const {
     if (auto chars{characteristics::Procedure::Characterize(
diff --git a/flang/test/Evaluate/folding09.f90 b/flang/test/Evaluate/folding09.f90
index 863b5e873a1e5b..b9c00710a086ca 100644
--- a/flang/test/Evaluate/folding09.f90
+++ b/flang/test/Evaluate/folding09.f90
@@ -21,6 +21,7 @@ subroutine test(arr1, arr2, arr3, mat, alloc)
     real, intent(in), contiguous :: arr3(:)
     real, allocatable :: alloc(:)
     real :: scalar
+    character(5) charr(5)
     integer(kind=merge(1,-1,       is_contiguous(0)))               t01
     integer(kind=merge(1,-1,       is_contiguous(scalar)))          t02
     integer(kind=merge(1,-1,       is_contiguous(scalar + scalar))) t03
@@ -35,6 +36,12 @@ subroutine test(arr1, arr2, arr3, mat, alloc)
     integer(kind=merge(1,-1, .not. is_contiguous(arr3(1:10:2))))    t12
     integer(kind=merge(1,-1,       is_contiguous(f())))             t13
     integer(kind=merge(1,-1,       is_contiguous(alloc)))           t14
+    integer(kind=merge(1,-1,       is_contiguous(charr(:)(:))))     t15
+    integer(kind=merge(1,-1,       is_contiguous(charr(1)(2:3))))   t16
+    integer(kind=merge(1,-1,       is_contiguous(charr(:)(1:))))    t17
+    integer(kind=merge(1,-1,       is_contiguous(charr(:)(3:2))))   t18
+    integer(kind=merge(1,-1,       is_contiguous(charr(:)(1:5))))   t19
+    integer(kind=merge(1,-1, .not. is_contiguous(charr(:)(1:4))))   t20
     associate (x => arr2)
       block
         integer(kind=merge(1,-1,is_contiguous(x))) n
diff --git a/flang/test/Lower/HLFIR/maxloc.f90 b/flang/test/Lower/HLFIR/maxloc.f90
index 166a1b9db1724e..539affad2d7df1 100644
--- a/flang/test/Lower/HLFIR/maxloc.f90
+++ b/flang/test/Lower/HLFIR/maxloc.f90
@@ -341,8 +341,8 @@ end subroutine test_unknown_char_len_result
 ! CHECK-DAG:       %[[C1_7:.*]] = arith.constant 1 : index
 ! CHECK-DAG:       %[[C3_8:.*]] = arith.constant 3 : index
 ! CHECK-DAG:       %[[C3_9:.*]] = arith.constant 3 : index
-! CHECK-DAG:       %[[ARRAY_BOX:.*]] = hlfir.designate %[[ARRAY]]#0 (%[[C1]]:%[[C3_0]]:%[[C1_3]], %[[C1]]:%[[C3_1]]:%[[C1_5]]) substr %[[C1_7]], %[[C3_8]]  shape %[[SHAPE]] typeparams %[[C3_9]]
-! CHECK:           %[[EXPR:.*]] = hlfir.maxloc %[[ARRAY_BOX]] {fastmath = #arith.fastmath<contract>} : (!fir.box<!fir.array<3x3x!fir.char<1,3>>>) -> !hlfir.expr<2xi32>
+! CHECK-DAG:       %[[ARRAY_REF:.*]] = hlfir.designate %[[ARRAY]]#0 (%[[C1]]:%[[C3_0]]:%[[C1_3]], %[[C1]]:%[[C3_1]]:%[[C1_5]]) substr %[[C1_7]], %[[C3_8]]  shape %[[SHAPE]] typeparams %[[C3_9]] : (!fir.ref<!fir.array<3x3x!fir.char<1,3>>>, index, index, index, index, index, index, index, index, !fir.shape<2>, index) -> !fir.ref<!fir.array<3x3x!fir.char<1,3>>>
+! CHECK:           %[[EXPR:.*]] = hlfir.maxloc %[[ARRAY_REF]] {fastmath = #arith.fastmath<contract>} : (!fir.ref<!fir.array<3x3x!fir.char<1,3>>>) -> !hlfir.expr<2xi32>
 ! CHECK-NEXT:      hlfir.assign %[[EXPR]] to %[[RES]]#0 : !hlfir.expr<2xi32>, !fir.ref<!fir.array<2xi32>>
 ! CHECK-NEXT:      hlfir.destroy %[[EXPR]]
 ! CHECK-NEXT:      return
diff --git a/flang/test/Lower/HLFIR/maxval.f90 b/flang/test/Lower/HLFIR/maxval.f90
index 5adad286a77d28..32e1a80417a27b 100644
--- a/flang/test/Lower/HLFIR/maxval.f90
+++ b/flang/test/Lower/HLFIR/maxval.f90
@@ -254,8 +254,8 @@ end subroutine test_unknown_char_len_result
 ! CHECK-DAG:       %[[C1_7:.*]] = arith.constant 1 : index
 ! CHECK-DAG:       %[[C3_8:.*]] = arith.constant 3 : index
 ! CHECK-DAG:       %[[C3_9:.*]] = arith.constant 3 : index
-! CHECK-DAG:       %[[ARRAY_BOX:.*]] = hlfir.designate %[[ARRAY]]#0 (%[[C1]]:%[[C3_0]]:%[[C1_3]], %[[C1]]:%[[C3_1]]:%[[C1_5]]) substr %[[C1_7]], %[[C3_8]]  shape %[[SHAPE]] typeparams %[[C3_9]]
-! CHECK:           %[[EXPR:.*]] = hlfir.maxval %[[ARRAY_BOX]] {fastmath = #arith.fastmath<contract>} : (!fir.box<!fir.array<3x3x!fir.char<1,3>>>) -> !hlfir.expr<!fir.char<1,3>>
+! CHECK-DAG:       %[[ARRAY_REF:.*]] = hlfir.designate %[[ARRAY]]#0 (%[[C1]]:%[[C3_0]]:%[[C1_3]], %[[C1]]:%[[C3_1]]:%[[C1_5]]) substr %[[C1_7]], %[[C3_8]]  shape %[[SHAPE]] typeparams %[[C3_9]] : (!fir.ref<!fir.array<3x3x!fir.char<1,3>>>, index, index, index, index, index, index, index, index, !fir.shape<2>, index) -> !fir.ref<!fir.array<3x3x!fir.char<1,3>>>
+! CHECK:           %[[EXPR:.*]] = hlfir.maxval %[[ARRAY_REF]] {fastmath = #arith.fastmath<contract>} : (!fir.ref<!fir.array<3x3x!fir.char<1,3>>>) -> !hlfir.expr<!fir.char<1,3>>
 ! CHECK-NEXT:      hlfir.assign %[[EXPR]] to %[[RES]]#0 : !hlfir.expr<!fir.char<1,3>>, !fir.ref<!fir.char<1,3>>
 ! CHECK-NEXT:      hlfir.destroy %[[EXPR]]
 ! CHECK-NEXT:      return
diff --git a/flang/test/Lower/HLFIR/minloc.f90 b/flang/test/Lower/HLFIR/minloc.f90
index f835cf54b2a73e..ce149ffcfb54fd 100644
--- a/flang/test/Lower/HLFIR/minloc.f90
+++ b/flang/test/Lower/HLFIR/minloc.f90
@@ -341,8 +341,8 @@ end subroutine test_unknown_char_len_result
 ! CHECK-DAG:       %[[C1_7:.*]] = arith.constant 1 : index
 ! CHECK-DAG:       %[[C3_8:.*]] = arith.constant 3 : index
 ! CHECK-DAG:       %[[C3_9:.*]] = arith.constant 3 : index
-! CHECK-DAG:       %[[ARRAY_BOX:.*]] = hlfir.designate %[[ARRAY]]#0 (%[[C1]]:%[[C3_0]]:%[[C1_3]], %[[C1]]:%[[C3_1]]:%[[C1_5]]) substr %[[C1_7]], %[[C3_8]]  shape %[[SHAPE]] typeparams %[[C3_9]]
-! CHECK:           %[[EXPR:.*]] = hlfir.minloc %[[ARRAY_BOX]] {fastmath = #arith.fastmath<contract>} : (!fir.box<!fir.array<3x3x!fir.char<1,3>>>) -> !hlfir.expr<2xi32>
+! CHECK-DAG:       %[[ARRAY_REF:.*]] = hlfir.designate %[[ARRAY]]#0 (%[[C1]]:%[[C3_0]]:%[[C1_3]], %[[C1]]:%[[C3_1]]:%[[C1_5]]) substr %[[C1_7]], %[[C3_8]]  shape %[[SHAPE]] typeparams %[[C3_9]] : (!fir.ref<!fir.array<3x3x!fir.char<1,3>>>, index, index, index, index, index, index, index, index, !fir.shape<2>, index) -> !fir.ref<!fir.array<3x3x!fir.char<1,3>>>
+! CHECK:           %[[EXPR:.*]] = hlfir.minloc %[[ARRAY_REF]] {fastmath = #arith.fastmath<contract>} : (!fir.ref<!fir.array<3x3x!fir.char<1,3>>>) -> !hlfir.expr<2xi32>
 ! CHECK-NEXT:      hlfir.assign %[[EXPR]] to %[[RES]]#0 : !hlfir.expr<2xi32>, !fir.ref<!fir.array<2xi32>>
 ! CHECK-NEXT:      hlfir.destroy %[[EXPR]]
 ! CHECK-NEXT:      return
diff --git a/flang/test/Lower/HLFIR/minval.f90 b/flang/test/Lower/HLFIR/minval.f90
index 01b0ce77e2d30e..2ac9aba850b6f4 100644
--- a/flang/test/Lower/HLFIR/minval.f90
+++ b/flang/test/Lower/HLFIR/minval.f90
@@ -254,8 +254,8 @@ end subroutine test_unknown_char_len_result
 ! CHECK-DAG:       %[[C1_7:.*]] = arith.constant 1 : index
 ! CHECK-DAG:       %[[C3_8:.*]] = arith.constant 3 : index
 ! CHECK-DAG:       %[[C3_9:.*]] = arith.constant 3 : index
-! CHECK-DAG:       %[[ARRAY_BOX:.*]] = hlfir.designate %[[ARRAY]]#0 (%[[C1]]:%[[C3_0]]:%[[C1_3]], %[[C1]]:%[[C3_1]]:%[[C1_5]]) substr %[[C1_7]], %[[C3_8]]  shape %[[SHAPE]] typeparams %[[C3_9]]
-! CHECK:           %[[EXPR:.*]] = hlfir.minval %[[ARRAY_BOX]] {fastmath = #arith.fastmath<contract>} : (!fir.box<!fir.array<3x3x!fir.char<1,3>>>) -> !hlfir.expr<!fir.char<1,3>>
+! CHECK-DAG:       %[[ARRAY_REF:.*]] = hlfir.designate %[[ARRAY]]#0 (%[[C1]]:%[[C3_0]]:%[[C1_3]], %[[C1]]:%[[C3_1]]:%[[C1_5]]) substr %[[C1_7]], %[[C3_8]]  shape %[[SHAPE]] typeparams %[[C3_9]] : (!fir.ref<!fir.array<3x3x!fir.char<1,3>>>, index, index, index, index, index, index, index, index, !fir.shape<2>, index) -> !fir.ref<!fir.array<3x3x!fir.char<1,3>>>
+! CHECK:           %[[EXPR:.*]] = hlfir.minval %[[ARRAY_REF]] {fastmath = #arith.fastmath<contract>} : (!fir.ref<!fir.array<3x3x!fir.char<1,3>>>) -> !hlfir.expr<!fir.char<1,3>>
 ! CHECK-NEXT:      hlfir.assign %[[EXPR]] to %[[RES]]#0 : !hlfir.expr<!fir.char<1,3>>, !fir.ref<!fir.char<1,3>>
 ! CHECK-NEXT:      hlfir.destroy %[[EXPR]]
 ! CHECK-NEXT:      return

flang/lib/Evaluate/check-expression.cpp

klausler · 2024-11-13T19:20:59Z

Thanks for looking into that code; I’ve reworked it. From: jeanPerier ***@***.***> Date: Wednesday, November 13, 2024 at 02:50 To: llvm/llvm-project ***@***.***> Cc: Peter Klausler ***@***.***>, Author ***@***.***> Subject: Re: [llvm/llvm-project] [flang] Better IS_CONTIGUOUS folding for substrings (PR #115970) @jeanPerier commented on this pull request.

________________________________ In flang/lib/Evaluate/check-expression.cpp<#115970 (comment)>:

+ upperExpr = lenExpr;

+ } + if (!upperExpr) { + return std::nullopt; // could be empty substrings + } else if (auto upper{ToInt64(Fold(context_, std::move(*upperExpr)))}) { + if (*upper < *lower) { + return true; // empty substrings: vacuously contiguous + } else if (*lower > 1) { + return false; // lower > 1 + } else if (lenExpr) { + if (auto lenVal{ToInt64(Fold(context_, std::move(*lenExpr)))}; + lenVal && *lenVal != *upper) { + return false; // upper < length + } + } + } This seems a quite broad fallback, I think the code is incorrectly falling back into it for things like: subroutine test2(charr, n) character(10) :: charr(5) integer :: n print *, is_contiguous(charr(:)(1:n)) end subroutine This folds would now fold to true, while this is cannot be folded (and will be false if n is not 10 or smaller than 1 at runtime). It seems the code can only fall back to "return result" when it can be determined that the lower bound is one, and the upper bound is absent or equal to the length.

________________________________ In flang/lib/Evaluate/check-expression.cpp<#115970 (comment)>:

+ return true; // empty substrings: vacuously contiguous

+ } else if (*lower > 1) { + return false; // lower > 1 + } else if (lenExpr) { + if (auto lenVal{ToInt64(Fold(context_, std::move(*lenExpr)))}; + lenVal && *lenVal != *upper) { + return false; // upper < length + } + } + } + } else { + return std::nullopt; // lower bound not known + } + } + return result; + } For some reason I do not explain to myself it seems your patch is incorrectly causing the folowing is_contiguous to fold to true while the substring is not empty and the base is not contiguous: subroutine test(charr) character(10) :: charr(5) print *, is_contiguous(charr(1:5:2)(1:10)) end subroutine My comment about incorrectly falling back into the "return is_contigous(base)" cannot explain that since the base is not contiguous... — Reply to this email directly, view it on GitHub<#115970 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AIRI5XISOSE6PNHMHPOM2LT2AMVH7AVCNFSM6AAAAABRVIVFA6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDIMZSGU2DMOBTGQ>. You are receiving this because you authored the thread.Message ID: ***@***.***>

jeanPerier · 2024-11-14T09:35:58Z

flang/lib/Evaluate/check-expression.cpp

+        upperIsLen = len && *upper >= *len;
+      }
+    } else {
+      upperIsLen = true; // substring(n:)


It seems that some previous folding step may already have expended the absent upper into the length, blurring the information here for cases where the length is not a compile time constant:

subroutine test(charr) character(*) :: charr(5) print *, is_contiguous(charr(:)(1:)) end subroutine

Folds to is_contiguous(charr(::1_8)(1_8:int(charr%len,kind=8))) while it could be folded to true.

However, folding this may not be required by the standard, so this may be fine.

I do not find the standard very clear regarding the appearance of IS_CONTIGUOUS in constant expressions. As I understand it, it falls into the 10.1.12 (6) case since IS_CONTIGUOUS is an inquiry function. I am not sure how to read "a variable whose properties inquired about are not assumed, deferred, or defined by an expression that is not a constant expression" when the "property" is the contiguity.

jeanPerier

Question about one case, LGTM otherwise. Thanks for the update.

github-actions · 2024-11-14T20:28:45Z

✅ With the latest revision this PR passed the C/C++ code formatter.

At present, the compiler doesn't analyze substring references for contiguity. But there are cases where substrings can be known to be contiguous (scalar base, empty substring, or complete substring) or can be known to be discontiguous, and references to the intrinsic function IS_CONTIGUOUS in those cases may appear in constant expressions. Fixes llvm#115675.

klausler requested a review from jeanPerier November 13, 2024 00:44

llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:semantics labels Nov 13, 2024

jeanPerier reviewed Nov 13, 2024

View reviewed changes

flang/lib/Evaluate/check-expression.cpp Show resolved Hide resolved

flang/lib/Evaluate/check-expression.cpp Show resolved Hide resolved

klausler force-pushed the bug115675 branch from 4490f2d to 38e1b63 Compare November 13, 2024 19:20

jeanPerier reviewed Nov 14, 2024

View reviewed changes

jeanPerier approved these changes Nov 14, 2024

View reviewed changes

klausler force-pushed the bug115675 branch from 38e1b63 to 9870674 Compare November 14, 2024 20:25

klausler force-pushed the bug115675 branch from 9870674 to 7e77e64 Compare November 14, 2024 20:37

klausler merged commit 17daa84 into llvm:main Nov 14, 2024
8 checks passed

klausler deleted the bug115675 branch November 14, 2024 22:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[flang] Better IS_CONTIGUOUS folding for substrings #115970

[flang] Better IS_CONTIGUOUS folding for substrings #115970

Uh oh!

klausler commented Nov 13, 2024

Uh oh!

llvmbot commented Nov 13, 2024

Uh oh!

llvmbot commented Nov 13, 2024

Uh oh!

Uh oh!

Uh oh!

klausler commented Nov 13, 2024 via email

Uh oh!

jeanPerier Nov 14, 2024

Uh oh!

jeanPerier left a comment

Uh oh!

github-actions bot commented Nov 14, 2024 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

[flang] Better IS_CONTIGUOUS folding for substrings #115970

[flang] Better IS_CONTIGUOUS folding for substrings #115970

Uh oh!

Conversation

klausler commented Nov 13, 2024

Uh oh!

llvmbot commented Nov 13, 2024

Uh oh!

llvmbot commented Nov 13, 2024

Uh oh!

Uh oh!

Uh oh!

klausler commented Nov 13, 2024 via email

Uh oh!

jeanPerier Nov 14, 2024

Choose a reason for hiding this comment

Uh oh!

jeanPerier left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Nov 14, 2024 •

edited

Loading