Skip to content

Add IR conversion feedback to the DSLX interpreter#3882

Open
magancarz wants to merge 1 commit intogoogle:mainfrom
antmicro:ir-conversion-test
Open

Add IR conversion feedback to the DSLX interpreter#3882
magancarz wants to merge 1 commit intogoogle:mainfrom
antmicro:ir-conversion-test

Conversation

@magancarz
Copy link
Copy Markdown
Contributor

This PR adds an IR conversion test to the DSLX interpreter enabled by lower_to_ir flag which is set by default. It performs the IR conversion check on every non-parametric proc or function and returns error if any fails. Tests can also be included in the check by adding convert_tests flag.

@rw1nkler
Copy link
Copy Markdown
Contributor

FYI @richmckeever

@proppy
Copy link
Copy Markdown
Member

proppy commented Mar 5, 2026

should we also add this to typecheck_main?

@proppy
Copy link
Copy Markdown
Member

proppy commented Mar 6, 2026

This also need to be rebased.

@magancarz magancarz force-pushed the ir-conversion-test branch from 26b3fee to 254cd57 Compare March 6, 2026 12:39
@magancarz
Copy link
Copy Markdown
Contributor Author

I've rebased changes on the latest main.

should we also add this to typecheck_main?

Sounds reasonable. @richmckeever what do you think?

@mikex-oss
Copy link
Copy Markdown
Collaborator

There seems to be no way to configure IR convert options through xls_dslx_test.

Rather than overloading xls_dslx_test in this way, what do we think about a separate xls_dslx_ir_convert_test or similar that just calls xls_dslx_ir + build_test? This is similar to how we have a separate xls_dslx_fmt_test, rather than doing it as check_formatting = True inside xls_dslx_test.

@magancarz
Copy link
Copy Markdown
Contributor Author

@mikex-oss The intent behind IR conversion check when running DSLX tests was to prevent situations where the user would spend time on developing DSLX code that wouldn't be possible to convert to IR, which is an issue that would surface only when the user would run a xls_dslx_ir target. Moving this task to another target would make these changes unnecessary. @richmckeever What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants