Add support for indexing register array#81
Open
ffengyu wants to merge 2 commits intorems-project:masterfrom
Open
Add support for indexing register array#81ffengyu wants to merge 2 commits intorems-project:masterfrom
ffengyu wants to merge 2 commits intorems-project:masterfrom
Conversation
bacam
reviewed
Jul 22, 2024
Contributor
bacam
left a comment
There was a problem hiding this comment.
It would be nice to have a test too. I've just written one; I'll see if I can push it to your branch...
| Field(Box<BlockLoc>, SSAName, SSAName), | ||
| Addr(Box<BlockLoc>), | ||
| // Index is not assignable and don't need SSA Name, so the ssa number is always -1. | ||
| Index(Box<BlockLoc>, SSAName), |
Contributor
There was a problem hiding this comment.
Shouldn't this just use a u32 like Loc?
Author
There was a problem hiding this comment.
Yes, it's essentially a wrapper of u32. I use SSAName for consistency in style.
Collaborator
There was a problem hiding this comment.
It should just be u32. It should only be a SSAName if it was originally a Name in the non-SSA IR.
Name::from_u32(n) could maybe have a more descriptive name - it is not just creating a Name for the number, it's creating a name that indexes the nth element of the symbol table which is not what we want here.
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
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.
Isla doesn't support pmpcfg and pmpaddr in the constraints because their type is
both
ir::Val::Vector<_>, which falls into the default failure match arm offunction
smt_valuein file primop_until.rs.There are two ways to fix it: extract the element from the vector before passing
it to
smt_value, or provide the new matching pattern for vector insmt_value. Considering Z3's array theory is for an infinite array, we must addmany constraints to imitate the extensional behavior of fixed-size array. In the
meantime, seq theory is not supported in Isla at all. We choose the first
option.
We extract the option in the
get_loc_and_initializebecause the arguments ofsmt_valueare got from the result of this function. We add additional Loc typein smt_parser.lalrpop to combine the index and array together.
In the linearization.rc file, the ssa number is always -1 for the new Loc type because
the index is not variable and not assignable.