[flang] Merge pull request flang-compiler/f18#544 from flang-compiler/jpr-reshape-only-folding

RESHAPE without shared runtime/front-end descriptor API

Original-commit: flang-compiler/f18@24856b8238
Reviewed-on: https://github.com/flang-compiler/f18/pull/544

Due to a conflicting rebase during the linearizing of flang-compiler/f18, this commit squashes a number of other commits:

flang-compiler/f18@2ec1d85d39 Implement RESHAPE folding on Constant<T> only
flang-compiler/f18@5394630539 Enable RESHAPE folding tests
flang-compiler/f18@83b2b86253 Answer review comment + Add a common path for intrsinic function folding before specializing the folding per type. + Make reshape folding return an "invalid" intrinsic after errors are met so that warnings do not get re-generated. + Misc style changes
flang-compiler/f18@2e5c29f280 add missing file to previous commit...
flang-compiler/f18@9bd5ad9875 Document issue #518 workaround
flang-compiler/f18@a4f8f51831 Go back to clang-format version 7.01
flang-compiler/f18@e871e58a52 answer comment regarding naming and interface
flang-compiler/f18@145c7c1ece Merge branch 'master' into jpr-reshape-only-folding Too many logical conflicts to simply rebase.

flang/documentation/C++style.md

@@ -13,6 +13,8 @@ is clear on usage, follow it.
   follow it.  [Google's](https://google.github.io/styleguide/cppguide.html)
   is pretty good and comes with lots of justifications for its rules.
 * Reasonable exceptions to these guidelines can be made.
+* Be aware of some workarounds for known issues in older C++ compilers that should
+  still be able to compile f18. They are listed at the end of this document.
 ## In particular:
 ### Files
 1. File names should use dashes, not underscores.  C++ sources have the
@@ -227,6 +229,8 @@ build time; don't solve build time problems by writing programs that
 produce source code when macros and templates suffice; don't write macros
 when templates suffice.  Templates are statically typed, checked by the
 compiler, and are (or should be) visible to debuggers.
+
+
 ### Exceptions to these guidelines
 Reasonable exceptions will be allowed; these guidelines cannot anticipate
 all situations.
@@ -237,3 +241,39 @@ in a way that leads to weirdly capitalized abbreviations in names
 like `Http`.
 Consistency is one of many aspects in the pursuit of clarity,
 but not an end in itself.
+
+## C++ compiler bug workarounds
+Below is a list of workarounds for C++ compiler bugs met with f18 that, even
+if the bugs are fixed in latest C++ compiler versions, need to be applied so
+that all desired tool-chains can compile f18.
+### Explicitly move noncopyable local variable into optional results
+
+The following code is legal C++ but fails to compile with the
+default Ubuntu 18.04 g++ compiler (7.4.0-1ubuntu1~18.0.4.1):
+
+```
+class CantBeCopied {
+ public:
+ CantBeCopied(const CantBeCopied&) = delete;
+ CantBeCopied(CantBeCopied&&) = default;
+ CantBeCopied() {}
+};
+std::optional<CantBeCopied> fooNOK() {
+  CantBeCopied result;
+  return result; // Legal C++, but does not compile with Ubuntu 18.04 default g++
+}
+std::optional<CantBeCopied> fooOK() {
+  CantBeCopied result;
+  return {std::move(result)}; // Compiles OK everywhere
+}
+```
+The underlying bug is actually not specific to `std::optional` but this is the most common
+case in f18 where the issue may occur. The actual bug can be reproduced with any class `B`
+that has a perfect forwarding constructor taking `CantBeCopied` as argument:
+`template<typename CantBeCopied> B(CantBeCopied&& x) x_{std::forward<CantBeCopied>(x)} {}`.
+In such scenarios, Ubuntu 18.04 g++ fails to instantiate the move constructor
+and to construct the returned value as it should, instead it complains about a
+missing copy constructor.
+
+Local result variables do not need to and should not be explicitly moved into optionals
+if they have a copy constructor.

flang/lib/common/Fortran.h

@@ -53,5 +53,8 @@ ENUM_CLASS(IoSpecKind, Access, Action, Advance, Asynchronous, Blank, Decimal,
     Convert,  // nonstandard
     Dispose,  // nonstandard
 )
+
+// Fortran arrays may have up to 15 dimensions (See Fortran 2018 section 5.4.6).
+static constexpr int maxRank{15};
 }
 #endif  // FORTRAN_COMMON_FORTRAN_H_

