[flang-commits] [PATCH] D131101: [flang] Clarify CheckReductionDIM()

[Peter Klausler via Phabricator via flang-commits]

klausler created this revision.
klausler added a reviewer: vdonaldson.
klausler added a project: Flang.
Herald added a subscriber: jdoerfert.
Herald added a project: All.
klausler requested review of this revision.

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.


https://reviews.llvm.org/D131101

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


Index: flang/lib/Evaluate/fold-reduction.h
===================================================================
--- flang/lib/Evaluate/fold-reduction.h
+++ 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);
 
Index: flang/lib/Evaluate/fold-reduction.cpp
===================================================================
--- flang/lib/Evaluate/fold-reduction.cpp
+++ 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(


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D131101.449751.patch
Type: text/x-patch
Size: 2467 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/flang-commits/attachments/20220803/db20d3bd/attachment.bin>