When an interface method argument uses a non-string type (e.g., a
TableGen class reference like `Foo` instead of a string literal
`"Foo"`), the code in `InterfaceMethod::InterfaceMethod` would crash
with an assertion failure in `cast<StringInit>`. Replace the unchecked
cast with a `dyn_cast` and emit a `PrintFatalError` with a descriptive
error message pointing to the source location and identifying which
argument has the wrong type.
Before this fix:
mlir-tblgen: Assertion `isa<To>(Val) && "cast<Ty>() argument of
incompatible type\!"' failed.
After this fix:
error: expected string type for interface method argument #0 ('arg') ...
Fixes#61869
Assisted-by: Claude Code
This was slotted for removal in LLVM 19 in the release notes of LLVM 17,
we're way past it now.
If your dialect still has:
> let usePropertiesForAttributes = 1;
Then you can just remove this line (it was the default value).
If you were setting it to 0:
> let usePropertiesForAttributes = 0;
Then you need to adopt the new behavior, please report any issue with
this migration.
This PR converts `DialectReductionPatternInterface` using ODS.
It also introduces a new Interface Method class:
`PureVirtualInterfaceMethod` which creates the method as pure virtual.
This patch adds support for `PredTypeTrait` and `PredAttrTrait` in type
and attribute definitions, enabling declarative predicate-based
verification similar to how `PredOpTrait` works for operations.
## Motivation
In 802bf02 (from 2021), `PredTypeTrait`/`PredAttrTrait` were defined in
TableGen but not implemented in the code generator. Using them causes
mlir-tblgen to crash with an assertion failure when trying to cast
`PredTrait` to `InterfaceTrait`. This patch fixes the crash and
implements the actual verification code generation.
## Usage
Use `$paramName` syntax in predicates to reference type/attribute
parameters:
```tablegen
def MyType : MyDialect_Type<"MyType",
[PredTypeTrait<"value must be positive", CPred<"$value > 0">>]> {
let parameters = (ins "unsigned":$value);
let mnemonic = "my_type";
let assemblyFormat = "`<` $value `>`";
}
```
This generates verification code in `verifyInvariantsImpl()`:
```cpp
if (!(value > 0)) {
emitError() << "failed to verify that value must be positive";
return ::mlir::failure();
}
```
This PR converts OpAsmDialectInterface using ODS.
It also introduces a new Interface Method class `InterfaceMethodDeclaration` which will declare the function without definition.
Currently, Dialect Interfaces can't be defined in ODS. This PR adds the
support for dialect interfaces. It follows the same approach with other
interfaces and extends on top of `Interface` class defined in
`mlir/TableGen/Interfaces.h`.
Given the following input:
```tablegen
#ifndef MY_INTERFACES
#define MY_INTERFACES
include "mlir/IR/Interfaces.td"
def DialectInlinerInterface : DialectInterface<"DialectInlinerInterface"> {
let description = [{
Define a base inlining interface class to allow for dialects to opt-in to the inliner.
}];
let cppNamespace = "::mlir";
let methods = [
InterfaceMethod<
/*desc=*/ [{
Returns true if the given region 'src' can be inlined into the region
'dest' that is attached to an operation registered to the current dialect.
'valueMapping' contains any remapped values from within the 'src' region.
This can be used to examine what values will replace entry arguments into
the 'src' region, for example.
}],
/*returnType=*/ "bool",
/*methodName=*/ "isLegalToInline",
/*args=*/ (ins "::mlir::Region *":$dest, "::mlir::Region *":$src, "::mlir::IRMapping &":$valueMapping),
/*methodBody=*/ [{
return true;
}]
>
];
}
#endif
```
It will generate the following code:
```cpp
/*===- TableGen'erated file -------------------------------------*- C++ -*-===*\
|* *|
|* Dialect Interface Declarations *|
|* *|
|* Automatically generated file, do not edit! *|
|* *|
\*===----------------------------------------------------------------------===*/
namespace mlir {
/// Define a base inlining interface class to allow for dialects to opt-in to the inliner.
class DialectInlinerInterface : public ::mlir::DialectInterface::Base<DialectInlinerInterface> {
public:
/// Returns true if the given region 'src' can be inlined into the region
/// 'dest' that is attached to an operation registered to the current dialect.
/// 'valueMapping' contains any remapped values from within the 'src' region.
/// This can be used to examine what values will replace entry arguments into
/// the 'src' region, for example.
virtual bool isLegalToInline(::mlir::Region * dest, ::mlir::Region * src, ::mlir::IRMapping & valueMapping) const;
protected:
DialectInlinerInterface(::mlir::Dialect *dialect) : Base(dialect) {}
};
} // namespace mlir
bool ::mlir::DialectInlinerInterface::isLegalToInline(::mlir::Region * dest, ::mlir::Region * src, ::mlir::IRMapping & valueMapping) const {
return true;
}
```
Currently, the declarative pattern rewrite generator will always print
the [source]:[line](s) from which a pattern came. This is a useful
debugging hint, but it causes problem when absolute paths are used as
arguments to mlir-tblgen (which LLVM's build rules automatically do).
Specifially, it causes the source to be tied to the build location,
harning reproducability and our collective ability to get ccache hits
from, say, separate worktrees.
This commit resolves the issue by replacing absolute paths in thes
"Generated from:" comments with their filenames. (The alternative would
have been to implement an entire file-prefix-map the way the C compilers
do, but since this is an isolated incident, I chose to resolve it
locally.)
This enables printing, for example, the attribute value from a
mismatched predicate. Example of resultant output (here made
non-negative report value seen as sign-extended int):
```
PDL/ops.mlir:21:1: error: 'pdl.pattern' op attribute 'benefit' failed to satisfy constraint: 16-bit signless integer attribute whose value is non-negative (got -31)
pdl.pattern @rewrite_with_args : benefit(-31) {
^
```
This is primarily the mechanism and didn't change any existing
constraints. I also attempted to keep the error format as close to the
original as possible - but did notice 2 errors that were inconsistent
with the rest and updated them to be consistent.
Op constraints are emitted as static standalone functions and need not
be surrounded by the Dialect's C++ namespace. Currently they are, and
this change stops emitting a namespace around these static functions.
This patch fixes a bug occurring when properties are mixed with any of
the InferType traits, causing tblgen to crash. A simple reproducer is:
```
def _TypeInferredPropOp : NS_Op<"type_inferred_prop_op_with_properties", [
AllTypesMatch<["value", "result"]>
]> {
let arguments = (ins Property<"unsigned">:$prop, AnyType:$value);
let results = (outs AnyType:$result);
let hasCustomAssemblyFormat = 1;
}
```
The issue occurs because of the call:
```
op.getArgToOperandOrAttribute(infer.getIndex());
```
To understand better the issue, consider:
```
attrOrOperandMapping = [Operand0]
arguments = [Prop0, Operand0]
```
In this case, `infer.getIndex()` will return `1` for `Operand0`, but
`getArgToOperandOrAttribute` expects `0`, causing the discrepancy that
causes the crash.
The fix is to change `attrOrOperandMapping` to also include props.
These are identified by misc-include-cleaner. I've filtered out those
that break builds. Also, I'm staying away from llvm-config.h,
config.h, and Compiler.h, which likely cause platform- or
compiler-specific build failures.
Following a recent discovery of a method being defined both "inline" and
"declaration" (declaration implying no definition), verify the method
properties in general to fail early in the development and avoid
accidental bugs (especially for "opt-in" features).
This commit adds support for non-attribute properties (such as
StringProp and I64Prop) in declarative rewrite patterns. The handling
for properties follows the handling for attributes in most cases,
including in the generation of static matchers.
Constraints that are shared between multiple types are supported by
making the constraint matcher a templated function, which is the
equivalent to passing ::mlir::Attribute for an arbitrary C++ type.
This PR introduces the `vector.to_elements` op, which decomposes a
vector into its scalar elements. This operation is symmetrical to the
existing `vector.from_elements`.
Examples:
```
// Decompose a 0-D vector.
%0 = vector.to_elements %v0 : vector<f32>
// %0 = %v0[0]
// Decompose a 1-D vector.
%0:2 = vector.to_elements %v1 : vector<2xf32>
// %0#0 = %v1[0]
// %0#1 = %v1[1]
// Decompose a 2-D.
%0:6 = vector.to_elements %v2 : vector<2x3xf32>
// %0#0 = %v2[0, 0]
// %0#1 = %v2[0, 1]
// %0#2 = %v2[0, 2]
// %0#3 = %v2[1, 0]
// %0#4 = %v2[1, 1]
// %0#5 = %v2[1, 2]
```
This op is aimed at reducing code size when modeling "structured" vector
extractions and simplifying canonicalizations of large sequences of
`vector.extract` and `vector.insert` ops into `vector.shuffle` and other
sophisticated ops that can re-arrange vector elements.
Now that `Property` is a `PropConstraint`, hook it up to the same
constraint-uniquing machinery that other types of constraints use. This
will primarily save on code size for types, like enums, that have
inherent constraints which are shared across many operations.
In preparation for allowing non-attribute properties in the declaritive
rewrite pattern system, make `Property` a subclass of `PropConstraint`
in tablegen and add a CK_Prop to the Constraint class for tablegen.
Like `TypeConstraint` but unlike other constraints, a `PropConstraint`
has an additional field - the C++ interface type of the property being
constraint (if it's known).
AFAICT, all callers of getInputFilename consume the string right away.
Nobody seems to rely on the "copy" behavior that comes with returning
"const std::string".
Rename `ListInit::getValues()` to `getElements()` to better match with
other `ListInit` members like `getElement`. Keep `getValues()` for
existing downstream code but mark it deprecated.
Starting with C++17, std::string::find accepts anything that can be
converted to std::string_view, including StringRef, allowing us to
avoid creating temporary instances of std::string.
method body or default impl must be true empty. Even they contain only
spaces, ``mlir-tblgen`` considers they are non-empty and generates
invalid code lead to segment fault. It's very hard to debug.
```c++
InterfaceMethod<
...
/*methodBody=*/ [{ }], // This must be true empty. Leaving a space here can lead to segment fault which is hard to figure out why
/*defaultImpl=*/ [{
...
}]
```
This PR trim spaces when method body or default implementation of
interface method is not empty. Now ``mlir-tblgen`` generates valid code
even when they contain only spaces.
---------
Co-authored-by: Fung Xie <ftse@nvidia.com>
Co-authored-by: Mehdi Amini <joker.eph@gmail.com>
Since the introduction of `OpAsm{Type,Attr}Interface` (#121187), it is
possible to generate alias in AsmPrinter solely from the type/attribute
itself without consulting the `OpAsmDialectInterface`. This means the
behavior can be put in tablegen file near the type/attribute definition.
A common pattern is to just use the type/attr mnemonic as the alias.
Previously, like #130479/#130481/#130483, this means adding a default
implementation to `extraClassDeclaration` in `LLVM_Attr` base class.
However, as attribute definition may override `extraClassDeclaration`,
it might be preferred to have a new field in tablegen to specify this
behavior.
This commit adds a `genMnemonicAlias` field to `AttrOrTypeDef`, when
enabled, makes `mlir-tblgen` emit a default implementation of `getAlias`
using mnemonic. When `OpAsm{Attr,Type}Interface` is not specified by the
user, `tblgen` will automatically add the interface.
For users wanting other alias behavior, they can ignore such field and
still use `extraClassDeclaration` way.
This implements the result of the discussion at:
https://discourse.llvm.org/t/rfc-report-fatal-error-and-the-default-value-of-gencrashdialog/73587
There are two different use cases for report_fatal_error, so replace it
with two functions reportFatalInternalError() and
reportFatalUsageError(). The former indicates a bug in LLVM and
generates a crash dialog. The latter does not. The names have been
suggested by rnk and people seemed to like them.
This replaces a lot of the usages that passed an explicit value for
GenCrashDiag. I did not bulk replace remaining report_fatal_error usage
-- they probably require case by case review for which function to use.
The error is triggered when an attribute or type uses an APInt typed
parameter with the generated equality operator. If the compared APInts
have different bit widths the equality operator triggers an assert. This
is dangerous, since `StorageUniquer` for types and attributes uses the
equality operator when a hash collision appears. As such, it is
necessary to use custom provided comarator or `APIntParameter` that
already has it.
This commit also replaces uses of the raw `APInt` parameter with the
`APIntParameter` and removes the no longer necessary custom StorageClass
for the `BitVectorAttr` from the SMT dialect that was a workaround for
the described issue.
---------
Co-authored-by: Tobias Gysi <tobias.gysi@nextsilicon.com>
I observed that we have the boundary comments in the codebase like:
```
//===----------------------------------------------------------------------===//
// ...
//===----------------------------------------------------------------------===//
```
I also observed that there are incomplete boundary comments. The
revision is generated by a script that completes the boundary comments.
```
//===----------------------------------------------------------------------===//
// ...
...
```
Signed-off-by: hanhanW <hanhan0912@gmail.com>
This commit pulls apart the inherent attribute dependence of classes
like EnumAttrInfo and EnumAttrCase, factoring them out into simpler
EnumCase and EnumInfo variants. This allows specifying the cases of an
enum without needing to make the cases, or the EnumInfo itself, a
subclass of SignlessIntegerAttrBase.
The existing classes are retained as subclasses of the new ones, both
for backwards compatibility and to allow attribute-specific information.
In addition, the new BitEnum class changes its default printer/parser
behavior: cases when multiple keywords appear, like having both nuw and
nsw in overflow flags, will no longer be quoted by the operator<<, and
the FieldParser instance will now expect multiple keywords. All
instances of BitEnumAttr retain the old behavior.
This moves the EnumAttrCase and EnumAttr classes from Attribute.h/.cpp
to a new EnumInfo.h/cpp and renames them to EnumCase and EnumInfo,
respectively.
This doesn't change any of the tablegen files or any user-facing aspects
of the enum attribute generation system, just reorganizes code in order
to make main PR (#132148) shorter.
Trying to constrain two results to be of the same type using
`AllTypesMatch` would cause `mlir-tablgen` to crash on this
assertion[1].
Example:
```tblgen
def OpL5 : NS_Op<"op_with_same_but_unconstraint_results",
[AllTypesMatch<["result_a", "result_b"]>]> {
let results = (outs AnyType:$result_a, AnyType:$result_b);
}
```
This is because there was a small bug when constructing the `inferences`
graph from these constraints: The sources should be specified by the
combined arg/result index (in other words, with results negative) not
with the result index.
[1]
99612a3a18/mlir/lib/TableGen/Operator.cpp (L526)
Note that PointerUnion::{is,get} have been soft deprecated in
PointerUnion.h:
// FIXME: Replace the uses of is(), get() and dyn_cast() with
// isa<T>, cast<T> and the llvm::dyn_cast<T>
Give the properties from tablegen a `predicate` field that holds the
predicate that the property needs to satisfy, if one exists, and hook
that field up to verifier generation.
* Strip calls to raw_string_ostream::flush(), which is essentially a no-op
* Strip unneeded calls to raw_string_ostream::str(), to avoid excess indirection.
The `hasSummary` and `hasDescription` functions are currently useless as
they check if the corresponding `summary` and `description` are present.
However, these values are set to a default value of `""`, and so these
functions always return true.
This PR changes these functions to check if the summary and description
are just whitespace, which is presumably closer to their original
intent.
@math-fehr
@zero9178
