[hw] Add hw.donttouch operation

[seldridge]

trafficstars

Add a hw.donttouch operation which is an inner symbol user that indicates that the inner symbol should not be optimized, removed, or otherwise touched by the compiler.

This is added to fill a hole where unused inner symbols are being used to mean "don't touch". The lack of a user means that all inner symbols are effectively "don't touch" when only certain ones should be. Concretely, this is being added as a step towards better optimization of read probes.

[teqdruid]

I haven't looked over this PR, but this feels like it should be a dialect attribute. Those have problems related to passes not retaining them though.

On Mon, Jun 9, 2025, 2:10 PM Will Dietz @.***> wrote:

@.**** commented on this pull request.

In include/circt/Dialect/HW/HWStructure.td https://github.com/llvm/circt/pull/8547#discussion_r2136217735:

@@ -653,6 +654,22 @@ def HierPathOp : HWOp<"hierpath", }]; }

+def DontTouchOp : HWOp<"donttouch"> {

• let summary = "A user of an inner ref that blocks removal";
• let description = [{
• An operation that marks an InnerRef as having external users. The operation
• with the InnerRef should not be optimized in any way and may be used
• externally by name.
• This operation roughly implements the notion of "don't touch" that many
• Verilog tools understand.
• }];
• let arguments = (ins InnerRefAttr:$ref);
• let assemblyFormat = [{
• $ref attr-dict
• }];

implement InnerRefUserOpInterface, for verification the reference exists/is valid .

In include/circt/Dialect/HW/HWStructure.td https://github.com/llvm/circt/pull/8547#discussion_r2136221295:

@@ -653,6 +654,22 @@ def HierPathOp : HWOp<"hierpath", }]; }

+def DontTouchOp : HWOp<"donttouch"> {

As-is, this has no results, symbol, and is not side-effecting.

I think this should be marked as having side-effects to avoid being dropped. It should be (if we care?) valid to CSE these.

I'd expect these all at top-level of an IRN -- should we add that restriction/expectation ? That is, these are expected/supported as siblings to hw.module's not within them, that sort of thing? (if this is intended to be more flexible, nevermind...)

— Reply to this email directly, view it on GitHub https://github.com/llvm/circt/pull/8547#pullrequestreview-2910895647, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALNXYAA7UALXCN7G2J2Q5L3CXE2VAVCNFSM6AAAAAB65RTGE6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDSMJQHA4TKNRUG4 . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[uenoku]

Is there difference between public hierpath op which points to the an inner symbol?

[seldridge]

@teqdruid wrote:

I haven't looked over this PR, but this feels like it should be a dialect attribute. Those have problems related to passes not retaining them though.

That was roughly how FIRRTL handled it and we could go in this direction. However, it's not currently how this is implemented. Currently, hardware donttouch is implemented with a dangling inner symbol. This could be viewed as a patch of the existing design point.

@uenoku wrote:

Is there difference between public hierpath op which points to the an inner symbol?

In terms of current effect, no. In terms of what a public hierpath op means, I'm not sure. I think this would have basically the same meaning/restrictions. I was. avoiding the public hierpath op as it seemed better not to abuse that to also mean "don't touch".

[seldridge]

We discussed this internally and the rough consensus was that an attribute is definitely better. This is a relatively annoying lift, though. Alternatively, changing this to a single, variadic operation was viewed as better given that this is a relatively static thing.

[teqdruid]

We discussed this internally and the rough consensus was that an attribute is definitely better. This is a relatively annoying lift, though.

What, AI isn't capable of making the change?

[seldridge]

What, AI isn't capable of making the change?

It's more the implications of it. Currently unused inner symbols are being used for this and that is (assumedly) blocking optimizations (as it is having this effect). This then needs a new attribute that all optimizations, passes, etc. understand the meaning of it. This is how we handle this in FIRRTL dialect, it's just pretty invasive.

I think we went the inner symbol option originally as it doesn't have this problem. If something has an inner symbol, then it can't be deleted unless you analyze users (and that should be the job of inner symbol DCE, not your folder/pass). However, inner symbol DCE is only done in FIRRTL (primarily because verification of inner symbols isn't yet automatically supported as we don't have the hw.design container op).

[teqdruid]

I understand how invasive a change this could end up being. I was mostly joking... Current AI models don't work terribly well on our code base. But I've heard stories about AI being let loose on complete code bases and doing really well. So as of next month, all changes to CIRCT will be trivial, right?

[seldridge]

Only if you can figure out how to enable AI code review on the project. 😉

[teqdruid]

https://github.com/llvm/circt/pull/8486

Does not everyone have this option?