[PATCH] D57240: [RISCV] Don't incorrectly force relocation for %pcrel_lo

[James Clarke via Phabricator via llvm-commits]

jrtc27 added a comment.

In D57240#1385323 <https://reviews.llvm.org/D57240#1385323>, @lewis-revill wrote:

> In D57240#1385261 <https://reviews.llvm.org/D57240#1385261>, @jrtc27 wrote:
>
> > Could we not have a situation where the pcrel_hi20 is seen *after* the lo12, in which case the lo12 would be forced but the hi20 could still be evaluated? E.g. I think the following would be problematic:
> >
> >   foo:
> >       j 1f
> >   0:  addi a0, a0, %pcrel_lo(1f)
> >       ret
> >   1: auipc a0, %pcrel_hi(bar)
> >       j 0b
> >  
> >   .align 2
> >   bar:
> >       # ...
> >
> >
> > Obviously extremely pathological code, but nonetheless valid.
>
>
> Good call. It's a pathological case but maybe there are other cases as well. It's not a nice situation. Somehow we have to ensure there can never be a pcrel_lo12 relocation without a corresponding pcrel_hi20 relocation. It might help to know whether the pcrel_hi20 has been seen at all, regardless of whether it was resolved.
>
> So I guess the intended behaviour might be:
>
> pcrel_hi20 seen and not resolved (IE relocation emitted) -> force relocation
>  pcrel_hi20 seen and resolved -> don't force relocation
>  pcrel_hi20 not seen -> don't force relocation? If evaluatePcRelLo thinks it can be evaluated does it matter if the pcrel_hi20 can or not?
>
> I'll have another think and take a look at your testcase.


My suspicion is that it would be easier to teach `MCAssembler::evaluateFixup` how to deal with an extra level of indirection, since the problem stems from it believing the relocation can be evaluated and not knowing about the *real* symbol/relocation we care about.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57240/new/

https://reviews.llvm.org/D57240