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

Add support for SHT_RELA in vkgc #2857

Merged
merged 1 commit into from
Nov 29, 2023
Merged

Conversation

jayfoad
Copy link
Member

@jayfoad jayfoad commented Nov 29, 2023

Currently LLVM generates relocations in an ELF SHT_REL section. To allow for changing the compiler to use SHT_RELA instead, this patch updates vkgcElfReader and vkgcPipelineDumper to support both section types.

Currently LLVM generates relocations in an ELF SHT_REL section. To
allow for changing the compiler to use SHT_RELA instead, this patch
updates vkgcElfReader and vkgcPipelineDumper to support both section
types.
@jayfoad jayfoad requested a review from a team as a code owner November 29, 2023 14:56
@jayfoad
Copy link
Member Author

jayfoad commented Nov 29, 2023

TBH I am not too happy with this. vkgcElfReader is all based around known section names instead of section types, and now it is assuming that only one of ".rel.text" and ".rela.text" will exist.

@amdvlk-admin
Copy link

Test summary for commit 631d2e2

CTS tests (Failed: 0/138378)
  • Built with version 1.3.5.2
  • Ubuntu navi3x, Srdcvk
    • Passed: 35162/69163 (50.8%)
    • Failed: 0/69163 (0.0%)
    • Not Supported: 34001/69163 (49.2%)
    • Warnings: 0/69163 (0.0%)
    Ubuntu navi2x, Srdcvk
    • Passed: 35242/69215 (50.9%)
    • Failed: 0/69215 (0.0%)
    • Not Supported: 33973/69215 (49.1%)
    • Warnings: 0/69215 (0.0%)

Copy link
Member

@Flakebi Flakebi left a comment

Choose a reason for hiding this comment

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

Changes look good to me.
I agree that matching on section names is a crude thing to do.

reloc->useExplicitAddend = false;
auto *section = m_sections[m_relocSecIdx];

if (section->secHead.sh_type == SHT_REL) {
Copy link
Member Author

Choose a reason for hiding this comment

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

For consistency I should probably have checked the section name here instead of the type. But I could not bear to add a string compare to this otherwise fast function.

Copy link
Member

Choose a reason for hiding this comment

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

Moving in the right direction, even if it's by a little bit, is a good thing ;)

@jayfoad jayfoad merged commit 1742c99 into GPUOpen-Drivers:dev Nov 29, 2023
10 checks passed
@jayfoad jayfoad deleted the vkgc-rela branch November 29, 2023 16:20
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.

4 participants