[llvm] r212108 - Add the -arch flag support to llvm-size like what was done to llvm-nm
Kevin Enderby
enderby at apple.com
Tue Jul 1 13:59:11 PDT 2014
On Jul 1, 2014, at 1:29 PM, Jim Grosbach <grosbach at apple.com> wrote:
>
> A few formatting things. Rather than note them individually, I suggest just running clang-format on the patch. I use the vim integration script clang-format.py from the clang/tools/clang-format directory (there’s usage instructions at the top of the script) and works pretty well for me.
OK, I’ll look at that.
>
> Generally, re-walking the -arch command line list multiple times seems odd. Would it be better to walk it once and construct a list of cputype/subtype pairs to compare against in the rest of the code?
The reason for the multiple walking of the -arch command line list is to make the tools do this three bits of logic in this order (with different printing of the header stuff, and errors):
1) If there is no "-arch all" and -arch flags then each arch flag is looked for in a universal file and if there are more than one -arch flags then “(for architecture XYZ)” is printed, else if just one -arch flag this is not printed as the slices are processed. And producing errors for missing -arch’s in a universal file.
2) If there is no -arch flags look for the host architecture in a universal file and if found process just as if was a thin file. That is no “(for architecture XYZ)” is printed.
3) Lastly process all architectures, if more than one architecture is in the universal file then print "(for architecture XYZ)” with each.
The goal is to present the output as if would be natural if not in a universal file. That is native by default and you can’t tell it came from a universal file. Or if you add just one -arch just like it was native on that arch. And only add the "(for architecture XYZ)” when it is needed to understand the output when listing multiple
architecture in the output.
Not sure how to get this logic by walking the -arch command line list just once. But open to suggestions. My goal is to march the logic in the darwin tools and get character for character matching output with the existing darwin tools. So that llvm-nm and llvm-size can some day replace them and scripts that I don’t even know about keep working.
Kev
>
>> On Jul 1, 2014, at 10:19 AM, Kevin Enderby <enderby at apple.com> wrote:
>>
>> Author: enderby
>> Date: Tue Jul 1 12:19:10 2014
>> New Revision: 212108
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=212108&view=rev
>> Log:
>> Add the -arch flag support to llvm-size like what was done to llvm-nm
>> to select the slice out of a Mach-O universal file. This also includes
>> support for -arch all, selecting the host architecture by default from
>> a universal file and checking if -arch is used with a standard Mach-O
>> it matches that architecture.
>>
>> Modified:
>> llvm/trunk/test/Object/size-trivial-macho.test
>> llvm/trunk/tools/llvm-size/llvm-size.cpp
>>
>> Modified: llvm/trunk/test/Object/size-trivial-macho.test
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Object/size-trivial-macho.test?rev=212108&r1=212107&r2=212108&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/Object/size-trivial-macho.test (original)
>> +++ llvm/trunk/test/Object/size-trivial-macho.test Tue Jul 1 12:19:10 2014
>> @@ -10,10 +10,14 @@ RUN: llvm-size -format darwin %p/Inputs/
>> RUN: | FileCheck %s -check-prefix mAR
>> RUN: llvm-size -m -x -l %p/Inputs/hello-world.macho-x86_64 \
>> RUN: | FileCheck %s -check-prefix mxl
>> -RUN: llvm-size %p/Inputs/macho-universal.x86_64.i386 \
>> +RUN: llvm-size -arch all %p/Inputs/macho-universal.x86_64.i386 \
>> RUN: | FileCheck %s -check-prefix u
>> -RUN: llvm-size %p/Inputs/macho-universal-archive.x86_64.i386 \
>> +RUN: llvm-size -arch i386 %p/Inputs/macho-universal.x86_64.i386 \
>> +RUN: | FileCheck %s -check-prefix u-i386
>> +RUN: llvm-size -arch all %p/Inputs/macho-universal-archive.x86_64.i386 \
>> RUN: | FileCheck %s -check-prefix uAR
>> +RUN: llvm-size -arch x86_64 %p/Inputs/macho-universal-archive.x86_64.i386 \
>> +RUN: | FileCheck %s -check-prefix uAR-x86_64
>>
>> A: section size addr
>> A: __text 12 0
>> @@ -74,6 +78,12 @@ u: __TEXT __DATA __OBJC others dec hex
>> u: 4096 0 0 4294971392 4294975488 100002000 {{.*}}/macho-universal.x86_64.i386 (for architecture x86_64)
>> u: 4096 0 0 8192 12288 3000 {{.*}}/macho-universal.x86_64.i386 (for architecture i386)
>>
>> +u-i386: __TEXT __DATA __OBJC others dec hex
>> +u-i386: 4096 0 0 8192 12288 3000
>> +
>> uAR: __TEXT __DATA __OBJC others dec hex
>> uAR: 136 0 0 32 168 a8 {{.*}}/macho-universal-archive.x86_64.i386(hello.o) (for architecture x86_64)
>> uAR: 5 4 0 0 9 9 {{.*}}/macho-universal-archive.x86_64.i386(foo.o) (for architecture i386)
>> +
>> +uAR-x86_64: __TEXT __DATA __OBJC others dec hex
>> +uAR-x86_64: 136 0 0 32 168 a8 {{.*}}/macho-universal-archive.x86_64.i386(hello.o)
>>
>> Modified: llvm/trunk/tools/llvm-size/llvm-size.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-size/llvm-size.cpp?rev=212108&r1=212107&r2=212108&view=diff
>> ==============================================================================
>> --- llvm/trunk/tools/llvm-size/llvm-size.cpp (original)
>> +++ llvm/trunk/tools/llvm-size/llvm-size.cpp Tue Jul 1 12:19:10 2014
>> @@ -57,6 +57,12 @@ cl::opt<bool> DarwinLongFormat("l",
>> cl::desc("When format is darwin, use long format "
>> "to include addresses and offsets."));
>>
>> +static cl::list<std::string>
>> + ArchFlags("arch",
>> + cl::desc("architecture(s) from a Mach-O file to dump"),
>> + cl::ZeroOrMore);
>> +bool ArchAll = false;
>
> static bool ArchAll.
>
> Later, it’d be really great to get rid of the globals this file uses. Not for this patch, though.
>
>> +
>> enum RadixTy {octal = 8, decimal = 10, hexadecimal = 16};
>> static cl::opt<unsigned int>
>> Radix("-radix",
>> @@ -413,6 +419,40 @@ static void PrintObjectSectionSizes(Obje
>> }
>> }
>>
>> +/// @brief Checks to see if the @p o ObjectFile is a Mach-O file and if it is
>> +/// and there is a list of architecture flags specified then check to
>> +/// make sure this Mach-O file is one of those architectures or all
>> +/// architectures was specificed. If not then an error is generated and
>> +/// this routine returns false. Else it returns true.
>> +static bool checkMachOAndArchFlags(ObjectFile *o, StringRef file) {
>
> Variable names start w/ caps. “O” and “File”. Same thing in other places.
>
>> + if (isa<MachOObjectFile>(o) && !ArchAll && ArchFlags.size() != 0) {
>> + MachOObjectFile *MachO = dyn_cast<MachOObjectFile>(o);
>> + bool ArchFound = false;
>> + MachO::mach_header H;
>> + MachO::mach_header_64 H_64;
>> + Triple T;
>> + if (MachO->is64Bit()) {
>> + H_64 = MachO->MachOObjectFile::getHeader64();
>> + T = MachOObjectFile::getArch(H_64.cputype, H_64.cpusubtype);
>> + } else {
>> + H = MachO->MachOObjectFile::getHeader();
>> + T = MachOObjectFile::getArch(H.cputype, H.cpusubtype);
>> + }
>> + unsigned i;
>> + for (i = 0; i < ArchFlags.size(); ++i){
>> + if (ArchFlags[i] == T.getArchName())
>> + ArchFound = true;
>> + break;
>> + }
>> + if (!ArchFound) {
>> + errs() << ToolName << ": file: " << file
>> + << " does not contain architecture: " << ArchFlags[i] << ".\n”;
>
> size(1) issues this diagnostic for each listed -arch that isn’t in the fat file and gives normal output for each that is. For example,
>
> enkidu: ~/tmp $ size -arch armv7 -arch armv7s t.o
> __TEXT __DATA __OBJC others dec hex
> error: /Applications/Xcode.app/Contents/Developer/Toolchains/OSX10.10.xctoolchain/usr/bin/size: file: t.o does not contain architecture: armv7
> 8 0 0 0 8 8
> enkidu: ~/tmp $
>
> llvm-size, on the other hand, silently ignores the not-present -arch command option(s) if it finds any of them in the file.
>
>
>> + return false;
>> + }
>> + }
>> + return true;
>> +}
>> +
>> /// @brief Print the section sizes for @p file. If @p file is an archive, print
>> /// the section sizes for each archive member.
>> static void PrintFileSectionSizes(StringRef file) {
>> @@ -444,6 +484,8 @@ static void PrintFileSectionSizes(String
>> }
>> if (ObjectFile *o = dyn_cast<ObjectFile>(&*ChildOrErr.get())) {
>> MachOObjectFile *MachO = dyn_cast<MachOObjectFile>(o);
>> + if (!checkMachOAndArchFlags(o, file))
>> + return;
>> if (OutputFormat == sysv)
>> outs() << o->getFileName() << " (ex " << a->getFileName()
>> << "):\n";
>> @@ -460,8 +502,155 @@ static void PrintFileSectionSizes(String
>> }
>> } else if (MachOUniversalBinary *UB =
>> dyn_cast<MachOUniversalBinary>(binary.get())) {
>> - // This is a Mach-O universal binary. Iterate over each object and
>> - // display its sizes.
>> + // If we have a list of architecture flags specified dump only those.
>> + if (!ArchAll && ArchFlags.size() != 0) {
>> + // Look for a slice in the universal binary that matches each ArchFlag.
>> + bool ArchFound;
>> + for (unsigned i = 0; i < ArchFlags.size(); ++i){
>> + ArchFound = false;
>> + for (MachOUniversalBinary::object_iterator I = UB->begin_objects(),
>> + E = UB->end_objects();
>> + I != E; ++I) {
>
> These loops can very likely use C++11 style.
>
> "for (const auto Object : UB)"
>
>> + if (ArchFlags[i] == I->getArchTypeName()){
>> + ArchFound = true;
>> + ErrorOr<std::unique_ptr<ObjectFile>> UO = I->getAsObjectFile();
>> + std::unique_ptr<Archive> UA;
>> + if (UO) {
>> + if (ObjectFile *o = dyn_cast<ObjectFile>(&*UO.get())) {
>> + MachOObjectFile *MachO = dyn_cast<MachOObjectFile>(o);
>> + if (OutputFormat == sysv)
>> + outs() << o->getFileName() << " :\n";
>> + else if(MachO && OutputFormat == darwin) {
>> + if (moreThanOneFile || ArchFlags.size() > 1)
>> + outs() << o->getFileName() << " (for architecture "
>> + << I->getArchTypeName() << "): \n";
>> + }
>> + PrintObjectSectionSizes(o);
>> + if (OutputFormat == berkeley) {
>> + if (!MachO || moreThanOneFile || ArchFlags.size() > 1)
>> + outs() << o->getFileName() << " (for architecture "
>> + << I->getArchTypeName() << ")";
>> + outs() << "\n";
>> + }
>> + }
>> + }
>> + else if (!I->getAsArchive(UA)) {
>> + // This is an archive. Iterate over each member and display its
>> + //sizes.
>> + for (object::Archive::child_iterator i = UA->child_begin(),
>> + e = UA->child_end();
>> + i != e; ++i) {
>> + ErrorOr<std::unique_ptr<Binary>> ChildOrErr = i->getAsBinary();
>> + if (std::error_code EC = ChildOrErr.getError()) {
>> + errs() << ToolName << ": " << file << ": " << EC.message()
>> + << ".\n";
>> + continue;
>> + }
>> + if (ObjectFile *o = dyn_cast<ObjectFile>(&*ChildOrErr.get())) {
>> + MachOObjectFile *MachO = dyn_cast<MachOObjectFile>(o);
>> + if (OutputFormat == sysv)
>> + outs() << o->getFileName() << " (ex " << UA->getFileName()
>> + << "):\n";
>> + else if(MachO && OutputFormat == darwin)
>> + outs() << UA->getFileName() << "(" << o->getFileName()
>> + << ")" << " (for architecture "
>> + << I->getArchTypeName() << "):\n";
>> + PrintObjectSectionSizes(o);
>> + if (OutputFormat == berkeley) {
>> + if (MachO) {
>> + outs() << UA->getFileName() << "(" << o->getFileName()
>> + << ")";
>> + if (ArchFlags.size() > 1)
>> + outs() << " (for architecture "
>> + << I->getArchTypeName() << ")";
>> + outs() << "\n";
>> + }
>> + else
>> + outs() << o->getFileName() << " (ex " << UA->getFileName()
>> + << ")\n";
>> + }
>> + }
>> + }
>> + }
>> + }
>> + }
>> + if (!ArchFound) {
>> + errs() << ToolName << ": file: " << file
>> + << " does not contain architecture" << ArchFlags[i] << ".\n";
>> + return;
>> + }
>> + }
>> + return;
>> + }
>> + // No architecture flags were specified so if this contains a slice that
>> + // matches the host architecture dump only that.
>> + if (!ArchAll) {
>> + StringRef HostArchName =
>> + MachOObjectFile::getHostArch().getArchName();
>> + for (MachOUniversalBinary::object_iterator I = UB->begin_objects(),
>> + E = UB->end_objects();
>> + I != E; ++I) {
>> + if (HostArchName == I->getArchTypeName()){
>> + ErrorOr<std::unique_ptr<ObjectFile>> UO = I->getAsObjectFile();
>> + std::unique_ptr<Archive> UA;
>> + if (UO) {
>> + if (ObjectFile *o = dyn_cast<ObjectFile>(&*UO.get())) {
>> + MachOObjectFile *MachO = dyn_cast<MachOObjectFile>(o);
>> + if (OutputFormat == sysv)
>> + outs() << o->getFileName() << " :\n";
>> + else if(MachO && OutputFormat == darwin) {
>> + if (moreThanOneFile)
>> + outs() << o->getFileName() << " (for architecture "
>> + << I->getArchTypeName() << "):\n";
>> + }
>> + PrintObjectSectionSizes(o);
>> + if (OutputFormat == berkeley) {
>> + if (!MachO || moreThanOneFile)
>> + outs() << o->getFileName() << " (for architecture "
>> + << I->getArchTypeName() << ")";
>> + outs() << "\n";
>> + }
>> + }
>> + }
>> + else if (!I->getAsArchive(UA)) {
>> + // This is an archive. Iterate over each member and display its
>> + // sizes.
>> + for (object::Archive::child_iterator i = UA->child_begin(),
>> + e = UA->child_end();
>> + i != e; ++i) {
>> + ErrorOr<std::unique_ptr<Binary>> ChildOrErr = i->getAsBinary();
>> + if (std::error_code EC = ChildOrErr.getError()) {
>> + errs() << ToolName << ": " << file << ": " << EC.message()
>> + << ".\n";
>> + continue;
>> + }
>> + if (ObjectFile *o = dyn_cast<ObjectFile>(&*ChildOrErr.get())) {
>> + MachOObjectFile *MachO = dyn_cast<MachOObjectFile>(o);
>> + if (OutputFormat == sysv)
>> + outs() << o->getFileName() << " (ex " << UA->getFileName()
>> + << "):\n";
>> + else if(MachO && OutputFormat == darwin)
>> + outs() << UA->getFileName() << "(" << o->getFileName() << ")"
>> + << " (for architecture " << I->getArchTypeName()
>> + << "):\n";
>> + PrintObjectSectionSizes(o);
>> + if (OutputFormat == berkeley) {
>> + if (MachO)
>> + outs() << UA->getFileName() << "(" << o->getFileName()
>> + << ")\n";
>> + else
>> + outs() << o->getFileName() << " (ex " << UA->getFileName()
>> + << ")\n";
>> + }
>> + }
>> + }
>> + }
>> + return;
>> + }
>> + }
>> + }
>> + // Either all architectures have been specified or none have been specified
>> + // and this does not contain the host architecture so dump all the slices.
>> bool moreThanOneArch = UB->getNumberOfObjects() > 1;
>> for (MachOUniversalBinary::object_iterator I = UB->begin_objects(),
>> E = UB->end_objects();
>> @@ -521,6 +710,8 @@ static void PrintFileSectionSizes(String
>> }
>> }
>> } else if (ObjectFile *o = dyn_cast<ObjectFile>(binary.get())) {
>> + if (!checkMachOAndArchFlags(o, file))
>> + return;
>> if (OutputFormat == sysv)
>> outs() << o->getFileName() << " :\n";
>> PrintObjectSectionSizes(o);
>> @@ -552,6 +743,20 @@ int main(int argc, char **argv) {
>> if (RadixShort.getNumOccurrences())
>> Radix = RadixShort;
>>
>> + for (unsigned i = 0; i < ArchFlags.size(); ++i){
>> + if (ArchFlags[i] == "all") {
>> + ArchAll = true;
>> + }
>> + else {
>> + Triple T = MachOObjectFile::getArch(ArchFlags[i]);
>> + if (T.getArch() == Triple::UnknownArch){
>> + outs() << ToolName << ": for the -arch option: Unknown architecture "
>> + << "named '" << ArchFlags[i] << "’";
>
> Need a ‘\n’ at the end of the diagnostic.
>
> Phrasing is a bit odd. Maybe just use what size(1) does (w/o the list of valid arch, as that’s pretty long these days). "unknown architecture specification flag: -arch <name>”
>
> Something is odd, here, though. I’m getting this diagnostic for “-arch armv7”.
>
>
>> + return 1;
>> + }
>> + }
>> + }
>> +
>> if (InputFilenames.size() == 0)
>> InputFilenames.push_back("a.out");
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
More information about the llvm-commits
mailing list