This is an archive of the discontinued LLVM Phabricator instance.

LLD: Move ELF/Mips.cpp to ELF/Arch/MipsArchTree.cpp
ClosedPublic

Authored by kzhuravl on Jun 19 2017, 11:36 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

kzhuravl created this revision.Jun 19 2017, 11:36 AM
ruiu edited edge metadata.Jun 19 2017, 11:48 AM

I think we shouldn't do this. It might make sense to move this file under Arch, but merging it with Arch/Mips.cpp decreases readability as the file now contains totally unrelated functions.

In D34358#784407, @ruiu wrote:

I think we shouldn't do this. It might make sense to move this file under Arch, but merging it with Arch/Mips.cpp decreases readability as the file now contains totally unrelated functions.

I had a similar feeling but was not sure. Do you have a name suggestion if it is moved under Arch?

Adding Simon Atanasyan for his opinion.

ruiu added a comment.Jun 19 2017, 12:30 PM

How about renaming the file MipsArchTree.cpp or something like that?

kzhuravl updated this revision to Diff 103100.Jun 19 2017, 1:46 PM
kzhuravl retitled this revision from LLD: Move the rest of the Mips into Arch/Mips.cpp to LLD: Move ELF/Mips.cpp to ELF/Arch/MipsArchTree.cpp.

Address review feedback.

In D34358#784445, @ruiu wrote:

How about renaming the file MipsArchTree.cpp or something like that?

Done.

ruiu accepted this revision.Jun 19 2017, 1:50 PM

LGTM

ELF/Arch/MipsArchTree.cpp
1 ↗(On Diff #103100)

Fix this line.

This revision is now accepted and ready to land.Jun 19 2017, 1:50 PM
This revision was automatically updated to reflect the committed changes.
kzhuravl marked an inline comment as done.
lld/trunk/ELF/CMakeLists.txt