[mlir][CRunnerUtils] Enable compilation with C++11 toolchain on microcontroller platforms.

[nicolasvasilache]

Summary

The C runner utils API was still not vanilla enough for certain use
cases on embedded ARM SDKs, this enables such cases.

Adding people more widely for historical Windows related build issues.

[nicolasvasilache]

Update commit message.

[aartbik]

LGTM if others see no Windows issue

mlir/include/mlir/ExecutionEngine/CRunnerUtils.h

48

perhaps use () on the part after : too for readability
(assuming this does not violate some llvm brevity rule I don't know yet :-)

[rriddle]

What kind of constraints are we placing on the runner utils? The rest of the codebase is C++14, and it is quite likely that C++14 features might sneak in here later.

[nicolasvasilache]

@rriddle

CRunnerUtils:
Entities in this file are must be retargetable, including on targets without a C++ runtime.
I'll add the must compile with C++11 constraint in doc too.

If you have suggestions on how to test this concretely, I'd be happy to add it.

RunnerUtils in general: no constraints, it includes CRunnerUtils.

[flaub]

I don't have MSVC at this minute but this seems OK. Is it that the older compiler doesn't understand constexpr functions? I guess there's no fundamental reason the accessor operators need to be constexpr.

mlir/include/mlir/ExecutionEngine/CRunnerUtils.h

48

Chaining multiple ternary operations just seems less readable than the original.

[stella.stamenova]

Adding Nate who is going to own the Windows MLIR Buildbot during my maternity :).

[mehdi_amini]

In D76031#1918291, @nicolasvasilache wrote:

@rriddle

CRunnerUtils:
Entities in this file are must be retargetable, including on targets without a C++ runtime.
I'll add the must compile with C++11 constraint in doc too.

If you have suggestions on how to test this concretely, I'd be happy to add it.

Force-build it in c++11 mode even when the rest of the project is built with c++14 or above?

[NathanielMcVicar]

The mlir-check target passes with MS C++ 19.24.28319 (except for the known failure). If there is another test that would be helpful on Windows let me know.

[nicolasvasilache]

mlir/include/mlir/ExecutionEngine/CRunnerUtils.h

48

I agree but I have recollections from olden days that strict C++11 can only deduce the type if the body is a single line return statement.
The person who submitted this internally has a strict C++11 toolchain.
I found a few places that mention this limit (e.g. grep "single line" in https://en.cppreference.com/w/cpp/language/constexpr)
But I didn't go and dig into the standard for a 1-line readability regression.

[nicolasvasilache]

mlir/include/mlir/ExecutionEngine/CRunnerUtils.h

48

@flaub

I can confirm that with the CMake change to force C++11 I get the following error on the old code:

/usr/local/google/home/ntv/work/github/llvm-project/mlir/include/mlir/ExecutionEngine/CRunnerUtils.h:48:3: error: use of this statement in a constexpr function is a C++14 extension [-Werror,-Wc++14-extensions]
  if (N <= 1) return 1;
  ^
/usr/local/google/home/ntv/work/github/llvm-project/mlir/include/mlir/ExecutionEngine/CRunnerUtils.h:49:3: error: multiple return statements in constexpr function is a C++14 extension [-Werror,-Wc++14-extensions]
  return (isPowerOf2(N) ? N : 2 * nextPowerOf2((N + 1) / 2));
  ^
/usr/local/google/home/ntv/work/github/llvm-project/mlir/include/mlir/ExecutionEngine/CRunnerUtils.h:48:15: note: previous return statement is here
  if (N <= 1) return 1;

[nicolasvasilache]

Force C++11 in cmake, address readability comment.

[nicolasvasilache]

@stella.stamenova thank you and congrats ! :)

@NathanielMcVicar thanks for checking, I made a minor CMake addition, I do not expect it to break the Win build but please double check for me? Thanks!

mlir/include/mlir/ExecutionEngine/CRunnerUtils.h

 //===- CRunnerUtils.h - Utils for debugging MLIR execution ----------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file declares basic classes and functions to manipulate structured MLIR
