[all-commits] [llvm/llvm-project] 8634a8: [RISCV] Fix evaluating %pcrel_lo against global an...

[James Clarke via All-commits]

  Branch: refs/heads/release/10.x
  Home:   https://github.com/llvm/llvm-project
  Commit: 8634a82910eba78279a69fcba0925d3a602a0563
      https://github.com/llvm/llvm-project/commit/8634a82910eba78279a69fcba0925d3a602a0563
  Author: James Clarke <jrtc27 at jrtc27.com>
  Date:   2020-01-23 (Thu, 23 Jan 2020)

  Changed paths:
    M llvm/include/llvm/MC/MCAsmBackend.h
    M llvm/include/llvm/MC/MCFixupKindInfo.h
    M llvm/lib/MC/MCAssembler.cpp
    M llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
    M llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
    M llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp
    M llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.h
    M llvm/test/MC/RISCV/pcrel-fixups.s
    M llvm/test/MC/RISCV/pcrel-lo12-invalid.s
    M llvm/test/MC/RISCV/rv32i-aliases-valid.s
    M llvm/test/MC/RISCV/rv32i-valid.s
    M llvm/test/MC/RISCV/rv64i-aliases-valid.s

  Log Message:
  -----------
  [RISCV] Fix evaluating %pcrel_lo against global and weak symbols

Summary:
Previously, we would erroneously turn %pcrel_lo(label), where label has
a %pcrel_hi against a weak symbol, into %pcrel_lo(label + offset), as
evaluatePCRelLo would believe the target independent logic was going to
fold it. Moreover, even if that were fixed, shouldForceRelocation lacks
an MCAsmLayout and thus cannot evaluate the %pcrel_hi fixup to a value
and check the symbol, so we would then erroneously constant-fold the
%pcrel_lo whilst leaving the %pcrel_hi intact. After D72197, this same
sequence also occurs for symbols with global binding, which is triggered
in real-world code.

Instead, as discussed in D71978, we introduce a new FKF_IsTarget flag to
avoid these kinds of issues. All the resolution logic happens in one
place, with no coordination required between RISCAsmBackend and
RISCVMCExpr to ensure they implement the same logic twice. Although the
implementation of %pcrel_hi can be left as target independent, we make
it target dependent to ensure that they are handled identically to
%pcrel_lo, otherwise we risk one of them being constant folded but the
other being preserved. This also allows us to properly support fixup
pairs where the instructions are in different fragments.

Reviewers: asb, lenary, efriedma

Reviewed By: efriedma

Subscribers: arichardson, hiraditya, rbar, johnrusso, simoncook, sabuasal, niosHD, kito-cheng, shiva0217, MaskRay, zzheng, edward-jones, rogfer01, MartinMosbeck, brucehoult, the_o, rkruppe, PkmX, jocewei, psnobl, benna, Jim, s.egerton, pzheng, sameer.abuasal, apazos, luismarques, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D73211

(cherry picked from commit 3f5976c97dbfefb4669abcf968bd79a9a64c18e0)


  Commit: a3982a59ce34039f63fff35c6c0562cf6fd5c771
      https://github.com/llvm/llvm-project/commit/a3982a59ce34039f63fff35c6c0562cf6fd5c771
  Author: James Clarke <jrtc27 at jrtc27.com>
  Date:   2020-01-23 (Thu, 23 Jan 2020)

  Changed paths:
    M lld/test/ELF/riscv-pcrel-hilo-error.s

  Log Message:
  -----------
  [test] Fix lld/test/ELF/riscv-pcrel-hilo-error.s after D73211

(cherry picked from commit ddfe8751b16a1d57b0586fb48d1109c98234bc3f)


Compare: https://github.com/llvm/llvm-project/compare/5d37ce7e19c9...a3982a59ce34