[AIGLongestPathAnalysis] Implement incremental longest path analysis for IR transformations #8890

uenoku · 2025-08-25T08:54:53Z

This commit introduces IncrementalLongestPathAnalysis to support lazy delay computation during IR mutations in synthesis passes, with the DatapathToComb conversion pass as the first use case.

The implementation adds the IncrementalLongestPathAnalysis class that extends the existing analysis with incremental capabilities through the PatternRewriter::Listener interface to track mutations and maintain validity. New getOrComputeDelay() and getOrComputePaths() methods enable on-demand computation, while an OperationAnalyzer class handles dynamic analysis of Comb and HW dialect operations through temporary module synthesis.

The analysis requires a top-down rewriting approach to maintain correctness, as bottom-up mutations can invalidate paths that depend on erased values. The incremental framework validates mutations to prevent breaking existing timing paths and provides early error detection for inappropriate modifications to critical path values.

cowardsa

Thanks for this connection Hideto - looks really valuable for improving synthesis results! I also like the interface but wanted to clarify usage a little:

Is it the responsibility of the pass author to call notifyOperation... whenever you make changes to the IR? Or is this automatically hooked up with existing MLIR infra?
Does knowing precisely where changes have been made (due to IR transformations) mean we can only look at local changes to the delay paths?

P.s. if you hook up the comb delay model interface then others (myself included) can add additional or more precise models as needed - similar to the KnownBits Analysis

lib/Conversion/DatapathToComb/DatapathToComb.cpp

lib/Dialect/AIG/Analysis/LongestPathAnalysis.cpp

uenoku · 2025-08-26T10:01:18Z

Is it the responsibility of the pass author to call notifyOperation... whenever you make changes to the IR? Or is this automatically hooked up with existing MLIR infra?

Yes, it's user's responsibility to notify the mutation but MLIR's DialectConversion/PartialConversion/PatternRewriter automatically send notification to the listener as long as the listener is registered (config.listener = analysis; is the corresponding code). So it's shouldn't be a much problem if MLIR infra was used.

Does knowing precisely where changes have been made (due to IR transformations) mean we can only look at local changes to the delay paths?

Yes, that's current limitation. Currently IncrementalLongestPathAnalysis will only accept local scope (hw::HWModuleOp).
User could use normal LongestPathAnalysis for mlir::ModuleOp but it doesn't support incremental updates now.

For inter-module timing analysis I'm inclined to prefer synth.required_times/arrival_times operation (or attribute?) to attach timing information across modules (similar SDC) so that local timing analysis can use information of global timing analysis, and periodically sync the timing info globally.

hw.module @Foo(in %x: i3) {
    // bit 0/1/2 arrive at time 1/5/10.
    %x_with_timing = synth.arrival_times %x [10, 5, 1] : i3
    %out = hw.instance @Bar(%x_with_timing): (i3) -> i2
    %out_with_timing = synth.arrival_times %out_timing [100, 20] : i2
    
}

… IR transformations This commit introduces IncrementalLongestPathAnalysis to support lazy computation of delay information during IR mutations in synthesis passes. The analysis is updated incrementally so that we can lazily compute the delay while performing IR mutations. A first use case is the DatapathToComb conversion pass. Key changes include adding the IncrementalLongestPathAnalysis class that extends the existing longest path analysis with incremental update capabilities. The implementation includes the PatternRewriter::Listener interface to track IR mutations and maintain analysis validity. New getOrComputeDelay() and getOrComputePaths() methods enable on-demand delay computation, and the incremental analysis is integrated into DatapathCompressOpConversion for timing-aware compress operation lowering. The longest path analysis is inherently complex and use-def chains must be taken into account when rewriting the IR. A top-down rewriting approach is necessary to maintain analysis correctness, as bottom-up mutations can invalidate paths that depend on erased values. The incremental analysis validates that IR mutations don't break existing timing paths and provides early error detection when values used in critical paths are inappropriately modified.

uenoku · 2025-08-29T10:36:01Z

@cowardsa LongestPathAnalysis.cpp requires more clean up and testing, so not ready for review but the analysis now returns timing model (= logic depth in AIG) for most of comb/hw operations (so it should be usable for datapath lowering).
Certainly we can introduce an op interface to override some of the analysis but I'd like to do that in the follow-up.
I made changes on DatapathToComb which requires analysis to work. Specifically I replaced DialectConversion with GreedyRewirter due to the lack of top-down rewrite in DialectConversion. I hope that's fine change.

This is a change of AIG depth for your benchmark set for the current PR FYI.

add_three 12 -> 12
blend 49 -> 49
dot_product 43 -> 43
-----------
fmaa 72 -> 67
fmaa_shae 45 -> 43
fma 64  -> 61

cowardsa · 2025-08-29T11:18:51Z

This is fantastic! Very pleasing to see this play out in the results as well - you need the top-down traversal so that we have lowered everything which the current operator depends on to ensure that we can compute the timing information. Is that correct? I actually like this a lot as it also means that partial_product operators will be lowered before we reach the compressor trees that consume their outputs. Nice!

I'm hoping to have a timing-driven compressor tree PR ready today or early next week - but will perhaps separate out the datapathToComb changes to avoid conflicts :)

uenoku · 2025-08-30T03:51:35Z

ensure that we can compute the timing information. Is that correct?

Exactly! It's necessary to topologically sort the IR beforehand because HWModule has a graph region but after that top-down lowering should ensure that operand is lowered before the operation.

…able timing-aware DatapathToComb optimization This commit enhances the AIG longest path analysis infrastructure to support operations from the Comb and HW dialects, enabling comprehensive timing analysis across mixed-dialect circuits. The DatapathToComb pass has been updated to use timing-aware optimization strategies through the incremental longest path analysis framework. The longest path analysis now handles comb.and, comb.or, comb.xor, and hw.constant operations alongside existing AIG operations, providing more accurate delay calculations for circuits that haven't been fully lowered to AIG. An Oracle class has been introduced to dynamically analyze unsupported operations by creating temporary modules and running synthesis pipelines to extract timing characteristics. The DatapathToComb pass integrates IncrementalLongestPathAnalysis as a listener in its greedy rewrite driver, enabling timing-aware transformations. The compressor lowering logic now sorts addends by arrival time to minimize critical path delays during Wallace tree construction. Configuration options for timing-aware datapath optimizations have been added to the synthesis pipeline. Test coverage has been expanded to verify longest path analysis behavior with mixed dialects and ensure proper timing optimization during datapath-to-combinational lowering. This enables more sophisticated timing optimization strategies throughout the CIRCT synthesis pipeline by providing accurate delay information across

lib/Dialect/AIG/Analysis/PrintLongestPathAnalysis.cpp

cowardsa

Incremental review - will continue but hit a weird GitHub interface bug...

cowardsa · 2025-09-01T12:19:55Z

lib/Conversion/DatapathToComb/DatapathToComb.cpp

+      for (size_t j = 0; j < addends[0].size(); ++j) {
+        SmallVector<std::pair<int64_t, Value>> delays;
+        for (auto &addend : addends) {
+          auto delay = analysis->getOrComputeMaxDelay(addend[j], 0);
+          if (failed(delay))
+            return rewriter.notifyMatchFailure(op,
+                                               "Failed to get delay for input");
+          delays.push_back(std::make_pair(*delay, addend[j]));
+        }
+        std::stable_sort(delays.begin(), delays.end(),
+                         [](const std::pair<int64_t, Value> &a,
+                            const std::pair<int64_t, Value> &b) {
+                           return a.first < b.first;
+                         });
+        for (size_t i = 0; i < addends.size(); ++i)
+          addends[i][j] = delays[i].second;
+      }


FYI - will pass the "delays" directly to the constructor in an upcoming PR - but current placeholder looks good

cowardsa · 2025-09-01T12:25:03Z

lib/Conversion/DatapathToComb/DatapathToComb.cpp

+  // Set the listener to update timing information
+  // HACK: Setting max iterations to 2 to ensure that the patterns are one-shot,
+  // making sure target operations are datapath operations are replaced.
+  config.setMaxIterations(2).setListener(analysis).setUseTopDownTraversal(true);


Is it possible to add a check after applying the rewriter to ensure that there are no "Illegal" operators from the Datapath dialect - to ensure we satisfy the conditions imposed by the conversion pass?

Added in runOnOperation 👍

cowardsa · 2025-09-01T12:30:06Z

lib/Dialect/AIG/Analysis/LongestPathAnalysis.cpp

+//===----------------------------------------------------------------------===//
+// OperationAnalyzer
+//===----------------------------------------------------------------------===//
+
+// OperationAnalyzer handles timing analysis for individual operations that
+// haven't been converted to AIG yet. It creates isolated modules for each
+// unique operation type/signature combination, runs the conversion pipeline
+// (HW -> Comb -> AIG), and analyzes the resulting AIG representation.
+//
+// This is used as a fallback when the main analysis encounters operations
+// that don't have direct AIG equivalents. The analyzer:
+// 1. Creates a wrapper HW module for the operation
+// 2. Runs the standard conversion pipeline to lower it to AIG
+// 3. Analyzes the resulting AIG to compute timing paths
+// 4. Caches results based on operation name and function signature


This is great! It really nicely captures any improvements to the lowering code itself e.g. adding a new faster adder architecture! This assumes uniform arrival times for all inputs is that correct?

Yes, input arrival times are assumed to be uniform.

cowardsa · 2025-09-01T12:40:49Z

lib/Dialect/AIG/Analysis/LongestPathAnalysis.cpp

+  // Temporary module used to hold wrapper modules during analysis
+  // Each operation gets its own wrapper module created inside this parent
+  mlir::OwningOpRef<mlir::ModuleOp> moduleOp;


Non-blocking! Presumably the scope is a single operation, therefore no input limits are known e.g. if I perform:
{4'd0, a} + {4'd0, b}

Does this get viewed as an 8-bit adder for the purpose of this local lowering? I think this is a perfectly reasonable over-approximation for guiding initial lowering decisions. If we envision a future iterative timing closure loop then we may need a more accurate delay model.

Does this get viewed as an 8-bit adder for the purpose of this local lowering?

Yes, constants are not visible so it will be viewed as 8-bit adder. Maybe we can include the known bits as a part of analysis, e.g. cache[{opName, funcType, inputKnownBits]} that makes things more accurate.

cowardsa

This is really great! I'll be interested to test the performance but the interface looks ideal and will track changes to the lowering code. The tests are also very nice - look forward to seeing how we might incorporate this across the synthesis pipeline :)

cowardsa · 2025-09-01T12:49:58Z

lib/Dialect/AIG/Analysis/LongestPathAnalysis.cpp

+  auto opName = op->getName();
+  auto functionType = getFunctionTypeForOp(op);
+  auto key = std::make_pair(opName, functionType);
+  auto it = cache.find(key);


cowardsa · 2025-09-01T12:52:30Z

lib/Dialect/AIG/Analysis/LongestPathAnalysis.cpp

+  // Connect module inputs to cloned operation operands
+  // Handle type mismatches with bitcast operations
+  for (auto arg : hwModule.getBodyBlock()->getArguments()) {
+    Value input = arg;
+    auto idx = arg.getArgNumber();
+
+    // Insert bitcast if input port type differs from operand type
+    if (input.getType() != cloned->getOperand(idx).getType())
+      input = builder.create<hw::BitcastOp>(
+          op->getLoc(), cloned->getOperand(idx).getType(), input);
+
+    cloned->setOperand(idx, input);
+  }


Q: Why do we get type mismatches? Is this always safe to cast to different bitwidths?

LongestPathAnalysis doesn't look dataflow through array/struct, so it's necessary to use integers for ports. Integers will be bit-cast to original type and they always have the same bitwidth.

uenoku force-pushed the dev/hidetou/incremental branch from e01ee81 to fd778a3 Compare August 25, 2025 08:55

uenoku requested a review from cowardsa August 25, 2025 08:56

uenoku force-pushed the dev/hidetou/incremental branch from fd778a3 to 0f473d2 Compare August 25, 2025 08:58

cowardsa reviewed Aug 26, 2025

View reviewed changes

uenoku force-pushed the dev/hidetou/incremental branch 4 times, most recently from efa345e to da0e7d3 Compare August 29, 2025 10:31

uenoku force-pushed the dev/hidetou/incremental branch 2 times, most recently from e4b2787 to e0d8429 Compare September 1, 2025 04:07

uenoku marked this pull request as ready for review September 1, 2025 04:09

uenoku force-pushed the dev/hidetou/incremental branch 2 times, most recently from d990563 to 831be2d Compare September 1, 2025 04:40

uenoku force-pushed the dev/hidetou/incremental branch from 831be2d to 0cf5b31 Compare September 1, 2025 04:48

uenoku commented Sep 1, 2025

View reviewed changes

lib/Dialect/AIG/Analysis/PrintLongestPathAnalysis.cpp Show resolved Hide resolved

uenoku requested a review from cowardsa September 1, 2025 08:35

cowardsa reviewed Sep 1, 2025

View reviewed changes

cowardsa approved these changes Sep 1, 2025

View reviewed changes

Address comments

1000216

uenoku merged commit 330598e into main Sep 2, 2025
7 checks passed

uenoku deleted the dev/hidetou/incremental branch September 2, 2025 04:23

[AIGLongestPathAnalysis] Implement incremental longest path analysis for IR transformations #8890

[AIGLongestPathAnalysis] Implement incremental longest path analysis for IR transformations #8890

Uh oh!

Conversation

uenoku commented Aug 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cowardsa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

uenoku commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

uenoku commented Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cowardsa commented Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

uenoku commented Aug 30, 2025

Uh oh!

Uh oh!

cowardsa 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!

cowardsa Sep 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

uenoku Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cowardsa 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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

uenoku commented Aug 25, 2025 •

edited

Loading

uenoku commented Aug 26, 2025 •

edited

Loading

uenoku commented Aug 29, 2025 •

edited

Loading

cowardsa commented Aug 29, 2025 •

edited

Loading

cowardsa Sep 1, 2025 •

edited

Loading

uenoku Sep 2, 2025 •

edited

Loading