-// types at runtime. Entities in this file are must be retargetable, including
-// on targets without a C++ runtime.
+// types at runtime. Entities in this file must be compliant with C++11 and be
+// retargetable, including on targets without a C++ runtime.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef EXECUTIONENGINE_CRUNNERUTILS_H_
 #define EXECUTIONENGINE_CRUNNERUTILS_H_
 
 #ifdef _WIN32
 #ifndef MLIR_CRUNNERUTILS_EXPORT

 //===----------------------------------------------------------------------===//
 // Codegen-compatible structures for Vector type.
 //===----------------------------------------------------------------------===//
 namespace detail {
 
 constexpr bool isPowerOf2(int N) { return (!(N & (N - 1))); }
 
 constexpr unsigned nextPowerOf2(int N) {
-  if (N <= 1)
-    return 1;
-  return isPowerOf2(N) ? N : 2 * nextPowerOf2((N + 1) / 2);
+  return (N <= 1) ? 1 : (isPowerOf2(N) ? N : (2 * nextPowerOf2((N + 1) / 2)));
 }
 
 template <typename T, int Dim, bool IsPowerOf2>
 struct Vector1D;
 
 template <typename T, int Dim>
 struct Vector1D<T, Dim, /*IsPowerOf2=*/true> {
   Vector1D() {
     static_assert(detail::nextPowerOf2(sizeof(T[Dim])) == sizeof(T[Dim]),
                   "size error");
   }
-  constexpr T &operator[](unsigned i) { return vector[i]; }
-  constexpr const T &operator[](unsigned i) const { return vector[i]; }
+  inline T &operator[](unsigned i) { return vector[i]; }
+  inline const T &operator[](unsigned i) const { return vector[i]; }
 
 private:
   T vector[Dim];
 };
 
 // 1-D vector, padded to the next power of 2 allocation.
 // Specialization occurs to avoid zero size arrays (which fail in -Werror).
 template <typename T, int Dim>
 struct Vector1D<T, Dim, /*IsPowerOf2=*/false> {
   Vector1D() {
     static_assert(detail::nextPowerOf2(sizeof(T[Dim])) > sizeof(T[Dim]),
                   "size error");
     static_assert(detail::nextPowerOf2(sizeof(T[Dim])) < 2 * sizeof(T[Dim]),
                   "size error");
   }
-  constexpr T &operator[](unsigned i) { return vector[i]; }
-  constexpr const T &operator[](unsigned i) const { return vector[i]; }
+  inline T &operator[](unsigned i) { return vector[i]; }
+  inline const T &operator[](unsigned i) const { return vector[i]; }
 
 private:
   T vector[Dim];
   char padding[detail::nextPowerOf2(sizeof(T[Dim])) - sizeof(T[Dim])];
 };
 } // end namespace detail
 
 // N-D vectors recurse down to 1-D.
 template <typename T, int Dim, int... Dims>
 struct Vector {
-  constexpr Vector<T, Dims...> &operator[](unsigned i) { return vector[i]; }
-  constexpr const Vector<T, Dims...> &operator[](unsigned i) const {
+  inline Vector<T, Dims...> &operator[](unsigned i) { return vector[i]; }
+  inline const Vector<T, Dims...> &operator[](unsigned i) const {
     return vector[i];
   }
 
 private:
   Vector<T, Dims...> vector[Dim];
 };
 
 // 1-D vectors in LLVM are automatically padded to the next power of 2.

mlir/lib/ExecutionEngine/CMakeLists.txt

target_link_libraries(MLIRExecutionEngine
   LLVMScalarOpts
   LLVMTarget
   LLVMVectorize
   LLVMTransformUtils
 
   ${outlibs})
 
 add_llvm_library(mlir_c_runner_utils SHARED CRunnerUtils.cpp)
+set_property(TARGET mlir_c_runner_utils PROPERTY CXX_STANDARD 11)
 add_llvm_library(mlir_c_runner_utils_static CRunnerUtils.cpp)
+set_property(TARGET mlir_c_runner_utils_static PROPERTY CXX_STANDARD 11)
 target_compile_definitions(mlir_c_runner_utils PRIVATE mlir_c_runner_utils_EXPORTS)
 
 add_llvm_library(mlir_runner_utils SHARED RunnerUtils.cpp)
 target_link_libraries(mlir_runner_utils
   PUBLIC
   mlir_c_runner_utils_static
 )
 target_compile_definitions(mlir_runner_utils PRIVATE mlir_runner_utils_EXPORTS)