[llvm] r259578 - Correct size calculations for ELF files
Rafael EspĂndola via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 2 14:30:07 PST 2016
This should probably have been reviewed before commit
> Added:
> llvm/trunk/test/tools/llvm-size/Inputs/
> llvm/trunk/test/tools/llvm-size/Inputs/1.o
Why do you need to check in a object file? For size you should be able
to use a .s and llvm-mc. It should also have a better name.
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-size/basic.test?rev=259578&r1=259577&r2=259578&view=diff
> ==============================================================================
> --- llvm/trunk/test/tools/llvm-size/basic.test (original)
> +++ llvm/trunk/test/tools/llvm-size/basic.test Tue Feb 2 15:41:49 2016
> @@ -1,2 +1,28 @@
> RUN: llvm-size %t.blah 2>&1 | FileCheck --check-prefix=ENOENT %s
This is a completely different test, please use a new file.
> +/// @brief Remove unneeded ELF sections from calculation
We don't use @brief. It can be implicit.
> +static bool ConsiderForSize(ObjectFile *Obj, SectionRef Section) {
Function names should start with a lower case.
> + if (Obj->isELF()) {
> + switch (static_cast<ELFSectionRef>(Section).getType()) {
> + case ELF::SHT_NULL:
> + case ELF::SHT_SYMTAB:
> + case ELF::SHT_STRTAB:
> + case ELF::SHT_REL:
> + case ELF::SHT_RELA:
> + return false;
> + }
> + }
> + return true;
It seems better to just move the isELF check out of the function and
pass just the section to it.
Given the number of issues for such a small file I would recommend
reverting it recommitting the fixed version.
Cheers,
Rafael
More information about the llvm-commits
mailing list