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
This commit is contained in:
River Riddle 2019-08-19 15:26:43 -07:00 committed by A. Unique TensorFlower
parent 36f48063dd
commit 305516fcd3
9 changed files with 194 additions and 104 deletions

View File

@ -1025,6 +1025,8 @@ class PredOpTrait<string descr, Pred pred> : 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.

View File

@ -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 &region,
ArrayRef<OperandType> arguments,
ArrayRef<Type> argTypes) = 0;
ArrayRef<Type> argTypes,
bool enableNameShadowing = false) = 0;
/// Parses a region if present.
virtual ParseResult parseOptionalRegion(Region &region,
ArrayRef<OperandType> arguments,
ArrayRef<Type> argTypes) = 0;
ArrayRef<Type> 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

View File

@ -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<Value *> &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<SMLoc> getDefinitionLoc(StringRef name, unsigned number) {
/// has been already reference.
llvm::Optional<SMLoc> 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 &region,
ArrayRef<std::pair<SSAUseInfo, Type>> entryArguments);
ArrayRef<std::pair<SSAUseInfo, Type>> entryArguments,
bool isIsolatedNameScope = false);
/// Parse a region body into 'region'.
ParseResult parseRegionBody(Region &region);
@ -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<std::pair<Value *, SMLoc>> &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<SmallVector<std::pair<Value *, SMLoc>, 1>> values;
/// This keeps track of all of the values defined by a specific name scope.
SmallVector<llvm::StringSet<>, 2> definitionsPerScope;
};
/// A list of isolated name scopes.
SmallVector<IsolatedSSANameScope, 2> 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<DenseMap<StringRef, std::pair<Block *, SMLoc>>, 2> blocksByName;
SmallVector<DenseMap<Block *, SMLoc>, 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<SmallVector<std::pair<Value *, SMLoc>, 1>> values;
/// This keeps track of all of the values defined by a specific name scope.
SmallVector<llvm::StringSet<>, 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<Value *, SMLoc> forwardRefPlaceholders;
@ -2706,10 +2733,14 @@ ParseResult OperationParser::finalize() {
// SSA Value Handling
//===----------------------------------------------------------------------===//
void OperationParser::pushSSANameScope() {
void OperationParser::pushSSANameScope(bool isIsolated) {
blocksByName.push_back(DenseMap<StringRef, std::pair<Block *, SMLoc>>());
forwardRef.push_back(DenseMap<Block *, SMLoc>());
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 &currentNameScope = 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<std::pair<Value *, SMLoc>> &
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 &region, ArrayRef<OperandType> arguments,
ArrayRef<Type> argTypes) override {
ArrayRef<Type> 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 &region,
ArrayRef<OperandType> arguments,
ArrayRef<Type> argTypes) override {
ArrayRef<Type> 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<Value *, 2> 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 &region,
ArrayRef<std::pair<OperationParser::SSAUseInfo, Type>> entryArguments) {
ArrayRef<std::pair<OperationParser::SSAUseInfo, Type>> 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()) {

View File

@ -103,7 +103,7 @@ func @bad_alloc_wrong_dynamic_dim_count() {
^bb0:
%0 = constant 7 : index
// Test alloc with wrong number of dynamic dimensions.
%1 = alloc(%0)[%1] : memref<2x4xf32, (d0, d1)[s0] -> ((d0 + s0), d1), 1> // expected-error {{custom op 'alloc' dimension operand count does not equal memref dynamic dimension count}}
%1 = alloc(%0)[%1] : memref<2x4xf32, (d0, d1)[s0] -> ((d0 + s0), d1), 1> // expected-error {{op 'std.alloc' dimension operand count does not equal memref dynamic dimension count}}
return
}
@ -165,7 +165,7 @@ func @calls(%arg0: i32) {
func @func_with_ops(f32) {
^bb0(%a : f32):
%sf = addf %a, %a, %a : f32 // expected-error {{custom op 'addf' expected 2 operands}}
%sf = addf %a, %a, %a : f32 // expected-error {{custom op 'std.addf' expected 2 operands}}
}
// -----

View File

@ -199,7 +199,8 @@ func @incomplete_for() {
#map0 = (d0) -> (d0 floordiv 4)
func @reference_to_iv_in_bound() {
// expected-error@+1 {{operand use before it's defined}}
// expected-error@+2 {{region entry argument '%i0' is already in use}}
// expected-note@+1 {{previously referenced here}}
affine.for %i0 = #map0(%i0) to 10 {
}
}
@ -396,8 +397,8 @@ func @undef() {
// -----
func @duplicate_induction_var() {
affine.for %i = 1 to 10 { // expected-note {{previously defined here}}
affine.for %i = 1 to 10 { // expected-error {{redefinition of SSA value '%i'}}
affine.for %i = 1 to 10 { // expected-note {{previously referenced here}}
affine.for %i = 1 to 10 { // expected-error {{region entry argument '%i' is already in use}}
}
}
return
@ -712,9 +713,10 @@ func @f(f32) {
func @f(%m : memref<?x?xf32>) {
affine.for %i0 = 0 to 42 {
// expected-error@+1 {{operand #2 does not dominate this use}}
// expected-note@+1 {{previously referenced here}}
%x = load %m[%i0, %i1] : memref<?x?xf32>
}
// expected-error@+1 {{region entry argument '%i1' is already in use}}
affine.for %i1 = 0 to 42 {
}
return

View File

@ -1036,3 +1036,32 @@ func @special_float_values_in_tensors() {
// CHECK: sparse<[{{\[}}1, 1, 0], [0, 1, 1]], [0xFFFFFFFF, 0x7F800001]>
"foo"(){bar = sparse<[[1,1,0],[0,1,1]], [0xFFFFFFFF, 0x7F800001]> : tensor<2x2x2xf32>} : () -> ()
}
// Test parsing of an op with multiple region arguments, and without a
// delimiter.
// CHECK-LABEL: func @op_with_region_args
func @op_with_region_args() {
// CHECK: "test.polyfor"() ( {
// CHECK-NEXT: ^bb{{.*}}(%{{.*}}: index, %{{.*}}: index, %{{.*}}: index):
test.polyfor %i, %j, %k {
"foo"() : () -> ()
}
return
}
// Test allowing different name scopes for regions isolated from above.
// CHECK-LABEL: func @op_with_passthrough_region_args
func @op_with_passthrough_region_args() {
// CHECK: [[VAL:%.*]] = constant
// CHECK: "test.isolated_region"([[VAL]])
// CHECK-NEXT: ^{{.*}}([[ARG:%.*]]: index)
// CHECK-NEXT: "foo.consumer"([[ARG]]) : (index)
%0 = constant 10 : index
test.isolated_region %0 {
"foo.consumer"(%0) : (index) -> ()
}
return
}

View File

@ -133,18 +133,3 @@ func @fail_to_convert_region() {
}) : () -> ()
return
}
// -----
// Test parsing of an op with multiple region arguments, and without a
// delimiter.
// CHECK-LABEL: func @op_with_region_args
func @op_with_region_args() {
// CHECK: "test.polyfor"() ( {
// CHECK-NEXT: ^bb{{.*}}(%{{.*}}: index, %{{.*}}: index, %{{.*}}: index):
test.polyfor %i, %j, %k {
"foo"() : () -> ()
}
return
}

View File

@ -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<OpAsmParser::OperandType, 4> 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<Type, 4> argTypes(ivsInfo.size(), builder.getIndexType());
if (parser->parseRegion(*body, ivsInfo, argTypes))
return failure();
return success();
return parser->parseRegion(*body, ivsInfo, argTypes);
}
//===----------------------------------------------------------------------===//

View File

@ -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";