flang/lib/evaluate/constant.cc

@@ -61,22 +61,53 @@ ConstantSubscript ConstantBounds::SubscriptsToOffset(
   return offset;
 }
 
-bool ConstantBounds::IncrementSubscripts(ConstantSubscripts &indices) const {
+bool ConstantBounds::IncrementSubscripts(
+    ConstantSubscripts &indices, const std::vector<int> *dimOrder) const {
   int rank{GetRank(shape_)};
   CHECK(GetRank(indices) == rank);
+  CHECK(!dimOrder || static_cast<int>(dimOrder->size()) == rank);
   for (int j{0}; j < rank; ++j) {
-    auto lb{lbounds_[j]};
-    CHECK(indices[j] >= lb);
-    if (++indices[j] < lb + shape_[j]) {
+    ConstantSubscript k{dimOrder ? (*dimOrder)[j] : j};
+    auto lb{lbounds_[k]};
+    CHECK(indices[k] >= lb);
+    if (++indices[k] < lb + shape_[k]) {
       return true;
     } else {
-      CHECK(indices[j] == lb + shape_[j]);
-      indices[j] = lb;
+      CHECK(indices[k] == lb + shape_[k]);
+      indices[k] = lb;
     }
   }
   return false;  // all done
 }
 
+std::optional<std::vector<int>> ValidateDimensionOrder(
+    int rank, const std::vector<int> &order) {
+  std::vector<int> dimOrder(rank);
+  if (static_cast<int>(order.size()) == rank) {
+    std::bitset<common::maxRank> seenDimensions;
+    for (int j{0}; j < rank; ++j) {
+      int dim{order[j]};
+      if (dim < 1 || dim > rank || seenDimensions.test(dim - 1)) {
+        return std::nullopt;
+      }
+      dimOrder[dim - 1] = j;
+      seenDimensions.set(dim - 1);
+    }
+    return dimOrder;
+  } else {
+    return std::nullopt;
+  }
+}
+
+bool IsValidShape(const ConstantSubscripts &shape) {
+  for (ConstantSubscript extent : shape) {
+    if (extent < 0) {
+      return false;
+    }
+  }
+  return shape.size() <= common::maxRank;
+}
+
 template<typename RESULT, typename ELEMENT>
 ConstantBase<RESULT, ELEMENT>::ConstantBase(
     std::vector<Element> &&x, ConstantSubscripts &&sh, Result res)
@@ -108,6 +139,22 @@ auto ConstantBase<RESULT, ELEMENT>::Reshape(
   return elements;
 }
 
+template<typename RESULT, typename ELEMENT>
+std::size_t ConstantBase<RESULT, ELEMENT>::CopyFrom(
+    const ConstantBase<RESULT, ELEMENT> &source, std::size_t count,
+    ConstantSubscripts &resultSubscripts, const std::vector<int> *dimOrder) {
+  std::size_t copied{0};
+  ConstantSubscripts sourceSubscripts{source.lbounds()};
+  while (copied < count) {
+    values_.at(SubscriptsToOffset(resultSubscripts)) =
+        source.values_.at(source.SubscriptsToOffset(sourceSubscripts));
+    copied++;
+    source.IncrementSubscripts(sourceSubscripts);
+    IncrementSubscripts(resultSubscripts, dimOrder);
+  }
+  return copied;
+}
+
 template<typename T>
 auto Constant<T>::At(const ConstantSubscripts &index) const -> Element {
   return Base::values_.at(Base::SubscriptsToOffset(index));
@@ -118,6 +165,12 @@ auto Constant<T>::Reshape(ConstantSubscripts &&dims) const -> Constant {
   return {Base::Reshape(dims), std::move(dims)};
 }
 
+template<typename T>
+std::size_t Constant<T>::CopyFrom(const Constant<T> &source, std::size_t count,
+    ConstantSubscripts &resultSubscripts, const std::vector<int> *dimOrder) {
+  return Base::CopyFrom(source, count, resultSubscripts, dimOrder);
+}
+
 // Constant<Type<TypeCategory::Character, KIND> specializations
 template<int KIND>
 Constant<Type<TypeCategory::Character, KIND>>::Constant(
@@ -190,6 +243,27 @@ auto Constant<Type<TypeCategory::Character, KIND>>::Reshape(
   return {length_, std::move(elements), std::move(dims)};
 }
 
+template<int KIND>
+std::size_t Constant<Type<TypeCategory::Character, KIND>>::CopyFrom(
+    const Constant<Type<TypeCategory::Character, KIND>> &source,
+    std::size_t count, ConstantSubscripts &resultSubscripts,
+    const std::vector<int> *dimOrder) {
+  CHECK(length_ == source.length_);
+  std::size_t copied{0};
+  std::size_t elementBytes{length_ * sizeof(decltype(values_[0]))};
+  ConstantSubscripts sourceSubscripts{source.lbounds()};
+  while (copied < count) {
+    auto *dest{&values_.at(SubscriptsToOffset(resultSubscripts) * length_)};
+    const auto *src{&source.values_.at(
+        source.SubscriptsToOffset(sourceSubscripts) * length_)};
+    std::memcpy(dest, src, elementBytes);
+    copied++;
+    source.IncrementSubscripts(sourceSubscripts);
+    IncrementSubscripts(resultSubscripts, dimOrder);
+  }
+  return copied;
+}
+
 // Constant<SomeDerived> specialization
 Constant<SomeDerived>::Constant(const StructureConstructor &x)
   : Base{x.values(), Result{x.derivedTypeSpec()}} {}
@@ -233,5 +307,11 @@ auto Constant<SomeDerived>::Reshape(ConstantSubscripts &&dims) const
   return {result().derivedTypeSpec(), Base::Reshape(dims), std::move(dims)};
 }
 
+std::size_t Constant<SomeDerived>::CopyFrom(const Constant<SomeDerived> &source,
+    std::size_t count, ConstantSubscripts &resultSubscripts,
+    const std::vector<int> *dimOrder) {
+  return Base::CopyFrom(source, count, resultSubscripts, dimOrder);
+}
+
 INSTANTIATE_CONSTANT_TEMPLATES
 }

flang/lib/evaluate/constant.h

@@ -43,6 +43,14 @@ inline int GetRank(const ConstantSubscripts &s) {
 
 std::size_t TotalElementCount(const ConstantSubscripts &);
 
+// Validate dimension re-ordering like ORDER in RESHAPE.
+// On success, return a vector that can be used as dimOrder in
+// ConstantBound::IncrementSubscripts.
+std::optional<std::vector<int>> ValidateDimensionOrder(
+    int rank, const std::vector<int> &order);
+
+bool IsValidShape(const ConstantSubscripts &);
+
 class ConstantBounds {
 public:
   ConstantBounds() = default;
@@ -55,9 +63,13 @@ public:
   int Rank() const { return GetRank(shape_); }
   Constant<SubscriptInteger> SHAPE() const;
 
-  // Increments a vector of subscripts in Fortran array order (first dimension
-  // varying most quickly).  Returns false when last element was visited.
-  bool IncrementSubscripts(ConstantSubscripts &) const;
+  // If no optional dimension order argument is passed, increments a vector of
+  // subscripts in Fortran array order (first dimension varying most quickly).
+  // Otherwise, increments the vector of subscripts according to the given
+  // dimension order (dimension dimOrder[0] varying most quickly. Dimensions
+  // indexing is zero based here.) Returns false when last element was visited.
+  bool IncrementSubscripts(
+      ConstantSubscripts &, const std::vector<int> *dimOrder = nullptr) const;
 
 protected:
   ConstantSubscript SubscriptsToOffset(const ConstantSubscripts &) const;
@@ -100,6 +112,8 @@ public:
 
 protected:
   std::vector<Element> Reshape(const ConstantSubscripts &) const;
+  std::size_t CopyFrom(const ConstantBase &source, std::size_t count,
+      ConstantSubscripts &resultSubscripts, const std::vector<int> *dimOrder);
 
   Result result_;
   std::vector<Element> values_;
@@ -126,6 +140,8 @@ public:
   Element At(const ConstantSubscripts &) const;
 
   Constant Reshape(ConstantSubscripts &&) const;
+  std::size_t CopyFrom(const Constant &source, std::size_t count,
+      ConstantSubscripts &resultSubscripts, const std::vector<int> *dimOrder);
 };
 
 template<int KIND>
@@ -164,6 +180,8 @@ public:
   static constexpr DynamicType GetType() {
     return {TypeCategory::Character, KIND};
   }
+  std::size_t CopyFrom(const Constant &source, std::size_t count,
+      ConstantSubscripts &resultSubscripts, const std::vector<int> *dimOrder);
 
 private:
   Scalar<Result> values_;  // one contiguous string
@@ -196,6 +214,8 @@ public:
   StructureConstructor At(const ConstantSubscripts &) const;
 
   Constant Reshape(ConstantSubscripts &&) const;
+  std::size_t CopyFrom(const Constant &source, std::size_t count,
+      ConstantSubscripts &resultSubscripts, const std::vector<int> *dimOrder);
 };
 
 FOR_EACH_LENGTHLESS_INTRINSIC_KIND(extern template class ConstantBase, )

flang/test/evaluate/CMakeLists.txt

@@ -127,6 +127,7 @@ set(FOLDING_TESTS
   folding03.f90
   folding04.f90
   folding05.f90
+  folding06.f90
 )
 
 

flang/test/evaluate/folding06.f90

@@ -0,0 +1,68 @@
+! Copyright (c) 2019, NVIDIA CORPORATION.  All rights reserved.
+!
+! Licensed under the Apache License, Version 2.0 (the "License");
+! you may not use this file except in compliance with the License.
+! You may obtain a copy of the License at
+!
+!     http://www.apache.org/licenses/LICENSE-2.0
+!
+! Unless required by applicable law or agreed to in writing, software
+! distributed under the License is distributed on an "AS IS" BASIS,
+! WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+! implied.
+! See the License for the specific language governing permissions and
+! limitations under the License.
+
+! Test transformational intrinsic function folding
+
+module m
+
+  type A
+    real(4) x
+    integer(8) i
+  end type
+
+  integer(8), parameter :: new_shape(:) = [2, 4]
+  integer(4), parameter :: order(2) = [2, 1]
+
+
+  ! Testing integers (similar to real and complex)
+  integer(4), parameter :: int_source(:) = [1, 2, 3, 4, 5, 6]
+  integer(4), parameter :: int_pad(2) = [7, 8]
+  integer(4), parameter :: int_expected_result(:, :) = reshape([1, 5, 2, 6, 3, 7, 4, 8], new_shape)
+  integer(4), parameter :: int_result(:, :) = reshape(int_source, new_shape, int_pad, order)
+  logical, parameter :: test_reshape_integer_1 = all(int_expected_result == int_result)
+  logical, parameter :: test_reshape_integer_2 = all(shape(int_result, 8).EQ.new_shape)
+
+
+  ! Testing characters
+  character(kind=1, len=3), parameter ::char_source(:) = ["abc", "def", "ghi", "jkl", "mno", "pqr"]
+  character(kind=1,len=3), parameter :: char_pad(2) = ["stu", "vxy"]
+
+  character(kind=1, len=3), parameter :: char_expected_result(:, :) = &
+    reshape(["abc", "mno", "def", "pqr", "ghi", "stu", "jkl", "vxy"], new_shape)
+
+  character(kind=1, len=3), parameter :: char_result(:, :) = &
+    reshape(char_source, new_shape, char_pad, order)
+
+  logical, parameter :: test_reshape_char_1 = all(char_result == char_expected_result)
+  logical, parameter :: test_reshape_char_2 = all(shape(char_result, 8).EQ.new_shape)
+
+
+  ! Testing derived types
+  type(A), parameter :: derived_source(:) = &
+    [A(x=1.5, i=1), A(x=2.5, i=2), A(x=3.5, i=3), A(x=4.5, i=4), A(x=5.5, i=5), A(x=6.5, i=6)]
+
+  type(A), parameter :: derived_pad(2) = [A(x=7.5, i=7), A(x=8.5, i=8)]
+
+  type(A), parameter :: derived_expected_result(:, :) = &
+    reshape([a::a(x=1.5_4,i=1_8),a(x=5.5_4,i=5_8),a(x=2.5_4,i=2_8), a(x=6.5_4,i=6_8), &
+      a(x=3.5_4,i=3_8),a(x=7.5_4,i=7_8),a(x=4.5_4,i=4_8),a(x=8.5_4,i=8_8)], new_shape)
+
+  type(A), parameter :: derived_result(:, :) = reshape(derived_source, new_shape, derived_pad, order)
+
+  logical, parameter :: test_reshape_derived_1 = all((derived_result%x.EQ.derived_expected_result%x) &
+      .AND.(derived_result%i.EQ.derived_expected_result%i))
+
+  logical, parameter :: test_reshape_derived_2 = all(shape(derived_result).EQ.new_shape)
+end module