[PATCH] [ELF][X86_64] Handle R_X86_64_PC64 relocation

Rui Ueyama ruiu at google.com
Mon Feb 23 10:35:24 PST 2015


LGTM

On Sun, Feb 22, 2015 at 9:16 PM, Davide Italiano <davide at freebsd.org> wrote:

> Commments inline.
>
>
> REPOSITORY
>   rL LLVM
>
> ================
> Comment at: lib/ReaderWriter/ELF/X86_64/X86_64RelocationHandler.cpp:62
> @@ +61,3 @@
> +  int64_t result = (uint64_t)((S + A) - P);
> +  *reinterpret_cast<llvm::support::ulittle64_t *>(location) =
> +      result |
> ----------------
> ruiu wrote:
> > Can you add using llvm::support::ulittle64_t at beginning of the file
> and use that here?
> This follows the style of the rest of the file, wouldn't be better commit
> the patch as is and change the style for the whole file in a subsequent
> pass?
>

Maybe. If we do that these three lines can fit in one line.


>
> ================
> Comment at: lib/ReaderWriter/ELF/X86_64/X86_64RelocationHandler.cpp:64
> @@ +63,3 @@
> +      result |
> +      (uint64_t) * reinterpret_cast<llvm::support::ulittle64_t
> *>(location);
> +}
> ----------------
> ruiu wrote:
> > Not sure if clang-format formatted like this, but I think there should
> be no space around "*".
> Ditto. No, it did not, I tried to be consistent to what was already there.
>

OK. But this doesn't follow the style guide, I believe. It may be better to
fix them in a different patch.


> http://reviews.llvm.org/D7820
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150223/893f8456/attachment.html>


More information about the llvm-commits mailing list