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 crash on reasonable input #8245

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

[LLHD] Mem2Reg crash on reasonable input #8245

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

Comments

@fabianschuiki
Copy link
Contributor

The following crashes circt-opt --llhd-mem2reg:

module {
  hw.module @Foo(in %a : i1, in %b : i32, out x : i32) {
    %0 = llhd.constant_time <0ns, 0d, 1e>
    %c0_i32 = hw.constant 0 : i32
    %c42_i32 = hw.constant 42 : i32
    %false = hw.constant false
    %a_0 = llhd.sig name "a" %false : i1
    %b_1 = llhd.sig name "b" %c0_i32 : i32
    %x = llhd.sig %c0_i32 : i32
    %1 = llhd.prb %b_1 : !hw.inout<i32>
    %2 = llhd.prb %x : !hw.inout<i32>
    %3 = llhd.prb %a_0 : !hw.inout<i1>
    llhd.process {
      cf.br ^bb1
    ^bb1:  // 2 preds: ^bb0, ^bb3
      %5 = llhd.prb %b_1 : !hw.inout<i32>
      llhd.drv %x, %5 after %0 : !hw.inout<i32>
      %6 = llhd.prb %a_0 : !hw.inout<i1>
      cf.cond_br %6, ^bb2, ^bb3
    ^bb2:  // pred: ^bb1
      %7 = llhd.prb %x : !hw.inout<i32>
      %8 = comb.add %7, %c42_i32 : i32
      llhd.drv %x, %8 after %0 : !hw.inout<i32>
      cf.br ^bb3
    ^bb3:  // 2 preds: ^bb1, ^bb2
      %9 = llhd.prb %x : !hw.inout<i32>
      func.call @printikus(%9) : (i32) -> ()
      llhd.wait (%1, %2, %3 : i32, i32, i1), ^bb1
    }
    llhd.drv %a_0, %a after %0 : !hw.inout<i1>
    llhd.drv %b_1, %b after %0 : !hw.inout<i32>
    %4 = llhd.prb %x : !hw.inout<i32>
    hw.output %4 : i32
  }
  func.func private @printikus(%arg0: i32) {
    return
  }
}
@fabianschuiki fabianschuiki added bug Something isn't working LLHD labels Feb 15, 2025
@fabianschuiki
Copy link
Contributor Author

Reduced failing case:

hw.module @Foo() {
  %0 = llhd.constant_time <0ns, 0d, 1e>
  %c0_i42 = hw.constant 0 : i42
  %x = llhd.sig %c0_i42 : i42
  %y = llhd.sig %c0_i42 : i42
  llhd.process {
    %1 = llhd.prb %x : !hw.inout<i42>
    llhd.drv %y, %1 after %0 : !hw.inout<i42>
    %2 = llhd.prb %y : !hw.inout<i42>
    func.call @use_i42(%2) : (i42) -> ()
    llhd.halt
  }
}
func.func private @use_i42(%arg0: i42)

fabianschuiki added a commit that referenced this issue Feb 18, 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.
@fabianschuiki fabianschuiki linked a pull request Feb 18, 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 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