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

Introduce __smo intrinsic #3564

Merged
merged 21 commits into from
Dec 14, 2022
Merged

Introduce __smo intrinsic #3564

merged 21 commits into from
Dec 14, 2022

Conversation

mohammadfawaz
Copy link
Contributor

@mohammadfawaz mohammadfawaz commented Dec 9, 2022

Closes #3457

Following how __log works for the most part.

Tasks:

Nothing particularly special about this change except for IR gen. The new intrinsic has the following signature:

__smo<T>(recipient: b256, data: T, output_index: u64, coins: u64)

while the IR instruction has the following signature:

smo recipient_and_message, message_size, output_index, coins

which matches the smo opcode. IR gen builds recipient_and_message by creating a new struct where the first field is the recipient (a b256), the second field is the smo ID (just like log IDs, but we don't have a special register to use here so we need to use the message data), and the third field is the actual user data passed in via the intrinsic:

struct recipient_and_message {
    recipient: b256,
    smo_id: u64,
    user_data: T
}

The smo_id above is the same as the one used in the JSON ABI for each smo.

Assembly builder simply convert the IR instruction into an smo opcode directly.

Finally, add a send_typed_message to the standard library that allows sending arbitrary types and added an SDK test for it. The SDK can't decode the types yet so I had to check for raw bytes.

@mohammadfawaz mohammadfawaz self-assigned this Dec 9, 2022
@mohammadfawaz mohammadfawaz added compiler: ir IRgen and sway-ir including optimization passes language feature Core language features visible to end users ABI Everything to do the ABI, especially the JSON representation compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen compiler: parser Everything to do with the parser compiler: codegen Everything to do with IR->ASM, register allocation, etc. labels Dec 9, 2022
mohammadfawaz added a commit to FuelLabs/fuels-rs that referenced this pull request Dec 12, 2022
This is needed for FuelLabs/sway#3409.
Basically, we want to be able to decode the data sent via `smo`, just
like logs, so messages need to store information in the JSON ABI.

Doesn't look like this impacts anything at the moment as everything
compiles fine.

Spec change: FuelLabs/fuel-specs#444
Compiler change: FuelLabs/sway#3564 that needs
this (currently using a local copy of `fuels-rs`)
- IR + asm unit test
- E2E test to check the JSON ABI
- Enhance IR verifier
- Documentation of __smo in the Sway Book
@mohammadfawaz mohammadfawaz marked this pull request as ready for review December 12, 2022 21:15
@mohammadfawaz mohammadfawaz requested a review from a team December 12, 2022 21:15
@mohammadfawaz mohammadfawaz marked this pull request as draft December 12, 2022 22:09
@mohammadfawaz mohammadfawaz marked this pull request as ready for review December 13, 2022 03:27
@mohammadfawaz mohammadfawaz marked this pull request as draft December 13, 2022 03:59
@mohammadfawaz
Copy link
Contributor Author

Sorry for the trivial JSON ABI file diffs, again

@mohammadfawaz mohammadfawaz marked this pull request as ready for review December 13, 2022 04:32
Copy link
Contributor

@otrho otrho left a comment

Choose a reason for hiding this comment

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

Do we need to add an SDK test to make sure the message actually gets through correctly? Is that possible?

docs/book/src/reference/compiler_intrinsics.md Outdated Show resolved Hide resolved
sway-core/src/lib.rs Outdated Show resolved Hide resolved
vaivaswatha
vaivaswatha previously approved these changes Dec 13, 2022
Copy link
Contributor

@vaivaswatha vaivaswatha left a comment

Choose a reason for hiding this comment

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

Looks fine to me (after Toby's comments). So approving in advance.

Co-authored-by: Toby Hutton <toby@grusly.com>
@mohammadfawaz
Copy link
Contributor Author

Do we need to add an SDK test to make sure the message actually gets through correctly? Is that possible?

Likely better to do it here so I did. I originally thought it can be done in #3577 but might as well get it done.

@mohammadfawaz mohammadfawaz requested review from nfurfaro and a team December 13, 2022 17:00
nfurfaro
nfurfaro previously approved these changes Dec 13, 2022
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.

I just left 1 question but looks great overall!

@otrho otrho requested a review from nfurfaro December 14, 2022 01:19
@mohammadfawaz mohammadfawaz enabled auto-merge (squash) December 14, 2022 01:53
@mohammadfawaz mohammadfawaz merged commit 967f223 into master Dec 14, 2022
@mohammadfawaz mohammadfawaz deleted the mohammadfawaz/3457 branch December 14, 2022 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ABI Everything to do the ABI, especially the JSON representation compiler: codegen Everything to do with IR->ASM, register allocation, etc. compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen compiler: ir IRgen and sway-ir including optimization passes compiler: parser Everything to do with the parser language feature Core language features visible to end users
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introduce an __smo intrinsic and messages types in the JSON ABI
4 participants