Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mlir] Update comment about propertiesAttr (NFC) #89633

Closed
wants to merge 3 commits into from

Conversation

Mogball
Copy link
Contributor

@Mogball Mogball commented Apr 22, 2024

[mlir] Update comment about propertiesAttr (NFC)

The comment is misleading because propertiesAttr is not actually
ignored when the operation isn't unregistered.

Mogball and others added 3 commits April 22, 2024 16:41
Adds an option to `mlir-tblgen -gen-op-defs` `op-shard-count=N` that divides the
op class definitions and op list into N segments, e.g.

```
// mlir-tblgen -gen-op-defs -op-shard-count=2

void FooDialect::initialize() {
  addOperations<
  >();
  addOperations<
  >();
}

```

When split across multiple source files, this can help significantly improve
dialect compile time for dialects with a large opset.

stack-info: PR: #89423, branch: users/mogball/pr_1
This PR massively reorganizes the Test dialect's source files. It moves
manually-written op hooks into `TestOpDefs.cpp`, moves format custom
directive parsers and printers into `TestFormatUtils`, adds missing
comment blocks, and moves around where generated source files are
included for types, attributes, enums, etc. into their own source file.

This will hopefully help navigate the test dialect source code, but also
speeds up compile time of the test dialect by putting generated source
files into separate compilation units.

This also sets up the test dialect to shard its op definitions, done in
the next PR.

stack-info: PR: #89424, branch: users/mogball/pr_2
This PR uses the new op sharding mechanism in tablegen to shard the test
dialect's op definitions. This breaks the definition of ops into
multiple source files, speeding up compile time of the test dialect
dramatically. This improves developer cycle times when iterating on the
test dialect.

stack-info: PR: #89628, branch: users/Mogball/stack/1
@llvmbot llvmbot added mlir:core MLIR Core Infrastructure mlir labels Apr 22, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 22, 2024

@llvm/pr-subscribers-mlir-affine
@llvm/pr-subscribers-mlir-dlti
@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-core

Author: Jeff Niu (Mogball)

Changes

[mlir] Update comment about propertiesAttr (NFC)

The comment is misleading because propertiesAttr is not actually
ignored when the operation isn't unregistered.


Full diff: https://github.com/llvm/llvm-project/pull/89633.diff

1 Files Affected:

  • (modified) mlir/include/mlir/IR/OperationSupport.h (+6-3)
diff --git a/mlir/include/mlir/IR/OperationSupport.h b/mlir/include/mlir/IR/OperationSupport.h
index 2c1c490aac49b8..2c6e8253b4327a 100644
--- a/mlir/include/mlir/IR/OperationSupport.h
+++ b/mlir/include/mlir/IR/OperationSupport.h
@@ -960,9 +960,12 @@ struct OperationState {
   /// Regions that the op will hold.
   SmallVector<std::unique_ptr<Region>, 1> regions;
 
-  // If we're creating an unregistered operation, this Attribute is used to
-  // build the properties. Otherwise it is ignored. For registered operations
-  // see the `getOrAddProperties` method.
+  // This Attribute is used to opaquely construct the properties of the
+  // operation. If we're creating an unregistered operation, the Attribute is
+  // used as-is as the Properties storage of the operation. Otherwise, the
+  // operation properties are constructed opaquely using its
+  // `setPropertiesFromAttr` hook. Note that `getOrAddProperties` is the
+  // preferred method to construct properties from C++.
   Attribute propertiesAttr;
 
 private:

@Mogball Mogball requested a review from rupprecht as a code owner April 22, 2024 17:09
@llvmbot llvmbot added mlir:affine bazel "Peripheral" support tier build system: utils/bazel mlir:dlti mlir:func labels Apr 22, 2024
@Mogball Mogball closed this Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bazel "Peripheral" support tier build system: utils/bazel mlir:affine mlir:core MLIR Core Infrastructure mlir:dlti mlir:func mlir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants