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

Change constants in BHive conversion script #39

Merged

Conversation

boomanaiden154
Copy link
Collaborator

@boomanaiden154 boomanaiden154 commented Jan 27, 2024

This patch adjusts the constants in the BHive conversion script from the default values added in with the script to the values used within the original BHive paper. These constants were experimentally determined within the BHive paper to be high enough to not underflow/access addresses in the first page yet low enough to not access any addresses above the virtual address space ceiling. Anecdotally, these constants have also seemed to work better than the original ones. In addition, this patch prefixes the memory value with zeroes so that llvm-exegesis is able to assume the correct bit width.

This patch adjusts the constants in the BHive conversion script to the
values used within the original BHive paper. Anecdotally, these
constants have also seemed to work better than the original ones. In
addition, this patch prefixes the memory value with zeroes so that
llvm-exegesis is able to assume the correct bit width.
@mtrofin
Copy link
Collaborator

mtrofin commented Feb 3, 2024

the values used within the original BHive paper. Anecdotally, these constants have also seemed to work better than the original ones.

which are the original counter values, the ones in the original paper or the ones before this change?

@@ -64,6 +65,14 @@ int main(int argc, char* argv[]) {
gematria::ConvertHexToString(kInitialRegVal);
std::string initial_mem_val_str =
gematria::ConvertHexToString(kInitialMemVal);
// Prefix the string with zeroes as llvm-exegesis assumes the bit width
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is unrelated?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Slightly unrelated, but necessary to make this patch work properly. We're assuming the memory value has a width of 64 bits, but only setting the first x bits. Exegesis determines the width of the value based on how many characters are in the string, so we need to explicitly print out the leading zeros.

@boomanaiden154
Copy link
Collaborator Author

which are the original counter values, the ones in the original paper or the ones before this change?

The ones before this change. We're switching to the values in the BHive paper here.

@mtrofin
Copy link
Collaborator

mtrofin commented Feb 3, 2024

which are the original counter values, the ones in the original paper or the ones before this change?

The ones before this change. We're switching to the values in the BHive paper here.

Ack, if you can update the patch description

@boomanaiden154 boomanaiden154 merged commit 7d528cc into google:main Feb 3, 2024
6 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.

2 participants