Skip to content
This repository has been archived by the owner on Mar 24, 2023. It is now read-only.

Enums and dataflow processing (includes #64) #65

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

Conversation

DomHenderson
Copy link

@DomHenderson DomHenderson commented Mar 8, 2023

Pull Request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build-related changes
  • Documentation content changes
  • Testing
  • Other (please describe):

What is the current behavior?

Issue Number: N/A

What is the new behavior?

  • Support for enum type declarations
  • enum_init
  • enum_match

Enum types are implemented as structs with a preceding index field, and then one field for each unique enum subtype (i.e. if the enum is always a felt then the struct will only have two fields). Each version of enum init takes a single argument, allocating it on the stack alongside the index and any other fields which will not be assigned any particular value. This includes the comparison testing infrastructure because it's what I'm using to test it, but can be decoupled if/when needed Enum match extracts each field from the enum struct, switches on the index field, and utilises the dataflow graph to forward data onto its destinations.

This has required improving data flow handling and adding phi instructions. To summarise:

  • Processing order changed to funcs->blocks->statements
  • Funcs creates a function definition for each user function
  • Blocks creates the required basic block structure within each function. Currently this creates one block per sierra statement because llvm should easily be able to optimise this, but it's done using a BTreeMap indexed on statement index so that it will work with larger blocks too
  • Variables are stored per block, which stops them being nuked by function calls or overwritten by control flow convergence
  • Additionally the predecessor and successor ids of each block are stored for easy access
  • When a variable's value is needed, there are helper functions that check whether it's available in the current block and insert phi instructions if not
  • Statement processing is now (imo) somewhat simplified and is back to being its own pass
  • Processing order is defined such that each block is only processed once all its predecessors are processed
  • This avoids bugs that were previously arising from reprocessing blocks

Does this introduce a breaking change?

  • Yes
  • No

Other information

@codecov
Copy link

codecov bot commented Mar 15, 2023

Codecov Report

Patch coverage: 90.54% and project coverage change: +4.56 🎉

Comparison is base (43e33bf) 86.27% compared to head (9c9c10b) 90.84%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #65      +/-   ##
==========================================
+ Coverage   86.27%   90.84%   +4.56%     
==========================================
  Files          36       38       +2     
  Lines        1741     2151     +410     
==========================================
+ Hits         1502     1954     +452     
+ Misses        239      197      -42     
Impacted Files Coverage Δ
core/src/sierra/process/funcs.rs 72.17% <70.75%> (-27.83%) ⬇️
core/src/sierra/process/dataflow.rs 88.23% <88.23%> (ø)
core/src/sierra/llvm_compiler.rs 95.37% <89.47%> (-0.30%) ⬇️
core/src/sierra/process/statements.rs 93.75% <93.43%> (+17.82%) ⬆️
...re/src/sierra/corelib_functions/enums/enum_init.rs 94.82% <94.82%> (ø)
core/src/sierra/types/sierra_enum.rs 95.77% <95.77%> (ø)
core/src/sierra/debug.rs 88.05% <98.11%> (+3.05%) ⬆️
core/src/sierra/process/corelib.rs 100.00% <100.00%> (+2.04%) ⬆️
core/src/sierra/process/types.rs 94.11% <100.00%> (-0.33%) ⬇️
...rc/sierra/statements_processing/jump/enum_match.rs 100.00% <100.00%> (ø)
... and 2 more

... and 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@DomHenderson DomHenderson changed the title [WIP] Enums Enums and dataflow processing (includes #64) Mar 15, 2023
@DomHenderson DomHenderson marked this pull request as ready for review March 15, 2023 04:42
@DomHenderson DomHenderson marked this pull request as draft March 15, 2023 13:53
@DomHenderson
Copy link
Author

Spotted a couple small todos that are in scope, should be back out of draft again soon

@DomHenderson DomHenderson marked this pull request as ready for review March 15, 2023 16:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant