[PATCH] D16820: Implement common symbol size calculation in llvm-size

[Rafael Espíndola via llvm-commits]

On 1 March 2016 at 17:38, khemant at codeaurora.org <khemant at codeaurora.org> wrote:
> khemant added inline comments.
>
> ================
> Comment at: test/tools/llvm-size/X86/test-common.s:17
> @@ +16,3 @@
> +       movl    $1, x
> +       movl    $1, y
> +       movl    $1, z
> ----------------
> rafael wrote:
>> Do you actually need all this? To test just the common size I don't see you need any asm instruction.
> I needed some section with non zero size for case where no common is specified.

So just a nop will do, right?

> Does that matter?

Yes. Please try to keep test as small as possible to make it obvious
what is being tested.

>
> ================
> Comment at: tools/llvm-size/llvm-size.cpp:370
> @@ +369,3 @@
> +    if (ELFCommons) {
> +      uint64_t common_size = getCommonSize(Obj);
> +      total += common_size;
> ----------------
> rafael wrote:
>> CommonSize
> There are variables with all lower case. Do you want me to converst all locals to upper before merging this patch with upper case variables pertaining to this change?

Not in the same patch, but a previous patch that just fixes the
existing style would be awesome.

Cheers,
Rafael