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

[RISCV][LLD] Add RISCV zcmt optimise in linker relaxation #68551

Closed
wants to merge 36 commits into from

Conversation

Xinlong-Wu
Copy link
Contributor

This patch is moved from https://reviews.llvm.org/D134600

This patch implements optimizations for the zcmt extension in lld.

A new TableJumpSectio has been added.

Scans each R_RISCV_CALL/R_RISCV_CALL_PLT relocType in each section before the linker relaxation, recording the name of the symbol.

In finalizeContents the recorded symbol names are sorted in descending order by the number of jumps.

Optimise and insert a new cm.jt/cm.jalt during the relax process. in the process, we reused theR_RISCV_JAL relocType

co-author: @ScottEgerton

@jrtc27
Copy link
Collaborator

jrtc27 commented Oct 9, 2023

This patch is moved from https://reviews.llvm.org/D134600

Why? That loses all the context in the Phabricator review. Keep the review there.

@github-actions
Copy link

github-actions bot commented Oct 9, 2023

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 7cc1bfaf371c4a816cf4e62fe31d8515bf8f6fbd 20d59b945dcd57fb523118e3116492a2863a7b4d -- lld/ELF/Arch/RISCV.cpp lld/ELF/Config.h lld/ELF/Driver.cpp lld/ELF/SyntheticSections.h lld/ELF/Target.h lld/ELF/Writer.cpp
View the diff from clang-format here.
diff --git a/lld/ELF/Arch/RISCV.cpp b/lld/ELF/Arch/RISCV.cpp
index 859dcdb10fd8..7adaa6925ddf 100644
--- a/lld/ELF/Arch/RISCV.cpp
+++ b/lld/ELF/Arch/RISCV.cpp
@@ -607,7 +607,7 @@ static void initSymbolAnchors() {
 }
 
 static bool relaxTableJump(const InputSection &sec, size_t i, uint64_t loc,
-                      Relocation &r, uint32_t &remove) {
+                           Relocation &r, uint32_t &remove) {
   if (!in.riscvTableJumpSection || !in.riscvTableJumpSection->isFinalized)
     return false;
 

@Xinlong-Wu
Copy link
Contributor Author

This patch is moved from https://reviews.llvm.org/D134600

Why? That loses all the context in the Phabricator review. Keep the review there.

yes, I will keep it there.

But the Phabricator shutdown timeline said that

Phabricator becomes read-only after October 1

So I thought it may means we can't add comments or update it? That's why I moved it from Phabricator.

If you mean we still can review&update it at Phabricator, I will close this pr. )

@jrtc27
Copy link
Collaborator

jrtc27 commented Oct 9, 2023

This patch is moved from https://reviews.llvm.org/D134600

Why? That loses all the context in the Phabricator review. Keep the review there.

yes, I will keep it there.

But the Phabricator shutdown timeline said that

Phabricator becomes read-only after October 1

So I thought it may means we can't add comments or update it? That's why I moved it from Phabricator.

If you mean we still can review&update it at Phabricator, I will close this pr. )

Scroll down the thread and you will find https://discourse.llvm.org/t/update-on-github-pull-requests/71540/125. There's consensus that Phabricator will remain usable for existing revisions for a while longer for this specific kind of situation, and that migrating revisions off of it loses valuable context and should be avoided.

@Xinlong-Wu
Copy link
Contributor Author

This patch is moved from https://reviews.llvm.org/D134600

Why? That loses all the context in the Phabricator review. Keep the review there.

yes, I will keep it there.
But the Phabricator shutdown timeline said that

Phabricator becomes read-only after October 1

So I thought it may means we can't add comments or update it? That's why I moved it from Phabricator.
If you mean we still can review&update it at Phabricator, I will close this pr. )

Scroll down the thread and you will find https://discourse.llvm.org/t/update-on-github-pull-requests/71540/125. There's consensus that Phabricator will remain usable for existing revisions for a while longer for this specific kind of situation, and that migrating revisions off of it loses valuable context and should be avoided.

That make sense. I will close this pr

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.

None yet

2 participants