Skip to content

Conversation

@mgudim
Copy link
Contributor

@mgudim mgudim commented Jul 3, 2025

Note that this is not 100% functionally correct, but all spec benchmarks seem ok, at least in my testing.

I've only measured cycles on small benchmark extracted from x264. I compiled this benchmark for a couple of upstreamed cpus, but I didn't notice significant difference. It does make a significant difference for our cpu though

@mgudim mgudim requested review from lukel97, preames and topperc July 3, 2025 23:18
Copy link
Contributor

@lukel97 lukel97 left a comment

Choose a reason for hiding this comment

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

Do you have a test diff which shows the scheduling change?

@preames
Copy link
Collaborator

preames commented Jul 23, 2025

Note that this is not 100% functionally correct,

This seems like a rather major problem. Why should we spend review time on something which is known not to be correct? Is there an purpose to having the off-by-default option?

@mgudim
Copy link
Contributor Author

mgudim commented Jul 23, 2025

Note that this is not 100% functionally correct,

This seems like a rather major problem. Why should we spend review time on something which is known not to be correct? Is there an purpose to having the off-by-default option?

@preames Sorry, I didn't mean to ask for a review on this or merge this. This is the "quick and dirty" solution. In case you wanted to see the performance impact.

I posted the draft of what this code should look like here: #149574 (comment)

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