Skip to content

Conversation

@lsrcz
Copy link
Collaborator

@lsrcz lsrcz commented Sep 17, 2025

This pull request adds a flag to the DSLX-to-IR converter that forces all functions to use the implicit-token calling convention.
We need this to enable IR transformations that insert assertions into existing programs, which requires converting the function to the implicit-token convention.
Previously, this was impossible because the activation bit could not be inferred from the converted IR alone.
To support this behavior, we introduce a new flag that instructs the converter to generate implicit tokens for all functions.

This includes a breaking change to the C API to include the flag.

@lsrcz lsrcz requested a review from cdleary September 17, 2025 23:59
GetModuleMemberTypeName(*member.value()), node->identifier(),
node->span().ToString(file_table())));
}},
Visitor{[&](Function* f) { return DefAlias(f, /*to=*/node); },
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did this change? hard to see

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is an auto formatter artifact and I reverted it.

@lsrcz
Copy link
Collaborator Author

lsrcz commented Sep 21, 2025

The CI failure does not seem to be related.

xls/dslx/cpp_transpiler/cpp_transpiler_test.cc: Found non-absolute includes:
  path/to/some_types.h
  path/to/other_types.h
  /tmp/fake_path.h
  path/to/some_types.h
  path/to/other_types.h

@proppy proppy requested a review from huangjd October 7, 2025 06:26
@proppy
Copy link
Member

proppy commented Oct 8, 2025

@lsrcz can you confirm which version of xlsynth is compatible with this change?

@lsrcz
Copy link
Collaborator Author

lsrcz commented Oct 8, 2025

@proppy This one was on top of commit 3646c63. Should I update to the latest main branch?

@proppy
Copy link
Member

proppy commented Oct 9, 2025

@proppy This one was on top of commit 3646c63. Should I update to the latest main branch?

Sorry I meant which version of the xlsynth's rust crate is compatible w/ this change.

@lsrcz
Copy link
Collaborator Author

lsrcz commented Oct 10, 2025

@proppy I think it is compatible with xlsynth's rust crate since v0.6.0.

@proppy
Copy link
Member

proppy commented Oct 10, 2025

@proppy I think it is compatible with xlsynth's rust crate since v0.6.0.

understood, will make sure we import v0.6.0 before landing this /cc @hongted

@copybara-service copybara-service bot merged commit 0428080 into google:main Oct 13, 2025
4 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants