From 5b3f9220cae46897040ec02fa2e2c920662e9250 Mon Sep 17 00:00:00 2001 From: River Riddle Date: Mon, 23 Dec 2019 21:08:46 -0800 Subject: [PATCH 1/5] NFC: Rename printOptionValue to printValue to fix MSVC build. MSVC has trouble resolving the static 'printOptionValue' from the method on llvm::cl::opt/list. This change renames the static method to avoid this conflict. PiperOrigin-RevId: 286978351 --- include/mlir/Pass/PassOptions.h | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/include/mlir/Pass/PassOptions.h b/include/mlir/Pass/PassOptions.h index 0ecb7ba970a0..66f4e860f4fb 100644 --- a/include/mlir/Pass/PassOptions.h +++ b/include/mlir/Pass/PassOptions.h @@ -83,22 +83,19 @@ class PassOptions : protected llvm::cl::SubCommand { /// Utility methods for printing option values. template - static void printOptionValue(raw_ostream &os, - GenericOptionParser &parser, - const DataT &value) { + static void printValue(raw_ostream &os, GenericOptionParser &parser, + const DataT &value) { if (Optional argStr = parser.findArgStrForValue(value)) os << argStr; else llvm_unreachable("unknown data value for option"); } template - static void printOptionValue(raw_ostream &os, ParserT &parser, - const DataT &value) { + static void printValue(raw_ostream &os, ParserT &parser, const DataT &value) { os << value; } template - static void printOptionValue(raw_ostream &os, ParserT &parser, - const bool &value) { + static void printValue(raw_ostream &os, ParserT &parser, const bool &value) { os << (value ? StringRef("true") : StringRef("false")); } @@ -129,7 +126,7 @@ class PassOptions : protected llvm::cl::SubCommand { /// Print the name and value of this option to the given stream. void print(raw_ostream &os) final { os << this->ArgStr << '='; - printOptionValue(os, this->getParser(), this->getValue()); + printValue(os, this->getParser(), this->getValue()); } /// Copy the value from the given option into this one. @@ -172,7 +169,7 @@ class PassOptions : protected llvm::cl::SubCommand { void print(raw_ostream &os) final { os << this->ArgStr << '='; auto printElementFn = [&](const DataType &value) { - printOptionValue(os, this->getParser(), value); + printValue(os, this->getParser(), value); }; interleave(*this, os, printElementFn, ","); } From efc35740c5622fa82690dcffb41dc42ebfb41a1b Mon Sep 17 00:00:00 2001 From: Tung Le Duc Date: Tue, 24 Dec 2019 16:02:10 +0900 Subject: [PATCH 2/5] Fix the wrong computation of dynamic strides for lowering AllocOp to LLVM --- lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp | 5 +++-- test/Conversion/StandardToLLVM/convert-memref-ops.mlir | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp b/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp index 0c96cc5e9c7d..23c3903346de 100644 --- a/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp +++ b/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp @@ -1054,14 +1054,15 @@ struct AllocOpLowering : public LLVMLegalizationPattern { // Iterate strides in reverse order, compute runningStride and strideValues. auto nStrides = strides.size(); SmallVector strideValues(nStrides, nullptr); - for (auto indexedStride : llvm::enumerate(llvm::reverse(strides))) { + for (auto indexedStride : llvm::enumerate(strides)) { int64_t index = nStrides - 1 - indexedStride.index(); if (strides[index] == MemRefType::getDynamicStrideOrOffset()) // Identity layout map is enforced in the match function, so we compute: // `runningStride *= sizes[index]` runningStride = runningStride - ? rewriter.create(loc, runningStride, sizes[index]) + ? rewriter.create(loc, runningStride, + sizes[index + 1]) : createIndexConstant(rewriter, loc, 1); else runningStride = createIndexConstant(rewriter, loc, strides[index]); diff --git a/test/Conversion/StandardToLLVM/convert-memref-ops.mlir b/test/Conversion/StandardToLLVM/convert-memref-ops.mlir index d92ded7f3aa9..706e2de2691f 100644 --- a/test/Conversion/StandardToLLVM/convert-memref-ops.mlir +++ b/test/Conversion/StandardToLLVM/convert-memref-ops.mlir @@ -101,8 +101,8 @@ func @mixed_alloc(%arg0: index, %arg1: index) -> memref { // CHECK-NEXT: %[[off:.*]] = llvm.mlir.constant(0 : index) : !llvm.i64 // CHECK-NEXT: llvm.insertvalue %[[off]], %{{.*}}[2] : !llvm<"{ float*, float*, i64, [3 x i64], [3 x i64] }"> // CHECK-NEXT: %[[st2:.*]] = llvm.mlir.constant(1 : index) : !llvm.i64 -// CHECK-NEXT: %[[st1:.*]] = llvm.mul %{{.*}}, %[[c42]] : !llvm.i64 -// CHECK-NEXT: %[[st0:.*]] = llvm.mul %{{.*}}, %[[M]] : !llvm.i64 +// CHECK-NEXT: %[[st1:.*]] = llvm.mul %{{.*}}, %[[N]] : !llvm.i64 +// CHECK-NEXT: %[[st0:.*]] = llvm.mul %{{.*}}, %[[c42]] : !llvm.i64 // CHECK-NEXT: llvm.insertvalue %[[M]], %{{.*}}[3, 0] : !llvm<"{ float*, float*, i64, [3 x i64], [3 x i64] }"> // CHECK-NEXT: llvm.insertvalue %[[st0]], %{{.*}}[4, 0] : !llvm<"{ float*, float*, i64, [3 x i64], [3 x i64] }"> // CHECK-NEXT: llvm.insertvalue %[[c42]], %{{.*}}[3, 1] : !llvm<"{ float*, float*, i64, [3 x i64], [3 x i64] }"> @@ -142,7 +142,7 @@ func @dynamic_alloc(%arg0: index, %arg1: index) -> memref { // CHECK-NEXT: %[[off:.*]] = llvm.mlir.constant(0 : index) : !llvm.i64 // CHECK-NEXT: llvm.insertvalue %[[off]], %{{.*}}[2] : !llvm<"{ float*, float*, i64, [2 x i64], [2 x i64] }"> // CHECK-NEXT: %[[st1:.*]] = llvm.mlir.constant(1 : index) : !llvm.i64 -// CHECK-NEXT: %[[st0:.*]] = llvm.mul %{{.*}}, %[[M]] : !llvm.i64 +// CHECK-NEXT: %[[st0:.*]] = llvm.mul %{{.*}}, %[[N]] : !llvm.i64 // CHECK-NEXT: llvm.insertvalue %[[M]], %{{.*}}[3, 0] : !llvm<"{ float*, float*, i64, [2 x i64], [2 x i64] }"> // CHECK-NEXT: llvm.insertvalue %[[st0]], %{{.*}}[4, 0] : !llvm<"{ float*, float*, i64, [2 x i64], [2 x i64] }"> // CHECK-NEXT: llvm.insertvalue %[[N]], %{{.*}}[3, 1] : !llvm<"{ float*, float*, i64, [2 x i64], [2 x i64] }"> From 7d7afcb855b87c3700c8afba454faf698bc5c9d4 Mon Sep 17 00:00:00 2001 From: Tung Le Duc Date: Tue, 24 Dec 2019 16:09:06 +0900 Subject: [PATCH 3/5] Edit comments --- lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp b/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp index 23c3903346de..b7d5fb1417bd 100644 --- a/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp +++ b/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp @@ -1058,7 +1058,7 @@ struct AllocOpLowering : public LLVMLegalizationPattern { int64_t index = nStrides - 1 - indexedStride.index(); if (strides[index] == MemRefType::getDynamicStrideOrOffset()) // Identity layout map is enforced in the match function, so we compute: - // `runningStride *= sizes[index]` + // `runningStride *= sizes[index + 1]` runningStride = runningStride ? rewriter.create(loc, runningStride, From fe23cee9640e8ef6b65c5e6c89de1373a11a1769 Mon Sep 17 00:00:00 2001 From: Tung Le Duc Date: Sat, 28 Dec 2019 22:48:17 +0900 Subject: [PATCH 4/5] Remove enumerate --- lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp b/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp index b7d5fb1417bd..03ad8cac7e15 100644 --- a/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp +++ b/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp @@ -1054,8 +1054,8 @@ struct AllocOpLowering : public LLVMLegalizationPattern { // Iterate strides in reverse order, compute runningStride and strideValues. auto nStrides = strides.size(); SmallVector strideValues(nStrides, nullptr); - for (auto indexedStride : llvm::enumerate(strides)) { - int64_t index = nStrides - 1 - indexedStride.index(); + for (unsigned i = 0; i < strides.size(); ++i) { + int64_t index = nStrides - 1 - i; if (strides[index] == MemRefType::getDynamicStrideOrOffset()) // Identity layout map is enforced in the match function, so we compute: // `runningStride *= sizes[index + 1]` From aa18969882b7da46db7c9c86afa9a1bd11232da5 Mon Sep 17 00:00:00 2001 From: Tung Le Duc Date: Sat, 28 Dec 2019 22:55:44 +0900 Subject: [PATCH 5/5] Use nStrides instead of strides.size() --- lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp b/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp index 03ad8cac7e15..c456b00987d7 100644 --- a/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp +++ b/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp @@ -1054,7 +1054,7 @@ struct AllocOpLowering : public LLVMLegalizationPattern { // Iterate strides in reverse order, compute runningStride and strideValues. auto nStrides = strides.size(); SmallVector strideValues(nStrides, nullptr); - for (unsigned i = 0; i < strides.size(); ++i) { + for (unsigned i = 0; i < nStrides; ++i) { int64_t index = nStrides - 1 - i; if (strides[index] == MemRefType::getDynamicStrideOrOffset()) // Identity layout map is enforced in the match function, so we compute: