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

Add Trees That Grow-style extension fields to SetupValue #1914

Closed
RyanGlScott opened this issue Aug 22, 2023 · 0 comments · Fixed by #1919
Closed

Add Trees That Grow-style extension fields to SetupValue #1914

RyanGlScott opened this issue Aug 22, 2023 · 0 comments · Fixed by #1919
Labels
tech debt Issues that document or involve technical debt type: enhancement Issues describing an improvement to an existing feature or capability

Comments

@RyanGlScott
Copy link
Contributor

Currently, SetupValue is defined like this:

-- | From the manual: \"The SetupValue type corresponds to values that can occur
-- during symbolic execution, which includes both 'Term' values, pointers, and
-- composite types consisting of either of these (both structures and arrays).\"
data SetupValue ext where
SetupVar :: AllocIndex -> SetupValue ext
SetupTerm :: TypedTerm -> SetupValue ext
SetupNull :: B (HasSetupNull ext) -> SetupValue ext

As this comment indicates, the design of SetupValue was inspired by the Trees That Grow paper, which allows defining an AST that can be extended with information specific to particular backends. Specifically, the ext type parameter is instantiated with one of LLVM, JVM, or MIR, and these determine whether fields with types such as B (HasSetupNull ext) are inhabited. If they are inhabited, these fields evaluate to (), indicating that that constructor is valid in that particular backend. If they are not inhabited, then these fields evaluate to Void, indicating that that constructor is never meant to be used in that particular backend.

Unfortunately, SetupValue only commits halfway to the Trees That Grow design. The definition for SetupStruct is:

-- | If the 'Bool' is 'True', it's a (LLVM) packed struct
SetupStruct :: B (HasSetupStruct ext) -> Bool -> [SetupValue ext] -> SetupValue ext

This Bool field is terrible, as the notion of packed structs is only ever used by the LLVM backend. Despite this, other backends will still have to pass False when using SetupStruct, even though this Bool is immaterial in these backends. This goes against the entire philosophy of Trees That Grow, which offers a way to have information that is specific to each backend without requiring other backends to care about it.

I propose that we refactor the Trees That Grow machinery in SetupValue slightly to fix this infelicity. Instead of having type family HasSetupStruct ext :: Bool, type family HasSetupNull ext :: Bool, etc., we should instead have something like type family XSetupStruct ext :: Type. Just as before, we will provide instances of XSetupStruct et al. for LLVM, JVM, and MIR, and if a particular construct isn't used in a backend, we will declare XSetupStruct etc. to evaluate to Void. If a construct is used in a particular backend, however, then XSetupStruct etc. should evaluate to the type used in that specific backend rather than just (). To wit, this means that we would have:

type instance XSetupStruct LLVM = Bool -- True for packed structs, False for other structs
type instance XSetupStruct JVM = Void
...

Now that JVM backend doesn't need to care about packed structs at all.

This refactoring would be especially useful for the in-development MIR backend for SAW (see #1859), as various MIR constructs need additional information not required by other backends:

  • MIR allows length-0 arrays, provided that the type of the array is known. SetupArray does not store this type, however.
  • MIR structs need to know the type of the struct, since MIR has a nominal type system instead of LLVM's structural type system. Again, SetupStruct does not have a place to store this type.
@RyanGlScott RyanGlScott added type: enhancement Issues describing an improvement to an existing feature or capability tech debt Issues that document or involve technical debt labels Aug 22, 2023
RyanGlScott added a commit that referenced this issue Aug 24, 2023
This removes all of the `HasSetup* :: Bool` type families in favor of new `X*
:: Type` type families. These largely serve the same purpose of distinguishing
which constructors of `SetupValue` are permissible in each language backend,
but they are permit bundling additional, language-specific information into a
`SetupValue`. For instance, the `XSetupStruct` instance for `LLVM` evaluates to
`Bool`, representing whether we are dealing with an LLVM packed struct or not,
and the `XSetupCast` instance for `LLVM` evaluates to an LLVM `Type` to
represent the type to cast to.

One consequence of this design, aside from some `SetupValue` fields moving
around, is that `ppSetupValue` now must perform case analysis on which language
backend is currently in use in order to pretty-print any backend-specific data.
This is accomplished with the new `SAWExt` data type and the corresponding
`IsExt` class.

There is a lot of code that shifts around in this patch, but everything is
still functionally equivalent. In a subsequent patch, I will introduce
additional data into MIR-specific extension fields.

Fixes #1914.
RyanGlScott added a commit that referenced this issue Sep 1, 2023
This removes all of the `HasSetup* :: Bool` type families in favor of new `X*
:: Type` type families. These largely serve the same purpose of distinguishing
which constructors of `SetupValue` are permissible in each language backend,
but they are permit bundling additional, language-specific information into a
`SetupValue`. For instance, the `XSetupStruct` instance for `LLVM` evaluates to
`Bool`, representing whether we are dealing with an LLVM packed struct or not,
and the `XSetupCast` instance for `LLVM` evaluates to an LLVM `Type` to
represent the type to cast to.

One consequence of this design, aside from some `SetupValue` fields moving
around, is that `ppSetupValue` now must perform case analysis on which language
backend is currently in use in order to pretty-print any backend-specific data.
This is accomplished with the new `SAWExt` data type and the corresponding
`IsExt` class.

There is a lot of code that shifts around in this patch, but everything is
still functionally equivalent. In a subsequent patch, I will introduce
additional data into MIR-specific extension fields.

Fixes #1914.
@mergify mergify bot closed this as completed in #1919 Sep 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tech debt Issues that document or involve technical debt type: enhancement Issues describing an improvement to an existing feature or capability
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant