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

[SandboxIR][Doc] Add a Doc file for Sandbox IR #98691

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

vporpo
Copy link
Contributor

@vporpo vporpo commented Jul 12, 2024

This is under User Guides > Additional Topics > Sandbox IR.

This is under User Guides > Additional Topics > Sandbox IR.
@tschuett
Copy link
Member

I don't mind if you don't want to link the RFC. A hint that will it will be the foundation for a new SLP vectorizer would be nice.

@vporpo
Copy link
Contributor Author

vporpo commented Jul 12, 2024

Well, the IR itself is decoupled and not vectorizer-specific. We can link to the RFC once we start adding the Vector-specific extensions.

Sandbox IR is an IR layer on top of LLVM IR that allow you to save/restore its state.

# API
The Sandbox IR API is designed to make it feel like LLVM.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The Sandbox IR API is designed to make it feel like LLVM.
The Sandbox IR API is designed to feel like LLVM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


# API
The Sandbox IR API is designed to make it feel like LLVM.
So Sandbox IR replicates many common API classes and functions to mirror the LLVM API.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
So Sandbox IR replicates many common API classes and functions to mirror the LLVM API.
, replicating many common API classes and functions to mirror the LLVM API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

# API
The Sandbox IR API is designed to make it feel like LLVM.
So Sandbox IR replicates many common API classes and functions to mirror the LLVM API.
The class hierarchy is quite similar (but in the `sandboxir` namespace), for example here is a small part of it:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The class hierarchy is quite similar (but in the `sandboxir` namespace), for example here is a small part of it:
The class hierarchy is similar but in the `llvm::sandboxir` namespace. For example:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

}
```

Sandbox IR is also using a similar type system with `isa<>, cast<>, dyn_cast<>`.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd drop this line, too detailed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

- Reverse mapping: LLVM IR Value -> Sandbox IR Value
This mapping is stored in `sandboxir::Context::LLVMValueToValue`.

For example `sandboxir::User::getOperand(OpIdx)` for a `sandboxir::User * U` works as follows:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
For example `sandboxir::User::getOperand(OpIdx)` for a `sandboxir::User * U` works as follows:
For example `sandboxir::User::getOperand(OpIdx)` for a `sandboxir::User *U` works as follows:

and below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

- It makes sure that Sandbox IR and LLVM IR are always in sync, which helps avoid bugs and removes the need for a lowering step.
- No need for serialization/de-serialization infrastructure as we can rely on LLVM IR for it.

The implementation is straight-forward:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The implementation is straight-forward:
The implementation is straightforward:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

For example, for `sandboxir::User::setOperand(OpIdx, sandboxir::Value *Op)`:
- We get the corresponding LLVM User: `llvm::User *LLVMU = cast<llvm::User>(Val)`
- Next we get the corresponding LLVM Operand: `llvm::Value *LLVMOp = Op->Val`
- Finally we modify `LLVMU`'s operand: `LLVMU->setOperand(OpIdx, LLVMOp)`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Finally we modify `LLVMU`'s operand: `LLVMU->setOperand(OpIdx, LLVMOp)`.
- Finally we modify `LLVMU`'s operand: `LLVMU->setOperand(OpIdx, LLVMOp)`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

# Design

## Sandbox IR Value <-> LLVM IR Value Mapping
LLVM IR and Sandbox IR are always in sync.
Copy link
Contributor

Choose a reason for hiding this comment

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

this sentence seems out of place. either explain it more (maybe earlier), combine it with the Sandbox IR is Write-Through section

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I expanded on that a bit.

@@ -288,3 +290,6 @@ Additional Topics

:doc:`RISCV/RISCVVectorExtension`
This document describes how the RISC-V Vector extension can be expressed in LLVM IR and how code is generated for it in the backend.

:doc:`Sandbox IR <SandboxIR>`
Describes the design and usage of Sandbox IR, a transactional layer over LLVM IR.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Describes the design and usage of Sandbox IR, a transactional layer over LLVM IR.
This document describes the design and usage of Sandbox IR, a transactional layer over LLVM IR.

(for consistency with above)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Collaborator

@slackito slackito left a comment

Choose a reason for hiding this comment

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

LGTM. I assume further details on the save/restore mechanism will be added when the corresponding code lands, right?

@@ -0,0 +1,52 @@
# Sandbox IR: A transactional layer over LLVM IR

Sandbox IR is an IR layer on top of LLVM IR that allow you to save/restore its state.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Sandbox IR is an IR layer on top of LLVM IR that allow you to save/restore its state.
Sandbox IR is an IR layer on top of LLVM IR that allows you to save/restore its state.

@vporpo
Copy link
Contributor Author

vporpo commented Jul 13, 2024

Yes, we will keep adding things to the doc as we are adding more functionality to the code.


## Sandbox IR Value <-> LLVM IR Value Mapping
Each LLVM IR Value maps to a single Sandbox IR Value.
The reverse is also true in most cases, except for multi-Instruction Sandbox IR Instructions that may be defined in extensions the Sandbox IR.
Copy link
Member

Choose a reason for hiding this comment

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

"except for multi-Instruction Sandbox IR Instructions that may be defined in extensions the Sandbox IR."

Could you rephrase?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that was pretty bad :P I rephrased it. Though I'm not sure how to best differentiate the "base" SandboxIR and the future extensions.

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.

None yet

5 participants