Skip to content

Add eld as default linker for Qualcomm Hexagon compiler #14632

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

quic-k
Copy link

@quic-k quic-k commented May 22, 2025

Qualcomm opensourced their linker recently https://github.com/qualcomm/eld
This PR sets eld as the default linker for Qualcomm's Hexagon compiler.

@quic-k quic-k requested review from dcbaker and jpakkane as code owners May 22, 2025 06:24
@quic-k
Copy link
Author

quic-k commented May 22, 2025

hi @dcbaker
2 checks fail, but I don't think that is due to my changes

@dcbaker
Copy link
Member

dcbaker commented May 22, 2025

We can't change the linker ID, that's a part of the DSL, with compiler.get_linker_id().

It also doesn't look like eld is just a rename of their existing qlcd linker, but it's a completely different linker.

So, we should have a new representation for that, instead of renaming the old one. Also, please, add the new representation in one patch, and use it in a second patch. There's some documentation changes for this too.

@quic-k
Copy link
Author

quic-k commented May 23, 2025

thanks for the quick review
yes, a new representation for eld makes sense
I have added eld as a new linker in the first commit and used it for Qualcomm's Hexagon compiler in the second commit
also, let know what docs changes, if any, are required

@dcbaker
Copy link
Member

dcbaker commented May 23, 2025

This all looks good, thanks.

There's a page called the reference table (can't remember what the .md name is) that has a table of linker id's, that needs to be updated. Also, a release snippet that support was added would be good.

@quic-k
Copy link
Author

quic-k commented May 26, 2025

thanks
addressed documentation changes

quic-k added 4 commits May 28, 2025 11:29
ELD is Qualcomm's open-source embedded linker.
https://github.com/qualcomm/eld

Signed-off-by: Kushal Pal <kushpal@qti.qualcomm.com>
Signed-off-by: Kushal Pal <kushpal@qti.qualcomm.com>
Signed-off-by: Kushal Pal <kushpal@qti.qualcomm.com>
Signed-off-by: Kushal Pal <kushpal@qti.qualcomm.com>
@dcbaker
Copy link
Member

dcbaker commented May 29, 2025

Thanks for making all of the changes!

@thesamesam, any more comments, or are you happy to merge?

Copy link
Member

@thesamesam thesamesam left a comment

Choose a reason for hiding this comment

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

Looks good to me now! Please do merge.

@eli-schwartz
Copy link
Member

Sone of these changes look like they're all the same logical unit of change, and should be squashed into a single logical commit. e.g. documentation changes should be one change.

(It is not clear to me whether the first two commits are doing one thing -- adding support for detecting eld -- or two things -- adding support for eld and making meson switch the default linker to use.)

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