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

CFG path contraction condition was removing entry blocks #912

Merged
merged 2 commits into from
Jan 23, 2024

Conversation

macrologist
Copy link
Contributor

Addresses #911

Path contraction in the CFG construction was allowing for entry blocks to be removed, despite declaring such behavior to be illegal in the comment preceding that removal.

@macrologist
Copy link
Contributor Author

Compiling

DECLARE ro BIT
DECLARE shot_count INTEGER[1]

LABEL @START
SUB shot_count[0] 1
JUMP-UNLESS @END shot_count[0]
JUMP @START
LABEL @END

should emit something like


DECLARE ro BIT
DECLARE shot_count INTEGER

JUMP @START                             # Entering/exiting rewiring: (#(0 1 2 3 4 5 6 7) . #(0 1 2 3 4 5 6 7))
LABEL @END                              # Entering rewiring: #(0 1 2 3 4 5 6 7)
HALT                                    # Exiting rewiring: #(0 1 2 3 4 5 6 7)
LABEL @START                            # Entering rewiring: #(0 1 2 3 4 5 6 7)
MEASURE 0 ro[0]
SUB shot_count[0] 1
JUMP-WHEN @START shot_count[0]
JUMP @END                               # Exiting rewiring: #(0 1 2 3 4 5 6 7)

@macrologist
Copy link
Contributor Author

bah hold on. RESETS are now generating a pointless jump-to-next-position sequence:

RESET

is becomming

JUMP @RESET-3323                       
LABEL @RESET-3323                       
RESET

working on it.

@macrologist
Copy link
Contributor Author

Should be good. Tests pass on my Mac

Fixed bug in cfg path contraction

- removed superfluous check on number of children of a block's parents
- a labelled block should only be merged with its solitary parent when that
parent's outgoing edge is unconditional
Copy link
Member

@stylewarning stylewarning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just add a couple tests for the previously failing cases?

@macrologist
Copy link
Contributor Author

macrologist commented Jan 22, 2024

@stylewarning I added tests to cover the inciting example and one other

@stylewarning stylewarning merged commit 13c41e3 into quil-lang:master Jan 23, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants