[flang-commits] [PATCH] D104586: [flang] Update Reshape runtime library routine and its corresponding unit test

[Peter Klausler via Phabricator via flang-commits]

klausler added inline comments.


================
Comment at: flang/runtime/transformational.cpp:355
 // F2018 16.9.163
-OwningPtr<Descriptor> RTNAME(Reshape)(const Descriptor &source,
+void RTNAME(Reshape)(Descriptor &result, const Descriptor &source,
     const Descriptor &shape, const Descriptor *pad, const Descriptor *order,
----------------
needs extern "C" to match API in header


================
Comment at: flang/runtime/transformational.cpp:397
     SubscriptValue orderSubscript{order->GetDimension(0).LowerBound()};
-    for (SubscriptValue j{0}; j < resultRank; ++j, ++orderSubscript) {
-      auto k{GetInt64(order->OffsetElement<char>(orderSubscript),
-          shapeElementBytes, terminator)};
+    auto *addr = order->OffsetElement<char>(orderSubscript);
+    std::size_t orderElementBytes{order->ElementBytes()};
----------------
klausler wrote:
> jeanPerier wrote:
> > I think `auto k{GetInt64(shape.Element<char>(&orderSubscript), orderElementBytes, terminator)}` might be more idiomatic than manually incrementing the byte offset. @klausler, is this correct ?
> Yes.  When something in the runtime can be written as an application of subscripts to an array, that's usually the right way to do it.
braces, please


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104586/new/

https://reviews.llvm.org/D104586