[PATCH] D99666: [flang] Implement reductions in the runtime

[Jean Perier via Phabricator via llvm-commits]

jeanPerier accepted this revision.
jeanPerier added a comment.
This revision is now accepted and ready to land.

LGTM, thanks for implementing all these so fast !



================
Comment at: flang/runtime/reduction.h:36
+// argument present.  These cases allocate and return a pointer to
+// an allocatable's descriptor.
+//
----------------
> These cases allocate and return a pointer to an allocatable's descriptor.

That part of the comment is not valid anymore.


================
Comment at: flang/unittests/RuntimeGTest/Reduction.cpp:50
+  for (const auto &x : data) {
+    EXPECT_GT(elements, 0);
+    StoreElement(p, x, elemLen);
----------------
I am getting warnings with gcc 8.3 regarding signed/unsigned compare here:


```
In file included from flang/unittests/RuntimeGTest/Reduction.cpp:10:
llvm/utils/unittest/googletest/include/gtest/gtest.h: In instantiation of ‘testing::AssertionResult testing::internal::CmpHelperGT(const char*, const char*, const T1&, const T2&) [with T1 = long unsigned int; T2 = int]’:
flang/unittests/RuntimeGTest/Reduction.cpp:50:5:   required from ‘Fortran::runtime::OwningPtr<Fortran::runtime::Descriptor> MakeArray(const std::vector<int>&, const std::vector<A>&, std::size_t) [with Fortran::common::TypeCategory CAT = (Fortran::common::TypeCategory)0; int KIND = 4; A = int; Fortran::runtime::OwningPtr<Fortran::runtime::Descriptor> = std::unique_ptr<Fortran::runtime::Descriptor, Fortran::runtime::OwningPtrDeleter<Fortran::runtime::Descriptor> >; std::size_t = long unsigned int]’
flang/unittests/RuntimeGTest/Reduction.cpp:60:74:   required from here
llvm/utils/unittest/googletest/include/gtest/gtest.h:1530:28: warning: comparison of integer expressions of different signedness: ‘const long unsigned int’ and ‘const int’ [-Wsign-compare]
llvm/utils/unittest/googletest/include/gtest/gtest.h:1510:7:
   if (val1 op val2) {\
```



================
Comment at: flang/unittests/RuntimeGTest/Reduction.cpp:245
+      shape, std::vector<std::int32_t>{false, false, true, true})};
+  ASSERT_EQ(array->ElementBytes(), 4);
+  EXPECT_EQ(RTNAME(All4)(*array, __FILE__, __LINE__), false);
----------------
Same here, I see a gcc 8.3.0 warning regarding signed/unsigned compare that I think stems from here.
```
warning: comparison of integer expressions of different signedness: ‘const long unsigned int’ and ‘const int’
flang/unittests/RuntimeGTest/Reduction.cpp:245:3:   required from here
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99666