Skip to content
This repository was archived by the owner on Oct 18, 2022. It is now read-only.

Expose kall to user ASM#22

Merged
ben-chain merged 6 commits intodevelop-0.7from
feat/0_7-inline-kall
Apr 21, 2021
Merged

Expose kall to user ASM#22
ben-chain merged 6 commits intodevelop-0.7from
feat/0_7-inline-kall

Conversation

@ben-chain
Copy link
Copy Markdown
Contributor

Description

This allows us to put kall directly into inline yul, by making the kall builtin use imaginary opcodes which don't get transpiled. (Still gets find-replaced at the end, of course!)

@snario snario requested review from K-Ho and smartcontracts April 13, 2021 16:11
@snario
Copy link
Copy Markdown

snario commented Apr 13, 2021

Is there documentation somewhere on what kall is?

@smartcontracts
Copy link
Copy Markdown
Contributor

Ben explained the trick to me and it works. I think it would be worth explaining this in more detail in comments within the code though.

@ben-chain
Copy link
Copy Markdown
Contributor Author

Update: I am not gonna merge this one until we've used the soljson.js file in local testing with the contracts. This has been published here in a way that we can use a custom version before updating the main json!

@snario
Copy link
Copy Markdown

snario commented Apr 19, 2021

Can I propose that we come up with a different name than kall? Maybe em_call? It seems like kall is just a modification of call with a letter making the opcode name sound like the word "call", but it doesn't really say much about what this function is actually doing. This may be out of scope for this pull request.

@ben-chain
Copy link
Copy Markdown
Contributor Author

@snario I want to merge this PR now that it's confirmed working but agree naming could be improved. Maybe Yul IR compiler is the right time to make the switch.

@ben-chain ben-chain merged commit bfa0224 into develop-0.7 Apr 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants