[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