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

vporpo · 2024-07-12T20:32:04Z

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

tschuett · 2024-07-12T20:39:25Z

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 · 2024-07-12T20:45:49Z

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

aeubanks · 2024-07-12T22:49:53Z

llvm/docs/SandboxIR.md

+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.


Suggested change

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

The Sandbox IR API is designed to feel like LLVM.

aeubanks · 2024-07-12T22:50:50Z

llvm/docs/SandboxIR.md

+
+# 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.


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.

aeubanks · 2024-07-12T22:51:31Z

llvm/docs/SandboxIR.md

+# 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:


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:

aeubanks · 2024-07-12T22:52:02Z

llvm/docs/SandboxIR.md

+}
+```
+
+Sandbox IR is also using a similar type system with `isa<>, cast<>, dyn_cast<>`.


I'd drop this line, too detailed

aeubanks · 2024-07-12T23:03:45Z

llvm/docs/SandboxIR.md

+- 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:


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

aeubanks · 2024-07-12T23:07:43Z

llvm/docs/SandboxIR.md

+- 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:


Suggested change

The implementation is straight-forward:

The implementation is straightforward:

aeubanks · 2024-07-12T23:08:14Z

llvm/docs/SandboxIR.md

+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)`.


Suggested change

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

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

aeubanks · 2024-07-12T23:09:28Z

llvm/docs/SandboxIR.md

+# Design
+
+## Sandbox IR Value <-> LLVM IR Value Mapping
+LLVM IR and Sandbox IR are always in sync.


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

I expanded on that a bit.

aeubanks · 2024-07-12T23:10:17Z

llvm/docs/UserGuides.rst

@@ -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.


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)

slackito

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

slackito · 2024-07-13T00:13:00Z

llvm/docs/SandboxIR.md

@@ -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.


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 · 2024-07-13T00:23:06Z

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

tmsri · 2024-07-13T01:07:19Z

llvm/docs/SandboxIR.md

+
+## 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.


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

Could you rephrase?

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.

maybe add a separate section about extensions?

aeubanks · 2024-07-15T17:10:09Z

llvm/docs/SandboxIR.md

+This mapping is stored in `sandboxir::Context::LLVMValueToValue`.
+
+For example `sandboxir::User::getOperand(OpIdx)` for a `sandboxir::User *U` works as follows:
+- First we find the LLVM User: `llvm::User * LLVMU = U->Val`.


Suggested change

- First we find the LLVM User: `llvm::User * LLVMU = U->Val`.

- First we find the LLVM User: `llvm::User *LLVMU = U->Val`.

aeubanks · 2024-07-15T17:12:21Z

llvm/docs/SandboxIR.md

+- 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 straightforward:


maybe remove this line? it doesn't add very much and the colon implies the next line is the start of a list, which it isn't here

aeubanks · 2024-07-15T17:13:00Z

llvm/docs/SandboxIR.md

+Sandbox IR is designed to rely on LLVM IR for its state.
+So any change made to Sandbox IR objects directly updates the corresponding LLVM IR.
+
+This has the following benefits:


do you want to mention that you can pass llvm::Instructions to various cost modeling APIs? or is that too specific to put here?

Yeah I can add this point.

aeubanks

lg with comments, thanks for writing this up

llvm/docs/SandboxIR.md

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

vporpo requested review from slackito, echristo, tschuett, aeubanks and tmsri July 12, 2024 20:32

aeubanks reviewed Jul 12, 2024

View reviewed changes

slackito approved these changes Jul 13, 2024

View reviewed changes

tmsri reviewed Jul 13, 2024

View reviewed changes

vporpo force-pushed the SBDoc branch from 012ba59 to b931be8 Compare July 13, 2024 02:18

aeubanks reviewed Jul 15, 2024

View reviewed changes

aeubanks approved these changes Jul 15, 2024

View reviewed changes

llvm/docs/SandboxIR.md Outdated Show resolved Hide resolved

llvm/docs/SandboxIR.md Show resolved Hide resolved

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

35a2996

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

vporpo force-pushed the SBDoc branch from 65c3668 to 35a2996 Compare July 15, 2024 20:35

vporpo merged commit 4a23d41 into llvm:main Jul 15, 2024
6 of 7 checks passed

vporpo deleted the SBDoc branch July 17, 2024 17:26

yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024

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

9c42983

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

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

	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.

	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:

	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:

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

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

	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.

		@@ -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.

	- First we find the LLVM User: `llvm::User * LLVMU = U->Val`.
	- First we find the LLVM User: `llvm::User *LLVMU = U->Val`.

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

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

Uh oh!

Conversation

vporpo commented Jul 12, 2024

Uh oh!

tschuett commented Jul 12, 2024

Uh oh!

vporpo commented Jul 12, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

slackito left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vporpo commented Jul 13, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aeubanks left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!