-
Notifications
You must be signed in to change notification settings - Fork 3
Ensure SourceInfo is preserved in inline pass and offsets are kepts #541
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
Conversation
|
kaihsin
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.
LGTM.
|
The CI is failing tho |
| f"Unsupported dialect `{stmt.dialect.name}` from statement {stmt.name}" | ||
| ) | ||
| self.curr_block.stmts.append(stmt) | ||
| if stmt.source is None: |
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.
since stmts.append calls Statement.insert which sets the source info, it's now important to set the source before.
kaihsin
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.
CI is failing
|
I broke my own unit test in the last commit. I'll fix it. |
|
The issue is actually subtle. Let's consider this kernel: @squin.kernel
def inline(q):
squin.correlated_qubit_loss(0.1, qubits=q)
@squin.kernel
def my_test():
q = squin.qubit.new(3)
inline(q)
squin.correlated_qubit_loss(0.1, qubits=q)
squin.x(q[0])I want the source info to correctly point to the So, the source info will always be coming from the inlined stdlib kernels, which is not helpful to the user. Ideally I would want inlining of source info to stop right before stdlib. A hacky way to do this is the following code in inline.py: if call_like.source is not None:
for block in inline_region.blocks:
if block.source is None or "stdlib" in block.source.file:
block.source = call_like.source
for stmt in block.stmts:
if stmt.source is None or "stdlib" in stmt.source.file:
stmt.source = call_like.sourceBut this is bad. Do you have any recommendations how to break the inline chain at the right spot? EDIT: The proper way would be to have a source info stack and a believe that's how MLIR handles this. |
Previously call_site.f_lineno was used but this definition changed by an offset of 1 in Python 3.11
You can verify this, e.g., by running this script:
class MyDecorator:
def __call__(self, func):
frame = inspect.currentframe()
call_site = frame.f_back
func_def_line = func.__code__.co_firstlineno
if sys.version_info < (3, 11):
assert call_site.f_lineno - func_def_line == 1
else:
assert call_site.f_lineno - func_def_line == 2
return func
decorator = MyDecorator()
@decorator # Line 26
def test_func(): # Line 27
pass
293cda1 to
d3756a1
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
…541) This PR ensures that source info is preserved after inlining or cloning in `.similar()`. Additionally I found that offsets were lost (which are now added in `lowering.py`). I tested all of these changes on kirin 0.17.30 together with bloqade-circuit. Blocks QuEraComputing/bloqade-circuit#552 Addresses #540
This PR ensures that source info is preserved after inlining or cloning in
.similar().Additionally I found that offsets were lost (which are now added in
lowering.py).I tested all of these changes on kirin 0.17.30 together with bloqade-circuit.
Blocks QuEraComputing/bloqade-circuit#552
Addresses #540