Skip to content
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

Use replace_call!() to replace Expr(:call, ...) values #180

Merged
merged 1 commit into from
Jul 11, 2023

Conversation

staticfloat
Copy link
Collaborator

This is necessary to prevent the callinfo field from falling out of sync with the call itself, causing future optimization passes (such as inlining) to compute incorrect results.

This is necessary to prevent the callinfo field from falling out of sync
with the call itself, causing future optimization passes (such as inlining)
to compute incorrect results.
@staticfloat staticfloat requested review from Keno and oxinabox and removed request for Keno July 10, 2023 21:56
@staticfloat
Copy link
Collaborator Author

@oxinabox Can I give you the task of finding other places within Diffractor where we replace Expr(:call, ..) values, and use replace_call!() there as well? We don't have to do this for IR statements we're inserting, only ones we're overwriting.

@codecov
Copy link

codecov bot commented Jul 10, 2023

Codecov Report

Patch coverage: 62.50% and project coverage change: +0.01 🎉

Comparison is base (dc3c60e) 52.42% compared to head (077de69) 52.44%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #180      +/-   ##
==========================================
+ Coverage   52.42%   52.44%   +0.01%     
==========================================
  Files          27       27              
  Lines        2722     2723       +1     
==========================================
+ Hits         1427     1428       +1     
  Misses       1295     1295              
Impacted Files Coverage Δ
src/codegen/forward_demand.jl 44.74% <25.00%> (+0.15%) ⬆️
src/stage1/compiler_utils.jl 95.65% <100.00%> (+0.41%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@oxinabox
Copy link
Member

sure, will do

@oxinabox oxinabox self-assigned this Jul 11, 2023
@oxinabox
Copy link
Member

Having looked at every instance of Expr(:call and :inst] and stmt I can't find any others
there is one that replaces it in a IncrementalCompact but i think that is fine.
the rest are inserts.

@oxinabox oxinabox merged commit e308c82 into main Jul 11, 2023
6 checks passed
@oxinabox oxinabox deleted the sf/replace_call branch July 11, 2023 05:52
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.

2 participants