Skip to content

Conversation

@markram1729
Copy link
Contributor

Resolves #8967

@markram1729 markram1729 marked this pull request as ready for review September 17, 2025 06:00
Copy link
Member

@uenoku uenoku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! 🙏🙏🙏 Could you change python bindings/tests as well?

@markram1729
Copy link
Contributor Author

Thank you! 🙏🙏🙏 Could you change python bindings/tests as well?

Sure!

};

// An OpenPath represents a path from a fan-in with an associated
// An OpenPath represents a path from a startPoint with an associated
Copy link
Member

@uenoku uenoku Sep 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: it's a bit tedious but could you use "start point" in the non code part?

Suggested change
// An OpenPath represents a path from a startPoint with an associated
// An OpenPath represents a path from a start point with an associated

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure!

@markram1729 markram1729 marked this pull request as draft September 17, 2025 11:37
@markram1729
Copy link
Contributor Author

Hi @uenoku , I have resolved as mentioned

I am having issue at

// JSON-SAME{LITERAL}: "start_point":{"bit_pos":0,"instance_path":[],"name":"a"},

How can i make this check with JSON-SAME{LITERAL} , I set the order after history because
Json is emitting lexographically

fan_in used to emit before history
now start_point emits after history []

can you suggest how can i resolve this

also please suggest if i need to change anything at PrintLongestPathAnalysis.cpp

@markram1729 markram1729 marked this pull request as ready for review September 18, 2025 10:13
@uenoku
Copy link
Member

uenoku commented Sep 18, 2025

It seems the following works. You can just remove trailing commas.

diff --git a/test/Dialect/Synth/longest-paths-report.mlir b/test/Dialect/Synth/longest-paths-report.mlir
index a3e76500f..19cac028c 100644
--- a/test/Dialect/Synth/longest-paths-report.mlir
+++ b/test/Dialect/Synth/longest-paths-report.mlir
@@ -37,10 +37,10 @@
 // JSON-SAME{LITERAL}:    [{"level":1,"count":1,"percentage":50},
 // JSON-SAME{LITERAL}:     {"level":2,"count":1,"percentage":100}]
 // JSON-SAME{LITERAL}:    "top_paths":[
-// JSON-SAME{LITERAL}:     {"end_point":{"bit_pos":0,"instance_path":[],"name":"y"},
-// JSON-SAME{LITERAL}:      "path":{"delay":2,"history":[{"comment":"output port","delay":2,"object":{"bit_pos":0,"instance_path":[],"name":"c2.x"}},
-// JSON-SAME{LITERAL}:      "start_point":{"bit_pos":0,"instance_path":[],"name":"a"},
-// JSON-SAME{LITERAL}:      "root":"parent"},
+// JSON-SAME{LITERAL}:     {"end_point":{"bit_pos":0,"instance_path":[],"name":"y"}
+// JSON-SAME{LITERAL}:      "path":{"delay":2,"history":[{"comment":"output port","delay":2,"object":{"bit_pos":0,"instance_path":[],"name":"c2.x"}}
+// JSON-SAME{LITERAL}:      "start_point":{"bit_pos":0,"instance_path":[],"name":"a"}
+// JSON-SAME{LITERAL}:      "root":"parent"
 // Make sure the second path is reported.
 // JSON-SAME{LITERAL}:     {"end_point":{"bit_pos":0,"instance_path":[],"name":"x"},
 

Copy link
Member

@uenoku uenoku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, really thank you for doing this!

@uenoku uenoku merged commit f446e29 into llvm:main Sep 18, 2025
7 checks passed
@markram1729 markram1729 deleted the exaSynthfan branch September 19, 2025 04:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Synth] Replace terminology fan_in/fan_out with start_point/end_point in LongestPathAnalysis

2 participants