-
Notifications
You must be signed in to change notification settings - Fork 220
Add helper for specializing parametric functions #3141
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
base: main
Are you sure you want to change the base?
Conversation
|
could this potentially also be used to add a flag to |
@proppy yeah, are you thinking about using a parametric function as the top function, and specialize with some parameters? |
|
This PR is now blocked by #3061 as the test suite is referring to functions defined there |
👍 yes, this can easily be done w/ a wrapper function today, but is cumbersome to do w/o code generation when you want to instanciate a lot of function variants w/ different sizes. /cc @richmckeever |
Yeah. I used to do code generation to serve my purpose, but replacing all the invocation sites is hard to do without a programmatic way to manipulate the DSLX. |
|
@richmckeever Any chance you'll be able to take a look soon? |
661bd6d to
29d2794
Compare
|
@mikex-oss Has just resolved the conflict. Please have a look. Thanks! |
d5d5971 to
b8e7f88
Compare
proppy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re-approving on behalf of @dplassgit
| std::filesystem::path module_path = module->fs_path().value_or( | ||
| std::filesystem::path("invocation_test.x")); | ||
|
|
||
| XLS_ASSERT_OK_AND_ASSIGN( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this fails internal checks w/:
xls/dslx/frontend/function_specializer_test.cc:299
TypecheckModule(std::move(cloned_module), module_path.string(), tc_import_data.get()) returned error: INTERNAL: XLS_RET_CHECK failure (xls/dslx/type_system_v2/inference_table.cc:1062) span_filename == module.fs_path()->generic_string() (<specialization:invocation_test:select_poly_34@> vs. invocation_test.x) MakeTypeCheckedNumber span filename must match module fs_path; module name: `invocation_test`; span: `<specialization:invocation_test:select_poly_34@0x5234ffeb63a8>:12:1-15:2`; module fs_path: `invocation_test.x`
=== Source Location Trace: ===
xls/common/status/status_builder.cc:197
xls/dslx/type_system_v2/inference_table.cc:1062
xls/dslx/type_system_v2/inference_table_converter_impl.cc:1756
xls/dslx/type_system_v2/inference_table_converter_impl.cc:720
xls/dslx/type_system_v2/inference_table_converter_impl.cc:241
xls/dslx/parse_and_typecheck.cc:153
xls/dslx/frontend/function_specializer_test.cc:299
|
@lsrcz can you rebase? |
proppy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to rebase and handle conflict in order to be able to import this.
b8e7f88 to
c9328b0
Compare
|
Hi @proppy, I have just rebased the PR and resolved the conflicts. Please take a look. Thank you! |
c9328b0 to
eb1cf4f
Compare
This pull request adds a helper for specializing a parametric function with a given environment. The resulting function will be inserted back to the source module.
The C API exposed a "batched" specializing helper that takes an typechecked module and return a cloned typechecked module with all specialized functions inserted. This is needed as our current use of C APIs is assuming that a module is immutable.