[mlir] NFC: Rename index_t to index_type

[ro]

Summary

mlir currently fails to build on Solaris:

/vol/llvm/src/llvm-project/dist/mlir/lib/Conversion/VectorToLoops/ConvertVectorToLoops.cpp:78:20: error: reference to 'index_t' is ambiguous
  IndexHandle zero(index_t(0)), one(index_t(1));
                   ^
/usr/include/sys/types.h:103:16: note: candidate found by name lookup is 'index_t'
typedef short           index_t;
                        ^
/vol/llvm/src/llvm-project/dist/mlir/include/mlir/EDSC/Builders.h:27:8: note: candidate found by name lookup is 'mlir::edsc::index_t'
struct index_t {
       ^

and many more.

Given that POSIX reserves all identifiers ending in _t 2.2.2 The Name Space, it seems
quite unwise to use such identifiers in user code, even more so without a distinguished
prefix.

The following patch fixes this by adding a mlir_ prefix as is already used in other
cases.

Tested on amd64-pc-solaris2.11 and sparcv9-sun-solaris2.11.

Ok for master?

Btw., it would be helpful if mlir were either listed in the global CODE_OWNERS.TXT or had one of its own.

[nicolasvasilache]

Hello Rainer,

could we sed this to index_type instead and address the _t suffix once and for all as you suggest in your description of the problem?
I am not attached to _t and I see the downside now.

[ro]

Rename index_t to index_type instead.

[ro]

In D72619#1817169, @nicolasvasilache wrote:

Hello Rainer,

could we sed this to index_type instead and address the _t suffix once and for all as you suggest in your description of the problem?
I am not attached to _t and I see the downside now.

Sure, done now. Tested as before.

However, while all mlir tests PASS on Solaris/amd64, there are some failures on Solaris/sparcv9 (which existed before, the patch is just meant to address the compile failure).

MLIR-Unit :: TableGen/./MLIRTableGenTests/StructsGenTest.GetElements

MLIR :: AffineOps/ops.mlir
MLIR :: Conversion/GPUToSPIRV/simple.mlir
MLIR :: Conversion/LoopsToGPU/imperfect_linalg.mlir
MLIR :: Conversion/StandardToLLVM/convert-to-llvmir.mlir
MLIR :: Conversion/StandardToSPIRV/legalization.mlir
MLIR :: Conversion/StandardToSPIRV/std-to-spirv.mlir
MLIR :: Conversion/StandardToSPIRV/subview-to-spirv.mlir
MLIR :: Conversion/VectorToLoops/vector-to-loops.mlir
MLIR :: Dialect/FxpMathOps/lower-uniform-casts.mlir
MLIR :: Dialect/FxpMathOps/lower-uniform-real-math-addew.mlir
MLIR :: Dialect/FxpMathOps/lower-uniform-real-math-mulew.mlir
MLIR :: Dialect/LLVMIR/roundtrip.mlir
MLIR :: Dialect/Linalg/fusion-2-level.mlir
MLIR :: Dialect/Linalg/fusion.mlir
MLIR :: Dialect/Linalg/promote.mlir
MLIR :: Dialect/Linalg/tile.mlir
MLIR :: Dialect/Linalg/tile_conv.mlir
MLIR :: Dialect/Linalg/tile_indexed_generic.mlir
MLIR :: Dialect/Linalg/transform-patterns.mlir
MLIR :: Dialect/QuantOps/canonicalize.mlir
MLIR :: Dialect/QuantOps/convert-const.mlir
MLIR :: Dialect/SPIRV/Serialization/constant.mlir
MLIR :: Dialect/SPIRV/Transforms/abi-load-store.mlir
MLIR :: Dialect/SPIRV/Transforms/abi-simple.mlir
MLIR :: Dialect/SPIRV/canonicalize.mlir
MLIR :: Dialect/SPIRV/structure-ops.mlir
MLIR :: Dialect/SPIRV/target-and-abi.mlir
MLIR :: Dialect/VectorOps/invalid.mlir
MLIR :: Dialect/VectorOps/ops.mlir
MLIR :: IR/attribute.mlir
MLIR :: IR/core-ops.mlir
MLIR :: IR/invalid-ops.mlir
MLIR :: IR/parser.mlir
MLIR :: IR/pretty-attributes.mlir
MLIR :: IR/traits.mlir
MLIR :: Quantizer/matmul.mlir
MLIR :: Target/llvmir.mlir
MLIR :: Transforms/Vectorize/vectorize_1d.mlir
MLIR :: Transforms/Vectorize/vectorize_2d.mlir
MLIR :: Transforms/canonicalize.mlir
MLIR :: Transforms/constant-fold.mlir
MLIR :: mlir-cpu-runner/linalg_integration_test.mlir
MLIR :: mlir-cpu-runner/simple.mlir
MLIR :: mlir-cpu-runner/unranked_memref.mlir
MLIR :: mlir-cpu-runner/utils.mlir
MLIR :: mlir-tblgen/pattern.mlir

I haven't even started looking what might be wrong here (endianess issue?).

[merge_guards_bot]

Unit tests: pass. 61892 tests passed, 0 failed and 782 were skipped.

clang-tidy: unknown.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[nicolasvasilache]

understood, thank you!

[ro]

Turns out I forgot to rerun clang-format-diff.py after renaming mlir_index_t to
index_type. Fixed now. Also updated mlir/docs/EDSC.md to match.

[merge_guards_bot]

Unit tests: pass. 61909 tests passed, 0 failed and 783 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

mlir/docs/EDSC.md

ValueHandle i(indexType),
               j(indexType),
               lb(f->getArgument(0)),
               ub(f->getArgument(1));
   ValueHandle f7(constant_float(llvm::APFloat(7.0f), f32Type)),
               f13(constant_float(llvm::APFloat(13.0f), f32Type)),
               i7(constant_int(7, 32)),
               i13(constant_int(13, 32));
   AffineLoopNestBuilder(&i, lb, ub, 3)([&]{
-      lb * index_t(3) + ub;
-      lb + index_t(3);
+      lb * index_type(3) + ub;
+      lb + index_type(3);
       AffineLoopNestBuilder(&j, lb, ub, 2)([&]{
-          ceilDiv(index_t(31) * floorDiv(i + j * index_t(3), index_t(32)),
-                  index_t(32));
+          ceilDiv(index_type(31) * floorDiv(i + j * index_type(3), index_type(32)),
+                  index_type(32));
           ((f7 + f13) / f7) % f13 - f7 * f13;
           ((i7 + i13) / i7) % i13 - i7 * i13;
       });
   });
 ```
 
 ## IndexedValue
 

mlir/include/mlir/EDSC/Builders.h

 #include "mlir/Dialect/StandardOps/Ops.h"
 #include "mlir/IR/Builders.h"
 #include "mlir/Transforms/FoldUtils.h"
 
 namespace mlir {
 
 namespace edsc {
 
-struct index_t {
-  explicit index_t(int64_t v) : v(v) {}
+struct index_type {
+  explicit index_type(int64_t v) : v(v) {}
   explicit operator int64_t() { return v; }
   int64_t v;
 };
 
 class BlockHandle;
 class CapturableHandle;
 class NestedBuilder;
 class ValueHandle;
public:
   /// been constructed in the past and that is captured "now" in the program.
   explicit ValueHandle(Value v) : t(v.getType()), v(v) {}
 
   /// Builds a ConstantIndexOp of value `cst`. The constant is created at the
   /// current insertion point.
   /// This implicit constructor is provided to each build an eager Value for a
   /// constant at the current insertion point in the IR. An implicit constructor
   /// allows idiomatic expressions mixing ValueHandle and literals.
-  ValueHandle(index_t cst);
+  ValueHandle(index_type cst);
 
   /// ValueHandle is a value type, use the default copy constructor.
   ValueHandle(const ValueHandle &other) = default;
 
   /// ValueHandle is a value type, the assignment operator typechecks before
   /// assigning.
   ValueHandle &operator=(const ValueHandle &other);
 

mlir/include/mlir/EDSC/Intrinsics.h

 /// declarations without boilerplate such as:
 ///
 /// ```c++
 ///    IndexHandle i, j, k;
 /// ```
 struct IndexHandle : public ValueHandle {
   explicit IndexHandle()
       : ValueHandle(ScopedContext::getBuilder().getIndexType()) {}
-  explicit IndexHandle(index_t v) : ValueHandle(v) {}
+  explicit IndexHandle(index_type v) : ValueHandle(v) {}
   explicit IndexHandle(Value v) : ValueHandle(v) {
     assert(v.getType() == ScopedContext::getBuilder().getIndexType() &&
            "Expected index type");
   }
   explicit IndexHandle(ValueHandle v) : ValueHandle(v) {
     assert(v.getType() == ScopedContext::getBuilder().getIndexType() &&
            "Expected index type");
   }

 class ValueHandleArray {
 public:
   ValueHandleArray(ArrayRef<ValueHandle> vals) {
     values.append(vals.begin(), vals.end());
   }
   ValueHandleArray(ArrayRef<IndexHandle> vals) {
     values.append(vals.begin(), vals.end());
   }
-  ValueHandleArray(ArrayRef<index_t> vals) {
+  ValueHandleArray(ArrayRef<index_type> vals) {
     SmallVector<IndexHandle, 8> tmp(vals.begin(), vals.end());
     values.append(tmp.begin(), tmp.end());
   }
   operator ArrayRef<Value>() { return values; }
 
 private:
   ValueHandleArray() = default;
   SmallVector<Value, 8> values;

mlir/lib/Conversion/VectorToLoops/ConvertVectorToLoops.cpp

 template <typename TransferOpTy>
 static SmallVector<edsc::ValueHandle, 8> clip(TransferOpTy transfer,
                                               edsc::MemRefView &view,
                                               ArrayRef<edsc::IndexHandle> ivs) {
   using namespace mlir::edsc;
   using namespace edsc::op;
   using edsc::intrinsics::select;
 
-  IndexHandle zero(index_t(0)), one(index_t(1));
+  IndexHandle zero(index_type(0)), one(index_type(1));
   SmallVector<edsc::ValueHandle, 8> memRefAccess(transfer.indices());
   SmallVector<edsc::ValueHandle, 8> clippedScalarAccessExprs(
       memRefAccess.size(), edsc::IndexHandle());
 
   // Indices accessing to remote memory are clipped and their expressions are
   // returned in clippedScalarAccessExprs.
   for (unsigned memRefDim = 0; memRefDim < clippedScalarAccessExprs.size();
        ++memRefDim) {

mlir/lib/EDSC/Builders.cpp

assert(ScopedContext::getCurrentScopedContext() &&
          "Unexpected Null ScopedContext");
   return ScopedContext::getCurrentScopedContext()->location;
 }
 
 MLIRContext *mlir::edsc::ScopedContext::getContext() {
   return getBuilder().getContext();
 }
 
-mlir::edsc::ValueHandle::ValueHandle(index_t cst) {
+mlir::edsc::ValueHandle::ValueHandle(index_type cst) {
   auto &b = ScopedContext::getBuilder();
   auto loc = ScopedContext::getLocation();
   v = b.create<ConstantIndexOp>(loc, cst.v).getResult();
   t = v.getType();
 }
 
 ValueHandle &mlir::edsc::ValueHandle::operator=(const ValueHandle &other) {
   assert(t == other.t && "Wrong type capture");

mlir/lib/EDSC/Helpers.cpp

static SmallVector<ValueHandle, 8> getMemRefSizes(Value memRef) {
 
   SmallVector<ValueHandle, 8> res;
   res.reserve(memRefType.getShape().size());
   const auto &shape = memRefType.getShape();
   for (unsigned idx = 0, n = shape.size(); idx < n; ++idx) {
     if (shape[idx] == -1) {
       res.push_back(ValueHandle::create<DimOp>(memRef, idx));
     } else {
-      res.push_back(static_cast<index_t>(shape[idx]));
+      res.push_back(static_cast<index_type>(shape[idx]));
     }
   }
   return res;
 }
 
 mlir::edsc::MemRefView::MemRefView(Value v) : base(v) {
   assert(v.getType().isa<MemRefType>() && "MemRefType expected");
 
   auto memrefSizeValues = getMemRefSizes(v);
   for (auto &size : memrefSizeValues) {
-    lbs.push_back(static_cast<index_t>(0));
+    lbs.push_back(static_cast<index_type>(0));
     ubs.push_back(size);
     steps.push_back(1);
   }
 }
 
 mlir::edsc::VectorView::VectorView(Value v) : base(v) {
   auto vectorType = v.getType().cast<VectorType>();
 
   for (auto s : vectorType.getShape()) {
-    lbs.push_back(static_cast<index_t>(0));
-    ubs.push_back(static_cast<index_t>(s));
+    lbs.push_back(static_cast<index_type>(0));
+    ubs.push_back(static_cast<index_type>(s));
     steps.push_back(1);
   }
 }

mlir/test/EDSC/builder-api-test.cpp

TEST_FUNC(builder_dynamic_for_func_args) {
   ScopedContext scope(builder, f.getLoc());
   ValueHandle i(indexType), j(indexType), lb(f.getArgument(0)),
       ub(f.getArgument(1));
   ValueHandle f7(constant_float(llvm::APFloat(7.0f), f32Type));
   ValueHandle f13(constant_float(llvm::APFloat(13.0f), f32Type));
   ValueHandle i7(constant_int(7, 32));
   ValueHandle i13(constant_int(13, 32));
   AffineLoopNestBuilder(&i, lb, ub, 3)([&] {
-    lb *index_t(3) + ub;
-    lb + index_t(3);
+    lb *index_type(3) + ub;
+    lb + index_type(3);
     AffineLoopNestBuilder(&j, lb, ub, 2)([&] {
-      ceilDiv(index_t(31) * floorDiv(i + j * index_t(3), index_t(32)),
-              index_t(32));
+      ceilDiv(index_type(31) * floorDiv(i + j * index_type(3), index_type(32)),
+              index_type(32));
       ((f7 + f13) / f7) % f13 - f7 *f13;
       ((i7 + i13) / i7) % i13 - i7 *i13;
     });
   });
 
   // clang-format off
   // CHECK-LABEL: func @builder_dynamic_for_func_args(%{{.*}}: index, %{{.*}}: index) {
   //     CHECK:  affine.for %{{.*}} = affine_map<(d0) -> (d0)>(%{{.*}}) to affine_map<(d0) -> (d0)>(%{{.*}}) step 3 {
TEST_FUNC(custom_ops) {
   CustomOperation<ValueHandle> MY_CUSTOM_OP("my_custom_op");
   CustomOperation<OperationHandle> MY_CUSTOM_OP_0("my_custom_op_0");
   CustomOperation<OperationHandle> MY_CUSTOM_OP_2("my_custom_op_2");
 
   // clang-format off
   ValueHandle vh(indexType), vh20(indexType), vh21(indexType);
   OperationHandle ih0, ih2;
   IndexHandle m, n, M(f.getArgument(0)), N(f.getArgument(1));
-  IndexHandle ten(index_t(10)), twenty(index_t(20));
+  IndexHandle ten(index_type(10)), twenty(index_type(20));
   AffineLoopNestBuilder({&m, &n}, {M, N}, {M + ten, N + twenty}, {1, 1})([&]{
     vh = MY_CUSTOM_OP({m, m + n}, {indexType}, {});
     ih0 = MY_CUSTOM_OP_0({m, m + n}, {});
     ih2 = MY_CUSTOM_OP_2({m, m + n}, {indexType, indexType});
     // These captures are verbose for now, can improve when used in practice.
     vh20 = ValueHandle(ih2.getOperation()->getResult(0));
     vh21 = ValueHandle(ih2.getOperation()->getResult(1));
     MY_CUSTOM_OP({vh20, vh21}, {indexType}, {});