[FIRRTL][LowerLayers] Update rwprobe operations if possible.

[dtzSiFive]

Fixes first example in #7365 .

[dtzSiFive]

Future work: detecting and reject captures of rwprobe's re:bind layer extraction (presently causes verifier failure).

[seldridge]

Some questions about the errors. However, the non-error logic seems good.

Consider doing the FailureOr refactor as a separate commit as that is entirely fine and can go in on main without review of the other pieces.

[seldridge]

This seems more like a verifier error than a LowerLayers error.

I.e., this is trying to create a rwprobe that can be used to force something outside the layerblock which seems to violate the guarantee that no layerblock operation will "drive" anything outside the layerblock.

WDYT?

[seldridge]

Can this error message be made more clear? This is pretty low-level in terms of LowerLayers' implementation.

Echoing later comments, the fact that this can't be moved feels like this may be a missing verifier.

[dtzSiFive]

Hmm, I'll see, thanks! If so, these errors would be unreachable which has implications for this PR.
The error refactoring still is useful for a few reasons, including routing out errors about trying to capture a rwprobe in a bind layer (vs the creation of a rwprobe that this PR is concerned about).

---

Okay so re:verifiers -- #7372 covers this re:blocking rwprobe of hardware with ambient layer color not sufficient.

However, this can still happen as with the example in the test added with this PR -- where enablelayers colors the hardware so the rwprobe is valid and can be used reliably to force hardware that is of the right color.

Use of a layerblock within a module with enablelayers of that color is kinda curious:

You can explicitly use probes that target hardware in layerblock of color X outside the layer X by using it in a module with enablelayers X (per spec), including forcing hardware in the layerblock from the enablelayers'd module (right)? This is bidirectional driving between the two, so seems reasonable you could create rwprobe's in the enablelayers'd module and use from layerblock of that color?

Related:
"Should layerblock already enabled by enablelayers be lowered to bind -- or maybe just specialized on since it /must/ be enabled unconditionally anyway?"

And re:"what would you do that" -- presently if you wanted to put things under layer X.Y you'd have to wrap it in a layerblock X even in an enablelayers X module, is one reason.

Anyway /if/ this is something to support, seems we should also allow simply driving hardware from within a layerblock to hardware of that ambient color... maybe?

🤔

[dtzSiFive]

But the error should be made more clear, at the very least 👍.

I wrote this with a different case in mind -- in a sibling unmerged layer of the same color that was extracted separately, but I think that's unreachable due to the rwprobe dominance concern so maybe this is the only way and it's less obviously valid, although gets hard to enforce across instantiation (or extmodule's) since basically it's allowed to force hardware of the layer color (as long as it's not in the same module as the layerblock?):

firrtl.circuit "RWProbeCrossLayer" {
  firrtl.layer @A bind { }
  firrtl.module private @Child(out %p : !firrtl.rwprobe<uint<5>, @A>) attributes {layers = [@A], annotations = [{class = "firrtl.passes.InlineAnnotation"}]} {
    %z = firrtl.constant 0 : !firrtl.uint<5>
    %w = firrtl.node sym @sym %z {annotations = [{class = "firrtl.transforms.DontTouchAnnotation"}]} : !firrtl.uint<5>
    %rw = firrtl.ref.rwprobe <@Child::@sym> : !firrtl.rwprobe<uint<5>>
    %rw_c = firrtl.ref.cast %rw : (!firrtl.rwprobe<uint<5>>) -> !firrtl.rwprobe<uint<5>, @A>
    firrtl.ref.define %p, %rw_c : !firrtl.rwprobe<uint<5>, @A>
  }
  firrtl.module @RWProbeCrossLayer() attributes {layers = [@A]} {
    %rwp = firrtl.instance c {layers = [@A]} @Child(out p : !firrtl.rwprobe<uint<5>, @A>)
    firrtl.layerblock @A {
      %val = firrtl.constant 3 : !firrtl.uint<5>
      %c = firrtl.constant 1 : !firrtl.uint<1>
      firrtl.ref.force_initial %c, %rwp, %val : !firrtl.uint<1>, !firrtl.rwprobe<uint<5>, @A>, !firrtl.uint<5>
    }
  }
}

This will verify fine BUT with inlining AND a layersink that buries everything except the target node (since DontTouch'd?) we'll end up with a rwprobe of hardware that was valid but no longer is once we see all the pieces, that sort of thing.

Hopefully this makes sense. WDYT?

I'll update the test/circuit/module name since they reflect my original test intent anyway 😅 .

[dtzSiFive]

This is tricky to be a verifier, as it is only reachable if the rwprobe target is not sunk into the layer.

I made a pass to improve the diagnostic and chase down the target to point to what isn't working.

[dtzSiFive]

Post-merge review welcome as always!