Add Op registration to symbol table inside OpBuilder #4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…gument Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
…ctly but is only debug Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
…row checker issues Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
lima-limon-inc
commented
May 21, 2025
hir-macros/src/operation.rs
Outdated
Comment on lines
267
to
288
| Some(OperationFieldType::Jamon) => { | ||
| // std::dbg!("MANTECA MANTECA"); | ||
| // std::dbg!(&field_ty); | ||
| create_params.push(OpCreateParam { | ||
| param_ty: OpCreateParamType::CustomField(field_name.clone(), field_ty), | ||
| r#default: field.attrs.default.is_present(), | ||
| }); | ||
| // TODO: Remove from fields, only used when building | ||
| named_fields.push(syn::Field { | ||
| attrs: field.attrs.forwarded, | ||
| vis: field.vis, | ||
| mutability: syn::FieldMutability::None, | ||
| ident: Some(field_name.clone()), | ||
| colon_token: Some(syn::token::Colon(field_span)), | ||
| ty: field.ty, | ||
| }); | ||
| self.parent_jamon = Some(OpJamon { | ||
| name: field_name, | ||
| // span: field_span, | ||
| }); | ||
| // continue; | ||
| } |
There was a problem hiding this comment.
Not used currently, left just in case
This reverts commit 80bd170.
This reverts commit 0e0dc1e.
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
…table Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
…e is now a trait Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
lima-limon-inc
force-pushed
the
fabrizioorsi/op-builder-symbol-table
branch
from
7a10a92 to
7a395a5
Compare
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
lima-limon-inc
commented
May 26, 2025
Comment on lines
1
to
12
| public builtin.function @test(v0: u32, v1: u32) -> u32 { | ||
| ^block0(v0: u32, v1: u32): | ||
| ^block1(v0: u32, v1: u32): | ||
| v2 = scf.index_switch v0 : u32 | ||
| case 0 { | ||
| ^block1: | ||
| ^block2: | ||
| v3 = arith.constant 0 : u32; | ||
| scf.yield v3; | ||
| } | ||
| default { | ||
| ^block2: | ||
| ^block3: | ||
| v4 = arith.mul v0, v1 : u32 #[overflow = checked]; | ||
| scf.yield v4; |
There was a problem hiding this comment.
These tests got updated because now they contain a WorldBuilder which wasn't there before. The only thing that (should)'ve changed is the block number (which increased by one).
… longer passed. Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
lima-limon-inc
force-pushed
the
fabrizioorsi/op-builder-symbol-table
branch
from
e019ae7 to
7c38701
Compare
… used. Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
…rait Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
lima-limon-inc
force-pushed
the
fabrizioorsi/op-builder-symbol-table
branch
from
2bbcc9f to
e224ec1
Compare
This reverts commit bdefabe.
This reverts commit edf7a65.
This reverts commit 5c13e5d.
This reverts commit 21eab6e. Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
lima-limon-inc
force-pushed
the
fabrizioorsi/op-builder-symbol-table
branch
from
0cdb2b0 to
50e8faa
Compare
|
Edit: I removed the struct |
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes issue number 404
Currently, building an operation and adding it to a symbol table are done separately. This PR makes operation insertion into a symbol table happen inside the Operation Builder, thus encapsulating it with Operation creation.
To enable it, a new trait
BelongsInSymbolTablewas created. Operations that have this trait are going to be stored inside the symbol table of a different operation.This meant that these operations needed a reference to their parent's symbol table at operation creation. To support this,
a new
OpCreateParamTypevariant was created calledBuildOnlyParameter. This is used for arguments that are only used in theOperation::creationandOperationBuilderfunctions, in this case a reference to the caller'sSymbolTable.Besides the addition of a new function parameter, the
#[operation]macro was expanded to handle Symbol Table insertion.Originally, Operations were built, verified and returned inside the
BuildOp::to_tokensmacro.However, adding conditional symbol-table insertion made the macro a bit too convoluted (see here).
So in order to make the flow more consistent and easier to follow, I went for splitting the macro up into 3 different stages:
BuildOp,ModifyOpandReturnOpBuildOpwas basically unchanged. The only change was that it now doesn't return the built op, it only builds it and stores it in a variable.ModifyOpwas created and is the place where the conditional symbol-table insertion logic resides. This is also "extensible", other post-build functions can be inserted inside it and should be able to simply "chain" their functionality. It can be seen as a "post build hook".ResultOp: This last macro is simply in charge of returning the op. I added an#[allow(dead_code)]simply to keep all the*Opmacros in the same format