[Mlir-commits] [mlir] cb17771 - [mlir] Remove successor operands from the Operation class
River Riddle
llvmlistbot at llvm.org
Thu Mar 5 12:58:56 PST 2020
Author: River Riddle
Date: 2020-03-05T12:53:02-08:00
New Revision: cb1777127c0f4a908c46dbbe8a948d8549427112
URL: https://github.com/llvm/llvm-project/commit/cb1777127c0f4a908c46dbbe8a948d8549427112
DIFF: https://github.com/llvm/llvm-project/commit/cb1777127c0f4a908c46dbbe8a948d8549427112.diff
LOG: [mlir] Remove successor operands from the Operation class
Summary:
This revision removes all of the functionality related to successor operands on the core Operation class. This greatly simplifies a lot of handling of operands, as well as successors. For example, DialectConversion no longer needs a special "matchAndRewrite" for branching terminator operations.(Note, the existing method was also broken for operations with variadic successors!!)
This also enables terminator operations to define their own relationships with successor arguments, instead of the hardcoded "pass-through" behavior that exists today.
Differential Revision: https://reviews.llvm.org/D75318
Added:
Modified:
mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
mlir/include/mlir/Dialect/SPIRV/SPIRVControlFlowOps.td
mlir/include/mlir/Dialect/StandardOps/IR/Ops.td
mlir/include/mlir/IR/OpDefinition.h
mlir/include/mlir/IR/OpImplementation.h
mlir/include/mlir/IR/Operation.h
mlir/include/mlir/IR/OperationSupport.h
mlir/include/mlir/IR/UseDefLists.h
mlir/include/mlir/Transforms/DialectConversion.h
mlir/lib/Conversion/GPUToNVVM/LowerGpuOpsToNVVMOps.cpp
mlir/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp
mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
mlir/lib/Dialect/SPIRV/SPIRVOps.cpp
mlir/lib/IR/AsmPrinter.cpp
mlir/lib/IR/Operation.cpp
mlir/lib/IR/OperationSupport.cpp
mlir/lib/Parser/Parser.cpp
mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
mlir/lib/Transforms/DialectConversion.cpp
mlir/lib/Transforms/Utils/RegionUtils.cpp
mlir/test/Dialect/SPIRV/control-flow-ops.mlir
mlir/test/IR/invalid.mlir
mlir/test/IR/parser.mlir
mlir/test/Transforms/canonicalize-dce.mlir
mlir/test/lib/TestDialect/TestOps.td
mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
mlir/tools/mlir-tblgen/OpFormatGen.cpp
Removed:
################################################################################
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index 2ab9041bc2a0..1132b2728738 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
@@ -70,40 +70,20 @@ class LLVM_ZeroResultOp<string mnemonic, list<OpTrait> traits = []> :
LLVM_Op<mnemonic, traits>, Results<(outs)>,
LLVM_TwoBuilders<LLVM_VoidResultTypeOpBuilder, LLVM_ZeroResultOpBuilder>;
+// Opaque builder used for terminator operations that contain successors.
+def LLVM_TerminatorPassthroughOpBuilder : OpBuilder<
+ "Builder *, OperationState &result, ValueRange operands, "
+ "SuccessorRange destinations, ArrayRef<NamedAttribute> attributes = {}",
+ [{
+ result.addOperands(operands);
+ result.addSuccessors(destinations);
+ result.addAttributes(attributes);
+ }]>;
+
// Base class for LLVM terminator operations. All terminator operations have
// zero results and an optional list of successors.
class LLVM_TerminatorOp<string mnemonic, list<OpTrait> traits = []> :
- LLVM_Op<mnemonic, !listconcat(traits, [Terminator])> {
- let builders = [
- OpBuilder<
- "Builder *, OperationState &result, "
- "ValueRange properOperands, "
- "ArrayRef<Block *> destinations, "
- "ArrayRef<ValueRange> operands, "
- "ArrayRef<NamedAttribute> attributes = {}",
- [{
- result.addOperands(properOperands);
- for (auto kvp : llvm::zip(destinations, operands)) {
- result.addSuccessor(std::get<0>(kvp), std::get<1>(kvp));
- }
- for (auto namedAttr : attributes) {
- result.addAttribute(namedAttr.first, namedAttr.second);
- }
- }]
- >,
- OpBuilder<
- "Builder *builder, OperationState &result, "
- "ValueRange properOperands, "
- "ArrayRef<Block *> destinations, "
- "ArrayRef<NamedAttribute> attributes = {}",
- [{
- SmallVector<ValueRange, 2> operands(destinations.size(), {});
- build(builder, result, properOperands,
- destinations, operands, attributes);
- }]
- >,
- ];
-}
+ LLVM_Op<mnemonic, !listconcat(traits, [Terminator])>;
// Class for arithmetic binary operations.
class LLVM_ArithmeticOp<string mnemonic, string builderFunc,
@@ -317,23 +297,33 @@ def LLVM_FPTruncOp : LLVM_CastOp<"fptrunc", "CreateFPTrunc">;
// Call-related operations.
def LLVM_InvokeOp : LLVM_Op<"invoke", [
- DeclareOpInterfaceMethods<BranchOpInterface>, Terminator
+ AttrSizedOperandSegments,
+ DeclareOpInterfaceMethods<BranchOpInterface>,
+ Terminator
]>,
Arguments<(ins OptionalAttr<FlatSymbolRefAttr>:$callee,
- Variadic<LLVM_Type>)>,
+ Variadic<LLVM_Type>:$operands,
+ Variadic<LLVM_Type>:$normalDestOperands,
+ Variadic<LLVM_Type>:$unwindDestOperands)>,
Results<(outs Variadic<LLVM_Type>)> {
let successors = (successor AnySuccessor:$normalDest,
AnySuccessor:$unwindDest);
let builders = [OpBuilder<
+ "Builder *b, OperationState &result, ArrayRef<Type> tys, "
+ "FlatSymbolRefAttr callee, ValueRange ops, Block* normal, "
+ "ValueRange normalOps, Block* unwind, ValueRange unwindOps",
+ [{
+ result.addAttribute("callee", callee);
+ build(b, result, tys, ops, normal, normalOps, unwind, unwindOps);
+ }]>,
+ OpBuilder<
"Builder *b, OperationState &result, ArrayRef<Type> tys, "
"ValueRange ops, Block* normal, "
"ValueRange normalOps, Block* unwind, ValueRange unwindOps",
[{
- result.addTypes(tys);
- result.addOperands(ops);
- result.addSuccessor(normal, normalOps);
- result.addSuccessor(unwind, unwindOps);
+ build(b, result, tys, /*callee=*/FlatSymbolRefAttr(), ops, normalOps,
+ unwindOps, normal, unwind);
}]>];
let verifier = [{ return ::verify(*this); }];
let parser = [{ return parseInvokeOp(parser, result); }];
@@ -463,14 +453,38 @@ def LLVM_FreezeOp : LLVM_OneResultOp<"freeze", [SameOperandsAndResultType]>,
// Terminators.
def LLVM_BrOp : LLVM_TerminatorOp<"br",
[DeclareOpInterfaceMethods<BranchOpInterface>]> {
+ let arguments = (ins Variadic<LLVM_Type>:$destOperands);
let successors = (successor AnySuccessor:$dest);
- let assemblyFormat = "$dest attr-dict";
+ let assemblyFormat = [{
+ $dest (`(` $destOperands^ `:` type($destOperands) `)`)? attr-dict
+ }];
+ let builders = [LLVM_TerminatorPassthroughOpBuilder];
}
def LLVM_CondBrOp : LLVM_TerminatorOp<"cond_br",
- [DeclareOpInterfaceMethods<BranchOpInterface>]> {
- let arguments = (ins LLVMI1:$condition);
+ [AttrSizedOperandSegments, DeclareOpInterfaceMethods<BranchOpInterface>]> {
+ let arguments = (ins LLVMI1:$condition,
+ Variadic<LLVM_Type>:$trueDestOperands,
+ Variadic<LLVM_Type>:$falseDestOperands);
let successors = (successor AnySuccessor:$trueDest, AnySuccessor:$falseDest);
- let assemblyFormat = "$condition `,` successors attr-dict";
+ let assemblyFormat = [{
+ $condition `,`
+ $trueDest (`(` $trueDestOperands^ `:` type($trueDestOperands) `)`)? `,`
+ $falseDest (`(` $falseDestOperands^ `:` type($falseDestOperands) `)`)?
+ attr-dict
+ }];
+
+ let builders = [OpBuilder<
+ "Builder *builder, OperationState &result, Value condition,"
+ "Block *trueDest, ValueRange trueOperands,"
+ "Block *falseDest, ValueRange falseOperands", [{
+ build(builder, result, condition, trueOperands, falseOperands, trueDest,
+ falseDest);
+ }]>, OpBuilder<
+ "Builder *builder, OperationState &result, Value condition,"
+ "Block *trueDest, Block *falseDest, ValueRange falseOperands = {}", [{
+ build(builder, result, condition, trueDest, ValueRange(), falseDest,
+ falseOperands);
+ }]>, LLVM_TerminatorPassthroughOpBuilder];
}
def LLVM_ReturnOp : LLVM_TerminatorOp<"return", []>,
Arguments<(ins Variadic<LLVM_Type>:$args)> {
diff --git a/mlir/include/mlir/Dialect/SPIRV/SPIRVControlFlowOps.td b/mlir/include/mlir/Dialect/SPIRV/SPIRVControlFlowOps.td
index 5ef825af4ea1..704db7c05bf1 100644
--- a/mlir/include/mlir/Dialect/SPIRV/SPIRVControlFlowOps.td
+++ b/mlir/include/mlir/Dialect/SPIRV/SPIRVControlFlowOps.td
@@ -20,7 +20,7 @@ include "mlir/Analysis/ControlFlowInterfaces.td"
// -----
-def SPV_BranchOp : SPV_Op<"Branch",[
+def SPV_BranchOp : SPV_Op<"Branch", [
DeclareOpInterfaceMethods<BranchOpInterface>, InFunctionScope,
Terminator]> {
let summary = "Unconditional branch to target block.";
@@ -44,7 +44,7 @@ def SPV_BranchOp : SPV_Op<"Branch",[
```
}];
- let arguments = (ins);
+ let arguments = (ins Variadic<SPV_Type>:$targetOperands);
let results = (outs);
@@ -56,7 +56,8 @@ def SPV_BranchOp : SPV_Op<"Branch",[
OpBuilder<
"Builder *, OperationState &state, "
"Block *successor, ValueRange arguments = {}", [{
- state.addSuccessor(successor, arguments);
+ state.addSuccessors(successor);
+ state.addOperands(arguments);
}]
>
];
@@ -73,14 +74,16 @@ def SPV_BranchOp : SPV_Op<"Branch",[
let autogenSerialization = 0;
- let assemblyFormat = "successors attr-dict";
+ let assemblyFormat = [{
+ $target (`(` $targetOperands^ `:` type($targetOperands) `)`)? attr-dict
+ }];
}
// -----
def SPV_BranchConditionalOp : SPV_Op<"BranchConditional", [
- DeclareOpInterfaceMethods<BranchOpInterface>, InFunctionScope,
- Terminator]> {
+ AttrSizedOperandSegments, DeclareOpInterfaceMethods<BranchOpInterface>,
+ InFunctionScope, Terminator]> {
let summary = [{
If Condition is true, branch to true block, otherwise branch to false
block.
@@ -121,13 +124,15 @@ def SPV_BranchConditionalOp : SPV_Op<"BranchConditional", [
let arguments = (ins
SPV_Bool:$condition,
+ Variadic<SPV_Type>:$trueTargetOperands,
+ Variadic<SPV_Type>:$falseTargetOperands,
OptionalAttr<I32ArrayAttr>:$branch_weights
);
let results = (outs);
let successors = (successor AnySuccessor:$trueTarget,
- AnySuccessor:$falseTarget);
+ AnySuccessor:$falseTarget);
let builders = [
OpBuilder<
@@ -136,21 +141,18 @@ def SPV_BranchConditionalOp : SPV_Op<"BranchConditional", [
"Block *falseBlock, ValueRange falseArguments, "
"Optional<std::pair<uint32_t, uint32_t>> weights = {}",
[{
- state.addOperands(condition);
- state.addSuccessor(trueBlock, trueArguments);
- state.addSuccessor(falseBlock, falseArguments);
+ ArrayAttr weightsAttr;
if (weights) {
- auto attr =
+ weightsAttr =
builder->getI32ArrayAttr({static_cast<int32_t>(weights->first),
static_cast<int32_t>(weights->second)});
- state.addAttribute("branch_weights", attr);
}
+ build(builder, state, condition, trueArguments, falseArguments,
+ weightsAttr, trueBlock, falseBlock);
}]
>
];
- let skipDefaultBuilders = 1;
-
let autogenSerialization = 0;
let extraClassDeclaration = [{
@@ -165,34 +167,22 @@ def SPV_BranchConditionalOp : SPV_Op<"BranchConditional", [
/// Returns the number of arguments to the true target block.
unsigned getNumTrueBlockArguments() {
- return getNumSuccessorOperands(kTrueIndex);
+ return trueTargetOperands().size();
}
/// Returns the number of arguments to the false target block.
unsigned getNumFalseBlockArguments() {
- return getNumSuccessorOperands(kFalseIndex);
+ return falseTargetOperands().size();
}
// Iterator and range support for true target block arguments.
- operand_iterator true_block_argument_begin() {
- return operand_begin() + getTrueBlockArgumentIndex();
- }
- operand_iterator true_block_argument_end() {
- return true_block_argument_begin() + getNumTrueBlockArguments();
- }
operand_range getTrueBlockArguments() {
- return {true_block_argument_begin(), true_block_argument_end()};
+ return trueTargetOperands();
}
// Iterator and range support for false target block arguments.
- operand_iterator false_block_argument_begin() {
- return true_block_argument_end();
- }
- operand_iterator false_block_argument_end() {
- return false_block_argument_begin() + getNumFalseBlockArguments();
- }
operand_range getFalseBlockArguments() {
- return {false_block_argument_begin(), false_block_argument_end()};
+ return falseTargetOperands();
}
private:
diff --git a/mlir/include/mlir/Dialect/StandardOps/IR/Ops.td b/mlir/include/mlir/Dialect/StandardOps/IR/Ops.td
index 1d312202f7b6..e6e28e8d00a6 100644
--- a/mlir/include/mlir/Dialect/StandardOps/IR/Ops.td
+++ b/mlir/include/mlir/Dialect/StandardOps/IR/Ops.td
@@ -347,10 +347,13 @@ def BranchOp : Std_Op<"br",
^bb3(%3: tensor<*xf32>):
}];
+ let arguments = (ins Variadic<AnyType>:$destOperands);
let successors = (successor AnySuccessor:$dest);
- let builders = [OpBuilder<"Builder *, OperationState &result, Block *dest", [{
- result.addSuccessor(dest, llvm::None);
+ let builders = [OpBuilder<"Builder *, OperationState &result, Block *dest, "
+ "ValueRange destOperands = {}", [{
+ result.addSuccessors(dest);
+ result.addOperands(destOperands);
}]>];
// BranchOp is fully verified by traits.
@@ -365,7 +368,9 @@ def BranchOp : Std_Op<"br",
}];
let hasCanonicalizer = 1;
- let assemblyFormat = "$dest attr-dict";
+ let assemblyFormat = [{
+ $dest (`(` $destOperands^ `:` type($destOperands) `)`)? attr-dict
+ }];
}
//===----------------------------------------------------------------------===//
@@ -671,7 +676,8 @@ def CmpIOp : Std_Op<"cmpi",
//===----------------------------------------------------------------------===//
def CondBranchOp : Std_Op<"cond_br",
- [DeclareOpInterfaceMethods<BranchOpInterface>, Terminator]> {
+ [AttrSizedOperandSegments, DeclareOpInterfaceMethods<BranchOpInterface>,
+ Terminator]> {
let summary = "conditional branch operation";
let description = [{
The "cond_br" operation represents a conditional branch operation in a
@@ -688,9 +694,24 @@ def CondBranchOp : Std_Op<"cond_br",
...
}];
- let arguments = (ins I1:$condition);
+ let arguments = (ins I1:$condition,
+ Variadic<AnyType>:$trueDestOperands,
+ Variadic<AnyType>:$falseDestOperands);
let successors = (successor AnySuccessor:$trueDest, AnySuccessor:$falseDest);
+ let builders = [OpBuilder<
+ "Builder *builder, OperationState &result, Value condition,"
+ "Block *trueDest, ValueRange trueOperands,"
+ "Block *falseDest, ValueRange falseOperands", [{
+ build(builder, result, condition, trueOperands, falseOperands, trueDest,
+ falseDest);
+ }]>, OpBuilder<
+ "Builder *builder, OperationState &result, Value condition,"
+ "Block *trueDest, Block *falseDest, ValueRange falseOperands = {}", [{
+ build(builder, result, condition, trueDest, ValueRange(), falseDest,
+ falseOperands);
+ }]>];
+
// CondBranchOp is fully verified by traits.
let verifier = ?;
@@ -722,23 +743,13 @@ def CondBranchOp : Std_Op<"cond_br",
setOperand(getTrueDestOperandIndex() + idx, value);
}
- operand_iterator true_operand_begin() {
- return operand_begin() + getTrueDestOperandIndex();
- }
- operand_iterator true_operand_end() {
- return true_operand_begin() + getNumTrueOperands();
- }
- operand_range getTrueOperands() {
- return {true_operand_begin(), true_operand_end()};
- }
+ operand_range getTrueOperands() { return trueDestOperands(); }
- unsigned getNumTrueOperands() {
- return getNumSuccessorOperands(trueIndex);
- }
+ unsigned getNumTrueOperands() { return getTrueOperands().size(); }
/// Erase the operand at 'index' from the true operand list.
void eraseTrueOperand(unsigned index) {
- getOperation()->eraseSuccessorOperand(trueIndex, index);
+ eraseSuccessorOperand(trueIndex, index);
}
// Accessors for operands to the 'false' destination.
@@ -751,21 +762,13 @@ def CondBranchOp : Std_Op<"cond_br",
setOperand(getFalseDestOperandIndex() + idx, value);
}
- operand_iterator false_operand_begin() { return true_operand_end(); }
- operand_iterator false_operand_end() {
- return false_operand_begin() + getNumFalseOperands();
- }
- operand_range getFalseOperands() {
- return {false_operand_begin(), false_operand_end()};
- }
+ operand_range getFalseOperands() { return falseDestOperands(); }
- unsigned getNumFalseOperands() {
- return getNumSuccessorOperands(falseIndex);
- }
+ unsigned getNumFalseOperands() { return getFalseOperands().size(); }
/// Erase the operand at 'index' from the false operand list.
void eraseFalseOperand(unsigned index) {
- getOperation()->eraseSuccessorOperand(falseIndex, index);
+ eraseSuccessorOperand(falseIndex, index);
}
private:
@@ -779,7 +782,12 @@ def CondBranchOp : Std_Op<"cond_br",
}];
let hasCanonicalizer = 1;
- let assemblyFormat = "$condition `,` successors attr-dict";
+ let assemblyFormat = [{
+ $condition `,`
+ $trueDest (`(` $trueDestOperands^ `:` type($trueDestOperands) `)`)? `,`
+ $falseDest (`(` $falseDestOperands^ `:` type($falseDestOperands) `)`)?
+ attr-dict
+ }];
}
//===----------------------------------------------------------------------===//
diff --git a/mlir/include/mlir/IR/OpDefinition.h b/mlir/include/mlir/IR/OpDefinition.h
index 1a8132178a50..efbe31bad599 100644
--- a/mlir/include/mlir/IR/OpDefinition.h
+++ b/mlir/include/mlir/IR/OpDefinition.h
@@ -668,10 +668,6 @@ class IsTerminator : public TraitBase<ConcreteType, IsTerminator> {
static LogicalResult verifyTrait(Operation *op) {
return impl::verifyIsTerminator(op);
}
-
- unsigned getNumSuccessorOperands(unsigned index) {
- return this->getOperation()->getNumSuccessorOperands(index);
- }
};
/// This class provides verification for ops that are known to have zero
diff --git a/mlir/include/mlir/IR/OpImplementation.h b/mlir/include/mlir/IR/OpImplementation.h
index 54ecf8cde39c..dbd7b0b0b982 100644
--- a/mlir/include/mlir/IR/OpImplementation.h
+++ b/mlir/include/mlir/IR/OpImplementation.h
@@ -62,9 +62,12 @@ class OpAsmPrinter {
/// provide a valid type for the attribute.
virtual void printAttributeWithoutType(Attribute attr) = 0;
- /// Print a successor, and use list, of a terminator operation given the
- /// terminator and the successor index.
- virtual void printSuccessorAndUseList(Operation *term, unsigned index) = 0;
+ /// Print the given successor.
+ virtual void printSuccessor(Block *successor) = 0;
+
+ /// Print the successor and its operands.
+ virtual void printSuccessorAndUseList(Block *successor,
+ ValueRange succOperands) = 0;
/// If the specified operation has attributes, print out an attribute
/// dictionary with their values. elidedAttrs allows the client to ignore
@@ -120,8 +123,7 @@ class OpAsmPrinter {
/// Print the complete type of an operation in functional form.
void printFunctionalType(Operation *op) {
- printFunctionalType(op->getNonSuccessorOperands().getTypes(),
- op->getResultTypes());
+ printFunctionalType(op->getOperandTypes(), op->getResultTypes());
}
/// Print the two given type ranges in a functional form.
template <typename InputRangeT, typename ResultRangeT>
@@ -188,6 +190,11 @@ inline OpAsmPrinter &operator<<(OpAsmPrinter &p, bool value) {
return p << (value ? StringRef("true") : "false");
}
+inline OpAsmPrinter &operator<<(OpAsmPrinter &p, Block *value) {
+ p.printSuccessor(value);
+ return p;
+}
+
template <typename ValueRangeT>
inline OpAsmPrinter &operator<<(OpAsmPrinter &p,
const ValueTypeRange<ValueRangeT> &types) {
@@ -574,15 +581,16 @@ class OpAsmParser {
// Successor Parsing
//===--------------------------------------------------------------------===//
+ /// Parse a single operation successor.
+ virtual ParseResult parseSuccessor(Block *&dest) = 0;
+
+ /// Parse an optional operation successor.
+ virtual OptionalParseResult parseOptionalSuccessor(Block *&dest) = 0;
+
/// Parse a single operation successor and its operand list.
virtual ParseResult
parseSuccessorAndUseList(Block *&dest, SmallVectorImpl<Value> &operands) = 0;
- /// Parse an optional operation successor and its operand list.
- virtual OptionalParseResult
- parseOptionalSuccessorAndUseList(Block *&dest,
- SmallVectorImpl<Value> &operands) = 0;
-
//===--------------------------------------------------------------------===//
// Type Parsing
//===--------------------------------------------------------------------===//
diff --git a/mlir/include/mlir/IR/Operation.h b/mlir/include/mlir/IR/Operation.h
index b2f8a7a109fc..781dbcd3eb0d 100644
--- a/mlir/include/mlir/IR/Operation.h
+++ b/mlir/include/mlir/IR/Operation.h
@@ -374,7 +374,7 @@ class Operation final
}
//===--------------------------------------------------------------------===//
- // Terminators
+ // Successors
//===--------------------------------------------------------------------===//
MutableArrayRef<BlockOperand> getBlockOperands() {
@@ -387,24 +387,8 @@ class Operation final
succ_iterator successor_end() { return getSuccessors().end(); }
SuccessorRange getSuccessors() { return SuccessorRange(this); }
- /// Return the operands of this operation that are *not* successor arguments.
- operand_range getNonSuccessorOperands();
-
- operand_range getSuccessorOperands(unsigned index);
-
- Value getSuccessorOperand(unsigned succIndex, unsigned opIndex) {
- assert(!isKnownNonTerminator() && "only terminators may have successors");
- assert(opIndex < getNumSuccessorOperands(succIndex));
- return getOperand(getSuccessorOperandIndex(succIndex) + opIndex);
- }
-
bool hasSuccessors() { return numSuccs != 0; }
unsigned getNumSuccessors() { return numSuccs; }
- unsigned getNumSuccessorOperands(unsigned index) {
- assert(!isKnownNonTerminator() && "only terminators may have successors");
- assert(index < getNumSuccessors());
- return getBlockOperands()[index].numSuccessorOperands;
- }
Block *getSuccessor(unsigned index) {
assert(index < getNumSuccessors());
@@ -412,37 +396,6 @@ class Operation final
}
void setSuccessor(Block *block, unsigned index);
- /// Erase a specific operand from the operand list of the successor at
- /// 'index'.
- void eraseSuccessorOperand(unsigned succIndex, unsigned opIndex) {
- assert(succIndex < getNumSuccessors());
- assert(opIndex < getNumSuccessorOperands(succIndex));
- getOperandStorage().eraseOperand(getSuccessorOperandIndex(succIndex) +
- opIndex);
- --getBlockOperands()[succIndex].numSuccessorOperands;
- }
-
- /// Get the index of the first operand of the successor at the provided
- /// index.
- unsigned getSuccessorOperandIndex(unsigned index);
-
- /// Return a pair (successorIndex, successorArgIndex) containing the index
- /// of the successor that `operandIndex` belongs to and the index of the
- /// argument to that successor that `operandIndex` refers to.
- ///
- /// If `operandIndex` is not a successor operand, None is returned.
- Optional<std::pair<unsigned, unsigned>>
- decomposeSuccessorOperandIndex(unsigned operandIndex);
-
- /// Returns the `BlockArgument` corresponding to operand `operandIndex` in
- /// some successor, or None if `operandIndex` isn't a successor operand index.
- Optional<BlockArgument> getSuccessorBlockArgument(unsigned operandIndex) {
- auto decomposed = decomposeSuccessorOperandIndex(operandIndex);
- if (!decomposed.hasValue())
- return None;
- return getSuccessor(decomposed->first)->getArgument(decomposed->second);
- }
-
//===--------------------------------------------------------------------===//
// Accessors for various properties of operations
//===--------------------------------------------------------------------===//
diff --git a/mlir/include/mlir/IR/OperationSupport.h b/mlir/include/mlir/IR/OperationSupport.h
index 3bbacd5b205a..2d4a96823d48 100644
--- a/mlir/include/mlir/IR/OperationSupport.h
+++ b/mlir/include/mlir/IR/OperationSupport.h
@@ -40,6 +40,7 @@ class Pattern;
class Region;
class ResultRange;
class RewritePattern;
+class SuccessorRange;
class Type;
class Value;
class ValueRange;
@@ -316,7 +317,12 @@ struct OperationState {
attributes.append(newAttributes.begin(), newAttributes.end());
}
- void addSuccessor(Block *successor, ValueRange succOperands);
+ /// Add an array of successors.
+ void addSuccessors(ArrayRef<Block *> newSuccessors) {
+ successors.append(newSuccessors.begin(), newSuccessors.end());
+ }
+ void addSuccessors(Block *successor) { successors.push_back(successor); }
+ void addSuccessors(SuccessorRange newSuccessors);
/// Create a region that should be attached to the operation. These regions
/// can be filled in immediately without waiting for Operation to be
diff --git a/mlir/include/mlir/IR/UseDefLists.h b/mlir/include/mlir/IR/UseDefLists.h
index f4433384106c..2375f6adc81d 100644
--- a/mlir/include/mlir/IR/UseDefLists.h
+++ b/mlir/include/mlir/IR/UseDefLists.h
@@ -295,13 +295,6 @@ class BlockOperand : public IROperand<BlockOperand, Block *> {
/// Return which operand this is in the operand list of the User.
unsigned getOperandNumber();
-
-private:
- /// The number of OpOperands that correspond with this block operand.
- unsigned numSuccessorOperands = 0;
-
- /// Allow access to 'numSuccessorOperands'.
- friend Operation;
};
//===----------------------------------------------------------------------===//
diff --git a/mlir/include/mlir/Transforms/DialectConversion.h b/mlir/include/mlir/Transforms/DialectConversion.h
index cbba8209d402..48ce470b88dd 100644
--- a/mlir/include/mlir/Transforms/DialectConversion.h
+++ b/mlir/include/mlir/Transforms/DialectConversion.h
@@ -227,44 +227,13 @@ class ConversionPattern : public RewritePattern {
/// Hook for derived classes to implement rewriting. `op` is the (first)
/// operation matched by the pattern, `operands` is a list of rewritten values
/// that are passed to this operation, `rewriter` can be used to emit the new
- /// operations. This function must be reimplemented if the
- /// ConversionPattern ever needs to replace an operation that does not
- /// have successors. This function should not fail. If some specific cases of
+ /// operations. This function should not fail. If some specific cases of
/// the operation are not supported, these cases should not be matched.
virtual void rewrite(Operation *op, ArrayRef<Value> operands,
ConversionPatternRewriter &rewriter) const {
llvm_unreachable("unimplemented rewrite");
}
- /// Hook for derived classes to implement rewriting. `op` is the (first)
- /// operation matched by the pattern, `properOperands` is a list of rewritten
- /// values that are passed to the operation itself, `destinations` is a list
- /// of (potentially rewritten) successor blocks, `operands` is a list of lists
- /// of rewritten values passed to each of the successors, co-indexed with
- /// `destinations`, `rewriter` can be used to emit the new operations. It must
- /// be reimplemented if the ConversionPattern ever needs to replace a
- /// terminator operation that has successors. This function should not fail
- /// the pass. If some specific cases of the operation are not supported,
- /// these cases should not be matched.
- virtual void rewrite(Operation *op, ArrayRef<Value> properOperands,
- ArrayRef<Block *> destinations,
- ArrayRef<ArrayRef<Value>> operands,
- ConversionPatternRewriter &rewriter) const {
- llvm_unreachable("unimplemented rewrite for terminators");
- }
-
- /// Hook for derived classes to implement combined matching and rewriting.
- virtual PatternMatchResult
- matchAndRewrite(Operation *op, ArrayRef<Value> properOperands,
- ArrayRef<Block *> destinations,
- ArrayRef<ArrayRef<Value>> operands,
- ConversionPatternRewriter &rewriter) const {
- if (!match(op))
- return matchFailure();
- rewrite(op, properOperands, destinations, operands, rewriter);
- return matchSuccess();
- }
-
/// Hook for derived classes to implement combined matching and rewriting.
virtual PatternMatchResult
matchAndRewrite(Operation *op, ArrayRef<Value> operands,
@@ -297,21 +266,6 @@ struct OpConversionPattern : public ConversionPattern {
ConversionPatternRewriter &rewriter) const final {
rewrite(cast<SourceOp>(op), operands, rewriter);
}
- void rewrite(Operation *op, ArrayRef<Value> properOperands,
- ArrayRef<Block *> destinations,
- ArrayRef<ArrayRef<Value>> operands,
- ConversionPatternRewriter &rewriter) const final {
- rewrite(cast<SourceOp>(op), properOperands, destinations, operands,
- rewriter);
- }
- PatternMatchResult
- matchAndRewrite(Operation *op, ArrayRef<Value> properOperands,
- ArrayRef<Block *> destinations,
- ArrayRef<ArrayRef<Value>> operands,
- ConversionPatternRewriter &rewriter) const final {
- return matchAndRewrite(cast<SourceOp>(op), properOperands, destinations,
- operands, rewriter);
- }
PatternMatchResult
matchAndRewrite(Operation *op, ArrayRef<Value> operands,
ConversionPatternRewriter &rewriter) const final {
@@ -328,24 +282,6 @@ struct OpConversionPattern : public ConversionPattern {
llvm_unreachable("must override matchAndRewrite or a rewrite method");
}
- virtual void rewrite(SourceOp op, ArrayRef<Value> properOperands,
- ArrayRef<Block *> destinations,
- ArrayRef<ArrayRef<Value>> operands,
- ConversionPatternRewriter &rewriter) const {
- llvm_unreachable("unimplemented rewrite for terminators");
- }
-
- virtual PatternMatchResult
- matchAndRewrite(SourceOp op, ArrayRef<Value> properOperands,
- ArrayRef<Block *> destinations,
- ArrayRef<ArrayRef<Value>> operands,
- ConversionPatternRewriter &rewriter) const {
- if (!match(op))
- return matchFailure();
- rewrite(op, properOperands, destinations, operands, rewriter);
- return matchSuccess();
- }
-
virtual PatternMatchResult
matchAndRewrite(SourceOp op, ArrayRef<Value> operands,
ConversionPatternRewriter &rewriter) const {
diff --git a/mlir/lib/Conversion/GPUToNVVM/LowerGpuOpsToNVVMOps.cpp b/mlir/lib/Conversion/GPUToNVVM/LowerGpuOpsToNVVMOps.cpp
index 3c2d964d2526..b14968a5e663 100644
--- a/mlir/lib/Conversion/GPUToNVVM/LowerGpuOpsToNVVMOps.cpp
+++ b/mlir/lib/Conversion/GPUToNVVM/LowerGpuOpsToNVVMOps.cpp
@@ -90,8 +90,7 @@ struct GPUAllReduceOpLowering : public ConvertToLLVMPattern {
// Add branch before inserted body, into body.
block = block->getNextNode();
- rewriter.create<LLVM::BrOp>(loc, ArrayRef<Value>{},
- llvm::makeArrayRef(block), ValueRange());
+ rewriter.create<LLVM::BrOp>(loc, ValueRange(), block);
// Replace all gpu.yield ops with branch out of body.
for (; block != split; block = block->getNextNode()) {
@@ -100,8 +99,7 @@ struct GPUAllReduceOpLowering : public ConvertToLLVMPattern {
continue;
rewriter.setInsertionPointToEnd(block);
rewriter.replaceOpWithNewOp<LLVM::BrOp>(
- terminator, ArrayRef<Value>{}, llvm::makeArrayRef(split),
- ValueRange(terminator->getOperand(0)));
+ terminator, terminator->getOperand(0), split);
}
// Return accumulator result.
@@ -254,13 +252,10 @@ struct GPUAllReduceOpLowering : public ConvertToLLVMPattern {
Block *continueBlock = rewriter.splitBlock(elseBlock, elseBlock->begin());
rewriter.setInsertionPointToEnd(currentBlock);
- rewriter.create<LLVM::CondBrOp>(loc, llvm::makeArrayRef(condition),
- ArrayRef<Block *>{thenBlock, elseBlock});
+ rewriter.create<LLVM::CondBrOp>(loc, condition, thenBlock, elseBlock);
auto addBranch = [&](ValueRange operands) {
- rewriter.create<LLVM::BrOp>(loc, ArrayRef<Value>{},
- llvm::makeArrayRef(continueBlock),
- llvm::makeArrayRef(operands));
+ rewriter.create<LLVM::BrOp>(loc, operands, continueBlock);
};
rewriter.setInsertionPointToStart(thenBlock);
@@ -645,8 +640,7 @@ struct GPUReturnOpLowering : public ConvertToLLVMPattern {
PatternMatchResult
matchAndRewrite(Operation *op, ArrayRef<Value> operands,
ConversionPatternRewriter &rewriter) const override {
- rewriter.replaceOpWithNewOp<LLVM::ReturnOp>(op, operands,
- ArrayRef<Block *>());
+ rewriter.replaceOpWithNewOp<LLVM::ReturnOp>(op, operands);
return matchSuccess();
}
};
diff --git a/mlir/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp b/mlir/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp
index 3113f8ae46f1..e38322943cfe 100644
--- a/mlir/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp
+++ b/mlir/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp
@@ -2185,13 +2185,10 @@ struct OneToOneLLVMTerminatorLowering
using Super = OneToOneLLVMTerminatorLowering<SourceOp, TargetOp>;
PatternMatchResult
- matchAndRewrite(Operation *op, ArrayRef<Value> properOperands,
- ArrayRef<Block *> destinations,
- ArrayRef<ArrayRef<Value>> operands,
+ matchAndRewrite(Operation *op, ArrayRef<Value> operands,
ConversionPatternRewriter &rewriter) const override {
- SmallVector<ValueRange, 2> operandRanges(operands.begin(), operands.end());
- rewriter.replaceOpWithNewOp<TargetOp>(op, properOperands, destinations,
- operandRanges, op->getAttrs());
+ rewriter.replaceOpWithNewOp<TargetOp>(op, operands, op->getSuccessors(),
+ op->getAttrs());
return this->matchSuccess();
}
};
@@ -2213,13 +2210,12 @@ struct ReturnOpLowering : public LLVMLegalizationPattern<ReturnOp> {
// If ReturnOp has 0 or 1 operand, create it and return immediately.
if (numArguments == 0) {
rewriter.replaceOpWithNewOp<LLVM::ReturnOp>(
- op, ArrayRef<Value>(), ArrayRef<Block *>(), op->getAttrs());
+ op, ArrayRef<Type>(), ArrayRef<Value>(), op->getAttrs());
return matchSuccess();
}
if (numArguments == 1) {
rewriter.replaceOpWithNewOp<LLVM::ReturnOp>(
- op, ArrayRef<Value>(operands.front()), ArrayRef<Block *>(),
- op->getAttrs());
+ op, ArrayRef<Type>(), operands.front(), op->getAttrs());
return matchSuccess();
}
@@ -2234,8 +2230,8 @@ struct ReturnOpLowering : public LLVMLegalizationPattern<ReturnOp> {
op->getLoc(), packedType, packed, operands[i],
rewriter.getI64ArrayAttr(i));
}
- rewriter.replaceOpWithNewOp<LLVM::ReturnOp>(
- op, llvm::makeArrayRef(packed), ArrayRef<Block *>(), op->getAttrs());
+ rewriter.replaceOpWithNewOp<LLVM::ReturnOp>(op, ArrayRef<Type>(), packed,
+ op->getAttrs());
return matchSuccess();
}
};
@@ -2742,10 +2738,8 @@ struct AtomicCmpXchgOpLowering : public LoadStoreOpLowering<AtomicRMWOp> {
auto memRefType = atomicOp.getMemRefType();
auto dataPtr = getDataPtr(loc, memRefType, adaptor.memref(),
adaptor.indices(), rewriter, getModule());
- auto init = rewriter.create<LLVM::LoadOp>(loc, dataPtr);
- std::array<Value, 1> brRegionOperands{init};
- std::array<ValueRange, 1> brOperands{brRegionOperands};
- rewriter.create<LLVM::BrOp>(loc, ArrayRef<Value>{}, loopBlock, brOperands);
+ Value init = rewriter.create<LLVM::LoadOp>(loc, dataPtr);
+ rewriter.create<LLVM::BrOp>(loc, init, loopBlock);
// Prepare the body of the loop block.
rewriter.setInsertionPointToStart(loopBlock);
@@ -2768,19 +2762,14 @@ struct AtomicCmpXchgOpLowering : public LoadStoreOpLowering<AtomicRMWOp> {
loc, pairType, dataPtr, loopArgument, select, successOrdering,
failureOrdering);
// Extract the %new_loaded and %ok values from the pair.
- auto newLoaded = rewriter.create<LLVM::ExtractValueOp>(
+ Value newLoaded = rewriter.create<LLVM::ExtractValueOp>(
loc, valueType, cmpxchg, rewriter.getI64ArrayAttr({0}));
- auto ok = rewriter.create<LLVM::ExtractValueOp>(
+ Value ok = rewriter.create<LLVM::ExtractValueOp>(
loc, boolType, cmpxchg, rewriter.getI64ArrayAttr({1}));
// Conditionally branch to the end or back to the loop depending on %ok.
- std::array<Value, 1> condBrProperOperands{ok};
- std::array<Block *, 2> condBrDestinations{endBlock, loopBlock};
- std::array<Value, 1> condBrRegionOperands{newLoaded};
- std::array<ValueRange, 2> condBrOperands{ArrayRef<Value>{},
- condBrRegionOperands};
- rewriter.create<LLVM::CondBrOp>(loc, condBrProperOperands,
- condBrDestinations, condBrOperands);
+ rewriter.create<LLVM::CondBrOp>(loc, ok, endBlock, ArrayRef<Value>(),
+ loopBlock, newLoaded);
// The 'result' of the atomic_rmw op is the newly loaded value.
rewriter.replaceOp(op, {newLoaded});
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index 567ddee94d7d..7a15d579127d 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -250,9 +250,9 @@ static ParseResult parseStoreOp(OpAsmParser &parser, OperationState &result) {
return success();
}
-///===----------------------------------------------------------------------===//
+///===---------------------------------------------------------------------===//
/// LLVM::InvokeOp
-///===----------------------------------------------------------------------===//
+///===---------------------------------------------------------------------===//
Optional<OperandRange> InvokeOp::getSuccessorOperands(unsigned index) {
assert(index < getNumSuccessors() && "invalid successor index");
@@ -278,7 +278,7 @@ static LogicalResult verify(InvokeOp op) {
return success();
}
-static void printInvokeOp(OpAsmPrinter &p, InvokeOp &op) {
+static void printInvokeOp(OpAsmPrinter &p, InvokeOp op) {
auto callee = op.callee();
bool isDirect = callee.hasValue();
@@ -292,17 +292,16 @@ static void printInvokeOp(OpAsmPrinter &p, InvokeOp &op) {
p << '(' << op.getOperands().drop_front(isDirect ? 0 : 1) << ')';
p << " to ";
- p.printSuccessorAndUseList(op.getOperation(), 0);
+ p.printSuccessorAndUseList(op.normalDest(), op.normalDestOperands());
p << " unwind ";
- p.printSuccessorAndUseList(op.getOperation(), 1);
-
- p.printOptionalAttrDict(op.getAttrs(), {"callee"});
-
- SmallVector<Type, 8> argTypes(
- llvm::drop_begin(op.getOperandTypes(), isDirect ? 0 : 1));
+ p.printSuccessorAndUseList(op.unwindDest(), op.unwindDestOperands());
- p << " : "
- << FunctionType::get(argTypes, op.getResultTypes(), op.getContext());
+ p.printOptionalAttrDict(op.getAttrs(),
+ {InvokeOp::getOperandSegmentSizeAttr(), "callee"});
+ p << " : ";
+ p.printFunctionalType(
+ llvm::drop_begin(op.getOperandTypes(), isDirect ? 0 : 1),
+ op.getResultTypes());
}
/// <operation> ::= `llvm.invoke` (function-id | ssa-use) `(` ssa-use-list `)`
@@ -316,6 +315,7 @@ static ParseResult parseInvokeOp(OpAsmParser &parser, OperationState &result) {
llvm::SMLoc trailingTypeLoc;
Block *normalDest, *unwindDest;
SmallVector<Value, 4> normalOperands, unwindOperands;
+ Builder &builder = parser.getBuilder();
// Parse an operand list that will, in practice, contain 0 or 1 operand. In
// case of an indirect call, there will be 1 operand before `(`. In case of a
@@ -351,7 +351,6 @@ static ParseResult parseInvokeOp(OpAsmParser &parser, OperationState &result) {
return parser.emitError(trailingTypeLoc,
"expected function with 0 or 1 result");
- Builder &builder = parser.getBuilder();
auto *llvmDialect =
builder.getContext()->getRegisteredDialect<LLVM::LLVMDialect>();
LLVM::LLVMType llvmResultType;
@@ -390,8 +389,15 @@ static ParseResult parseInvokeOp(OpAsmParser &parser, OperationState &result) {
result.addTypes(llvmResultType);
}
- result.addSuccessor(normalDest, normalOperands);
- result.addSuccessor(unwindDest, unwindOperands);
+ result.addSuccessors({normalDest, unwindDest});
+ result.addOperands(normalOperands);
+ result.addOperands(unwindOperands);
+
+ result.addAttribute(
+ InvokeOp::getOperandSegmentSizeAttr(),
+ builder.getI32VectorAttr({static_cast<int32_t>(operands.size()),
+ static_cast<int32_t>(normalOperands.size()),
+ static_cast<int32_t>(unwindOperands.size())}));
return success();
}
diff --git a/mlir/lib/Dialect/SPIRV/SPIRVOps.cpp b/mlir/lib/Dialect/SPIRV/SPIRVOps.cpp
index 907f8f82f62f..3642f47a2bcc 100644
--- a/mlir/lib/Dialect/SPIRV/SPIRVOps.cpp
+++ b/mlir/lib/Dialect/SPIRV/SPIRVOps.cpp
@@ -971,7 +971,6 @@ static ParseResult parseBranchConditionalOp(OpAsmParser &parser,
auto &builder = parser.getBuilder();
OpAsmParser::OperandType condInfo;
Block *dest;
- SmallVector<Value, 4> destOperands;
// Parse the condition.
Type boolTy = builder.getI1Type();
@@ -996,17 +995,24 @@ static ParseResult parseBranchConditionalOp(OpAsmParser &parser,
}
// Parse the true branch.
+ SmallVector<Value, 4> trueOperands;
if (parser.parseComma() ||
- parser.parseSuccessorAndUseList(dest, destOperands))
+ parser.parseSuccessorAndUseList(dest, trueOperands))
return failure();
- state.addSuccessor(dest, destOperands);
+ state.addSuccessors(dest);
+ state.addOperands(trueOperands);
// Parse the false branch.
- destOperands.clear();
+ SmallVector<Value, 4> falseOperands;
if (parser.parseComma() ||
- parser.parseSuccessorAndUseList(dest, destOperands))
+ parser.parseSuccessorAndUseList(dest, falseOperands))
return failure();
- state.addSuccessor(dest, destOperands);
+ state.addSuccessors(dest);
+ state.addOperands(falseOperands);
+ state.addAttribute(
+ spirv::BranchConditionalOp::getOperandSegmentSizeAttr(),
+ builder.getI32VectorAttr({1, static_cast<int32_t>(trueOperands.size()),
+ static_cast<int32_t>(falseOperands.size())}));
return success();
}
@@ -1024,11 +1030,11 @@ static void print(spirv::BranchConditionalOp branchOp, OpAsmPrinter &printer) {
}
printer << ", ";
- printer.printSuccessorAndUseList(branchOp.getOperation(),
- spirv::BranchConditionalOp::kTrueIndex);
+ printer.printSuccessorAndUseList(branchOp.getTrueBlock(),
+ branchOp.getTrueBlockArguments());
printer << ", ";
- printer.printSuccessorAndUseList(branchOp.getOperation(),
- spirv::BranchConditionalOp::kFalseIndex);
+ printer.printSuccessorAndUseList(branchOp.getFalseBlock(),
+ branchOp.getFalseBlockArguments());
}
static LogicalResult verify(spirv::BranchConditionalOp branchOp) {
diff --git a/mlir/lib/IR/AsmPrinter.cpp b/mlir/lib/IR/AsmPrinter.cpp
index ac2648846b24..d24bb505c53f 100644
--- a/mlir/lib/IR/AsmPrinter.cpp
+++ b/mlir/lib/IR/AsmPrinter.cpp
@@ -1964,9 +1964,13 @@ class OperationPrinter : public ModulePrinter, private OpAsmPrinter {
/*withKeyword=*/true);
}
+ /// Print the given successor.
+ void printSuccessor(Block *successor) override;
+
/// Print an operation successor with the operands used for the block
/// arguments.
- void printSuccessorAndUseList(Operation *term, unsigned index) override;
+ void printSuccessorAndUseList(Block *successor,
+ ValueRange succOperands) override;
/// Print the given region.
void printRegion(Region ®ion, bool printEntryBlockArgs,
@@ -2062,23 +2066,14 @@ void OperationPrinter::printGenericOp(Operation *op) {
os << '"';
printEscapedString(op->getName().getStringRef(), os);
os << "\"(";
-
- // Get the list of operands that are not successor operands.
- unsigned totalNumSuccessorOperands = 0;
- unsigned numSuccessors = op->getNumSuccessors();
- for (unsigned i = 0; i < numSuccessors; ++i)
- totalNumSuccessorOperands += op->getNumSuccessorOperands(i);
- unsigned numProperOperands = op->getNumOperands() - totalNumSuccessorOperands;
- interleaveComma(op->getOperands().take_front(numProperOperands),
- [&](Value value) { printValueID(value); });
-
+ interleaveComma(op->getOperands(), [&](Value value) { printValueID(value); });
os << ')';
// For terminators, print the list of successors and their operands.
- if (numSuccessors != 0) {
+ if (op->getNumSuccessors() != 0) {
os << '[';
- interleaveComma(llvm::seq<unsigned>(0, numSuccessors),
- [&](unsigned i) { printSuccessorAndUseList(op, i); });
+ interleaveComma(op->getSuccessors(),
+ [&](Block *successor) { printBlockName(successor); });
os << ']';
}
@@ -2167,12 +2162,14 @@ void OperationPrinter::printValueID(Value value, bool printResultNo) const {
state->getSSANameState().printValueID(value, printResultNo, os);
}
-void OperationPrinter::printSuccessorAndUseList(Operation *term,
- unsigned index) {
- printBlockName(term->getSuccessor(index));
+void OperationPrinter::printSuccessor(Block *successor) {
+ printBlockName(successor);
+}
- auto succOperands = term->getSuccessorOperands(index);
- if (succOperands.begin() == succOperands.end())
+void OperationPrinter::printSuccessorAndUseList(Block *successor,
+ ValueRange succOperands) {
+ printBlockName(successor);
+ if (succOperands.empty())
return;
os << '(';
diff --git a/mlir/lib/IR/Operation.cpp b/mlir/lib/IR/Operation.cpp
index 2af425d65406..0d207fd1a6aa 100644
--- a/mlir/lib/IR/Operation.cpp
+++ b/mlir/lib/IR/Operation.cpp
@@ -111,14 +111,10 @@ Operation *Operation::create(Location location, OperationName name,
NamedAttributeList attributes,
ArrayRef<Block *> successors, unsigned numRegions,
bool resizableOperandList) {
- unsigned numSuccessors = successors.size();
-
// We only need to allocate additional memory for a subset of results.
unsigned numTrailingResults = OpResult::getNumTrailing(resultTypes.size());
-
- // Input operands are nullptr-separated for each successor, the null operands
- // aren't actually stored.
- unsigned numOperands = operands.size() - numSuccessors;
+ unsigned numSuccessors = successors.size();
+ unsigned numOperands = operands.size();
// Compute the byte size for the operation and the operand storage.
auto byteSize = totalSizeToAlloc<detail::TrailingOpResult, BlockOperand,
@@ -152,56 +148,17 @@ Operation *Operation::create(Location location, OperationName name,
for (unsigned i = 0; i != numRegions; ++i)
new (&op->getRegion(i)) Region(op);
- // Initialize the results and operands.
+ // Initialize the operands.
new (&op->getOperandStorage())
detail::OperandStorage(numOperands, resizableOperandList);
auto opOperands = op->getOpOperands();
+ for (unsigned i = 0; i != numOperands; ++i)
+ new (&opOperands[i]) OpOperand(op, operands[i]);
- // Initialize normal operands.
- unsigned operandIt = 0, operandE = operands.size();
- unsigned nextOperand = 0;
- for (; operandIt != operandE; ++operandIt) {
- // Null operands are used as sentinels between successor operand lists. If
- // we encounter one here, break and handle the successor operands lists
- // separately below.
- if (!operands[operandIt])
- break;
- new (&opOperands[nextOperand++]) OpOperand(op, operands[operandIt]);
- }
-
- unsigned currentSuccNum = 0;
- if (operandIt == operandE) {
- // Verify that the amount of sentinel operands is equivalent to the number
- // of successors.
- assert(currentSuccNum == numSuccessors);
- return op;
- }
-
- assert(!op->isKnownNonTerminator() &&
- "Unexpected nullptr in operand list when creating non-terminator.");
- auto instBlockOperands = op->getBlockOperands();
- unsigned *succOperandCount = nullptr;
-
- for (; operandIt != operandE; ++operandIt) {
- // If we encounter a sentinel branch to the next operand update the count
- // variable.
- if (!operands[operandIt]) {
- assert(currentSuccNum < numSuccessors);
-
- new (&instBlockOperands[currentSuccNum])
- BlockOperand(op, successors[currentSuccNum]);
- succOperandCount =
- &instBlockOperands[currentSuccNum].numSuccessorOperands;
- ++currentSuccNum;
- continue;
- }
- new (&opOperands[nextOperand++]) OpOperand(op, operands[operandIt]);
- ++(*succOperandCount);
- }
-
- // Verify that the amount of sentinel operands is equivalent to the number of
- // successors.
- assert(currentSuccNum == numSuccessors);
+ // Initialize the successors.
+ auto blockOperands = op->getBlockOperands();
+ for (unsigned i = 0; i != numSuccessors; ++i)
+ new (&blockOperands[i]) BlockOperand(op, successors[i]);
return op;
}
@@ -564,49 +521,6 @@ void Operation::setSuccessor(Block *block, unsigned index) {
getBlockOperands()[index].set(block);
}
-auto Operation::getNonSuccessorOperands() -> operand_range {
- return getOperands().take_front(hasSuccessors() ? getSuccessorOperandIndex(0)
- : getNumOperands());
-}
-
-/// Get the index of the first operand of the successor at the provided
-/// index.
-unsigned Operation::getSuccessorOperandIndex(unsigned index) {
- assert(!isKnownNonTerminator() && "only terminators may have successors");
- assert(index < getNumSuccessors());
-
- // Count the number of operands for each of the successors after, and
- // including, the one at 'index'. This is based upon the assumption that all
- // non successor operands are placed at the beginning of the operand list.
- auto blockOperands = getBlockOperands().drop_front(index);
- unsigned postSuccessorOpCount =
- std::accumulate(blockOperands.begin(), blockOperands.end(), 0u,
- [](unsigned cur, const BlockOperand &operand) {
- return cur + operand.numSuccessorOperands;
- });
- return getNumOperands() - postSuccessorOpCount;
-}
-
-Optional<std::pair<unsigned, unsigned>>
-Operation::decomposeSuccessorOperandIndex(unsigned operandIndex) {
- assert(!isKnownNonTerminator() && "only terminators may have successors");
- assert(operandIndex < getNumOperands());
- unsigned currentOperandIndex = getNumOperands();
- auto blockOperands = getBlockOperands();
- for (unsigned i = 0, e = getNumSuccessors(); i < e; i++) {
- unsigned successorIndex = e - i - 1;
- currentOperandIndex -= blockOperands[successorIndex].numSuccessorOperands;
- if (currentOperandIndex <= operandIndex)
- return std::make_pair(successorIndex, operandIndex - currentOperandIndex);
- }
- return None;
-}
-
-auto Operation::getSuccessorOperands(unsigned index) -> operand_range {
- unsigned succOperandIndex = getSuccessorOperandIndex(index);
- return getOperands().slice(succOperandIndex, getNumSuccessorOperands(index));
-}
-
/// Attempt to fold this operation using the Op's registered foldHook.
LogicalResult Operation::fold(ArrayRef<Attribute> operands,
SmallVectorImpl<OpFoldResult> &results) {
@@ -645,39 +559,20 @@ Operation *Operation::cloneWithoutRegions(BlockAndValueMapping &mapper) {
SmallVector<Value, 8> operands;
SmallVector<Block *, 2> successors;
- operands.reserve(getNumOperands() + getNumSuccessors());
+ // Remap the operands.
+ operands.reserve(getNumOperands());
+ for (auto opValue : getOperands())
+ operands.push_back(mapper.lookupOrDefault(opValue));
- if (getNumSuccessors() == 0) {
- // Non-branching operations can just add all the operands.
- for (auto opValue : getOperands())
- operands.push_back(mapper.lookupOrDefault(opValue));
- } else {
- // We add the operands separated by nullptr's for each successor.
- unsigned firstSuccOperand =
- getNumSuccessors() ? getSuccessorOperandIndex(0) : getNumOperands();
- auto opOperands = getOpOperands();
-
- unsigned i = 0;
- for (; i != firstSuccOperand; ++i)
- operands.push_back(mapper.lookupOrDefault(opOperands[i].get()));
-
- successors.reserve(getNumSuccessors());
- for (unsigned succ = 0, e = getNumSuccessors(); succ != e; ++succ) {
- successors.push_back(mapper.lookupOrDefault(getSuccessor(succ)));
-
- // Add sentinel to delineate successor operands.
- operands.push_back(nullptr);
-
- // Remap the successors operands.
- for (auto operand : getSuccessorOperands(succ))
- operands.push_back(mapper.lookupOrDefault(operand));
- }
- }
+ // Remap the successors.
+ successors.reserve(getNumSuccessors());
+ for (Block *successor : getSuccessors())
+ successors.push_back(mapper.lookupOrDefault(successor));
- unsigned numRegions = getNumRegions();
- auto *newOp =
- Operation::create(getLoc(), getName(), getResultTypes(), operands, attrs,
- successors, numRegions, hasResizableOperandsList());
+ // Create the new operation.
+ auto *newOp = Operation::create(getLoc(), getName(), getResultTypes(),
+ operands, attrs, successors, getNumRegions(),
+ hasResizableOperandsList());
// Remember the mapping of any results.
for (unsigned i = 0, e = getNumResults(); i != e; ++i)
diff --git a/mlir/lib/IR/OperationSupport.cpp b/mlir/lib/IR/OperationSupport.cpp
index 1d561473f755..ef949323d18d 100644
--- a/mlir/lib/IR/OperationSupport.cpp
+++ b/mlir/lib/IR/OperationSupport.cpp
@@ -42,15 +42,11 @@ OperationState::OperationState(Location location, StringRef name,
}
void OperationState::addOperands(ValueRange newOperands) {
- assert(successors.empty() && "Non successor operands should be added first.");
operands.append(newOperands.begin(), newOperands.end());
}
-void OperationState::addSuccessor(Block *successor, ValueRange succOperands) {
- successors.push_back(successor);
- // Insert a sentinel operand to mark a barrier between successor operands.
- operands.push_back(nullptr);
- operands.append(succOperands.begin(), succOperands.end());
+void OperationState::addSuccessors(SuccessorRange newSuccessors) {
+ successors.append(newSuccessors.begin(), newSuccessors.end());
}
Region *OperationState::addRegion() {
diff --git a/mlir/lib/Parser/Parser.cpp b/mlir/lib/Parser/Parser.cpp
index 661bddf8107a..9c699284b746 100644
--- a/mlir/lib/Parser/Parser.cpp
+++ b/mlir/lib/Parser/Parser.cpp
@@ -3301,13 +3301,11 @@ class OperationParser : public Parser {
/// Parse an operation instance.
ParseResult parseOperation();
- /// Parse a single operation successor and its operand list.
- ParseResult parseSuccessorAndUseList(Block *&dest,
- SmallVectorImpl<Value> &operands);
+ /// Parse a single operation successor.
+ ParseResult parseSuccessor(Block *&dest);
/// Parse a comma-separated list of operation successors in brackets.
- ParseResult parseSuccessors(SmallVectorImpl<Block *> &destinations,
- SmallVectorImpl<SmallVector<Value, 4>> &operands);
+ ParseResult parseSuccessors(SmallVectorImpl<Block *> &destinations);
/// Parse an operation instance that is in the generic form.
Operation *parseGenericOperation();
@@ -3797,27 +3795,16 @@ ParseResult OperationParser::parseOperation() {
return success();
}
-/// Parse a single operation successor and its operand list.
+/// Parse a single operation successor.
///
-/// successor ::= block-id branch-use-list?
-/// branch-use-list ::= `(` ssa-use-list ':' type-list-no-parens `)`
+/// successor ::= block-id
///
-ParseResult
-OperationParser::parseSuccessorAndUseList(Block *&dest,
- SmallVectorImpl<Value> &operands) {
+ParseResult OperationParser::parseSuccessor(Block *&dest) {
// Verify branch is identifier and get the matching block.
if (!getToken().is(Token::caret_identifier))
return emitError("expected block name");
dest = getBlockNamed(getTokenSpelling(), getToken().getLoc());
consumeToken();
-
- // Handle optional arguments.
- if (consumeIf(Token::l_paren) &&
- (parseOptionalSSAUseAndTypeList(operands) ||
- parseToken(Token::r_paren, "expected ')' to close argument list"))) {
- return failure();
- }
-
return success();
}
@@ -3825,18 +3812,15 @@ OperationParser::parseSuccessorAndUseList(Block *&dest,
///
/// successor-list ::= `[` successor (`,` successor )* `]`
///
-ParseResult OperationParser::parseSuccessors(
- SmallVectorImpl<Block *> &destinations,
- SmallVectorImpl<SmallVector<Value, 4>> &operands) {
+ParseResult
+OperationParser::parseSuccessors(SmallVectorImpl<Block *> &destinations) {
if (parseToken(Token::l_square, "expected '['"))
return failure();
- auto parseElt = [this, &destinations, &operands]() {
+ auto parseElt = [this, &destinations] {
Block *dest;
- SmallVector<Value, 4> destOperands;
- auto res = parseSuccessorAndUseList(dest, destOperands);
+ ParseResult res = parseSuccessor(dest);
destinations.push_back(dest);
- operands.push_back(destOperands);
return res;
};
return parseCommaSeparatedListUntil(Token::r_square, parseElt,
@@ -3880,24 +3864,23 @@ Operation *OperationParser::parseGenericOperation() {
// Parse the operand list.
SmallVector<SSAUseInfo, 8> operandInfos;
-
if (parseToken(Token::l_paren, "expected '(' to start operand list") ||
parseOptionalSSAUseList(operandInfos) ||
parseToken(Token::r_paren, "expected ')' to end operand list")) {
return nullptr;
}
- // Parse the successor list but don't add successors to the result yet to
- // avoid messing up with the argument order.
- SmallVector<Block *, 2> successors;
- SmallVector<SmallVector<Value, 4>, 2> successorOperands;
+ // Parse the successor list.
if (getToken().is(Token::l_square)) {
// Check if the operation is a known terminator.
const AbstractOperation *abstractOp = result.name.getAbstractOperation();
if (abstractOp && !abstractOp->hasProperty(OperationProperty::Terminator))
return emitError("successors in non-terminator"), nullptr;
- if (parseSuccessors(successors, successorOperands))
+
+ SmallVector<Block *, 2> successors;
+ if (parseSuccessors(successors))
return nullptr;
+ result.addSuccessors(successors);
}
// Parse the region list.
@@ -3948,13 +3931,6 @@ Operation *OperationParser::parseGenericOperation() {
return nullptr;
}
- // Add the successors, and their operands after the proper operands.
- for (auto succ : llvm::zip(successors, successorOperands)) {
- Block *successor = std::get<0>(succ);
- const SmallVector<Value, 4> &operands = std::get<1>(succ);
- result.addSuccessor(successor, operands);
- }
-
// Parse a location if one is present.
if (parseOptionalTrailingLocation(result.location))
return nullptr;
@@ -4421,20 +4397,31 @@ class CustomOpAsmParser : public OpAsmParser {
// Successor Parsing
//===--------------------------------------------------------------------===//
- /// Parse a single operation successor and its operand list.
- ParseResult
- parseSuccessorAndUseList(Block *&dest,
- SmallVectorImpl<Value> &operands) override {
- return parser.parseSuccessorAndUseList(dest, operands);
+ /// Parse a single operation successor.
+ ParseResult parseSuccessor(Block *&dest) override {
+ return parser.parseSuccessor(dest);
}
/// Parse an optional operation successor and its operand list.
- OptionalParseResult
- parseOptionalSuccessorAndUseList(Block *&dest,
- SmallVectorImpl<Value> &operands) override {
+ OptionalParseResult parseOptionalSuccessor(Block *&dest) override {
if (parser.getToken().isNot(Token::caret_identifier))
return llvm::None;
- return parseSuccessorAndUseList(dest, operands);
+ return parseSuccessor(dest);
+ }
+
+ /// Parse a single operation successor and its operand list.
+ ParseResult
+ parseSuccessorAndUseList(Block *&dest,
+ SmallVectorImpl<Value> &operands) override {
+ if (parseSuccessor(dest))
+ return failure();
+
+ // Handle optional arguments.
+ if (succeeded(parseOptionalLParen()) &&
+ (parser.parseOptionalSSAUseAndTypeList(operands) || parseRParen())) {
+ return failure();
+ }
+ return success();
}
//===--------------------------------------------------------------------===//
diff --git a/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp b/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
index 5441385acb03..6a774acdf6bf 100644
--- a/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
+++ b/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
@@ -634,21 +634,29 @@ LogicalResult Importer::processInstruction(llvm::Instruction *inst) {
auto *brInst = cast<llvm::BranchInst>(inst);
OperationState state(loc,
brInst->isConditional() ? "llvm.cond_br" : "llvm.br");
- SmallVector<Value, 4> ops;
if (brInst->isConditional()) {
Value condition = processValue(brInst->getCondition());
if (!condition)
return failure();
- ops.push_back(condition);
+ state.addOperands(condition);
}
- state.addOperands(ops);
- SmallVector<Block *, 4> succs;
- for (auto *succ : llvm::reverse(brInst->successors())) {
+
+ std::array<int32_t, 3> operandSegmentSizes = {1, 0, 0};
+ for (int i : llvm::seq<int>(0, brInst->getNumSuccessors())) {
+ auto *succ = brInst->getSuccessor(i);
SmallVector<Value, 4> blockArguments;
if (failed(processBranchArgs(brInst, succ, blockArguments)))
return failure();
- state.addSuccessor(blocks[succ], blockArguments);
+ state.addSuccessors(blocks[succ]);
+ state.addOperands(blockArguments);
+ operandSegmentSizes[i + 1] = blockArguments.size();
}
+
+ if (brInst->isConditional()) {
+ state.addAttribute(LLVM::CondBrOp::getOperandSegmentSizeAttr(),
+ b.getI32VectorAttr(operandSegmentSizes));
+ }
+
b.createOperation(state);
return success();
}
diff --git a/mlir/lib/Transforms/DialectConversion.cpp b/mlir/lib/Transforms/DialectConversion.cpp
index ed81b588875c..a58dd05d0812 100644
--- a/mlir/lib/Transforms/DialectConversion.cpp
+++ b/mlir/lib/Transforms/DialectConversion.cpp
@@ -1005,33 +1005,7 @@ ConversionPattern::matchAndRewrite(Operation *op,
SmallVector<Value, 4> operands;
auto &dialectRewriter = static_cast<ConversionPatternRewriter &>(rewriter);
dialectRewriter.getImpl().remapValues(op->getOperands(), operands);
-
- // If this operation has no successors, invoke the rewrite directly.
- if (op->getNumSuccessors() == 0)
- return matchAndRewrite(op, operands, dialectRewriter);
-
- // Otherwise, we need to remap the successors.
- SmallVector<Block *, 2> destinations;
- destinations.reserve(op->getNumSuccessors());
-
- SmallVector<ArrayRef<Value>, 2> operandsPerDestination;
- unsigned firstSuccessorOperand = op->getSuccessorOperandIndex(0);
- for (unsigned i = 0, seen = 0, e = op->getNumSuccessors(); i < e; ++i) {
- destinations.push_back(op->getSuccessor(i));
-
- // Lookup the successors operands.
- unsigned n = op->getNumSuccessorOperands(i);
- operandsPerDestination.push_back(
- llvm::makeArrayRef(operands.data() + firstSuccessorOperand + seen, n));
- seen += n;
- }
-
- // Rewrite the operation.
- return matchAndRewrite(
- op,
- llvm::makeArrayRef(operands.data(),
- operands.data() + firstSuccessorOperand),
- destinations, operandsPerDestination, dialectRewriter);
+ return matchAndRewrite(op, operands, dialectRewriter);
}
//===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Transforms/Utils/RegionUtils.cpp b/mlir/lib/Transforms/Utils/RegionUtils.cpp
index 911268606414..57535fec7380 100644
--- a/mlir/lib/Transforms/Utils/RegionUtils.cpp
+++ b/mlir/lib/Transforms/Utils/RegionUtils.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "mlir/Transforms/RegionUtils.h"
+#include "mlir/Analysis/ControlFlowInterfaces.h"
#include "mlir/IR/Block.h"
#include "mlir/IR/Operation.h"
#include "mlir/IR/RegionGraphTraits.h"
@@ -172,8 +173,9 @@ static bool isUseSpeciallyKnownDead(OpOperand &use, LiveMap &liveMap) {
// node, rather than to the terminator op itself, a terminator op can't e.g.
// "print" the value of a successor operand.
if (owner->isKnownTerminator()) {
- if (auto arg = owner->getSuccessorBlockArgument(operandIndex))
- return !liveMap.wasProvenLive(*arg);
+ if (BranchOpInterface branchInterface = dyn_cast<BranchOpInterface>(owner))
+ if (auto arg = branchInterface.getSuccessorBlockArgument(operandIndex))
+ return !liveMap.wasProvenLive(*arg);
return false;
}
return false;
@@ -200,6 +202,29 @@ static bool isOpIntrinsicallyLive(Operation *op) {
}
static void propagateLiveness(Region ®ion, LiveMap &liveMap);
+
+static void propagateTerminatorLiveness(Operation *op, LiveMap &liveMap) {
+ // Terminators are always live.
+ liveMap.setProvedLive(op);
+
+ // Check to see if we can reason about the successor operands and mutate them.
+ BranchOpInterface branchInterface = dyn_cast<BranchOpInterface>(op);
+ if (!branchInterface || !branchInterface.canEraseSuccessorOperand()) {
+ for (Block *successor : op->getSuccessors())
+ for (BlockArgument arg : successor->getArguments())
+ liveMap.setProvedLive(arg);
+ return;
+ }
+
+ // If we can't reason about the operands to a successor, conservatively mark
+ // all arguments as live.
+ for (unsigned i = 0, e = op->getNumSuccessors(); i != e; ++i) {
+ if (!branchInterface.getSuccessorOperands(i))
+ for (BlockArgument arg : op->getSuccessor(i)->getArguments())
+ liveMap.setProvedLive(arg);
+ }
+}
+
static void propagateLiveness(Operation *op, LiveMap &liveMap) {
// All Value's are either a block argument or an op result.
// We call processValue on those cases.
@@ -208,6 +233,10 @@ static void propagateLiveness(Operation *op, LiveMap &liveMap) {
for (Region ®ion : op->getRegions())
propagateLiveness(region, liveMap);
+ // Process terminator operations.
+ if (op->isKnownTerminator())
+ return propagateTerminatorLiveness(op, liveMap);
+
// Process the op itself.
if (isOpIntrinsicallyLive(op)) {
liveMap.setProvedLive(op);
@@ -238,6 +267,10 @@ static void propagateLiveness(Region ®ion, LiveMap &liveMap) {
static void eraseTerminatorSuccessorOperands(Operation *terminator,
LiveMap &liveMap) {
+ BranchOpInterface branchOp = dyn_cast<BranchOpInterface>(terminator);
+ if (!branchOp)
+ return;
+
for (unsigned succI = 0, succE = terminator->getNumSuccessors();
succI < succE; succI++) {
// Iterating successors in reverse is not strictly needed, since we
@@ -245,15 +278,17 @@ static void eraseTerminatorSuccessorOperands(Operation *terminator,
// since it will promote later operands of the terminator being erased
// first, reducing the quadratic-ness.
unsigned succ = succE - succI - 1;
- for (unsigned argI = 0, argE = terminator->getNumSuccessorOperands(succ);
- argI < argE; argI++) {
+ Optional<OperandRange> succOperands = branchOp.getSuccessorOperands(succ);
+ if (!succOperands)
+ continue;
+ Block *successor = terminator->getSuccessor(succ);
+
+ for (unsigned argI = 0, argE = succOperands->size(); argI < argE; ++argI) {
// Iterating args in reverse is needed for correctness, to avoid
// shifting later args when earlier args are erased.
unsigned arg = argE - argI - 1;
- Value value = terminator->getSuccessor(succ)->getArgument(arg);
- if (!liveMap.wasProvenLive(value)) {
- terminator->eraseSuccessorOperand(succ, arg);
- }
+ if (!liveMap.wasProvenLive(successor->getArgument(arg)))
+ branchOp.eraseSuccessorOperand(succ, arg);
}
}
}
diff --git a/mlir/test/Dialect/SPIRV/control-flow-ops.mlir b/mlir/test/Dialect/SPIRV/control-flow-ops.mlir
index a6dcac1f029f..141d2c1aa1c3 100644
--- a/mlir/test/Dialect/SPIRV/control-flow-ops.mlir
+++ b/mlir/test/Dialect/SPIRV/control-flow-ops.mlir
@@ -24,7 +24,7 @@ func @branch_argument() -> () {
// -----
func @missing_accessor() -> () {
- // expected-error @+1 {{requires 1 successor but found 0}}
+ // expected-error @+2 {{expected block name}}
spv.Branch
}
@@ -117,7 +117,7 @@ func @wrong_condition_type() -> () {
func @wrong_accessor_count() -> () {
%true = spv.constant true
// expected-error @+1 {{requires 2 successors but found 1}}
- "spv.BranchConditional"(%true)[^one] : (i1) -> ()
+ "spv.BranchConditional"(%true)[^one] {operand_segment_sizes = dense<[1, 0, 0]>: vector<3xi32>} : (i1) -> ()
^one:
spv.Return
^two:
@@ -129,7 +129,8 @@ func @wrong_accessor_count() -> () {
func @wrong_number_of_weights() -> () {
%true = spv.constant true
// expected-error @+1 {{must have exactly two branch weights}}
- "spv.BranchConditional"(%true)[^one, ^two] {branch_weights = [1 : i32, 2 : i32, 3 : i32]} : (i1) -> ()
+ "spv.BranchConditional"(%true)[^one, ^two] {branch_weights = [1 : i32, 2 : i32, 3 : i32],
+ operand_segment_sizes = dense<[1, 0, 0]>: vector<3xi32>} : (i1) -> ()
^one:
spv.Return
^two:
diff --git a/mlir/test/IR/invalid.mlir b/mlir/test/IR/invalid.mlir
index 12e3bfdcdec5..80b83fe60ec3 100644
--- a/mlir/test/IR/invalid.mlir
+++ b/mlir/test/IR/invalid.mlir
@@ -153,7 +153,7 @@ func @block_arg_no_type() {
func @block_arg_no_close_paren() {
^bb42:
- br ^bb2( // expected-error at +1 {{expected ')' to close argument list}}
+ br ^bb2( // expected-error at +1 {{expected ':'}}
return
}
diff --git a/mlir/test/IR/parser.mlir b/mlir/test/IR/parser.mlir
index 3baf0642e8b0..4a8caf7d3d37 100644
--- a/mlir/test/IR/parser.mlir
+++ b/mlir/test/IR/parser.mlir
@@ -439,11 +439,11 @@ func @bbargs() -> (i16, i8) {
func @verbose_terminators() -> (i1, i17) {
%0:2 = "foo"() : () -> (i1, i17)
// CHECK: br ^bb1(%{{.*}}#0, %{{.*}}#1 : i1, i17)
- "std.br"()[^bb1(%0#0, %0#1 : i1, i17)] : () -> ()
+ "std.br"(%0#0, %0#1)[^bb1] : (i1, i17) -> ()
^bb1(%x : i1, %y : i17):
// CHECK: cond_br %{{.*}}, ^bb2(%{{.*}} : i17), ^bb3(%{{.*}}, %{{.*}} : i1, i17)
- "std.cond_br"(%x)[^bb2(%y : i17), ^bb3(%x, %y : i1, i17)] : (i1) -> ()
+ "std.cond_br"(%x, %y, %x, %y) [^bb2, ^bb3] {operand_segment_sizes = dense<[1, 1, 2]>: vector<3xi32>} : (i1, i17, i1, i17) -> ()
^bb2(%a : i17):
%true = constant 1 : i1
@@ -844,8 +844,8 @@ func @terminator_with_regions() {
// CHECK-LABEL: func @unregistered_term
func @unregistered_term(%arg0 : i1) -> i1 {
- // CHECK-NEXT: "unregistered_br"()[^bb1(%{{.*}} : i1)] : () -> ()
- "unregistered_br"()[^bb1(%arg0 : i1)] : () -> ()
+ // CHECK-NEXT: "unregistered_br"(%{{.*}})[^bb1] : (i1) -> ()
+ "unregistered_br"(%arg0)[^bb1] : (i1) -> ()
^bb1(%arg1 : i1):
return %arg1 : i1
diff --git a/mlir/test/Transforms/canonicalize-dce.mlir b/mlir/test/Transforms/canonicalize-dce.mlir
index 5d0da2167fc1..eee83559979f 100644
--- a/mlir/test/Transforms/canonicalize-dce.mlir
+++ b/mlir/test/Transforms/canonicalize-dce.mlir
@@ -20,7 +20,7 @@ func @f(%arg0: f32) {
// CHECK-NEXT: return
func @f(%arg0: f32) {
- "test.br"()[^succ(%arg0: f32)] : () -> ()
+ "test.br"(%arg0)[^succ] : (f32) -> ()
^succ(%0: f32):
return
}
@@ -141,7 +141,7 @@ func @f(%arg0: f32) {
// Test case: Test the mechanics of deleting multiple block arguments.
// CHECK: func @f(%arg0: tensor<1xf32>, %arg1: tensor<2xf32>, %arg2: tensor<3xf32>, %arg3: tensor<4xf32>, %arg4: tensor<5xf32>)
-// CHECK-NEXT: "test.br"()[^bb1(%arg1, %arg3 : tensor<2xf32>, tensor<4xf32>)
+// CHECK-NEXT: "test.br"(%arg1, %arg3)[^bb1] : (tensor<2xf32>, tensor<4xf32>)
// CHECK-NEXT: ^bb1([[VAL0:%.+]]: tensor<2xf32>, [[VAL1:%.+]]: tensor<4xf32>):
// CHECK-NEXT: "foo.print"([[VAL0]])
// CHECK-NEXT: "foo.print"([[VAL1]])
@@ -154,7 +154,7 @@ func @f(
%arg2: tensor<3xf32>,
%arg3: tensor<4xf32>,
%arg4: tensor<5xf32>) {
- "test.br"()[^succ(%arg0, %arg1, %arg2, %arg3, %arg4 : tensor<1xf32>, tensor<2xf32>, tensor<3xf32>, tensor<4xf32>, tensor<5xf32>)] : () -> ()
+ "test.br"(%arg0, %arg1, %arg2, %arg3, %arg4)[^succ] : (tensor<1xf32>, tensor<2xf32>, tensor<3xf32>, tensor<4xf32>, tensor<5xf32>) -> ()
^succ(%t1: tensor<1xf32>, %t2: tensor<2xf32>, %t3: tensor<3xf32>, %t4: tensor<4xf32>, %t5: tensor<5xf32>):
"foo.print"(%t2) : (tensor<2xf32>) -> ()
"foo.print"(%t4) : (tensor<4xf32>) -> ()
diff --git a/mlir/test/lib/TestDialect/TestOps.td b/mlir/test/lib/TestDialect/TestOps.td
index 5ee4a46eb818..904003e1461c 100644
--- a/mlir/test/lib/TestDialect/TestOps.td
+++ b/mlir/test/lib/TestDialect/TestOps.td
@@ -462,6 +462,7 @@ def UpdateAttr : Pat<(I32ElementsAttrOp $attr),
def TestBranchOp : TEST_Op<"br",
[DeclareOpInterfaceMethods<BranchOpInterface>, Terminator]> {
+ let arguments = (ins Variadic<AnyType>:$targetOperands);
let successors = (successor AnySuccessor:$target);
}
diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index f4db32a3315c..2b751e00c0d0 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -609,9 +609,8 @@ void OpEmitter::genNamedSuccessorGetters() {
if (successor.name.empty())
continue;
- // Generate the accessors for a variadic successor.
+ // Generate the accessors for a variadic successor list.
if (successor.isVariadic()) {
- // Generate the getter.
auto &m = opClass.newMethod("SuccessorRange", successor.name);
m.body() << formatv(
" return {std::next(this->getOperation()->successor_begin(), {0}), "
@@ -620,21 +619,8 @@ void OpEmitter::genNamedSuccessorGetters() {
continue;
}
- // Generate the block getter.
auto &m = opClass.newMethod("Block *", successor.name);
m.body() << formatv(" return this->getOperation()->getSuccessor({0});", i);
-
- // Generate the all-operands getter.
- auto &operandsMethod = opClass.newMethod(
- "Operation::operand_range", (successor.name + "Operands").str());
- operandsMethod.body() << formatv(
- " return this->getOperation()->getSuccessorOperands({0});", i);
-
- // Generate the individual-operand getter.
- auto &operandMethod = opClass.newMethod(
- "Value", (successor.name + "Operand").str(), "unsigned index");
- operandMethod.body() << formatv(
- " return this->getOperation()->getSuccessorOperand({0}, index);", i);
}
}
@@ -1044,14 +1030,9 @@ void OpEmitter::buildParamList(std::string ¶mList,
}
/// Insert parameters for the block and operands for each successor.
- const char *variadicSuccCode =
- ", ArrayRef<Block *> {0}, ArrayRef<ValueRange> {0}Operands";
- const char *succCode = ", Block *{0}, ValueRange {0}Operands";
- for (const NamedSuccessor &namedSuccessor : op.getSuccessors()) {
- if (namedSuccessor.isVariadic())
- paramList += llvm::formatv(variadicSuccCode, namedSuccessor.name).str();
- else
- paramList += llvm::formatv(succCode, namedSuccessor.name).str();
+ for (const NamedSuccessor &succ : op.getSuccessors()) {
+ paramList += (succ.isVariadic() ? ", ArrayRef<Block *> " : ", Block *");
+ paramList += succ.name;
}
}
@@ -1123,14 +1104,7 @@ void OpEmitter::genCodeForAddingArgAndRegionForBuilder(OpMethodBody &body,
// Push all successors to the result.
for (const NamedSuccessor &namedSuccessor : op.getSuccessors()) {
- if (namedSuccessor.isVariadic()) {
- body << formatv(" for (int i = 0, e = {1}.size(); i != e; ++i)\n"
- " {0}.addSuccessor({1}[i], {1}Operands[i]);\n",
- builderOpState, namedSuccessor.name);
- continue;
- }
-
- body << formatv(" {0}.addSuccessor({1}, {1}Operands);\n", builderOpState,
+ body << formatv(" {0}.addSuccessors({1});\n", builderOpState,
namedSuccessor.name);
}
}
@@ -1488,9 +1462,7 @@ void OpEmitter::genTraits() {
int numVariadicOperands = op.getNumVariadicOperands();
// Add operand size trait.
- // Note: Successor operands are also included in the operation's operand list,
- // so we always need to use VariadicOperands in the presence of successors.
- if (numVariadicOperands != 0 || op.getNumSuccessors()) {
+ if (numVariadicOperands != 0) {
if (numOperands == numVariadicOperands)
opClass.addTrait("OpTrait::VariadicOperands");
else
diff --git a/mlir/tools/mlir-tblgen/OpFormatGen.cpp b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
index 1f92e1e9d962..3249d06bb764 100644
--- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
@@ -418,24 +418,20 @@ const char *const functionalTypeParserCode = R"(
///
/// {0}: The name for the successor list.
const char *successorListParserCode = R"(
- SmallVector<std::pair<Block *, SmallVector<Value, 4>>, 2> {0}Successors;
+ SmallVector<Block *, 2> {0}Successors;
{
Block *succ;
- SmallVector<Value, 4> succOperands;
- // Parse the first successor.
- auto firstSucc = parser.parseOptionalSuccessorAndUseList(succ,
- succOperands);
+ auto firstSucc = parser.parseOptionalSuccessor(succ);
if (firstSucc.hasValue()) {
if (failed(*firstSucc))
return failure();
- {0}Successors.emplace_back(succ, succOperands);
+ {0}Successors.emplace_back(succ);
// Parse any trailing successors.
while (succeeded(parser.parseOptionalComma())) {
- succOperands.clear();
- if (parser.parseSuccessorAndUseList(succ, succOperands))
+ if (parser.parseSuccessor(succ))
return failure();
- {0}Successors.emplace_back(succ, succOperands);
+ {0}Successors.emplace_back(succ);
}
}
}
@@ -446,19 +442,10 @@ const char *successorListParserCode = R"(
/// {0}: The name of the successor.
const char *successorParserCode = R"(
Block *{0}Successor = nullptr;
- SmallVector<Value, 4> {0}Operands;
- if (parser.parseSuccessorAndUseList({0}Successor, {0}Operands))
+ if (parser.parseSuccessor({0}Successor))
return failure();
)";
-/// The code snippet used to resolve a list of parsed successors.
-///
-/// {0}: The name of the successor list.
-const char *resolveSuccessorListParserCode = R"(
- for (auto &succAndArgs : {0}Successors)
- result.addSuccessor(succAndArgs.first, succAndArgs.second);
-)";
-
/// Get the name used for the type list for the given type directive operand.
/// 'isVariadic' is set to true if the operand has variadic types.
static StringRef getTypeListName(Element *arg, bool &isVariadic) {
@@ -802,19 +789,16 @@ void OperationFormat::genParserSuccessorResolution(Operator &op,
bool hasAllSuccessors = llvm::any_of(
elements, [](auto &elt) { return isa<SuccessorsDirective>(elt.get()); });
if (hasAllSuccessors) {
- body << llvm::formatv(resolveSuccessorListParserCode, "full");
+ body << " result.addSuccessors(fullSuccessors);\n";
return;
}
// Otherwise, handle each successor individually.
for (const NamedSuccessor &successor : op.getSuccessors()) {
- if (successor.isVariadic()) {
- body << llvm::formatv(resolveSuccessorListParserCode, successor.name);
- continue;
- }
-
- body << llvm::formatv(" result.addSuccessor({0}Successor, {0}Operands);\n",
- successor.name);
+ if (successor.isVariadic())
+ body << " result.addSuccessors(" << successor.name << "Successors);\n";
+ else
+ body << " result.addSuccessors(" << successor.name << "Successor);\n";
}
}
@@ -957,28 +941,14 @@ static void genElementPrinter(Element *element, OpMethodBody &body,
body << " p << " << operand->getVar()->name << "();\n";
} else if (auto *successor = dyn_cast<SuccessorVariable>(element)) {
const NamedSuccessor *var = successor->getVar();
- if (var->isVariadic()) {
- body << " {\n"
- << " auto succRange = " << var->name << "();\n"
- << " auto opSuccBegin = getOperation()->successor_begin();\n"
- << " int i = succRange.begin() - opSuccBegin;\n"
- << " int e = i + succRange.size();\n"
- << " interleaveComma(llvm::seq<int>(i, e), p, [&](int i) {\n"
- << " p.printSuccessorAndUseList(*this, i);\n"
- << " });\n"
- << " }\n";
- return;
- }
-
- unsigned index = successor->getVar() - op.successor_begin();
- body << " p.printSuccessorAndUseList(*this, " << index << ");\n";
+ if (var->isVariadic())
+ body << " interleaveComma(" << var->name << "(), p);\n";
+ else
+ body << " p << " << var->name << "();\n";
} else if (isa<OperandsDirective>(element)) {
body << " p << getOperation()->getOperands();\n";
} else if (isa<SuccessorsDirective>(element)) {
- body << " interleaveComma(llvm::seq<int>(0, "
- "getOperation()->getNumSuccessors()), p, [&](int i) {"
- << " p.printSuccessorAndUseList(*this, i);"
- << " });\n";
+ body << " interleaveComma(getOperation()->getSuccessors(), p);\n";
} else if (auto *dir = dyn_cast<TypeDirective>(element)) {
body << " p << ";
genTypeOperandPrinter(dir->getOperand(), body) << ";\n";
More information about the Mlir-commits
mailing list