This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [gold] Merging string literals with bigger alignment


Hi Cary,

There was no feedback from Ian for this patch: I rebased it and did
the changes that you mentioned.
As a "fully fledged"(c) gold maintainer (congratulations btw :) )
could you please take a look again?

All tests pass on x86-64-gnu-linux.
If ok, could someone commit please? I don't have commit access.

thank you,
Alexander



2013/4/4 Cary Coutant <ccoutant@google.com>:
>> Hi, I implemented (b2) as we discussed above. Could you please take a look.
>
> +  // We assume here that the beginning of the section is correctly
> +  // aligned, so each string within the section must retain the same
> +  // modulo.
> +  uint64_t init_align_modulo = (uint64_t) pdata % this->addralign();
>
> The type should be uintptr_t, and the cast should be
> reinterpret_cast<uintptr_t>(pdata):
>
>   uintptr_t init_align_modulo = (reinterpret_cast<uintptr_t>(pdata)
>                                  & (this->addralign() - 1));
>
> +         // Within merge input section each string must be aligned.
> +         if ((uint64_t) p % this->addralign() != init_align_modulo)
> +           has_misaligned_strings = true;
>
> With a variable operand, the % operator is quite a bit slower than
> masking. Please rewrite this as
>
>           if ((reinterpret_cast<uintptr_t>(p) & (this->addralign() - 1))
>               != init_align_modulo)
>             has_misaligned_strings = true;
>
> Aside from those changes, this looks good to me. Thanks!
>
> (You still need Ian's approval to commit.)
>
> -cary

Attachment: merging_string_literals_05.patch
Description: Binary data


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]