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.

Sign up for GitHub

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

Jump to bottom

[BOLT] Fix reorder data test for RISC-V #68996

Merged
merged 2 commits into from
Oct 16, 2023
Merged

[BOLT] Fix reorder data test for RISC-V #68996

merged 2 commits into from
Oct 16, 2023

Conversation

mtvec
Copy link
Contributor

@mtvec mtvec commented Oct 13, 2023 •
edited
Loading

On RISC-V, small data objects are put in the .sdata section by default. This causes the reorder-data-writable-ptload.c test to fail since it hard-codes the section to optimize to .data.

This patch passes the -fPIC -pie flags to clang to ensure the objects are added to .data on RISC-V.

@mtvec
[BOLT] Fix reorder data test for RISC-V
e1805ef 
On RISC-V, small data objects are put in the `.sdata` section by
default. This causes the `reorder-data-writable-ptload.c` to fail since
it hard-codes the section to optimize to `.data`.

This patch passes the `-msmall-data-limit=0` option to clang on RISC-V
to ensure the objects are added to `.data`.
@mtvec mtvec added the BOLT label Oct 13, 2023
@yota9
Copy link
Member

yota9 commented Oct 13, 2023

As I see in clang adding "-fPIC -pie" to test might help too, I think it would be better solution, could you please try it?
The second option might be changing int hot1 = 42 to int hot1[8] = {42}; for example, what do you think?

@mtvec
Copy link
Contributor Author

mtvec commented Oct 16, 2023

As I see in clang adding "-fPIC -pie" to test might help too, I think it would be better solution, could you please try it?

That works as well and is a bit cleaner. Thanks!

yota9
yota9 approved these changes Oct 16, 2023
View reviewed changes
Copy link
Member

@yota9 yota9 left a comment

Choose a reason for hiding this comment

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

LGTM! Please fix the patch description before merging

@mtvec mtvec merged commit 8592241 into llvm:main Oct 16, 2023
3 checks passed
@madhur13490 madhur13490 mentioned this pull request Oct 20, 2023
Closed
@mtvec mtvec deleted the bolt-fix-test-riscv branch October 23, 2023 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yota9 yota9 yota9 approved these changes

@aaupov aaupov Awaiting requested review from aaupov aaupov is a code owner

@maksfb maksfb Awaiting requested review from maksfb maksfb is a code owner

@rafaelauler rafaelauler Awaiting requested review from rafaelauler rafaelauler is a code owner

Assignees
No one assigned
Labels
BOLT
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mtvec @yota9