[PATCH] D46249: [LLD][ELF][AArch64] Add missing LO12 relocation to usesOnlyLowPageBits

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 3 06:48:06 PDT 2018


grimar added a comment.

In https://reviews.llvm.org/D46249#1086334, @peter.smith wrote:

> In https://reviews.llvm.org/D46249#1086324, @grimar wrote:
>
> > In https://reviews.llvm.org/D46249#1086321, @peter.smith wrote:
> >
> > > Perhaps it is worth a comment instead as the function name implies a complete list. For example "// not a complete list, excludes relocations that can always be resolved statically such as TLS local exec."
> >
> >
> > Yeah, maybe. I guess it already contains entries it never uses, btw?
>
>
> I think the ones that are already there can be used as they are one of R_GOT, R_PLT, R_TLSDESC, or (!AbsVal && !RelE) can be true. The local exec relocations are always AbsVal = true and all have a RelExpr of R_TLS.
>
> I'm not hugely fussed either way, I was just unsure of whether to add new LO12_NC relocations to that function or not.


Ok. So, personally my opinion - I would not add any code that is unproven to be useful. This applies to this patch.


https://reviews.llvm.org/D46249





More information about the llvm-commits mailing list