Skip to content

[BOLT][Instrumentation] Initial instrumentation support for RISCV64 #133882

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

Merged
merged 3 commits into from
Apr 17, 2025
Merged

[BOLT][Instrumentation] Initial instrumentation support for RISCV64 #133882

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1ae74d5
[Bolt][Instrumentation] Initial instrumentation support for RISCV64
WangJee Apr 1, 2025
5917020
Merge branch 'main' into main
WangJee Apr 15, 2025
9bde5ab
[Bolt][Instrumentation] Initial instrumentation support for RISCV64
WangJee Apr 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion bolt/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ endforeach()

set(BOLT_ENABLE_RUNTIME_default OFF)
if ((CMAKE_SYSTEM_PROCESSOR STREQUAL "x86_64"
OR CMAKE_SYSTEM_PROCESSOR MATCHES "^(arm64|aarch64)$")
OR CMAKE_SYSTEM_PROCESSOR MATCHES "^(arm64|aarch64)$"
OR CMAKE_SYSTEM_PROCESSOR STREQUAL "riscv64")
AND (CMAKE_SYSTEM_NAME STREQUAL "Linux"
OR CMAKE_SYSTEM_NAME STREQUAL "Darwin")
AND (NOT CMAKE_CROSSCOMPILING))
Expand Down
4 changes: 4 additions & 0 deletions bolt/lib/Core/Relocation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ static bool isSupportedRISCV(uint32_t Type) {
case ELF::R_RISCV_LO12_S:
case ELF::R_RISCV_64:
case ELF::R_RISCV_TLS_GOT_HI20:
case ELF::R_RISCV_TLS_GD_HI20:
case ELF::R_RISCV_TPREL_HI20:
case ELF::R_RISCV_TPREL_ADD:
case ELF::R_RISCV_TPREL_LO12_I:
Expand Down Expand Up @@ -236,6 +237,7 @@ static size_t getSizeForTypeRISCV(uint32_t Type) {
case ELF::R_RISCV_64:
case ELF::R_RISCV_GOT_HI20:
case ELF::R_RISCV_TLS_GOT_HI20:
case ELF::R_RISCV_TLS_GD_HI20:
// See extractValueRISCV for why this is necessary.
return 8;
}
Expand Down Expand Up @@ -491,6 +493,7 @@ static uint64_t extractValueRISCV(uint32_t Type, uint64_t Contents,
return extractBImmRISCV(Contents);
case ELF::R_RISCV_GOT_HI20:
case ELF::R_RISCV_TLS_GOT_HI20:
case ELF::R_RISCV_TLS_GD_HI20:
// We need to know the exact address of the GOT entry so we extract the
// value from both the AUIPC and L[D|W]. We cannot rely on the symbol in the
// relocation for this since it simply refers to the object that is stored
Expand Down Expand Up @@ -707,6 +710,7 @@ static bool isPCRelativeRISCV(uint32_t Type) {
case ELF::R_RISCV_RVC_BRANCH:
case ELF::R_RISCV_32_PCREL:
case ELF::R_RISCV_TLS_GOT_HI20:
case ELF::R_RISCV_TLS_GD_HI20:
return true;
}
}
Expand Down
10 changes: 5 additions & 5 deletions bolt/lib/Rewrite/RewriteInstance.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2926,12 +2926,12 @@ void RewriteInstance::handleRelocation(const SectionRef &RelocatedSection,

if (BinaryData *BD = BC->getBinaryDataContainingAddress(SymbolAddress)) {
// Note: this assertion is trying to check sanity of BinaryData objects
// but AArch64 has inferred and incomplete object locations coming from
// GOT/TLS or any other non-trivial relocation (that requires creation
// of sections and whose symbol address is not really what should be
// encoded in the instruction). So we essentially disabled this check
// but AArch64 and RISCV has inferred and incomplete object locations
// coming from GOT/TLS or any other non-trivial relocation (that requires
// creation of sections and whose symbol address is not really what should
// be encoded in the instruction). So we essentially disabled this check
// for AArch64 and live with bogus names for objects.
assert((IsAArch64 || IsSectionRelocation ||
assert((IsAArch64 || BC->isRISCV() || IsSectionRelocation ||
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please update the comment above and substitute AArch64 with either RISC or non-x86?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, it has been modified

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you please help me merge it, because I don't have the permission to merge yet?

BD->nameStartsWith(SymbolName) ||
BD->nameStartsWith("PG" + SymbolName) ||
(BD->nameStartsWith("ANONYMOUS") &&
Expand Down
Loading
Loading