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 jumps support in do while loops #29

Closed
wants to merge 1 commit into from
Closed

Conversation

RRozak
Copy link
Member

@RRozak RRozak commented Nov 7, 2022

Previously do while loops were parsed as normal while loops with a copy of body inserted before a loop. If there was break statement inside a body, the error was thrown: break isn't underneath a loop. Similar situation was with continue statement. So I created separate node for that kind of loop and I added converting it into while loop in V3LinkJump, when break and continue statements are handled.

Now I explain the change in addPrev implementation. Without it, the statement
whilep->addPrev(bodyp->cloneTree(false));
moved whilep node at the end of statements in a block.
Here is an example:

      while (1) begin
         do
            ++incr;
         while (incr < 9);
         incr++;
         break;

In the example, the inner while is put after postincrementation and break stataments.
Here is the AST (without the change, PREADD and POSTADD subtrees are shortened):

    1:2:3: JUMPBLOCK 0x555556d20000 <e122#> {c4ah}
    1:2:3:1: WHILE 0x555556d16370 <e120#> {c4ah}
    1:2:3:1:2: CONST 0x555556cc2300 <e66> {c4ao} @dt=0x555556cd61a0@(G/swu32/1)  ?32?sh1
    1:2:3:1:3: BEGIN 0x555556cee380 <e25> {c4ar} [UNNAMED]
    1:2:3:1:3:1: PREADD 0x555556d16630 <e116#> {c6an} @dt=0@
    1:2:3:1:3:1: POSTADD 0x555556d16210 <e62> {c8ao} @dt=0@
    1:2:3:1:3:1: JUMPGO 0x555556d20180 <e124#> {c9ak} -> JUMPLABEL 0x555556d200c0 <e119#> {c4ah} -> JUMPBLOCK 0x555556d20000 <e122#> {c4ah}
    1:2:3:1:3:1: WHILE 0x555556d16580 <e117#> {c5ak}
    1:2:3:1:3:1:2: LT 0x555556d160b0 <e110#> {c7aw} @dt=0x555556cd6820@(G/w1)
    1:2:3:1:3:1:2:1: VARREF 0x555556cd0360 <e80#> {c7ar} @dt=0@  incr [RV] <- VAR 0x555556cce180 <e16> {c3al} @dt=0@  incr [FUNC] [VAUTOM]  VAR
    1:2:3:1:3:1:2:2: CONST 0x555556cc2500 <e44> {c7ay} @dt=0x555556cd6750@(G/swu32/4)  ?32?sh9
    1:2:3:1:3:1:3: PREADD 0x555556d16000 <e108#> {c6an} @dt=0@
    1:2:3:2: JUMPLABEL 0x555556d200c0 <e119#> {c4ah} -> JUMPBLOCK 0x555556d20000 <e122#> {c4ah}

And the one after the change (correct one):

    1:2:3: JUMPBLOCK 0x555556d20000 <e122#> {c4ah}
    1:2:3:1: WHILE 0x555556d16370 <e120#> {c4ah}
    1:2:3:1:2: CONST 0x555556cc2300 <e66> {c4ao} @dt=0x555556cd61a0@(G/swu32/1)  ?32?sh1
    1:2:3:1:3: BEGIN 0x555556cee380 <e25> {c4ar} [UNNAMED]
    1:2:3:1:3:1: PREADD 0x555556d16630 <e116#> {c6an} @dt=0@
    1:2:3:1:3:1: WHILE 0x555556d16580 <e117#> {c5ak}
    1:2:3:1:3:1:2: LT 0x555556d160b0 <e110#> {c7aw} @dt=0x555556cd6820@(G/w1)
    1:2:3:1:3:1:2:1: VARREF 0x555556cd0360 <e80#> {c7ar} @dt=0@  incr [RV] <- VAR 0x555556cce180 <e16> {c3al} @dt=0@  incr [FUNC] [VAUTOM]  VAR
    1:2:3:1:3:1:2:2: CONST 0x555556cc2500 <e44> {c7ay} @dt=0x555556cd6750@(G/swu32/4)  ?32?sh9
    1:2:3:1:3:1:3: PREADD 0x555556d16000 <e108#> {c6an} @dt=0@
    1:2:3:1:3:1: POSTADD 0x555556d16210 <e62> {c8ao} @dt=0@
    1:2:3:1:3:1: JUMPGO 0x555556d20180 <e124#> {c9ak} -> JUMPLABEL 0x555556d200c0 <e119#> {c4ah} -> JUMPBLOCK 0x555556d20000 <e122#> {c4ah}
    1:2:3:2: JUMPLABEL 0x555556d200c0 <e119#> {c4ah} -> JUMPBLOCK 0x555556d20000 <e122#> {c4ah}

So old addPrev implementation breaks the order of other nodes, which sometimes may cause a different behavior.

Copy link

@rkapuscik rkapuscik left a comment

Choose a reason for hiding this comment

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

Looks OK, just one minor suggestion.

src/V3LinkJump.cpp Show resolved Hide resolved
@RRozak RRozak force-pushed the rrozak/jumps-do-while branch from ef7f0aa to b0fb378 Compare November 8, 2022 09:36
@RRozak
Copy link
Member Author

RRozak commented Nov 8, 2022

Created PR to mainline: verilator#3731

@RRozak RRozak closed this Nov 8, 2022
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