[PATCH] [Layout] Assign ordinals in resolution order
Rui Ueyama
ruiu at google.com
Tue Oct 8 22:09:52 PDT 2013
Overall this looks pretty good! :) After submitting this patch, the file ordinal will be assigned as the same order as LinkingContext::nextFile() returns file objects. I believe currently it's the same order as the command line file order in all flavors, but it's not really enforced. We probably should add a comment to nextFile() about the fact.
================
Comment at: include/lld/Core/File.h:80
@@ -82,1 +79,3 @@
+ /// Sets the command line order of the file.
+ virtual void setOrdinal(uint64_t ordinal) const { _ordinal = ordinal; }
----------------
Why virtual?
================
Comment at: lib/Core/Resolver.cpp:306
@@ +305,3 @@
+ if (nextFile->kind() == File::kindArchiveLibrary) {
+ if (nextFile->getOrdinal() != (uint64_t)(-1))
+ nextFile->setOrdinal(_context.getNextOrdinalAndIncrement());
----------------
I'd define File::hasOrdinal() and move this comparison into that function.
================
Comment at: lib/Core/Resolver.cpp:302
@@ -299,1 +301,3 @@
+ if (nextFile->kind() == File::kindObject) {
+ nextFile->setOrdinal(_context.getNextOrdinalAndIncrement());
handleFile(*nextFile);
----------------
Don't you need to check if an ordinal has already been assigned?
http://llvm-reviews.chandlerc.com/D1865
More information about the llvm-commits
mailing list