This is an archive of the discontinued LLVM Phabricator instance.

[NFC] Optimize vector usage in lld
ClosedPublic

Authored by gAlfonso-bit on Dec 7 2022, 9:14 AM.

Details

Reviewers
MaskRay
int3
Group Reviewers
Restricted Project
Commits
rG3df4c5a92f7f: [NFC] Optimize vector usage in lld
Summary

By using emplace_back, as well as converting some loops to for-each, we can do more efficient vectorization.

Make copy constructor for TemporaryFile noexcept.

Diff Detail

Event Timeline

gAlfonso-bit created this revision.Dec 7 2022, 9:14 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptDec 7 2022, 9:14 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
gAlfonso-bit requested review of this revision.Dec 7 2022, 9:14 AM

Fix formatting

int3 added a subscriber: int3.Dec 8 2022, 7:15 PM
int3 added inline comments.
lld/COFF/Writer.cpp
521 ↗(On Diff #481332)

codebase convention is to use explicit type instead of auto as long as it's not too ugly

lld/ELF/InputFiles.cpp
1709 ↗(On Diff #481332)

c seems like a better name for a char

also there's no need to take this by reference

lld/ELF/MapFile.cpp
248 ↗(On Diff #481332)

let's keep this, seems like it was intentionally done to help readability

int3 requested changes to this revision.Jan 24 2023, 5:04 PM

clearing queue

This revision now requires changes to proceed.Jan 24 2023, 5:04 PM
gAlfonso-bit marked 3 inline comments as done.
int3 accepted this revision.Jan 25 2023, 5:08 PM

Thanks!

lld/COFF/DriverUtils.cpp
350 ↗(On Diff #492237)

maybe add a remark about this noexcept to the commit message

This revision is now accepted and ready to land.Jan 25 2023, 5:08 PM
andrewng added inline comments.
lld/ELF/InputFiles.cpp
1711 ↗(On Diff #492237)

Just saw this in passing, I think c needs to be a reference as the intention is to change the character in s.

gAlfonso-bit marked an inline comment as done.
gAlfonso-bit edited the summary of this revision. (Show Details)
gAlfonso-bit marked an inline comment as done.

Done!

@int3 I do not have permission to merge this. Could you?

This revision was automatically updated to reflect the committed changes.
int3 added a comment.Mar 3 2023, 9:51 AM

Just noticed I'd messed up the author attribution in the commit. Sorry ):