Skip to content

Conversation

@teqdruid
Copy link
Contributor

Adds the ability for users to load the MLIR file and preprocess pre-lowering ops.

Adds the ability for users to load the MLIR file and preprocess
pre-lowering ops.
@teqdruid teqdruid requested a review from Copilot June 30, 2025 19:26
@teqdruid teqdruid added the PyCDE Python CIRCT Design Entry API label Jun 30, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a hook to System.import_mlir allowing users to preprocess or skip MLIR operations before lowering.
Key changes:

  • Introduce a preprocess_op callback parameter in import_mlir
  • Update the import loop to apply preprocess_op and skip None results
  • Extend the existing test to verify skipping an extern module via preprocess_op

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
frontends/PyCDE/test/test_import_hw_modules.py Remove unused IrModule import and add a preprocess_op test
frontends/PyCDE/src/pycde/system.py Add preprocess_op parameter to import_mlir signature and implement preprocessing logic
Comments suppressed due to low confidence (1)

frontends/PyCDE/test/test_import_hw_modules.py:65

  • The test references sys.argv but sys is not imported in this file. Add import sys at the top to avoid a NameError.
system = System([Top], output_directory=sys.argv[1])

Comment on lines +167 to +168
preprocess_op: Optional[Callable[[ir.OpView],
Optional[ir.OpView]]] = None,
Copy link

Copilot AI Jun 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The function signature line for preprocess_op is very long and exceeds typical line-length recommendations. Consider breaking it into multiple lines or using a type alias for readability.

Suggested change
preprocess_op: Optional[Callable[[ir.OpView],
Optional[ir.OpView]]] = None,
preprocess_op: PreprocessOpType = None,

Copilot uses AI. Check for mistakes.
@teqdruid teqdruid merged commit 23d2665 into main Jun 30, 2025
8 checks passed
@teqdruid teqdruid deleted the teqdruid/pycde-import-preprocess branch June 30, 2025 19:43
TaoBi22 pushed a commit to TaoBi22/circt that referenced this pull request Jul 17, 2025
Adds the ability for users to load the MLIR file and preprocess
or skip pre-lowering ops.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PyCDE Python CIRCT Design Entry API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants