[PATCH] D71978: [RISCV] Fix evalutePCRelLo for symbols at the end of a fragment
James Clarke via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 7 16:37:16 PST 2020
jrtc27 added a comment.
In D71978#1809128 <https://reviews.llvm.org/D71978#1809128>, @efriedma wrote:
> > it requires . and .Ltmp0 be in the same fragment [...] This is checked earlier by ensuring findAssociatedFragment() matches for each.
>
> That check you're mentioning doesn't actually do what you say it does; both findAssociatedFragment() actually return the fragment containing .Ltmp0.
Hm, you're right, I'm not sure what that's actually checking then, it seems like findAssociatedFragment by definition will end up using AUIPCSRE->findAssociatedFragment()?
>> This logic therefore needs to be mirrored in evaluatePCRelLo so that they agree on what fragment they're talking about for the AUIPC fixup.
>
> This makes sense. Please add comments linking the two.
Actually, thinking about this more, since `getPCRelHiFixup` returns a non-null fixup if and only if its offset matches the (potentially zeroed because it's at the end of the fragment) AUIPCSymbol's offset, we should just use TargetFixup->getOffset and not have to worry about duplicating the condition? (But still with a comment in evaluatePCRelLo explaining why we have to use that and not the symbol's offset despite them looking very similar)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71978/new/
https://reviews.llvm.org/D71978
More information about the llvm-commits
mailing list