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

[LLHD] Mem2Reg creates drives to read-only signals #8246

Open
fabianschuiki opened this issue Feb 15, 2025 · 0 comments · May be fixed by #8244
Open

[LLHD] Mem2Reg creates drives to read-only signals #8246

fabianschuiki opened this issue Feb 15, 2025 · 0 comments · May be fixed by #8244
Labels
bug Something isn't working LLHD

Comments

@fabianschuiki
Copy link
Contributor

The following only reads signal %a_0, but LLHD's mem2reg pass still inserts a corresponding drive. Reproduce with circt-opt --llhd-mem2reg on the following:

module {
  hw.module @Foo(in %a : i32) {
    %0 = llhd.constant_time <0ns, 0d, 1e>
    %c0_i32 = hw.constant 0 : i32
    %a_0 = llhd.sig name "a" %c0_i32 : i32
    %1 = llhd.prb %a_0 : !hw.inout<i32>
    llhd.process {
      cf.br ^bb1
    ^bb1:  // 2 preds: ^bb0, ^bb1
      %2 = llhd.prb %a_0 : !hw.inout<i32>
      func.call @dummy(%2) : (i32) -> ()
      llhd.wait (%1 : i32), ^bb1
    }
    llhd.drv %a_0, %a after %0 : !hw.inout<i32>
    hw.output
  }
  func.func private @dummy(%arg0: i32) {
    return
  }
}

The pass should probably track if a definition comes from a probe inserted by the pass, or an actual drive to the signal.

@fabianschuiki fabianschuiki added bug Something isn't working LLHD labels Feb 15, 2025
fabianschuiki added a commit that referenced this issue Feb 19, 2025
Implement a dedicated LLHD pass that promotes signal/memory slots to SSA
values and forwards drives to probes. The pass is distinct from MLIR's
upstream Mem2Reg pass in that it also works for regions where the memory
slots are defined outside the region. It also understands `llhd.wait`
and `llhd.halt` operations which may suspend execution of a process and
therefore allows the values of signals to change before execution
resumes. To deal with this, the pass uses a lattice to propagate the
need for slot definitions backwards from probes, and to propagate
reaching definitions for slots forward from drives and initial probes.
Using the lattice, the pass can forward driven values to probes as long
as there is no process suspension in between (`llhd.wait`), and it will
insert probes immediately after and drives immediately before suspension
points. This moves most intra-process data flow into SSA values and
block arguments, and retains probes and drives around the suspension
point to interact with ops outside the process.

This first implementation only supports unconditional blocking drives,
and only probes and drives that interact with a `llhd.sig` directly
without any field projections or aliasing. The pass is designed to deal
with these in the future though: a more detailed aliasing analysis can
determine which definitions are invalidated by drives, drives to field
projections can be converted into field insertions on the whole
definition, and drive conditions can be propagated along definitions to
the drives inserted by the pass ahead of suspension points.

Fixes #8245
Fixes #8246
@fabianschuiki fabianschuiki linked a pull request Feb 19, 2025 that will close this issue
fabianschuiki added a commit that referenced this issue Feb 19, 2025
Implement a dedicated LLHD pass that promotes signal/memory slots to SSA
values and forwards drives to probes. The pass is distinct from MLIR's
upstream Mem2Reg pass in that it also works for regions where the memory
slots are defined outside the region. It also understands `llhd.wait`
and `llhd.halt` operations which may suspend execution of a process and
therefore allows the values of signals to change before execution
resumes. To deal with this, the pass uses a lattice to propagate the
need for slot definitions backwards from probes, and to propagate
reaching definitions for slots forward from drives and initial probes.
Using the lattice, the pass can forward driven values to probes as long
as there is no process suspension in between (`llhd.wait`), and it will
insert probes immediately after and drives immediately before suspension
points. This moves most intra-process data flow into SSA values and
block arguments, and retains probes and drives around the suspension
point to interact with ops outside the process.

This first implementation only supports unconditional blocking drives,
and only probes and drives that interact with a `llhd.sig` directly
without any field projections or aliasing. The pass is designed to deal
with these in the future though: a more detailed aliasing analysis can
determine which definitions are invalidated by drives, drives to field
projections can be converted into field insertions on the whole
definition, and drive conditions can be propagated along definitions to
the drives inserted by the pass ahead of suspension points.

Fixes #8245
Fixes #8246
fabianschuiki added a commit that referenced this issue Feb 22, 2025
Implement a dedicated LLHD pass that promotes signal/memory slots to SSA
values and forwards drives to probes. The pass is distinct from MLIR's
upstream Mem2Reg pass in that it also works for regions where the memory
slots are defined outside the region. It also understands `llhd.wait`
and `llhd.halt` operations which may suspend execution of a process and
therefore allows the values of signals to change before execution
resumes. To deal with this, the pass uses a lattice to propagate the
need for slot definitions backwards from probes, and to propagate
reaching definitions for slots forward from drives and initial probes.
Using the lattice, the pass can forward driven values to probes as long
as there is no process suspension in between (`llhd.wait`), and it will
insert probes immediately after and drives immediately before suspension
points. This moves most intra-process data flow into SSA values and
block arguments, and retains probes and drives around the suspension
point to interact with ops outside the process.

This first implementation only supports unconditional blocking drives,
and only probes and drives that interact with a `llhd.sig` directly
without any field projections or aliasing. The pass is designed to deal
with these in the future though: a more detailed aliasing analysis can
determine which definitions are invalidated by drives, drives to field
projections can be converted into field insertions on the whole
definition, and drive conditions can be propagated along definitions to
the drives inserted by the pass ahead of suspension points.

Fixes #8245
Fixes #8246
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working LLHD
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant