From 5e9dc8a5cc153269f25c07507228df946b43a605 Mon Sep 17 00:00:00 2001 From: River Riddle Date: Mon, 19 Aug 2019 15:26:43 -0700 Subject: [PATCH] Allow isolated regions to form isolated SSA name scopes in the printer. This will allow for naming values the same as existing SSA values for regions attached to operations that are isolated from above. This fits in with how the system already allows separate name scopes for sibling regions. This name shadowing can be enabled in the custom parser of operations by setting the 'enableNameShadowing' flag to true when calling 'parseRegion'. %arg = constant 10 : i32 foo.op { %arg = constant 10 : i32 } PiperOrigin-RevId: 264255999 --- tensorflow/opensource_only.files | 92 ++++----- third_party/mlir/include/mlir/IR/OpBase.td | 2 + .../mlir/include/mlir/IR/OpImplementation.h | 16 +- third_party/mlir/lib/Parser/Parser.cpp | 183 +++++++++++------- .../mlir/test/lib/TestDialect/TestDialect.cpp | 27 ++- .../mlir/test/lib/TestDialect/TestOps.td | 12 ++ 6 files changed, 203 insertions(+), 129 deletions(-) diff --git a/tensorflow/opensource_only.files b/tensorflow/opensource_only.files index 7c4fc5e6811..ee78249d851 100644 --- a/tensorflow/opensource_only.files +++ b/tensorflow/opensource_only.files @@ -11,94 +11,94 @@ tensorflow/python/tpu/profiler/pip_package/build_pip_package.sh tensorflow/python/tpu/profiler/pip_package/setup.py tensorflow/stream_executor/build_defs.bzl tensorflow/third_party/BUILD -tensorflow/third_party/__init__.py tensorflow/third_party/android/BUILD -tensorflow/third_party/android/android_configure.BUILD.tpl tensorflow/third_party/android/android.bzl.tpl +tensorflow/third_party/android/android_configure.BUILD.tpl tensorflow/third_party/android/android_configure.bzl +tensorflow/third_party/__init__.py tensorflow/third_party/arm_neon_2_x86_sse.BUILD tensorflow/third_party/astor.BUILD -tensorflow/third_party/backports_weakref.BUILD tensorflow/third_party/boringssl/BUILD +tensorflow/third_party/backports_weakref.BUILD tensorflow/third_party/clang_toolchain/BUILD tensorflow/third_party/clang_toolchain/cc_configure_clang.bzl tensorflow/third_party/clang_toolchain/download_clang.bzl tensorflow/third_party/codegen.BUILD tensorflow/third_party/com_google_absl.BUILD -tensorflow/third_party/cub.BUILD tensorflow/third_party/common.bzl +tensorflow/third_party/cub.BUILD tensorflow/third_party/curl.BUILD tensorflow/third_party/cython.BUILD -tensorflow/third_party/double_conversion.BUILD -tensorflow/third_party/eigen.BUILD -tensorflow/third_party/eigen3/BUILD tensorflow/third_party/eigen3/Eigen/Cholesky tensorflow/third_party/eigen3/Eigen/Core tensorflow/third_party/eigen3/Eigen/Eigenvalues tensorflow/third_party/eigen3/Eigen/LU tensorflow/third_party/eigen3/Eigen/QR tensorflow/third_party/eigen3/Eigen/SVD +tensorflow/third_party/eigen3/BUILD tensorflow/third_party/eigen3/LICENSE tensorflow/third_party/eigen3/gpu_packet_math.patch tensorflow/third_party/eigen3/unsupported/Eigen/CXX11/Tensor tensorflow/third_party/eigen3/unsupported/Eigen/CXX11/FixedPoint tensorflow/third_party/eigen3/unsupported/Eigen/CXX11/ThreadPool -tensorflow/third_party/eigen3/unsupported/Eigen/CXX11/src/FixedPoint/MatMatProduct.h tensorflow/third_party/eigen3/unsupported/Eigen/CXX11/src/FixedPoint/FixedPointTypes.h +tensorflow/third_party/eigen3/unsupported/Eigen/CXX11/src/FixedPoint/MatMatProduct.h tensorflow/third_party/eigen3/unsupported/Eigen/CXX11/src/FixedPoint/MatMatProductAVX2.h tensorflow/third_party/eigen3/unsupported/Eigen/CXX11/src/FixedPoint/MatMatProductNEON.h -tensorflow/third_party/eigen3/unsupported/Eigen/CXX11/src/FixedPoint/MatVecProduct.h tensorflow/third_party/eigen3/unsupported/Eigen/CXX11/src/FixedPoint/PacketMathAVX2.h tensorflow/third_party/eigen3/unsupported/Eigen/CXX11/src/FixedPoint/PacketMathAVX512.h +tensorflow/third_party/eigen3/unsupported/Eigen/CXX11/src/FixedPoint/MatVecProduct.h tensorflow/third_party/eigen3/unsupported/Eigen/CXX11/src/FixedPoint/TypeCastingAVX2.h tensorflow/third_party/eigen3/unsupported/Eigen/CXX11/src/FixedPoint/TypeCastingAVX512.h -tensorflow/third_party/eigen3/unsupported/Eigen/SpecialFunctions tensorflow/third_party/eigen3/unsupported/Eigen/MatrixFunctions +tensorflow/third_party/eigen3/unsupported/Eigen/SpecialFunctions +tensorflow/third_party/double_conversion.BUILD +tensorflow/third_party/eigen.BUILD tensorflow/third_party/enum34.BUILD -tensorflow/third_party/farmhash.BUILD tensorflow/third_party/fft2d/BUILD tensorflow/third_party/fft2d/LICENSE tensorflow/third_party/fft2d/fft.h tensorflow/third_party/fft2d/fft2d.BUILD tensorflow/third_party/fft2d/fft2d.h +tensorflow/third_party/farmhash.BUILD tensorflow/third_party/functools32.BUILD tensorflow/third_party/gast.BUILD tensorflow/third_party/git/BUILD.tpl tensorflow/third_party/git/BUILD tensorflow/third_party/git/git_configure.bzl tensorflow/third_party/gif.BUILD -tensorflow/third_party/googleapis.BUILD -tensorflow/third_party/gpus/BUILD tensorflow/third_party/gpus/crosstool/BUILD -tensorflow/third_party/gpus/crosstool/BUILD.tpl tensorflow/third_party/gpus/crosstool/LICENSE -tensorflow/third_party/gpus/crosstool/clang/bin/crosstool_wrapper_driver_rocm.tpl +tensorflow/third_party/gpus/crosstool/BUILD.tpl tensorflow/third_party/gpus/crosstool/clang/bin/crosstool_wrapper_driver_is_not_gcc.tpl +tensorflow/third_party/gpus/crosstool/clang/bin/crosstool_wrapper_driver_rocm.tpl tensorflow/third_party/gpus/crosstool/windows/msvc_wrapper_for_nvcc.py.tpl -tensorflow/third_party/gpus/cuda/BUILD +tensorflow/third_party/gpus/BUILD tensorflow/third_party/gpus/cuda/BUILD.tpl tensorflow/third_party/gpus/cuda/BUILD.windows.tpl +tensorflow/third_party/gpus/cuda/BUILD tensorflow/third_party/gpus/cuda/LICENSE tensorflow/third_party/gpus/cuda/build_defs.bzl.tpl tensorflow/third_party/gpus/cuda/cuda_config.h.tpl tensorflow/third_party/gpus/cuda_configure.bzl -tensorflow/third_party/gpus/find_cuda_config.py tensorflow/third_party/gpus/rocm/BUILD tensorflow/third_party/gpus/rocm/BUILD.tpl -tensorflow/third_party/gpus/rocm/rocm_config.h.tpl tensorflow/third_party/gpus/rocm/build_defs.bzl.tpl +tensorflow/third_party/gpus/rocm/rocm_config.h.tpl +tensorflow/third_party/gpus/find_cuda_config.py tensorflow/third_party/gpus/rocm_configure.bzl +tensorflow/third_party/googleapis.BUILD tensorflow/third_party/grpc/BUILD tensorflow/third_party/icu/udata.patch -tensorflow/third_party/jsoncpp.BUILD tensorflow/third_party/kafka/BUILD tensorflow/third_party/kafka/config.patch -tensorflow/third_party/libxsmm.BUILD -tensorflow/third_party/linenoise.BUILD +tensorflow/third_party/jsoncpp.BUILD tensorflow/third_party/llvm/BUILD tensorflow/third_party/llvm/expand_cmake_vars.py tensorflow/third_party/llvm/llvm.autogenerated.BUILD tensorflow/third_party/llvm/llvm.bzl +tensorflow/third_party/linenoise.BUILD +tensorflow/third_party/libxsmm.BUILD tensorflow/third_party/lmdb.BUILD tensorflow/third_party/mkl/BUILD tensorflow/third_party/mkl/LICENSE @@ -112,45 +112,45 @@ tensorflow/third_party/mpi/BUILD tensorflow/third_party/mpi_collectives/BUILD tensorflow/third_party/nanopb.BUILD tensorflow/third_party/nccl/BUILD -tensorflow/third_party/nccl/LICENSE tensorflow/third_party/nccl/archive.BUILD -tensorflow/third_party/nccl/archive.patch +tensorflow/third_party/nccl/LICENSE tensorflow/third_party/nccl/build_defs.bzl.tpl +tensorflow/third_party/nccl/archive.patch tensorflow/third_party/nccl/system.BUILD.tpl tensorflow/third_party/nccl/nccl_configure.bzl tensorflow/third_party/ngraph/BUILD tensorflow/third_party/ngraph/LICENSE tensorflow/third_party/ngraph/NGRAPH_LICENSE -tensorflow/third_party/ngraph/build_defs.bzl tensorflow/third_party/ngraph/ngraph.BUILD +tensorflow/third_party/ngraph/build_defs.bzl tensorflow/third_party/ngraph/ngraph_tf.BUILD tensorflow/third_party/ngraph/nlohmann_json.BUILD tensorflow/third_party/ngraph/tbb.BUILD tensorflow/third_party/opt_einsum.BUILD tensorflow/third_party/pcre.BUILD tensorflow/third_party/png.BUILD -tensorflow/third_party/pprof.BUILD tensorflow/third_party/protobuf/BUILD tensorflow/third_party/png_fix_rpi.patch -tensorflow/third_party/py/BUILD.tpl -tensorflow/third_party/py/BUILD +tensorflow/third_party/pprof.BUILD tensorflow/third_party/py/numpy/BUILD +tensorflow/third_party/py/BUILD +tensorflow/third_party/py/BUILD.tpl tensorflow/third_party/py/python_configure.bzl -tensorflow/third_party/pybind11.BUILD tensorflow/third_party/python_runtime/BUILD +tensorflow/third_party/pybind11.BUILD tensorflow/third_party/repo.bzl tensorflow/third_party/six.BUILD tensorflow/third_party/snappy.BUILD +tensorflow/third_party/sycl/crosstool/BUILD tensorflow/third_party/sqlite.BUILD tensorflow/third_party/swig.BUILD -tensorflow/third_party/sycl/crosstool/BUILD tensorflow/third_party/systemlibs/BUILD tensorflow/third_party/systemlibs/BUILD.tpl tensorflow/third_party/systemlibs/absl_py.BUILD tensorflow/third_party/systemlibs/absl_py.absl.flags.BUILD tensorflow/third_party/systemlibs/absl_py.absl.testing.BUILD -tensorflow/third_party/systemlibs/boringssl.BUILD tensorflow/third_party/systemlibs/astor.BUILD +tensorflow/third_party/systemlibs/boringssl.BUILD tensorflow/third_party/systemlibs/build_defs.bzl.tpl tensorflow/third_party/systemlibs/curl.BUILD tensorflow/third_party/systemlibs/cython.BUILD @@ -161,9 +161,9 @@ tensorflow/third_party/systemlibs/google_cloud_cpp.BUILD tensorflow/third_party/systemlibs/google_cloud_cpp.google.cloud.bigtable.BUILD tensorflow/third_party/systemlibs/googleapis.BUILD tensorflow/third_party/systemlibs/grpc.BUILD +tensorflow/third_party/systemlibs/jsoncpp.BUILD tensorflow/third_party/systemlibs/lmdb.BUILD tensorflow/third_party/systemlibs/nsync.BUILD -tensorflow/third_party/systemlibs/jsoncpp.BUILD tensorflow/third_party/systemlibs/opt_einsum.BUILD tensorflow/third_party/systemlibs/pcre.BUILD tensorflow/third_party/systemlibs/png.BUILD @@ -171,31 +171,30 @@ tensorflow/third_party/systemlibs/protobuf.BUILD tensorflow/third_party/systemlibs/protobuf.bzl tensorflow/third_party/systemlibs/re2.BUILD tensorflow/third_party/systemlibs/six.BUILD -tensorflow/third_party/systemlibs/swig.BUILD tensorflow/third_party/systemlibs/snappy.BUILD tensorflow/third_party/systemlibs/sqlite.BUILD +tensorflow/third_party/systemlibs/swig.BUILD tensorflow/third_party/systemlibs/syslibs_configure.bzl tensorflow/third_party/systemlibs/termcolor.BUILD tensorflow/third_party/systemlibs/zlib.BUILD tensorflow/third_party/tensorrt/BUILD tensorflow/third_party/tensorrt/BUILD.tpl -tensorflow/third_party/tensorrt/LICENSE tensorflow/third_party/tensorrt/build_defs.bzl.tpl +tensorflow/third_party/tensorrt/LICENSE tensorflow/third_party/tensorrt/tensorrt/include/tensorrt_config.h.tpl tensorflow/third_party/tensorrt/tensorrt_configure.bzl tensorflow/third_party/termcolor.BUILD tensorflow/third_party/tflite_mobilenet.BUILD tensorflow/third_party/tflite_mobilenet_float.BUILD tensorflow/third_party/tflite_mobilenet_quant.BUILD -tensorflow/third_party/tflite_ovic_testdata.BUILD tensorflow/third_party/toolchains/clang6/BUILD -tensorflow/third_party/toolchains/clang6/CROSSTOOL.tpl tensorflow/third_party/toolchains/clang6/README.md +tensorflow/third_party/toolchains/clang6/CROSSTOOL.tpl tensorflow/third_party/toolchains/clang6/clang.BUILD tensorflow/third_party/toolchains/clang6/repo.bzl tensorflow/third_party/toolchains/BUILD -tensorflow/third_party/toolchains/cpus/arm/BUILD tensorflow/third_party/toolchains/cpus/arm/arm_compiler_configure.bzl +tensorflow/third_party/toolchains/cpus/arm/BUILD tensorflow/third_party/toolchains/cpus/arm/cc_config.bzl.tpl tensorflow/third_party/toolchains/cpus/py/BUILD tensorflow/third_party/toolchains/cpus/py3/BUILD @@ -226,13 +225,13 @@ tensorflow/third_party/toolchains/preconfig/ubuntu14.04/gcc-nvcc-cuda10.0/cc_too tensorflow/third_party/toolchains/preconfig/ubuntu14.04/py3/BUILD tensorflow/third_party/toolchains/preconfig/ubuntu14.04/tensorrt5/BUILD tensorflow/third_party/toolchains/preconfig/ubuntu14.04/tensorrt5/build_defs.bzl -tensorflow/third_party/toolchains/preconfig/ubuntu16.04/clang/BUILD tensorflow/third_party/toolchains/preconfig/ubuntu16.04/clang/cc_toolchain_config.bzl +tensorflow/third_party/toolchains/preconfig/ubuntu16.04/clang/BUILD tensorflow/third_party/toolchains/preconfig/ubuntu16.04/clang/dummy_toolchain.bzl -tensorflow/third_party/toolchains/preconfig/ubuntu16.04/cuda10.0-cudnn7/cuda/build_defs.bzl tensorflow/third_party/toolchains/preconfig/ubuntu16.04/cuda10.0-cudnn7/cuda/BUILD -tensorflow/third_party/toolchains/preconfig/ubuntu16.04/gcc5-rocm/cc_toolchain_config.bzl +tensorflow/third_party/toolchains/preconfig/ubuntu16.04/cuda10.0-cudnn7/cuda/build_defs.bzl tensorflow/third_party/toolchains/preconfig/ubuntu16.04/gcc5-rocm/BUILD +tensorflow/third_party/toolchains/preconfig/ubuntu16.04/gcc5-rocm/cc_toolchain_config.bzl tensorflow/third_party/toolchains/preconfig/ubuntu16.04/gcc7_manylinux2010/BUILD tensorflow/third_party/toolchains/preconfig/ubuntu16.04/gcc7_manylinux2010/cc_toolchain_config.bzl tensorflow/third_party/toolchains/preconfig/ubuntu16.04/gcc7_manylinux2010/dummy_toolchain.bzl @@ -241,21 +240,22 @@ tensorflow/third_party/toolchains/preconfig/ubuntu16.04/gcc7_manylinux2010-nvcc- tensorflow/third_party/toolchains/preconfig/ubuntu16.04/py/BUILD tensorflow/third_party/toolchains/preconfig/ubuntu16.04/py3/BUILD tensorflow/third_party/toolchains/preconfig/ubuntu16.04/py3_opt/BUILD -tensorflow/third_party/toolchains/preconfig/ubuntu16.04/rocm/rocm/BUILD tensorflow/third_party/toolchains/preconfig/ubuntu16.04/rocm/rocm/build_defs.bzl +tensorflow/third_party/toolchains/preconfig/ubuntu16.04/rocm/rocm/BUILD tensorflow/third_party/toolchains/preconfig/ubuntu16.04/tensorrt5/BUILD tensorflow/third_party/toolchains/preconfig/ubuntu16.04/tensorrt5.1/BUILD tensorflow/third_party/toolchains/preconfig/ubuntu16.04/tensorrt5.1/build_defs.bzl -tensorflow/third_party/toolchains/preconfig/win_1803/BUILD tensorflow/third_party/toolchains/preconfig/win_1803/bazel_025/BUILD +tensorflow/third_party/toolchains/preconfig/win_1803/BUILD tensorflow/third_party/toolchains/preconfig/win_1803/py36/BUILD tensorflow/third_party/toolchains/remote/BUILD -tensorflow/third_party/toolchains/remote/BUILD.tpl tensorflow/third_party/toolchains/remote/configure.bzl tensorflow/third_party/toolchains/remote/execution.bzl.tpl +tensorflow/third_party/toolchains/remote/BUILD.tpl tensorflow/third_party/tflite_smartreply.BUILD -tensorflow/third_party/wrapt.BUILD +tensorflow/third_party/tflite_ovic_testdata.BUILD tensorflow/third_party/zlib.BUILD +tensorflow/third_party/wrapt.BUILD tensorflow/tools/ci_build/remote/BUILD tensorflow/tools/def_file_filter/BUILD tensorflow/tools/def_file_filter/BUILD.tpl @@ -265,18 +265,18 @@ tensorflow/tools/lib_package/BUILD tensorflow/tools/lib_package/LibTensorFlowTest.java tensorflow/tools/lib_package/README.md tensorflow/tools/lib_package/concat_licenses.sh -tensorflow/tools/lib_package/libtensorflow_java_test.sh tensorflow/tools/lib_package/libtensorflow_test.c +tensorflow/tools/lib_package/libtensorflow_java_test.sh tensorflow/tools/lib_package/libtensorflow_test.sh tensorflow/tools/pip_package/BUILD tensorflow/tools/pip_package/MANIFEST.in tensorflow/tools/pip_package/README tensorflow/tools/pip_package/build_pip_package.sh tensorflow/tools/pip_package/check_load_py_test.py -tensorflow/tools/pip_package/setup.py tensorflow/tools/pip_package/pip_smoke_test.py -tensorflow/tools/pip_package/simple_console.py +tensorflow/tools/pip_package/setup.py tensorflow/tools/pip_package/simple_console_for_windows.py +tensorflow/tools/pip_package/simple_console.py tensorflow/virtual_root_template_v1.__init__.py tensorflow/virtual_root_template_v2.__init__.py llvm/llvm/projects/google_mlir/WORKSPACE \ No newline at end of file diff --git a/third_party/mlir/include/mlir/IR/OpBase.td b/third_party/mlir/include/mlir/IR/OpBase.td index f1349799dc8..1cd61463455 100644 --- a/third_party/mlir/include/mlir/IR/OpBase.td +++ b/third_party/mlir/include/mlir/IR/OpBase.td @@ -1025,6 +1025,8 @@ class PredOpTrait : OpTrait { def Broadcastable : NativeOpTrait<"BroadcastableTwoOperandsOneResult">; // X op Y == Y op X def Commutative : NativeOpTrait<"IsCommutative">; +// Op is isolated from above. +def IsolatedFromAbove : NativeOpTrait<"IsIsolatedFromAbove">; // Op results are float or vectors/tensors thereof. def ResultsAreFloatLike : NativeOpTrait<"ResultsAreFloatLike">; // Op has no side effect. diff --git a/third_party/mlir/include/mlir/IR/OpImplementation.h b/third_party/mlir/include/mlir/IR/OpImplementation.h index 49a53142c60..9f9b12b82dd 100644 --- a/third_party/mlir/include/mlir/IR/OpImplementation.h +++ b/third_party/mlir/include/mlir/IR/OpImplementation.h @@ -411,18 +411,24 @@ public: /// Parses a region. Any parsed blocks are appended to "region" and must be /// moved to the op regions after the op is created. The first block of the - /// region takes "arguments" of types "argTypes". + /// region takes "arguments" of types "argTypes". If "enableNameShadowing" is + /// set to true, the argument names are allowed to shadow the names of other + /// existing SSA values defined above the region scope. "enableNameShadowing" + /// can only be set to true for regions attached to operations that are + /// "IsolatedFromAbove". virtual ParseResult parseRegion(Region ®ion, ArrayRef arguments, - ArrayRef argTypes) = 0; + ArrayRef argTypes, + bool enableNameShadowing = false) = 0; /// Parses a region if present. virtual ParseResult parseOptionalRegion(Region ®ion, ArrayRef arguments, - ArrayRef argTypes) = 0; + ArrayRef argTypes, + bool enableNameShadowing = false) = 0; - /// Parse a region argument. Region arguments define new values; so this also - /// checks if values with the same name have not been defined yet. + /// Parse a region argument, this argument is resolved when calling + /// 'parseRegion'. virtual ParseResult parseRegionArgument(OperandType &argument) = 0; /// Parse zero or more region arguments with a specified surrounding diff --git a/third_party/mlir/lib/Parser/Parser.cpp b/third_party/mlir/lib/Parser/Parser.cpp index 09f30521ef1..4cac1987ffe 100644 --- a/third_party/mlir/lib/Parser/Parser.cpp +++ b/third_party/mlir/lib/Parser/Parser.cpp @@ -2527,7 +2527,7 @@ public: }; /// Push a new SSA name scope to the parser. - void pushSSANameScope(); + void pushSSANameScope(bool isIsolated); /// Pop the last SSA name scope from the parser. ParseResult popSSANameScope(); @@ -2551,12 +2551,12 @@ public: ParseResult parseOptionalSSAUseAndTypeList(SmallVectorImpl &results); /// Return the location of the value identified by its name and number if it - /// has been already defined. Placeholder values are considered undefined. - llvm::Optional getDefinitionLoc(StringRef name, unsigned number) { + /// has been already reference. + llvm::Optional getReferenceLoc(StringRef name, unsigned number) { + auto &values = isolatedNameScopes.back().values; if (!values.count(name) || number >= values[name].size()) return {}; - Value *value = values[name][number].first; - if (value && !isForwardRefPlaceholder(value)) + if (values[name][number].first) return values[name][number].second; return {}; } @@ -2588,8 +2588,11 @@ public: //===--------------------------------------------------------------------===// /// Parse a region into 'region' with the provided entry block arguments. + /// 'isIsolatedNameScope' indicates if the naming scope of this region is + /// isolated from those above. ParseResult parseRegion(Region ®ion, - ArrayRef> entryArguments); + ArrayRef> entryArguments, + bool isIsolatedNameScope = false); /// Parse a region body into 'region'. ParseResult parseRegionBody(Region ®ion); @@ -2633,9 +2636,10 @@ private: bool eraseForwardRef(Block *block) { return forwardRef.back().erase(block); } /// Record that a definition was added at the current scope. - void recordDefinition(StringRef def) { - definitionsPerScope.back().insert(def); - } + void recordDefinition(StringRef def); + + /// Get the value entry for the given SSA name. + SmallVectorImpl> &getSSAValueEntry(StringRef name); /// Create a forward reference placeholder value with the given location and /// result type. @@ -2646,19 +2650,42 @@ private: return forwardRefPlaceholders.count(value); } + /// This struct represents an isolated SSA name scope. This scope may contain + /// other nested non-isolated scopes. These scopes are used for operations + /// that are known to be isolated to allow for reusing names within their + /// regions, even if those names are used above. + struct IsolatedSSANameScope { + /// Record that a definition was added at the current scope. + void recordDefinition(StringRef def) { + definitionsPerScope.back().insert(def); + } + + /// Push a nested name scope. + void pushSSANameScope() { definitionsPerScope.push_back({}); } + + /// Pop a nested name scope. + void popSSANameScope() { + for (auto &def : definitionsPerScope.pop_back_val()) + values.erase(def.getKey()); + } + + /// This keeps track of all of the SSA values we are tracking for each name + /// scope, indexed by their name. This has one entry per result number. + llvm::StringMap, 1>> values; + + /// This keeps track of all of the values defined by a specific name scope. + SmallVector, 2> definitionsPerScope; + }; + + /// A list of isolated name scopes. + SmallVector isolatedNameScopes; + /// This keeps track of the block names as well as the location of the first /// reference for each nested name scope. This is used to diagnose invalid /// block references and memoize them. SmallVector>, 2> blocksByName; SmallVector, 2> forwardRef; - /// This keeps track of all of the SSA values we are tracking for each name - /// scope, indexed by their name. This has one entry per result number. - llvm::StringMap, 1>> values; - - /// This keeps track of all of the values defined by a specific name scope. - SmallVector, 2> definitionsPerScope; - /// These are all of the placeholders we've made along with the location of /// their first reference, to allow checking for use of undefined values. DenseMap forwardRefPlaceholders; @@ -2706,10 +2733,14 @@ ParseResult OperationParser::finalize() { // SSA Value Handling //===----------------------------------------------------------------------===// -void OperationParser::pushSSANameScope() { +void OperationParser::pushSSANameScope(bool isIsolated) { blocksByName.push_back(DenseMap>()); forwardRef.push_back(DenseMap()); - definitionsPerScope.push_back({}); + + // Push back a new name definition scope. + if (isIsolated) + isolatedNameScopes.push_back({}); + isolatedNameScopes.back().pushSSANameScope(); } ParseResult OperationParser::popSSANameScope() { @@ -2733,17 +2764,21 @@ ParseResult OperationParser::popSSANameScope() { return failure(); } - // Drop any values defined in this scope from the value map. - for (auto &def : definitionsPerScope.pop_back_val()) - values.erase(def.getKey()); - blocksByName.pop_back(); + // Pop the next nested namescope. If there is only one internal namescope, + // just pop the isolated scope. + auto ¤tNameScope = isolatedNameScopes.back(); + if (currentNameScope.definitionsPerScope.size() == 1) + isolatedNameScopes.pop_back(); + else + currentNameScope.popSSANameScope(); + blocksByName.pop_back(); return success(); } /// Register a definition of a value with the symbol table. ParseResult OperationParser::addDefinition(SSAUseInfo useInfo, Value *value) { - auto &entries = values[useInfo.name]; + auto &entries = getSSAValueEntry(useInfo.name); // Make sure there is a slot for this value. if (entries.size() <= useInfo.number) @@ -2817,7 +2852,7 @@ ParseResult OperationParser::parseSSAUse(SSAUseInfo &result) { /// Given an unbound reference to an SSA value and its type, return the value /// it specifies. This returns null on failure. Value *OperationParser::resolveSSAUse(SSAUseInfo useInfo, Type type) { - auto &entries = values[useInfo.name]; + auto &entries = getSSAValueEntry(useInfo.name); // If we have already seen a value of this name, return it. if (useInfo.number < entries.size() && entries[useInfo.number].first) { @@ -2906,6 +2941,17 @@ ParseResult OperationParser::parseOptionalSSAUseAndTypeList( return success(); } +/// Record that a definition was added at the current scope. +void OperationParser::recordDefinition(StringRef def) { + isolatedNameScopes.back().recordDefinition(def); +} + +/// Get the value entry for the given SSA name. +SmallVectorImpl> & +OperationParser::getSSAValueEntry(StringRef name) { + return isolatedNameScopes.back().values[name]; +} + /// Create and remember a new placeholder for a forward reference. Value *OperationParser::createForwardRefPlaceholder(SMLoc loc, Type type) { // Forward references are always created as operations, because we just need @@ -3186,22 +3232,15 @@ Operation *OperationParser::parseGenericOperation() { namespace { class CustomOpAsmParser : public OpAsmParser { public: - CustomOpAsmParser(SMLoc nameLoc, StringRef opName, OperationParser &parser) - : nameLoc(nameLoc), opName(opName), parser(parser) {} + CustomOpAsmParser(SMLoc nameLoc, const AbstractOperation *opDefinition, + OperationParser &parser) + : nameLoc(nameLoc), opDefinition(opDefinition), parser(parser) {} /// Parse an instance of the operation described by 'opDefinition' into the /// provided operation state. - ParseResult parseOperation(const AbstractOperation *opDefinition, - OperationState *opState) { + ParseResult parseOperation(OperationState *opState) { if (opDefinition->parseAssembly(this, opState)) return failure(); - - // Check that none of the operands of the current operation reference an - // entry block argument for any of the region. - for (auto *entryArg : parsedRegionEntryArgumentPlaceholders) - if (llvm::is_contained(opState->operands, entryArg)) - return emitError(nameLoc, "operand use before it's defined"); - return success(); } @@ -3215,7 +3254,8 @@ public: /// Emit a diagnostic at the specified location and return failure. InFlightDiagnostic emitError(llvm::SMLoc loc, const Twine &message) override { emittedError = true; - return parser.emitError(loc, "custom op '" + opName + "' " + message); + return parser.emitError(loc, "custom op '" + opDefinition->name + "' " + + message); } llvm::SMLoc getCurrentLocation() override { @@ -3533,7 +3573,8 @@ public: /// Parse a region that takes `arguments` of `argTypes` types. This /// effectively defines the SSA values of `arguments` and assignes their type. ParseResult parseRegion(Region ®ion, ArrayRef arguments, - ArrayRef argTypes) override { + ArrayRef argTypes, + bool enableNameShadowing) override { assert(arguments.size() == argTypes.size() && "mismatching number of arguments and types"); @@ -3545,42 +3586,31 @@ public: OperationParser::SSAUseInfo operandInfo = {operand.name, operand.number, operand.location}; regionArguments.emplace_back(operandInfo, type); - - // Create a placeholder for this argument so that we can detect invalid - // references to region arguments. - Value *value = parser.resolveSSAUse(operandInfo, type); - if (!value) - return failure(); - parsedRegionEntryArgumentPlaceholders.emplace_back(value); } - return parser.parseRegion(region, regionArguments); + // Try to parse the region. + assert((!enableNameShadowing || + opDefinition->hasProperty(OperationProperty::IsolatedFromAbove)) && + "name shadowing is only allowed on isolated regions"); + if (parser.parseRegion(region, regionArguments, enableNameShadowing)) + return failure(); + return success(); } /// Parses a region if present. ParseResult parseOptionalRegion(Region ®ion, ArrayRef arguments, - ArrayRef argTypes) override { + ArrayRef argTypes, + bool enableNameShadowing) override { if (parser.getToken().isNot(Token::l_brace)) return success(); - return parseRegion(region, arguments, argTypes); + return parseRegion(region, arguments, argTypes, enableNameShadowing); } - /// Parse a region argument. Region arguments define new values, so this also - /// checks if the values with the same name has not been defined yet. The - /// type of the argument will be resolved later by a call to `parseRegion`. + /// Parse a region argument. The type of the argument will be resolved later + /// by a call to `parseRegion`. ParseResult parseRegionArgument(OperandType &argument) override { - // Use parseOperand to fill in the OperandType structure. - if (parseOperand(argument)) - return failure(); - if (auto defLoc = parser.getDefinitionLoc(argument.name, argument.number)) { - parser.emitError(argument.location, - "redefinition of SSA value '" + argument.name + "'") - .attachNote(parser.getEncodedSourceLocation(*defLoc)) - << "previously defined here"; - return failure(); - } - return success(); + return parseOperand(argument); } /// Parse a region argument if present. @@ -3649,14 +3679,11 @@ public: } private: - /// A set of placeholder value definitions for parsed region arguments. - SmallVector parsedRegionEntryArgumentPlaceholders; - /// The source location of the operation name. SMLoc nameLoc; - /// The name of the operation. - StringRef opName; + /// The abstract information of the operation. + const AbstractOperation *opDefinition; /// The main operation parser. OperationParser &parser; @@ -3669,7 +3696,6 @@ private: Operation *OperationParser::parseCustomOperation() { auto opLoc = getToken().getLoc(); auto opName = getTokenSpelling(); - CustomOpAsmParser opAsmParser(opLoc, opName, *this); auto *opDefinition = AbstractOperation::lookup(opName, getContext()); if (!opDefinition && !opName.contains('.')) { @@ -3682,7 +3708,7 @@ Operation *OperationParser::parseCustomOperation() { } if (!opDefinition) { - opAsmParser.emitError(opLoc, "is unknown"); + emitError(opLoc) << "custom op '" << opName << "' is unknown"; return nullptr; } @@ -3700,7 +3726,8 @@ Operation *OperationParser::parseCustomOperation() { // Have the op implementation take a crack and parsing this. OperationState opState(srcLocation, opDefinition->name); CleanupOpStateRegions guard{opState}; - if (opAsmParser.parseOperation(opDefinition, &opState)) + CustomOpAsmParser opAsmParser(opLoc, opDefinition, *this); + if (opAsmParser.parseOperation(&opState)) return nullptr; // If it emitted an error, we failed. @@ -3721,7 +3748,8 @@ Operation *OperationParser::parseCustomOperation() { /// ParseResult OperationParser::parseRegion( Region ®ion, - ArrayRef> entryArguments) { + ArrayRef> entryArguments, + bool isIsolatedNameScope) { // Parse the '{'. if (parseToken(Token::l_brace, "expected '{' to begin a region")) return failure(); @@ -3732,19 +3760,28 @@ ParseResult OperationParser::parseRegion( auto currentPt = opBuilder.saveInsertionPoint(); // Push a new named value scope. - pushSSANameScope(); + pushSSANameScope(isIsolatedNameScope); // Parse the first block directly to allow for it to be unnamed. Block *block = new Block(); // Add arguments to the entry block. if (!entryArguments.empty()) { - for (auto &placeholderArgPair : entryArguments) + for (auto &placeholderArgPair : entryArguments) { + auto &argInfo = placeholderArgPair.first; + // Ensure that the argument was not already defined. + if (auto defLoc = getReferenceLoc(argInfo.name, argInfo.number)) { + return emitError(argInfo.loc, "region entry argument '" + argInfo.name + + "' is already in use") + .attachNote(getEncodedSourceLocation(*defLoc)) + << "previously referenced here"; + } if (addDefinition(placeholderArgPair.first, block->addArgument(placeholderArgPair.second))) { delete block; return failure(); } + } // If we had named arguments, then don't allow a block name. if (getToken().is(Token::caret_identifier)) @@ -4008,7 +4045,7 @@ ParseResult ModuleParser::parseModule(ModuleOp module) { OperationParser opParser(getState(), module); // Module itself is a name scope. - opParser.pushSSANameScope(); + opParser.pushSSANameScope(/*isIsolated=*/true); while (1) { switch (getToken().getKind()) { diff --git a/third_party/mlir/test/lib/TestDialect/TestDialect.cpp b/third_party/mlir/test/lib/TestDialect/TestDialect.cpp index f71eff9fd3a..40faa0dccdf 100644 --- a/third_party/mlir/test/lib/TestDialect/TestDialect.cpp +++ b/third_party/mlir/test/lib/TestDialect/TestDialect.cpp @@ -34,10 +34,30 @@ TestDialect::TestDialect(MLIRContext *context) allowUnknownOperations(); } +//===----------------------------------------------------------------------===// +// Test IsolatedRegionOp - parse passthrough region arguments. +//===----------------------------------------------------------------------===// + +static ParseResult parseIsolatedRegionOp(OpAsmParser *parser, + OperationState *result) { + OpAsmParser::OperandType argInfo; + Type argType = parser->getBuilder().getIndexType(); + + // Parse the input operand. + if (parser->parseOperand(argInfo) || + parser->resolveOperand(argInfo, argType, result->operands)) + return failure(); + + // Parse the body region, and reuse the operand info as the argument info. + Region *body = result->addRegion(); + return parser->parseRegion(*body, argInfo, argType, + /*enableNameShadowing=*/true); +} + //===----------------------------------------------------------------------===// // Test PolyForOp - parse list of region arguments. //===----------------------------------------------------------------------===// -ParseResult parsePolyForOp(OpAsmParser *parser, OperationState *result) { +static ParseResult parsePolyForOp(OpAsmParser *parser, OperationState *result) { SmallVector ivsInfo; // Parse list of region arguments without a delimiter. if (parser->parseRegionArgumentList(ivsInfo)) @@ -47,10 +67,7 @@ ParseResult parsePolyForOp(OpAsmParser *parser, OperationState *result) { Region *body = result->addRegion(); auto &builder = parser->getBuilder(); SmallVector argTypes(ivsInfo.size(), builder.getIndexType()); - if (parser->parseRegion(*body, ivsInfo, argTypes)) - return failure(); - - return success(); + return parser->parseRegion(*body, ivsInfo, argTypes); } //===----------------------------------------------------------------------===// diff --git a/third_party/mlir/test/lib/TestDialect/TestOps.td b/third_party/mlir/test/lib/TestDialect/TestOps.td index 8a22adfba88..bf6dae296ab 100644 --- a/third_party/mlir/test/lib/TestDialect/TestOps.td +++ b/third_party/mlir/test/lib/TestDialect/TestOps.td @@ -562,6 +562,18 @@ def TestValidOp : TEST_Op<"valid", [Terminator]>, // Test region argument list parsing. //===----------------------------------------------------------------------===// +def IsolatedRegionOp : TEST_Op<"isolated_region", [IsolatedFromAbove]> { + let summary = "isolated region operation"; + let description = [{ + Test op with an isolated region, to test passthrough region arguments. Each + argument is of index type. + }]; + + let arguments = (ins Index:$input); + let regions = (region SizedRegion<1>:$region); + let parser = [{ return ::parse$cppClass(parser, result); }]; +} + def PolyForOp : TEST_Op<"polyfor"> { let summary = "polyfor operation";