[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