[flang-commits] [flang] 9d08498 - [flang] Clarify CheckReductionDIM()

[Peter Klausler via flang-commits]

Author: Peter Klausler
Date: 2022-08-09T05:51:45-07:00
New Revision: 9d084982a38526fb8ba15867ecceec3e9b82a6f8

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

LOG: [flang] Clarify CheckReductionDIM()

This utility routine in constant folding should return false when
a DIM= actual argument to a reduction intrinsic function has a
value that prevents folding, and true when folding can proceed.
The implementation was returning true in cases where a DIM=
argument was present but not constant.

Clarify the code and add commentary: a true result means that
there is no DIM= actual argument present, or that a DIM= argument
exists, is constant, and has a value that is in range.

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

Added: 
    

Modified: 
    flang/lib/Evaluate/fold-reduction.cpp
    flang/lib/Evaluate/fold-reduction.h

Removed: 
    


################################################################################
diff  --git a/flang/lib/Evaluate/fold-reduction.cpp b/flang/lib/Evaluate/fold-reduction.cpp
index 56f4b70b4f667..c5f5e1996b6b1 100644
--- a/flang/lib/Evaluate/fold-reduction.cpp
+++ b/flang/lib/Evaluate/fold-reduction.cpp
@@ -11,23 +11,26 @@
 namespace Fortran::evaluate {
 bool CheckReductionDIM(std::optional<int> &dim, FoldingContext &context,
     ActualArguments &arg, std::optional<int> dimIndex, int rank) {
-  if (dimIndex && static_cast<std::size_t>(*dimIndex) < arg.size()) {
-    if (auto *dimConst{
-            Folder<SubscriptInteger>{context}.Folding(arg[*dimIndex])}) {
-      if (auto dimScalar{dimConst->GetScalarValue()}) {
-        auto dimVal{dimScalar->ToInt64()};
-        if (dimVal >= 1 && dimVal <= rank) {
-          dim = dimVal;
-        } else {
-          context.messages().Say(
-              "DIM=%jd is not valid for an array of rank %d"_err_en_US,
-              static_cast<std::intmax_t>(dimVal), rank);
-          return false;
-        }
+  if (!dimIndex || static_cast<std::size_t>(*dimIndex) >= arg.size() ||
+      !arg[*dimIndex]) {
+    dim.reset();
+    return true; // no DIM= argument
+  }
+  if (auto *dimConst{
+          Folder<SubscriptInteger>{context}.Folding(arg[*dimIndex])}) {
+    if (auto dimScalar{dimConst->GetScalarValue()}) {
+      auto dimVal{dimScalar->ToInt64()};
+      if (dimVal >= 1 && dimVal <= rank) {
+        dim = dimVal;
+        return true; // DIM= exists and is a valid constant
+      } else {
+        context.messages().Say(
+            "DIM=%jd is not valid for an array of rank %d"_err_en_US,
+            static_cast<std::intmax_t>(dimVal), rank);
       }
     }
   }
-  return true;
+  return false; // DIM= bad or not scalar constant
 }
 
 Constant<LogicalResult> *GetReductionMASK(

diff  --git a/flang/lib/Evaluate/fold-reduction.h b/flang/lib/Evaluate/fold-reduction.h
index d8a55df7013f6..8043212820f59 100644
--- a/flang/lib/Evaluate/fold-reduction.h
+++ b/flang/lib/Evaluate/fold-reduction.h
@@ -15,7 +15,10 @@
 
 namespace Fortran::evaluate {
 
-// Fold and validate a DIM= argument.  Returns false on error.
+// Fold and validate a DIM= argument.  Returns true (with &dim empty)
+// when DIM= is not present or (with &dim set) when DIM= is present, constant,
+// and valid.  Returns false, possibly with an error message, when
+// DIM= is present but either not constant or not valid.
 bool CheckReductionDIM(std::optional<int> &dim, FoldingContext &,
     ActualArguments &, std::optional<int> dimIndex, int rank);