Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Conditionally IMemRecv pushed back too far after Stage merging #39

Closed
dz333 opened this issue Aug 8, 2021 · 2 comments
Closed

Conditionally IMemRecv pushed back too far after Stage merging #39

dz333 opened this issue Aug 8, 2021 · 2 comments
Labels
bug Something isn't working

Comments

@dz333
Copy link
Collaborator

dz333 commented Aug 8, 2021

The following generates the wrong code at the moment:

split {
    case(a): {
       wdata <- mem[addr];
    ...
    default: {
     wdata <- 3;
     }
---
    x = (a) ? wdata : wdata + 1;

The computation for x uses the default wdata value instead of the one from the memory read, becuase the IMemRecv() command is ordered after the computation of x in the stage command list. This should not be generated in this order.

This is likely a result of stage merging as well -> when the IMemRecv is merged into the stage assigning to x, it should be at the beginning of the command list.

@dz333 dz333 added the bug Something isn't working label Aug 8, 2021
@dz333
Copy link
Collaborator Author

dz333 commented Sep 17, 2021

Attached is an example PDL circuit that will cause this exact issue:
lateRecv.txt

In the following snippet from the output of the PDL compiler (generated BSV code):

    Int#(32) _Stage__4_wdata = fifo_Start_TO_Stage__4.first.wdata;
    ...
    Int#(32) _Stage__4_x = ?;
    _Stage__4_x = ( ( _Stage__4_cond == 2'd0 ) ? _Stage__4_wdata : ( _Stage__4_wdata + 32'd1 ) );
    if ( ( _Stage__4___condStage__3 == 1'd0 ))
    begin
        _Stage__4_wdata = mem.peekResp(_Stage__4__request_0);
    end

x ignores the result coming back from memory (i.e., mem.peekResp(...)).

The compiler should generate all code that writes to a variable, before it is used in that stage.
The fixed version would simply have the conditional assignment to wdata be moved earlier:


    Int#(32) _Stage__4_wdata = fifo_Start_TO_Stage__4.first.wdata;
    if ( ( _Stage__4___condStage__3 == 1'd0 ))
    begin
        _Stage__4_wdata = mem.peekResp(_Stage__4__request_0);
    end
    ...
    Int#(32) _Stage__4_x = ?;
    _Stage__4_x = ( ( _Stage__4_cond == 2'd0 ) ? _Stage__4_wdata : ( _Stage__4_wdata + 32'd1 ) );
  

@dz333
Copy link
Collaborator Author

dz333 commented Nov 11, 2021

Resolved by #50

@dz333 dz333 closed this as completed Nov 11, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant