Skip to content
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

Improve efficiency of branch history table #271

Merged
merged 1 commit into from
Nov 22, 2023

Conversation

RinHizakura
Copy link
Collaborator

There are some small mistakes/defects in the original history table design:

  • The least recent branch history won't be checked because the latest history will be put at index ir->branch_table_count, but we only check the range in [0, ir->branch_table_count)
  • If ir->branch_table_count rings back to index 0, the previous history will be dropped because we only check the range in [0, ir->branch_table_count)
  • The PC for every branch history entry can be put densely since they are the key for comparison, so they could be beneficial for cache and probably provide chance to optimize using SIMD operation

@jserv jserv requested a review from qwe661234 November 22, 2023 14:21
@RinHizakura
Copy link
Collaborator Author

RinHizakura commented Nov 22, 2023

It's worth mentioning that the check of the history table is changed to a static range [0, HISTORY_SIZE). I assume that it is impossible to branch to PC 0, so the condition if (ir->branch_table->PC[i] == PC) will always be false for a non-initialized entry. If the assumption is not always correct, more checks should be introduced to avoid accessing the non-initialized object.

Copy link
Collaborator

@qwe661234 qwe661234 left a comment

Choose a reason for hiding this comment

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

Right, the line 52 should be HISTORY_SIZE, instead of ir->branch_table_count. The other modification save 1 byte memory (maybe more bytes becasue of memory alignment) for non indirect jump instruction IR.

@qwe661234
Copy link
Collaborator

qwe661234 commented Nov 22, 2023

It's worth mentioning that the check of the history table is changed to a static range [0, HISTORY_SIZE). I assume that it is impossible to branch to PC 0, so the condition if (ir->branch_table->PC[i] == PC) will always be false for a non-initialized entry. If the assumption is not always correct, more check should be introduced to avoid to access the NULL object.

The error only occurs when jump target is PC 0, but it is not common. Other solution is initializing the PC array to an odd value because the odd jump target is impossible.

src/decode.h Outdated Show resolved Hide resolved
@RinHizakura RinHizakura force-pushed the master branch 2 times, most recently from af524d2 to eafabd4 Compare November 22, 2023 15:04
jserv

This comment was marked as outdated.

@jserv
Copy link
Contributor

jserv commented Nov 22, 2023

By the way, I am wondering which value should be chosen for HISTORY_SIZE.
@qwe661234, Any considerations or explanations for making this adjustment?

@qwe661234
Copy link
Collaborator

By the way, I am wondering which value should be chosen for HISTORY_SIZE. @qwe661234, Any considerations or explanations for making this adjustment?

Based on my observations, the majority of branch targets typically number less than 10. However, for certain hard-to-predict branches, the count exceeds 20. I believe the branch history table is tailored to enhance the performance of indirect jump instructions featuring a limited number of branch targets. Therefore, I propose that the value 16 is suited for this purpose.

@qwe661234 qwe661234 closed this Nov 22, 2023
@qwe661234 qwe661234 reopened this Nov 22, 2023
Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Consider the following change for git commit message:

The previous implementation of the branch history table had some limitations that prevented us from fully leveraging its capabilities. This commit aims to improve access to recent history entries and make it more cache-friendly.

The previous implementation of the branch history table had
some limitations that prevented us from fully leveraging its
capabilities. This commit aims to improve access to recent
history entries and make it more cache-friendly.
@RinHizakura
Copy link
Collaborator Author

By the way, I am wondering which value should be chosen for HISTORY_SIZE.

Actually I am thinking to optimize this with SIMD operation, and HISTORY_SIZE needs to be 8 consider 256 bits vector. But the idea seems to be a sledgehammer to crack a nut at the first glance.

RinHizakura@ad1c7e5

@RinHizakura RinHizakura requested a review from jserv November 22, 2023 16:34
@jserv jserv merged commit dd7ce2e into sysprog21:master Nov 22, 2023
21 checks passed
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