[RISCV] Fix evaluating %pcrel_lo against global and weak symbols

[jrtc27]

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.

[efriedma]

This approach seems much easier to understand.

Can you add a testcase for the case where the auipc and addi are in different fragments, like I mentioned in D71978?

llvm/include/llvm/MC/MCAsmBackend.h

114

Can the "Target" be a const reference?

llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp

279

MCAssembler::evaluateFixup emits an error if evaluateAsRelocatable fails... maybe worth adding a comment if you're intentionally not emitting a similar error?

283

If "Target.getSymB()" is true, should getPCRelHiFixup fail?

323

shouldForceRelocation(Asm, AUIPCFixup, AUIPCTarget)? (I think the difference is significant; please make sure we have a testcase that covers it.)

llvm/test/MC/RISCV/pcrel-fixups.s

86

Did you mean to make both global_function and weak_function weak?

[jrtc27]

In D73211#1834739, @efriedma wrote:

This approach seems much easier to understand.

Can you add a testcase for the case where the auipc and addi are in different fragments, like I mentioned in D71978?

Thanks for the quick review. I shall add that and address the comments.

llvm/include/llvm/MC/MCAsmBackend.h

114

Yes, it probably should be. In theory a backend could leave the fixup unresolved but mutate the target, however that seems rare (and is precisely what we're trying to avoid with RISC-V...). If a backend grows to need it it can always be de-consted later.

llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp

279

Yes, it's deliberately omitted since evaluateFixup will emit the same error when it sees the real pcrel_hi. I will add a comment.

283

That was intended to be AUIPCTarget, but that is already handled below in general regardless of the variant kind so I will just remove this.

323

It currently doesn't matter, the intent was only to pick up the FeatureRelax etc code path. But you're right, if I pass AUIPCFixup, things are better, and I can delete the WasForced case statements above (validating it's a valid fixup should already have happened in getPCRelHiFixup).

llvm/test/MC/RISCV/pcrel-fixups.s

86

We both know the answer is no :) will fix.

[jrtc27]

Addressed review comments.

[jrtc27]

Pass correct target to shouldForceRelocation (although unused).

[jrtc27]

llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp

323

After removing the big switch earlier in the function, this is caught by at least one test in relocations.s which uses the GOT (and I assume also the TLS-related relocations later in the file), and it is now using the correct (AUIPC) fixup. It also uses the correct target, although that is not currently consulted so it does not matter.

[efriedma]

LGTM

[arichardson]

@jrtc27 This should probably also go in the release branch or was the commit that exposed the problem made after the branch point?

[asb]

I've filed a request to merge this into the release branch here https://bugs.llvm.org/show_bug.cgi?id=44631 - thanks for the fix James!

llvm/include/llvm/MC/MCAsmBackend.h

public:
   /// Hook which indicates if the target requires a fixup to be generated when
   /// handling an align directive in an executable section
   virtual bool shouldInsertFixupForCodeAlign(MCAssembler &Asm,
                                              const MCAsmLayout &Layout,
                                              MCAlignFragment &AF) {
     return false;
   }
 
+  virtual bool evaluateTargetFixup(const MCAssembler &Asm,
+                                   const MCAsmLayout &Layout,
+                                   const MCFixup &Fixup, const MCFragment *DF,
+                                   MCValue &Target, uint64_t &Value,
+                                   bool &WasForced) {
+    llvm_unreachable("Need to implement hook if target has custom fixups");
+  }
+
   /// Apply the \p Value for given \p Fixup into the provided data fragment, at
   /// the offset specified by the fixup and following the fixup kind as
   /// appropriate. Errors (such as an out of range fixup value) should be
   /// reported via \p Ctx.
   /// The  \p STI is present only for fragments of type MCRelaxableFragment and
   /// MCDataFragment with hasInstructions() == true.
   virtual void applyFixup(const MCAssembler &Asm, const MCFixup &Fixup,
                           const MCValue &Target, MutableArrayRef<char> Data,

llvm/include/llvm/MC/MCFixupKindInfo.h

 /// Target independent information on a fixup kind.
 struct MCFixupKindInfo {
   enum FixupKindFlags {
     /// Is this fixup kind PCrelative? This is used by the assembler backend to
     /// evaluate fixup values in a target independent manner when possible.
     FKF_IsPCRel = (1 << 0),
 
     /// Should this fixup kind force a 4-byte aligned effective PC value?
-    FKF_IsAlignedDownTo32Bits = (1 << 1)
+    FKF_IsAlignedDownTo32Bits = (1 << 1),
+
+    /// Should this fixup be evaluated in a target dependent manner?
+    FKF_IsTarget = (1 << 2)
   };
 
   /// A target specific name for the fixup kind. The names will be unique for
   /// distinct kinds on any given target.
   const char *Name;
 
   /// The bit offset to write the relocation into.
   unsigned TargetOffset;

llvm/lib/MC/MCAssembler.cpp

if (const MCSymbolRefExpr *RefB = Target.getSymB()) {
     if (RefB->getKind() != MCSymbolRefExpr::VK_None) {
       Ctx.reportError(Fixup.getLoc(),
                       "unsupported subtraction of qualified symbol");
       return true;
     }
   }
 
   assert(getBackendPtr() && "Expected assembler backend");
+  bool IsTarget = getBackendPtr()->getFixupKindInfo(Fixup.getKind()).Flags &
+                  MCFixupKindInfo::FKF_IsTarget;
+
+  if (IsTarget)
+    return getBackend().evaluateTargetFixup(*this, Layout, Fixup, DF, Target,
+                                            Value, WasForced);
+
   bool IsPCRel = getBackendPtr()->getFixupKindInfo(Fixup.getKind()).Flags &
                  MCFixupKindInfo::FKF_IsPCRel;
 
   bool IsResolved = false;
   if (IsPCRel) {
     if (Target.getSymB()) {
       IsResolved = false;
     } else if (!Target.getSymA()) {

llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h

public:
   bool shouldInsertExtraNopBytesForCodeAlign(const MCAlignFragment &AF,
                                              unsigned &Size) override;
 
   // Insert target specific fixup type for alignment directive in code section.
   bool shouldInsertFixupForCodeAlign(MCAssembler &Asm,
                                      const MCAsmLayout &Layout,
                                      MCAlignFragment &AF) override;
 
+  bool evaluateTargetFixup(const MCAssembler &Asm, const MCAsmLayout &Layout,
+                           const MCFixup &Fixup, const MCFragment *DF,
+                           MCValue &Target, uint64_t &Value,
+                           bool &WasForced) override;
+
   void applyFixup(const MCAssembler &Asm, const MCFixup &Fixup,
                   const MCValue &Target, MutableArrayRef<char> Data,
                   uint64_t Value, bool IsResolved,
                   const MCSubtargetInfo *STI) const override;
 
   std::unique_ptr<MCObjectTargetWriter>
   createObjectTargetWriter() const override;
 
const MCFixupKindInfo &getFixupKindInfo(MCFixupKind Kind) const override {
     const static MCFixupKindInfo Infos[] = {
       // This table *must* be in the order that the fixup_* kinds are defined in
       // RISCVFixupKinds.h.
       //
       // name                      offset bits  flags
       { "fixup_riscv_hi20",         12,     20,  0 },
       { "fixup_riscv_lo12_i",       20,     12,  0 },
       { "fixup_riscv_lo12_s",        0,     32,  0 },
-      { "fixup_riscv_pcrel_hi20",   12,     20,  MCFixupKindInfo::FKF_IsPCRel },
-      { "fixup_riscv_pcrel_lo12_i", 20,     12,  MCFixupKindInfo::FKF_IsPCRel },
-      { "fixup_riscv_pcrel_lo12_s",  0,     32,  MCFixupKindInfo::FKF_IsPCRel },
+      { "fixup_riscv_pcrel_hi20",   12,     20,
+        MCFixupKindInfo::FKF_IsPCRel | MCFixupKindInfo::FKF_IsTarget },
+      { "fixup_riscv_pcrel_lo12_i", 20,     12,
+        MCFixupKindInfo::FKF_IsPCRel | MCFixupKindInfo::FKF_IsTarget },
+      { "fixup_riscv_pcrel_lo12_s",  0,     32,
+        MCFixupKindInfo::FKF_IsPCRel | MCFixupKindInfo::FKF_IsTarget },
       { "fixup_riscv_got_hi20",     12,     20,  MCFixupKindInfo::FKF_IsPCRel },
       { "fixup_riscv_tprel_hi20",   12,     20,  0 },
       { "fixup_riscv_tprel_lo12_i", 20,     12,  0 },
       { "fixup_riscv_tprel_lo12_s",  0,     32,  0 },
       { "fixup_riscv_tprel_add",     0,      0,  0 },
       { "fixup_riscv_tls_got_hi20", 12,     20,  MCFixupKindInfo::FKF_IsPCRel },
       { "fixup_riscv_tls_gd_hi20",  12,     20,  MCFixupKindInfo::FKF_IsPCRel },
       { "fixup_riscv_jal",          12,     20,  MCFixupKindInfo::FKF_IsPCRel },

llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp

 //===-- RISCVAsmBackend.cpp - RISCV Assembler Backend ---------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "RISCVAsmBackend.h"
 #include "RISCVMCExpr.h"
 #include "llvm/ADT/APInt.h"
+#include "llvm/MC/MCAsmLayout.h"
 #include "llvm/MC/MCAssembler.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCDirectives.h"
 #include "llvm/MC/MCELFObjectWriter.h"
 #include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCObjectWriter.h"
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/MC/MCValue.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
 
 // If linker relaxation is enabled, or the relax option had previously been
 // enabled, always emit relocations even if the fixup can be resolved. This is
 // necessary for correctness as offsets may change during relaxation.
 bool RISCVAsmBackend::shouldForceRelocation(const MCAssembler &Asm,
                                             const MCFixup &Fixup,
                                             const MCValue &Target) {
-  bool ShouldForce = false;
-
   switch (Fixup.getTargetKind()) {
   default:
     break;
   case FK_Data_1:
   case FK_Data_2:
   case FK_Data_4:
   case FK_Data_8:
     if (Target.isAbsolute())
       return false;
     break;
   case RISCV::fixup_riscv_got_hi20:
   case RISCV::fixup_riscv_tls_got_hi20:
   case RISCV::fixup_riscv_tls_gd_hi20:
     return true;
-  case RISCV::fixup_riscv_pcrel_lo12_i:
-  case RISCV::fixup_riscv_pcrel_lo12_s:
-    // For pcrel_lo12, force a relocation if the target of the corresponding
-    // pcrel_hi20 is not in the same fragment.
-    const MCFixup *T = cast<RISCVMCExpr>(Fixup.getValue())->getPCRelHiFixup();
-    if (!T) {
-      Asm.getContext().reportError(Fixup.getLoc(),
-                                   "could not find corresponding %pcrel_hi");
-      return false;
-    }
 
-    switch (T->getTargetKind()) {
-    default:
-      llvm_unreachable("Unexpected fixup kind for pcrel_lo12");
-      break;
-    case RISCV::fixup_riscv_got_hi20:
-    case RISCV::fixup_riscv_tls_got_hi20:
-    case RISCV::fixup_riscv_tls_gd_hi20:
-      ShouldForce = true;
-      break;
-    case RISCV::fixup_riscv_pcrel_hi20: {
-      MCFragment *TFragment = T->getValue()->findAssociatedFragment();
-      MCFragment *FixupFragment = Fixup.getValue()->findAssociatedFragment();
-      assert(FixupFragment && "We should have a fragment for this fixup");
-      ShouldForce =
-          !TFragment || TFragment->getParent() != FixupFragment->getParent();
-      break;
-    }
-    }
-    break;
-  }
-
-  return ShouldForce || STI.getFeatureBits()[RISCV::FeatureRelax] ||
-         ForceRelocs;
+  return STI.getFeatureBits()[RISCV::FeatureRelax] || ForceRelocs;
 }
 
 bool RISCVAsmBackend::fixupNeedsRelaxationAdvanced(const MCFixup &Fixup,
                                                    bool Resolved,
                                                    uint64_t Value,
                                                    const MCRelaxableFragment *DF,
                                                    const MCAsmLayout &Layout,
                                                    const bool WasForced) const {
case RISCV::fixup_riscv_rvc_branch: {
     Value = (Bit8 << 12) | (Bit4_3 << 10) | (Bit7_6 << 5) | (Bit2_1 << 3) |
             (Bit5 << 2);
     return Value;
   }
 
   }
 }
 
+bool RISCVAsmBackend::evaluateTargetFixup(
+    const MCAssembler &Asm, const MCAsmLayout &Layout, const MCFixup &Fixup,
+    const MCFragment *DF, MCValue &Target, uint64_t &Value, bool &WasForced) {
+  const MCFixup *AUIPCFixup;
+  const MCFragment *AUIPCDF;
+  MCValue AUIPCTarget;
+  switch (Fixup.getTargetKind()) {
+  default:
+    llvm_unreachable("Unexpected fixup kind!");
+  case RISCV::fixup_riscv_pcrel_hi20:
+    AUIPCFixup = &Fixup;
+    AUIPCDF = DF;
+    AUIPCTarget = Target;
+    break;
+  case RISCV::fixup_riscv_pcrel_lo12_i:
+  case RISCV::fixup_riscv_pcrel_lo12_s: {
+    AUIPCFixup = cast<RISCVMCExpr>(Fixup.getValue())->getPCRelHiFixup(&AUIPCDF);
+    if (!AUIPCFixup) {
+      Asm.getContext().reportError(Fixup.getLoc(),
+                                   "could not find corresponding %pcrel_hi");
+      return true;
+    }
+
+    const MCExpr *AUIPCExpr = AUIPCFixup->getValue();
+    if (!AUIPCExpr->evaluateAsRelocatable(AUIPCTarget, &Layout, AUIPCFixup))
+      return true;
+
+    if (const MCSymbolRefExpr *RefB = Target.getSymB()) {
+      if (RefB->getKind() != MCSymbolRefExpr::VK_None)
+        return true;
+    }
+    break;
+  }
+  }
+
+  switch (AUIPCFixup->getTargetKind()) {
+  default:
+    llvm_unreachable("Unexpected %pcrel_hi fixup kind");
+    break;
+  case RISCV::fixup_riscv_got_hi20:
+  case RISCV::fixup_riscv_tls_got_hi20:
+  case RISCV::fixup_riscv_tls_gd_hi20:
+    WasForced = true;
+    return false;
+  case RISCV::fixup_riscv_pcrel_hi20:
+    break;
+  }
+
+  if (!AUIPCTarget.getSymA() || AUIPCTarget.getSymB())
+    return false;
+
+  const MCSymbolRefExpr *A = AUIPCTarget.getSymA();
+  const MCSymbol &SA = A->getSymbol();
+  if (A->getKind() != MCSymbolRefExpr::VK_None || SA.isUndefined())
+    return false;
+
+  auto *Writer = Asm.getWriterPtr();
+  if (!Writer)
+    return false;
+
+  bool IsResolved = Writer->isSymbolRefDifferenceFullyResolvedImpl(
+      Asm, SA, *AUIPCDF, false, true);
+  if (!IsResolved)
+    return false;
+
+  Value = Layout.getSymbolOffset(SA) + AUIPCTarget.getConstant();
+  Value -= Layout.getFragmentOffset(AUIPCDF) + AUIPCFixup->getOffset();
+
+  if (shouldForceRelocation(Asm, Fixup, Target)) {
+    WasForced = true;
+    return false;
+  }
+
+  return true;
+}
+
 void RISCVAsmBackend::applyFixup(const MCAssembler &Asm, const MCFixup &Fixup,
                                  const MCValue &Target,
                                  MutableArrayRef<char> Data, uint64_t Value,
                                  bool IsResolved,
                                  const MCSubtargetInfo *STI) const {
   MCContext &Ctx = Asm.getContext();
   MCFixupKindInfo Info = getFixupKindInfo(Fixup.getKind());
   if (!Value)

llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.h

public:
   };
 
 private:
   const MCExpr *Expr;
   const VariantKind Kind;
 
   int64_t evaluateAsInt64(int64_t Value) const;
 
-  bool evaluatePCRelLo(MCValue &Res, const MCAsmLayout *Layout,
-                       const MCFixup *Fixup) const;
-
   explicit RISCVMCExpr(const MCExpr *Expr, VariantKind Kind)
       : Expr(Expr), Kind(Kind) {}
 
 public:
   static const RISCVMCExpr *create(const MCExpr *Expr, VariantKind Kind,
                                    MCContext &Ctx);
 
   VariantKind getKind() const { return Kind; }
 
   const MCExpr *getSubExpr() const { return Expr; }
 
   /// Get the corresponding PC-relative HI fixup that a VK_RISCV_PCREL_LO
-  /// points to.
+  /// points to, and optionally the fragment containing it.
   ///
   /// \returns nullptr if this isn't a VK_RISCV_PCREL_LO pointing to a
   /// known PC-relative HI fixup.
-  const MCFixup *getPCRelHiFixup() const;
+  const MCFixup *getPCRelHiFixup(const MCFragment **DFOut) const;
 
   void printImpl(raw_ostream &OS, const MCAsmInfo *MAI) const override;
   bool evaluateAsRelocatableImpl(MCValue &Res, const MCAsmLayout *Layout,
                                  const MCFixup *Fixup) const override;
   void visitUsedExpr(MCStreamer &Streamer) const override;
   MCFragment *findAssociatedFragment() const override {
     return getSubExpr()->findAssociatedFragment();
   }

llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp

if (HasVariant)
     OS << '%' << getVariantKindName(getKind()) << '(';
   Expr->print(OS, MAI);
   if (Kind == VK_RISCV_CALL_PLT)
     OS << "@plt";
   if (HasVariant)
     OS << ')';
 }
 
-const MCFixup *RISCVMCExpr::getPCRelHiFixup() const {
+const MCFixup *RISCVMCExpr::getPCRelHiFixup(const MCFragment **DFOut) const {
   MCValue AUIPCLoc;
   if (!getSubExpr()->evaluateAsRelocatable(AUIPCLoc, nullptr, nullptr))
     return nullptr;
 
   const MCSymbolRefExpr *AUIPCSRE = AUIPCLoc.getSymA();
   if (!AUIPCSRE)
     return nullptr;
 
for (const MCFixup &F : DF->getFixups()) {
 
     switch ((unsigned)F.getKind()) {
     default:
       continue;
     case RISCV::fixup_riscv_got_hi20:
     case RISCV::fixup_riscv_tls_got_hi20:
     case RISCV::fixup_riscv_tls_gd_hi20:
     case RISCV::fixup_riscv_pcrel_hi20:
+      if (DFOut)
+        *DFOut = DF;
       return &F;
     }
   }
 
   return nullptr;
 }
 
-bool RISCVMCExpr::evaluatePCRelLo(MCValue &Res, const MCAsmLayout *Layout,
-                                  const MCFixup *Fixup) const {
-  // VK_RISCV_PCREL_LO has to be handled specially.  The MCExpr inside is
-  // actually the location of a auipc instruction with a VK_RISCV_PCREL_HI fixup
-  // pointing to the real target.  We need to generate an MCValue in the form of
-  // (<real target> + <offset from this fixup to the auipc fixup>).  The Fixup
-  // is pcrel relative to the VK_RISCV_PCREL_LO fixup, so we need to add the
-  // offset to the VK_RISCV_PCREL_HI Fixup from VK_RISCV_PCREL_LO to correct.
-
-  // Don't try to evaluate if the fixup will be forced as a relocation (e.g.
-  // as linker relaxation is enabled). If we evaluated pcrel_lo in this case,
-  // the modified fixup will be converted into a relocation that no longer
-  // points to the pcrel_hi as the linker requires.
-  auto &RAB =
-      static_cast<RISCVAsmBackend &>(Layout->getAssembler().getBackend());
-  if (RAB.willForceRelocations())
-    return false;
-
-  MCValue AUIPCLoc;
-  if (!getSubExpr()->evaluateAsValue(AUIPCLoc, *Layout))
-    return false;
-
-  const MCSymbolRefExpr *AUIPCSRE = AUIPCLoc.getSymA();
-  // Don't try to evaluate %pcrel_hi/%pcrel_lo pairs that cross fragment
-  // boundries.
-  if (!AUIPCSRE ||
-      findAssociatedFragment() != AUIPCSRE->findAssociatedFragment())
-    return false;
-
-  const MCSymbol *AUIPCSymbol = &AUIPCSRE->getSymbol();
-  if (!AUIPCSymbol)
-    return false;
-
-  const MCFixup *TargetFixup = getPCRelHiFixup();
-  if (!TargetFixup)
-    return false;
-
-  if ((unsigned)TargetFixup->getKind() != RISCV::fixup_riscv_pcrel_hi20)
-    return false;
-
-  MCValue Target;
-  if (!TargetFixup->getValue()->evaluateAsValue(Target, *Layout))
-    return false;
-
-  if (!Target.getSymA() || !Target.getSymA()->getSymbol().isInSection())
-    return false;
-
-  if (&Target.getSymA()->getSymbol().getSection() !=
-      findAssociatedFragment()->getParent())
-    return false;
-
-  // We must use TargetFixup rather than AUIPCSymbol here. They will almost
-  // always have the same offset, except for the case when AUIPCSymbol is at
-  // the end of a fragment and the fixup comes from offset 0 in the next
-  // fragment.
-  uint64_t AUIPCOffset = TargetFixup->getOffset();
-
-  Res = MCValue::get(Target.getSymA(), nullptr,
-                     Target.getConstant() + (Fixup->getOffset() - AUIPCOffset));
-  return true;
-}
-
 bool RISCVMCExpr::evaluateAsRelocatableImpl(MCValue &Res,
                                             const MCAsmLayout *Layout,
                                             const MCFixup *Fixup) const {
-  if (Kind == VK_RISCV_PCREL_LO && evaluatePCRelLo(Res, Layout, Fixup))
-    return true;
-
   if (!getSubExpr()->evaluateAsRelocatable(Res, Layout, Fixup))
     return false;
 
   // Some custom fixup types are not valid with symbol difference expressions
   if (Res.getSymA() && Res.getSymB()) {
     switch (getKind()) {
     default:
       return true;

llvm/test/MC/RISCV/pcrel-fixups.s

 # RUN: llvm-mc -triple riscv32 -mattr=-relax -filetype obj %s \
 # RUN:    | llvm-objdump -M no-aliases -d -r - \
 # RUN:    | FileCheck --check-prefix NORELAX %s
 # RUN: llvm-mc -triple riscv32 -mattr=+relax -filetype obj %s \
 # RUN:    | llvm-objdump -M no-aliases -d -r - \
 # RUN:    | FileCheck --check-prefix RELAX %s
 # RUN: llvm-mc -triple riscv64 -mattr=-relax -filetype obj %s \
 # RUN:    | llvm-objdump -M no-aliases -d -r - \
 # RUN:    | FileCheck --check-prefix NORELAX %s
 # RUN: llvm-mc -triple riscv64 -mattr=+relax -filetype obj %s \
 # RUN:    | llvm-objdump -M no-aliases -d -r - \
 # RUN:    | FileCheck --check-prefix RELAX %s
 
 # Fixups for %pcrel_hi / %pcrel_lo can be evaluated within a section,
-# regardless of the fragment containing the target address.
+# regardless of the fragment containing the target address, provided symbol
+# binding allows it.
 
 function:
 .Lpcrel_label1:
-  auipc  a0, %pcrel_hi(other_function)
+  auipc  a0, %pcrel_hi(local_function)
   addi  a1, a0, %pcrel_lo(.Lpcrel_label1)
 # NORELAX: auipc  a0, 0
 # NORELAX-NOT: R_RISCV
 # NORELAX: addi a1, a0, 16
 # NORELAX-NOT: R_RISCV
 
 # RELAX: auipc  a0, 0
-# RELAX: R_RISCV_PCREL_HI20 other_function
+# RELAX: R_RISCV_PCREL_HI20 local_function
 # RELAX: R_RISCV_RELAX  *ABS*
 # RELAX: addi a1, a0, 0
 # RELAX: R_RISCV_PCREL_LO12_I .Lpcrel_label1
 # RELAX: R_RISCV_RELAX  *ABS*
 
   .p2align  2   # Cause a new fragment be emitted here
 .Lpcrel_label2:
-  auipc  a0, %pcrel_hi(other_function)
+  auipc  a0, %pcrel_hi(local_function)
   addi  a1, a0, %pcrel_lo(.Lpcrel_label2)
 # NORELAX: auipc  a0, 0
 # NORELAX-NOT: R_RISCV
 # NORELAX: addi a1, a0, 8
 # NORELAX-NOT: R_RISCV
 
 # RELAX: auipc  a0, 0
-# RELAX: R_RISCV_PCREL_HI20 other_function
+# RELAX: R_RISCV_PCREL_HI20 local_function
 # RELAX: R_RISCV_RELAX  *ABS*
 # RELAX: addi a1, a0, 0
 # RELAX: R_RISCV_PCREL_LO12_I .Lpcrel_label2
 # RELAX: R_RISCV_RELAX  *ABS*
 
-  .type other_function,@function
-other_function:
+  .type local_function,@function
+local_function:
   ret
 
+# Check handling of symbol binding.
+
+.Lpcrel_label3:
+  auipc a0, %pcrel_hi(global_function)
+  addi  a1, a0, %pcrel_lo(.Lpcrel_label3)
+# NORELAX: auipc  a0, 0
+# NORELAX: R_RISCV_PCREL_HI20 global_function
+# NORELAX: addi a1, a0, 0
+# NORELAX: R_RISCV_PCREL_LO12_I .Lpcrel_label3
+
+# RELAX: auipc  a0, 0
+# RELAX: R_RISCV_PCREL_HI20 global_function
+# RELAX: R_RISCV_RELAX  *ABS*
+# RELAX: addi a1, a0, 0
+# RELAX: R_RISCV_PCREL_LO12_I .Lpcrel_label3
+# RELAX: R_RISCV_RELAX  *ABS*
+
+.Lpcrel_label4:
+  auipc a0, %pcrel_hi(weak_function)
+  addi  a1, a0, %pcrel_lo(.Lpcrel_label4)
+# NORELAX: auipc  a0, 0
+# NORELAX: R_RISCV_PCREL_HI20 weak_function
+# NORELAX: addi a1, a0, 0
+# NORELAX: R_RISCV_PCREL_LO12_I .Lpcrel_label4
+
+# RELAX: auipc  a0, 0
+# RELAX: R_RISCV_PCREL_HI20 weak_function
+# RELAX: R_RISCV_RELAX  *ABS*
+# RELAX: addi a1, a0, 0
+# RELAX: R_RISCV_PCREL_LO12_I .Lpcrel_label4
+# RELAX: R_RISCV_RELAX  *ABS*
+
+  .weak global_function
+  .type global_function,@function
+global_function:
+  ret
+
+  .weak weak_function
+  .type weak_function,@function
+weak_function:
+  ret

llvm/test/MC/RISCV/pcrel-lo12-invalid.s

 # RUN: not llvm-mc -triple riscv32 -mattr=-relax -filetype obj < %s -o /dev/null 2>&1 | FileCheck %s
 # RUN: not llvm-mc -triple riscv32 -mattr=+relax -filetype obj < %s -o /dev/null 2>&1 | FileCheck %s
 
 1:
   addi a0, a0, %pcrel_lo(1b) # CHECK: :[[@LINE]]:3: error: could not find corresponding %pcrel_hi
+  addi a0, a0, %pcrel_lo(0x123456) # CHECK: :[[@LINE]]:3: error: could not find corresponding %pcrel_hi
+  addi a0, a0, %pcrel_lo(foo) # CHECK: :[[@LINE]]:3: error: could not find corresponding %pcrel_hi

llvm/test/MC/RISCV/rv32i-aliases-valid.s

 # RUN: llvm-mc %s -triple=riscv32 -riscv-no-aliases \
 # RUN:     | FileCheck -check-prefixes=CHECK-EXPAND,CHECK-INST %s
 # RUN: llvm-mc %s -triple=riscv32 \
 # RUN:     | FileCheck -check-prefixes=CHECK-EXPAND,CHECK-ALIAS %s
 # RUN: llvm-mc -filetype=obj -triple riscv32 < %s \
 # RUN:     | llvm-objdump -M no-aliases -d -r - \
 # RUN:     | FileCheck -check-prefixes=CHECK-OBJ-NOALIAS,CHECK-EXPAND,CHECK-INST %s
 # RUN: llvm-mc -filetype=obj -triple riscv32 < %s \
 # RUN:     | llvm-objdump -d -r - \
 # RUN:     | FileCheck -check-prefixes=CHECK-EXPAND,CHECK-ALIAS %s
 
 # The following check prefixes are used in this test:
 # CHECK-INST.....Match the canonical instr (tests alias to instr. mapping)
 # CHECK-ALIAS....Match the alias (tests instr. to alias mapping)
 # CHECK-EXPAND...Match canonical instr. unconditionally (tests alias expansion)
 
+# Needed for testing valid %pcrel_lo expressions
+.Lpcrel_hi0: auipc a0, %pcrel_hi(foo)
 
 # CHECK-INST: addi a0, zero, 0
 # CHECK-ALIAS: mv a0, zero
 li x10, 0
 # CHECK-EXPAND: addi a0, zero, 1
 li x10, 1
 # CHECK-EXPAND: addi a0, zero, -1
 li x10, -1

 
 # CHECK-EXPAND: lui a2, 524288
 li x12, 0x80000000
 # CHECK-EXPAND: addi a2, zero, -1
 li x12, 0xFFFFFFFF
 
 # CHECK-EXPAND: addi a0, zero, 1110
 li a0, %lo(0x123456)
-# CHECK-OBJ-NOALIAS: addi a0, zero, 0
-# CHECK-OBJ: R_RISCV_PCREL_LO12
-li a0, %pcrel_lo(0x123456)
 
 # CHECK-OBJ-NOALIAS: addi a0, zero, 0
 # CHECK-OBJ: R_RISCV_LO12
 li a0, %lo(foo)
 # CHECK-OBJ-NOALIAS: addi a0, zero, 0
 # CHECK-OBJ: R_RISCV_PCREL_LO12
-li a0, %pcrel_lo(foo)
+li a0, %pcrel_lo(.Lpcrel_hi0)
 
 .equ CONST, 0x123456
 # CHECK-EXPAND: lui a0, 291
 # CHECK-EXPAND: addi a0, a0, 1110
 li a0, CONST
 # CHECK-EXPAND: lui a0, 291
 # CHECK-EXPAND: addi a0, a0, 1111
 li a0, CONST+1

llvm/test/MC/RISCV/rv32i-valid.s

 # RUN: llvm-mc %s -triple=riscv32 -riscv-no-aliases -show-encoding \
 # RUN:     | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
 # RUN: llvm-mc %s -triple riscv64 -riscv-no-aliases -show-encoding \
 # RUN:     | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
 # RUN: llvm-mc -filetype=obj -triple=riscv32 < %s \
 # RUN:     | llvm-objdump -M no-aliases -d -r - \
 # RUN:     | FileCheck -check-prefixes=CHECK-OBJ,CHECK-ASM-AND-OBJ %s
 # RUN: llvm-mc -filetype=obj -triple=riscv64 < %s \
 # RUN:     | llvm-objdump -M no-aliases -d -r - \
 # RUN:     | FileCheck -check-prefixes=CHECK-OBJ,CHECK-ASM-AND-OBJ %s
 
 .equ CONST, 30
 
+# Needed for testing valid %pcrel_lo expressions
+.Lpcrel_hi0: auipc a0, %pcrel_hi(foo)
+
 # CHECK-ASM-AND-OBJ: lui a0, 2
 # CHECK-ASM: encoding: [0x37,0x25,0x00,0x00]
 lui a0, 2
 # CHECK-ASM-AND-OBJ: lui s11, 552960
 # CHECK-ASM: encoding: [0xb7,0x0d,0x00,0x87]
 lui s11, (0x87000000>>12)
 # CHECK-ASM-AND-OBJ: lui a0, 0
 # CHECK-ASM: encoding: [0x37,0x05,0x00,0x00]

 # CHECK-ASM-AND-OBJ: lw a0, 97(a2)
 # CHECK-ASM: encoding: [0x03,0x25,0x16,0x06]
 lw a0, 97(a2)
 # CHECK-ASM: lbu s5, %lo(foo)(s6)
 # CHECK-ASM: encoding: [0x83,0x4a,0bAAAA1011,A]
 # CHECK-OBJ: lbu s5, 0(s6)
 # CHECK-OBJ: R_RISCV_LO12
 lbu s5, %lo(foo)(s6)
-# CHECK-ASM: lhu t3, %pcrel_lo(foo)(t3)
+# CHECK-ASM: lhu t3, %pcrel_lo(.Lpcrel_hi0)(t3)
 # CHECK-ASM: encoding: [0x03,0x5e,0bAAAA1110,A]
 # CHECK-OBJ: lhu t3, 0(t3)
 # CHECK-OBJ: R_RISCV_PCREL_LO12
-lhu t3, %pcrel_lo(foo)(t3)
+lhu t3, %pcrel_lo(.Lpcrel_hi0)(t3)
 # CHECK-ASM-AND-OBJ: lb t0, 30(t1)
 # CHECK-ASM: encoding: [0x83,0x02,0xe3,0x01]
 lb t0, CONST(t1)
 # CHECK-ASM-AND-OBJ: lb s0, 0(s1)
 # CHECK-ASM: encoding: [0x03,0x84,0x04,0x00]
 lb s0, (0)(s1)
 # CHECK-ASM-AND-OBJ: lb s0, 156(s1)
 # CHECK-ASM: encoding: [0x03,0x84,0xc4,0x09]

llvm/test/MC/RISCV/rv64i-aliases-valid.s

 # The following check prefixes are used in this test:
 # CHECK-INST.....Match the canonical instr (tests alias to instr. mapping)
 # CHECK-ALIAS....Match the alias (tests instr. to alias mapping)
 # CHECK-EXPAND...Match canonical instr. unconditionally (tests alias expansion)
 
 # TODO ld
 # TODO sd
 
+# Needed for testing valid %pcrel_lo expressions
+.Lpcrel_hi0: auipc a0, %pcrel_hi(foo)
+
 # CHECK-INST: addi a0, zero, 0
 # CHECK-ALIAS: mv a0, zero
 li x10, 0
 # CHECK-EXPAND: addi a0, zero, 1
 li x10, 1
 # CHECK-EXPAND: addi a0, zero, -1
 li x10, -1
 # CHECK-EXPAND: addi a0, zero, 2047

 # CHECK-EXPAND: slli t4, t4, 13
 # CHECK-EXPAND: addi t4, t4, -272
 li t4, 0x123456789abcdef0
 # CHECK-EXPAND: addi t5, zero, -1
 li t5, 0xFFFFFFFFFFFFFFFF
 
 # CHECK-EXPAND: addi a0, zero, 1110
 li a0, %lo(0x123456)
-# CHECK-OBJ-NOALIAS: addi a0, zero, 0
-# CHECK-OBJ: R_RISCV_PCREL_LO12
-li a0, %pcrel_lo(0x123456)
 
 # CHECK-OBJ-NOALIAS: addi a0, zero, 0
 # CHECK-OBJ: R_RISCV_LO12
 li a0, %lo(foo)
 # CHECK-OBJ-NOALIAS: addi a0, zero, 0
 # CHECK-OBJ: R_RISCV_PCREL_LO12
-li a0, %pcrel_lo(foo)
+li a0, %pcrel_lo(.Lpcrel_hi0)
 
 .equ CONST, 0x123456
 # CHECK-EXPAND: lui a0, 291
 # CHECK-EXPAND: addiw a0, a0, 1110
 li a0, CONST
 
 .equ CONST, 0x654321
 # CHECK-EXPAND: lui a0, 1620