[HW] Add HWConstProp pass for inter-module constant propagation by prithayan · Pull Request #9207 · llvm/circt

prithayan · 2025-11-07T17:42:43Z

This PR adds the HWConstProp pass that performs inter-module constant propagation across hardware module boundaries in the HW dialect.

The pass uses the Sparse Conditional Constant Propagation (SCCP) with lattice-based analysis
Key Steps:

Initialization
- Mark public module inputs as overdefined (can't be constant)
- Discover constants from hw.constant operations
- Mark external/generated module outputs as overdefined
- Enqueue operations for propagation
Propagation
- Process operations from two queues (overdefined first, then constants)
- Propagate constants through:
  - Module outputs → instance results
  - Instance inputs → module arguments
  - Combinational operations (fold when possible)
- Use lattice meet: unknown ⊔ constant = constant, constant₁ ⊔ constant₂ = overdefined
Cycle Handling
- Mark remaining unknown values as overdefined
- Re-propagate to fold newly-resolved constants
Folding
- Replace SSA values with discovered constants
- Remove dead operations

uenoku

LGTM, do you think it makes sense to add to Firtool pipeline (with opt out flag) for better coverage? Also does this block constprop across ports with inner symbol?

uenoku · 2025-11-12T10:04:23Z

+  // CHECK-NEXT: %wire = hw.wire %c42_i8 : i8
+  // CHECK-NEXT: hw.output %wire : i8
+  %c42_i8 = hw.constant 42 : i8
+  %wire = hw.wire %c42_i8 : i8


Do we need this restriction? I think wire without inner symbols can be optimized?

uenoku · 2025-11-14T17:24:08Z

 }

+def HWConstProp : Pass<
+  "hw-constprop", "mlir::ModuleOp"


nit: can we name this as imconstprop for consistency with FIRRTL?

seldridge

Partial review. I'd like to give this another look!

seldridge · 2025-11-14T18:55:16Z

+#include "mlir/Pass/Pass.h"
+#include "llvm/Support/Debug.h"
+
+#define DEBUG_TYPE "hw-constprop"


When changing to imconstprop, make that change here.

Also, I figured out a way to not have to hard-code this, but it requires reorganizing the file a little bit. See how this works here: https://github.com/llvm/circt/blob/main/lib/Dialect/FIRRTL/Transforms/LowerDomains.cpp#L69

seldridge · 2025-11-14T18:55:56Z

+  void markUnknownValuesOverdefined(hw::HWModuleOp module);
+  std::pair<unsigned, unsigned> fold();
+
+public:


This public is unnecessary as this is already in a public scope.

seldridge · 2025-11-14T18:56:25Z

+  /**
+   * Returns the lattice value associated with an SSA value.
+   *
+   * `std::nullopt` is unknown, `IntegerAttr{}` is overdefined.
+   */


Nit: I think this needs doxygen-style comments not Javadoc style, i.e., ///.

seldridge · 2025-11-14T18:56:39Z

+//===----------------------------------------------------------------------===//
+
+namespace {
+class ConstantPropagation {


Please include comments for this and for all member functions/members.

I'm still grokking why there are three queues here and this would help with that!

seldridge · 2025-11-14T18:58:23Z

+                auto module = dyn_cast_or_null<HWModuleOp>(referencedOp);
+                return !module;


I think this can be:

Suggested change

auto module = dyn_cast_or_null<HWModuleOp>(referencedOp);

return !module;

return !isa_and_nonnull<HWModuleOp>(referencedOp);

seldridge · 2025-11-14T19:00:23Z

+          // Mark wires as overdefined since they can be targeted by force.
+          markOverdefined(wire.getResult());


Is there a less-strict check that can work here? Like, just if this has an inner symbol?

seldridge · 2025-11-14T19:02:08Z

+      if (attr) {
+        valueQueue.push_back(user);
+      } else {
+        overdefQueue.push_back(user);
+      }


Nit:

Suggested change

if (attr) {

valueQueue.push_back(user);

} else {

overdefQueue.push_back(user);

}

if (attr)

valueQueue.push_back(user);

else

overdefQueue.push_back(user);

seldridge · 2025-11-14T19:04:46Z

+private:
+  hw::InstanceGraph &graph;
+  DenseMap<Value, IntegerAttr> values;
+  DenseSet<Operation *> inQueue;


Calling this a "queue" is a bit disingenuous.

seldridge · 2025-11-14T19:07:10Z

+std::optional<IntegerAttr> ConstantPropagation::map(Value value) {
+  if (auto constant = value.getDefiningOp<hw::ConstantOp>())
+    return constant.getValueAttr();
+
+  auto it = values.find(value);
+  if (it == values.end())
+    return std::nullopt;
+
+  return it->second;
+}


Calling this map seems a bit odd given that it's "getMaybeConstant". (That is a terrible name!) Something describing what this is doing would be better.

prithayan requested a review from darthscsi as a code owner November 7, 2025 17:42

uenoku reviewed Nov 14, 2025

View reviewed changes

seldridge reviewed Nov 14, 2025

View reviewed changes

prithayan added 2 commits January 2, 2026 11:59

[HW] Add HWConstProp pass for inter-module constant propagation

5d2d09f

[HW] Address review comments.

4da5e2d

prithayan force-pushed the dev/pbarua/hwconstprop branch from 1127b04 to 4da5e2d Compare January 2, 2026 20:00

prithayan added 2 commits January 5, 2026 08:28

Constprop wires without symbols

ab422d9

Fix gcc error

cf35b4b

		auto module = dyn_cast_or_null<HWModuleOp>(referencedOp);
		return !module;

	auto module = dyn_cast_or_null<HWModuleOp>(referencedOp);
	return !module;
	return !isa_and_nonnull<HWModuleOp>(referencedOp);

		// Mark wires as overdefined since they can be targeted by force.
		markOverdefined(wire.getResult());

Conversation

prithayan commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

uenoku left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seldridge left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

prithayan commented Nov 7, 2025 •

edited

Loading