Skip to content

Conversation

@Stylie777
Copy link
Contributor

@Stylie777 Stylie777 commented Dec 2, 2025

When #152736 was initially merged, the assert that checks for the chunksize when applying a static-chunked schedule was incorrect. While it would not have changed the behaviour of the assert, the string attached to it would have been emitted in cases where it was simplified.

This was raised here: #152736 (comment)

Testing for this was explored, but this assert is a last chance failure point that should never be reached as applyWorkshareLoop decides the EffectiveScheduleType based on the existence of ChunkSize or DistScheduleChunkSize, so this will only trigger if there are issues with that conversion, and UnitTesting already exists for applyWorkshareLoop

When llvm#152736 was initially merged, the assert that checks for the
chunksize when applying a static-chunked schedule was incorrect.
While it would not have changed the behaviour of the assert, the
string attached to it would have been emitted in cases where it
was simplified.
@github-actions
Copy link

github-actions bot commented Dec 2, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@Stylie777 Stylie777 merged commit e5603da into llvm:main Dec 4, 2025
10 checks passed
@Stylie777 Stylie777 deleted the dist_schedule_assert_fix branch December 4, 2025 10:00
kcloudy0717 pushed a commit to kcloudy0717/llvm-project that referenced this pull request Dec 4, 2025
When llvm#152736 was initially merged, the assert that checks for the
chunksize when applying a static-chunked schedule was incorrect. While
it would not have changed the behaviour of the assert, the string
attached to it would have been emitted in cases where it was simplified.

This was raised here:
llvm#152736 (comment)

Testing for this was explored, but this assert is a last chance failure
point that should never be reached as applyWorkshareLoop decides the
`EffectiveScheduleType` based on the existence of `ChunkSize` or
`DistScheduleChunkSize`, so this will only trigger if there are issues
with that conversion, and UnitTesting already exists for
`applyWorkshareLoop`
honeygoyal pushed a commit to honeygoyal/llvm-project that referenced this pull request Dec 9, 2025
When llvm#152736 was initially merged, the assert that checks for the
chunksize when applying a static-chunked schedule was incorrect. While
it would not have changed the behaviour of the assert, the string
attached to it would have been emitted in cases where it was simplified.

This was raised here:
llvm#152736 (comment)

Testing for this was explored, but this assert is a last chance failure
point that should never be reached as applyWorkshareLoop decides the
`EffectiveScheduleType` based on the existence of `ChunkSize` or
`DistScheduleChunkSize`, so this will only trigger if there are issues
with that conversion, and UnitTesting already exists for
`applyWorkshareLoop`
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.

2 participants