Remove unused Toy dialect files and clean up the example structure - #9
Merged
Conversation
Alwaysproblem
commented
Mar 8, 2026
Owner
- Deleted obsolete ToyCombine.td, AST.cpp, struct-codegen.toy, and toyc.cpp files from the Ch8 example.
- Updated README.md to remove references to the deleted files and examples.
- Modified build_deps.sh and sync_deps.sh scripts to ensure proper dependency management.
- Added explicit TypeID definitions in MyExtension classes across multiple transform examples.
- Improved comments and formatting in MyExtension.td and MyExtensionTypes.td for clarity.
- Updated debug logging in MyExtension.cpp to use the new logging macros.
- Deleted obsolete ToyCombine.td, AST.cpp, struct-codegen.toy, and toyc.cpp files from the Ch8 example. - Updated README.md to remove references to the deleted files and examples. - Modified build_deps.sh and sync_deps.sh scripts to ensure proper dependency management. - Added explicit TypeID definitions in MyExtension classes across multiple transform examples. - Improved comments and formatting in MyExtension.td and MyExtensionTypes.td for clarity. - Updated debug logging in MyExtension.cpp to use the new logging macros.
There was a problem hiding this comment.
Pull request overview
This PR cleans up the MLIR Toy and Transform tutorial examples by removing the obsolete Chapter 8 Toy dialect implementation and modernizing several remaining chapters (Ch1–Ch7 + transform_Ch2–Ch4) to match newer MLIR/LLVM APIs and logging patterns.
Changes:
- Removed the entire obsolete
Ch8example (sources, TableGen, build files, and sample inputs) and dropped it from the build + README. - Updated build/deps scripts to sync and build against a newer llvm-project revision.
- Modernized several Ch1–Ch7 sources (API updates, pass argument names, debug logging, op creation patterns) and added explicit
TypeIDdefinitions for Transform extensions.
Reviewed changes
Copilot reviewed 78 out of 80 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| mlir/example/transform_Ch4/lib/MyExtension.cpp | Switch debug output to DebugLog (LDBG) and add explicit extension TypeID. |
| mlir/example/transform_Ch3/lib/MyExtension.cpp | Add explicit extension TypeID. |
| mlir/example/transform_Ch3/include/MyExtensionTypes.td | Minor comment formatting update (but introduces trailing whitespace). |
| mlir/example/transform_Ch3/include/MyExtension.td | Update op argument constraints / comment formatting (but introduces trailing whitespace). |
| mlir/example/transform_Ch2/lib/MyExtension.cpp | Add explicit extension TypeID. |
| mlir/example/transform_Ch2/include/MyExtension.td | Comment formatting update (but introduces trailing whitespace). |
| mlir/example/scripts/sync_deps.sh | Update llvm-project clone ref to a specific llvmorg-* tag. |
| mlir/example/scripts/build_deps.sh | Adjust workspace/path handling and update CMake configuration invocation. |
| mlir/example/README.md | Remove Ch8 usage documentation. |
| mlir/example/Ch8/toyc.cpp | Deleted obsolete Ch8 toy compiler entrypoint. |
| mlir/example/Ch8/struct-codegen.toy | Deleted obsolete Ch8 sample. |
| mlir/example/Ch8/parser/AST.cpp | Deleted obsolete Ch8 AST dumper implementation. |
| mlir/example/Ch8/mlir/ToyCombine.td | Deleted obsolete Ch8 rewrite patterns (TableGen). |
| mlir/example/Ch8/mlir/ToyCombine.cpp | Deleted obsolete Ch8 canonicalization implementation. |
| mlir/example/Ch8/mlir/ShapeInferencePass.cpp | Deleted obsolete Ch8 pass implementation. |
| mlir/example/Ch8/mlir/MLIRGen.cpp | Deleted obsolete Ch8 MLIR generation implementation. |
| mlir/example/Ch8/mlir/LowerToLLVM.cpp | Deleted obsolete Ch8 lowering to LLVM implementation. |
| mlir/example/Ch8/mlir/LowerToAffineLoops.cpp | Deleted obsolete Ch8 lowering to affine implementation. |
| mlir/example/Ch8/matmul.toy.mlir | Deleted obsolete Ch8 MLIR input sample. |
| mlir/example/Ch8/matmul.toy | Deleted obsolete Ch8 source input sample. |
| mlir/example/Ch8/include/toy/ShapeInferenceInterface.td | Deleted obsolete Ch8 interface TableGen. |
| mlir/example/Ch8/include/toy/ShapeInferenceInterface.h | Deleted obsolete Ch8 generated interface header. |
| mlir/example/Ch8/include/toy/Passes.h | Deleted obsolete Ch8 pass declarations. |
| mlir/example/Ch8/include/toy/Parser.h | Deleted obsolete Ch8 parser header. |
| mlir/example/Ch8/include/toy/Ops.td | Deleted obsolete Ch8 ops TableGen. |
| mlir/example/Ch8/include/toy/MLIRGen.h | Deleted obsolete Ch8 MLIRGen header. |
| mlir/example/Ch8/include/toy/Lexer.h | Deleted obsolete Ch8 lexer header. |
| mlir/example/Ch8/include/toy/Dialect.h | Deleted obsolete Ch8 dialect header. |
| mlir/example/Ch8/include/toy/CMakeLists.txt | Deleted obsolete Ch8 TableGen build definitions. |
| mlir/example/Ch8/include/toy/AST.h | Deleted obsolete Ch8 AST header. |
| mlir/example/Ch8/include/CMakeLists.txt | Deleted obsolete Ch8 include subdir build hook. |
| mlir/example/Ch8/CMakeLists.txt | Deleted obsolete Ch8 executable build definition. |
| mlir/example/Ch7/toyc.cpp | Update includes/registry setup; make helpers static; register LLVM inliner interface. |
| mlir/example/Ch7/parser/AST.cpp | Make printLitHelperstatic. |
| mlir/example/Ch7/mlir/ShapeInferencePass.cpp | Switch to DebugLog (LDBG) and add pass argument name. |
| mlir/example/Ch7/mlir/MLIRGen.cpp | Update op creation patterns to Op::create; minor API updates. |
| mlir/example/Ch7/mlir/LowerToLLVM.cpp | Convert patterns to OpConversionPattern; update builder calls; add pass argument. |
| mlir/example/Ch7/mlir/LowerToAffineLoops.cpp | Convert patterns to OpConversionPattern; refactor loop lowering helper; add pass argument. |
| mlir/example/Ch7/mlir/Dialect.cpp | Update builder usage to Op::create; adjust call interface setter cast. |
| mlir/example/Ch7/include/toy/Ops.td | Update comments/builders to reflect Op::create usage; update GenericCallOp signature fields. |
| mlir/example/Ch7/include/toy/Lexer.h | Add missing <cstdlib> include. |
| mlir/example/Ch6/toyc.cpp | Same modernizations as Ch7 (includes, static helpers, inliner interface registration). |
| mlir/example/Ch6/parser/AST.cpp | Make printLitHelperstatic. |
| mlir/example/Ch6/mlir/ShapeInferencePass.cpp | Switch to DebugLog (LDBG) and add pass argument name. |
| mlir/example/Ch6/mlir/MLIRGen.cpp | Update op creation patterns to Op::create; minor API updates. |
| mlir/example/Ch6/mlir/LowerToLLVM.cpp | Convert patterns to OpConversionPattern; update builder calls; add pass argument. |
| mlir/example/Ch6/mlir/LowerToAffineLoops.cpp | Convert patterns to OpConversionPattern; refactor loop lowering helper; add pass argument. |
| mlir/example/Ch6/mlir/Dialect.cpp | Update builder usage to Op::create; adjust call interface setter cast. |
| mlir/example/Ch6/include/toy/Ops.td | Update comments/builders to reflect Op::create usage; update GenericCallOp signature fields. |
| mlir/example/Ch6/include/toy/Lexer.h | Add missing <cstdlib> include. |
| mlir/example/Ch5/toyc.cpp | Update includes and make helper functions static. |
| mlir/example/Ch5/parser/AST.cpp | Make printLitHelperstatic. |
| mlir/example/Ch5/mlir/ShapeInferencePass.cpp | Switch to DebugLog (LDBG) and add pass argument name. |
| mlir/example/Ch5/mlir/MLIRGen.cpp | Update op creation patterns to Op::create; minor API updates. |
| mlir/example/Ch5/mlir/LowerToAffineLoops.cpp | Convert patterns to OpConversionPattern; refactor loop lowering helper; add pass argument. |
| mlir/example/Ch5/mlir/Dialect.cpp | Update builder usage to Op::create; adjust call interface setter cast. |
| mlir/example/Ch5/include/toy/Ops.td | Update comments/builders to reflect Op::create usage; update GenericCallOp signature fields. |
| mlir/example/Ch5/include/toy/Lexer.h | Add missing <cstdlib> include. |
| mlir/example/Ch4/toyc.cpp | Make helper functions static. |
| mlir/example/Ch4/parser/AST.cpp | Make printLitHelperstatic. |
| mlir/example/Ch4/mlir/ShapeInferencePass.cpp | Switch to DebugLog (LDBG) and add pass argument name. |
| mlir/example/Ch4/mlir/MLIRGen.cpp | Update op creation patterns to Op::create; minor API updates. |
| mlir/example/Ch4/mlir/Dialect.cpp | Update builder usage to Op::create; adjust call interface setter cast. |
| mlir/example/Ch4/include/toy/Ops.td | Update comments/builders to reflect Op::create usage; update GenericCallOp signature fields. |
| mlir/example/Ch4/include/toy/Lexer.h | Add missing <cstdlib> include. |
| mlir/example/Ch3/toyc.cpp | Make helper functions static. |
| mlir/example/Ch3/parser/AST.cpp | Make printLitHelperstatic. |
| mlir/example/Ch3/mlir/MLIRGen.cpp | Update op creation patterns to Op::create; minor API updates. |
| mlir/example/Ch3/include/toy/Ops.td | Update comments/builders to reflect Op::create usage. |
| mlir/example/Ch3/include/toy/Lexer.h | Add missing <cstdlib> include. |
| mlir/example/Ch2/toyc.cpp | Make helper functions static. |
| mlir/example/Ch2/parser/AST.cpp | Make printLitHelperstatic. |
| mlir/example/Ch2/mlir/MLIRGen.cpp | Update op creation patterns to Op::create; minor API updates. |
| mlir/example/Ch2/include/toy/Ops.td | Update comments/builders to reflect Op::create usage. |
| mlir/example/Ch2/include/toy/Lexer.h | Add missing <cstdlib> include. |
| mlir/example/Ch1/toyc.cpp | Make helper function static. |
| mlir/example/Ch1/parser/AST.cpp | Make printLitHelperstatic. |
| mlir/example/Ch1/include/toy/Lexer.h | Add missing <cstdlib> include. |
| mlir/example/CMakeLists.txt | Remove Ch8 from the build. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+24
to
33
| // Define the new operation. By convention, prefix its name with the name of the dialect | ||
| // extension, "my.". The full operation name will be further prefixed with "transform.". | ||
| def ChangeCallTargetOp : Op<Transform_Dialect, "my.change_call_target", | ||
| // Indicate that the operation implements the required TransformOpInterface and | ||
| // MemoryEffectsOpInterface. Use the TransformEach trait to provide the | ||
| // MemoryEffectsOpInterface. Use the TransformEach trait to provide the | ||
| // implementation for TransformOpInterface. | ||
| [TransformOpInterface, TransformEachOpTrait, | ||
| DeclareOpInterfaceMethods<MemoryEffectsOpInterface>]> { | ||
| // Provide a brief and a full description. It is recommended that the latter describes | ||
| // Provide a brief and a full description. It is recommended that the latter describes | ||
| // the effects on the operands and how the operation processes various failure modes. |
There was a problem hiding this comment.
Several comment lines in this hunk now end with trailing whitespace. The repo has a trailing-whitespace pre-commit hook enabled, so these will cause CI/local hooks to fail. Please strip the trailing spaces on these lines.
Comment on lines
+22
to
30
| // Define the new operation. By convention, prefix its name with the name of the dialect | ||
| // extension, "my.". The full operation name will be further prefixed with "transform.". | ||
| def ChangeCallTargetOp : Op<Transform_Dialect, "my.change_call_target", | ||
| // Indicate that the operation implements the required TransformOpInterface and | ||
| // MemoryEffectsOpInterface. | ||
| [DeclareOpInterfaceMethods<TransformOpInterface>, | ||
| DeclareOpInterfaceMethods<MemoryEffectsOpInterface>]> { | ||
| // Provide a brief and a full description. It is recommended that the latter describes | ||
| // Provide a brief and a full description. It is recommended that the latter describes | ||
| // the effects on the operands and how the operation processes various failure modes. |
There was a problem hiding this comment.
Several comment lines in this hunk now end with trailing whitespace. The repo has a trailing-whitespace pre-commit hook enabled, so these will cause CI/local hooks to fail. Please strip the trailing spaces on these lines.
Comment on lines
+22
to
+27
| if [[ -f "/usr/bin/git" ]]; then | ||
| WORKSPACEROOT=$(git rev-parse --show-toplevel)/mlir/example || WORKSPACEROOT=`pwd` | ||
| fi | ||
| cd ${WORKSPACEROOT} | ||
There was a problem hiding this comment.
WORKSPACEROOT is only set when /usr/bin/git exists; on systems where git is installed elsewhere (or missing), WORKSPACEROOT remains unset and cd ${WORKSPACEROOT} will fail. Consider initializing WORKSPACEROOT unconditionally (e.g., derive it from the script location, and/or use command -v git), and quote it when cd-ing to handle spaces safely.
Comment on lines
53
to
56
| cmake -GNinja \ | ||
| "-H$LLVM_SRC_DIR/llvm" \ | ||
| "-B$build_dir" \ | ||
| "-Hllvm" \ | ||
| "-B$build_dir" \ | ||
| -DCMAKE_BUILD_TYPE=Debug \ |
There was a problem hiding this comment.
The CMake invocation is passing "-H llvm" and "-B $build_dir" as single arguments that contain spaces. CMake expects -S/-B (or legacy -H/-B) to be separate tokens (e.g., -S llvm -B "$build_dir"), otherwise argument parsing can break and the build won’t configure.
Comment on lines
+27
to
28
| // The usual components of a type such as description, mnemonic and assembly format | ||
| // should be provided. |
There was a problem hiding this comment.
This line introduces trailing whitespace, which will fail the repo’s trailing-whitespace pre-commit hook. Please remove the extra space at the end of the comment line.
| -DLLVM_ENABLE_LLD=OFF \ | ||
| -DLLVM_ENABLE_BACKTRACES=OFF \ | ||
| -DLLVM_INCLUDE_UTILS=ON \ | ||
| -DCMAKE_INSTALL_PREFIX=${install_dir} \ |
There was a problem hiding this comment.
install_dir is derived from script arguments or WORKSPACEROOT and is expanded unquoted in -DCMAKE_INSTALL_PREFIX=${install_dir}, which allows shell metacharacters in that value to break out of the cmake command. If this script is ever invoked with install_dir (e.g., the second CLI argument) influenced by untrusted input, an attacker could supply a value like /tmp/x; rm -rf / and cause arbitrary commands to execute with the script's privileges. Quote or otherwise sanitize install_dir (and other similar variables) before expanding them into shell commands so that characters like ;, &, and backticks cannot be interpreted by the shell.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.