Skip to content

[CI] Added Quick Titanium S10 Tests #3104

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 4, 2025
Merged

Conversation

AlexandreSinger
Copy link
Contributor

The titanium benchmarks were not being tested by the CI. Added the Titanium benchmarks which could be run in under around 2 hours to NightlyTest7.

5 circuits in this benchmark set currently fail through VTR. The failures are mainly in the initial placer, which is struggling to create an initial placement when logical blocks can be placed into different physical block types which are constrained resources.

@AlexandreSinger AlexandreSinger force-pushed the feature-s10-titanium-task branch from 3b52715 to d2d6aa9 Compare June 4, 2025 00:14
@AlexandreSinger
Copy link
Contributor Author

@vaughnbetz I have added the working Titanium circuits to NightlyTest7 with this PR. I will raise an issue on the failing tests. I found that the failing tests were, in fact, running 2 iterations of the initial placer (which is correct). I think the issue with the initial placer is that the logical blocks which can be placed in either LABs or MLABs may be placed too early and are taking the resources of logical blocks which must be placed in LAB tiles or must be placed in MLAB tiles.

The titanium benchmarks were not being tested by the CI. Added the
Titanium benchmarks which could be run in under around 2 hours to
NightlyTest7.

5 circuits in this benchmark set currently fail through VTR. The
failures are mainly in the initial placer, which is struggling to create
an initial placement when logical blocks can be placed into different
physical block types which are constrained resources.
@AlexandreSinger AlexandreSinger force-pushed the feature-s10-titanium-task branch from d2d6aa9 to 892af1c Compare June 4, 2025 01:06
@vaughnbetz
Copy link
Contributor

Thanks @AlexandreSinger ! Is this with an auto sized device? If so, it is strange. @amin1377 : I think you landed some initial placement changes a while ago. Do blocks eventually do an exhaustive search?

@AlexandreSinger
Copy link
Contributor Author

Thanks @AlexandreSinger ! Is this with an auto sized device? If so, it is strange. @amin1377 : I think you landed some initial placement changes a while ago. Do blocks eventually do an exhaustive search?

Hi Vaughn, yes, this is with an auto-sized device. I raised an issue on this topic which provides a bit more information: #3105

I think a "simple" solution would be to place the logical blocks which can only be placed in one and only one physical tile first (since they are more constrained), and then placing the more free logical blocks second. We could add a third iteration of the initial placer which does this, which would prevent this update from affecting other benchmarks. Initial Placement is quite quick, so running 3 iterations of it is not crazy in my opinion.

@amin1377
Copy link
Contributor

amin1377 commented Jun 4, 2025

@vaughnbetz: You’re right; if centroid placement fails, it will eventually fall back to an exhaustive search to find a location for the blocks.

I'm not sure if this is related to what Alex is observing, but I recall that Sara encountered a similar issue two years ago. In her case, it was caused by DSP chains: although the number of resources was sufficient, the column height had to be increased beyond what seemed necessary in order to fit the chains properly.

@vaughnbetz
Copy link
Contributor

@AlexandreSinger : your proposed fix makes sense to me. Here's a tweak that could also help: detect what kind of block failed to place and move it to the front in later iterations (we already detect blocks that failed, but we could generalize that to put blocks that failed to place first in the second iteration, and block types that failed right after the blocks that failed). We should also check that those terms swamp any "connected to placed blocks" terms so they dominate in later iterations. Anybody want to try coding a fix?

Copy link
Contributor

@vaughnbetz vaughnbetz left a comment

Choose a reason for hiding this comment

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

LGTM

@vaughnbetz vaughnbetz merged commit e6df482 into master Jun 4, 2025
32 checks passed
@vaughnbetz vaughnbetz deleted the feature-s10-titanium-task branch June 4, 2025 22:13
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.

3 participants