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

Re-introduce the single arg contract call optimisation for IR. #1272

Merged
merged 1 commit into from
Apr 17, 2022

Conversation

otrho
Copy link
Contributor

@otrho otrho commented Apr 15, 2022

Re-introduce the special case where if there's a single argument to a function call it is embedded directly in the call frame rather than being a pointer to the args elsewhere.

This will fix for errors we currently get when trying to call some contract from the SDK.

I'm halfway through working out what else needs to be fixed for contract calls + SDK but I've been on the road a bit and haven't had time. So I thought I'd submit this PR now in case it's blocking anyone.

Closes #1138.

@otrho otrho force-pushed the otrho/fix_ir_contract_call_args branch from f7103c1 to 3bedde4 Compare April 15, 2022 12:18
@otrho otrho self-assigned this Apr 15, 2022
@otrho otrho added bug Something isn't working compiler General compiler. Should eventually become more specific as the issue is triaged lib: std Standard library labels Apr 15, 2022
@otrho otrho force-pushed the otrho/fix_ir_contract_call_args branch from 3bedde4 to ae3142e Compare April 15, 2022 13:01
@otrho
Copy link
Contributor Author

otrho commented Apr 15, 2022

Argh, the E2E test is failing, I assume because the code expects the single arg optimisation but the IR doesn't compile them. But I may be wrong... it's getting late for me, I'll revisit tomorrow.

Copy link
Contributor

@nfurfaro nfurfaro left a comment

Choose a reason for hiding this comment

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

Nice to see stdlib tests re-enabled! Needs another set of eyes on IR stuff as I'm not familiar enough with it to give a thorough review.

@otrho otrho force-pushed the otrho/fix_ir_contract_call_args branch 2 times, most recently from 86e45b6 to da47b75 Compare April 16, 2022 05:09
Re-introduce the special case where if there's a single argument to a
function call it is embedded directly in the call frame rather than
being a pointer to the args elsewhere.
@otrho otrho force-pushed the otrho/fix_ir_contract_call_args branch from da47b75 to e5064f8 Compare April 17, 2022 02:47
@otrho otrho marked this pull request as ready for review April 17, 2022 02:59
Copy link
Contributor

@mohammadfawaz mohammadfawaz left a comment

Choose a reason for hiding this comment

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

I looked at this carefully and I think it's reasonable.
image

@otrho otrho merged commit 61d77dd into master Apr 17, 2022
@otrho otrho deleted the otrho/fix_ir_contract_call_args branch April 17, 2022 23:50
Comment on lines +839 to +840
// the unnecessary copying eventually, though it feels like we're jumping
// through a bunch of hoops here (employing the single arg optimisation) for
Copy link
Contributor

Choose a reason for hiding this comment

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

@otrho is letting his political opinions leak into the code again

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's just an observation. No judgement. 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compiler General compiler. Should eventually become more specific as the issue is triaged lib: std Standard library
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Re-enable all the checks in the lib-std tests
4 participants